From aeadcbbca81f23b23a92c35592ead6e681aa79ca Mon Sep 17 00:00:00 2001 From: FMorschel Date: Mon, 16 Feb 2026 23:32:59 -0800 Subject: [PATCH] [linter, DAS] Makes `use_null_aware_elements` to report on cascade elements Fixes: https://github.com/dart-lang/sdk/issues/62660 Change-Id: I8daee991353cae128ea84cf8d71ee430a884f550 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/480560 Reviewed-by: Nate Biggs Auto-Submit: Felipe Morschel Reviewed-by: Brian Wilkerson Commit-Queue: Nate Biggs --- ..._check_to_null_aware_element_or_entry.dart | 20 +-- .../correction/dart/import_library.dart | 5 +- ...k_to_null_aware_element_or_entry_test.dart | 125 ++++++++++++++++++ pkg/dartdev/lib/src/templates.dart | 2 +- pkg/dev_compiler/lib/src/kernel/compiler.dart | 2 +- .../lib/src/kernel/compiler_new.dart | 2 +- .../kernel/expression_compiler_worker.dart | 2 +- .../src/rules/use_null_aware_elements.dart | 17 ++- .../rules/use_null_aware_elements_test.dart | 64 ++++++++- 9 files changed, 218 insertions(+), 21 deletions(-) diff --git a/pkg/analysis_server/lib/src/services/correction/dart/convert_null_check_to_null_aware_element_or_entry.dart b/pkg/analysis_server/lib/src/services/correction/dart/convert_null_check_to_null_aware_element_or_entry.dart index 16e829c0f33..7d52c7e66ec 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/convert_null_check_to_null_aware_element_or_entry.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/convert_null_check_to_null_aware_element_or_entry.dart @@ -36,7 +36,9 @@ class ConvertNullCheckToNullAwareElementOrEntry )) { if (node.caseClause == null) { // An element or entry of the form `if (x != null) ...`. - if (thenElement is SimpleIdentifier) { + if (thenElement + case SpreadElement(expression: SimpleIdentifier element) || + SimpleIdentifier element) { // In case of a list or set element with a promotable target, we // simply replace the entire element with the then-element prefixed by // '?'. @@ -44,11 +46,13 @@ class ConvertNullCheckToNullAwareElementOrEntry // `if (x != null) x` is rewritten as `?x` await builder.addDartFileEdit(file, (builder) { builder.addSimpleReplacement( - range.startStart(node, thenElement), - '?', + range.startStart(node, element), + thenElement is SpreadElement ? '...?' : '?', ); }); - } else if (thenElement is PostfixExpression) { + } else if (thenElement + case SpreadElement(expression: PostfixExpression element) || + PostfixExpression element) { // In case of a list or set element with a getter target, we replace // the entire element with the then-element target identifier prefixed // by '?'. Note that in the case of a getter target, the null-check @@ -57,10 +61,10 @@ class ConvertNullCheckToNullAwareElementOrEntry // `if (x != null) x!` is rewritten as `?x` await builder.addDartFileEdit(file, (builder) { builder.addSimpleReplacement( - range.startStart(node, thenElement), - '?', + range.startStart(node, element), + thenElement is SpreadElement ? '...?' : '?', ); - builder.addDeletion(range.endEnd(thenElement.operand, thenElement)); + builder.addDeletion(range.endEnd(element.operand, element)); }); } else if (thenElement is MapLiteralEntry) { // In case of a map entry we need to check if it's the key that's @@ -146,7 +150,7 @@ class ConvertNullCheckToNullAwareElementOrEntry await builder.addDartFileEdit(file, (builder) { builder.addSimpleReplacement( range.startStart(node, condition), - '?', + thenElement is SpreadElement ? '...?' : '?', ); builder.addDeletion(range.endEnd(condition, node)); }); diff --git a/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart b/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart index 83e77ee30db..392836ce69f 100644 --- a/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart +++ b/pkg/analysis_server/lib/src/services/correction/dart/import_library.dart @@ -74,10 +74,7 @@ class ImportLibrary extends MultiCorrectionProducer { if (names.isEmpty) { return const []; } - return [ - for (var name in names) - if (await name.producers case var producers?) ...producers, - ]; + return [for (var name in names) ...?(await name.producers)]; } /// A map of all the diagnostic codes that this fix can be applied to and the diff --git a/pkg/analysis_server/test/src/services/correction/fix/convert_null_check_to_null_aware_element_or_entry_test.dart b/pkg/analysis_server/test/src/services/correction/fix/convert_null_check_to_null_aware_element_or_entry_test.dart index d04ad67f400..c9fdd538575 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/convert_null_check_to_null_aware_element_or_entry_test.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/convert_null_check_to_null_aware_element_or_entry_test.dart @@ -390,6 +390,131 @@ Set f(int? x) { ?x, }; } +'''); + } + + Future test_spread_case_getter_list() async { + await resolveTestCode(''' +List? get x => null; +List f() => [ + if (x case var y?) ...y, +]; +'''); + await assertHasFix(''' +List? get x => null; +List f() => [ + ...?x, +]; +'''); + } + + Future test_spread_case_getter_map() async { + await resolveTestCode(''' +Map? get x => null; +Map f() => { + if (x case var y?) ...y, +}; +'''); + await assertHasFix(''' +Map? get x => null; +Map f() => { + ...?x, +}; +'''); + } + + Future test_spread_case_list() async { + await resolveTestCode(''' +List f(List? x) => [ + if (x case var y?) ...y, +]; +'''); + await assertHasFix(''' +List f(List? x) => [ + ...?x, +]; +'''); + } + + Future test_spread_case_map() async { + await resolveTestCode(''' +Map f(Map? x) => { + if (x case var y?) ...y, +}; +'''); + await assertHasFix(''' +Map f(Map? x) => { + ...?x, +}; +'''); + } + + Future test_spread_getter_list() async { + await resolveTestCode(''' +List? get x => null; +List f() => [ + if (x != null) ...x!, +]; +'''); + await assertHasFix(''' +List? get x => null; +List f() => [ + ...?x, +]; +'''); + } + + Future test_spread_getter_map() async { + await resolveTestCode(''' +Map? get x => null; +Map f() => { + if (x != null) ...x!, +}; +'''); + await assertHasFix(''' +Map? get x => null; +Map f() => { + ...?x, +}; +'''); + } + + Future test_spread_list() async { + await resolveTestCode(''' +List f(List? x) => [ + if (x != null) ...x, +]; +'''); + await assertHasFix(''' +List f(List? x) => [ + ...?x, +]; +'''); + } + + Future test_spread_map() async { + await resolveTestCode(''' +Map f(Map? x) => { + if (x != null) ...x, +}; +'''); + await assertHasFix(''' +Map f(Map? x) => { + ...?x, +}; +'''); + } + + Future test_spread_set() async { + await resolveTestCode(''' +Set f(List? x) => { + if (x != null) ...x, +}; +'''); + await assertHasFix(''' +Set f(List? x) => { + ...?x, +}; '''); } } diff --git a/pkg/dartdev/lib/src/templates.dart b/pkg/dartdev/lib/src/templates.dart index 52e5ccc58c4..0961feb253e 100644 --- a/pkg/dartdev/lib/src/templates.dart +++ b/pkg/dartdev/lib/src/templates.dart @@ -100,7 +100,7 @@ abstract class Generator implements Comparable { 'description': description, 'year': DateTime.now().year.toString(), 'author': '', - if (additionalVars != null) ...additionalVars, + ...?additionalVars, }; for (TemplateFile file in files) { diff --git a/pkg/dev_compiler/lib/src/kernel/compiler.dart b/pkg/dev_compiler/lib/src/kernel/compiler.dart index 51dac497455..112b76ca4c0 100644 --- a/pkg/dev_compiler/lib/src/kernel/compiler.dart +++ b/pkg/dev_compiler/lib/src/kernel/compiler.dart @@ -7139,7 +7139,7 @@ class ProgramCompiler extends ComputeOnceConstantVisitor ); return [ if (types && typeArguments != null) ...typeArguments, - if (positionalArguments != null) ...positionalArguments, + ...?positionalArguments, if (namedArguments != null) js_ast.ObjectInitializer([...namedArguments]), ]; } diff --git a/pkg/dev_compiler/lib/src/kernel/compiler_new.dart b/pkg/dev_compiler/lib/src/kernel/compiler_new.dart index 3564543dabd..37543bd5590 100644 --- a/pkg/dev_compiler/lib/src/kernel/compiler_new.dart +++ b/pkg/dev_compiler/lib/src/kernel/compiler_new.dart @@ -8010,7 +8010,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor ); return [ if (types && typeArguments != null) ...typeArguments, - if (positionalArguments != null) ...positionalArguments, + ...?positionalArguments, if (namedArguments != null) js_ast.ObjectInitializer([...namedArguments]), ]; } diff --git a/pkg/dev_compiler/lib/src/kernel/expression_compiler_worker.dart b/pkg/dev_compiler/lib/src/kernel/expression_compiler_worker.dart index 92318bed026..12edea14ce8 100644 --- a/pkg/dev_compiler/lib/src/kernel/expression_compiler_worker.dart +++ b/pkg/dev_compiler/lib/src/kernel/expression_compiler_worker.dart @@ -249,7 +249,7 @@ class ExpressionCompilerWorker { ..fileSystem = fileSystem ..omitPlatform = true ..environmentDefines = addGeneratedVariables({ - if (environmentDefines != null) ...environmentDefines, + ...?environmentDefines, }, enableAsserts: enableAsserts) ..explicitExperimentalFlags = explicitExperimentalFlags ..onDiagnostic = _onDiagnosticHandler(errors, warnings, infos) diff --git a/pkg/linter/lib/src/rules/use_null_aware_elements.dart b/pkg/linter/lib/src/rules/use_null_aware_elements.dart index b0df7fe473e..75e01f11751 100644 --- a/pkg/linter/lib/src/rules/use_null_aware_elements.dart +++ b/pkg/linter/lib/src/rules/use_null_aware_elements.dart @@ -75,7 +75,10 @@ class _Visitor extends SimpleAstVisitor { // {..., if (x != null) x, ...} case ( PromotableElementImpl(), - SimpleIdentifier(canonicalElement: var reference), + SimpleIdentifier(canonicalElement: var reference) || + SpreadElement( + expression: SimpleIdentifier(canonicalElement: var reference), + ), ): // List and set elements with getters: // @@ -84,9 +87,15 @@ class _Visitor extends SimpleAstVisitor { case ( GetterElement(), PostfixExpression( - operand: SimpleIdentifier(canonicalElement: var reference), - operator: Token(lexeme: '!'), - ), + operand: SimpleIdentifier(canonicalElement: var reference), + operator: Token(lexeme: '!'), + ) || + SpreadElement( + expression: PostfixExpression( + operand: SimpleIdentifier(canonicalElement: var reference), + operator: Token(lexeme: '!'), + ), + ), ): if (nullCheckTarget == reference) { rule.reportAtToken(node.ifKeyword); diff --git a/pkg/linter/test/rules/use_null_aware_elements_test.dart b/pkg/linter/test/rules/use_null_aware_elements_test.dart index 33b065ad6ec..e2c3f1170b9 100644 --- a/pkg/linter/test/rules/use_null_aware_elements_test.dart +++ b/pkg/linter/test/rules/use_null_aware_elements_test.dart @@ -16,7 +16,7 @@ void main() { @reflectiveTest class UseNullAwareElementsTest extends LintRuleTest { @override - String get lintRule => 'use_null_aware_elements'; + String get lintRule => LintNames.use_null_aware_elements; test_nonPromotable_nullCheck_list() async { await assertDiagnostics( @@ -168,4 +168,66 @@ List f(int? x) { } '''); } + + test_spread_getter_list() async { + await assertDiagnostics( + ''' +List? get x => null; +List f() => [ + if (x != null) ...x!, + if (x case var y?) ...y, +]; +''', + [lint(47, 2), lint(71, 2)], + ); + } + + test_spread_getter_map() async { + await assertDiagnostics( + ''' +Map? get x => null; +Map f() => { + if (x != null) ...x!, + if (x case var y?) ...y, +}; +''', + [lint(55, 2), lint(79, 2)], + ); + } + + test_spread_list() async { + await assertDiagnostics( + ''' +List f(List? x) => [ + if (x != null) ...x, + if (x case var y?) ...y, +]; +''', + [lint(33, 2), lint(56, 2)], + ); + } + + test_spread_map() async { + await assertDiagnostics( + ''' +Map f(Map? x) => { + if (x != null) ...x, + if (x case var y?) ...y, +}; +''', + [lint(41, 2), lint(64, 2)], + ); + } + + test_spread_set() async { + await assertDiagnostics( + ''' +Set f(List? x) => { + if (x != null) ...x, + if (x case var y?) ...y, +}; +''', + [lint(32, 2), lint(55, 2)], + ); + } }