diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -45,6 +45,7 @@ #include "StringIntegerAssignmentCheck.h" #include "StringLiteralWithEmbeddedNulCheck.h" #include "SuspiciousEnumUsageCheck.h" +#include "SuspiciousIncludeCheck.h" #include "SuspiciousMemsetUsageCheck.h" #include "SuspiciousMissingCommaCheck.h" #include "SuspiciousSemicolonCheck.h" @@ -140,6 +141,8 @@ "bugprone-string-literal-with-embedded-nul"); CheckFactories.registerCheck( "bugprone-suspicious-enum-usage"); + CheckFactories.registerCheck( + "bugprone-suspicious-include"); CheckFactories.registerCheck( "bugprone-suspicious-memset-usage"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -37,6 +37,7 @@ StringIntegerAssignmentCheck.cpp StringLiteralWithEmbeddedNulCheck.cpp SuspiciousEnumUsageCheck.cpp + SuspiciousIncludeCheck.cpp SuspiciousMemsetUsageCheck.cpp SuspiciousMissingCommaCheck.cpp SuspiciousSemicolonCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h @@ -0,0 +1,57 @@ +//===--- 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 diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp @@ -0,0 +1,108 @@ +//===--- 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(*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 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 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 diff --git a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h --- a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h +++ b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h @@ -11,6 +11,7 @@ #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" @@ -36,6 +37,12 @@ /// 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 ",;"; } @@ -45,6 +52,11 @@ 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 +getFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions); + /// Decides whether a file has one of the specified file extensions. bool isFileExtension(StringRef FileName, const FileExtensionsSet &FileExtensions); diff --git a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp --- a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp +++ b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp @@ -53,13 +53,20 @@ return true; } -bool isFileExtension(StringRef FileName, - const FileExtensionsSet &FileExtensions) { +llvm::Optional +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 diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -82,6 +82,13 @@ Checks for usages of identifiers reserved for use by the implementation. +- New :doc:`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 ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst @@ -0,0 +1,32 @@ +.. 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 // 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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a new file mode 100644 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp new file mode 100644 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp new file mode 100644 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c new file mode 100644 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc new file mode 100644 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx new file mode 100644 diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp b/clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp new file mode 100644 diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp @@ -0,0 +1,24 @@ +// 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 + +// CHECK-MESSAGES: [[@LINE+1]]:13: warning: suspicious #include of file with '.cc' extension +# include + +// CHECK-MESSAGES: [[@LINE+1]]:14: warning: suspicious #include of file with '.cxx' extension +# include