Author: d0k Date: Mon Nov 21 09:28:50 2016 New Revision: 287544 URL: http://llvm.org/viewvc/llvm-project?rev=287544&view=rev Log: [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. Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp clang-tools-extra/trunk/include-fixer/IncludeFixer.h clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp?rev=287544&r1=287543&r2=287544&view=diff ============================================================================== --- clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp (original) +++ clang-tools-extra/trunk/include-fixer/IncludeFixer.cpp Mon Nov 21 09:28:50 2016 @@ -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::MaybeDiagno 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<find_all_symbols::SymbolInfo> 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::MaybeDiagno 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 IncludeFixerSemaSo } DEBUG(llvm::dbgs() << "TypoScopeQualifiers: " << TypoScopeString << "\n"); - query(QueryString, TypoScopeString, SymbolRange); + std::vector<find_all_symbols::SymbolInfo> 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::mini /// Get the include fixer context for the queried symbol. IncludeFixerContext IncludeFixerSemaSource::getIncludeFixerContext( const clang::SourceManager &SourceManager, - clang::HeaderSearch &HeaderSearch) const { + clang::HeaderSearch &HeaderSearch, + ArrayRef<find_all_symbols::SymbolInfo> MatchedSymbols) const { std::vector<find_all_symbols::SymbolInfo> SymbolCandidates; for (const auto &Symbol : MatchedSymbols) { std::string FilePath = Symbol.getFilePath().str(); @@ -332,8 +338,9 @@ IncludeFixerContext IncludeFixerSemaSour return IncludeFixerContext(FilePath, QuerySymbolInfos, SymbolCandidates); } -bool IncludeFixerSemaSource::query(StringRef Query, StringRef ScopedQualifiers, - tooling::Range Range) { +std::vector<find_all_symbols::SymbolInfo> +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(Strin 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(Strin // 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<find_all_symbols::SymbolInfo> 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<tooling::Replacements> createIncludeFixerReplacements( Modified: clang-tools-extra/trunk/include-fixer/IncludeFixer.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/IncludeFixer.h?rev=287544&r1=287543&r2=287544&view=diff ============================================================================== --- clang-tools-extra/trunk/include-fixer/IncludeFixer.h (original) +++ clang-tools-extra/trunk/include-fixer/IncludeFixer.h Mon Nov 21 09:28:50 2016 @@ -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<find_all_symbols::SymbolInfo> MatchedSymbols) const; + + /// Get the global matched symbols. + ArrayRef<find_all_symbols::SymbolInfo> getMatchedSymbols() const { + return MatchedSymbols; + } private: /// Query the database for a given identifier. - bool query(StringRef Query, StringRef ScopedQualifiers, tooling::Range Range); + std::vector<find_all_symbols::SymbolInfo> + query(StringRef Query, StringRef ScopedQualifiers, tooling::Range Range); CompilerInstance *CI; Modified: clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp?rev=287544&r1=287543&r2=287544&view=diff ============================================================================== --- clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp (original) +++ clang-tools-extra/trunk/test/include-fixer/yamldb_plugin.cpp Mon Nov 21 09:28:50 2016 @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits