[clang-tools-extra] [clangd] Dont require confirmation for include-cleaner batch-fixes (PR #76826)
kadir çetinkaya via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 3 07:09:10 PST 2024
https://github.com/kadircet created https://github.com/llvm/llvm-project/pull/76826
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 :)
>From 5e9a851584dcfd730fda7d85b54101ebea89f8a3 Mon Sep 17 00:00:00 2001
From: Kadir Cetinkaya <kadircet at google.com>
Date: Wed, 3 Jan 2024 16:06:13 +0100
Subject: [PATCH] [clangd] Dont require confirmation for include-cleaner
batch-fixes
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 :)
---
clang-tools-extra/clangd/IncludeCleaner.cpp | 27 +-------
.../test/include-cleaner-batch-fix.test | 68 -------------------
2 files changed, 1 insertion(+), 94 deletions(-)
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": {
More information about the cfe-commits
mailing list