From 87eb9667bf81a0cb713a5910585cf6043ae96b71 Mon Sep 17 00:00:00 2001 From: Benjamin Kramer Date: Mon, 21 Nov 2016 15:28:50 +0000 Subject: [PATCH] [include-fixer plugin] Make the plugin emit proper fixits in case multiple errors are found. The standalone tool only fixes the first one and we managed to bake that assumption into the code :( Also fix a crash when no header is found at all. llvm-svn: 287544 --- clang-tools-extra/include-fixer/IncludeFixer.cpp | 37 ++++++++++++++-------- clang-tools-extra/include-fixer/IncludeFixer.h | 15 ++++++--- .../test/include-fixer/yamldb_plugin.cpp | 7 ++++ 3 files changed, 42 insertions(+), 17 deletions(-) diff --git a/clang-tools-extra/include-fixer/IncludeFixer.cpp b/clang-tools-extra/include-fixer/IncludeFixer.cpp index 21fce32..083682c 100644 --- a/clang-tools-extra/include-fixer/IncludeFixer.cpp +++ b/clang-tools-extra/include-fixer/IncludeFixer.cpp @@ -62,7 +62,8 @@ public: IncludeFixerContext getIncludeFixerContext(const clang::SourceManager &SourceManager, clang::HeaderSearch &HeaderSearch) const { - return SemaSource.getIncludeFixerContext(SourceManager, HeaderSearch); + return SemaSource.getIncludeFixerContext(SourceManager, HeaderSearch, + SemaSource.getMatchedSymbols()); } private: @@ -156,9 +157,10 @@ bool IncludeFixerSemaSource::MaybeDiagnoseMissingCompleteType( T.getUnqualifiedType().getAsString(context.getPrintingPolicy()); DEBUG(llvm::dbgs() << "Query missing complete type '" << QueryString << "'"); // Pass an empty range here since we don't add qualifier in this case. - query(QueryString, "", tooling::Range()); + std::vector MatchedSymbols = + query(QueryString, "", tooling::Range()); - if (GenerateDiagnostics) { + if (!MatchedSymbols.empty() && GenerateDiagnostics) { TypoCorrection Correction; FileID FID = CI->getSourceManager().getFileID(Loc); StringRef Code = CI->getSourceManager().getBufferData(FID); @@ -167,7 +169,8 @@ bool IncludeFixerSemaSource::MaybeDiagnoseMissingCompleteType( addDiagnosticsForContext( Correction, getIncludeFixerContext(CI->getSourceManager(), - CI->getPreprocessor().getHeaderSearchInfo()), + CI->getPreprocessor().getHeaderSearchInfo(), + MatchedSymbols), Code, StartOfFile, CI->getASTContext()); for (const PartialDiagnostic &PD : Correction.getExtraDiagnostics()) CI->getSema().Diag(Loc, PD); @@ -273,17 +276,19 @@ clang::TypoCorrection IncludeFixerSemaSource::CorrectTypo( } DEBUG(llvm::dbgs() << "TypoScopeQualifiers: " << TypoScopeString << "\n"); - query(QueryString, TypoScopeString, SymbolRange); + std::vector MatchedSymbols = + query(QueryString, TypoScopeString, SymbolRange); clang::TypoCorrection Correction(Typo.getName()); Correction.setCorrectionRange(SS, Typo); - if (GenerateDiagnostics) { + if (!MatchedSymbols.empty() && GenerateDiagnostics) { FileID FID = SM.getFileID(Typo.getLoc()); StringRef Code = SM.getBufferData(FID); SourceLocation StartOfFile = SM.getLocForStartOfFile(FID); addDiagnosticsForContext( Correction, - getIncludeFixerContext(SM, CI->getPreprocessor().getHeaderSearchInfo()), + getIncludeFixerContext(SM, CI->getPreprocessor().getHeaderSearchInfo(), + MatchedSymbols), Code, StartOfFile, CI->getASTContext()); } return Correction; @@ -316,7 +321,8 @@ std::string IncludeFixerSemaSource::minimizeInclude( /// Get the include fixer context for the queried symbol. IncludeFixerContext IncludeFixerSemaSource::getIncludeFixerContext( const clang::SourceManager &SourceManager, - clang::HeaderSearch &HeaderSearch) const { + clang::HeaderSearch &HeaderSearch, + ArrayRef MatchedSymbols) const { std::vector SymbolCandidates; for (const auto &Symbol : MatchedSymbols) { std::string FilePath = Symbol.getFilePath().str(); @@ -332,8 +338,9 @@ IncludeFixerContext IncludeFixerSemaSource::getIncludeFixerContext( return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates); } -bool IncludeFixerSemaSource::query(StringRef Query, StringRef ScopedQualifiers, - tooling::Range Range) { +std::vector +IncludeFixerSemaSource::query(StringRef Query, StringRef ScopedQualifiers, + tooling::Range Range) { assert(!Query.empty() && "Empty query!"); // Save all instances of an unidentified symbol. @@ -347,7 +354,7 @@ bool IncludeFixerSemaSource::query(StringRef Query, StringRef ScopedQualifiers, Query == QuerySymbolInfos.front().RawIdentifier) { QuerySymbolInfos.push_back({Query.str(), ScopedQualifiers, Range}); } - return false; + return {}; } DEBUG(llvm::dbgs() << "Looking up '" << Query << "' at "); @@ -375,12 +382,16 @@ bool IncludeFixerSemaSource::query(StringRef Query, StringRef ScopedQualifiers, // It's unsafe to do nested search for the identifier with scoped namespace // context, it might treat the identifier as a nested class of the scoped // namespace. - MatchedSymbols = SymbolIndexMgr.search(QueryString, /*IsNestedSearch=*/false); + std::vector MatchedSymbols = + SymbolIndexMgr.search(QueryString, /*IsNestedSearch=*/false); if (MatchedSymbols.empty()) MatchedSymbols = SymbolIndexMgr.search(Query); DEBUG(llvm::dbgs() << "Having found " << MatchedSymbols.size() << " symbols\n"); - return !MatchedSymbols.empty(); + // We store a copy of MatchedSymbols in a place where it's globally reachable. + // This is used by the standalone version of the tool. + this->MatchedSymbols = MatchedSymbols; + return MatchedSymbols; } llvm::Expected createIncludeFixerReplacements( diff --git a/clang-tools-extra/include-fixer/IncludeFixer.h b/clang-tools-extra/include-fixer/IncludeFixer.h index 8ad8888..28fc28a 100644 --- a/clang-tools-extra/include-fixer/IncludeFixer.h +++ b/clang-tools-extra/include-fixer/IncludeFixer.h @@ -115,13 +115,20 @@ public: clang::HeaderSearch &HeaderSearch) const; /// Get the include fixer context for the queried symbol. - IncludeFixerContext - getIncludeFixerContext(const clang::SourceManager &SourceManager, - clang::HeaderSearch &HeaderSearch) const; + IncludeFixerContext getIncludeFixerContext( + const clang::SourceManager &SourceManager, + clang::HeaderSearch &HeaderSearch, + ArrayRef MatchedSymbols) const; + + /// Get the global matched symbols. + ArrayRef getMatchedSymbols() const { + return MatchedSymbols; + } private: /// Query the database for a given identifier. - bool query(StringRef Query, StringRef ScopedQualifiers, tooling::Range Range); + std::vector + query(StringRef Query, StringRef ScopedQualifiers, tooling::Range Range); CompilerInstance *CI; diff --git a/clang-tools-extra/test/include-fixer/yamldb_plugin.cpp b/clang-tools-extra/test/include-fixer/yamldb_plugin.cpp index 5ec9f89..cadba9d 100644 --- a/clang-tools-extra/test/include-fixer/yamldb_plugin.cpp +++ b/clang-tools-extra/test/include-fixer/yamldb_plugin.cpp @@ -1,6 +1,7 @@ // RUN: c-index-test -test-load-source-reparse 2 all %s -Xclang -add-plugin -Xclang clang-include-fixer -fspell-checking -Xclang -plugin-arg-clang-include-fixer -Xclang -input=%p/Inputs/fake_yaml_db.yaml 2>&1 | FileCheck %s foo f; +unknown u; // CHECK: yamldb_plugin.cpp:3:1: error: unknown type name 'foo'; did you mean 'foo'? // CHECK: Number FIX-ITs = 1 @@ -8,3 +9,9 @@ foo f; // CHECK: yamldb_plugin.cpp:3:1: note: Add '#include "foo.h"' to provide the missing declaration [clang-include-fixer] // CHECK: Number FIX-ITs = 1 // CHECK: FIX-IT: Replace [3:1 - 3:4] with "#include "foo.h" +// CHECK: yamldb_plugin.cpp:4:1: +// CHECK: error: unknown type name 'unknown'; did you mean 'unknown'? +// CHECK: Number FIX-ITs = 1 +// CHECK: FIX-IT: Replace [4:1 - 4:8] with "unknown" +// CHECK-NOT: error +// CHECK-NOT: FIX-IT -- 2.7.4