diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/CMakeLists.txt @@ -7,6 +7,7 @@ ${CMAKE_CURRENT_SOURCE_DIR}/clang-tidy-config.h.cmake ${CMAKE_CURRENT_BINARY_DIR}/clang-tidy-config.h) include_directories(BEFORE ${CMAKE_CURRENT_BINARY_DIR}) +include_directories(BEFORE "${CMAKE_CURRENT_SOURCE_DIR}/../include-cleaner/include") add_clang_library(clangTidy ClangTidy.cpp @@ -38,6 +39,7 @@ clangSerialization clangTooling clangToolingCore + clangIncludeCleaner ) if(CLANG_TIDY_ENABLE_STATIC_ANALYZER) diff --git a/clang-tools-extra/clang-tidy/google/CMakeLists.txt b/clang-tools-extra/clang-tidy/google/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/google/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/google/CMakeLists.txt @@ -15,6 +15,7 @@ GlobalNamesInHeadersCheck.cpp GlobalVariableDeclarationCheck.cpp GoogleTidyModule.cpp + IncludeCleanerCheck.cpp IntegerTypesCheck.cpp OverloadedUnaryAndCheck.cpp TodoCommentCheck.cpp diff --git a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp --- a/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp @@ -22,6 +22,7 @@ #include "FunctionNamingCheck.h" #include "GlobalNamesInHeadersCheck.h" #include "GlobalVariableDeclarationCheck.h" +#include "IncludeCleanerCheck.h" #include "IntegerTypesCheck.h" #include "OverloadedUnaryAndCheck.h" #include "TodoCommentCheck.h" @@ -49,6 +50,7 @@ "google-explicit-constructor"); CheckFactories.registerCheck( "google-global-names-in-headers"); + CheckFactories.registerCheck("google-include-cleaner"); CheckFactories.registerCheck( "google-objc-avoid-nsobject-new"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.h b/clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.h @@ -0,0 +1,55 @@ +//===--- IncludeCleanerCheck.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_GOOGLE_INCLUDECLEANERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_INCLUDECLEANERCHECK_H + +#include "../ClangTidyCheck.h" +#include "../ClangTidyDiagnosticConsumer.h" +#include "clang-include-cleaner/Record.h" +#include "clang-include-cleaner/Types.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Basic/LLVM.h" +#include "clang/Basic/SourceLocation.h" +#include "clang/Lex/HeaderSearch.h" +#include "clang/Tooling/Syntax/Tokens.h" +#include +#include + +namespace clang::tidy::google { + +struct MissingIncludeInfo { + SourceLocation SymRefLocation; + std::string MissingHeaderSpelling; +}; + +/// FIXME: Write a short description. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/google/include-cleaner.html +class IncludeCleanerCheck : public ClangTidyCheck { +public: + IncludeCleanerCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void onEndOfTranslationUnit() override; + +private: + include_cleaner::RecordedPP RecordedPreprocessor; + include_cleaner::PragmaIncludes RecordedPI; + HeaderSearch *HS; + Decl *TUDecl; + const SourceManager *SM; +}; + +} // namespace clang::tidy::google + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_INCLUDECLEANERCHECK_H diff --git a/clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.cpp b/clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.cpp @@ -0,0 +1,173 @@ +//===--- IncludeCleanerCheck.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 "IncludeCleanerCheck.h" +#include "clang-include-cleaner/Analysis.h" +#include "clang-include-cleaner/Record.h" +#include "clang-include-cleaner/Types.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/DeclBase.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Basic/FileEntry.h" +#include "clang/Format/Format.h" +#include "clang/Lex/Preprocessor.h" +#include "clang/Tooling/Core/Replacement.h" +#include "clang/Tooling/Inclusions/HeaderIncludes.h" +#include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseSet.h" +#include "llvm/ADT/StringRef.h" +#include +#include +#include + +using namespace clang::ast_matchers; + +namespace clang::tidy::google { + +void IncludeCleanerCheck::registerMatchers(MatchFinder *Finder) { + if (!getLangOpts().CPlusPlus) + return; + Finder->addMatcher(translationUnitDecl().bind("top"), this); +} + +void IncludeCleanerCheck::registerPPCallbacks(const SourceManager &SM, + Preprocessor *PP, + Preprocessor *ModuleExpanderPP) { + // llvm::errs() << "Registering PP callbacks!!\n"; + PP->addPPCallbacks(RecordedPreprocessor.record(*PP)); + HS = &PP->getHeaderSearchInfo(); + RecordedPI.record(SM, *PP); +} + +void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) { + // llvm::errs() << "Running the check!!\n"; + TUDecl = const_cast(Result.Nodes.getNodeAs("top")); + SM = Result.SourceManager; + + // llvm::errs() << "Finished!!\n"; +} + +// TODO(bakalova) Write unit tests +// TODO(bakalova) Refactor/cleanup +// TODO(bakalova) Write docs +void IncludeCleanerCheck::onEndOfTranslationUnit() { + const FileEntry *MainFile = SM->getFileEntryForID(SM->getMainFileID()); + llvm::DenseSet Used; + std::vector Missing; + // llvm::errs() << "Let's run the analysis!!\n"; + walkUsed(TUDecl, RecordedPreprocessor.MacroReferences, &RecordedPI, *SM, + [&](const include_cleaner::SymbolReference &Ref, + llvm::ArrayRef Providers) { + // std::string SymName; + // switch (Ref.Target.kind()) { + // case include_cleaner::Symbol::Declaration: + // SymName = + // llvm::dyn_cast(&Ref.Target.declaration()) + // ->getQualifiedNameAsString(); + // llvm::errs() << SymName << "\n"; + // break; + // } + bool Satisfied = false; + for (const include_cleaner::Header &H : Providers) { + // llvm::errs() << "Checking provider: " << + // include_cleaner::spellHeader(H, + // *HS, MainFile) + // << "\n"; + if (H.kind() == include_cleaner::Header::Physical && + H.physical() == MainFile) + Satisfied = true; + // llvm::errs() << SymName << " satisfied because provided by + // main file\n"; + for (const include_cleaner::Include *I : + RecordedPreprocessor.Includes.match(H)) { + Used.insert(I); + Satisfied = true; + // llvm::errs() << SymName << " satisfied because provider + // matched\n"; + } + } + if (!Satisfied && !Providers.empty() && + Ref.RT == include_cleaner::RefType::Explicit) + // llvm::errs() << "missing include in walkUsed: " + // << + // include_cleaner::spellHeader(Providers.front(), + // *HS, MainFile) + // << "\n"; + Missing.push_back( + {Ref.RefLocation, include_cleaner::spellHeader( + Providers.front(), *HS, MainFile)}); + }); + + std::vector Unused; + for (const include_cleaner::Include &I : + RecordedPreprocessor.Includes.all()) { + // llvm::errs() << "recorded include: " << I.quote() << "\n"; + if (Used.contains(&I) || !I.Resolved) + continue; + if (RecordedPI.shouldKeep(I.Line)) + continue; + // Check if main file is the public interface for a private header. If so + // we shouldn't diagnose it as unused. + if (auto PHeader = RecordedPI.getPublic(I.Resolved); !PHeader.empty()) { + PHeader = PHeader.trim("<>\""); + // Since most private -> public mappings happen in a verbatim way, we + // check textually here. This might go wrong in presence of symlinks or + // header mappings. But that's not different than rest of the places. + if (MainFile->tryGetRealPathName().endswith(PHeader)) + continue; + } + + Unused.push_back(&I); + } + + for (const auto *Inc : Unused) { + // llvm::errs() << "unused include in analysis: " << Inc->quote() << "\n"; + std::string Description("Unused include "); + Description += Inc->quote(); + diag(Inc->HashLocation, Description) + << FixItHint::CreateRemoval(Inc->HashLocation); + } + + // TODO(bakalova) Move to separate function + auto FileStyle = format::getStyle( + format::DefaultFormatStyle, MainFile->tryGetRealPathName(), + format::DefaultFallbackStyle, SM->getBufferData(SM->getMainFileID()), + &SM->getFileManager().getVirtualFileSystem()); + if (!FileStyle) { + FileStyle = format::getLLVMStyle(); + } + for (const auto &Inc : Missing) { + // llvm::errs() << "missing include in analysis: " << + // Inc.MissingHeaderSpelling + // << "\n"; + std::string Description("Missing include "); + Description += Inc.MissingHeaderSpelling; + + // TODO(bakalova) Move to separate function + tooling::HeaderIncludes HeaderIncludes( + MainFile->tryGetRealPathName(), SM->getBufferData(SM->getMainFileID()), + FileStyle->IncludeStyle); + bool Angled = llvm::StringRef{Inc.MissingHeaderSpelling}.starts_with("<"); + // We might suggest insertion of an existing include in edge cases, e.g., + // include is present in a PP-disabled region, or spelling of the header + // turns out to be the same as one of the unresolved includes in the + // main file. + std::optional Replacement = HeaderIncludes.insert( + llvm::StringRef{Inc.MissingHeaderSpelling}.trim("\"<>"), Angled, + tooling::IncludeDirective::Include); + if (!Replacement.has_value()) + continue; + diag(Inc.SymRefLocation, Description) << FixItHint::CreateInsertion( + SM->getComposedLoc(SM->getMainFileID(), Replacement->getOffset()), + "#include " + Inc.MissingHeaderSpelling + "\n"); + } +} + +} // namespace clang::tidy::google 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 @@ -138,6 +138,11 @@ Warns when an rvalue reference function parameter is never moved within the function body. +- New :doc:`google-include-cleaner + ` check. + + FIXME: add release notes. + - New :doc:`llvmlibc-inline-function-decl ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/google/include-cleaner.rst b/clang-tools-extra/docs/clang-tidy/checks/google/include-cleaner.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/google/include-cleaner.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - google-include-cleaner + +google-include-cleaner +====================== + +FIXME: Describe what patterns does the check detect and why. Give examples. 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 @@ -223,6 +223,7 @@ `google-default-arguments `_, `google-explicit-constructor `_, "Yes" `google-global-names-in-headers `_, + `google-include-cleaner `_, "Yes" `google-objc-avoid-nsobject-new `_, `google-objc-avoid-throwing-exception `_, `google-objc-function-naming `_, @@ -477,7 +478,7 @@ `cppcoreguidelines-explicit-virtual-functions `_, `modernize-use-override `_, "Yes" `cppcoreguidelines-macro-to-enum `_, `modernize-macro-to-enum `_, "Yes" `cppcoreguidelines-non-private-member-variables-in-classes `_, `misc-non-private-member-variables-in-classes `_, - `cppcoreguidelines-use-default-member-init `_, `modernize-use-default-member-init `_, + `cppcoreguidelines-use-default-member-init `_, `modernize-use-default-member-init `_, "Yes" `fuchsia-header-anon-namespaces `_, `google-build-namespaces `_, `google-readability-braces-around-statements `_, `readability-braces-around-statements `_, "Yes" `google-readability-function-size `_, `readability-function-size `_, diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h @@ -18,6 +18,7 @@ #define CLANG_INCLUDE_CLEANER_RECORD_H #include "clang-include-cleaner/Types.h" +#include "clang/Basic/SourceLocation.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallVector.h" @@ -52,6 +53,10 @@ /// to the structure. void record(const CompilerInstance &CI); + /// Installs an analysing PPCallback and CommentHandler and populates results + /// to the structure. + void record(const SourceManager &SM, Preprocessor &P); + /// Returns true if the given #include of the main-file should never be /// removed. bool shouldKeep(unsigned HashLineNumber) const { diff --git a/clang-tools-extra/include-cleaner/lib/Record.cpp b/clang-tools-extra/include-cleaner/lib/Record.cpp --- a/clang-tools-extra/include-cleaner/lib/Record.cpp +++ b/clang-tools-extra/include-cleaner/lib/Record.cpp @@ -19,6 +19,8 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/Inclusions/HeaderAnalysis.h" #include "clang/Tooling/Inclusions/StandardLibrary.h" +#include +#include namespace clang::include_cleaner { namespace { @@ -151,6 +153,10 @@ : SM(CI.getSourceManager()), HeaderInfo(CI.getPreprocessor().getHeaderSearchInfo()), Out(Out), UniqueStrings(Arena) {} + RecordPragma(const SourceManager &SM, const Preprocessor &P, PragmaIncludes *Out) + : SM(SM), + HeaderInfo(P.getHeaderSearchInfo()), Out(Out), + UniqueStrings(Arena) {} void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, @@ -342,6 +348,12 @@ CI.getPreprocessor().addPPCallbacks(std::move(Record)); } +void PragmaIncludes::record(const SourceManager &SM, Preprocessor &P) { + auto Record = std::make_unique(SM, P, this); + P.addCommentHandler(Record.get()); + P.addPPCallbacks(std::move(Record)); +} + llvm::StringRef PragmaIncludes::getPublic(const FileEntry *F) const { auto It = IWYUPublic.find(F->getUniqueID()); if (It == IWYUPublic.end()) diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/bar.h b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/bar.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/bar.h @@ -0,0 +1,4 @@ +#pragma once +#include "baz.h" +#include "private.h" +int bar(); \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/baz.h b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/baz.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/baz.h @@ -0,0 +1,2 @@ +#pragma once +int baz(); \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/foo.h b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/foo.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/foo.h @@ -0,0 +1,2 @@ +#pragma once +void foo(); \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/private.h b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/private.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/Inputs/private.h @@ -0,0 +1,2 @@ + // IWYU pragma: private, include "public.h" +int foobar(); \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/include-cleaner.cpp b/clang-tools-extra/test/clang-tidy/checkers/google/include-cleaner.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/include-cleaner.cpp @@ -0,0 +1,17 @@ +// RUN: %check_clang_tidy %s google-include-cleaner %t -- -- -I%S/Inputs -isystem%S/system +#include "bar.h" +// CHECK-FIXES: {{^}}#include "baz.h"{{$}} +#include "foo.h" +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Unused include "foo.h" [google-include-cleaner] +// CHECK-FIXES: {{^}} +// CHECK-FIXES: {{^}}#include {{$}} +#include +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Unused include [google-include-cleaner] +// CHECK-FIXES: {{^}} +int BarResult = bar(); +int BazResult = baz(); +// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Missing include "baz.h" [google-include-cleaner] +std::string HelloString; +// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Missing include [google-include-cleaner] +int FooBarResult = foobar(); +// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: Missing include "public.h" [google-include-cleaner] \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/system/string.h b/clang-tools-extra/test/clang-tidy/checkers/google/system/string.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/system/string.h @@ -0,0 +1,2 @@ +#pragma once +namespace std { class string {}; } \ No newline at end of file diff --git a/clang-tools-extra/test/clang-tidy/checkers/google/system/vector.h b/clang-tools-extra/test/clang-tidy/checkers/google/system/vector.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/google/system/vector.h @@ -0,0 +1,4 @@ +#pragma once +#include + +namespace std { class vector {}; } \ No newline at end of file