[clang-tools-extra] 1e0669b - [clang-tidy] New check: bugprone-suspicious-include
Jonathan Roelofs via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 9 14:59:44 PDT 2020
Author: Jonathan Roelofs
Date: 2020-03-09T15:54:32-06:00
New Revision: 1e0669bfe05f0f48ee88152c4a1d581f484f8d67
URL: https://github.com/llvm/llvm-project/commit/1e0669bfe05f0f48ee88152c4a1d581f484f8d67
DIFF: https://github.com/llvm/llvm-project/commit/1e0669bfe05f0f48ee88152c4a1d581f484f8d67.diff
LOG: [clang-tidy] New check: bugprone-suspicious-include
Detects and fixes suspicious code like: `#include "foo.cpp"`.
Inspired by: https://twitter.com/lefticus/status/1228458240364687360?s=20
https://reviews.llvm.org/D74669
Added:
clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.cpp
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/a.hpp
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.c
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cc
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/c.cxx
clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/i.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
Modified:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
clang-tools-extra/docs/ReleaseNotes.rst
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index 86936c678562..9dcb315a257a 100644
--- 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 @@ class BugproneModule : public ClangTidyModule {
"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>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index c9078ed390d1..a24f3bc7eb0d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -37,6 +37,7 @@ add_clang_library(clangTidyBugproneModule
StringIntegerAssignmentCheck.cpp
StringLiteralWithEmbeddedNulCheck.cpp
SuspiciousEnumUsageCheck.cpp
+ SuspiciousIncludeCheck.cpp
SuspiciousMemsetUsageCheck.cpp
SuspiciousMissingCommaCheck.cpp
SuspiciousSemicolonCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
new file mode 100644
index 000000000000..ade7b111fc9a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
@@ -0,0 +1,105 @@
+//===--- 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) {
+ 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
diff --git a/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h b/clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.h
new file mode 100644
index 000000000000..674292da1ee8
--- /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/utils/FileExtensionsUtils.cpp b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
index d6f4b2ae3f7c..4eaf8bc6f392 100644
--- a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp
@@ -53,13 +53,20 @@ bool parseFileExtensions(StringRef AllFileExtensions,
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
diff --git a/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h b/clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
index 43fc573aad65..e5dfae1ba24c 100644
--- 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 @@ bool isSpellingLocInHeaderFile(SourceLocation Loc, SourceManager &SM,
/// 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 @@ 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);
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3a58649ea985..4526cf251d8c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -98,6 +98,12 @@ New checks
Finds recursive functions and diagnoses them.
+- New :doc:`bugprone-suspicious-includei
+ <clang-tidy/checks/bugprone-suspicious-include>` check.
+
+ Finds includes that appear to be referring to implementation files (which
+ tends to cause ODR violations), and diagnoses them.
+
New check aliases
^^^^^^^^^^^^^^^^^
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
index 000000000000..95a90c4f1fb8
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-include.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - bugprone-suspicious-include
+
+bugprone-suspicious-include
+===========================
+
+The checker 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.
+ #import "Stegosaurus.c" // Warning, fliename 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.
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
index 000000000000..e69de29bb2d1
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
index 000000000000..e69de29bb2d1
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
index 000000000000..e69de29bb2d1
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
index 000000000000..e69de29bb2d1
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
index 000000000000..e69de29bb2d1
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
index 000000000000..e69de29bb2d1
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
index 000000000000..e69de29bb2d1
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
index 000000000000..72d1e3b70f80
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-include.cpp
@@ -0,0 +1,27 @@
+// 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]]:10: warning: suspicious #import of file with '.c' extension
+#import "c.c"
+
+// 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>
More information about the cfe-commits
mailing list