[clang-tools-extra] [clangd] Dont require confirmation for include-cleaner batch-fixes (PR #76826)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 3 07:09:40 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clangd
Author: kadir çetinkaya (kadircet)
<details>
<summary>Changes</summary>
False negative/positive rate has decreased to the degree that these
extra confirmations no longer provide any value, but only create
friction in the happy case.
When we have false analysis, people usually need to apply the fixes and
run the builds to discover the failure. At that point they can add
relevant IWYU pragmas to guide analysis, and will be more likely to
create bug reports due to extra annoyance :)
---
Full diff: https://github.com/llvm/llvm-project/pull/76826.diff
2 Files Affected:
- (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+1-26)
- (modified) clang-tools-extra/clangd/test/include-cleaner-batch-fix.test (-68)
``````````diff
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 563a1f5d22f5b5..672140a6f2b4d8 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -48,7 +48,6 @@
#include <cassert>
#include <iterator>
#include <map>
-#include <memory>
#include <optional>
#include <string>
#include <utility>
@@ -237,18 +236,6 @@ removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes) {
Diag.Fixes.front().Edits.begin(),
Diag.Fixes.front().Edits.end());
}
-
- // TODO(hokein): emit a suitable text for the label.
- ChangeAnnotation Annotation = {/*label=*/"",
- /*needsConfirmation=*/true,
- /*description=*/""};
- static const ChangeAnnotationIdentifier RemoveAllUnusedID =
- "RemoveAllUnusedIncludes";
- for (unsigned I = 0; I < RemoveAll.Edits.size(); ++I) {
- ChangeAnnotationIdentifier ID = RemoveAllUnusedID + std::to_string(I);
- RemoveAll.Edits[I].annotationId = ID;
- RemoveAll.Annotations.push_back({ID, Annotation});
- }
return RemoveAll;
}
@@ -268,20 +255,8 @@ addAllMissingIncludes(llvm::ArrayRef<Diag> MissingIncludeDiags) {
Edits.try_emplace(Edit.newText, Edit);
}
}
- // FIXME(hokein): emit used symbol reference in the annotation.
- ChangeAnnotation Annotation = {/*label=*/"",
- /*needsConfirmation=*/true,
- /*description=*/""};
- static const ChangeAnnotationIdentifier AddAllMissingID =
- "AddAllMissingIncludes";
- unsigned I = 0;
- for (auto &It : Edits) {
- ChangeAnnotationIdentifier ID = AddAllMissingID + std::to_string(I++);
+ for (auto &It : Edits)
AddAllMissing.Edits.push_back(std::move(It.second));
- AddAllMissing.Edits.back().annotationId = ID;
-
- AddAllMissing.Annotations.push_back({ID, Annotation});
- }
return AddAllMissing;
}
Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) {
diff --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
index af7cdba5b05f43..07ebe1009a78f6 100644
--- a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
+++ b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
@@ -157,21 +157,10 @@
# CHECK-NEXT: {
# CHECK-NEXT: "arguments": [
# CHECK-NEXT: {
-# CHECK-NEXT: "changeAnnotations": {
-# CHECK-NEXT: "AddAllMissingIncludes0": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: },
-# CHECK-NEXT: "AddAllMissingIncludes1": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: }
-# CHECK-NEXT: },
# CHECK-NEXT: "documentChanges": [
# CHECK-NEXT: {
# CHECK-NEXT: "edits": [
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0",
# CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -185,7 +174,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1",
# CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -213,29 +201,10 @@
# CHECK-NEXT: {
# CHECK-NEXT: "arguments": [
# CHECK-NEXT: {
-# CHECK-NEXT: "changeAnnotations": {
-# CHECK-NEXT: "AddAllMissingIncludes0": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: },
-# CHECK-NEXT: "AddAllMissingIncludes1": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: },
-# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: },
-# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: }
-# CHECK-NEXT: },
# CHECK-NEXT: "documentChanges": [
# CHECK-NEXT: {
# CHECK-NEXT: "edits": [
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0",
# CHECK-NEXT: "newText": "",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -249,7 +218,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1",
# CHECK-NEXT: "newText": "",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -263,7 +231,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0",
# CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -277,7 +244,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1",
# CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -342,21 +308,10 @@
# CHECK-NEXT: {
# CHECK-NEXT: "arguments": [
# CHECK-NEXT: {
-# CHECK-NEXT: "changeAnnotations": {
-# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: },
-# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: }
-# CHECK-NEXT: },
# CHECK-NEXT: "documentChanges": [
# CHECK-NEXT: {
# CHECK-NEXT: "edits": [
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0",
# CHECK-NEXT: "newText": "",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -370,7 +325,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1",
# CHECK-NEXT: "newText": "",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -398,29 +352,10 @@
# CHECK-NEXT: {
# CHECK-NEXT: "arguments": [
# CHECK-NEXT: {
-# CHECK-NEXT: "changeAnnotations": {
-# CHECK-NEXT: "AddAllMissingIncludes0": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: },
-# CHECK-NEXT: "AddAllMissingIncludes1": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: },
-# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: },
-# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
-# CHECK-NEXT: "label": "",
-# CHECK-NEXT: "needsConfirmation": true
-# CHECK-NEXT: }
-# CHECK-NEXT: },
# CHECK-NEXT: "documentChanges": [
# CHECK-NEXT: {
# CHECK-NEXT: "edits": [
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0",
# CHECK-NEXT: "newText": "",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -434,7 +369,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1",
# CHECK-NEXT: "newText": "",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -448,7 +382,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0",
# CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@@ -462,7 +395,6 @@
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: {
-# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1",
# CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
``````````
</details>
https://github.com/llvm/llvm-project/pull/76826
More information about the cfe-commits
mailing list