Skip to content

Commit

Permalink
fix for convert_to_null_aware_operators
Browse files Browse the repository at this point in the history 
Change-Id: I3d256ab5d474b91796226aba08058250dc2451c2
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116362
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
  • Loading branch information
@pq
pq authored and commit-bot@chromium.org committed Sep 9, 2019
1 parent 0851b9a commit 27f803d
Show file tree
Hide file tree
Showing 9 changed files with 166 additions and 93 deletions.
8 changes: 2 additions & 6 deletions pkg/analysis_server/lib/src/services/correction/assist.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ class DartAssistKind {
static const CONVERT_INTO_EXPRESSION_BODY = const AssistKind(
'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
associatedErrorCodes: <String>['prefer_final_fields']);
'dart.assist.convert.getterToFinalField', 30, "Convert to final field");
static const CONVERT_INTO_FOR_INDEX = const AssistKind(
'dart.assist.convert.forEachToForIndex', 30, "Convert to for-index loop");
static const CONVERT_INTO_GENERIC_FUNCTION_SYNTAX = const AssistKind(
Expand Down Expand Up @@ -102,9 +100,7 @@ class DartAssistKind {
30,
"Convert to normal parameter");
static const CONVERT_TO_NULL_AWARE = const AssistKind(
'dart.assist.convert.toNullAware', 30, "Convert to use '?.'",
// todo (pq): migrate to (conditional) fix
associatedErrorCodes: <String>['prefer_null_aware_operators']);
'dart.assist.convert.toNullAware', 30, "Convert to use '?.'");
static const CONVERT_TO_PACKAGE_IMPORT = const AssistKind(
'dart.assist.convert.relativeToPackageImport',
30,
Expand Down
94 changes: 7 additions & 87 deletions pkg/analysis_server/lib/src/services/correction/assist_internal.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,11 @@ class AssistProcessor extends BaseProcessor {
await _addProposal_convertToIsNotEmpty();
await _addProposal_convertToMultilineString();
await _addProposal_convertToNormalParameter();
await _addProposal_convertToNullAware();
if (!_containsErrorCode(
{LintNames.prefer_null_aware_operators},
)) {
await _addProposal_convertToNullAware();
}
await _addProposal_convertToPackageImport();
await _addProposal_convertToSingleQuotedString();
await _addProposal_encapsulateField();
Expand Down Expand Up @@ -1301,92 +1305,8 @@ class AssistProcessor extends BaseProcessor {
}

Future<void> _addProposal_convertToNullAware() async {
AstNode node = this.node;
if (node is! ConditionalExpression) {
_coverageMarker();
return;
}
ConditionalExpression conditional = node;
Expression condition = conditional.condition.unParenthesized;
SimpleIdentifier identifier;
Expression nullExpression;
Expression nonNullExpression;
int periodOffset;

if (condition is BinaryExpression) {
//
// Identify the variable being compared to `null`, or return if the
// condition isn't a simple comparison of `null` to a variable's value.
//
Expression leftOperand = condition.leftOperand;
Expression rightOperand = condition.rightOperand;
if (leftOperand is NullLiteral && rightOperand is SimpleIdentifier) {
identifier = rightOperand;
} else if (rightOperand is NullLiteral &&
leftOperand is SimpleIdentifier) {
identifier = leftOperand;
} else {
_coverageMarker();
return;
}
if (identifier.staticElement is! LocalElement) {
_coverageMarker();
return;
}
//
// Identify the expression executed when the variable is `null` and when
// it is non-`null`. Return if the `null` expression isn't a null literal
// or if the non-`null` expression isn't a method invocation whose target
// is the save variable being compared to `null`.
//
if (condition.operator.type == TokenType.EQ_EQ) {
nullExpression = conditional.thenExpression;
nonNullExpression = conditional.elseExpression;
} else if (condition.operator.type == TokenType.BANG_EQ) {
nonNullExpression = conditional.thenExpression;
nullExpression = conditional.elseExpression;
}
if (nullExpression == null || nonNullExpression == null) {
_coverageMarker();
return;
}
if (nullExpression.unParenthesized is! NullLiteral) {
_coverageMarker();
return;
}
Expression unwrappedExpression = nonNullExpression.unParenthesized;
Expression target;
Token operator;
if (unwrappedExpression is MethodInvocation) {
target = unwrappedExpression.target;
operator = unwrappedExpression.operator;
} else if (unwrappedExpression is PrefixedIdentifier) {
target = unwrappedExpression.prefix;
operator = unwrappedExpression.period;
} else {
_coverageMarker();
return;
}
if (operator.type != TokenType.PERIOD) {
_coverageMarker();
return;
}
if (!(target is SimpleIdentifier &&
target.staticElement == identifier.staticElement)) {
_coverageMarker();
return;
}
periodOffset = operator.offset;

DartChangeBuilder changeBuilder = _newDartChangeBuilder();
await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) {
builder.addDeletion(range.startStart(node, nonNullExpression));
builder.addSimpleInsertion(periodOffset, '?');
builder.addDeletion(range.endEnd(nonNullExpression, node));
});
_addAssistFromBuilder(
changeBuilder, DartAssistKind.CONVERT_TO_NULL_AWARE);
}
final changeBuilder = await createBuilder_convertToNullAware();
_addAssistFromBuilder(changeBuilder, DartAssistKind.CONVERT_TO_NULL_AWARE);
}

Future<void> _addProposal_convertToPackageImport() async {
Expand Down
89 changes: 89 additions & 0 deletions pkg/analysis_server/lib/src/services/correction/base_processor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,95 @@ abstract class BaseProcessor {
return validChange ? changeBuilder : null;
}

Future<ChangeBuilder> createBuilder_convertToNullAware() async {
AstNode node = this.node;
if (node is! ConditionalExpression) {
_coverageMarker();
return null;
}
ConditionalExpression conditional = node;
Expression condition = conditional.condition.unParenthesized;
SimpleIdentifier identifier;
Expression nullExpression;
Expression nonNullExpression;
int periodOffset;

if (condition is BinaryExpression) {
//
// Identify the variable being compared to `null`, or return if the
// condition isn't a simple comparison of `null` to a variable's value.
//
Expression leftOperand = condition.leftOperand;
Expression rightOperand = condition.rightOperand;
if (leftOperand is NullLiteral && rightOperand is SimpleIdentifier) {
identifier = rightOperand;
} else if (rightOperand is NullLiteral &&
leftOperand is SimpleIdentifier) {
identifier = leftOperand;
} else {
_coverageMarker();
return null;
}
if (identifier.staticElement is! LocalElement) {
_coverageMarker();
return null;
}
//
// Identify the expression executed when the variable is `null` and when
// it is non-`null`. Return if the `null` expression isn't a null literal
// or if the non-`null` expression isn't a method invocation whose target
// is the save variable being compared to `null`.
//
if (condition.operator.type == TokenType.EQ_EQ) {
nullExpression = conditional.thenExpression;
nonNullExpression = conditional.elseExpression;
} else if (condition.operator.type == TokenType.BANG_EQ) {
nonNullExpression = conditional.thenExpression;
nullExpression = conditional.elseExpression;
}
if (nullExpression == null || nonNullExpression == null) {
_coverageMarker();
return null;
}
if (nullExpression.unParenthesized is! NullLiteral) {
_coverageMarker();
return null;
}
Expression unwrappedExpression = nonNullExpression.unParenthesized;
Expression target;
Token operator;
if (unwrappedExpression is MethodInvocation) {
target = unwrappedExpression.target;
operator = unwrappedExpression.operator;
} else if (unwrappedExpression is PrefixedIdentifier) {
target = unwrappedExpression.prefix;
operator = unwrappedExpression.period;
} else {
_coverageMarker();
return null;
}
if (operator.type != TokenType.PERIOD) {
_coverageMarker();
return null;
}
if (!(target is SimpleIdentifier &&
target.staticElement == identifier.staticElement)) {
_coverageMarker();
return null;
}
periodOffset = operator.offset;

DartChangeBuilder changeBuilder = _newDartChangeBuilder();
await changeBuilder.addFileEdit(file, (DartFileEditBuilder builder) {
builder.addDeletion(range.startStart(node, nonNullExpression));
builder.addSimpleInsertion(periodOffset, '?');
builder.addDeletion(range.endEnd(nonNullExpression, node));
});
return changeBuilder;
}
return null;
}

Future<ChangeBuilder> createBuilder_convertToExpressionFunctionBody() async {
// prepare current body
FunctionBody body = getEnclosingFunctionBody();
Expand Down
2 changes: 2 additions & 0 deletions pkg/analysis_server/lib/src/services/correction/fix.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ class DartFixKind {
'CONVERT_TO_LINE_COMMENT', 50, "Convert to line documentation comment");
static const CONVERT_TO_NAMED_ARGUMENTS = const FixKind(
'CONVERT_TO_NAMED_ARGUMENTS', 50, "Convert to named arguments");
static const CONVERT_TO_NULL_AWARE =
const FixKind('CONVERT_TO_NULL_AWARE', 50, "Convert to use '?.'");
static const CREATE_CLASS =
const FixKind('CREATE_CLASS', 50, "Create class '{0}'");
static const CREATE_CONSTRUCTOR =
Expand Down
8 changes: 8 additions & 0 deletions pkg/analysis_server/lib/src/services/correction/fix_internal.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,9 @@ class FixProcessor extends BaseProcessor {
await _addFix_addConst();
await _addFix_replaceNewWithConst();
}
if (name == LintNames.prefer_null_aware_operators) {
await _addFix_convertToNullAware();
}
if (errorCode.name == LintNames.slash_for_doc_comments) {
await _addFix_convertDocumentationIntoLine();
}
Expand Down Expand Up @@ -735,6 +738,11 @@ class FixProcessor extends BaseProcessor {
_addFixFromBuilder(changeBuilder, DartFixKind.ADD_CURLY_BRACES);
}

Future<void> _addFix_convertToNullAware() async {
final changeBuilder = await createBuilder_convertToNullAware();
_addFixFromBuilder(changeBuilder, DartFixKind.CONVERT_TO_NULL_AWARE);
}

Future<void> _addFix_addExplicitCast() async {
if (coveredNode is! Expression) {
return;
Expand Down
2 changes: 2 additions & 0 deletions pkg/analysis_server/lib/src/services/linter/lint_names.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class LintNames {
'prefer_if_elements_to_conditional_expressions';
static const String prefer_is_empty = 'prefer_is_empty';
static const String prefer_is_not_empty = 'prefer_is_not_empty';
static const String prefer_null_aware_operators =
'prefer_null_aware_operators';
static const String slash_for_doc_comments = 'slash_for_doc_comments';
static const String type_annotate_public_apis = 'type_annotate_public_apis';
static const String type_init_formals = 'type_init_formals';
Expand Down
13 changes: 13 additions & 0 deletions pkg/analysis_server/test/src/services/correction/assist/convert_to_null_aware_test.dart
View file
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 @@ -95,6 +96,18 @@ int f(A a) => a?.m();
''');
}

test_equal_nullOnLeft_noAssistWithLint() async {
createAnalysisOptionsFile(lints: [LintNames.prefer_null_aware_operators]);
verifyNoTestUnitErrors = false;
await resolveTestUnit('''
abstract class A {
int m();
}
int f(A a) => null == a ? null : a.m();
''');
await assertNoAssist();
}

test_equal_nullOnRight() async {
await resolveTestUnit('''
abstract class A {
Expand Down
41 changes: 41 additions & 0 deletions pkg/analysis_server/test/src/services/correction/fix/convert_to_null_aware_test.dart
View file
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(ConvertToNullAwareTest);
});
}

@reflectiveTest
class ConvertToNullAwareTest extends FixProcessorLintTest {
@override
FixKind get kind => DartFixKind.CONVERT_TO_NULL_AWARE;

@override
String get lintCode => LintNames.prefer_null_aware_operators;

/// More coverage in the `convert_to_null_aware_test.dart` assist test.
test_equal_nullOnLeft() async {
await resolveTestUnit('''
abstract class A {
int m();
}
int f(A a) => null == a /*LINT*/? null : a.m();
''');
await assertHasFix('''
abstract class A {
int m();
}
int f(A a) => a?.m();
''');
}
}
2 changes: 2 additions & 0 deletions pkg/analysis_server/test/src/services/correction/fix/test_all.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ 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;
import 'convert_to_named_arguments_test.dart' as convert_to_named_arguments;
import 'convert_to_null_aware_test.dart' as convert_to_null_aware;
import 'create_class_test.dart' as create_class;
import 'create_constructor_for_final_fields_test.dart'
as create_constructor_for_final_field;
Expand Down Expand Up @@ -154,6 +155,7 @@ main() {
convert_to_if_element.main();
convert_to_int_literal.main();
convert_to_named_arguments.main();
convert_to_null_aware.main();
create_class.main();
create_constructor_for_final_field.main();
create_constructor_super.main();
Expand Down

0 comments on commit 27f803d

Please sign in to comment.