diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -163,10 +163,14 @@ llvm_unreachable("Unknown symbol kind"); } -std::vector generateMissingIncludeDiagnostics( +struct MissingIncludeDiag { + Diag D; + const MissingIncludeDiagInfo * DInfo = nullptr; +}; +std::vector generateMissingIncludeDiagnostics( ParsedAST &AST, llvm::ArrayRef MissingIncludes, llvm::StringRef Code) { - std::vector Result; + std::vector 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 UnusedIncludes) { +Fix removeAllUnusedIncludes(llvm::ArrayRef 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 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 MissingIncludeDiags) { +Fix addAllMissingIncludes( + llvm::ArrayRef 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 Edits; + llvm::StringMap> 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 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 RemoveAllUnused;; if (UnusedIncludes.size() > 1) - RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes); + RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes, Code); - std::vector MissingIncludeDiags = generateMissingIncludeDiagnostics( + auto MissingIncludeDiags = generateMissingIncludeDiagnostics( AST, Findings.MissingIncludes, Code); std::optional AddAllMissing; if (MissingIncludeDiags.size() > 1) - AddAllMissing = addAllMissingIncludes(MissingIncludeDiags); + AddAllMissing = addAllMissingIncludes(MissingIncludeDiags, Code); std::optional 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 Result; + for (auto& Diag : MissingIncludeDiags) + Result.push_back(std::move(Diag.D)); + llvm::move(UnusedIncludes, std::back_inserter(Result)); return Result; } diff --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test --- a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test +++ b/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: },