[PATCH] D152589: [clang-tidy] Add readability test for not allowing relative includes

[Erez Amihud via Phabricator via cfe-commits]

ErezAmihud abandoned this revision.
ErezAmihud added a comment.

Closed, because I don't have time to work on it


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152589/new/

https://reviews.llvm.org/D152589

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152589: Add readability test for not allowing relative includes

[Erez Amihud via Phabricator via cfe-commits]

ErezAmihud created this revision.
ErezAmihud added reviewers: clang-tools-extra, njames93.
ErezAmihud created this object with edit policy "Administrators".
ErezAmihud added a project: clang-tools-extra.
Herald added subscribers: PiotrZSL, carlosgalvezp.
Herald added a project: All.
ErezAmihud requested review of this revision.
Herald added a subscriber: cfe-commits.

Hi, it would be nice to add a check that warn if it finds a relative include. 
This is to allow people to easily conform to the canonical project structure - 
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1204r0.html

The check issue a warning for every relative include (e.g. any include that 
uses regular quotes).

Github issue:
https://github.com/llvm/llvm-project/issues/63226


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152589

Files:
  clang-tools-extra/clang-tidy/readability/AbsoluteIncludesOnlyCheck.cpp
  clang-tools-extra/clang-tidy/readability/AbsoluteIncludesOnlyCheck.h
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/absolute-includes-only.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/absolute-includes-only.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/absolute-includes-only.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/absolute-includes-only.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s readability-absolute-includes-only %t -- -- -isystem %clang_tidy_headers -fmodules
+
+// clang-format off
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: relative include found, use only absolute includes
+#include "j.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: relative include found, use only absolute includes
+#include "llvm/a.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: relative include found, use only absolute includes
+#include "clang/../b.h"
+
+
+#include 
+
+#include 
+
Index: clang-tools-extra/docs/clang-tidy/checks/readability/absolute-includes-only.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/absolute-includes-only.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - readability-absolute-includes-only
+
+readability-absolute-includes-only
+==
+
+Finds relative includes inside of the code and warn about them.
+
+Meaning this check disallow the use of quote includes (""), and allow only angular brackets includes (<>).
+
+This is relevant for the canonical project structure specified in paper p1204r0.
+The relevant part is the src-dir where relative includes are discussed: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1204r0.html#src-dir
+
+Example (taken from the paper):
+
+.. code-block:: c++
+
+  // #include "utility.hpp"   // Wrong.
+  // #include// Wrong.
+  // #include "../hello/utility.hpp"  // Wrong.
+
+  #include 
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -337,6 +337,7 @@
`portability-restrict-system-includes `_, "Yes"
`portability-simd-intrinsics `_,
`portability-std-allocator-const `_,
+   `readability-absolute-includes-only `_,
`readability-avoid-const-params-in-decls `_, "Yes"
`readability-avoid-unconditional-preprocessor-if `_,
`readability-braces-around-statements `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -181,6 +181,12 @@
 
   Finds uses of ``std::endl`` on streams and replaces them with ``'\n'``.
 
+- New :doc:`readability-absolute-includes-only
+  ` check.
+
+Finds relative includes in your code and warn about them. for example
+don't use "readability/absolute-includes-only", use 
+
 - New :doc:`readability-avoid-unconditional-preprocessor-if
   ` check.
 
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AbsoluteIncludesOnlyCheck.h"
 #include "AvoidConstParamsInDecls.h"
 #include "AvoidUnconditionalPreprocessorIfCheck.h"
 #include "BracesAroundStatementsCheck.h"
@@ -60,6 +61,8 @@
 class ReadabilityModule : public ClangTidyModul

[PATCH] D152589: Add readability test for not allowing relative includes

[Erez Amihud via Phabricator via cfe-commits]

ErezAmihud updated this revision to Diff 530102.
ErezAmihud added a comment.

Remove a line in absolute-includes-only.cpp (fix formatting)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152589/new/

https://reviews.llvm.org/D152589

Files:
  clang-tools-extra/clang-tidy/readability/AbsoluteIncludesOnlyCheck.cpp
  clang-tools-extra/clang-tidy/readability/AbsoluteIncludesOnlyCheck.h
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/absolute-includes-only.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/absolute-includes-only.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/absolute-includes-only.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/absolute-includes-only.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s readability-absolute-includes-only %t -- -- -isystem %clang_tidy_headers -fmodules
+
+// clang-format off
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: relative include found, use only absolute includes
+#include "j.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: relative include found, use only absolute includes
+#include "llvm/a.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:10: warning: relative include found, use only absolute includes
+#include "clang/../b.h"
+
+
+#include 
+
+#include 
+
Index: clang-tools-extra/docs/clang-tidy/checks/readability/absolute-includes-only.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/readability/absolute-includes-only.rst
@@ -0,0 +1,21 @@
+.. title:: clang-tidy - readability-absolute-includes-only
+
+readability-absolute-includes-only
+==
+
+Finds relative includes inside of the code and warn about them.
+
+Meaning this check disallow the use of quote includes (""), and allow only angular brackets includes (<>).
+
+This is relevant for the canonical project structure specified in paper p1204r0.
+The relevant part is the src-dir where relative includes are discussed: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1204r0.html#src-dir
+
+Example (taken from the paper):
+
+.. code-block:: c++
+
+  // #include "utility.hpp"   // Wrong.
+  // #include// Wrong.
+  // #include "../hello/utility.hpp"  // Wrong.
+
+  #include 
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -337,6 +337,7 @@
`portability-restrict-system-includes `_, "Yes"
`portability-simd-intrinsics `_,
`portability-std-allocator-const `_,
+   `readability-absolute-includes-only `_,
`readability-avoid-const-params-in-decls `_, "Yes"
`readability-avoid-unconditional-preprocessor-if `_,
`readability-braces-around-statements `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -181,6 +181,12 @@
 
   Finds uses of ``std::endl`` on streams and replaces them with ``'\n'``.
 
+- New :doc:`readability-absolute-includes-only
+  ` check.
+
+Finds relative includes in your code and warn about them. for example
+don't use "readability/absolute-includes-only", use 
+
 - New :doc:`readability-avoid-unconditional-preprocessor-if
   ` check.
 
Index: clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -9,6 +9,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "AbsoluteIncludesOnlyCheck.h"
 #include "AvoidConstParamsInDecls.h"
 #include "AvoidUnconditionalPreprocessorIfCheck.h"
 #include "BracesAroundStatementsCheck.h"
@@ -60,6 +61,8 @@
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+CheckFactories.registerCheck(
+"readability-absolute-includes-only");
 CheckFactories.registerCheck(
 "readability-avoid-const-params-in-decls");
 CheckFactories.registerCheck(
Index: clang-tools-extra/clang-tidy/readability/CMakeLists.txt
===
--- clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_library(clangTid

[PATCH] D152589: [clang-tidy] Add readability test for not allowing relative includes

[Erez Amihud via Phabricator via cfe-commits]

ErezAmihud added a comment.

In D152589#4410930 , @PiotrZSL wrote:

> Even with that for me for example, I would consider `#include "xyz"` be fine, 
> but `#include "../xyz"` would be a red flag, so please consider adding 
> `StrictMode` option to check so if `StrictMode` is not set then includes from 
> same directory, or subdirectory would be allowed, but includes that contain 
> `..` would not.
> With that settings, I could use that check, and probably many other users 
> could.

I want to make sure I understand.
The ideal situation would be to create `misc-no-relative-includes` check that 
checks for `..` in paths (meaning - relative paths), and have a `StrictMode` 
which does the check for angled-bracket includes.

BTW The reason I initially checked for angled-bracket includes is that 
according to this  quote 
includes search in paths relative to the current file, and the angled-bracket 
includes check only in the defined include directories.
In this case there is no risk that the include would get a file that is not 
relative to the include directory the user specified in the compile flags.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152589/new/

https://reviews.llvm.org/D152589

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits