[clang-tidy] New check: bugprone-suspicious-include

[jroelofs]

Summary

Detects and fixes suspicious code like: #include "foo.cpp".

Inspired by: https://twitter.com/lefticus/status/1228458240364687360?s=20

[Eugene.Zelenko]

How about .cc and .cxx extension? It'll be reasonable to provide option to configure list of extensions.

Please add documentation and mention new check in Release Notes.

clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp

49 ↗

(On Diff #244829)

Unnecessary empty line.

[njames93]

I have a feeling this check should be called something along the lines of bugprone-suspicous-include.

clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp

62 ↗

(On Diff #244829)

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

[jroelofs]

In D74669#1878168, @njames93 wrote:

I have a feeling this check should be called something along the lines of bugprone-suspicous-include.

That's a much better name, I like it.

clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp

62 ↗

(On Diff #244829)

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.

[njames93]

clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp

11–13 ↗

(On Diff #244829)

You don't need to include or use the AST matchers in a preprocessor only check.

50 ↗

(On Diff #244829)

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).

62 ↗

(On Diff #244829)

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.

clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.h

31 ↗

(On Diff #244829)

This should be marked override.

[jroelofs]

• Renamed to bugprone-suspicious-include
• Removed FixIts in favor of note: suggestions, iff the suggested file exists on the header search path.
• Removed unnecessary unused parameter casts-to-void.

[jroelofs]

clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp

50 ↗

(On Diff #244829)

Sorry, did this out of habit. Indeed it isn't actually needed.

[jroelofs]

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h

27

FIXME: still need to write docs for this.

[jroelofs]

clang-tools-extra/clang-tidy/misc/NoIncludeCPPCheck.cpp

62 ↗

(On Diff #244829)

I'd imagine that people doing this intentionally (unity builds come to mind) would turn off this check. That said, is there something better this check could be doing to accommodate those people?

[jroelofs]

git format-patch

[jroelofs]

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

60

FIXME: is this a reasonable SourceLocation to give to LookupFile?

[njames93]

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

44

This function is used for #import on modules. Do we still want the same behaviour when importing modules? More thinking about the extension suggestions with that one.

60

Main thing is to make sure that the same location is used for the warnings and the notes.
I personally found this the most pleasing diagnostics

SourceLocation DiagLoc = FilenameRange.getBegin().getLocWithOffset(1);

/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11: warning: suspicious #include of file with .cpp extension [bugprone-suspicious-include]
#include "a.cpp"
          ^
/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11: note: did you mean to include 'a'?
/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11: note: did you mean to include 'a.h'?
/home/nathan/src/llvm-project/build/Release/tools/clang/tools/extra/test/clang-tidy/checkers/Output/bugprone-suspicious-include.cpp.tmp.cpp:7:11: note: did you mean to include 'a.hpp'?

[jroelofs]

Implement review feedback re: DiagLoc

[jroelofs]

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

60

I don't have much experience with modules to lean on for intuition on what this check should do with them. I've added a test to codify what the pass currently does in that case, but if there's something better it should be trying when searching for suggestions, let's make it do that instead.

[jroelofs]

Make the diagnostics more accurate.

[njames93]

I'd suggest possibly adding 2 Options at Global Level SourceFileExtensions and HeaderFileExtensions, both would take semicolon seperated lists.
Reason they are Global is there are probably other checks that could use them.
The SourceFileExtensions could be defaulted to .c;.cc;.cpp;.cxx and the HeaderFileExtensions .h;.hh;.hpp;<empty>.
You need the <empty> because how Options get parsed, then it can manually be transformed to "" when you read it.

[aaron.ballman]

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

51–52

We already have HeaderFileExtensionsUtils.h -- we should be using that for the header file extensions (which should be configurable).

It might make sense to make those utilities be general for other kinds of file extensions as well, such as source files. I think that list should also be configurable.

56

Should probably add ' quotes around the extension to make it more obvious where the extension starts and stops.

[jroelofs]

Implement review feedback:

• Re-purpose HeaderFileExtensionsUtils.h to support headers *and* sources.
• Surround file extension in diagnostic with ''s.

I have these as two separate patches locally, but I don't know how to represent that in phabricator, so I've uploaded the diff across both. If this gets approved, I intend to push them as separate commits.

[njames93]

In D74669#1892577, @jroelofs wrote:

Implement review feedback:

• Re-purpose HeaderFileExtensionsUtils.h to support headers *and* sources.
• Surround file extension in diagnostic with ''s.

I have these as two separate patches locally, but I don't know how to represent that in phabricator, so I've uploaded the diff across both. If this gets approved, I intend to push them as separate commits.

You can create a review that just re-purposes HeaderFileExtensionsUtils then set that review as a parent of this review

[Eugene.Zelenko]

In D74669#1892577, @jroelofs wrote:

Implement review feedback:

• Re-purpose HeaderFileExtensionsUtils.h to support headers *and* sources.
• Surround file extension in diagnostic with ''s.

I have these as two separate patches locally, but I don't know how to represent that in phabricator, so I've uploaded the diff across both. If this gets approved, I intend to push them as separate commits.

You could merge two patches (diff to base version) and update diff in this request.

[aaron.ballman]

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

77

Please don't use auto here (the type isn't spelled out in the initialization).

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp

36

Delimiter here as well.

54

Extension

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h

9

Can drop the HEADER from these macros.

48

delimiter -> Delimiter per our usual naming conventions.

[njames93]

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

45

Follow the convention of the other checks and use HeaderFileExtensions as the option name. Probably call the other option ImplementationFileExtensions

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp

42

This could be changed to

if (!llvm::all_of(Extension, isAlphanumeric))
  return false;

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h

38

A lot of configuration options for clang tidy use semi-colon ; seperated lists. Would it not be wise to carry on the convention and use semi colon here (and below) as well. It could break a few peoples configuration but that would be realised fairly quickly and updated

[jroelofs]

implement review feedback

[njames93]

Adding the parent revision means you don't need to have those changes in this patch. It will cause issues down the line. best thing is to run a diff from this to the parent and just include those changes

[jroelofs]

In D74669#1902107, @njames93 wrote:

Adding the parent revision means you don't need to have those changes in this patch.

IIUC, that is what I've already done:

https://reviews.llvm.org/D74669 is git show c4dd6f5903e -U999
https://reviews.llvm.org/D75489 is git show 0cda7e0b9ed -U999

where c4dd6f5903e is the parent of 0cda7e0b9ed.

The suggestion to switch over to ;s as delimiters is what's causing this diff to still touch so many files.

clang-tools-extra/clang-tidy/bugprone/DynamicStaticInitializersCheck.h

24 ↗

(On Diff #247764)

";h;hh;hpp;hxx"

clang-tools-extra/clang-tidy/google/UnnamedNamespaceInHeaderCheck.h

25 ↗

(On Diff #247764)

";h;hh;hpp;hxx"

[jroelofs]

Split out https://reviews.llvm.org/D75621

[lebedev.ri]

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h

38

I think i'm missing the point of D75621.
Why can't we keep using , as delimitter?
Why do we have to change everything, introduce inconsistency
(is it still documented that , is still valid?),
and spam into terminal if , is found?

[njames93]

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h

38

This reason

➜  ~ clang-tidy test.cpp --config="{CheckOptions: [{ key: HeaderFileExtensions, value: h,,hpp,hxx }]}" -- -std=c++17
YAML:1:59: error: unknown key 'hpp'
{CheckOptions: [{ key: HeaderFileExtensions, value: h,,hpp,hxx }]}
                                                          ^
Error: invalid configuration specified.
Invalid argument
➜  ~ clang-tidy test.cpp --config="{CheckOptions: [{ key: HeaderFileExtensions, value: h;;hpp;hxx }]}" -- -std=c++17
➜  ~

[lebedev.ri]

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h

38

$ clang-tidy test.cpp --config="{CheckOptions: [{ key: HeaderFileExtensions, value: h,,hpp,hxx }]}" -- -std=c++17
YAML:1:59: error: unknown key 'hpp'
{CheckOptions: [{ key: HeaderFileExtensions, value: h,,hpp,hxx }]}
                                                          ^
Error: invalid configuration specified.
Invalid argument
$ clang-tidy test.cpp --config="{CheckOptions: [{ key: HeaderFileExtensions, value: \"h,,hpp,hxx\" }]}" -- -std=c++17
Error while processing /repositories/llvm-project/test.cpp.
error: no input files [clang-diagnostic-error]
error: no such file or directory: '/repositories/llvm-project/test.cpp' [clang-diagnostic-error]
error: unable to handle compilation, expected exactly one compiler job in '' [clang-diagnostic-error]
Found compiler error(s).

[jroelofs]

Fix comment, add docs.

[jroelofs]

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h

9

Missed this one before. Oops.
https://github.com/llvm/llvm-project/commit/c71ef7a85d2447903ff9240aff649a57d70b050d

[aaron.ballman]

LGTM aside from some documentation and testing nits.

clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst

16–18

Missing full stops at the end of the comments.

25

The -> the

26

contain "." -> contain a "."

27

using -> use
leaving -> leave

30

ImplementationFileExtensions ;-)

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

8

For those of you following along at home, a.h already exists on ToT (as does b.h). :-D

11

Can you add a test for:

#include "i.cpp"

to show that it suggests i.h (which already exists in-tree) but not i or i.hpp?

[jroelofs]

Implement review feedback.

[jroelofs]

Add missing release notes.

[jroelofs]

Thanks for the review!

https://github.com/llvm/llvm-project/commit/1e0669bfe05f0f48ee88152c4a1d581f484f8d67

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

11

happy to :)

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

108

Please keep alphabetical order.

111

Please synchronize with first statement in documentation.

clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst

7

Please omit The checker. Clang-tidy uses check in its terminology.

24

Please use single back-ticks for option values. Same above.

[jroelofs]

https://github.com/llvm/llvm-project/commit/52bbdad7d63fd060d102b3591b433d116a982255

[thakis]

Test falls in Windows: http://45.33.8.238/win/10088/step_8.txt

[thakis]

Reverted for now in 714466bf367dfd5062e1850179d27c37a9ec6b46

[jroelofs]

Don't fire on #imports.

The error on Windows suggests it would normally do the "right" thing anyway:

http://45.33.8.238/win/10088/step_8.txt

C:\src\llvm-project\out\gn\obj\clang-tools-extra\test\clang-tidy\checkers\Output\bugprone-suspicious-include.cpp.tmp.cpp:18:2: error: #import of type library is an unsupported Microsoft feature [clang-diagnostic-error]
#import "c.c"

[aaron.ballman]

LGTM with the changes for #import.

[jroelofs]

re-landed in https://github.com/llvm/llvm-project/commit/2c9cf9f4ddd01ae9eb47522266a6343104f9d0b5

[sylvestre.ledru]

Firefox uses a unified build model. For better performances in the binary, the C++ files are compiled as the same time from a single file (ex: Unified_cpp_netwerk_base3.cpp) which will include the .cpp files.

This isn't a common use case, so, don't hesitate to ignore me but checks triggers 4286 new issues. About 20 of them are legit.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "ReservedIdentifierCheck.h"
 #include "SignedCharMisuseCheck.h"
 #include "SizeofContainerCheck.h"
 #include "SizeofExpressionCheck.h"
 #include "StringConstructorCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
 #include "SuspiciousEnumUsageCheck.h"
+#include "SuspiciousIncludeCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
 #include "SuspiciousMissingCommaCheck.h"
 #include "SuspiciousSemicolonCheck.h"
 #include "SuspiciousStringCompareCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "TerminatingContinueCheck.h"
 #include "ThrowKeywordMissingCheck.h"
 #include "TooSmallLoopVariableCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<StringConstructorCheck>(
         "bugprone-string-constructor");
     CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
         "bugprone-string-integer-assignment");
     CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
         "bugprone-string-literal-with-embedded-nul");
     CheckFactories.registerCheck<SuspiciousEnumUsageCheck>(
         "bugprone-suspicious-enum-usage");
+    CheckFactories.registerCheck<SuspiciousIncludeCheck>(
+        "bugprone-suspicious-include");
     CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
         "bugprone-suspicious-memset-usage");
     CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(
         "bugprone-suspicious-missing-comma");
     CheckFactories.registerCheck<SuspiciousSemicolonCheck>(
         "bugprone-suspicious-semicolon");
     CheckFactories.registerCheck<SuspiciousStringCompareCheck>(
         "bugprone-suspicious-string-compare");

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   ReservedIdentifierCheck.cpp
   SignedCharMisuseCheck.cpp
   SizeofContainerCheck.cpp
   SizeofExpressionCheck.cpp
   StringConstructorCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp
   SuspiciousEnumUsageCheck.cpp
+  SuspiciousIncludeCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
   SuspiciousMissingCommaCheck.cpp
   SuspiciousSemicolonCheck.cpp
   SuspiciousStringCompareCheck.cpp
   SwappedArgumentsCheck.cpp
   TerminatingContinueCheck.cpp
   ThrowKeywordMissingCheck.cpp
   TooSmallLoopVariableCheck.cpp

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h

+//===--- SuspiciousIncludeCheck.h - clang-tidy ------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/FileExtensionsUtils.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Warns on inclusion of files whose names suggest that they're implementation
+/// files, instead of headers. E.g:
+///
+///     #include "foo.cpp"  // warning
+///     #include "bar.c"    // warning
+///     #include "baz.h"    // no diagnostic
+///
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+///     extensions of header files (The filename extensions should not contain
+///     "." prefix) ";h;hh;hpp;hxx" by default. For extension-less header
+///     files, using an empty string or leaving an empty string between ";" if
+///     there are other filename extensions.
+///
+///   - `ImplementationFileExtensions`: likewise, a semicolon-separated list of
+///     filename extensions of implementation files. "c;cc;cpp;cxx" by default.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-suspicious-include.html
+class SuspiciousIncludeCheck : public ClangTidyCheck {
+public:
+  SuspiciousIncludeCheck(StringRef Name, ClangTidyContext *Context);
+  void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+                           Preprocessor *ModuleExpanderPP) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+  utils::FileExtensionsSet HeaderFileExtensions;
+  utils::FileExtensionsSet ImplementationFileExtensions;
+
+private:
+  const std::string RawStringHeaderFileExtensions;
+  const std::string RawStringImplementationFileExtensions;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SUSPICIOUSINCLUDECHECK_H

clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp

+//===--- SuspiciousIncludeCheck.cpp - clang-tidy --------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "SuspiciousIncludeCheck.h"
+#include "clang/AST/ASTContext.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+namespace {
+class SuspiciousIncludePPCallbacks : public PPCallbacks {
+public:
+  explicit SuspiciousIncludePPCallbacks(SuspiciousIncludeCheck &Check,
+                                        const SourceManager &SM,
+                                        Preprocessor *PP)
+      : Check(Check), PP(PP) {}
+
+  void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
+                          StringRef FileName, bool IsAngled,
+                          CharSourceRange FilenameRange, const FileEntry *File,
+                          StringRef SearchPath, StringRef RelativePath,
+                          const Module *Imported,
+                          SrcMgr::CharacteristicKind FileType) override;
+
+private:
+  SuspiciousIncludeCheck &Check;
+  Preprocessor *PP;
+};
+} // namespace
+
+SuspiciousIncludeCheck::SuspiciousIncludeCheck(StringRef Name,
+                                               ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+          "HeaderFileExtensions", utils::defaultHeaderFileExtensions())),
+      RawStringImplementationFileExtensions(Options.getLocalOrGlobal(
+          "ImplementationFileExtensions",
+          utils::defaultImplementationFileExtensions())) {
+  if (!utils::parseFileExtensions(RawStringImplementationFileExtensions,
+                                  ImplementationFileExtensions,
+                                  utils::defaultFileExtensionDelimiters())) {
+    llvm::errs() << "Invalid implementation file extension: "
+                 << RawStringImplementationFileExtensions << "\n";
+  }
+
+  if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+                                  HeaderFileExtensions,
+                                  utils::defaultFileExtensionDelimiters())) {
+    llvm::errs() << "Invalid header file extension: "
+                 << RawStringHeaderFileExtensions << "\n";
+  }
+}
+
+void SuspiciousIncludeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "ImplementationFileExtensions",
+                RawStringImplementationFileExtensions);
+  Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+}
+
+void SuspiciousIncludeCheck::registerPPCallbacks(
+    const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+  PP->addPPCallbacks(
+      ::std::make_unique<SuspiciousIncludePPCallbacks>(*this, SM, PP));
+}
+
+void SuspiciousIncludePPCallbacks::InclusionDirective(
+    SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName,
+    bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File,
+    StringRef SearchPath, StringRef RelativePath, const Module *Imported,
+    SrcMgr::CharacteristicKind FileType) {
+  if (IncludeTok.getIdentifierInfo()->getPPKeywordID() == tok::pp_import)
+    return;
+
+  SourceLocation DiagLoc = FilenameRange.getBegin().getLocWithOffset(1);
+
+  const Optional<StringRef> IFE =
+      utils::getFileExtension(FileName, Check.ImplementationFileExtensions);
+  if (!IFE)
+    return;
+
+  Check.diag(DiagLoc, "suspicious #%0 of file with '%1' extension")
+      << IncludeTok.getIdentifierInfo()->getName() << *IFE;
+
+  for (const auto &HFE : Check.HeaderFileExtensions) {
+    SmallString<128> GuessedFileName(FileName);
+    llvm::sys::path::replace_extension(GuessedFileName,
+                                       (HFE.size() ? "." : "") + HFE);
+
+    const DirectoryLookup *CurDir;
+    Optional<FileEntryRef> File =
+        PP->LookupFile(DiagLoc, GuessedFileName, IsAngled, nullptr, nullptr,
+                       CurDir, nullptr, nullptr, nullptr, nullptr, nullptr);
+    if (File) {
+      Check.diag(DiagLoc, "did you mean to include '%0'?", DiagnosticIDs::Note)
+          << GuessedFileName;
+    }
+  }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h

 //===--- FileExtensionsUtils.h - clang-tidy --------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FILE_EXTENSIONS_UTILS_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FILE_EXTENSIONS_UTILS_H
 
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace tidy {
 namespace utils {
 
 typedef llvm::SmallSet<llvm::StringRef, 5> FileExtensionsSet;
 
 /// Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(SourceLocation Loc, const SourceManager &SM,
                                 const FileExtensionsSet &HeaderFileExtensions);
 
 /// Checks whether presumed location of \p Loc is in header file.
 bool isPresumedLocInHeaderFile(SourceLocation Loc, SourceManager &SM,
                                const FileExtensionsSet &HeaderFileExtensions);
 
 /// Checks whether spelling location of \p Loc is in header file.
 bool isSpellingLocInHeaderFile(SourceLocation Loc, SourceManager &SM,
                                const FileExtensionsSet &HeaderFileExtensions);
 
 /// Returns recommended default value for the list of header file
 /// extensions.
 inline StringRef defaultHeaderFileExtensions() { return ";h;hh;hpp;hxx"; }
 
+/// Returns recommended default value for the list of implementaiton file
+/// extensions.
+inline StringRef defaultImplementationFileExtensions() {
+  return "c;cc;cpp;cxx";
+}
+
 /// Returns recommended default value for the list of file extension
 /// delimiters.
 inline StringRef defaultFileExtensionDelimiters() { return ",;"; }
 
 /// Parses header file extensions from a semicolon-separated list.
 bool parseFileExtensions(StringRef AllFileExtensions,
                          FileExtensionsSet &FileExtensions,
                          StringRef Delimiters);
 
+/// Decides whether a file has a header file extension.
+/// Returns the file extension, if included in the provided set.
+llvm::Optional<StringRef>
+getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions);
+
 /// Decides whether a file has one of the specified file extensions.
 bool isFileExtension(StringRef FileName,
                      const FileExtensionsSet &FileExtensions);
 
 } // namespace utils
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_FILE_EXTENSIONS_UTILS_H

clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp

 
 bool isSpellingLocInHeaderFile(SourceLocation Loc, SourceManager &SM,
                                const FileExtensionsSet &HeaderFileExtensions) {
   SourceLocation SpellingLoc = SM.getSpellingLoc(Loc);
   return isFileExtension(SM.getFilename(SpellingLoc), HeaderFileExtensions);
 }
 
 bool parseFileExtensions(StringRef AllFileExtensions,
                          FileExtensionsSet &FileExtensions,
                          StringRef Delimiters) {
   SmallVector<StringRef, 5> Suffixes;
   for (char Delimiter : Delimiters) {
     if (AllFileExtensions.contains(Delimiter)) {
       AllFileExtensions.split(Suffixes, Delimiter);
       break;
     }
   }
 
   FileExtensions.clear();
   for (StringRef Suffix : Suffixes) {
     StringRef Extension = Suffix.trim();
     if (!llvm::all_of(Extension, isAlphanumeric))
       return false;
     FileExtensions.insert(Extension);
   }
   return true;
 }
 
-bool isFileExtension(StringRef FileName,
-                     const FileExtensionsSet &FileExtensions) {
+llvm::Optional<StringRef>
+getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions) {
   StringRef Extension = llvm::sys::path::extension(FileName);
   if (Extension.empty())
-    return false;
+    return llvm::None;
   // Skip "." prefix.
-  return FileExtensions.count(Extension.substr(1)) > 0;
+  if (!FileExtensions.count(Extension.substr(1)))
+    return llvm::None;
+  return Extension;
+}
+
+bool isFileExtension(StringRef FileName,
+                     const FileExtensionsSet &FileExtensions) {
+  return getFileExtension(FileName, FileExtensions).hasValue();
 }
 
 } // namespace utils
 } // namespace tidy
 } // namespace clang

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc
   result of a memory allocation function (``malloc()``, ``calloc()``,
   ``realloc()``, ``alloca()``) instead of its argument.
 
 - New :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` check.
 
   Checks for usages of identifiers reserved for use by the implementation.
 
+- New :doc:`bugprone-suspicious-include
+  <clang-tidy/checks/bugprone-suspicious-include>` check.
+
+  Finds cases where an include refers to what appears to be an implementation
+  file, which often leads to hard-to-track-down ODR violations, and diagnoses
+  them.
+
 - New :doc:`cert-oop57-cpp
   <clang-tidy/checks/cert-oop57-cpp>` check.
 
   Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
   ``memcmp`` and similar derivatives on non-trivial types.
 
 - New :doc:`objc-dealloc-in-category
   <clang-tidy/checks/objc-dealloc-in-category>` check.
 
   Finds implementations of -dealloc in Objective-C categories.
 
 - New :doc:`misc-no-recursion
   <clang-tidy/checks/misc-no-recursion>` check.
 
   Finds recursive functions and diagnoses them.
 
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
 - New alias :doc:`cert-dcl37-c
   <clang-tidy/checks/cert-dcl37-c>` to
   :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` was added.
 
 - New alias :doc:`cert-dcl51-cpp
   <clang-tidy/checks/cert-dcl51-cpp>` to
   :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone-reserved-identifier>` was added.

clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst

+.. title:: clang-tidy - bugprone-suspicious-include
+
+bugprone-suspicious-include
+===========================
+
+The check detects various cases when an include refers to what appears to be an
+implementation file, which often leads to hard-to-track-down ODR violations.
+
+Examples:
+
+.. code-block:: c++
+
+  #include "Dinosaur.hpp"     // OK, .hpp files tend not to have definitions.
+  #include "Pterodactyl.h"    // OK, .h files tend not to have definitions.
+  #include "Velociraptor.cpp" // Warning, filename is suspicious.
+  #include_next <stdio.c>     // Warning, filename is suspicious.
+
+Options
+-------
+.. option:: HeaderFileExtensions
+
+   Default value: `";h;hh;hpp;hxx"`
+   A semicolon-separated list of filename extensions of header files (the
+   filename extensions should not contain a "." prefix). For extension-less
+   header files, use an empty string or leave an empty string between ";"
+   if there are other filename extensions.
+
+.. option:: ImplementationFileExtensions
+
+   Default value: `"c;cc;cpp;cxx"`
+   Likewise, a semicolon-separated list of filename extensions of
+   implementation files.

clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp

+// RUN: %check_clang_tidy %s bugprone-suspicious-include %t -- -- -isystem %S/Inputs/Headers -fmodules
+
+// clang-format off
+
+// CHECK-MESSAGES: [[@LINE+4]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+3]]:11: note: did you mean to include 'a'?
+// CHECK-MESSAGES: [[@LINE+2]]:11: note: did you mean to include 'a.h'?
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'a.hpp'?
+#include "a.cpp"
+
+// CHECK-MESSAGES: [[@LINE+2]]:11: warning: suspicious #include of file with '.cpp' extension
+// CHECK-MESSAGES: [[@LINE+1]]:11: note: did you mean to include 'i.h'?
+#include "i.cpp"
+
+#include "b.h"
+
+// CHECK-MESSAGES: [[@LINE+1]]:16: warning: suspicious #include_next of file with '.c' extension
+#include_next <c.c>
+
+// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension
+# include  <c.cc>
+
+// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension
+#  include  <c.cxx>