dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

DAS: Rearrange code to promote variables better

Browse Source 
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 <pquitslund@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
This commit is contained in:
Sam Rawlins
2025-03-31 09:23:29 -07:00
committed by Commit Queue
parent d592a7eb26
commit 59fee86145
9 changed files with 64 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/analysis_server/lib/src/services/correction/dart/add_async.dart
+7 -7
View File
@@ -39,8 +39,8 @@ class AddAsync extends ResolvedCorrectionProducer {
switch (_type) {
case _Type.missingReturn:
var node = this.node;
FunctionBody? body;
DartType? returnType;
FunctionBody body;
DartType returnType;
switch (node) {
case FunctionDeclaration():
body = node.functionExpression.body;
@@ -48,18 +48,18 @@ class AddAsync extends ResolvedCorrectionProducer {
returnType = declaredElement.returnType;
} else if (node.declaredFragment case var declaredFragment?) {
returnType = declaredFragment.element.returnType;
} else {
return;
}
case MethodDeclaration():
body = node.body;
returnType = node.declaredFragment!.element.returnType;
}
if (body == null || returnType == null) {
return;
default:
return;
}
if (_isFutureVoid(returnType) && _hasNoReturns(body)) {
var final_body = body;
await builder.addDartFileEdit(file, (builder) {
builder.addSimpleInsertion(final_body.offset, 'async ');
builder.addSimpleInsertion(body.offset, 'async ');
});
}
case _Type.wrongReturnType:
pkg/analysis_server/lib/src/services/correction/dart/add_missing_enum_case_clauses.dart
+2 -2
View File
@@ -83,7 +83,7 @@ class AddMissingEnumCaseClauses extends ResolvedCorrectionProducer {
var singleIndent = utils.oneIndent;
var prefixString = prefix.isNotEmpty ? '$prefix.' : '';
var enumName_final = '$prefixString$enumName';
var prefixedEnumName = '$prefixString$enumName';
await builder.addDartFileEdit(file, (builder) {
builder.insertCaseClauseAtEnd(
switchKeyword: statement.switchKeyword,
@@ -110,7 +110,7 @@ class AddMissingEnumCaseClauses extends ResolvedCorrectionProducer {
// TODO(brianwilkerson): Consider inserting the names in order into the
// switch statement.
for (var constantName in unhandledEnumCases) {
addMissingCase('$enumName_final.$constantName');
addMissingCase('$prefixedEnumName.$constantName');
}
if (unhandledNullValue) {
addMissingCase('null');
pkg/analysis_server/lib/src/services/correction/dart/change_to_static_access.dart
+24 -20
View File
@@ -28,28 +28,32 @@ class ChangeToStaticAccess extends ResolvedCorrectionProducer {
@override
Future<void> compute(ChangeBuilder builder) async {
Expression? target;
Element2? invokedElement;
var identifier = node;
if (identifier is SimpleIdentifier) {
var parent = identifier.parent;
if (parent is MethodInvocation) {
if (parent.methodName == identifier) {
target = parent.target;
invokedElement = identifier.element;
}
} else if (parent is PrefixedIdentifier) {
if (parent.identifier == identifier) {
target = parent.prefix;
invokedElement = identifier.element;
}
}
}
if (target == null || invokedElement is! ExecutableElement2) {
if (identifier is! SimpleIdentifier) {
return;
}
Expression target;
var parent = identifier.parent;
if (parent case MethodInvocation(target: var parentTarget?)) {
if (parent.methodName != identifier) {
return;
}
target = parentTarget;
} else if (parent is PrefixedIdentifier) {
if (parent.identifier != identifier) {
return;
}
target = parent.prefix;
} else {
return;
}
var invokedElement = identifier.element;
if (invokedElement is! ExecutableElement2) {
return;
}
var target_final = target;
var declaringElement = invokedElement.enclosingElement2;
if (declaringElement is InterfaceElement2) {
@@ -57,7 +61,7 @@ class ChangeToStaticAccess extends ResolvedCorrectionProducer {
if (declaringElementName != null) {
_className = declaringElementName;
await builder.addDartFileEdit(file, (builder) {
builder.addReplacement(range.node(target_final), (builder) {
builder.addReplacement(range.node(target), (builder) {
builder.writeReference(declaringElement);
});
});
@@ -67,7 +71,7 @@ class ChangeToStaticAccess extends ResolvedCorrectionProducer {
if (extensionName != null) {
_className = extensionName;
await builder.addDartFileEdit(file, (builder) {
builder.addReplacement(range.node(target_final), (builder) {
builder.addReplacement(range.node(target), (builder) {
builder.writeReference(declaringElement);
});
});
pkg/analysis_server/lib/src/services/correction/dart/convert_to_expression_function_body.dart
+8 -10
View File
@@ -50,11 +50,11 @@ class ConvertToExpressionFunctionBody extends ResolvedCorrectionProducer {
if (statements.length != 1) {
return;
}
var onlyStatement = statements.first;
// prepare returned expression
Expression? returnExpression;
if (onlyStatement is ReturnStatement) {
returnExpression = onlyStatement.expression;
var onlyStatement = statements.single;
// Prepare the returned expression.
Expression returnExpression;
if (onlyStatement case ReturnStatement(:var expression?)) {
returnExpression = expression;
if (onlyStatement.returnKeyword.precedingComments != null) {
// TODO(srawlins): Include comments in fixed output.
// https://github.com/dart-lang/sdk/issues/29313
@@ -82,25 +82,23 @@ class ConvertToExpressionFunctionBody extends ResolvedCorrectionProducer {
// https://github.com/dart-lang/sdk/issues/29313
return;
}
}
if (returnExpression == null) {
} else {
return;
}
// Return expressions can be quite large, e.g. Flutter build() methods.
// Return expressions can be quite large, e.g. Flutter `build()` methods.
// It is surprising to see this Quick Assist deep in the function body.
if (selectionOffset >= returnExpression.offset) {
return;
}
var returnExpression_final = returnExpression;
await builder.addDartFileEdit(file, (builder) {
builder.addReplacement(range.node(body), (builder) {
if (body.isAsynchronous) {
builder.write('async ');
}
builder.write('=> ');
builder.write(utils.getNodeText(returnExpression_final));
builder.write(utils.getNodeText(returnExpression));
var parent = body.parent;
if (parent is! FunctionExpression ||
parent.parent is FunctionDeclaration) {
pkg/analysis_server/lib/src/services/correction/dart/convert_to_relative_import.dart
+2 -2
View File
@@ -74,10 +74,10 @@ class ConvertToRelativeImport extends ResolvedCorrectionProducer {
from: path.dirname(sourceUri.path),
);
var node_final = targetNode;
var uriNode = targetNode.uri;
await builder.addDartFileEdit(file, (builder) {
builder.addSimpleReplacement(
range.node(node_final.uri).getExpanded(-1),
range.node(uriNode).getExpanded(-1),
relativePath,
);
});
pkg/analysis_server/lib/src/services/correction/dart/remove_unused_parameter.dart
+7 -12
View File
@@ -25,23 +25,20 @@ class RemoveUnusedParameter extends ResolvedCorrectionProducer {
@override
Future<void> compute(ChangeBuilder builder) async {
// To work for the unused_parameter hint as well as the lint, we must
// allow for passing in `SimpleIdentifier`s.
// To work for the unused_element_parameter warning as well as the lint (?),
// we must allow for passing in `SimpleIdentifier`s.
var maybeParameter = node;
var maybeParameterParent = maybeParameter.parent;
if (maybeParameter is SimpleIdentifier && maybeParameterParent != null) {
maybeParameter = maybeParameterParent;
}
var parameter = maybeParameter;
if (parameter is! FormalParameter) {
if (maybeParameter is! FormalParameter) {
return;
}
var parent = parameter.parent;
if (parent is DefaultFormalParameter) {
parameter = parent;
}
var parent = maybeParameter.parent;
var parameter = parent is DefaultFormalParameter ? parent : maybeParameter;
var parameterList = parameter.parent;
if (parameterList is! FormalParameterList) {
@@ -50,7 +47,6 @@ class RemoveUnusedParameter extends ResolvedCorrectionProducer {
var parameters = parameterList.parameters;
var index = parameters.indexOf(parameter);
var parameter_final = parameter;
await builder.addDartFileEdit(file, (builder) {
if (index == 0) {
// Remove the first parameter in the list.
@@ -65,7 +61,7 @@ class RemoveUnusedParameter extends ResolvedCorrectionProducer {
);
} else {
var following = parameters[1];
if (parameter_final.isRequiredPositional &&
if (parameter.isRequiredPositional &&
!following.isRequiredPositional) {
// The parameter to be removed and the following parameter are not
// of the same kind, so there is a delimiter between them that we
@@ -85,8 +81,7 @@ class RemoveUnusedParameter extends ResolvedCorrectionProducer {
}
} else {
var preceding = parameters[index - 1];
if (preceding.isRequiredPositional &&
!parameter_final.isRequiredPositional) {
if (preceding.isRequiredPositional && !parameter.isRequiredPositional) {
// The parameter to be removed and the preceding parameter are not
// of the same kind, so there is a delimiter between them.
if (index == parameters.length - 1) {
pkg/analysis_server/lib/src/services/correction/dart/replace_null_with_closure.dart
+5 -6
View File
@@ -26,7 +26,7 @@ class ReplaceNullWithClosure extends ResolvedCorrectionProducer {
@override
Future<void> compute(ChangeBuilder builder) async {
AstNode? nodeToFix;
AstNode nodeToFix;
var parameters = const <FormalParameterElement>[];
var coveringNode = this.coveringNode;
@@ -41,18 +41,17 @@ class ReplaceNullWithClosure extends ResolvedCorrectionProducer {
}
}
nodeToFix = expression;
} else {
return;
}
} else if (coveringNode is NullLiteral) {
nodeToFix = coveringNode;
}
if (nodeToFix == null) {
} else {
return;
}
var nodeToFix_final = nodeToFix;
await builder.addDartFileEdit(file, (builder) {
builder.addReplacement(range.node(nodeToFix_final), (builder) {
builder.addReplacement(range.node(nodeToFix), (builder) {
builder.writeFormalParameters(parameters);
builder.write(' => null');
});
pkg/analysis_server/lib/src/services/correction/dart/replace_with_null_aware.dart
+6 -8
View File
@@ -45,11 +45,9 @@ class ReplaceWithNullAware extends ResolvedCorrectionProducer {
}
Future<void> _computeInChain(ChangeBuilder builder) async {
var node = coveringNode;
if (node is Expression) {
var node_final = node;
await builder.addDartFileEdit(file, (builder) {
var parent = node_final.parent;
await builder.addDartFileEdit(file, (builder) {
var node = coveringNode;
if (node case Expression(:var parent)) {
while (parent != null) {
if (parent is MethodInvocation && parent.target == node) {
var operator = parent.operator;
@@ -62,10 +60,10 @@ class ReplaceWithNullAware extends ResolvedCorrectionProducer {
break;
}
node = parent;
parent = node?.parent;
parent = node.parent;
}
});
}
}
});
}
Future<void> _computeSingle(ChangeBuilder builder) async {
pkg/analysis_server/lib/src/services/correction/dart/update_sdk_constraints.dart
+3 -5
View File
@@ -49,7 +49,7 @@ class UpdateSdkConstraints extends ResolvedCorrectionProducer {
length = spaceOffset;
}
String? newText;
String newText;
if (text == 'any') {
newText = '^$_minimumVersion';
} else if (text.startsWith('^')) {
@@ -58,14 +58,12 @@ class UpdateSdkConstraints extends ResolvedCorrectionProducer {
newText = '>=$_minimumVersion';
} else if (text.startsWith('>')) {
newText = '>=$_minimumVersion';
}
if (newText == null) {
} else {
return;
}
var newText_final = newText;
await builder.addYamlFileEdit(pubspecFile.path, (builder) {
builder.addSimpleReplacement(SourceRange(offset, length), newText_final);
builder.addSimpleReplacement(SourceRange(offset, length), newText);
});
}