Skip to content

Commit b222933

Browse files
FMorschelCommit Queue
FMorschel
authored and
Commit Queue
committed
[DAS] Adds remove const fix error to most common const errors
Following the work done at https://dart-review.googlesource.com/c/sdk/+/276941. But trying to merge this time. Fixes: #49818 Fixes: #53599 Change-Id: I7ed6890ec3d526b855dd71b45e0493cbaf419d6d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/410862 Reviewed-by: Samuel Rawlins <[email protected]> Auto-Submit: Felipe Morschel <[email protected]> Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent 50ff1c3 commit b222933

File tree

6 files changed

+716
-72
lines changed

6 files changed

+716
-72
lines changed

‎pkg/analysis_server/lib/src/services/correction/dart/remove_const.dart‎

Lines changed: 131 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'package:analysis_server/src/services/correction/fix.dart';
66
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
7+
import 'package:analysis_server_plugin/src/correction/fix_generators.dart';
78
import 'package:analyzer/dart/ast/token.dart';
89
import 'package:analyzer/dart/ast/visitor.dart';
910
import 'package:analyzer/error/error.dart';
@@ -14,15 +15,15 @@ import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dar
1415
import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
1516
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
1617
import 'package:analyzer_plugin/utilities/range_factory.dart';
18+
import 'package:collection/collection.dart';
1719

1820
class RemoveConst extends _RemoveConst {
1921
RemoveConst({required super.context});
2022

2123
@override
2224
CorrectionApplicability get applicability =>
23-
// Not predictably the correct action.
24-
CorrectionApplicability
25-
.singleLocation;
25+
// Not predictably the correct action.
26+
CorrectionApplicability.singleLocation;
2627

2728
@override
2829
FixKind get fixKind => DartFixKind.REMOVE_CONST;
@@ -42,9 +43,34 @@ class RemoveUnnecessaryConst extends _RemoveConst {
4243
FixKind get multiFixKind => DartFixKind.REMOVE_UNNECESSARY_CONST_MULTI;
4344
}
4445

46+
class _ChildrenVisitor extends GeneralizingAstVisitor<AstNode?> {
47+
AstNode? _selectedNode;
48+
49+
final int offset;
50+
final int end;
51+
52+
_ChildrenVisitor(this.offset, this.end);
53+
54+
AstNode get selectedNode => _selectedNode!;
55+
56+
@override
57+
AstNode? visitNode(AstNode node) {
58+
if (node.offset > offset || node.end < end) {
59+
return null;
60+
}
61+
if (node.offset <= offset && node.end >= end) {
62+
var result = super.visitNode(node);
63+
if (result != null) {
64+
return _selectedNode ??= result;
65+
}
66+
}
67+
return _selectedNode ??= node;
68+
}
69+
}
70+
4571
class _PushConstVisitor extends GeneralizingAstVisitor<void> {
4672
final DartFileEditBuilder builder;
47-
final AstNode excluded;
73+
final List<AstNode> excluded;
4874

4975
_PushConstVisitor(this.builder, this.excluded);
5076

@@ -56,13 +82,13 @@ class _PushConstVisitor extends GeneralizingAstVisitor<void> {
5682

5783
@override
5884
void visitInstanceCreationExpression(InstanceCreationExpression node) {
59-
if (node == excluded) {
60-
// Don't speculate whether arguments can be const.
61-
} else if (_containsExcluded(node)) {
85+
if (_containsExcluded(node)) {
6286
node.argumentList.visitChildren(this);
87+
} else if (excluded.any(node.contains)) {
88+
// Don't speculate whether arguments can be const.
6389
} else {
6490
if (node.keyword == null) {
65-
builder.addSimpleInsertion(node.offset, 'const ');
91+
builder.addSimpleInsertion(node.offset, '${Keyword.CONST.lexeme} ');
6692
}
6793
}
6894
}
@@ -72,9 +98,6 @@ class _PushConstVisitor extends GeneralizingAstVisitor<void> {
7298
node.expression.accept(this);
7399
}
74100

75-
@override
76-
void visitNode(AstNode node) {}
77-
78101
@override
79102
void visitSpreadElement(SpreadElement node) {
80103
node.expression.accept(this);
@@ -86,7 +109,7 @@ class _PushConstVisitor extends GeneralizingAstVisitor<void> {
86109
node.visitChildren(this);
87110
} else {
88111
if (node.constKeyword == null) {
89-
builder.addSimpleInsertion(node.offset, 'const ');
112+
builder.addSimpleInsertion(node.offset, '${Keyword.CONST.lexeme} ');
90113
}
91114
}
92115
}
@@ -102,15 +125,31 @@ class _PushConstVisitor extends GeneralizingAstVisitor<void> {
102125
}
103126

104127
bool _containsExcluded(AstNode node) {
105-
return excluded.withParents.contains(node);
128+
return {for (var e in excluded) ...e.withParents}.contains(node);
106129
}
107130
}
108131

109132
abstract class _RemoveConst extends ParsedCorrectionProducer {
110133
_RemoveConst({required super.context});
111134

135+
/// A map of all the error codes that this fix can be applied to and the
136+
/// generators that can be used to apply the fix.
137+
Map<ErrorCode, List<ProducerGenerator>> get _errorCodesWhereThisIsValid {
138+
var constructors = [RemoveUnnecessaryConst.new, RemoveConst.new];
139+
var nonLintMultiProducers = registeredFixGenerators.nonLintProducers;
140+
return {
141+
for (var MapEntry(:key, :value) in nonLintMultiProducers.entries)
142+
if (value.containsAny(constructors)) key: value,
143+
};
144+
}
145+
112146
@override
113147
Future<void> compute(ChangeBuilder builder) async {
148+
var diagnostic = this.diagnostic;
149+
if (diagnostic is! AnalysisError) {
150+
return;
151+
}
152+
114153
switch (node) {
115154
case ClassDeclaration declaration:
116155
var first = declaration.firstTokenAfterCommentAndMetadata;
@@ -132,28 +171,44 @@ abstract class _RemoveConst extends ParsedCorrectionProducer {
132171
if (constantContext != null) {
133172
var constKeyword = constantContext.$2;
134173
if (constKeyword != null) {
174+
var validErrors = _errorCodesWhereThisIsValid.keys.toList();
175+
var validDiagnostics = unitResult.errors.whereErrorCodeIn(
176+
validErrors,
177+
);
135178
switch (constantContext.$1) {
136179
case InstanceCreationExpression contextNode:
137-
await builder.addDartFileEdit(file, (builder) {
180+
var (:constNodes, :nodesWithDiagnostic) = contextNode
181+
.argumentList
182+
.arguments
183+
.withErrorCodeIn(validDiagnostics);
184+
await builder.addDartFileEdit(file, (builder) async {
138185
_deleteToken(builder, constKeyword);
139-
contextNode.accept(_PushConstVisitor(builder, expression));
186+
contextNode.accept(
187+
_PushConstVisitor(builder, nodesWithDiagnostic),
188+
);
140189
});
141190
case TypedLiteral contextNode:
142-
await builder.addDartFileEdit(file, (builder) {
191+
var (:constNodes, :nodesWithDiagnostic) = switch (contextNode) {
192+
ListLiteral list => list.elements,
193+
SetOrMapLiteral set => set.elements,
194+
}.withErrorCodeIn(validDiagnostics);
195+
await builder.addDartFileEdit(file, (builder) async {
143196
_deleteToken(builder, constKeyword);
144-
contextNode.accept(_PushConstVisitor(builder, expression));
197+
contextNode.accept(
198+
_PushConstVisitor(builder, nodesWithDiagnostic),
199+
);
145200
});
146201
case VariableDeclarationList contextNode:
202+
var (:constNodes, :nodesWithDiagnostic) = contextNode.variables
203+
.withErrorCodeIn(validDiagnostics);
147204
await builder.addDartFileEdit(file, (builder) {
148-
if (contextNode.type != null) {
149-
_deleteToken(builder, constKeyword);
150-
} else {
151-
builder.addSimpleReplacement(
152-
range.token(constKeyword),
153-
'var',
154-
);
155-
}
156-
contextNode.accept(_PushConstVisitor(builder, expression));
205+
builder.addSimpleReplacement(
206+
range.token(constKeyword),
207+
Keyword.FINAL.lexeme,
208+
);
209+
contextNode.accept(
210+
_PushConstVisitor(builder, nodesWithDiagnostic),
211+
);
157212
});
158213
}
159214
}
@@ -188,3 +243,53 @@ abstract class _RemoveConst extends ParsedCorrectionProducer {
188243
builder.addDeletion(range.startStart(token, token.next!));
189244
}
190245
}
246+
247+
extension on AnalysisError {
248+
int get end => offset + length;
249+
250+
bool isWithin(AstNode node) {
251+
return node.offset <= offset && node.end >= end;
252+
}
253+
}
254+
255+
extension on AstNode {
256+
bool contains(AstNode node) {
257+
return offset <= node.offset && end >= node.end;
258+
}
259+
}
260+
261+
extension on List<AstNode> {
262+
({List<AstNode> nodesWithDiagnostic, List<AstNode> constNodes})
263+
withErrorCodeIn(List<AnalysisError> diagnostics) {
264+
if (diagnostics.isEmpty) {
265+
return (constNodes: toList(), nodesWithDiagnostic: const []);
266+
}
267+
var invalidNodes = <AstNode>[];
268+
var constNodes = <AstNode>[];
269+
for (var node in this) {
270+
// If no error spans this node, it is valid.
271+
if (diagnostics.none((error) => error.isWithin(node))) {
272+
constNodes.add(node);
273+
continue;
274+
}
275+
var error = diagnostics.firstWhere((error) => error.isWithin(node));
276+
var visitor = _ChildrenVisitor(error.offset, error.end);
277+
node.accept(visitor);
278+
invalidNodes.add(visitor.selectedNode);
279+
}
280+
return (nodesWithDiagnostic: invalidNodes, constNodes: constNodes);
281+
}
282+
}
283+
284+
extension on List<AnalysisError> {
285+
List<AnalysisError> whereErrorCodeIn(List<ErrorCode> errors) =>
286+
where((error) {
287+
return errors.contains(error.errorCode);
288+
}).toList();
289+
}
290+
291+
extension<T> on Iterable<T> {
292+
bool containsAny(Iterable<T> values) {
293+
return values.any((v) => contains(v));
294+
}
295+
}

‎pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1196,25 +1196,25 @@ CompileTimeErrorCode.NON_CONSTANT_DEFAULT_VALUE_FROM_DEFERRED_LIBRARY:
11961196
notes: |-
11971197
Remove the `deferred` keyword from the import.
11981198
CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT:
1199-
status: noFix
1199+
status: hasFix
12001200
CompileTimeErrorCode.NON_CONSTANT_LIST_ELEMENT_FROM_DEFERRED_LIBRARY:
1201-
status: needsFix
1201+
status: hasFix
12021202
notes: |-
12031203
Remove the `deferred` keyword from the import.
12041204
CompileTimeErrorCode.NON_CONSTANT_MAP_ELEMENT:
1205-
status: noFix
1205+
status: hasFix
12061206
CompileTimeErrorCode.NON_CONSTANT_MAP_KEY:
1207-
status: noFix
1207+
status: hasFix
12081208
CompileTimeErrorCode.NON_CONSTANT_MAP_KEY_FROM_DEFERRED_LIBRARY:
1209-
status: needsFix
1209+
status: hasFix
12101210
notes: |-
12111211
Remove the `deferred` keyword from the import.
12121212
CompileTimeErrorCode.NON_CONSTANT_MAP_PATTERN_KEY:
12131213
status: hasFix
12141214
CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE:
1215-
status: noFix
1215+
status: hasFix
12161216
CompileTimeErrorCode.NON_CONSTANT_MAP_VALUE_FROM_DEFERRED_LIBRARY:
1217-
status: needsFix
1217+
status: hasFix
12181218
notes: |-
12191219
Remove the `deferred` keyword from the import.
12201220
CompileTimeErrorCode.NON_CONSTANT_RECORD_FIELD:
@@ -1228,7 +1228,7 @@ CompileTimeErrorCode.NON_CONSTANT_RELATIONAL_PATTERN_EXPRESSION:
12281228
notes: |-
12291229
Make the pattern expression a constant.
12301230
CompileTimeErrorCode.NON_CONSTANT_SET_ELEMENT:
1231-
status: noFix
1231+
status: hasFix
12321232
? CompileTimeErrorCode.NON_COVARIANT_TYPE_PARAMETER_POSITION_IN_REPRESENTATION_TYPE
12331233
: status: noFix
12341234
CompileTimeErrorCode.NON_EXHAUSTIVE_SWITCH_EXPRESSION:
@@ -1436,7 +1436,7 @@ CompileTimeErrorCode.RETURN_WITHOUT_VALUE:
14361436
CompileTimeErrorCode.SEALED_CLASS_SUBTYPE_OUTSIDE_OF_LIBRARY:
14371437
status: noFix
14381438
CompileTimeErrorCode.SET_ELEMENT_FROM_DEFERRED_LIBRARY:
1439-
status: needsFix
1439+
status: hasFix
14401440
notes: |-
14411441
Remove the `deferred` keyword from the import.
14421442
CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE_NULLABILITY:
@@ -1446,7 +1446,7 @@ CompileTimeErrorCode.SET_ELEMENT_TYPE_NOT_ASSIGNABLE:
14461446
CompileTimeErrorCode.SHARED_DEFERRED_PREFIX:
14471447
status: noFix
14481448
CompileTimeErrorCode.SPREAD_EXPRESSION_FROM_DEFERRED_LIBRARY:
1449-
status: needsFix
1449+
status: hasFix
14501450
notes: |-
14511451
Remove the `deferred` keyword from the import.
14521452
CompileTimeErrorCode.STATIC_ACCESS_TO_INSTANCE_MEMBER:

‎pkg/analysis_server/lib/src/services/correction/fix.dart‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,22 +1143,22 @@ abstract final class DartFixKind {
11431143
static const REMOVE_AWAIT = FixKind(
11441144
'dart.fix.remove.await',
11451145
DartFixKindPriority.standard,
1146-
'Remove await',
1146+
"Remove 'await'",
11471147
);
11481148
static const REMOVE_AWAIT_MULTI = FixKind(
11491149
'dart.fix.remove.await.multi',
11501150
DartFixKindPriority.inFile,
1151-
'Remove awaits in file',
1151+
"Remove 'await's in file",
11521152
);
11531153
static const REMOVE_BREAK = FixKind(
11541154
'dart.fix.remove.break',
11551155
DartFixKindPriority.standard,
1156-
'Remove break',
1156+
"Remove 'break'",
11571157
);
11581158
static const REMOVE_BREAK_MULTI = FixKind(
11591159
'dart.fix.remove.break.multi',
11601160
DartFixKindPriority.inFile,
1161-
'Remove unnecessary breaks in file',
1161+
"Remove unnecessary 'break's in file",
11621162
);
11631163
static const REMOVE_CHARACTER = FixKind(
11641164
'dart.fix.remove.character',
@@ -1193,7 +1193,7 @@ abstract final class DartFixKind {
11931193
static const REMOVE_CONST = FixKind(
11941194
'dart.fix.remove.const',
11951195
DartFixKindPriority.standard,
1196-
'Remove const',
1196+
"Remove 'const'",
11971197
);
11981198
static const REMOVE_CONSTRUCTOR = FixKind(
11991199
'dart.fix.remove.constructor',
@@ -1223,12 +1223,12 @@ abstract final class DartFixKind {
12231223
static const REMOVE_DEPRECATED_NEW_IN_COMMENT_REFERENCE = FixKind(
12241224
'dart.fix.remove.deprecatedNewInCommentReference',
12251225
DartFixKindPriority.standard,
1226-
'Remove deprecated new keyword',
1226+
"Remove deprecated 'new' keyword",
12271227
);
12281228
static const REMOVE_DEPRECATED_NEW_IN_COMMENT_REFERENCE_MULTI = FixKind(
12291229
'dart.fix.remove.deprecatedNewInCommentReference.multi',
12301230
DartFixKindPriority.inFile,
1231-
'Remove deprecated new keyword in file',
1231+
"Remove deprecated 'new' keyword in file",
12321232
);
12331233
static const REMOVE_DUPLICATE_CASE = FixKind(
12341234
'dart.fix.remove.duplicateCase',
@@ -1547,7 +1547,7 @@ abstract final class DartFixKind {
15471547
static const REMOVE_UNNECESSARY_CONST = FixKind(
15481548
'dart.fix.remove.unnecessaryConst',
15491549
DartFixKindPriority.standard,
1550-
'Remove unnecessary const keyword',
1550+
"Remove unnecessary 'const' keyword",
15511551
);
15521552
static const REMOVE_UNNECESSARY_CONST_MULTI = FixKind(
15531553
'dart.fix.remove.unnecessaryConst.multi',

0 commit comments

Comments
 (0)