From: Alex Lorenz Date: Mon, 16 Oct 2017 18:28:26 +0000 (+0000) Subject: [refactor] allow the use of refactoring diagnostics X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6de2efd1953adaa9a190b2cdfbe7b5c15f6d6efe;p=clang [refactor] allow the use of refactoring diagnostics 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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@315924 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/AllDiagnostics.h b/include/clang/Basic/AllDiagnostics.h index 3bbab923e0..1c83e2d0f8 100644 --- a/include/clang/Basic/AllDiagnostics.h +++ b/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 diff --git a/include/clang/Basic/CMakeLists.txt b/include/clang/Basic/CMakeLists.txt index decfdfde16..821c405913 100644 --- a/include/clang/Basic/CMakeLists.txt +++ b/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/include/clang/Basic/Diagnostic.td b/include/clang/Basic/Diagnostic.td index 54bd1489e3..52ccf350e6 100644 --- a/include/clang/Basic/Diagnostic.td +++ b/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/include/clang/Basic/DiagnosticIDs.h b/include/clang/Basic/DiagnosticIDs.h index 5ebed9435b..ca04ccacf6 100644 --- a/include/clang/Basic/DiagnosticIDs.h +++ b/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/include/clang/Basic/DiagnosticRefactoringKinds.td b/include/clang/Basic/DiagnosticRefactoringKinds.td new file mode 100644 index 0000000000..ba9c830d60 --- /dev/null +++ b/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/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h b/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h index 5e838a347c..36c982b063 100644 --- a/include/clang/Tooling/Refactoring/RefactoringActionRuleRequirements.h +++ b/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 @@ public: Expected evaluate(RefactoringRuleContext &Context) const { if (Context.getSelectionRange().isValid()) return Context.getSelectionRange(); - // FIXME: Use a diagnostic. - return llvm::make_error( - "refactoring action can't be initiated without a selection", - llvm::inconvertibleErrorCode()); + return Context.createDiagnosticError(diag::err_refactor_no_selection); } }; diff --git a/include/clang/Tooling/Refactoring/RefactoringDiagnostic.h b/include/clang/Tooling/Refactoring/RefactoringDiagnostic.h new file mode 100644 index 0000000000..6767dc708e --- /dev/null +++ b/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/include/clang/Tooling/Refactoring/RefactoringRuleContext.h b/include/clang/Tooling/Refactoring/RefactoringRuleContext.h index 877ddef737..54ea06d861 100644 --- a/include/clang/Tooling/Refactoring/RefactoringRuleContext.h +++ b/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 @@ public: 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 @@ private: /// 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/include/clang/module.modulemap b/include/clang/module.modulemap index a09027daeb..1e3262188f 100644 --- a/include/clang/module.modulemap +++ b/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/lib/Basic/DiagnosticIDs.cpp b/lib/Basic/DiagnosticIDs.cpp index 1a7b835ebc..4f59e550ba 100644 --- a/lib/Basic/DiagnosticIDs.cpp +++ b/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/lib/Tooling/Refactoring/Rename/RenamingAction.cpp b/lib/Tooling/Refactoring/Rename/RenamingAction.cpp index 547d7bb6bc..fdff6fd636 100644 --- a/lib/Tooling/Refactoring/Rename/RenamingAction.cpp +++ b/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 @@ public: return Selection.takeError(); const NamedDecl *ND = getNamedDeclAt(Context.getASTContext(), Selection->getBegin()); - if (!ND) { - // FIXME: Use a diagnostic. - return llvm::make_error("no symbol selected", - llvm::inconvertibleErrorCode()); - } + if (!ND) + return Context.createDiagnosticError( + Selection->getBegin(), diag::err_refactor_selection_no_symbol); return getCanonicalSymbolDeclaration(ND); } }; diff --git a/test/Refactor/LocalRename/NoSymbolSelectedError.cpp b/test/Refactor/LocalRename/NoSymbolSelectedError.cpp new file mode 100644 index 0000000000..b6e96e11b7 --- /dev/null +++ b/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/tools/clang-refactor/ClangRefactor.cpp b/tools/clang-refactor/ClangRefactor.cpp index 8a5c8e6630..ed9bf5ed28 100644 --- a/tools/clang-refactor/ClangRefactor.cpp +++ b/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 @@ public: /// 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 createCustomConsumer() { + virtual std::unique_ptr + createCustomConsumer() { return nullptr; } @@ -85,7 +87,8 @@ public: void print(raw_ostream &OS) override { TestSelections.dump(OS); } - std::unique_ptr createCustomConsumer() override { + std::unique_ptr + createCustomConsumer() override { return TestSelections.createConsumer(); } @@ -304,10 +307,20 @@ private: 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 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 @@ public: 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 @@ public: "The action must have at least one selection rule"); }; + std::unique_ptr 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/tools/clang-refactor/TestSupport.cpp b/tools/clang-refactor/TestSupport.cpp index 66e1bb7807..2f127bd7f6 100644 --- a/tools/clang-refactor/TestSupport.cpp +++ b/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 +std::unique_ptr TestSelectionRangesInFile::createConsumer() const { return llvm::make_unique(*this); } diff --git a/tools/clang-refactor/TestSupport.h b/tools/clang-refactor/TestSupport.h index 9d082a74ec..101f35bd94 100644 --- a/tools/clang-refactor/TestSupport.h +++ b/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 Callback) const; - std::unique_ptr createConsumer() const; + std::unique_ptr createConsumer() const; void dump(llvm::raw_ostream &OS) const; }; diff --git a/tools/clang-refactor/ToolRefactoringResultConsumer.h b/tools/clang-refactor/ToolRefactoringResultConsumer.h new file mode 100644 index 0000000000..64a994d88b --- /dev/null +++ b/tools/clang-refactor/ToolRefactoringResultConsumer.h @@ -0,0 +1,48 @@ +//===--- ToolRefactoringResultConsumer.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_TOOLS_CLANG_REFACTOR_TOOL_REFACTORING_RESULT_CONSUMER_H +#define LLVM_CLANG_TOOLS_CLANG_REFACTOR_TOOL_REFACTORING_RESULT_CONSUMER_H + +#include "clang/AST/ASTContext.h" +#include "clang/Tooling/Refactoring/RefactoringResultConsumer.h" + +namespace clang { +namespace refactor { + +/// An interface that subclasses the \c RefactoringResultConsumer interface +/// that stores the reference to the TU-specific diagnostics engine. +class ClangRefactorToolConsumerInterface + : public tooling::RefactoringResultConsumer { +public: + /// Called when a TU is entered. + void beginTU(ASTContext &Context) { + assert(!Diags && "Diags has been set"); + Diags = &Context.getDiagnostics(); + } + + /// Called when the tool is done with a TU. + void endTU() { + assert(Diags && "Diags unset"); + Diags = nullptr; + } + + DiagnosticsEngine &getDiags() const { + assert(Diags && "no diags"); + return *Diags; + } + +private: + DiagnosticsEngine *Diags = nullptr; +}; + +} // end namespace refactor +} // end namespace clang + +#endif // LLVM_CLANG_TOOLS_CLANG_REFACTOR_TOOL_REFACTORING_RESULT_CONSUMER_H diff --git a/tools/diagtool/DiagnosticNames.cpp b/tools/diagtool/DiagnosticNames.cpp index 4701f8e61c..b0ca7f9806 100644 --- a/tools/diagtool/DiagnosticNames.cpp +++ b/tools/diagtool/DiagnosticNames.cpp @@ -42,6 +42,7 @@ static const DiagnosticRecord BuiltinDiagnosticsByID[] = { #include "clang/Basic/DiagnosticCommentKinds.inc" #include "clang/Basic/DiagnosticSemaKinds.inc" #include "clang/Basic/DiagnosticAnalysisKinds.inc" +#include "clang/Basic/DiagnosticRefactoringKinds.inc" #undef DIAG }; diff --git a/unittests/Tooling/RefactoringActionRulesTest.cpp b/unittests/Tooling/RefactoringActionRulesTest.cpp index ffd54cc33e..132a3a4496 100644 --- a/unittests/Tooling/RefactoringActionRulesTest.cpp +++ b/unittests/Tooling/RefactoringActionRulesTest.cpp @@ -11,6 +11,7 @@ #include "RewriterTestContext.h" #include "clang/Tooling/Refactoring.h" #include "clang/Tooling/Refactoring/RefactoringActionRules.h" +#include "clang/Tooling/Refactoring/RefactoringDiagnostic.h" #include "clang/Tooling/Refactoring/Rename/SymbolName.h" #include "clang/Tooling/Tooling.h" #include "llvm/Support/Errc.h" @@ -128,12 +129,12 @@ TEST_F(RefactoringActionRulesTest, MyFirstRefactoringRule) { createReplacements(Rule, RefContext); ASSERT_TRUE(!ErrorOrResult); - std::string Message; - llvm::handleAllErrors( - ErrorOrResult.takeError(), - [&](llvm::StringError &Error) { Message = Error.getMessage(); }); - EXPECT_EQ(Message, - "refactoring action can't be initiated without a selection"); + unsigned DiagID; + llvm::handleAllErrors(ErrorOrResult.takeError(), + [&](DiagnosticError &Error) { + DiagID = Error.getDiagnostic().second.getDiagID(); + }); + EXPECT_EQ(DiagID, diag::err_refactor_no_selection); } }