diff --git a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/readability/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/readability/CMakeLists.txt @@ -11,6 +11,7 @@ ContainerSizeEmptyCheck.cpp ConvertMemberFunctionsToStatic.cpp DeleteNullPointerCheck.cpp + DuplicateIncludeCheck.cpp ElseAfterReturnCheck.cpp FunctionCognitiveComplexityCheck.cpp FunctionSizeCheck.cpp diff --git a/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.h new file mode 100644 --- /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/DuplicateIncludeCheck.cpp b/clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp new file mode 100644 --- /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 + +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; + +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 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(*this, SM)); +} + +} // namespace readability +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp b/clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp --- 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 @@ "readability-convert-member-functions-to-static"); CheckFactories.registerCheck( "readability-delete-null-pointer"); + CheckFactories.registerCheck( + "readability-duplicate-include"); CheckFactories.registerCheck( "readability-else-after-return"); CheckFactories.registerCheck( 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 @@ -124,6 +124,11 @@ 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 + ` check. + + Looks for duplicate includes and removes them. + - New :doc:`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 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -294,6 +294,7 @@ `readability-container-size-empty `_, "Yes" `readability-convert-member-functions-to-static `_, "Yes" `readability-delete-null-pointer `_, "Yes" + `readability-duplicate-include `_, "Yes" `readability-else-after-return `_, "Yes" `readability-function-cognitive-complexity `_, `readability-function-size `_, 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 --- /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 + #include + #include + +becomes + +.. code-block:: c++ + + #include + #include + +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 --- /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 --- /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 --- /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 --- /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 --- /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 --- /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 --- /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 +int b; +#include +int c; +// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include [readability-duplicate-include] +// CHECK-FIXES: {{^int a;$}} +// CHECK-FIXES-NEXT: {{^#include $}} +// CHECK-FIXES-NEXT: {{^int b;$}} +// CHECK-FIXES-NEXT: {{^int c;$}} + +int d; +#include +int e; +#include // 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 $}} +// 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 + +int j; +#include +int k; +#include +int l; +// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: duplicate include +// CHECK-FIXES: {{^int j;$}} +// CHECK-FIXES-NEXT: {{^#include $}} +// CHECK-FIXES-NEXT: {{^int k;$}} +// CHECK-FIXES-NEXT: {{^int l;$}} + +int m; + # include // 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 +int p; +// CHECK-FIXES: {{^int o;$}} +// CHECK-FIXES-NEXT: {{^#include $}} +// CHECK-FIXES-NEXT: {{^int p;$}} + +// undefining a macro resets the cache +#undef ARBITRARY_MACRO +int q; +#include +int r; +// CHECK-FIXES: {{^int q;$}} +// CHECK-FIXES-NEXT: {{^#include $}} +// CHECK-FIXES-NEXT: {{^int r;$}}