[refactor] allow the use of refactoring diagnostics

hyp · hyp · commit f5ca27cc3785 · 2017-10-16T18:28:26.000Z
This commit allows the refactoring library to use its own set of refactoring-specific diagnostics to reports things like initiation errors. Differential Revision: https://reviews.llvm.org/D38772 llvm-svn: 315924
diff --git a/clang/include/clang/Basic/AllDiagnostics.h b/clang/include/clang/Basic/AllDiagnostics.h
@@ -25,6 +25,7 @@
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Serialization/SerializationDiagnostic.h"
+#include "clang/Tooling/Refactoring/RefactoringDiagnostic.h"
 
 namespace clang {
 template <size_t SizeOfStr, typename FieldType>
diff --git a/clang/include/clang/Basic/CMakeLists.txt b/clang/include/clang/Basic/CMakeLists.txt
@@ -14,6 +14,7 @@ clang_diag_gen(Driver)
 clang_diag_gen(Frontend)
 clang_diag_gen(Lex)
 clang_diag_gen(Parse)
+clang_diag_gen(Refactoring)
 clang_diag_gen(Sema)
 clang_diag_gen(Serialization)
 clang_tablegen(DiagnosticGroups.inc -gen-clang-diag-groups
diff --git a/clang/include/clang/Basic/Diagnostic.td b/clang/include/clang/Basic/Diagnostic.td
@@ -138,6 +138,7 @@ include "DiagnosticDriverKinds.td"
 include "DiagnosticFrontendKinds.td"
 include "DiagnosticLexKinds.td"
 include "DiagnosticParseKinds.td"
+include "DiagnosticRefactoringKinds.td"
 include "DiagnosticSemaKinds.td"
 include "DiagnosticSerializationKinds.td"
 
diff --git a/clang/include/clang/Basic/DiagnosticIDs.h b/clang/include/clang/Basic/DiagnosticIDs.h
@@ -38,7 +38,8 @@ namespace clang {
       DIAG_SIZE_COMMENT       =  100,
       DIAG_SIZE_CROSSTU       =  100,
       DIAG_SIZE_SEMA          = 3500,
-      DIAG_SIZE_ANALYSIS      =  100
+      DIAG_SIZE_ANALYSIS      =  100,
+      DIAG_SIZE_REFACTORING   = 1000,
     };
     // Start position for diagnostics.
     enum {
@@ -53,7 +54,8 @@ namespace clang {
       DIAG_START_CROSSTU       = DIAG_START_COMMENT       + DIAG_SIZE_CROSSTU,
       DIAG_START_SEMA          = DIAG_START_CROSSTU       + DIAG_SIZE_COMMENT,
       DIAG_START_ANALYSIS      = DIAG_START_SEMA          + DIAG_SIZE_SEMA,
-      DIAG_UPPER_LIMIT         = DIAG_START_ANALYSIS      + DIAG_SIZE_ANALYSIS
+      DIAG_START_REFACTORING   = DIAG_START_ANALYSIS      + DIAG_SIZE_ANALYSIS,
+      DIAG_UPPER_LIMIT         = DIAG_START_REFACTORING   + DIAG_SIZE_REFACTORING
     };
 
     class CustomDiagInfo;
diff --git a/clang/include/clang/Basic/DiagnosticRefactoringKinds.td b/clang/include/clang/Basic/DiagnosticRefactoringKinds.td
@@ -0,0 +1,25 @@
+//==--- DiagnosticRefactoringKinds.td - refactoring diagnostics -----------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+//===----------------------------------------------------------------------===//
+// Refactoring Diagnostics
+//===----------------------------------------------------------------------===//
+
+let Component = "Refactoring" in {
+
+let CategoryName = "Refactoring Invocation Issue" in {
+
+def err_refactor_no_selection : Error<"refactoring action can't be initiated "
+  "without a selection">;
+def err_refactor_selection_no_symbol : Error<"there is no symbol at the given "
+  "location">;
+
+}
+
+} // end of Refactoring diagnostics
diff --git a/clang/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h b/clang/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h
@@ -11,6 +11,7 @@
 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_ACTION_RULE_REQUIREMENTS_H
 
 #include "clang/Basic/LLVM.h"
+#include "clang/Tooling/Refactoring/RefactoringDiagnostic.h"
 #include "clang/Tooling/Refactoring/RefactoringOption.h"
 #include "clang/Tooling/Refactoring/RefactoringRuleContext.h"
 #include "llvm/Support/Error.h"
@@ -47,10 +48,7 @@ class SourceRangeSelectionRequirement : public SourceSelectionRequirement {
   Expected<SourceRange> evaluate(RefactoringRuleContext &Context) const {
     if (Context.getSelectionRange().isValid())
       return Context.getSelectionRange();
-    // FIXME: Use a diagnostic.
-    return llvm::make_error<llvm::StringError>(
-        "refactoring action can't be initiated without a selection",
-        llvm::inconvertibleErrorCode());
+    return Context.createDiagnosticError(diag::err_refactor_no_selection);
   }
 };
 
diff --git a/clang/include/clang/Tooling/Refactoring/RefactoringDiagnostic.h b/clang/include/clang/Tooling/Refactoring/RefactoringDiagnostic.h
@@ -0,0 +1,30 @@
+//===--- RefactoringDiagnostic.h - ------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLING_REFACTORING_REFACTORINGDIAGNOSTIC_H
+#define LLVM_CLANG_TOOLING_REFACTORING_REFACTORINGDIAGNOSTIC_H
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/PartialDiagnostic.h"
+
+namespace clang {
+namespace diag {
+enum {
+#define DIAG(ENUM, FLAGS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR,      \
+             SHOWINSYSHEADER, CATEGORY)                                        \
+  ENUM,
+#define REFACTORINGSTART
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+#undef DIAG
+  NUM_BUILTIN_REFACTORING_DIAGNOSTICS
+};
+} // end namespace diag
+} // end namespace clang
+
+#endif // LLVM_CLANG_TOOLING_REFACTORING_REFACTORINGDIAGNOSTIC_H
diff --git a/clang/include/clang/Tooling/Refactoring/RefactoringRuleContext.h b/clang/include/clang/Tooling/Refactoring/RefactoringRuleContext.h
@@ -10,6 +10,7 @@
 #ifndef LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RULE_CONTEXT_H
 #define LLVM_CLANG_TOOLING_REFACTOR_REFACTORING_RULE_CONTEXT_H
 
+#include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/SourceManager.h"
 
 namespace clang {
@@ -50,6 +51,17 @@ class RefactoringRuleContext {
 
   void setASTContext(ASTContext &Context) { AST = &Context; }
 
+  /// Creates an llvm::Error value that contains a diagnostic.
+  ///
+  /// The errors should not outlive the context.
+  llvm::Error createDiagnosticError(SourceLocation Loc, unsigned DiagID) {
+    return DiagnosticError::create(Loc, PartialDiagnostic(DiagID, DiagStorage));
+  }
+
+  llvm::Error createDiagnosticError(unsigned DiagID) {
+    return createDiagnosticError(SourceLocation(), DiagID);
+  }
+
 private:
   /// The source manager for the translation unit / file on which a refactoring
   /// action might operate on.
@@ -60,6 +72,8 @@ class RefactoringRuleContext {
   /// An optional AST for the translation unit on which a refactoring action
   /// might operate on.
   ASTContext *AST = nullptr;
+  /// The allocator for diagnostics.
+  PartialDiagnostic::StorageAllocator DiagStorage;
 };
 
 } // end namespace tooling
diff --git a/clang/include/clang/module.modulemap b/clang/include/clang/module.modulemap
@@ -71,6 +71,7 @@ module Clang_Diagnostics {
   module Parse { header "Parse/ParseDiagnostic.h" export * }
   module Sema { header "Sema/SemaDiagnostic.h" export * }
   module Serialization { header "Serialization/SerializationDiagnostic.h" export * }
+  module Refactoring { header "Tooling/Refactoring/RefactoringDiagnostic.h" export * }
 }
 
 module Clang_Driver {
diff --git a/clang/lib/Basic/DiagnosticIDs.cpp b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -43,7 +43,7 @@ struct StaticDiagInfoRec {
   unsigned SFINAE : 2;
   unsigned WarnNoWerror : 1;
   unsigned WarnShowInSystemHeader : 1;
-  unsigned Category : 5;
+  unsigned Category : 6;
 
   uint16_t OptionGroupIndex;
 
@@ -88,6 +88,7 @@ VALIDATE_DIAG_SIZE(AST)
 VALIDATE_DIAG_SIZE(COMMENT)
 VALIDATE_DIAG_SIZE(SEMA)
 VALIDATE_DIAG_SIZE(ANALYSIS)
+VALIDATE_DIAG_SIZE(REFACTORING)
 #undef VALIDATE_DIAG_SIZE
 #undef STRINGIFY_NAME
 
@@ -112,6 +113,7 @@ static const StaticDiagInfoRec StaticDiagInfo[] = {
 #include "clang/Basic/DiagnosticCrossTUKinds.inc"
 #include "clang/Basic/DiagnosticSemaKinds.inc"
 #include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
 #undef DIAG
 };
 
@@ -150,6 +152,7 @@ CATEGORY(COMMENT, AST)
 CATEGORY(CROSSTU, COMMENT)
 CATEGORY(SEMA, CROSSTU)
 CATEGORY(ANALYSIS, SEMA)
+CATEGORY(REFACTORING, ANALYSIS)
 #undef CATEGORY
 
   // Avoid out of bounds reads.
diff --git a/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp b/clang/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
@@ -23,6 +23,7 @@
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
 #include "clang/Tooling/Refactoring/RefactoringAction.h"
+#include "clang/Tooling/Refactoring/RefactoringDiagnostic.h"
 #include "clang/Tooling/Refactoring/RefactoringOptions.h"
 #include "clang/Tooling/Refactoring/Rename/SymbolName.h"
 #include "clang/Tooling/Refactoring/Rename/USRFinder.h"
@@ -49,11 +50,9 @@ class SymbolSelectionRequirement : public SourceRangeSelectionRequirement {
       return Selection.takeError();
     const NamedDecl *ND =
         getNamedDeclAt(Context.getASTContext(), Selection->getBegin());
-    if (!ND) {
-      // FIXME: Use a diagnostic.
-      return llvm::make_error<StringError>("no symbol selected",
-                                           llvm::inconvertibleErrorCode());
-    }
+    if (!ND)
+      return Context.createDiagnosticError(
+          Selection->getBegin(), diag::err_refactor_selection_no_symbol);
     return getCanonicalSymbolDeclaration(ND);
   }
 };
diff --git a/clang/test/Refactor/LocalRename/NoSymbolSelectedError.cpp b/clang/test/Refactor/LocalRename/NoSymbolSelectedError.cpp
@@ -0,0 +1,8 @@
+// RUN: not clang-refactor local-rename -selection=%s:4:1 -new-name=Bar %s -- 2>&1 | FileCheck %s
+// RUN: clang-refactor local-rename -selection=test:%s -new-name=Bar %s -- 2>&1 | FileCheck --check-prefix=TESTCHECK %s
+
+class Baz { // CHECK: [[@LINE]]:1: error: there is no symbol at the given location
+};
+/*range=*/;
+// TESTCHECK:      1 '' results:
+// TESTCHECK-NEXT: there is no symbol at the given location
diff --git a/clang/tools/clang-refactor/ClangRefactor.cpp b/clang/tools/clang-refactor/ClangRefactor.cpp
@@ -15,6 +15,7 @@
 
 #include "TestSupport.h"
 #include "clang/Frontend/CommandLineSourceLoc.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Refactoring.h"
@@ -65,7 +66,8 @@ class SourceSelectionArgument {
   /// logic into the refactoring operation. The test-specific consumer
   /// ensures that the individual results in a particular test group are
   /// identical.
-  virtual std::unique_ptr<RefactoringResultConsumer> createCustomConsumer() {
+  virtual std::unique_ptr<ClangRefactorToolConsumerInterface>
+  createCustomConsumer() {
     return nullptr;
   }
 
@@ -85,7 +87,8 @@ class TestSourceSelectionArgument final : public SourceSelectionArgument {
 
   void print(raw_ostream &OS) override { TestSelections.dump(OS); }
 
-  std::unique_ptr<RefactoringResultConsumer> createCustomConsumer() override {
+  std::unique_ptr<ClangRefactorToolConsumerInterface>
+  createCustomConsumer() override {
     return TestSelections.createConsumer();
   }
 
@@ -304,10 +307,20 @@ class RefactoringActionSubcommand : public cl::SubCommand {
   RefactoringActionCommandLineOptions Options;
 };
 
-class ClangRefactorConsumer : public RefactoringResultConsumer {
+class ClangRefactorConsumer final : public ClangRefactorToolConsumerInterface {
 public:
+  ClangRefactorConsumer() {}
+
   void handleError(llvm::Error Err) override {
-    llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+    Optional<PartialDiagnosticAt> Diag = DiagnosticError::take(Err);
+    if (!Diag) {
+      llvm::errs() << llvm::toString(std::move(Err)) << "\n";
+      return;
+    }
+    llvm::cantFail(std::move(Err)); // This is a success.
+    DiagnosticBuilder DB(
+        getDiags().Report(Diag->first, Diag->second.getDiagID()));
+    Diag->second.Emit(DB);
   }
 
   void handle(AtomicChanges Changes) override {
@@ -468,8 +481,8 @@ class ClangRefactorTool {
       return true;
     }
 
-    bool HasFailed = false;
     ClangRefactorConsumer Consumer;
+    bool HasFailed = false;
     if (foreachTranslationUnit(DB, Sources, [&](ASTContext &AST) {
           RefactoringRuleContext Context(AST.getSourceManager());
           Context.setASTContext(AST);
@@ -488,21 +501,27 @@ class ClangRefactorTool {
                 "The action must have at least one selection rule");
           };
 
+          std::unique_ptr<ClangRefactorToolConsumerInterface> CustomConsumer;
+          if (HasSelection)
+            CustomConsumer = Subcommand.getSelection()->createCustomConsumer();
+          ClangRefactorToolConsumerInterface &ActiveConsumer =
+              CustomConsumer ? *CustomConsumer : Consumer;
+          ActiveConsumer.beginTU(AST);
           if (HasSelection) {
             assert(Subcommand.getSelection() && "Missing selection argument?");
             if (opts::Verbose)
               Subcommand.getSelection()->print(llvm::outs());
-            auto CustomConsumer =
-                Subcommand.getSelection()->createCustomConsumer();
             if (Subcommand.getSelection()->forAllRanges(
                     Context.getSources(), [&](SourceRange R) {
                       Context.setSelectionRange(R);
-                      InvokeRule(CustomConsumer ? *CustomConsumer : Consumer);
+                      InvokeRule(ActiveConsumer);
                     }))
               HasFailed = true;
+            ActiveConsumer.endTU();
             return;
           }
           // FIXME (Alex L): Implement non-selection based invocation path.
+          ActiveConsumer.endTU();
         }))
       return true;
     return HasFailed || applySourceChanges(Consumer.getSourceChanges());
diff --git a/clang/tools/clang-refactor/TestSupport.cpp b/clang/tools/clang-refactor/TestSupport.cpp
@@ -14,6 +14,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "TestSupport.h"
+#include "clang/Basic/DiagnosticError.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
@@ -106,7 +107,7 @@ bool printRewrittenSources(const tooling::AtomicChanges &Changes,
 }
 
 class TestRefactoringResultConsumer final
-    : public tooling::RefactoringResultConsumer {
+    : public ClangRefactorToolConsumerInterface {
 public:
   TestRefactoringResultConsumer(const TestSelectionRangesInFile &TestRanges)
       : TestRanges(TestRanges) {
@@ -182,10 +183,15 @@ bool TestRefactoringResultConsumer::handleAllResults() {
       std::string ErrorMessage;
       bool HasResult = !!Result;
       if (!HasResult) {
-        // FIXME: Handle diagnostic error as well.
-        handleAllErrors(Result.takeError(), [&](StringError &Err) {
-          ErrorMessage = Err.getMessage();
-        });
+        handleAllErrors(
+            Result.takeError(),
+            [&](StringError &Err) { ErrorMessage = Err.getMessage(); },
+            [&](DiagnosticError &Err) {
+              const PartialDiagnosticAt &Diag = Err.getDiagnostic();
+              llvm::SmallString<100> DiagText;
+              Diag.second.EmitToString(getDiags(), DiagText);
+              ErrorMessage = DiagText.str().str();
+            });
       }
       if (!CanonicalResult && !CanonicalErrorMessage) {
         if (HasResult)
@@ -248,7 +254,7 @@ bool TestRefactoringResultConsumer::handleAllResults() {
   return Failed;
 }
 
-std::unique_ptr<tooling::RefactoringResultConsumer>
+std::unique_ptr<ClangRefactorToolConsumerInterface>
 TestSelectionRangesInFile::createConsumer() const {
   return llvm::make_unique<TestRefactoringResultConsumer>(*this);
 }
diff --git a/clang/tools/clang-refactor/TestSupport.h b/clang/tools/clang-refactor/TestSupport.h
@@ -16,9 +16,9 @@
 #ifndef LLVM_CLANG_TOOLS_CLANG_REFACTOR_TEST_SUPPORT_H
 #define LLVM_CLANG_TOOLS_CLANG_REFACTOR_TEST_SUPPORT_H
 
+#include "ToolRefactoringResultConsumer.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Error.h"
@@ -65,7 +65,7 @@ struct TestSelectionRangesInFile {
   bool foreachRange(const SourceManager &SM,
                     llvm::function_ref<void(SourceRange)> Callback) const;
 
-  std::unique_ptr<tooling::RefactoringResultConsumer> createConsumer() const;
+  std::unique_ptr<ClangRefactorToolConsumerInterface> createConsumer() const;
 
   void dump(llvm::raw_ostream &OS) const;
 };
diff --git a/clang/tools/clang-refactor/ToolRefactoringResultConsumer.h b/clang/tools/clang-refactor/ToolRefactoringResultConsumer.h
diff --git a/clang/tools/diagtool/DiagnosticNames.cpp b/clang/tools/diagtool/DiagnosticNames.cpp
diff --git a/clang/unittests/Tooling/RefactoringActionRulesTest.cpp b/clang/unittests/Tooling/RefactoringActionRulesTest.cpp

Original file line number	Diff line number	Diff line change
`@@ -71,6 +71,7 @@ module Clang_Diagnostics {`
`71`	`71`	`module Parse { header "Parse/ParseDiagnostic.h" export * }`
`72`	`72`	`module Sema { header "Sema/SemaDiagnostic.h" export * }`
`73`	`73`	`module Serialization { header "Serialization/SerializationDiagnostic.h" export * }`
	`74`	`+ module Refactoring { header "Tooling/Refactoring/RefactoringDiagnostic.h" export * }`
`74`	`75`	`}`
`75`	`76`
`76`	`77`	`module Clang_Driver {`