Unnecessary null aware spread hint

Change-Id: I33645c0bf948c5a3c3b06a5c81a7b77b82fe191c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105734
Commit-Queue: Mike Fairhurst <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>

Author: MichaelRFairhurst

pkg/analyzer/lib/error/error.dart

@@ -687,13 +687,14 @@ const List<ErrorCode> errorCodeValues = const [
   StaticWarningCode.TYPE_PARAMETER_REFERENCED_BY_STATIC,
   StaticWarningCode.TYPE_TEST_WITH_NON_TYPE,
   StaticWarningCode.TYPE_TEST_WITH_UNDEFINED_NAME,
+  StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE,
   StaticWarningCode.UNDEFINED_CLASS,
   StaticWarningCode.UNDEFINED_CLASS_BOOLEAN,
   StaticWarningCode.UNDEFINED_IDENTIFIER,
   StaticWarningCode.UNDEFINED_IDENTIFIER_AWAIT,
   StaticWarningCode.UNDEFINED_NAMED_PARAMETER,
+  StaticWarningCode.UNNECESSARY_NULL_AWARE_SPREAD,
   StaticWarningCode.USE_OF_VOID_RESULT,
-  StaticWarningCode.UNCHECKED_USE_OF_NULLABLE_VALUE,
   StrongModeCode.ASSIGNMENT_CAST,
   StrongModeCode.COULD_NOT_INFER,
   StrongModeCode.DOWN_CAST_COMPOSITE,

pkg/analyzer/lib/src/dart/error/hint_codes.dart

@@ -802,6 +802,7 @@ class HintCode extends ErrorCode {
   static const HintCode UNNECESSARY_NO_SUCH_METHOD = const HintCode(
       'UNNECESSARY_NO_SUCH_METHOD', "Unnecessary 'noSuchMethod' declaration.",
       correction: "Try removing the declaration of 'noSuchMethod'.");
+
   /**
    * When the '?.' operator is used on a target that we know to be non-null,
    * it is unnecessary.

pkg/analyzer/lib/src/error/codes.dart

@@ -4904,6 +4904,15 @@ class StaticWarningCode extends ErrorCode {
           correction:
               "Try casting or check the value is not null before using it.");
 
+  /**
+   * When the '...?' operator is used on a value that we know to be non-null,
+   * it is unnecessary.
+   */
+  static const StaticWarningCode UNNECESSARY_NULL_AWARE_SPREAD =
+      const StaticWarningCode('UNNECESSARY_NULL_AWARE_SPREAD',
+          "The target expression cannot be null, and so '?' is not necessary.",
+          correction: "Replace the '...?' with a '...' in the spread.");
+
   /**
    * It is a static warning to assign void to any non-void type in dart.
    * compile-time error). Report that error specially for a better user

pkg/analyzer/lib/src/generated/error_verifier.dart

@@ -1303,6 +1303,8 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
   void visitSpreadElement(SpreadElement node) {
     if (node.spreadOperator.type != TokenType.PERIOD_PERIOD_PERIOD_QUESTION) {
       _checkForNullableDereference(node.expression);
+    } else {
+      _checkForUnnecessaryNullAware(node.expression, node.spreadOperator);
     }
     super.visitSpreadElement(node);
   }
@@ -5570,15 +5572,23 @@ class ErrorVerifier extends RecursiveAstVisitor<void> {
   }
 
   void _checkForUnnecessaryNullAware(Expression target, Token operator) {
-    if (operator.type != TokenType.QUESTION_PERIOD || !_isNonNullable) {
+    if (!_isNonNullable) {
+      return;
+    }
+
+    ErrorCode errorCode;
+    if (operator.type == TokenType.QUESTION_PERIOD) {
+      errorCode = HintCode.UNNECESSARY_NULL_AWARE_CALL;
+    } else if (operator.type == TokenType.PERIOD_PERIOD_PERIOD_QUESTION) {
+      errorCode = StaticWarningCode.UNNECESSARY_NULL_AWARE_SPREAD;
+    } else {
       return;
     }
 
     if (target.staticType != null &&
         (target.staticType as TypeImpl).nullabilitySuffix ==
             NullabilitySuffix.none) {
-      _errorReporter.reportErrorForToken(
-          HintCode.UNNECESSARY_NULL_AWARE_CALL, operator, []);
+      _errorReporter.reportErrorForToken(errorCode, operator, []);
     }
   }
 

pkg/analyzer/test/src/diagnostics/test_all.dart

@@ -162,6 +162,8 @@ import 'undefined_shown_name_test.dart' as undefined_shown_name;
 import 'unnecessary_cast_test.dart' as unnecessary_cast;
 import 'unnecessary_no_such_method_test.dart' as unnecessary_no_such_method;
 import 'unnecessary_null_aware_call_test.dart' as unnecessary_null_aware_call;
+import 'unnecessary_null_aware_spread_test.dart'
+    as unnecessary_null_aware_spread;
 import 'unnecessary_type_check_false_test.dart' as unnecessary_type_check_false;
 import 'unnecessary_type_check_true_test.dart' as unnecessary_type_check_true;
 import 'unused_catch_clause_test.dart' as unused_catch_clause;
@@ -291,6 +293,7 @@ main() {
     unnecessary_cast.main();
     unnecessary_no_such_method.main();
     unnecessary_null_aware_call.main();
+    unnecessary_null_aware_spread.main();
     unnecessary_type_check_false.main();
     unnecessary_type_check_true.main();
     unused_catch_clause.main();

pkg/analyzer/test/src/diagnostics/unnecessary_null_aware_spread_test.dart

@@ -0,0 +1,52 @@
+// 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:analyzer/src/dart/analysis/experiments.dart';
+import 'package:analyzer/src/error/codes.dart';
+import 'package:analyzer/src/generated/engine.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import '../dart/resolution/driver_resolution.dart';
+
+main() {
+  defineReflectiveSuite(() {
+    defineReflectiveTests(UnnecessaryNullAwareSpreadTest);
+  });
+}
+
+@reflectiveTest
+class UnnecessaryNullAwareSpreadTest extends DriverResolutionTest {
+  @override
+  AnalysisOptionsImpl get analysisOptions =>
+      AnalysisOptionsImpl()..enabledExperiments = [EnableString.non_nullable];
+
+  test_local_nonNullableSpread_nullableType() async {
+    await assertNoErrorsInCode('''
+f() {
+  List x = [];
+  [...x];
+}
+''');
+  }
+
+  test_local_nullableSpread_nonNullableType() async {
+    await assertErrorsInCode('''
+f() {
+  List x = [];
+  [...?x];
+}
+''', [
+      error(StaticWarningCode.UNNECESSARY_NULL_AWARE_SPREAD, 24, 4),
+    ]);
+  }
+
+  test_local_nullableSpread_nullableType() async {
+    await assertNoErrorsInCode('''
+f() {
+  List? x;
+  [...?x];
+}
+''');
+  }
+}