analyzer: Report omitted args for Deprecated.optional parameters in redirected constructor

Change-Id: I733f9436b924a23d60a0b2657c6330e2adc59098
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/457120
Commit-Queue: Samuel Rawlins <[email protected]>
Reviewed-by: Paul Berry <[email protected]>

Author: srawlins

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

@@ -36,6 +36,7 @@ class DeprecatedFunctionalityVerifier {
 
   void constructorDeclaration(ConstructorDeclaration node) {
     _checkForDeprecatedOptionalSuperParameters(node);
+    _checkForDeprecatedOptionalRedirectedParameters(node);
 
     // Check redirectiong constructor invocations in the initializer list.
     for (var redirectingConstructorInvocation
@@ -223,6 +224,46 @@ class DeprecatedFunctionalityVerifier {
     }
   }
 
+  void _checkForDeprecatedOptionalRedirectedParameters(
+    ConstructorDeclaration node,
+  ) {
+    if (node.redirectedConstructor?.element case var redirectedConstructor?) {
+      var SourceRange(offset: errorOffset, length: errorLength) =
+          node.errorRange;
+      var positionalArgumentCount = node.parameters.parameters
+          .where((p) => p.isPositional)
+          .length;
+      var namedArgumentNames = node.parameters.parameters
+          .where((p) => p.isNamed)
+          .map((p) => p.name?.lexeme)
+          .nonNulls
+          .toList();
+      var redirectedConstructorPositionalParameterCount = 0;
+      for (var parameter in redirectedConstructor.formalParameters) {
+        if (parameter.isPositional) {
+          redirectedConstructorPositionalParameterCount++;
+        }
+        if (!parameter.isOptional) continue;
+        if (!parameter.isDeprecatedWithKind('optional')) continue;
+        if (parameter.isPositional) {
+          if (redirectedConstructorPositionalParameterCount <=
+              positionalArgumentCount) {
+            continue;
+          }
+        } else {
+          if (namedArgumentNames.contains(parameter.name)) continue;
+        }
+
+        _diagnosticReporter.atOffset(
+          offset: errorOffset,
+          length: errorLength,
+          diagnosticCode: WarningCode.deprecatedOptional,
+          arguments: [parameter.name ?? '<unknown>'],
+        );
+      }
+    }
+  }
+
   void _checkForDeprecatedOptionalSuperParameters(ConstructorDeclaration node) {
     var superConstructorInvocations = node.initializers
         .whereType<SuperConstructorInvocation>();

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

@@ -59,6 +59,32 @@ void g() {
 ''');
   }
 
+  test_argumentGiven_redirectedFromFactory_parameter_named() async {
+    await assertNoErrorsInCode(r'''
+class C {
+  C();
+  factory C.two({int? p}) = D;
+}
+
+class D extends C {
+  D({@Deprecated.optional() int? p});
+}
+''');
+  }
+
+  test_argumentGiven_redirectedFromFactory_parameter_positional() async {
+    await assertNoErrorsInCode(r'''
+class C {
+  C();
+  factory C.two([int? p]) = D;
+}
+
+class D extends C {
+  D([@Deprecated.optional() int? p]);
+}
+''');
+  }
+
   test_argumentGiven_superInvocation() async {
     await assertNoErrorsInCode(r'''
 class C {
@@ -276,6 +302,67 @@ class C {
     );
   }
 
+  test_argumentOmitted_redirectedFromFactory_parameter_named() async {
+    await assertErrorsInCode(
+      r'''
+class C {
+  C();
+  factory C.two() = D;
+}
+
+class D extends C {
+  D({@Deprecated.optional() int? p});
+}
+''',
+      [error(WarningCode.deprecatedOptional, 27, 5)],
+    );
+  }
+
+  test_argumentOmitted_redirectedFromFactory_parameter_positional() async {
+    await assertErrorsInCode(
+      r'''
+class C {
+  C();
+  factory C.two() = D;
+}
+
+class D extends C {
+  D([@Deprecated.optional() int? p]);
+}
+''',
+      [error(WarningCode.deprecatedOptional, 27, 5)],
+    );
+  }
+
+  test_argumentOmitted_redirectedIndirectlyFromFactory_parameter_named() async {
+    // In this test, we ensure that no warning is reported for `C.two`. That is,
+    // we do not report indirect deprecation issues with redirecting factory
+    // constructors. The signature of `C.two` cannot be atomically adjusted to
+    // prepare for the new signature in `E.new`. The only way to prepare `C.two`
+    // is to simultaneously prepare `D.two`, and preparing `C.two` will either
+    // produce a new error or warning at `C.two` (if the signature is adjusted),
+    // or will make `C.two` indirectly compliant (like if `D.two` is changed to
+    // redirect differently, or not redirect, etc.).
+    await assertErrorsInCode(
+      r'''
+class C {
+  C();
+  factory C.two() = D.two;
+}
+
+class D extends C {
+  D();
+  factory D.two() = E;
+}
+
+class E extends D {
+  E({@Deprecated.optional() int? p});
+}
+''',
+      [error(WarningCode.deprecatedOptional, 84, 5)],
+    );
+  }
+
   test_argumentOmitted_superInvocation() async {
     await assertErrorsInCode(
       r'''