diff --git a/pkg/analysis_server/lib/src/services/correction/dart/add_null_check.dart b/pkg/analysis_server/lib/src/services/correction/dart/add_null_check.dart index 2447e96dca4..99703b691bb 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/add_null_check.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/add_null_check.dart @@ -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? fixArguments; @override Future 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 _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 _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); + }); + } } diff --git a/pkg/analysis_server/test/src/services/correction/fix/add_null_check_test.dart b/pkg/analysis_server/test/src/services/correction/fix/add_null_check_test.dart index 5c8fe18abfa..8e19eb710ce 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/add_null_check_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/add_null_check_test.dart @@ -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 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 test_indexExpression() async { + await resolveTestCode(''' +void f(List? l) { + l?[0] + 1; +} +'''); + await assertHasFix(''' +void f(List? l) { + l![0] + 1; +} +''', matchFixMessage: "Replace the '?' with a '!' in the invocation"); + } + + Future test_indexExpression_notLast() async { + await resolveTestCode(''' +void f(List? l) { + l?[0].sign + 1; +} +'''); + await assertHasFix(''' +void f(List? l) { + l![0].sign + 1; +} +'''); + } + + Future test_methodInvocation() async { + await resolveTestCode(''' +void f(String? s) { + s?.toString() + ''; +} +'''); + await assertHasFix(''' +void f(String? s) { + s!.toString() + ''; +} +'''); + } + + Future test_methodInvocation_notLast() async { + await resolveTestCode(''' +void f(String? s) { + s?.toString().toString() + ''; +} +'''); + await assertHasFix(''' +void f(String? s) { + s!.toString().toString() + ''; +} +'''); + } + + Future test_propertyAccess() async { + await resolveTestCode(''' +void f(String? s) { + s?.length > 1; +} +'''); + await assertHasFix(''' +void f(String? s) { + s!.length > 1; +} +'''); + } + + Future 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 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