diff options
24 files changed, 337 insertions, 299 deletions
diff --git a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp index 7a656a668e1..4a6707bae3a 100644 --- a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp +++ b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp @@ -19,6 +19,7 @@ #include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" #include "clang/Rewrite/Core/Rewriter.h" +#include "clang/Tooling/Core/Diagnostic.h" #include "clang/Tooling/DiagnosticsYaml.h" #include "clang/Tooling/ReplacementsYaml.h" #include "llvm/ADT/ArrayRef.h" @@ -169,9 +170,11 @@ groupReplacements(const TUReplacements &TUs, const TUDiagnostics &TUDs, for (const auto &TU : TUDs) for (const auto &D : TU.Diagnostics) - for (const auto &Fix : D.Fix) - for (const tooling::Replacement &R : Fix.second) - AddToGroup(R, true); + if (const auto *ChoosenFix = tooling::selectFirstFix(D)) { + for (const auto &Fix : *ChoosenFix) + for (const tooling::Replacement &R : Fix.second) + AddToGroup(R, true); + } // Sort replacements per file to keep consistent behavior when // clang-apply-replacements run on differents machine. diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 6af8863bc9a..6b8428bd929 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -35,6 +35,7 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Rewrite/Frontend/FixItRewriter.h" #include "clang/Rewrite/Frontend/FrontendActions.h" +#include "clang/Tooling/Core/Diagnostic.h" #if CLANG_ENABLE_STATIC_ANALYZER #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h" @@ -125,15 +126,17 @@ public: } auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]")) << Message.Message << Name; - for (const auto &FileAndReplacements : Error.Fix) { - for (const auto &Repl : FileAndReplacements.second) { - SourceLocation FixLoc; - ++TotalFixes; - bool CanBeApplied = false; - if (Repl.isApplicable()) { - SmallString<128> FixAbsoluteFilePath = Repl.getFilePath(); - Files.makeAbsolutePath(FixAbsoluteFilePath); - if (ApplyFixes) { + // FIXME: explore options to support interactive fix selection. + const llvm::StringMap<Replacements> *ChosenFix = selectFirstFix(Error); + if (ApplyFixes && ChosenFix) { + for (const auto &FileAndReplacements : *ChosenFix) { + for (const auto &Repl : FileAndReplacements.second) { + ++TotalFixes; + bool CanBeApplied = false; + if (Repl.isApplicable()) { + SourceLocation FixLoc; + SmallString<128> FixAbsoluteFilePath = Repl.getFilePath(); + Files.makeAbsolutePath(FixAbsoluteFilePath); tooling::Replacement R(FixAbsoluteFilePath, Repl.getOffset(), Repl.getLength(), Repl.getReplacementText()); @@ -158,28 +161,17 @@ public: llvm::errs() << "Can't resolve conflict, skipping the replacement.\n"; } - } else { CanBeApplied = true; ++AppliedFixes; } + FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset()); + FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied)); } - FixLoc = getLocation(FixAbsoluteFilePath, Repl.getOffset()); - SourceLocation FixEndLoc = - FixLoc.getLocWithOffset(Repl.getLength()); - // Retrieve the source range for applicable fixes. Macro definitions - // on the command line have locations in a virtual buffer and don't - // have valid file paths and are therefore not applicable. - CharSourceRange Range = - CharSourceRange::getCharRange(SourceRange(FixLoc, FixEndLoc)); - Diag << FixItHint::CreateReplacement(Range, - Repl.getReplacementText()); } - - if (ApplyFixes) - FixLocations.push_back(std::make_pair(FixLoc, CanBeApplied)); } } + reportFix(Diag, Error.Message.Fix); } for (auto Fix : FixLocations) { Diags.Report(Fix.first, Fix.second ? diag::note_fixit_applied @@ -250,10 +242,33 @@ private: return SourceMgr.getLocForStartOfFile(ID).getLocWithOffset(Offset); } + void reportFix(const DiagnosticBuilder &Diag, + const llvm::StringMap<Replacements> &Fix) { + for (const auto &FileAndReplacements : Fix) { + for (const auto &Repl : FileAndReplacements.second) { + if (!Repl.isApplicable()) + continue; + SmallString<128> FixAbsoluteFilePath = Repl.getFilePath(); + Files.makeAbsolutePath(FixAbsoluteFilePath); + SourceLocation FixLoc = + getLocation(FixAbsoluteFilePath, Repl.getOffset()); + SourceLocation FixEndLoc = FixLoc.getLocWithOffset(Repl.getLength()); + // Retrieve the source range for applicable fixes. Macro definitions + // on the command line have locations in a virtual buffer and don't + // have valid file paths and are therefore not applicable. + CharSourceRange Range = + CharSourceRange::getCharRange(SourceRange(FixLoc, FixEndLoc)); + Diag << FixItHint::CreateReplacement(Range, Repl.getReplacementText()); + } + } + } + void reportNote(const tooling::DiagnosticMessage &Message) { SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset); - Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0")) + auto Diag = + Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0")) << Message.Message; + reportFix(Diag, Message.Fix); } FileManager Files; diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index 0d6c0a94d45..0abd63ee56b 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -18,8 +18,10 @@ #include "ClangTidyDiagnosticConsumer.h" #include "ClangTidyOptions.h" #include "clang/AST/ASTDiagnostic.h" +#include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticOptions.h" #include "clang/Frontend/DiagnosticRenderer.h" +#include "clang/Tooling/Core/Diagnostic.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallString.h" #include <tuple> @@ -68,6 +70,9 @@ protected: SmallVectorImpl<CharSourceRange> &Ranges, ArrayRef<FixItHint> Hints) override { assert(Loc.isValid()); + tooling::DiagnosticMessage *DiagWithFix = + Level == DiagnosticsEngine::Note ? &Error.Notes.back() : &Error.Message; + for (const auto &FixIt : Hints) { CharSourceRange Range = FixIt.RemoveRange; assert(Range.getBegin().isValid() && Range.getEnd().isValid() && @@ -77,7 +82,8 @@ protected: tooling::Replacement Replacement(Loc.getManager(), Range, FixIt.CodeToInsert); - llvm::Error Err = Error.Fix[Replacement.getFilePath()].add(Replacement); + llvm::Error Err = + DiagWithFix->Fix[Replacement.getFilePath()].add(Replacement); // FIXME: better error handling (at least, don't let other replacements be // applied). if (Err) { @@ -581,9 +587,17 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() { // Compute error sizes. std::vector<int> Sizes; - for (const auto &Error : Errors) { + std::vector< + std::pair<ClangTidyError *, llvm::StringMap<tooling::Replacements> *>> + ErrorFixes; + for (auto &Error : Errors) { + if (const auto *Fix = tooling::selectFirstFix(Error)) + ErrorFixes.emplace_back( + &Error, const_cast<llvm::StringMap<tooling::Replacements> *>(Fix)); + } + for (const auto &ErrorAndFix : ErrorFixes) { int Size = 0; - for (const auto &FileAndReplaces : Error.Fix) { + for (const auto &FileAndReplaces : *ErrorAndFix.second) { for (const auto &Replace : FileAndReplaces.second) Size += Replace.getLength(); } @@ -592,8 +606,8 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() { // Build events from error intervals. std::map<std::string, std::vector<Event>> FileEvents; - for (unsigned I = 0; I < Errors.size(); ++I) { - for (const auto &FileAndReplace : Errors[I].Fix) { + for (unsigned I = 0; I < ErrorFixes.size(); ++I) { + for (const auto &FileAndReplace : *ErrorFixes[I].second) { for (const auto &Replace : FileAndReplace.second) { unsigned Begin = Replace.getOffset(); unsigned End = Begin + Replace.getLength(); @@ -608,7 +622,7 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() { } } - std::vector<bool> Apply(Errors.size(), true); + std::vector<bool> Apply(ErrorFixes.size(), true); for (auto &FileAndEvents : FileEvents) { std::vector<Event> &Events = FileAndEvents.second; // Sweep. @@ -627,10 +641,10 @@ void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() { assert(OpenIntervals == 0 && "Amount of begin/end points doesn't match"); } - for (unsigned I = 0; I < Errors.size(); ++I) { + for (unsigned I = 0; I < ErrorFixes.size(); ++I) { if (!Apply[I]) { - Errors[I].Fix.clear(); - Errors[I].Notes.emplace_back( + ErrorFixes[I].second->clear(); + ErrorFixes[I].first->Notes.emplace_back( "this fix will not be applied because it overlaps with another fix"); } } diff --git a/clang-tools-extra/clang-tidy/add_new_check.py b/clang-tools-extra/clang-tidy/add_new_check.py index 8cda7b48ea3..8f96cbd874e 100755 --- a/clang-tools-extra/clang-tidy/add_new_check.py +++ b/clang-tools-extra/clang-tidy/add_new_check.py @@ -137,7 +137,8 @@ void %(check_name)s::check(const MatchFinder::MatchResult &Result) { if (MatchedDecl->getName().startswith("awesome_")) return; diag(MatchedDecl->getLocation(), "function %%0 is insufficiently awesome") - << MatchedDecl + << MatchedDecl; + diag(MatchedDecl->getLocation(), "insert 'awesome'", DiagnosticIDs::Note) << FixItHint::CreateInsertion(MatchedDecl->getLocation(), "awesome_"); } diff --git a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp index 532dbfd4cdc..3dabca917ae 100644 --- a/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp @@ -154,7 +154,10 @@ void UnusedUsingDeclsCheck::onEndOfTranslationUnit() { for (const auto &Context : Contexts) { if (!Context.IsUsed) { diag(Context.FoundUsingDecl->getLocation(), "using decl %0 is unused") - << Context.FoundUsingDecl + << Context.FoundUsingDecl; + // Emit a fix and a fix description of the check; + diag(Context.FoundUsingDecl->getLocation(), + /*FixDescription=*/"remove the using", DiagnosticIDs::Note) << FixItHint::CreateRemoval(Context.UsingDeclRange); } } diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml index 3a5375e6e08..0703f62ccc6 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file1.yaml @@ -2,24 +2,25 @@ MainSourceFile: source1.cpp Diagnostics: - DiagnosticName: test-basic - Message: Fix - FilePath: $(path)/basic.h - FileOffset: 242 - Replacements: - - FilePath: $(path)/basic.h - Offset: 242 - Length: 26 - ReplacementText: 'auto & elem : ints' - - FilePath: $(path)/basic.h - Offset: 276 - Length: 22 - ReplacementText: '' - - FilePath: $(path)/basic.h - Offset: 298 - Length: 1 - ReplacementText: elem - - FilePath: $(path)/../basic/basic.h - Offset: 148 - Length: 0 - ReplacementText: 'override ' + DiagnosticMessage: + Message: Fix + FilePath: $(path)/basic.h + FileOffset: 242 + Replacements: + - FilePath: $(path)/basic.h + Offset: 242 + Length: 26 + ReplacementText: 'auto & elem : ints' + - FilePath: $(path)/basic.h + Offset: 276 + Length: 22 + ReplacementText: '' + - FilePath: $(path)/basic.h + Offset: 298 + Length: 1 + ReplacementText: elem + - FilePath: $(path)/../basic/basic.h + Offset: 148 + Length: 0 + ReplacementText: 'override ' ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml index 8f6f65ad457..de99d860cb5 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/basic/file2.yaml @@ -2,12 +2,13 @@ MainSourceFile: source2.cpp Diagnostics: - DiagnosticName: test-basic - Message: Fix - FilePath: $(path)/basic.h - FileOffset: 148 - Replacements: - - FilePath: $(path)/../basic/basic.h - Offset: 298 - Length: 1 - ReplacementText: elem + DiagnosticMessage: + Message: Fix + FilePath: $(path)/basic.h + FileOffset: 148 + Replacements: + - FilePath: $(path)/../basic/basic.h + Offset: 298 + Length: 1 + ReplacementText: elem ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml index 8505273617c..8dccc5674ee 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file1.yaml @@ -2,20 +2,21 @@ MainSourceFile: source1.cpp Diagnostics: - DiagnosticName: test-conflict - Message: Fix - FilePath: $(path)/common.h - FileOffset: 106 - Replacements: - - FilePath: $(path)/common.h - Offset: 106 - Length: 26 - ReplacementText: 'auto & i : ints' - - FilePath: $(path)/common.h - Offset: 140 - Length: 7 - ReplacementText: i - - FilePath: $(path)/common.h - Offset: 160 - Length: 12 - ReplacementText: '' + DiagnosticMessage: + Message: Fix + FilePath: $(path)/common.h + FileOffset: 106 + Replacements: + - FilePath: $(path)/common.h + Offset: 106 + Length: 26 + ReplacementText: 'auto & i : ints' + - FilePath: $(path)/common.h + Offset: 140 + Length: 7 + ReplacementText: i + - FilePath: $(path)/common.h + Offset: 160 + Length: 12 + ReplacementText: '' ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml index 32b35acfc27..d7339662fcb 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file2.yaml @@ -2,20 +2,21 @@ MainSourceFile: source2.cpp Diagnostics: - DiagnosticName: test-conflict - Message: Fix - FilePath: $(path)/common.h - FileOffset: 106 - Replacements: - - FilePath: $(path)/common.h - Offset: 106 - Length: 26 - ReplacementText: 'int & elem : ints' - - FilePath: $(path)/common.h - Offset: 140 - Length: 7 - ReplacementText: elem - - FilePath: $(path)/common.h - Offset: 169 - Length: 1 - ReplacementText: nullptr + DiagnosticMessage: + Message: Fix + FilePath: $(path)/common.h + FileOffset: 106 + Replacements: + - FilePath: $(path)/common.h + Offset: 106 + Length: 26 + ReplacementText: 'int & elem : ints' + - FilePath: $(path)/common.h + Offset: 140 + Length: 7 + ReplacementText: elem + - FilePath: $(path)/common.h + Offset: 169 + Length: 1 + ReplacementText: nullptr ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml index 88543e0a20b..fee1108c1b6 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/conflict/file3.yaml @@ -2,12 +2,13 @@ MainSourceFile: source1.cpp Diagnostics: - DiagnosticName: test-conflict - Message: Fix - FilePath: $(path)/common.h - FileOffset: 169 - Replacements: - - FilePath: $(path)/common.h - Offset: 169 - Length: 0 - ReplacementText: "(int*)" + DiagnosticMessage: + Message: Fix + FilePath: $(path)/common.h + FileOffset: 169 + Replacements: + - FilePath: $(path)/common.h + Offset: 169 + Length: 0 + ReplacementText: "(int*)" ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml index 06b058e3813..2c3d56e89ee 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/file1.yaml @@ -2,12 +2,13 @@ MainSourceFile: source1.cpp Diagnostics: - DiagnosticName: test-crlf - Message: Fix - FilePath: $(path)/crlf.cpp - FileOffset: 79 - Replacements: - - FilePath: $(path)/crlf.cpp - Offset: 79 - Length: 1 - ReplacementText: nullptr + DiagnosticMessage: + Message: Fix + FilePath: $(path)/crlf.cpp + FileOffset: 79 + Replacements: + - FilePath: $(path)/crlf.cpp + Offset: 79 + Length: 1 + ReplacementText: nullptr ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml index 3118cc6186c..48aafd9d2d0 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/format/no.yaml @@ -2,12 +2,13 @@ MainSourceFile: no.cpp Diagnostics: - DiagnosticName: test-no - Message: Fix - FilePath: $(path)/no.cpp - FileOffset: 94 - Replacements: - - FilePath: $(path)/no.cpp - Offset: 94 - Length: 3 - ReplacementText: 'auto ' + DiagnosticMessage: + Message: Fix + FilePath: $(path)/no.cpp + FileOffset: 94 + Replacements: + - FilePath: $(path)/no.cpp + Offset: 94 + Length: 3 + ReplacementText: 'auto ' ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml index 6ded4db364a..337004d7e57 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/format/yes.yaml @@ -4,24 +4,25 @@ MainSourceFile: yes.cpp Diagnostics: - DiagnosticName: test-yes - Message: Fix - FilePath: $(path)/yes.cpp - FileOffset: 494 - Replacements: - - FilePath: $(path)/yes.cpp - Offset: 494 - Length: 1 - ReplacementText: nullptr - - FilePath: $(path)/yes.cpp - Offset: 410 - Length: 1 - ReplacementText: nullptr - - FilePath: $(path)/yes.cpp - Offset: 454 - Length: 1 - ReplacementText: nullptr - - FilePath: $(path)/yes.cpp - Offset: 108 - Length: 38 - ReplacementText: 'auto ' + DiagnosticMessage: + Message: Fix + FilePath: $(path)/yes.cpp + FileOffset: 494 + Replacements: + - FilePath: $(path)/yes.cpp + Offset: 494 + Length: 1 + ReplacementText: nullptr + - FilePath: $(path)/yes.cpp + Offset: 410 + Length: 1 + ReplacementText: nullptr + - FilePath: $(path)/yes.cpp + Offset: 454 + Length: 1 + ReplacementText: nullptr + - FilePath: $(path)/yes.cpp + Offset: 108 + Length: 38 + ReplacementText: 'auto ' ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml index cf273c4820d..9e3de7b4d51 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file1.yaml @@ -2,13 +2,14 @@ MainSourceFile: identical.cpp Diagnostics: - DiagnosticName: test-identical-insertion - Message: Fix - FilePath: $(path)/identical.cpp - FileOffset: 12 - Replacements: - - FilePath: $(path)/identical.cpp - Offset: 12 - Length: 0 - ReplacementText: '0' + DiagnosticMessage: + Message: Fix + FilePath: $(path)/identical.cpp + FileOffset: 12 + Replacements: + - FilePath: $(path)/identical.cpp + Offset: 12 + Length: 0 + ReplacementText: '0' ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml index cf273c4820d..9e3de7b4d51 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/identical/file2.yaml @@ -2,13 +2,14 @@ MainSourceFile: identical.cpp Diagnostics: - DiagnosticName: test-identical-insertion - Message: Fix - FilePath: $(path)/identical.cpp - FileOffset: 12 - Replacements: - - FilePath: $(path)/identical.cpp - Offset: 12 - Length: 0 - ReplacementText: '0' + DiagnosticMessage: + Message: Fix + FilePath: $(path)/identical.cpp + FileOffset: 12 + Replacements: + - FilePath: $(path)/identical.cpp + Offset: 12 + Length: 0 + ReplacementText: '0' ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml index a8234388d7d..939839d97dc 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file1.yaml @@ -2,12 +2,13 @@ MainSourceFile: order-dependent.cpp Diagnostics: - DiagnosticName: test-order-dependent-insertion - Message: Fix - FilePath: $(path)/order-dependent.cpp - FileOffset: 12 - Replacements: - - FilePath: $(path)/order-dependent.cpp - Offset: 12 - Length: 0 - ReplacementText: '0' + DiagnosticMessage: + Message: Fix + FilePath: $(path)/order-dependent.cpp + FileOffset: 12 + Replacements: + - FilePath: $(path)/order-dependent.cpp + Offset: 12 + Length: 0 + ReplacementText: '0' ... diff --git a/clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml b/clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml index 634d3ca6d43..1219c3b0b27 100644 --- a/clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml +++ b/clang-tools-extra/test/clang-apply-replacements/Inputs/order-dependent/file2.yaml @@ -2,12 +2,13 @@ MainSourceFile: order-dependent.cpp Diagnostics: - DiagnosticName: test-order-dependent-insertion - Message: Fix - FilePath: $(path)/order-dependent.cpp - FileOffset: 12 - Replacements: - - FilePath: $(path)/order-dependent.cpp - Offset: 12 - Length: 0 - ReplacementText: '1' + DiagnosticMessage: + Message: Fix + FilePath: $(path)/order-dependent.cpp + FileOffset: 12 + Replacements: + - FilePath: $(path)/order-dependent.cpp + Offset: 12 + Length: 0 + ReplacementText: '1' ... diff --git a/clang-tools-extra/test/clang-tidy/export-diagnostics.cpp b/clang-tools-extra/test/clang-tidy/export-diagnostics.cpp index 5eda2042ad2..14a6a40dca3 100644 --- a/clang-tools-extra/test/clang-tidy/export-diagnostics.cpp +++ b/clang-tools-extra/test/clang-tidy/export-diagnostics.cpp @@ -13,16 +13,19 @@ X(f) // CHECK-YAML-NEXT: MainSourceFile: '{{.*}}-input.cpp' // CHECK-YAML-NEXT: Diagnostics: // CHECK-YAML-NEXT: - DiagnosticName: clang-diagnostic-missing-prototypes -// CHECK-YAML-NEXT: Message: 'no previous prototype for function ''ff''' -// CHECK-YAML-NEXT: FileOffset: 30 -// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp' +// CHECK-YAML-NEXT: DiagnosticMessage: +// CHECK-YAML-NEXT: Message: 'no previous prototype for function +// ''ff''' +// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp' +// CHECK-YAML-NEXT: FileOffset: 30 +// CHECK-YAML-NEXT: Replacements: [] // CHECK-YAML-NEXT: Notes: // CHECK-YAML-NEXT: - Message: 'expanded from macro ''X''' // CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp' // CHECK-YAML-NEXT: FileOffset: 18 +// CHECK-YAML-NEXT: Replacements: [] // CHECK-YAML-NEXT: - Message: expanded from here // CHECK-YAML-NEXT: FilePath: '' // CHECK-YAML-NEXT: FileOffset: 0 -// CHECK-YAML-NEXT: Replacements: [] +// CHECK-YAML-NEXT: Replacements: [] // CHECK-YAML-NEXT: ... - diff --git a/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp b/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp index c532a37a541..0a2a6779c34 100644 --- a/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp +++ b/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp @@ -26,7 +26,6 @@ makeTUDiagnostics(const std::string &MainSourceFile, StringRef DiagnosticName, TUs.push_back({MainSourceFile, {{DiagnosticName, Message, - Replacements, {}, Diagnostic::Warning, BuildDirectory}}}); diff --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h b/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h index 33139d72c29..560b0ff893c 100644 --- a/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h +++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h @@ -14,6 +14,8 @@ #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Frontend/CompilerInstance.h" #include "clang/Frontend/FrontendActions.h" +#include "clang/Tooling/Core/Diagnostic.h" +#include "clang/Tooling/Core/Replacement.h" #include "clang/Tooling/Refactoring.h" #include "clang/Tooling/Tooling.h" #include "llvm/ADT/Optional.h" @@ -130,16 +132,17 @@ runCheckOnCode(StringRef Code, std::vector<ClangTidyError> *Errors = nullptr, tooling::Replacements Fixes; std::vector<ClangTidyError> Diags = DiagConsumer.take(); for (const ClangTidyError &Error : Diags) { - for (const auto &FileAndFixes : Error.Fix) { - for (const auto &Fix : FileAndFixes.second) { - auto Err = Fixes.add(Fix); - // FIXME: better error handling. Keep the behavior for now. - if (Err) { - llvm::errs() << llvm::toString(std::move(Err)) << "\n"; - return ""; + if (const auto *ChosenFix = tooling::selectFirstFix(Error)) + for (const auto &FileAndFixes : *ChosenFix) { + for (const auto &Fix : FileAndFixes.second) { + auto Err = Fixes.add(Fix); + // FIXME: better error handling. Keep the behavior for now. + if (Err) { + llvm::errs() << llvm::toString(std::move(Err)) << "\n"; + return ""; + } } } - } } if (Errors) *Errors = std::move(Diags); diff --git a/clang/include/clang/Tooling/Core/Diagnostic.h b/clang/include/clang/Tooling/Core/Diagnostic.h index 646bb56d648..0491d1355d0 100644 --- a/clang/include/clang/Tooling/Core/Diagnostic.h +++ b/clang/include/clang/Tooling/Core/Diagnostic.h @@ -42,6 +42,9 @@ struct DiagnosticMessage { std::string Message; std::string FilePath; unsigned FileOffset; + + /// Fixes for this diagnostic, grouped by file path. + llvm::StringMap<Replacements> Fix; }; /// Represents the diagnostic with the level of severity and possible @@ -58,7 +61,6 @@ struct Diagnostic { StringRef BuildDirectory); Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message, - const llvm::StringMap<Replacements> &Fix, const SmallVector<DiagnosticMessage, 1> &Notes, Level DiagLevel, llvm::StringRef BuildDirectory); @@ -68,9 +70,6 @@ struct Diagnostic { /// Message associated to the diagnostic. DiagnosticMessage Message; - /// Fixes to apply, grouped by file path. - llvm::StringMap<Replacements> Fix; - /// Potential notes about the diagnostic. SmallVector<DiagnosticMessage, 1> Notes; @@ -94,6 +93,10 @@ struct TranslationUnitDiagnostics { std::vector<Diagnostic> Diagnostics; }; +// Get the first fix to apply for this diagnostic. +// Return nullptr if no fixes attached to the diagnostic. +const llvm::StringMap<Replacements> *selectFirstFix(const Diagnostic& D); + } // end namespace tooling } // end namespace clang #endif // LLVM_CLANG_TOOLING_CORE_DIAGNOSTIC_H diff --git a/clang/include/clang/Tooling/DiagnosticsYaml.h b/clang/include/clang/Tooling/DiagnosticsYaml.h index 1cd2a4e9a88..366ee6f6703 100644 --- a/clang/include/clang/Tooling/DiagnosticsYaml.h +++ b/clang/include/clang/Tooling/DiagnosticsYaml.h @@ -31,6 +31,20 @@ template <> struct MappingTraits<clang::tooling::DiagnosticMessage> { Io.mapRequired("Message", M.Message); Io.mapOptional("FilePath", M.FilePath); Io.mapOptional("FileOffset", M.FileOffset); + std::vector<clang::tooling::Replacement> Fixes; + for (auto &Replacements : M.Fix) { + for (auto &Replacement : Replacements.second) + Fixes.push_back(Replacement); + } + Io.mapRequired("Replacements", Fixes); + for (auto &Fix : Fixes) { + llvm::Error Err = M.Fix[Fix.getFilePath()].add(Fix); + if (Err) { + // FIXME: Implement better conflict handling. + llvm::errs() << "Fix conflicts with existing fix: " + << llvm::toString(std::move(Err)) << "\n"; + } + } } }; @@ -43,12 +57,11 @@ template <> struct MappingTraits<clang::tooling::Diagnostic> { : DiagLevel(clang::tooling::Diagnostic::Level::Warning) {} NormalizedDiagnostic(const IO &, const clang::tooling::Diagnostic &D) - : DiagnosticName(D.DiagnosticName), Message(D.Message), Fix(D.Fix), - Notes(D.Notes), DiagLevel(D.DiagLevel), - BuildDirectory(D.BuildDirectory) {} + : DiagnosticName(D.DiagnosticName), Message(D.Message), Notes(D.Notes), + DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory) {} clang::tooling::Diagnostic denormalize(const IO &) { - return clang::tooling::Diagnostic(DiagnosticName, Message, Fix, Notes, + return clang::tooling::Diagnostic(DiagnosticName, Message, Notes, DiagLevel, BuildDirectory); } @@ -64,28 +77,10 @@ template <> struct MappingTraits<clang::tooling::Diagnostic> { MappingNormalization<NormalizedDiagnostic, clang::tooling::Diagnostic> Keys( Io, D); Io.mapRequired("DiagnosticName", Keys->DiagnosticName); - Io.mapRequired("Message", Keys->Message.Message); - Io.mapRequired("FileOffset", Keys->Message.FileOffset); - Io.mapRequired("FilePath", Keys->Message.FilePath); + Io.mapRequired("DiagnosticMessage", Keys->Message); Io.mapOptional("Notes", Keys->Notes); // FIXME: Export properly all the different fields. - - std::vector<clang::tooling::Replacement> Fixes; - for (auto &Replacements : Keys->Fix) { - for (auto &Replacement : Replacements.second) { - Fixes.push_back(Replacement); - } - } - Io.mapRequired("Replacements", Fixes); - for (auto &Fix : Fixes) { - llvm::Error Err = Keys->Fix[Fix.getFilePath()].add(Fix); - if (Err) { - // FIXME: Implement better conflict handling. - llvm::errs() << "Fix conflicts with existing fix: " - << llvm::toString(std::move(Err)) << "\n"; - } - } } }; diff --git a/clang/lib/Tooling/Core/Diagnostic.cpp b/clang/lib/Tooling/Core/Diagnostic.cpp index 8351f3e000c..235bd7fc143 100644 --- a/clang/lib/Tooling/Core/Diagnostic.cpp +++ b/clang/lib/Tooling/Core/Diagnostic.cpp @@ -12,6 +12,7 @@ #include "clang/Tooling/Core/Diagnostic.h" #include "clang/Basic/SourceManager.h" +#include "llvm/ADT/STLExtras.h" namespace clang { namespace tooling { @@ -40,11 +41,21 @@ Diagnostic::Diagnostic(llvm::StringRef DiagnosticName, Diagnostic::Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message, - const llvm::StringMap<Replacements> &Fix, const SmallVector<DiagnosticMessage, 1> &Notes, Level DiagLevel, llvm::StringRef BuildDirectory) - : DiagnosticName(DiagnosticName), Message(Message), Fix(Fix), Notes(Notes), + : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes), DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {} +const llvm::StringMap<Replacements> *selectFirstFix(const Diagnostic& D) { + if (!D.Message.Fix.empty()) + return &D.Message.Fix; + auto Iter = llvm::find_if(D.Notes, [](const tooling::DiagnosticMessage &D) { + return !D.Fix.empty(); + }); + if (Iter != D.Notes.end()) + return &Iter->Fix; + return nullptr; +} + } // end namespace tooling } // end namespace clang diff --git a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp index 32a9c1e42a5..aaba2589111 100644 --- a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp +++ b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp @@ -20,11 +20,13 @@ using namespace clang::tooling; using clang::tooling::Diagnostic; static DiagnosticMessage makeMessage(const std::string &Message, int FileOffset, - const std::string &FilePath) { + const std::string &FilePath, + const StringMap<Replacements> &Fix) { DiagnosticMessage DiagMessage; DiagMessage.Message = Message; DiagMessage.FileOffset = FileOffset; DiagMessage.FilePath = FilePath; + DiagMessage.Fix = Fix; return DiagMessage; } @@ -32,10 +34,52 @@ static Diagnostic makeDiagnostic(StringRef DiagnosticName, const std::string &Message, int FileOffset, const std::string &FilePath, const StringMap<Replacements> &Fix) { - return Diagnostic(DiagnosticName, makeMessage(Message, FileOffset, FilePath), - Fix, {}, Diagnostic::Warning, "path/to/build/directory"); + return Diagnostic(DiagnosticName, + makeMessage(Message, FileOffset, FilePath, Fix), {}, + Diagnostic::Warning, "path/to/build/directory"); } +static const char *YAMLContent = + "---\n" + "MainSourceFile: 'path/to/source.cpp'\n" + "Diagnostics: \n" + " - DiagnosticName: 'diagnostic#1\'\n" + " DiagnosticMessage: \n" + " Message: 'message #1'\n" + " FilePath: 'path/to/source.cpp'\n" + " FileOffset: 55\n" + " Replacements: \n" + " - FilePath: 'path/to/source.cpp'\n" + " Offset: 100\n" + " Length: 12\n" + " ReplacementText: 'replacement #1'\n" + " - DiagnosticName: 'diagnostic#2'\n" + " DiagnosticMessage: \n" + " Message: 'message #2'\n" + " FilePath: 'path/to/header.h'\n" + " FileOffset: 60\n" + " Replacements: \n" + " - FilePath: 'path/to/header.h'\n" + " Offset: 62\n" + " Length: 2\n" + " ReplacementText: 'replacement #2'\n" + " - DiagnosticName: 'diagnostic#3'\n" + " DiagnosticMessage: \n" + " Message: 'message #3'\n" + " FilePath: 'path/to/source2.cpp'\n" + " FileOffset: 72\n" + " Replacements: []\n" + " Notes: \n" + " - Message: Note1\n" + " FilePath: 'path/to/note1.cpp'\n" + " FileOffset: 88\n" + " Replacements: []\n" + " - Message: Note2\n" + " FilePath: 'path/to/note2.cpp'\n" + " FileOffset: 99\n" + " Replacements: []\n" + "...\n"; + TEST(DiagnosticsYamlTest, serializesDiagnostics) { TranslationUnitDiagnostics TUD; TUD.MainSourceFile = "path/to/source.cpp"; @@ -55,9 +99,9 @@ TEST(DiagnosticsYamlTest, serializesDiagnostics) { TUD.Diagnostics.push_back(makeDiagnostic("diagnostic#3", "message #3", 72, "path/to/source2.cpp", {})); TUD.Diagnostics.back().Notes.push_back( - makeMessage("Note1", 88, "path/to/note1.cpp")); + makeMessage("Note1", 88, "path/to/note1.cpp", {})); TUD.Diagnostics.back().Notes.push_back( - makeMessage("Note2", 99, "path/to/note2.cpp")); + makeMessage("Note2", 99, "path/to/note2.cpp", {})); std::string YamlContent; raw_string_ostream YamlContentStream(YamlContent); @@ -65,80 +109,12 @@ TEST(DiagnosticsYamlTest, serializesDiagnostics) { yaml::Output YAML(YamlContentStream); YAML << TUD; - EXPECT_EQ("---\n" - "MainSourceFile: 'path/to/source.cpp'\n" - "Diagnostics: \n" - " - DiagnosticName: 'diagnostic#1\'\n" - " Message: 'message #1'\n" - " FileOffset: 55\n" - " FilePath: 'path/to/source.cpp'\n" - " Replacements: \n" - " - FilePath: 'path/to/source.cpp'\n" - " Offset: 100\n" - " Length: 12\n" - " ReplacementText: 'replacement #1'\n" - " - DiagnosticName: 'diagnostic#2'\n" - " Message: 'message #2'\n" - " FileOffset: 60\n" - " FilePath: 'path/to/header.h'\n" - " Replacements: \n" - " - FilePath: 'path/to/header.h'\n" - " Offset: 62\n" - " Length: 2\n" - " ReplacementText: 'replacement #2'\n" - " - DiagnosticName: 'diagnostic#3'\n" - " Message: 'message #3'\n" - " FileOffset: 72\n" - " FilePath: 'path/to/source2.cpp'\n" - " Notes: \n" - " - Message: Note1\n" - " FilePath: 'path/to/note1.cpp'\n" - " FileOffset: 88\n" - " - Message: Note2\n" - " FilePath: 'path/to/note2.cpp'\n" - " FileOffset: 99\n" - " Replacements: []\n" - "...\n", - YamlContentStream.str()); + EXPECT_EQ(YAMLContent, YamlContentStream.str()); } TEST(DiagnosticsYamlTest, deserializesDiagnostics) { - std::string YamlContent = "---\n" - "MainSourceFile: path/to/source.cpp\n" - "Diagnostics: \n" - " - DiagnosticName: 'diagnostic#1'\n" - " Message: 'message #1'\n" - " FileOffset: 55\n" - " FilePath: path/to/source.cpp\n" - " Replacements: \n" - " - FilePath: path/to/source.cpp\n" - " Offset: 100\n" - " Length: 12\n" - " ReplacementText: 'replacement #1'\n" - " - DiagnosticName: 'diagnostic#2'\n" - " Message: 'message #2'\n" - " FileOffset: 60\n" - " FilePath: path/to/header.h\n" - " Replacements: \n" - " - FilePath: path/to/header.h\n" - " Offset: 62\n" - " Length: 2\n" - " ReplacementText: 'replacement #2'\n" - " - DiagnosticName: 'diagnostic#3'\n" - " Message: 'message #3'\n" - " FileOffset: 98\n" - " FilePath: path/to/source.cpp\n" - " Notes:\n" - " - Message: Note1\n" - " FilePath: 'path/to/note1.cpp'\n" - " FileOffset: 66\n" - " - Message: Note2\n" - " FilePath: 'path/to/note2.cpp'\n" - " FileOffset: 77\n" - " Replacements: []\n" - "...\n"; TranslationUnitDiagnostics TUDActual; - yaml::Input YAML(YamlContent); + yaml::Input YAML(YAMLContent); YAML >> TUDActual; ASSERT_FALSE(YAML.error()); @@ -160,7 +136,7 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) { EXPECT_EQ("message #1", D1.Message.Message); EXPECT_EQ(55u, D1.Message.FileOffset); EXPECT_EQ("path/to/source.cpp", D1.Message.FilePath); - std::vector<Replacement> Fixes1 = getFixes(D1.Fix); + std::vector<Replacement> Fixes1 = getFixes(D1.Message.Fix); ASSERT_EQ(1u, Fixes1.size()); EXPECT_EQ("path/to/source.cpp", Fixes1[0].getFilePath()); EXPECT_EQ(100u, Fixes1[0].getOffset()); @@ -172,7 +148,7 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) { EXPECT_EQ("message #2", D2.Message.Message); EXPECT_EQ(60u, D2.Message.FileOffset); EXPECT_EQ("path/to/header.h", D2.Message.FilePath); - std::vector<Replacement> Fixes2 = getFixes(D2.Fix); + std::vector<Replacement> Fixes2 = getFixes(D2.Message.Fix); ASSERT_EQ(1u, Fixes2.size()); EXPECT_EQ("path/to/header.h", Fixes2[0].getFilePath()); EXPECT_EQ(62u, Fixes2[0].getOffset()); @@ -182,15 +158,15 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) { Diagnostic D3 = TUDActual.Diagnostics[2]; EXPECT_EQ("diagnostic#3", D3.DiagnosticName); EXPECT_EQ("message #3", D3.Message.Message); - EXPECT_EQ(98u, D3.Message.FileOffset); - EXPECT_EQ("path/to/source.cpp", D3.Message.FilePath); + EXPECT_EQ(72u, D3.Message.FileOffset); + EXPECT_EQ("path/to/source2.cpp", D3.Message.FilePath); EXPECT_EQ(2u, D3.Notes.size()); EXPECT_EQ("Note1", D3.Notes[0].Message); - EXPECT_EQ(66u, D3.Notes[0].FileOffset); + EXPECT_EQ(88u, D3.Notes[0].FileOffset); EXPECT_EQ("path/to/note1.cpp", D3.Notes[0].FilePath); EXPECT_EQ("Note2", D3.Notes[1].Message); - EXPECT_EQ(77u, D3.Notes[1].FileOffset); + EXPECT_EQ(99u, D3.Notes[1].FileOffset); EXPECT_EQ("path/to/note2.cpp", D3.Notes[1].FilePath); - std::vector<Replacement> Fixes3 = getFixes(D3.Fix); + std::vector<Replacement> Fixes3 = getFixes(D3.Message.Fix); EXPECT_TRUE(Fixes3.empty()); } |
