[PATCH] D74669: [clang-tidy] New check: misc-no-include-cpp

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:11-13
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
----------------
You don't need to include or use the AST matchers in a preprocessor only check.


================
Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:50
+
+  (void)FilenameRange;
+  (void)File;
----------------
I have a feeling this is to suppress some unused parameter linting check. If it is then it should be removed as unused parameters in an overridden function shouldn't emit a warning.
Same for below.

Side note: File a bug with whichever linter tool gave you that warning (if there was one).



================
Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp:62
+      Check.diag(HashLoc, "suspicious #include")
+          << FixItHint::CreateReplacement(FilenameRange,
+                                          ((IsAngled ? "<" : "\"") + FileName +
----------------
jroelofs wrote:
> njames93 wrote:
> > This replacement is dangerous, I have a feeling no fix-it should be provided or at least do a search of the include directories to see if file you are trying to include actually does exist. The correct file could be `*.hpp` like what boost uses for all its header files
> Yeah, perhaps the FixIt should only be added if there is a single candidate replacement that exists on the `-I` path.
> 
> Another option is to not add FixIts at all, and instead emit a list of `note:`s suggesting each of the candidates.
How about the case if someone wants to (for whatever reason) include something like this:

```
#include "example.h"
#include "example.cpp"
```
That looks intentional and a fix it shouldn't be emitted.


================
Comment at: clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.h:31
+      : ClangTidyCheck(Name, Context) {}
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP);
----------------
This should be marked `override`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74669