Skip to content

Commit 59fee86

Browse files
srawlinsCommit Queue
authored andcommitted
DAS: Rearrange code to promote variables better
The motivation here was to remove the local duplicate variables like `final_foo` or `foo_final`. These variables only existed because the variables they duplicate lose their promoted types inside the `builder.addDartFileEdit` closure. They lose their promoted type because they are multiply assigned in loops or not always promoted to be non-null. Often the fix is to replace a `if (x == null) return` with `} else { return; }`. I think this code more directly represents the flow of code, rather than relying on the nullity of a variable, like "is this variable still null? Oh then we must not have entered any of the situations above, so we should return." Change-Id: I9dd6686b7f4d9c6caf59c171179b129a5a957ba8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/418160 Reviewed-by: Phil Quitslund <[email protected]> Commit-Queue: Samuel Rawlins <[email protected]>
1 parent d592a7e commit 59fee86

9 files changed

+62
-70
lines changed

pkg/analysis_server/lib/src/services/correction/dart/add_async.dart

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,27 +39,27 @@ class AddAsync extends ResolvedCorrectionProducer {
3939
switch (_type) {
4040
case _Type.missingReturn:
4141
var node = this.node;
42-
FunctionBody? body;
43-
DartType? returnType;
42+
FunctionBody body;
43+
DartType returnType;
4444
switch (node) {
4545
case FunctionDeclaration():
4646
body = node.functionExpression.body;
4747
if (node.declaredFragment?.element case var declaredElement?) {
4848
returnType = declaredElement.returnType;
4949
} else if (node.declaredFragment case var declaredFragment?) {
5050
returnType = declaredFragment.element.returnType;
51+
} else {
52+
return;
5153
}
5254
case MethodDeclaration():
5355
body = node.body;
5456
returnType = node.declaredFragment!.element.returnType;
55-
}
56-
if (body == null || returnType == null) {
57-
return;
57+
default:
58+
return;
5859
}
5960
if (_isFutureVoid(returnType) && _hasNoReturns(body)) {
60-
var final_body = body;
6161
await builder.addDartFileEdit(file, (builder) {
62-
builder.addSimpleInsertion(final_body.offset, 'async ');
62+
builder.addSimpleInsertion(body.offset, 'async ');
6363
});
6464
}
6565
case _Type.wrongReturnType:

pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_case_clauses.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class AddMissingEnumCaseClauses extends ResolvedCorrectionProducer {
8383
var singleIndent = utils.oneIndent;
8484

8585
var prefixString = prefix.isNotEmpty ? '$prefix.' : '';
86-
var enumName_final = '$prefixString$enumName';
86+
var prefixedEnumName = '$prefixString$enumName';
8787
await builder.addDartFileEdit(file, (builder) {
8888
builder.insertCaseClauseAtEnd(
8989
switchKeyword: statement.switchKeyword,
@@ -110,7 +110,7 @@ class AddMissingEnumCaseClauses extends ResolvedCorrectionProducer {
110110
// TODO(brianwilkerson): Consider inserting the names in order into the
111111
// switch statement.
112112
for (var constantName in unhandledEnumCases) {
113-
addMissingCase('$enumName_final.$constantName');
113+
addMissingCase('$prefixedEnumName.$constantName');
114114
}
115115
if (unhandledNullValue) {
116116
addMissingCase('null');

pkg/analysis_server/lib/src/services/correction/dart/change_to_static_access.dart

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,36 +28,40 @@ class ChangeToStaticAccess extends ResolvedCorrectionProducer {
2828

2929
@override
3030
Future<void> compute(ChangeBuilder builder) async {
31-
Expression? target;
32-
Element2? invokedElement;
3331
var identifier = node;
34-
if (identifier is SimpleIdentifier) {
35-
var parent = identifier.parent;
36-
if (parent is MethodInvocation) {
37-
if (parent.methodName == identifier) {
38-
target = parent.target;
39-
invokedElement = identifier.element;
40-
}
41-
} else if (parent is PrefixedIdentifier) {
42-
if (parent.identifier == identifier) {
43-
target = parent.prefix;
44-
invokedElement = identifier.element;
45-
}
32+
if (identifier is! SimpleIdentifier) {
33+
return;
34+
}
35+
36+
Expression target;
37+
var parent = identifier.parent;
38+
if (parent case MethodInvocation(target: var parentTarget?)) {
39+
if (parent.methodName != identifier) {
40+
return;
4641
}
42+
target = parentTarget;
43+
} else if (parent is PrefixedIdentifier) {
44+
if (parent.identifier != identifier) {
45+
return;
46+
}
47+
target = parent.prefix;
48+
} else {
49+
return;
4750
}
48-
if (target == null || invokedElement is! ExecutableElement2) {
51+
52+
var invokedElement = identifier.element;
53+
if (invokedElement is! ExecutableElement2) {
4954
return;
5055
}
5156

52-
var target_final = target;
5357
var declaringElement = invokedElement.enclosingElement2;
5458

5559
if (declaringElement is InterfaceElement2) {
5660
var declaringElementName = declaringElement.name3;
5761
if (declaringElementName != null) {
5862
_className = declaringElementName;
5963
await builder.addDartFileEdit(file, (builder) {
60-
builder.addReplacement(range.node(target_final), (builder) {
64+
builder.addReplacement(range.node(target), (builder) {
6165
builder.writeReference(declaringElement);
6266
});
6367
});
@@ -67,7 +71,7 @@ class ChangeToStaticAccess extends ResolvedCorrectionProducer {
6771
if (extensionName != null) {
6872
_className = extensionName;
6973
await builder.addDartFileEdit(file, (builder) {
70-
builder.addReplacement(range.node(target_final), (builder) {
74+
builder.addReplacement(range.node(target), (builder) {
7175
builder.writeReference(declaringElement);
7276
});
7377
});

pkg/analysis_server/lib/src/services/correction/dart/convert_to_expression_function_body.dart

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ class ConvertToExpressionFunctionBody extends ResolvedCorrectionProducer {
5050
if (statements.length != 1) {
5151
return;
5252
}
53-
var onlyStatement = statements.first;
54-
// prepare returned expression
55-
Expression? returnExpression;
56-
if (onlyStatement is ReturnStatement) {
57-
returnExpression = onlyStatement.expression;
53+
var onlyStatement = statements.single;
54+
// Prepare the returned expression.
55+
Expression returnExpression;
56+
if (onlyStatement case ReturnStatement(:var expression?)) {
57+
returnExpression = expression;
5858
if (onlyStatement.returnKeyword.precedingComments != null) {
5959
// TODO(srawlins): Include comments in fixed output.
6060
// https://github.com/dart-lang/sdk/issues/29313
@@ -82,25 +82,23 @@ class ConvertToExpressionFunctionBody extends ResolvedCorrectionProducer {
8282
// https://github.com/dart-lang/sdk/issues/29313
8383
return;
8484
}
85-
}
86-
if (returnExpression == null) {
85+
} else {
8786
return;
8887
}
8988

90-
// Return expressions can be quite large, e.g. Flutter build() methods.
89+
// Return expressions can be quite large, e.g. Flutter `build()` methods.
9190
// It is surprising to see this Quick Assist deep in the function body.
9291
if (selectionOffset >= returnExpression.offset) {
9392
return;
9493
}
9594

96-
var returnExpression_final = returnExpression;
9795
await builder.addDartFileEdit(file, (builder) {
9896
builder.addReplacement(range.node(body), (builder) {
9997
if (body.isAsynchronous) {
10098
builder.write('async ');
10199
}
102100
builder.write('=> ');
103-
builder.write(utils.getNodeText(returnExpression_final));
101+
builder.write(utils.getNodeText(returnExpression));
104102
var parent = body.parent;
105103
if (parent is! FunctionExpression ||
106104
parent.parent is FunctionDeclaration) {

pkg/analysis_server/lib/src/services/correction/dart/convert_to_relative_import.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,10 @@ class ConvertToRelativeImport extends ResolvedCorrectionProducer {
7474
from: path.dirname(sourceUri.path),
7575
);
7676

77-
var node_final = targetNode;
77+
var uriNode = targetNode.uri;
7878
await builder.addDartFileEdit(file, (builder) {
7979
builder.addSimpleReplacement(
80-
range.node(node_final.uri).getExpanded(-1),
80+
range.node(uriNode).getExpanded(-1),
8181
relativePath,
8282
);
8383
});

pkg/analysis_server/lib/src/services/correction/dart/remove_unused_parameter.dart

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,20 @@ class RemoveUnusedParameter extends ResolvedCorrectionProducer {
2525

2626
@override
2727
Future<void> compute(ChangeBuilder builder) async {
28-
// To work for the unused_parameter hint as well as the lint, we must
29-
// allow for passing in `SimpleIdentifier`s.
28+
// To work for the unused_element_parameter warning as well as the lint (?),
29+
// we must allow for passing in `SimpleIdentifier`s.
3030
var maybeParameter = node;
3131
var maybeParameterParent = maybeParameter.parent;
3232
if (maybeParameter is SimpleIdentifier && maybeParameterParent != null) {
3333
maybeParameter = maybeParameterParent;
3434
}
3535

36-
var parameter = maybeParameter;
37-
if (parameter is! FormalParameter) {
36+
if (maybeParameter is! FormalParameter) {
3837
return;
3938
}
4039

41-
var parent = parameter.parent;
42-
if (parent is DefaultFormalParameter) {
43-
parameter = parent;
44-
}
40+
var parent = maybeParameter.parent;
41+
var parameter = parent is DefaultFormalParameter ? parent : maybeParameter;
4542

4643
var parameterList = parameter.parent;
4744
if (parameterList is! FormalParameterList) {
@@ -50,7 +47,6 @@ class RemoveUnusedParameter extends ResolvedCorrectionProducer {
5047

5148
var parameters = parameterList.parameters;
5249
var index = parameters.indexOf(parameter);
53-
var parameter_final = parameter;
5450
await builder.addDartFileEdit(file, (builder) {
5551
if (index == 0) {
5652
// Remove the first parameter in the list.
@@ -65,7 +61,7 @@ class RemoveUnusedParameter extends ResolvedCorrectionProducer {
6561
);
6662
} else {
6763
var following = parameters[1];
68-
if (parameter_final.isRequiredPositional &&
64+
if (parameter.isRequiredPositional &&
6965
!following.isRequiredPositional) {
7066
// The parameter to be removed and the following parameter are not
7167
// of the same kind, so there is a delimiter between them that we
@@ -85,8 +81,7 @@ class RemoveUnusedParameter extends ResolvedCorrectionProducer {
8581
}
8682
} else {
8783
var preceding = parameters[index - 1];
88-
if (preceding.isRequiredPositional &&
89-
!parameter_final.isRequiredPositional) {
84+
if (preceding.isRequiredPositional && !parameter.isRequiredPositional) {
9085
// The parameter to be removed and the preceding parameter are not
9186
// of the same kind, so there is a delimiter between them.
9287
if (index == parameters.length - 1) {

pkg/analysis_server/lib/src/services/correction/dart/replace_null_with_closure.dart

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class ReplaceNullWithClosure extends ResolvedCorrectionProducer {
2626

2727
@override
2828
Future<void> compute(ChangeBuilder builder) async {
29-
AstNode? nodeToFix;
29+
AstNode nodeToFix;
3030
var parameters = const <FormalParameterElement>[];
3131

3232
var coveringNode = this.coveringNode;
@@ -41,18 +41,17 @@ class ReplaceNullWithClosure extends ResolvedCorrectionProducer {
4141
}
4242
}
4343
nodeToFix = expression;
44+
} else {
45+
return;
4446
}
4547
} else if (coveringNode is NullLiteral) {
4648
nodeToFix = coveringNode;
47-
}
48-
49-
if (nodeToFix == null) {
49+
} else {
5050
return;
5151
}
5252

53-
var nodeToFix_final = nodeToFix;
5453
await builder.addDartFileEdit(file, (builder) {
55-
builder.addReplacement(range.node(nodeToFix_final), (builder) {
54+
builder.addReplacement(range.node(nodeToFix), (builder) {
5655
builder.writeFormalParameters(parameters);
5756
builder.write(' => null');
5857
});

pkg/analysis_server/lib/src/services/correction/dart/replace_with_null_aware.dart

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,9 @@ class ReplaceWithNullAware extends ResolvedCorrectionProducer {
4545
}
4646

4747
Future<void> _computeInChain(ChangeBuilder builder) async {
48-
var node = coveringNode;
49-
if (node is Expression) {
50-
var node_final = node;
51-
await builder.addDartFileEdit(file, (builder) {
52-
var parent = node_final.parent;
48+
await builder.addDartFileEdit(file, (builder) {
49+
var node = coveringNode;
50+
if (node case Expression(:var parent)) {
5351
while (parent != null) {
5452
if (parent is MethodInvocation && parent.target == node) {
5553
var operator = parent.operator;
@@ -62,10 +60,10 @@ class ReplaceWithNullAware extends ResolvedCorrectionProducer {
6260
break;
6361
}
6462
node = parent;
65-
parent = node?.parent;
63+
parent = node.parent;
6664
}
67-
});
68-
}
65+
}
66+
});
6967
}
7068

7169
Future<void> _computeSingle(ChangeBuilder builder) async {

pkg/analysis_server/lib/src/services/correction/dart/update_sdk_constraints.dart

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class UpdateSdkConstraints extends ResolvedCorrectionProducer {
4949
length = spaceOffset;
5050
}
5151

52-
String? newText;
52+
String newText;
5353
if (text == 'any') {
5454
newText = '^$_minimumVersion';
5555
} else if (text.startsWith('^')) {
@@ -58,14 +58,12 @@ class UpdateSdkConstraints extends ResolvedCorrectionProducer {
5858
newText = '>=$_minimumVersion';
5959
} else if (text.startsWith('>')) {
6060
newText = '>=$_minimumVersion';
61-
}
62-
if (newText == null) {
61+
} else {
6362
return;
6463
}
6564

66-
var newText_final = newText;
6765
await builder.addYamlFileEdit(pubspecFile.path, (builder) {
68-
builder.addSimpleReplacement(SourceRange(offset, length), newText_final);
66+
builder.addSimpleReplacement(SourceRange(offset, length), newText);
6967
});
7068
}
7169

0 commit comments

Comments
 (0)