[clang-tools-extra] r287544 - [include-fixer plugin] Make the plugin emit proper fixits in case multiple errors are found.

Benjamin Kramer via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 21 07:28:50 PST 2016


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




More information about the cfe-commits mailing list