[clangd] Retain main file fixes attached to diags from preamble

Clangd ignores fixits if the diagnsotics is outside the main file (e.g.
a note on a declaration from a header), but the fix might still be inside the
main file (e.g. change the function call).

This patch changes the logic to retain fixes that touch main file, if the
diagnostic owning them is still inside main file, even if they are attached to a
note outside the main file.

Differential Revision: https://reviews.llvm.org/D122315
This commit is contained in:
Kadir Cetinkaya 2022-03-23 15:22:47 +01:00
parent c6a9ce2b6b
commit 50f4f32b56
No known key found for this signature in database
GPG key ID: E39E36B8D2057ED6
2 changed files with 29 additions and 5 deletions

View file

@ -31,6 +31,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <cassert>
#include <cstddef>
#include <vector>
@ -463,7 +464,8 @@ void toLSPDiags(
// Main diagnostic should always refer to a range inside main file. If a
// diagnostic made it so for, it means either itself or one of its notes is
// inside main file.
// inside main file. It's also possible that there's a fix in the main file,
// but we preserve fixes iff primary diagnostic is in the main file.
if (D.InsideMainFile) {
Main.range = D.Range;
} else {
@ -701,13 +703,12 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
return;
}
bool InsideMainFile = isInsideMainFile(Info);
SourceManager &SM = Info.getSourceManager();
auto FillDiagBase = [&](DiagBase &D) {
fillNonLocationData(DiagLevel, Info, D);
D.InsideMainFile = InsideMainFile;
D.InsideMainFile = isInsideMainFile(Info);
D.Range = diagnosticRange(Info, *LangOpts);
D.File = std::string(SM.getFilename(Info.getLocation()));
D.AbsFile = getCanonicalPath(
@ -719,9 +720,9 @@ void StoreDiags::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
auto AddFix = [&](bool SyntheticMessage) -> bool {
assert(!Info.getFixItHints().empty() &&
"diagnostic does not have attached fix-its");
if (!InsideMainFile)
// No point in generating fixes, if the diagnostic is for a different file.
if (!LastDiag->InsideMainFile)
return false;
// Copy as we may modify the ranges.
auto FixIts = Info.getFixItHints().vec();
llvm::SmallVector<TextEdit, 1> Edits;

View file

@ -22,6 +22,7 @@
#include "support/Path.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticSema.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/ScopedPrinter.h"
#include "llvm/Support/TargetSelect.h"
#include "gmock/gmock.h"
@ -1680,6 +1681,28 @@ $fix[[ $diag[[#include "unused.h"]]
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
}
TEST(DiagnosticsTest, FixItFromHeader) {
llvm::StringLiteral Header(R"cpp(
void foo(int *);
void foo(int *, int);)cpp");
Annotations Source(R"cpp(
/*error-ok*/
void bar() {
int x;
$diag[[foo]]($fix[[]]x, 1);
})cpp");
TestTU TU;
TU.Code = Source.code().str();
TU.HeaderCode = Header.str();
EXPECT_THAT(
*TU.build().getDiagnostics(),
UnorderedElementsAre(AllOf(
Diag(Source.range("diag"), "no matching function for call to 'foo'"),
withFix(Fix(Source.range("fix"), "&",
"candidate function not viable: no known conversion from "
"'int' to 'int *' for 1st argument; take the address of "
"the argument with &")))));
}
} // namespace
} // namespace clangd
} // namespace clang