hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D149437
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
Index: clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
===================================================================
--- clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
+++ clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
@@ -152,11 +152,13 @@
# CHECK-NEXT: {
# CHECK-NEXT: "changeAnnotations": {
# CHECK-NEXT: "AddAllMissingIncludes0": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "description": "Line 2: {{.*}}",
+# CHECK-NEXT: "label": "Add {{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: },
# CHECK-NEXT: "AddAllMissingIncludes1": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "description": "Line 2: {{.*}}",
+# CHECK-NEXT: "label": "Add {{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -208,19 +210,21 @@
# CHECK-NEXT: {
# CHECK-NEXT: "changeAnnotations": {
# CHECK-NEXT: "AddAllMissingIncludes0": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "description": "Line 2: ' Foo* foo; Bar* bar;'",
+# CHECK-NEXT: "label": "Add {{.*}}bar.h{{.*}} for symbol 'Bar'",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: },
# CHECK-NEXT: "AddAllMissingIncludes1": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "description": "Line 2: ' Foo* foo; Bar* bar;'",
+# CHECK-NEXT: "label": "Add {{.*}}foo.h{{.*}} for symbol 'Foo'",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: },
# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "label": "Remove {{.*}}all1.h{{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: },
# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "label": "Remove {{.*}}all2.h{{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -337,11 +341,11 @@
# CHECK-NEXT: {
# CHECK-NEXT: "changeAnnotations": {
# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "label": "Remove {{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: },
# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "label": "Remove {{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -393,19 +397,21 @@
# CHECK-NEXT: {
# CHECK-NEXT: "changeAnnotations": {
# CHECK-NEXT: "AddAllMissingIncludes0": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "description": "Line 2: {{.*}}",
+# CHECK-NEXT: "label": "Add {{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: },
# CHECK-NEXT: "AddAllMissingIncludes1": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "description": "Line 2: {{.*}}",
+# CHECK-NEXT: "label": "Add {{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: },
# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "label": "Remove {{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: },
# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
-# CHECK-NEXT: "label": "",
+# CHECK-NEXT: "label": "Remove {{.*}}",
# CHECK-NEXT: "needsConfirmation": true
# CHECK-NEXT: }
# CHECK-NEXT: },
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -163,10 +163,14 @@
llvm_unreachable("Unknown symbol kind");
}
-std::vector<Diag> generateMissingIncludeDiagnostics(
+struct MissingIncludeDiag {
+ Diag D;
+ const MissingIncludeDiagInfo * DInfo = nullptr;
+};
+std::vector<MissingIncludeDiag> generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef<MissingIncludeDiagInfo> MissingIncludes,
llvm::StringRef Code) {
- std::vector<Diag> Result;
+ std::vector<MissingIncludeDiag> Result;
const Config &Cfg = Config::current();
if (Cfg.Diagnostics.MissingIncludes != Config::IncludesPolicy::Strict ||
Cfg.Diagnostics.SuppressAll ||
@@ -210,7 +214,9 @@
if (!Replacement.has_value())
continue;
- Diag &D = Result.emplace_back();
+ auto & R = Result.emplace_back();
+ R.DInfo = &SymbolWithMissingInclude;
+ auto & D = R.D;
D.Message =
llvm::formatv("No header providing \"{0}\" is directly included",
getSymbolName(SymbolWithMissingInclude.Symbol));
@@ -436,9 +442,9 @@
return {std::move(UnusedIncludes), std::move(MissingIncludes)};
}
-Fix removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes) {
+Fix removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes,
+ llvm::StringRef Code) {
assert(!UnusedIncludes.empty());
-
Fix RemoveAll;
RemoveAll.Message = "remove all unused includes";
for (const auto &Diag : UnusedIncludes) {
@@ -448,46 +454,64 @@
Diag.Fixes.front().Edits.end());
}
- // TODO(hokein): emit a suitable text for the label.
- ChangeAnnotation Annotation = {/*label=*/"",
- /*needsConfirmation=*/true,
- /*description=*/""};
+ llvm::SmallVector<llvm::StringRef> Lines;
+ Code.split(Lines, "\n");
static const ChangeAnnotationIdentifier RemoveAllUnusedID =
"RemoveAllUnusedIncludes";
for (unsigned I = 0; I < RemoveAll.Edits.size(); ++I) {
ChangeAnnotationIdentifier ID = RemoveAllUnusedID + std::to_string(I);
+ unsigned StartLine = RemoveAll.Edits[I].range.start.line;
+ assert(StartLine < Lines.size() && "Out of bound line number!");
+
RemoveAll.Edits[I].annotationId = ID;
- RemoveAll.Annotations.push_back({ID, Annotation});
+ RemoveAll.Annotations.push_back(
+ {ID, ChangeAnnotation{
+ /*label=*/llvm::formatv("Remove '{0}'", Lines[StartLine]),
+ /*needsConfirmation=*/true,
+ /*description=*/""}
+ });
}
return RemoveAll;
}
-Fix addAllMissingIncludes(llvm::ArrayRef<Diag> MissingIncludeDiags) {
+Fix addAllMissingIncludes(
+ llvm::ArrayRef<MissingIncludeDiag> MissingIncludeDiags,
+ llvm::StringRef Code) {
assert(!MissingIncludeDiags.empty());
Fix AddAllMissing;
AddAllMissing.Message = "add all missing includes";
// A map to deduplicate the edits with the same new text.
// newText (#include "my_missing_header.h") -> TextEdit.
- llvm::StringMap<TextEdit> Edits;
+ llvm::StringMap<std::pair<TextEdit, const MissingIncludeDiag*>> Edits;
for (const auto &Diag : MissingIncludeDiags) {
- assert(Diag.Fixes.size() == 1 && "Expected exactly one fix.");
- for (const auto& Edit : Diag.Fixes.front().Edits) {
- Edits.try_emplace(Edit.newText, Edit);
+ assert(Diag.D.Fixes.size() == 1 && "Expected exactly one fix.");
+ for (const auto& Edit : Diag.D.Fixes.front().Edits) {
+ Edits.try_emplace(Edit.newText, std::make_pair(Edit, &Diag));
}
}
- // FIXME(hokein): emit used symbol reference in the annotation.
- ChangeAnnotation Annotation = {/*label=*/"",
- /*needsConfirmation=*/true,
- /*description=*/""};
+ llvm::SmallVector<llvm::StringRef> Lines;
+ Code.split(Lines, "\n");
+
static const ChangeAnnotationIdentifier AddAllMissingID =
"AddAllMissingIncludes";
unsigned I = 0;
- for (auto &It : Edits) {
+ for (auto &[NewText, EditAndDiag] : Edits) {
ChangeAnnotationIdentifier ID = AddAllMissingID + std::to_string(I++);
- AddAllMissing.Edits.push_back(std::move(It.getValue()));
+ AddAllMissing.Edits.push_back(std::move(EditAndDiag.first));
AddAllMissing.Edits.back().annotationId = ID;
- AddAllMissing.Annotations.push_back({ID, Annotation});
+ unsigned StartLine = EditAndDiag.second->D.Range.start.line;
+ assert(StartLine < Lines.size() && "Out of bound line number!");
+
+ AddAllMissing.Annotations.push_back(
+ {ID,
+ ChangeAnnotation{
+ /*label=*/llvm::formatv("Add '{0}' for symbol '{1}'",
+ NewText.rtrim("\n"),
+ EditAndDiag.second->DInfo->Symbol),
+ /*needsConfirmation=*/true,
+ /*description=*/
+ llvm::formatv("Line {0}: '{1}'", StartLine, Lines[StartLine])}});
}
return AddAllMissing;
}
@@ -514,13 +538,13 @@
AST.tuPath(), Findings.UnusedIncludes, Code);
std::optional<Fix> RemoveAllUnused;;
if (UnusedIncludes.size() > 1)
- RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes);
+ RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes, Code);
- std::vector<Diag> MissingIncludeDiags = generateMissingIncludeDiagnostics(
+ auto MissingIncludeDiags = generateMissingIncludeDiagnostics(
AST, Findings.MissingIncludes, Code);
std::optional<Fix> AddAllMissing;
if (MissingIncludeDiags.size() > 1)
- AddAllMissing = addAllMissingIncludes(MissingIncludeDiags);
+ AddAllMissing = addAllMissingIncludes(MissingIncludeDiags, Code);
std::optional<Fix> FixAll;
if (RemoveAllUnused && AddAllMissing)
@@ -531,17 +555,18 @@
Out->Fixes.push_back(*F);
};
for (auto &Diag : MissingIncludeDiags) {
- AddBatchFix(AddAllMissing, &Diag);
- AddBatchFix(FixAll, &Diag);
+ AddBatchFix(AddAllMissing, &Diag.D);
+ AddBatchFix(FixAll, &Diag.D);
}
for (auto &Diag : UnusedIncludes) {
AddBatchFix(RemoveAllUnused, &Diag);
AddBatchFix(FixAll, &Diag);
}
- auto Result = std::move(MissingIncludeDiags);
- llvm::move(UnusedIncludes,
- std::back_inserter(Result));
+ std::vector<Diag> Result;
+ for (auto& Diag : MissingIncludeDiags)
+ Result.push_back(std::move(Diag.D));
+ llvm::move(UnusedIncludes, std::back_inserter(Result));
return Result;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
