Skip to content

Commit 2336f79

Browse files
kadircetkadircet
authored
[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 :)
1 parent f5efa74 commit 2336f79

File tree

2 files changed

+1
-99
lines changed

2 files changed

+1
-99
lines changed

‎clang-tools-extra/clangd/IncludeCleaner.cpp

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
#include <cassert>
4949
#include <iterator>
5050
#include <map>
51-
#include <memory>
5251
#include <optional>
5352
#include <string>
5453
#include <utility>
@@ -237,18 +236,6 @@ removeAllUnusedIncludes(llvm::ArrayRef<Diag> UnusedIncludes) {
237236
Diag.Fixes.front().Edits.begin(),
238237
Diag.Fixes.front().Edits.end());
239238
}
240-
241-
// TODO(hokein): emit a suitable text for the label.
242-
ChangeAnnotation Annotation = {/*label=*/"",
243-
/*needsConfirmation=*/true,
244-
/*description=*/""};
245-
static const ChangeAnnotationIdentifier RemoveAllUnusedID =
246-
"RemoveAllUnusedIncludes";
247-
for (unsigned I = 0; I < RemoveAll.Edits.size(); ++I) {
248-
ChangeAnnotationIdentifier ID = RemoveAllUnusedID + std::to_string(I);
249-
RemoveAll.Edits[I].annotationId = ID;
250-
RemoveAll.Annotations.push_back({ID, Annotation});
251-
}
252239
return RemoveAll;
253240
}
254241

@@ -268,20 +255,8 @@ addAllMissingIncludes(llvm::ArrayRef<Diag> MissingIncludeDiags) {
268255
Edits.try_emplace(Edit.newText, Edit);
269256
}
270257
}
271-
// FIXME(hokein): emit used symbol reference in the annotation.
272-
ChangeAnnotation Annotation = {/*label=*/"",
273-
/*needsConfirmation=*/true,
274-
/*description=*/""};
275-
static const ChangeAnnotationIdentifier AddAllMissingID =
276-
"AddAllMissingIncludes";
277-
unsigned I = 0;
278-
for (auto &It : Edits) {
279-
ChangeAnnotationIdentifier ID = AddAllMissingID + std::to_string(I++);
258+
for (auto &It : Edits)
280259
AddAllMissing.Edits.push_back(std::move(It.second));
281-
AddAllMissing.Edits.back().annotationId = ID;
282-
283-
AddAllMissing.Annotations.push_back({ID, Annotation});
284-
}
285260
return AddAllMissing;
286261
}
287262
Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) {
@@ -292,11 +267,6 @@ Fix fixAll(const Fix &RemoveAllUnused, const Fix &AddAllMissing) {
292267
FixAll.Edits.push_back(F);
293268
for (const auto &F : AddAllMissing.Edits)
294269
FixAll.Edits.push_back(F);
295-
296-
for (const auto &A : RemoveAllUnused.Annotations)
297-
FixAll.Annotations.push_back(A);
298-
for (const auto &A : AddAllMissing.Annotations)
299-
FixAll.Annotations.push_back(A);
300270
return FixAll;
301271
}
302272

‎clang-tools-extra/clangd/test/include-cleaner-batch-fix.test

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -157,21 +157,10 @@
157157
# CHECK-NEXT: {
158158
# CHECK-NEXT: "arguments": [
159159
# CHECK-NEXT: {
160-
# CHECK-NEXT: "changeAnnotations": {
161-
# CHECK-NEXT: "AddAllMissingIncludes0": {
162-
# CHECK-NEXT: "label": "",
163-
# CHECK-NEXT: "needsConfirmation": true
164-
# CHECK-NEXT: },
165-
# CHECK-NEXT: "AddAllMissingIncludes1": {
166-
# CHECK-NEXT: "label": "",
167-
# CHECK-NEXT: "needsConfirmation": true
168-
# CHECK-NEXT: }
169-
# CHECK-NEXT: },
170160
# CHECK-NEXT: "documentChanges": [
171161
# CHECK-NEXT: {
172162
# CHECK-NEXT: "edits": [
173163
# CHECK-NEXT: {
174-
# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0",
175164
# CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}",
176165
# CHECK-NEXT: "range": {
177166
# CHECK-NEXT: "end": {
@@ -185,7 +174,6 @@
185174
# CHECK-NEXT: }
186175
# CHECK-NEXT: },
187176
# CHECK-NEXT: {
188-
# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1",
189177
# CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}",
190178
# CHECK-NEXT: "range": {
191179
# CHECK-NEXT: "end": {
@@ -213,29 +201,10 @@
213201
# CHECK-NEXT: {
214202
# CHECK-NEXT: "arguments": [
215203
# CHECK-NEXT: {
216-
# CHECK-NEXT: "changeAnnotations": {
217-
# CHECK-NEXT: "AddAllMissingIncludes0": {
218-
# CHECK-NEXT: "label": "",
219-
# CHECK-NEXT: "needsConfirmation": true
220-
# CHECK-NEXT: },
221-
# CHECK-NEXT: "AddAllMissingIncludes1": {
222-
# CHECK-NEXT: "label": "",
223-
# CHECK-NEXT: "needsConfirmation": true
224-
# CHECK-NEXT: },
225-
# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
226-
# CHECK-NEXT: "label": "",
227-
# CHECK-NEXT: "needsConfirmation": true
228-
# CHECK-NEXT: },
229-
# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
230-
# CHECK-NEXT: "label": "",
231-
# CHECK-NEXT: "needsConfirmation": true
232-
# CHECK-NEXT: }
233-
# CHECK-NEXT: },
234204
# CHECK-NEXT: "documentChanges": [
235205
# CHECK-NEXT: {
236206
# CHECK-NEXT: "edits": [
237207
# CHECK-NEXT: {
238-
# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0",
239208
# CHECK-NEXT: "newText": "",
240209
# CHECK-NEXT: "range": {
241210
# CHECK-NEXT: "end": {
@@ -249,7 +218,6 @@
249218
# CHECK-NEXT: }
250219
# CHECK-NEXT: },
251220
# CHECK-NEXT: {
252-
# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1",
253221
# CHECK-NEXT: "newText": "",
254222
# CHECK-NEXT: "range": {
255223
# CHECK-NEXT: "end": {
@@ -263,7 +231,6 @@
263231
# CHECK-NEXT: }
264232
# CHECK-NEXT: },
265233
# CHECK-NEXT: {
266-
# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0",
267234
# CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}",
268235
# CHECK-NEXT: "range": {
269236
# CHECK-NEXT: "end": {
@@ -277,7 +244,6 @@
277244
# CHECK-NEXT: }
278245
# CHECK-NEXT: },
279246
# CHECK-NEXT: {
280-
# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1",
281247
# CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}",
282248
# CHECK-NEXT: "range": {
283249
# CHECK-NEXT: "end": {
@@ -342,21 +308,10 @@
342308
# CHECK-NEXT: {
343309
# CHECK-NEXT: "arguments": [
344310
# CHECK-NEXT: {
345-
# CHECK-NEXT: "changeAnnotations": {
346-
# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
347-
# CHECK-NEXT: "label": "",
348-
# CHECK-NEXT: "needsConfirmation": true
349-
# CHECK-NEXT: },
350-
# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
351-
# CHECK-NEXT: "label": "",
352-
# CHECK-NEXT: "needsConfirmation": true
353-
# CHECK-NEXT: }
354-
# CHECK-NEXT: },
355311
# CHECK-NEXT: "documentChanges": [
356312
# CHECK-NEXT: {
357313
# CHECK-NEXT: "edits": [
358314
# CHECK-NEXT: {
359-
# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0",
360315
# CHECK-NEXT: "newText": "",
361316
# CHECK-NEXT: "range": {
362317
# CHECK-NEXT: "end": {
@@ -370,7 +325,6 @@
370325
# CHECK-NEXT: }
371326
# CHECK-NEXT: },
372327
# CHECK-NEXT: {
373-
# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1",
374328
# CHECK-NEXT: "newText": "",
375329
# CHECK-NEXT: "range": {
376330
# CHECK-NEXT: "end": {
@@ -398,29 +352,10 @@
398352
# CHECK-NEXT: {
399353
# CHECK-NEXT: "arguments": [
400354
# CHECK-NEXT: {
401-
# CHECK-NEXT: "changeAnnotations": {
402-
# CHECK-NEXT: "AddAllMissingIncludes0": {
403-
# CHECK-NEXT: "label": "",
404-
# CHECK-NEXT: "needsConfirmation": true
405-
# CHECK-NEXT: },
406-
# CHECK-NEXT: "AddAllMissingIncludes1": {
407-
# CHECK-NEXT: "label": "",
408-
# CHECK-NEXT: "needsConfirmation": true
409-
# CHECK-NEXT: },
410-
# CHECK-NEXT: "RemoveAllUnusedIncludes0": {
411-
# CHECK-NEXT: "label": "",
412-
# CHECK-NEXT: "needsConfirmation": true
413-
# CHECK-NEXT: },
414-
# CHECK-NEXT: "RemoveAllUnusedIncludes1": {
415-
# CHECK-NEXT: "label": "",
416-
# CHECK-NEXT: "needsConfirmation": true
417-
# CHECK-NEXT: }
418-
# CHECK-NEXT: },
419355
# CHECK-NEXT: "documentChanges": [
420356
# CHECK-NEXT: {
421357
# CHECK-NEXT: "edits": [
422358
# CHECK-NEXT: {
423-
# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes0",
424359
# CHECK-NEXT: "newText": "",
425360
# CHECK-NEXT: "range": {
426361
# CHECK-NEXT: "end": {
@@ -434,7 +369,6 @@
434369
# CHECK-NEXT: }
435370
# CHECK-NEXT: },
436371
# CHECK-NEXT: {
437-
# CHECK-NEXT: "annotationId": "RemoveAllUnusedIncludes1",
438372
# CHECK-NEXT: "newText": "",
439373
# CHECK-NEXT: "range": {
440374
# CHECK-NEXT: "end": {
@@ -448,7 +382,6 @@
448382
# CHECK-NEXT: }
449383
# CHECK-NEXT: },
450384
# CHECK-NEXT: {
451-
# CHECK-NEXT: "annotationId": "AddAllMissingIncludes0",
452385
# CHECK-NEXT: "newText": "#include {{.*}}bar.h{{.*}}",
453386
# CHECK-NEXT: "range": {
454387
# CHECK-NEXT: "end": {
@@ -462,7 +395,6 @@
462395
# CHECK-NEXT: }
463396
# CHECK-NEXT: },
464397
# CHECK-NEXT: {
465-
# CHECK-NEXT: "annotationId": "AddAllMissingIncludes1",
466398
# CHECK-NEXT: "newText": "#include {{.*}}foo.h{{.*}}",
467399
# CHECK-NEXT: "range": {
468400
# CHECK-NEXT: "end": {

0 commit comments

Comments
 (0)