diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -132,6 +132,8 @@ } auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]")) << Message.Message << Name; + for (const FileByteRange &FBR : Error.Message.Ranges) + Diag << getRange(FBR); // FIXME: explore options to support interactive fix selection. const llvm::StringMap *ChosenFix; if (ApplyFixes != FB_NoFix && @@ -257,17 +259,13 @@ 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()); + FileByteRange FBR; + FBR.FilePath = Repl.getFilePath().str(); + FBR.FileOffset = Repl.getOffset(); + FBR.Length = Repl.getLength(); + + Diag << FixItHint::CreateReplacement(getRange(FBR), + Repl.getReplacementText()); } } } @@ -277,9 +275,22 @@ auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0")) << Message.Message; + for (const FileByteRange &FBR : Message.Ranges) + Diag << getRange(FBR); reportFix(Diag, Message.Fix); } + CharSourceRange getRange(const FileByteRange &Range) { + SmallString<128> AbsoluteFilePath{Range.FilePath}; + Files.makeAbsolutePath(AbsoluteFilePath); + SourceLocation BeginLoc = getLocation(AbsoluteFilePath, Range.FileOffset); + SourceLocation EndLoc = BeginLoc.getLocWithOffset(Range.Length); + // Retrieve the source range for applicable highlights and fixes. Macro + // definition on the command line have locations in a virtual buffer and + // don't have valid file paths and are therefore not applicable. + return CharSourceRange::getCharRange(BeginLoc, EndLoc); + } + FileManager Files; LangOptions LangOpts; // FIXME: use langopts from each original file IntrusiveRefCntPtr DiagOpts; diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -25,6 +25,7 @@ #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/SourceManager.h" #include "clang/Frontend/DiagnosticRenderer.h" +#include "clang/Lex/Lexer.h" #include "clang/Tooling/Core/Diagnostic.h" #include "clang/Tooling/Core/Replacement.h" #include "llvm/ADT/STLExtras.h" @@ -62,15 +63,31 @@ Loc.isValid() ? tooling::DiagnosticMessage(Message, Loc.getManager(), Loc) : tooling::DiagnosticMessage(Message); + + // Make sure that if a TokenRange is receieved from the check it is unfurled + // into a real CharRange for the diagnostic printer later. + // Whatever we store here gets decoupled from the current SourceManager, so + // we **have to** know the exact position and length of the highlight. + const auto &ToCharRange = [this, &Loc](const CharSourceRange &SourceRange) { + if (SourceRange.isCharRange()) + return SourceRange; + SourceLocation End = Lexer::getLocForEndOfToken( + SourceRange.getEnd(), 1, Loc.getManager(), LangOpts); + return CharSourceRange::getCharRange(SourceRange.getBegin(), End); + }; + if (Level == DiagnosticsEngine::Note) { Error.Notes.push_back(TidyMessage); + for (const CharSourceRange &SourceRange : Ranges) + Error.Notes.back().Ranges.emplace_back(Loc.getManager(), + ToCharRange(SourceRange)); return; } assert(Error.Message.Message.empty() && "Overwriting a diagnostic message"); Error.Message = TidyMessage; - for (const CharSourceRange &SourceRange : Ranges) { - Error.Ranges.emplace_back(Loc.getManager(), SourceRange); - } + for (const CharSourceRange &SourceRange : Ranges) + Error.Message.Ranges.emplace_back(Loc.getManager(), + ToCharRange(SourceRange)); } void emitDiagnosticLoc(FullSourceLoc Loc, PresumedLoc PLoc, diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -74,6 +74,13 @@ attached to warnings. These are typically cases where we are less confident the fix will have the desired effect. +- libToolingCore and Clang-Tidy was refactored and now checks can produce + highlights (`^~~~~` under fragments of the source code) in diagnostics. + Existing and new checks in the future can be expected to start implementing + this functionality. + This change only affects the visual rendering of diagnostics, and does not + alter the behavior of generated fixes. + New checks ^^^^^^^^^^ diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp --- a/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp @@ -22,7 +22,7 @@ // ''ff''' // CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp' // CHECK-YAML-NEXT: FileOffset: 30 -// CHECK-YAML-NEXT: Replacements: [] +// CHECK-YAML-NEXT: Replacements: [] // CHECK-YAML-NEXT: Notes: // CHECK-YAML-NEXT: - Message: 'expanded from macro ''X''' // CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp' @@ -52,10 +52,10 @@ // CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp' // CHECK-YAML-NEXT: FileOffset: 41 // CHECK-YAML-NEXT: Replacements: [] +// CHECK-YAML-NEXT: Ranges: +// CHECK-YAML-NEXT: - FilePath: '{{.*}}-input.cpp' +// CHECK-YAML-NEXT: FileOffset: 41 +// CHECK-YAML-NEXT: Length: 1 // CHECK-YAML-NEXT: Level: Error // CHECK-YAML-NEXT: BuildDirectory: '{{.*}}' -// CHECK-YAML-NEXT: Ranges: -// CHECK-YAML-NEXT: - FilePath: '{{.*}}-input.cpp' -// CHECK-YAML-NEXT: FileOffset: 41 -// CHECK-YAML-NEXT: Length: 1 // 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 --- a/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp +++ b/clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp @@ -23,13 +23,9 @@ const StringMap &Replacements, StringRef BuildDirectory) { TUDiagnostics TUs; - TUs.push_back({MainSourceFile, - {{DiagnosticName, - Message, - {}, - Diagnostic::Warning, - BuildDirectory, - {}}}}); + TUs.push_back( + {MainSourceFile, + {{DiagnosticName, Message, {}, Diagnostic::Warning, BuildDirectory}}}); return TUs; } diff --git a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp --- a/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp @@ -84,7 +84,7 @@ EXPECT_EQ(Input, test::runCheckOnCode(Input, &Errors)); EXPECT_EQ(Errors.size(), 1U); EXPECT_EQ(Errors[0].Message.Message, "message"); - EXPECT_THAT(Errors[0].Ranges, testing::IsEmpty()); + EXPECT_THAT(Errors[0].Message.Ranges, testing::IsEmpty()); // The diagnostic is anchored to the match, "return 5". EXPECT_EQ(Errors[0].Message.FileOffset, 10U); diff --git a/clang/include/clang/Tooling/Core/Diagnostic.h b/clang/include/clang/Tooling/Core/Diagnostic.h --- a/clang/include/clang/Tooling/Core/Diagnostic.h +++ b/clang/include/clang/Tooling/Core/Diagnostic.h @@ -26,6 +26,17 @@ namespace clang { namespace tooling { +/// Represents a range within a specific source file. +struct FileByteRange { + FileByteRange() = default; + + FileByteRange(const SourceManager &Sources, CharSourceRange Range); + + std::string FilePath; + unsigned FileOffset; + unsigned Length; +}; + /// Represents the diagnostic message with the error message associated /// and the information on the location of the problem. struct DiagnosticMessage { @@ -39,23 +50,17 @@ /// DiagnosticMessage(llvm::StringRef Message, const SourceManager &Sources, SourceLocation Loc); + std::string Message; std::string FilePath; unsigned FileOffset; /// Fixes for this diagnostic, grouped by file path. llvm::StringMap Fix; -}; - -/// Represents a range within a specific source file. -struct FileByteRange { - FileByteRange() = default; - FileByteRange(const SourceManager &Sources, CharSourceRange Range); - - std::string FilePath; - unsigned FileOffset; - unsigned Length; + /// Extra source ranges associated with the note, in addition to the location + /// of the Message itself. + llvm::SmallVector Ranges; }; /// Represents the diagnostic with the level of severity and possible @@ -73,8 +78,7 @@ Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message, const SmallVector &Notes, Level DiagLevel, - llvm::StringRef BuildDirectory, - const SmallVector &Ranges); + llvm::StringRef BuildDirectory); /// Name identifying the Diagnostic. std::string DiagnosticName; @@ -96,10 +100,6 @@ /// /// Note: it is empty in unittest. std::string BuildDirectory; - - /// Extra source ranges associated with the diagnostic (in addition to the - /// location of the Message above). - SmallVector Ranges; }; /// Collection of Diagnostics generated from a single translation unit. diff --git a/clang/include/clang/Tooling/DiagnosticsYaml.h b/clang/include/clang/Tooling/DiagnosticsYaml.h --- a/clang/include/clang/Tooling/DiagnosticsYaml.h +++ b/clang/include/clang/Tooling/DiagnosticsYaml.h @@ -54,6 +54,7 @@ << llvm::toString(std::move(Err)) << "\n"; } } + Io.mapOptional("Ranges", M.Ranges); } }; @@ -67,12 +68,11 @@ NormalizedDiagnostic(const IO &, const clang::tooling::Diagnostic &D) : DiagnosticName(D.DiagnosticName), Message(D.Message), Notes(D.Notes), - DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory), - Ranges(D.Ranges) {} + DiagLevel(D.DiagLevel), BuildDirectory(D.BuildDirectory) {} clang::tooling::Diagnostic denormalize(const IO &) { return clang::tooling::Diagnostic(DiagnosticName, Message, Notes, - DiagLevel, BuildDirectory, Ranges); + DiagLevel, BuildDirectory); } std::string DiagnosticName; @@ -80,7 +80,6 @@ SmallVector Notes; clang::tooling::Diagnostic::Level DiagLevel; std::string BuildDirectory; - SmallVector Ranges; }; static void mapping(IO &Io, clang::tooling::Diagnostic &D) { @@ -91,7 +90,6 @@ Io.mapOptional("Notes", Keys->Notes); Io.mapOptional("Level", Keys->DiagLevel); Io.mapOptional("BuildDirectory", Keys->BuildDirectory); - Io.mapOptional("Ranges", Keys->Ranges); } }; diff --git a/clang/lib/Tooling/Core/Diagnostic.cpp b/clang/lib/Tooling/Core/Diagnostic.cpp --- a/clang/lib/Tooling/Core/Diagnostic.cpp +++ b/clang/lib/Tooling/Core/Diagnostic.cpp @@ -53,10 +53,9 @@ Diagnostic::Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message, const SmallVector &Notes, - Level DiagLevel, llvm::StringRef BuildDirectory, - const SmallVector &Ranges) + Level DiagLevel, llvm::StringRef BuildDirectory) : DiagnosticName(DiagnosticName), Message(Message), Notes(Notes), - DiagLevel(DiagLevel), BuildDirectory(BuildDirectory), Ranges(Ranges) {} + DiagLevel(DiagLevel), BuildDirectory(BuildDirectory) {} const llvm::StringMap *selectFirstFix(const Diagnostic& D) { if (!D.Message.Fix.empty()) diff --git a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp --- a/clang/unittests/Tooling/DiagnosticsYamlTest.cpp +++ b/clang/unittests/Tooling/DiagnosticsYamlTest.cpp @@ -20,14 +20,16 @@ using namespace clang::tooling; using clang::tooling::Diagnostic; -static DiagnosticMessage makeMessage(const std::string &Message, int FileOffset, - const std::string &FilePath, - const StringMap &Fix) { +static DiagnosticMessage +makeMessage(const std::string &Message, int FileOffset, + const std::string &FilePath, const StringMap &Fix, + const SmallVector &Ranges) { DiagnosticMessage DiagMessage; DiagMessage.Message = Message; DiagMessage.FileOffset = FileOffset; DiagMessage.FilePath = FilePath; DiagMessage.Fix = Fix; + DiagMessage.Ranges = Ranges; return DiagMessage; } @@ -47,8 +49,8 @@ const StringMap &Fix, const SmallVector &Ranges) { return Diagnostic(DiagnosticName, - makeMessage(Message, FileOffset, FilePath, Fix), {}, - Diagnostic::Warning, "path/to/build/directory", Ranges); + makeMessage(Message, FileOffset, FilePath, Fix, Ranges), {}, + Diagnostic::Warning, "path/to/build/directory"); } static const char *YAMLContent = @@ -77,12 +79,12 @@ " Offset: 62\n" " Length: 2\n" " ReplacementText: 'replacement #2'\n" + " Ranges:\n" + " - FilePath: 'path/to/source.cpp'\n" + " FileOffset: 10\n" + " Length: 10\n" " Level: Warning\n" " BuildDirectory: 'path/to/build/directory'\n" - " Ranges:\n" - " - FilePath: 'path/to/source.cpp'\n" - " FileOffset: 10\n" - " Length: 10\n" " - DiagnosticName: 'diagnostic#3'\n" " DiagnosticMessage:\n" " Message: 'message #3'\n" @@ -123,9 +125,9 @@ 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); @@ -166,7 +168,7 @@ EXPECT_EQ(100u, Fixes1[0].getOffset()); EXPECT_EQ(12u, Fixes1[0].getLength()); EXPECT_EQ("replacement #1", Fixes1[0].getReplacementText()); - EXPECT_TRUE(D1.Ranges.empty()); + EXPECT_TRUE(D1.Message.Ranges.empty()); Diagnostic D2 = TUDActual.Diagnostics[1]; EXPECT_EQ("diagnostic#2", D2.DiagnosticName); @@ -179,10 +181,10 @@ EXPECT_EQ(62u, Fixes2[0].getOffset()); EXPECT_EQ(2u, Fixes2[0].getLength()); EXPECT_EQ("replacement #2", Fixes2[0].getReplacementText()); - EXPECT_EQ(1u, D2.Ranges.size()); - EXPECT_EQ("path/to/source.cpp", D2.Ranges[0].FilePath); - EXPECT_EQ(10u, D2.Ranges[0].FileOffset); - EXPECT_EQ(10u, D2.Ranges[0].Length); + EXPECT_EQ(1u, D2.Message.Ranges.size()); + EXPECT_EQ("path/to/source.cpp", D2.Message.Ranges[0].FilePath); + EXPECT_EQ(10u, D2.Message.Ranges[0].FileOffset); + EXPECT_EQ(10u, D2.Message.Ranges[0].Length); Diagnostic D3 = TUDActual.Diagnostics[2]; EXPECT_EQ("diagnostic#3", D3.DiagnosticName); @@ -198,5 +200,5 @@ EXPECT_EQ("path/to/note2.cpp", D3.Notes[1].FilePath); std::vector Fixes3 = getFixes(D3.Message.Fix); EXPECT_TRUE(Fixes3.empty()); - EXPECT_TRUE(D3.Ranges.empty()); + EXPECT_TRUE(D3.Message.Ranges.empty()); }