[clang-tools-extra] 2336f79 - [clangd] Dont require confirmation for include-cleaner batch-fixes (#76826)

via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 4 03:57:46 PST 2024


Author: kadir çetinkaya
Date: 2024-01-04T12:57:42+01:00
New Revision: 2336f792bc5a1d9195c1bd995b6040c13e73d4e7

URL: https://github.com/llvm/llvm-project/commit/2336f792bc5a1d9195c1bd995b6040c13e73d4e7
DIFF: https://github.com/llvm/llvm-project/commit/2336f792bc5a1d9195c1bd995b6040c13e73d4e7.diff

LOG: [clangd] Dont require confirmation for include-cleaner batch-fixes (#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 :)

Added: 
    

Modified: 
    clang-tools-extra/clangd/IncludeCleaner.cpp
    clang-tools-extra/clangd/test/include-cleaner-batch-fix.test

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 563a1f5d22f5b5..f86a121340f7fb 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) {
@@ -292,11 +267,6 @@ Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) {
     FixAll.Edits.push_back(F);
   for (const auto &F : AddAllMissing.Edits)
     FixAll.Edits.push_back(F);
-
-  for (const auto &A : RemoveAllUnused.Annotations)
-    FixAll.Annotations.push_back(A);
-  for (const auto &A : AddAllMissing.Annotations)
-    FixAll.Annotations.push_back(A);
   return FixAll;
 }
 

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