Skip to content

Commit

Permalink
fix for prefer_expression_bodies
Browse files Browse the repository at this point in the history
Change-Id: Ie1a76db4450f1b2447e8988e84f758d623810264
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116360
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
  • Loading branch information
pq authored and commit-bot@chromium.org committed Sep 9, 2019
1 parent c2e55a2 commit 9fd672b
Show file tree
Hide file tree
Showing 9 changed files with 164 additions and 83 deletions.
4 changes: 1 addition & 3 deletions pkg/analysis_server/lib/src/services/correction/assist.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,7 @@ class DartAssistKind {
static const CONVERT_INTO_BLOCK_BODY = const AssistKind(
'dart.assist.convert.bodyToBlock', 30, "Convert to block body");
static const CONVERT_INTO_EXPRESSION_BODY = const AssistKind(
'dart.assist.convert.bodyToExpression', 30, "Convert to expression body",
// todo (pq): migrate to (conditional) fix
associatedErrorCodes: <String>['prefer_expression_function_bodies']);
'dart.assist.convert.bodyToExpression', 30, "Convert to expression body");
static const CONVERT_INTO_FINAL_FIELD = const AssistKind(
'dart.assist.convert.getterToFinalField', 30, "Convert to final field",
// todo (pq): migrate to (conditional) fix
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,11 @@ class AssistProcessor extends BaseProcessor {
await _addProposal_convertToAsyncFunctionBody();
await _addProposal_convertToBlockFunctionBody();
await _addProposal_convertToDoubleQuotedString();
await _addProposal_convertToExpressionFunctionBody();
if (!_containsErrorCode(
{LintNames.prefer_expression_function_bodies},
)) {
await _addProposal_convertToExpressionFunctionBody();
}
await _addProposal_convertToFieldParameter();
await _addProposal_convertToForIndexLoop();
await _addProposal_convertToGenericFunctionSyntax();
Expand Down Expand Up @@ -175,39 +179,6 @@ class AssistProcessor extends BaseProcessor {
return assists;
}

FunctionBody getEnclosingFunctionBody() {
// TODO(brianwilkerson) Determine whether there is a reason why this method
// isn't just "return node.getAncestor((node) => node is FunctionBody);"
{
FunctionExpression function =
node.thisOrAncestorOfType<FunctionExpression>();
if (function != null) {
return function.body;
}
}
{
FunctionDeclaration function =
node.thisOrAncestorOfType<FunctionDeclaration>();
if (function != null) {
return function.functionExpression.body;
}
}
{
ConstructorDeclaration constructor =
node.thisOrAncestorOfType<ConstructorDeclaration>();
if (constructor != null) {
return constructor.body;
}
}
{
MethodDeclaration method = node.thisOrAncestorOfType<MethodDeclaration>();
if (method != null) {
return method.body;
}
}
return null;
}

void _addAssistFromBuilder(DartChangeBuilder builder, AssistKind kind,
{List args = null}) {
if (builder == null) {
Expand Down Expand Up @@ -894,52 +865,7 @@ class AssistProcessor extends BaseProcessor {
}

Future<void> _addProposal_convertToExpressionFunctionBody() async {
// prepare current body
FunctionBody body = getEnclosingFunctionBody();
if (body is! BlockFunctionBody || body.isGenerator) {
_coverageMarker();
return;
}
// prepare return statement
List<Statement> statements = (body as BlockFunctionBody).block.statements;
if (statements.length != 1) {
_coverageMarker();
return;
}
Statement onlyStatement = statements.first;
// prepare returned expression
Expression returnExpression;
if (onlyStatement is ReturnStatement) {
returnExpression = onlyStatement.expression;
} else if (onlyStatement is ExpressionStatement) {
returnExpression = onlyStatement.expression;
}
if (returnExpression == null) {
_coverageMarker();
return;
}

// 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) {
_coverageMarker();
return;
}

var changeBuilder = _newDartChangeBuilder();
await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) {
builder.addReplacement(range.node(body), (DartEditBuilder builder) {
if (body.isAsynchronous) {
builder.write('async ');
}
builder.write('=> ');
builder.write(_getNodeText(returnExpression));
if (body.parent is! FunctionExpression ||
body.parent.parent is FunctionDeclaration) {
builder.write(';');
}
});
});
final changeBuilder = await createBuilder_convertToExpressionFunctionBody();
_addAssistFromBuilder(
changeBuilder, DartAssistKind.CONVERT_INTO_EXPRESSION_BODY);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,93 @@ abstract class BaseProcessor {
return validChange ? changeBuilder : null;
}

Future<ChangeBuilder> createBuilder_convertToExpressionFunctionBody() async {
// prepare current body
FunctionBody body = getEnclosingFunctionBody();
if (body is! BlockFunctionBody || body.isGenerator) {
_coverageMarker();
return null;
}
// prepare return statement
List<Statement> statements = (body as BlockFunctionBody).block.statements;
if (statements.length != 1) {
_coverageMarker();
return null;
}
Statement onlyStatement = statements.first;
// prepare returned expression
Expression returnExpression;
if (onlyStatement is ReturnStatement) {
returnExpression = onlyStatement.expression;
} else if (onlyStatement is ExpressionStatement) {
returnExpression = onlyStatement.expression;
}
if (returnExpression == null) {
_coverageMarker();
return null;
}

// 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) {
_coverageMarker();
return null;
}

var changeBuilder = _newDartChangeBuilder();
await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) {
builder.addReplacement(range.node(body), (DartEditBuilder builder) {
if (body.isAsynchronous) {
builder.write('async ');
}
builder.write('=> ');
builder.write(_getNodeText(returnExpression));
if (body.parent is! FunctionExpression ||
body.parent.parent is FunctionDeclaration) {
builder.write(';');
}
});
});
return changeBuilder;
}

/// Returns the text of the given node in the unit.
String /* TODO (pq): make visible */ _getNodeText(AstNode node) =>
utils.getNodeText(node);

FunctionBody getEnclosingFunctionBody() {
// TODO(brianwilkerson) Determine whether there is a reason why this method
// isn't just "return node.getAncestor((node) => node is FunctionBody);"
{
FunctionExpression function =
node.thisOrAncestorOfType<FunctionExpression>();
if (function != null) {
return function.body;
}
}
{
FunctionDeclaration function =
node.thisOrAncestorOfType<FunctionDeclaration>();
if (function != null) {
return function.functionExpression.body;
}
}
{
ConstructorDeclaration constructor =
node.thisOrAncestorOfType<ConstructorDeclaration>();
if (constructor != null) {
return constructor.body;
}
}
{
MethodDeclaration method = node.thisOrAncestorOfType<MethodDeclaration>();
if (method != null) {
return method.body;
}
}
return null;
}

Future<ChangeBuilder>
createBuilder_addTypeAnnotation_VariableDeclaration() async {
AstNode node = this.node;
Expand Down
2 changes: 2 additions & 0 deletions pkg/analysis_server/lib/src/services/correction/fix.dart
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ class DartFixKind {
const FixKind('CONVERT_FLUTTER_CHILD', 50, "Convert to children:");
static const CONVERT_FLUTTER_CHILDREN =
const FixKind('CONVERT_FLUTTER_CHILDREN', 50, "Convert to child:");
static const CONVERT_INTO_EXPRESSION_BODY = const FixKind(
'CONVERT_INTO_EXPRESSION_BODY', 50, "Convert to expression body");
static const CONVERT_TO_FOR_ELEMENT =
const FixKind('CONVERT_TO_FOR_ELEMENT', 50, "Convert to a 'for' element");
static const CONVERT_TO_IF_ELEMENT =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,9 @@ class FixProcessor extends BaseProcessor {
if (errorCode.name == LintNames.prefer_const_declarations) {
await _addFix_replaceFinalWithConst();
}
if (errorCode.name == LintNames.prefer_expression_function_bodies) {
await _addFix_convertToExpressionBody();
}
if (errorCode.name == LintNames.prefer_for_elements_to_map_fromIterable) {
await _addFix_convertMapFromIterableToForLiteral();
}
Expand Down Expand Up @@ -1357,6 +1360,11 @@ class FixProcessor extends BaseProcessor {
_addFixFromBuilder(changeBuilder, DartFixKind.CONVERT_TO_FOR_ELEMENT);
}

Future<void> _addFix_convertToExpressionBody() async {
final changeBuilder = await createBuilder_convertToExpressionFunctionBody();
_addFixFromBuilder(changeBuilder, DartFixKind.CONVERT_INTO_EXPRESSION_BODY);
}

Future<void> _addFix_convertToIntLiteral() async {
final changeBuilder = await createBuilder_convertToIntLiteral();
_addFixFromBuilder(changeBuilder, DartFixKind.CONVERT_TO_INT_LITERAL);
Expand Down
2 changes: 2 additions & 0 deletions pkg/analysis_server/lib/src/services/linter/lint_names.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class LintNames {
static const String prefer_const_declarations = 'prefer_const_declarations';
static const String prefer_equal_for_default_values =
'prefer_equal_for_default_values';
static const String prefer_expression_function_bodies =
'prefer_expression_function_bodies';
static const String prefer_final_fields = 'prefer_final_fields';
static const String prefer_final_locals = 'prefer_final_locals';
static const String prefer_for_elements_to_map_fromIterable =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// BSD-style license that can be found in the LICENSE file.

import 'package:analysis_server/src/services/correction/assist.dart';
import 'package:analysis_server/src/services/linter/lint_names.dart';
import 'package:analyzer_plugin/utilities/assist/assist.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

Expand Down Expand Up @@ -41,6 +42,20 @@ class A {
''');
}

test_async_noAssistWithLint() async {
createAnalysisOptionsFile(
lints: [LintNames.prefer_expression_function_bodies]);
verifyNoTestUnitErrors = false;
await resolveTestUnit('''
class A {
mmm() async {
return 42;
}
}
''');
await assertNoAssist();
}

test_closure() async {
await resolveTestUnit('''
setup(x) {}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:analysis_server/src/services/correction/fix.dart';
import 'package:analysis_server/src/services/linter/lint_names.dart';
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

import 'fix_processor.dart';

main() {
defineReflectiveSuite(() {
defineReflectiveTests(ConvertIntoExpressionBodyTest);
});
}

@reflectiveTest
class ConvertIntoExpressionBodyTest extends FixProcessorLintTest {
@override
FixKind get kind => DartFixKind.CONVERT_INTO_EXPRESSION_BODY;

@override
String get lintCode => LintNames.prefer_expression_function_bodies;

/// More coverage in the `convert_into_expression_body_test.dart` assist test.
test_async() async {
await resolveTestUnit('''
class A {
mmm() async {
return 42; /*LINT*/
}
}
''');
await assertHasFix('''
class A {
mmm() async => 42;
}
''');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import 'convert_documentation_into_line_test.dart'
as convert_documentation_into_line;
import 'convert_flutter_child_test.dart' as convert_flutter_child;
import 'convert_flutter_children_test.dart' as convert_flutter_children;
import 'convert_into_expression_body_test.dart' as convert_into_expression_body;
import 'convert_to_for_element_test.dart' as convert_to_for_element;
import 'convert_to_if_element_test.dart' as convert_to_if_element;
import 'convert_to_int_literal_test.dart' as convert_to_int_literal;
Expand Down Expand Up @@ -148,6 +149,7 @@ main() {
convert_documentation_into_line.main();
convert_flutter_child.main();
convert_flutter_children.main();
convert_into_expression_body.main();
convert_to_for_element.main();
convert_to_if_element.main();
convert_to_int_literal.main();
Expand Down

0 comments on commit 9fd672b

Please sign in to comment.