replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[libtooling][clang-tidy] Fix diagnostics not highlighting fed SourceRanges 

Fixes bug http://bugs.llvm.org/show_bug.cgi?id=49000.

This patch allows Clang-Tidy checks to do

    diag(X->getLocation(), "text") << Y->getSourceRange();

and get the highlight of `Y` as expected:

    warning: text [blah-blah]
        xxx(something)
        ^   ~~~~~~~~~

Reviewed-By: aaron.ballman, njames93

Differential Revision: http://reviews.llvm.org/D98635
This commit is contained in:
Whisperity 2021-03-15 17:06:03 +01:00
parent c329a47d9e
commit 3b677b81ce
10 changed files with 98 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
clang-tools-extra/clang-tidy/ClangTidy.cpp
View File

@ -132,6 +132,8 @@ public:
}
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<Replacements> *ChosenFix;
if (ApplyFixes != FB_NoFix &&
@ -257,17 +259,13 @@ private:
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 @@ private:
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<DiagnosticOptions> DiagOpts;

23
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
View File

@ -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 @@ protected:
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,

7
clang-tools-extra/docs/ReleaseNotes.rst
View File

@ -74,6 +74,13 @@ Improvements to clang-tidy
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
^^^^^^^^^^

4
clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
View File

@ -52,10 +52,10 @@ int a[-1];
// CHECK-YAML-NEXT: FilePath: '{{.*}}-input.cpp'
// CHECK-YAML-NEXT: FileOffset: 41
// CHECK-YAML-NEXT: Replacements: []
// 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: Level: Error
// CHECK-YAML-NEXT: BuildDirectory: '{{.*}}'
// CHECK-YAML-NEXT: ...

10
clang-tools-extra/unittests/clang-apply-replacements/ApplyReplacementsTest.cpp
View File

@ -23,13 +23,9 @@ makeTUDiagnostics(const std::string &MainSourceFile, StringRef DiagnosticName,
const StringMap<Replacements> &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;
}

2
clang-tools-extra/unittests/clang-tidy/TransformerClangTidyCheckTest.cpp
View File

@ -84,7 +84,7 @@ TEST(TransformerClangTidyCheckTest, DiagnosticsCorrectlyGenerated) {
EXPECT_EQ(Input, test::runCheckOnCode<DiagOnlyCheck>(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);

32
clang/include/clang/Tooling/Core/Diagnostic.h
View File

@ -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 @@ struct DiagnosticMessage {
///
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<Replacements> 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<FileByteRange, 1> Ranges;
};
/// Represents the diagnostic with the level of severity and possible
@ -73,8 +78,7 @@ struct Diagnostic {
Diagnostic(llvm::StringRef DiagnosticName, const DiagnosticMessage &Message,
const SmallVector<DiagnosticMessage, 1> &Notes, Level DiagLevel,
llvm::StringRef BuildDirectory,
const SmallVector<FileByteRange, 1> &Ranges);
llvm::StringRef BuildDirectory);
/// Name identifying the Diagnostic.
std::string DiagnosticName;
@ -96,10 +100,6 @@ struct Diagnostic {
///
/// 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<FileByteRange, 1> Ranges;
};
/// Collection of Diagnostics generated from a single translation unit.

8
clang/include/clang/Tooling/DiagnosticsYaml.h
View File

@ -54,6 +54,7 @@ template <> struct MappingTraits<clang::tooling::DiagnosticMessage> {
<< llvm::toString(std::move(Err)) << "\n";
}
}
Io.mapOptional("Ranges", M.Ranges);
}
};
@ -67,12 +68,11 @@ template <> struct MappingTraits<clang::tooling::Diagnostic> {
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 @@ template <> struct MappingTraits<clang::tooling::Diagnostic> {
SmallVector<clang::tooling::DiagnosticMessage, 1> Notes;
clang::tooling::Diagnostic::Level DiagLevel;
std::string BuildDirectory;
SmallVector<clang::tooling::FileByteRange, 1> Ranges;
};
static void mapping(IO &Io, clang::tooling::Diagnostic &D) {
@ -91,7 +90,6 @@ template <> struct MappingTraits<clang::tooling::Diagnostic> {
Io.mapOptional("Notes", Keys->Notes);
Io.mapOptional("Level", Keys->DiagLevel);
Io.mapOptional("BuildDirectory", Keys->BuildDirectory);
Io.mapOptional("Ranges", Keys->Ranges);
}
};

5
clang/lib/Tooling/Core/Diagnostic.cpp
View File

@ -53,10 +53,9 @@ Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
Diagnostic::Diagnostic(llvm::StringRef DiagnosticName,
const DiagnosticMessage &Message,
const SmallVector<DiagnosticMessage, 1> &Notes,
Level DiagLevel, llvm::StringRef BuildDirectory,
const SmallVector<FileByteRange, 1> &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<Replacements> *selectFirstFix(const Diagnostic& D) {
if (!D.Message.Fix.empty())

32
clang/unittests/Tooling/DiagnosticsYamlTest.cpp
View File

@ -20,14 +20,16 @@ using namespace llvm;
using namespace clang::tooling;
using clang::tooling::Diagnostic;
static DiagnosticMessage makeMessage(const std::string &Message, int FileOffset,
const std::string &FilePath,
const StringMap<Replacements> &Fix) {
static DiagnosticMessage
makeMessage(const std::string &Message, int FileOffset,
const std::string &FilePath, const StringMap<Replacements> &Fix,
const SmallVector<FileByteRange, 1> &Ranges) {
DiagnosticMessage DiagMessage;
DiagMessage.Message = Message;
DiagMessage.FileOffset = FileOffset;
DiagMessage.FilePath = FilePath;
DiagMessage.Fix = Fix;
DiagMessage.Ranges = Ranges;
return DiagMessage;
}
@ -47,8 +49,8 @@ static Diagnostic makeDiagnostic(StringRef DiagnosticName,
const StringMap<Replacements> &Fix,
const SmallVector<FileByteRange, 1> &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 @@ static const char *YAMLContent =
" Offset: 62\n"
" Length: 2\n"
" ReplacementText: 'replacement #2'\n"
" Level: Warning\n"
" BuildDirectory: 'path/to/build/directory'\n"
" Ranges:\n"
" - FilePath: 'path/to/source.cpp'\n"
" FileOffset: 10\n"
" Length: 10\n"
" Level: Warning\n"
" BuildDirectory: 'path/to/build/directory'\n"
" - DiagnosticName: 'diagnostic#3'\n"
" DiagnosticMessage:\n"
" Message: 'message #3'\n"
@ -123,9 +125,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);
@ -166,7 +168,7 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) {
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 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) {
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 @@ TEST(DiagnosticsYamlTest, deserializesDiagnostics) {
EXPECT_EQ("path/to/note2.cpp", D3.Notes[1].FilePath);
std::vector<Replacement> Fixes3 = getFixes(D3.Message.Fix);
EXPECT_TRUE(Fixes3.empty());
EXPECT_TRUE(D3.Ranges.empty());
EXPECT_TRUE(D3.Message.Ranges.empty());
}