[analysis_server] fix AddNullCheck with null-aware operators

Fixes #50506

Change-Id: I8f7564ec7aaaf32888e9979254e450cd2633e307
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/274442
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Ahmed Ashour 2022-12-15 21:13:41 +00:00 committed by Commit Queue
parent 933d2cf4e7
commit f652c7962b
2 changed files with 172 additions and 1 deletions

View file

@ -13,10 +13,14 @@ import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/src/dart/ast/extensions.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
import 'package:analyzer_plugin/utilities/range_factory.dart';
class AddNullCheck extends CorrectionProducer {
@override
FixKind get fixKind => DartFixKind.ADD_NULL_CHECK;
FixKind fixKind = DartFixKind.ADD_NULL_CHECK;
@override
List<Object>? fixArguments;
@override
Future<void> compute(ChangeBuilder builder) async {
@ -24,9 +28,15 @@ class AddNullCheck extends CorrectionProducer {
// Don't suggest a feature that isn't supported.
return;
}
Expression? target;
final coveredNode = this.coveredNode;
var coveredNodeParent = coveredNode?.parent;
if (await _isNullAware(builder, coveredNode)) {
return;
}
if (coveredNode is SimpleIdentifier) {
if (coveredNodeParent is MethodInvocation) {
target = coveredNodeParent.realTarget;
@ -34,6 +44,10 @@ class AddNullCheck extends CorrectionProducer {
target = coveredNodeParent.prefix;
} else if (coveredNodeParent is PropertyAccess) {
target = coveredNodeParent.realTarget;
} else if (coveredNodeParent is CascadeExpression &&
await _isNullAware(
builder, coveredNodeParent.cascadeSections.first)) {
return;
} else {
target = coveredNode;
}
@ -81,6 +95,10 @@ class AddNullCheck extends CorrectionProducer {
// Adding a null check after an explicit `null` is pointless.
return;
}
if (await _isNullAware(builder, target)) {
return;
}
DartType? toType;
var parent = target.parent;
if (parent is AssignmentExpression && target == parent.rightHandSide) {
@ -160,4 +178,42 @@ class AddNullCheck extends CorrectionProducer {
});
});
}
/// Return `true` if the specified [node] or one of its parents `isNullAware`,
/// in which case [_replaceWithNullCheck] would also be called.
Future<bool> _isNullAware(ChangeBuilder builder, AstNode? node) async {
if (node is PropertyAccess) {
if (node.isNullAware) {
await _replaceWithNullCheck(builder, node.operator);
return true;
}
return await _isNullAware(builder, node.target);
} else if (node is MethodInvocation) {
var operator = node.operator;
if (operator != null && node.isNullAware) {
await _replaceWithNullCheck(builder, operator);
return true;
}
return await _isNullAware(builder, node.target);
} else if (node is IndexExpression) {
var question = node.question;
if (question != null) {
await _replaceWithNullCheck(builder, question);
return true;
}
return await _isNullAware(builder, node.target);
}
return false;
}
/// Replace the null aware [token] with the null check operator.
Future<void> _replaceWithNullCheck(ChangeBuilder builder, Token token) async {
fixKind = DartFixKind.REPLACE_WITH_NULL_AWARE;
var lexeme = token.lexeme;
var replacement = '!${lexeme.substring(1)}';
fixArguments = [lexeme, replacement];
await builder.addDartFileEdit(file, (builder) {
builder.addSimpleReplacement(range.token(token), replacement);
});
}
}

View file

@ -13,9 +13,124 @@ import 'fix_processor.dart';
void main() {
defineReflectiveSuite(() {
defineReflectiveTests(AddNullCheckTest);
defineReflectiveTests(AddNullCheckReplaceWithNullAwareTest);
});
}
@reflectiveTest
class AddNullCheckReplaceWithNullAwareTest extends FixProcessorTest {
@override
FixKind get kind => DartFixKind.REPLACE_WITH_NULL_AWARE;
Future<void> test_cascade() async {
await resolveTestCode('''
void g(int i) {}
void f(int? i) {
g(i?..sign);
}
''');
await assertHasFix('''
void g(int i) {}
void f(int? i) {
g(i!..sign);
}
''');
}
Future<void> test_indexExpression() async {
await resolveTestCode('''
void f(List<int>? l) {
l?[0] + 1;
}
''');
await assertHasFix('''
void f(List<int>? l) {
l![0] + 1;
}
''', matchFixMessage: "Replace the '?' with a '!' in the invocation");
}
Future<void> test_indexExpression_notLast() async {
await resolveTestCode('''
void f(List<int>? l) {
l?[0].sign + 1;
}
''');
await assertHasFix('''
void f(List<int>? l) {
l![0].sign + 1;
}
''');
}
Future<void> test_methodInvocation() async {
await resolveTestCode('''
void f(String? s) {
s?.toString() + '';
}
''');
await assertHasFix('''
void f(String? s) {
s!.toString() + '';
}
''');
}
Future<void> test_methodInvocation_notLast() async {
await resolveTestCode('''
void f(String? s) {
s?.toString().toString() + '';
}
''');
await assertHasFix('''
void f(String? s) {
s!.toString().toString() + '';
}
''');
}
Future<void> test_propertyAccess() async {
await resolveTestCode('''
void f(String? s) {
s?.length > 1;
}
''');
await assertHasFix('''
void f(String? s) {
s!.length > 1;
}
''');
}
Future<void> test_propertyAccess_coveredNode() async {
await resolveTestCode('''
void g(int i) {}
void f(int? i) {
g(i?.sign);
}
''');
await assertHasFix('''
void g(int i) {}
void f(int? i) {
g(i!.sign);
}
''');
}
Future<void> test_propertyAccess_notLast() async {
await resolveTestCode('''
void f(String? s) {
s?.length.sign > 1;
}
''');
await assertHasFix('''
void f(String? s) {
s!.length.sign > 1;
}
''');
}
}
@reflectiveTest
class AddNullCheckTest extends FixProcessorTest {
@override