[clang-tools-extra] d2e8fb3 - [clang-tidy] Add readability-duplicate-include check
via cfe-commits
cfe-commits at lists.llvm.org
Sun Jan 23 08:23:15 PST 2022
Author: Richard
Date: 2022-01-23T09:23:04-07:00
New Revision: d2e8fb331835fcc565929720781a5fd64e66fc17
URL: https://github.com/llvm/llvm-project/commit/d2e8fb331835fcc565929720781a5fd64e66fc17
DIFF: https://github.com/llvm/llvm-project/commit/d2e8fb331835fcc565929720781a5fd64e66fc17.diff
LOG: [clang-tidy] Add readability-duplicate-include check
Looks for duplicate includes and removes them.
Every time an include directive is processed, check a vector of filenames
to see if the included file has already been included. If so, it issues
a warning and a replacement to remove the entire line containing the
duplicated include directive.
When a macro is defined or undefined, the vector of filenames is cleared.
This enables including the same file multiple times, but getting
different expansions based on the set of active macros at the time of
inclusion. For example:
#undef NDEBUG
#include "assertion.h"
// ...code with assertions enabled
#define NDEBUG
#include "assertion.h"
// ...code with assertions disabled
Since macros are redefined between the inclusion of assertion.h,
they are not flagged as redundant.
Differential Revision: https://reviews.llvm.org/D7982
Added:
clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include2.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/iostream
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/string.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/types.h
clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
index eba0ab98cb37a..22ce8f62751ec 100644
--- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_library(clangTidyReadabilityModule
ContainerSizeEmptyCheck.cpp
ConvertMemberFunctionsToStatic.cpp
DeleteNullPointerCheck.cpp
+ DuplicateIncludeCheck.cpp
ElseAfterReturnCheck.cpp
FunctionCognitiveComplexityCheck.cpp
FunctionSizeCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
new file mode 100644
index 0000000000000..681b8399154a7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
@@ -0,0 +1,116 @@
+//===--- DuplicateIncludeCheck.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 "DuplicateIncludeCheck.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/Preprocessor.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallVector.h"
+#include <memory>
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+static SourceLocation advanceBeyondCurrentLine(const SourceManager &SM,
+ SourceLocation Start,
+ int Offset) {
+ const FileID Id = SM.getFileID(Start);
+ const unsigned LineNumber = SM.getSpellingLineNumber(Start);
+ while (SM.getFileID(Start) == Id &&
+ SM.getSpellingLineNumber(Start.getLocWithOffset(Offset)) == LineNumber)
+ Start = Start.getLocWithOffset(Offset);
+ return Start;
+}
+
+namespace {
+
+using FileList = SmallVector<StringRef>;
+
+class DuplicateIncludeCallbacks : public PPCallbacks {
+public:
+ DuplicateIncludeCallbacks(DuplicateIncludeCheck &Check,
+ const SourceManager &SM)
+ : Check(Check), SM(SM) {
+ // The main file doesn't participate in the FileChanged notification.
+ Files.emplace_back();
+ }
+
+ void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+ SrcMgr::CharacteristicKind FileType,
+ FileID PrevFID) override;
+
+ 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;
+
+ void MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) override;
+
+ void MacroUndefined(const Token &MacroNameTok, const MacroDefinition &MD,
+ const MacroDirective *Undef) override;
+
+private:
+ // A list of included files is kept for each file we enter.
+ SmallVector<FileList> Files;
+ DuplicateIncludeCheck &Check;
+ const SourceManager &SM;
+};
+
+void DuplicateIncludeCallbacks::FileChanged(SourceLocation Loc,
+ FileChangeReason Reason,
+ SrcMgr::CharacteristicKind FileType,
+ FileID PrevFID) {
+ if (Reason == EnterFile)
+ Files.emplace_back();
+ else
+ Files.pop_back();
+}
+
+void DuplicateIncludeCallbacks::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 (llvm::find(Files.back(), FileName) != Files.back().end()) {
+ // We want to delete the entire line, so make sure that [Start,End] covers
+ // everything.
+ SourceLocation Start =
+ advanceBeyondCurrentLine(SM, HashLoc, -1).getLocWithOffset(-1);
+ SourceLocation End =
+ advanceBeyondCurrentLine(SM, FilenameRange.getEnd(), 1);
+ Check.diag(HashLoc, "duplicate include")
+ << FixItHint::CreateRemoval(SourceRange{Start, End});
+ } else
+ Files.back().push_back(FileName);
+}
+
+void DuplicateIncludeCallbacks::MacroDefined(const Token &MacroNameTok,
+ const MacroDirective *MD) {
+ Files.back().clear();
+}
+
+void DuplicateIncludeCallbacks::MacroUndefined(const Token &MacroNameTok,
+ const MacroDefinition &MD,
+ const MacroDirective *Undef) {
+ Files.back().clear();
+}
+
+} // namespace
+
+void DuplicateIncludeCheck::registerPPCallbacks(
+ const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
+ PP->addPPCallbacks(std::make_unique<DuplicateIncludeCallbacks>(*this, SM));
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
new file mode 100644
index 0000000000000..b213e3a4b73cc
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h
@@ -0,0 +1,35 @@
+//===--- DuplicateIncludeCheck.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_READABILITY_DUPLICATE_INCLUDE_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATE_INCLUDE_CHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// \brief Find and remove duplicate #include directives.
+///
+/// Only consecutive include directives without any other preprocessor
+/// directives between them are analyzed.
+class DuplicateIncludeCheck : public ClangTidyCheck {
+public:
+ DuplicateIncludeCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_DUPLICATE_INCLUDE_CHECK_H
diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
index 2d6540283ded5..b0493d43ff318 100644
--- a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -16,6 +16,7 @@
#include "ContainerSizeEmptyCheck.h"
#include "ConvertMemberFunctionsToStatic.h"
#include "DeleteNullPointerCheck.h"
+#include "DuplicateIncludeCheck.h"
#include "ElseAfterReturnCheck.h"
#include "FunctionCognitiveComplexityCheck.h"
#include "FunctionSizeCheck.h"
@@ -71,6 +72,8 @@ class ReadabilityModule : public ClangTidyModule {
"readability-convert-member-functions-to-static");
CheckFactories.registerCheck<DeleteNullPointerCheck>(
"readability-delete-null-pointer");
+ CheckFactories.registerCheck<DuplicateIncludeCheck>(
+ "readability-duplicate-include");
CheckFactories.registerCheck<ElseAfterReturnCheck>(
"readability-else-after-return");
CheckFactories.registerCheck<FunctionCognitiveComplexityCheck>(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1f7228d5732bd..060af42521552 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -124,6 +124,11 @@ New checks
Finds cases where code could use ``data()`` rather than the address of the
element at index 0 in a container.
+- New :doc:`readability-duplicate-include
+ <clang-tidy/checks/readability-duplicate-include>` check.
+
+ Looks for duplicate includes and removes them.
+
- New :doc:`readability-identifier-length
<clang-tidy/checks/readability-identifier-length>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 9785c9f43a4b4..5878345bdfcfd 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -294,6 +294,7 @@ Clang-Tidy Checks
`readability-container-size-empty <readability-container-size-empty.html>`_, "Yes"
`readability-convert-member-functions-to-static <readability-convert-member-functions-to-static.html>`_, "Yes"
`readability-delete-null-pointer <readability-delete-null-pointer.html>`_, "Yes"
+ `readability-duplicate-include <readability-duplicate-include.html>`_, "Yes"
`readability-else-after-return <readability-else-after-return.html>`_, "Yes"
`readability-function-cognitive-complexity <readability-function-cognitive-complexity.html>`_,
`readability-function-size <readability-function-size.html>`_,
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst b/clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
new file mode 100644
index 0000000000000..45df7e1b84f3f
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability-duplicate-include.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - readability-duplicate-include
+
+readability-duplicate-include
+=============================
+
+Looks for duplicate includes and removes them. The check maintains a list of
+included files and looks for duplicates. If a macro is defined or undefined
+then the list of included files is cleared.
+
+Examples:
+
+.. code-block:: c++
+
+ #include <memory>
+ #include <vector>
+ #include <memory>
+
+becomes
+
+.. code-block:: c++
+
+ #include <memory>
+ #include <vector>
+
+Because of the intervening macro definitions, this code remains unchanged:
+
+.. code-block:: c++
+
+ #undef NDEBUG
+ #include "assertion.h"
+ // ...code with assertions enabled
+
+ #define NDEBUG
+ #include "assertion.h"
+ // ...code with assertions disabled
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
new file mode 100644
index 0000000000000..7d84adb816622
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
@@ -0,0 +1,15 @@
+#ifndef READABILITY_DUPLICATE_INCLUDE_H
+#define READABILITY_DUPLICATE_INCLUDE_H
+
+extern int g;
+#include "readability-duplicate-include2.h"
+extern int h;
+#include "readability-duplicate-include2.h"
+extern int i;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^extern int g;$}}
+// CHECK-FIXES-NEXT: {{^#include "readability-duplicate-include2.h"$}}
+// CHECK-FIXES-NEXT: {{^extern int h;$}}
+// CHECK-FIXES-NEXT: {{^extern int i;$}}
+
+#endif
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include2.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include2.h
new file mode 100644
index 0000000000000..58dfa757ee7ae
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include2.h
@@ -0,0 +1 @@
+// This file is intentionally empty.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/iostream b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/iostream
new file mode 100644
index 0000000000000..fcbabe12fc378
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/iostream
@@ -0,0 +1 @@
+// This file is intentionally empty.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/string.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/string.h
new file mode 100644
index 0000000000000..fcbabe12fc378
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/string.h
@@ -0,0 +1 @@
+// This file is intentionally empty.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h
new file mode 100644
index 0000000000000..58dfa757ee7ae
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h
@@ -0,0 +1 @@
+// This file is intentionally empty.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/types.h b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/types.h
new file mode 100644
index 0000000000000..fcbabe12fc378
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/types.h
@@ -0,0 +1 @@
+// This file is intentionally empty.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
new file mode 100644
index 0000000000000..f9a3c70ef86f4
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability-duplicate-include.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy %s readability-duplicate-include %t -- -- -isystem %S/Inputs/readability-duplicate-include/system -I %S/Inputs/readability-duplicate-include
+
+int a;
+#include <string.h>
+int b;
+#include <string.h>
+int c;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include [readability-duplicate-include]
+// CHECK-FIXES: {{^int a;$}}
+// CHECK-FIXES-NEXT: {{^#include <string.h>$}}
+// CHECK-FIXES-NEXT: {{^int b;$}}
+// CHECK-FIXES-NEXT: {{^int c;$}}
+
+int d;
+#include <iostream>
+int e;
+#include <iostream> // extra stuff that will also be removed
+int f;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int d;$}}
+// CHECK-FIXES-NEXT: {{^#include <iostream>$}}
+// CHECK-FIXES-NEXT: {{^int e;$}}
+// CHECK-FIXES-NEXT: {{^int f;$}}
+
+int g;
+#include "readability-duplicate-include.h"
+int h;
+#include "readability-duplicate-include.h"
+int i;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int g;$}}
+// CHECK-FIXES-NEXT: {{^#include "readability-duplicate-include.h"$}}
+// CHECK-FIXES-NEXT: {{^int h;$}}
+// CHECK-FIXES-NEXT: {{^int i;$}}
+
+#include <types.h>
+
+int j;
+#include <sys/types.h>
+int k;
+#include <sys/types.h>
+int l;
+// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include
+// CHECK-FIXES: {{^int j;$}}
+// CHECK-FIXES-NEXT: {{^#include <sys/types.h>$}}
+// CHECK-FIXES-NEXT: {{^int k;$}}
+// CHECK-FIXES-NEXT: {{^int l;$}}
+
+int m;
+ # include <string.h> // lots of space
+int n;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: duplicate include
+// CHECK-FIXES: {{^int m;$}}
+// CHECK-FIXES-NEXT: {{^int n;$}}
+
+// defining a macro in the main file resets the included file cache
+#define ARBITRARY_MACRO
+int o;
+#include <sys/types.h>
+int p;
+// CHECK-FIXES: {{^int o;$}}
+// CHECK-FIXES-NEXT: {{^#include <sys/types.h>$}}
+// CHECK-FIXES-NEXT: {{^int p;$}}
+
+// undefining a macro resets the cache
+#undef ARBITRARY_MACRO
+int q;
+#include <sys/types.h>
+int r;
+// CHECK-FIXES: {{^int q;$}}
+// CHECK-FIXES-NEXT: {{^#include <sys/types.h>$}}
+// CHECK-FIXES-NEXT: {{^int r;$}}
More information about the cfe-commits
mailing list