diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -1524,6 +1524,10 @@ &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo()); for (const auto &Inc : Includes.MainFileIncludes) Inserter->addExisting(Inc); + if (Includes.IwyuNoIncludes) { + for (const auto &Inc : *Includes.IwyuNoIncludes) + Inserter->addNoInclude(Inc); + } // Most of the cost of file proximity is in initializing the FileDistance // structures based on the observed includes, once per query. Conceptually diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h --- a/clang-tools-extra/clangd/Headers.h +++ b/clang-tools-extra/clangd/Headers.h @@ -78,6 +78,18 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &); bool operator==(const Inclusion &LHS, const Inclusion &RHS); +struct IwyuNoInclude { + std::string + Written; // For `IWYU pragma no_include `, it is ``. + Path Resolved; // Resolved path to written file. + int HashLine; // 0-based line number containing IWYU pragma + IwyuNoInclude() = default; + IwyuNoInclude(std::string Written, Path Resolved, int HashLine) + : Written(std::move(Written)), Resolved(std::move(Resolved)), + HashLine(HashLine) {} + friend bool operator<(const IwyuNoInclude &LHS, const IwyuNoInclude &RHS); +}; + // Contains information about one file in the build graph and its direct // dependencies. Doesn't own the strings it references (IncludeGraph is // self-contained). @@ -176,6 +188,8 @@ StdlibHeaders; std::vector MainFileIncludes; + std::optional> + IwyuNoIncludes; // IWYU pragma: no_include // We reserve HeaderID(0) for the main file and will manually check for that // in getID and getOrCreateID because the UniqueID is not stable when the @@ -219,6 +233,8 @@ void addExisting(const Inclusion &Inc); + void addNoInclude(const IwyuNoInclude &Inc); + /// Checks whether to add an #include of the header into \p File. /// An #include will not be added if: /// - Either \p DeclaringHeader or \p InsertedHeader is already (directly) @@ -258,7 +274,9 @@ StringRef BuildDir; HeaderSearch *HeaderSearchInfo = nullptr; llvm::StringSet<> IncludedHeaders; // Both written and resolved. - tooling::HeaderIncludes Inserter; // Computers insertion replacement. + llvm::StringSet<> IwyuNoIncludeHeaders; // Iwyu: no_include headers, with + // written and resolved. + tooling::HeaderIncludes Inserter; // Computers insertion replacement. }; } // namespace clangd diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -147,11 +147,43 @@ // will know that the next inclusion is behind the IWYU pragma. // FIXME: Support "IWYU pragma: begin_exports" and "IWYU pragma: // end_exports". - if (!Pragma->startswith("export") && !Pragma->startswith("keep")) - return false; unsigned Offset = SM.getFileOffset(Range.getBegin()); - LastPragmaKeepInMainFileLine = + const unsigned CurrentLine = SM.getLineNumber(SM.getMainFileID(), Offset) - 1; + if (Pragma->startswith("export") || Pragma->startswith("keep")) + LastPragmaKeepInMainFileLine = CurrentLine; + else if (Pragma->consume_front("no_include")) { + auto &NI = Out->IwyuNoIncludes; + if (!NI) + NI.emplace(); + auto Spelling = + Pragma->take_until([](char C) { return C == '\n'; }).trim(); + if (Spelling.empty()) + return false; + if (!(Spelling.front() == '<' && Spelling.back() == '>') && + !(Spelling.front() == '"' && Spelling.back() == '"')) + return false; + if (auto MaybeHeader = Spelling.trim("\"<>"); !MaybeHeader.empty()) { + SmallVector, 1> + Includers; + const auto FE = HeaderInfo.LookupFile( + /*Filename=*/MaybeHeader, + /*IncludeLoc=*/{}, /*isAngled=*/Spelling.starts_with("<"), + /*FromDir=*/nullptr, /*CurDir=*/nullptr, + /*Includers=*/Includers, /*SearchPath=*/nullptr, + /*RelativePath=*/nullptr, /*RequestingModule=*/nullptr, + /*SuggestedModule=*/nullptr, /*IsMapped=*/nullptr, + /*IsFrameworkFound=*/nullptr, + /*SkipCache=*/false, /*BuildSystemModule=*/false, + /*OpenFile=*/false, /*CacheFailures=*/false); + NI->insert(IwyuNoInclude( + /*Written=*/Spelling.str(), + /*Resolved=*/ + !FE ? "" : FE->getFileEntry().tryGetRealPathName().str(), + CurrentLine)); + } + return false; + } } else { // Memorize headers that have export pragmas in them. Include Cleaner // does not support them properly yet, so they will be not marked as @@ -300,6 +332,12 @@ IncludedHeaders.insert(Inc.Resolved); } +void IncludeInserter::addNoInclude(const IwyuNoInclude &Inc) { + IwyuNoIncludeHeaders.insert(Inc.Written); + if (!Inc.Resolved.empty()) + IwyuNoIncludeHeaders.insert(Inc.Resolved); +} + /// FIXME(ioeric): we might not want to insert an absolute include path if the /// path is not shortened. bool IncludeInserter::shouldInsertInclude( @@ -312,7 +350,13 @@ auto Included = [&](llvm::StringRef Header) { return IncludedHeaders.find(Header) != IncludedHeaders.end(); }; - return !Included(DeclaringHeader) && !Included(InsertedHeader.File); + auto IsIwyuNoInclude = [&](llvm::StringRef Header) { + return IwyuNoIncludeHeaders.find(Header) != IwyuNoIncludeHeaders.end(); + }; + auto ShouldInsert = [&](auto &Predicate) { + return !Predicate(DeclaringHeader) && !Predicate(InsertedHeader.File); + }; + return ShouldInsert(Included) && ShouldInsert(IsIwyuNoInclude); } std::optional @@ -369,5 +413,10 @@ RHS.Resolved, RHS.Written); } +bool operator<(const IwyuNoInclude &LHS, const IwyuNoInclude &RHS) { + // We want only 1 pragma for the same header. + return LHS.Written < RHS.Written; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -105,6 +105,9 @@ std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST, llvm::StringRef Code); +std::vector issueNoIncludesDiagnostics(ParsedAST &AST, + llvm::StringRef Code); + /// Affects whether standard library includes should be considered for /// removal. This is off by default for now due to implementation limitations: /// - macros are not tracked diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -576,5 +576,46 @@ return Result; } +std::vector issueNoIncludesDiagnostics(ParsedAST &AST, + llvm::StringRef Code) { + // C/C++ only + if (AST.getLangOpts().ObjC) + return {}; + std::vector Result; + std::string FileName = + AST.getSourceManager() + .getFileEntryRefForID(AST.getSourceManager().getMainFileID()) + ->getName() + .str(); + auto &IS = AST.getIncludeStructure(); + auto &NoIncludes = IS.IwyuNoIncludes; + if (!NoIncludes) + return Result; + for (auto &Inc : IS.MainFileIncludes) { + const auto It = NoIncludes->find( + IwyuNoInclude(Inc.Written, Inc.Resolved, Inc.HashLine)); + if (It == NoIncludes->end()) + continue; + Diag D; + D.Message = llvm::formatv( + "included header '{0}' is marked as `no_include` at line {1}", + StringRef(Inc.Written).trim("<>\""), It->HashLine + 1); + D.Name = "no-include-headers"; + D.Source = Diag::DiagSource::Clangd; + D.File = FileName; + D.Severity = DiagnosticsEngine::Warning; + D.Tags.push_back(Unnecessary); + D.Range = getDiagnosticRange(Code, Inc.HashOffset); + D.Fixes.emplace_back(); + D.Fixes.back().Message = "remove #include directive"; + D.Fixes.back().Edits.emplace_back(); + D.Fixes.back().Edits.back().range.start.line = Inc.HashLine; + D.Fixes.back().Edits.back().range.end.line = Inc.HashLine + 1; + D.InsideMainFile = true; + Result.push_back(std::move(D)); + } + return Result; +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -571,6 +571,10 @@ MainFileIncludes = Preamble->Includes.MainFileIncludes; for (const auto &Inc : Preamble->Includes.MainFileIncludes) Inserter->addExisting(Inc); + if (Preamble->Includes.IwyuNoIncludes) { + for (const auto &NoInc : *Preamble->Includes.IwyuNoIncludes) + Inserter->addNoInclude(NoInc); + } } // FIXME: Consider piping through ASTSignals to fetch this to handle the // case where a header file contains ObjC decls but no #imports. @@ -691,9 +695,14 @@ if (Result.Diags) { auto UnusedHeadersDiags = issueUnusedIncludesDiagnostics(Result, Inputs.Contents); + auto NoIncludeHeadersDiags = + issueNoIncludesDiagnostics(Result, Inputs.Contents); Result.Diags->insert(Result.Diags->end(), make_move_iterator(UnusedHeadersDiags.begin()), make_move_iterator(UnusedHeadersDiags.end())); + Result.Diags->insert(Result.Diags->end(), + make_move_iterator(NoIncludeHeadersDiags.begin()), + make_move_iterator(NoIncludeHeadersDiags.end())); } return std::move(Result); } diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.h b/clang-tools-extra/clangd/index/CanonicalIncludes.h --- a/clang-tools-extra/clangd/index/CanonicalIncludes.h +++ b/clang-tools-extra/clangd/index/CanonicalIncludes.h @@ -87,6 +87,8 @@ /// - friend: this is less common than private, has implementation difficulties, /// and affects behavior in a limited scope. /// - associated: extremely rare +/// Note that support for some other IWYU pragmas (export, keep, no_include...) +/// are implemented in IncludeStructure. std::unique_ptr collectIWYUHeaderMaps(CanonicalIncludes *Includes); diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp --- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp @@ -93,7 +93,8 @@ // Calculates the include path, or returns "" on error or header should not be // inserted. std::string calculate(PathRef Original, PathRef Preferred = "", - const std::vector &Inclusions = {}) { + const std::vector &Inclusions = {}, + const std::vector &IwyuNoIncludes = {}) { Clang = setupClang(); PreprocessOnlyAction Action; EXPECT_TRUE( @@ -111,6 +112,8 @@ &Clang->getPreprocessor().getHeaderSearchInfo()); for (const auto &Inc : Inclusions) Inserter.addExisting(Inc); + for (const auto &Inc : IwyuNoIncludes) + Inserter.addNoInclude(Inc); auto Inserted = ToHeaderFile(Preferred); if (!Inserter.shouldInsertInclude(Original, Inserted)) return ""; @@ -275,6 +278,29 @@ AllOf(written("\"bar.h\""), hasPragmaKeep(true)))); } +TEST_F(HeadersTest, IWYUPragmaNoInclude) { + FS.Files[MainFile] = R"cpp( + // IWYU pragma: no_include "baz.h" + #include "foo.h" + // IWYU pragma: no_include + // IWYU pragma: no_include "headers.hpp" invalid + // IWYU pragma: no_include no_quotes are considered invalid + // IWYU pragma: no_include + )cpp"; + FS.Files["foo.h"] = ""; + + auto NI = collectIncludes().IwyuNoIncludes; + ASSERT_TRUE(NI); + EXPECT_THAT(*NI, + UnorderedElementsAre(written("\"baz.h\""), written(""), + written(""))); + IwyuNoInclude Inc; + Inc.Written = "\"baz.h\""; + EXPECT_THAT(calculate("\"baz.h\"", "", {}, {Inc}), ""); + Inc.Written = ""; + EXPECT_THAT(calculate("", "", {}, {Inc}), ""); +} + TEST_F(HeadersTest, InsertInclude) { std::string Path = testPath("sub/bar.h"); FS.Files[Path] = ""; diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -9,6 +9,7 @@ #include "Annotations.h" #include "Config.h" #include "IncludeCleaner.h" +#include "Protocol.h" #include "SourceCode.h" #include "TestFS.h" #include "TestTU.h" @@ -611,6 +612,34 @@ EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty()); } +MATCHER_P(message, Message, "") { return arg.Message == Message; } +MATCHER_P2(range, Start, End, "") { + return arg.start == Start && arg.end == End; +} + +TEST(IncludeCleaner, IWYUPragmaNoInclude) { + TestTU TU; + TU.Code = R"cpp( +#include "foo.h" +#include "bar.h" +// IWYU pragma: no_include "bar.h" + )cpp"; + TU.AdditionalFiles["foo.h"] = guard(""); + TU.AdditionalFiles["bar.h"] = guard(""); + ParsedAST AST = TU.build(); + + auto Diags = issueNoIncludesDiagnostics(AST, TU.Code); + EXPECT_THAT( + Diags, + UnorderedElementsAre(message( + "included header 'bar.h' is marked as `no_include` at line 4"))); + ASSERT_TRUE(Diags.size() == 1); + EXPECT_THAT(Diags.back().Fixes.size(), 1); + ASSERT_TRUE(Diags.back().Fixes.back().Edits.size() == 1); + EXPECT_THAT(Diags.back().Fixes.back().Edits.back().range, + range(Position{2, 0}, Position{3, 0})); +} + TEST(IncludeCleaner, NoDiagsForObjC) { TestTU TU; TU.Code = R"cpp(