[cfe] Handle invalid typedefs and redirecting factories in lowering

Change-Id: I61d776434621fd7022d034f17e228ec553a7ff47
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209108
Reviewed-by: Dmitry Stefantsov <[email protected]>
Commit-Queue: Johnni Winther <[email protected]>

Author: johnniwinther

pkg/front_end/lib/src/fasta/kernel/constructor_tearoff_lowering.dart

@@ -253,7 +253,19 @@ SynthesizedFunctionNode buildRedirectingFactoryTearOffBody(
     FreshTypeParameters freshTypeParameters) {
   int fileOffset = tearOff.fileOffset;
 
+  List<TypeParameter> typeParameters;
+  if (target is Constructor) {
+    typeParameters = target.enclosingClass.typeParameters;
+  } else {
+    typeParameters = target.function!.typeParameters;
+  }
+
   if (!freshTypeParameters.substitution.isEmpty) {
+    if (typeArguments.length != typeParameters.length) {
+      // Error case: Use default types as type arguments.
+      typeArguments = new List<DartType>.generate(typeParameters.length,
+          (int index) => typeParameters[index].defaultType);
+    }
     if (typeArguments.isNotEmpty) {
       // Translate [typeArgument] into the context of the synthesized procedure.
       typeArguments = new List<DartType>.generate(
@@ -262,14 +274,21 @@ SynthesizedFunctionNode buildRedirectingFactoryTearOffBody(
               .substituteType(typeArguments[index]));
     }
   }
-
+  Map<TypeParameter, DartType> substitutionMap;
+  if (typeParameters.length == typeArguments.length) {
+    substitutionMap = new Map<TypeParameter, DartType>.fromIterables(
+        typeParameters, typeArguments);
+  } else {
+    // Error case: Substitute type parameters with `dynamic`.
+    substitutionMap = new Map<TypeParameter, DartType>.fromIterables(
+        typeParameters,
+        new List<DartType>.generate(
+            typeParameters.length, (int index) => const DynamicType()));
+  }
   Arguments arguments = _createArguments(tearOff, typeArguments, fileOffset);
   _createTearOffBody(tearOff, target, arguments);
   return new SynthesizedFunctionNode(
-      new Map<TypeParameter, DartType>.fromIterables(
-          target.enclosingClass!.typeParameters, typeArguments),
-      target.function!,
-      tearOff.function,
+      substitutionMap, target.function!, tearOff.function,
       identicalSignatures: false);
 }
 

pkg/front_end/lib/src/fasta/kernel/expression_generator.dart

@@ -3218,8 +3218,8 @@ class TypeUseGenerator extends AbstractReadOnlyAccessGenerator {
                     _helper.buildDartTypeArguments(typeArguments);
               }
               if (isGenericTypedefTearOff) {
-                if (isProperRenameForClass(
-                    _helper.typeEnvironment, aliasBuilder!.typedef)) {
+                if (isProperRenameForClass(_helper.typeEnvironment,
+                    aliasBuilder!.typedef, aliasBuilder.library.library)) {
                   return tearOffExpression;
                 }
                 Procedure? tearOffLowering =

pkg/front_end/lib/src/fasta/kernel/expression_generator_helper.dart

@@ -33,6 +33,7 @@ import 'kernel_ast_api.dart'
         FunctionNode,
         Initializer,
         InterfaceType,
+        Library,
         Member,
         Name,
         Procedure,
@@ -186,7 +187,8 @@ abstract class ExpressionGeneratorHelper implements InferenceHelper {
 }
 
 /// Checks that a generic [typedef] for a generic class.
-bool isProperRenameForClass(TypeEnvironment typeEnvironment, Typedef typedef) {
+bool isProperRenameForClass(
+    TypeEnvironment typeEnvironment, Typedef typedef, Library typedefLibrary) {
   DartType? rhsType = typedef.type;
   if (rhsType is! InterfaceType) {
     return false;
@@ -201,7 +203,7 @@ bool isProperRenameForClass(TypeEnvironment typeEnvironment, Typedef typedef) {
   for (int i = 0; i < fromParameters.length; ++i) {
     if (typeArguments[i] !=
         new TypeParameterType.withDefaultNullabilityForLibrary(
-            fromParameters[i], typedef.enclosingLibrary)) {
+            fromParameters[i], typedefLibrary)) {
       return false;
     }
   }

pkg/front_end/lib/src/fasta/kernel/kernel_helper.dart

@@ -59,32 +59,54 @@ class SynthesizedFunctionNode {
 
     // For mixin application constructors, the argument count is the same, but
     // for redirecting tear off lowerings, the argument count of the tear off
-    // can be less than that of the redirection target.
-
-    assert(_synthesized.positionalParameters.length <=
-        _original.positionalParameters.length);
-    for (int i = 0; i < _synthesized.positionalParameters.length; i++) {
-      cloneInitializer(_original.positionalParameters[i],
-          _synthesized.positionalParameters[i]);
-    }
+    // can be less than that of the redirection target or, in errors cases, be
+    // unrelated.
 
     if (identicalSignatures) {
+      assert(_synthesized.positionalParameters.length ==
+          _original.positionalParameters.length);
+      for (int i = 0; i < _synthesized.positionalParameters.length; i++) {
+        cloneInitializer(_original.positionalParameters[i],
+            _synthesized.positionalParameters[i]);
+      }
       assert(_synthesized.namedParameters.length ==
           _original.namedParameters.length);
       for (int i = 0; i < _synthesized.namedParameters.length; i++) {
         cloneInitializer(
             _original.namedParameters[i], _synthesized.namedParameters[i]);
       }
-    } else if (_synthesized.namedParameters.isNotEmpty) {
-      Map<String, VariableDeclaration> originalParameters = {};
-      for (int i = 0; i < _original.namedParameters.length; i++) {
-        originalParameters[_original.namedParameters[i].name!] =
-            _original.namedParameters[i];
+    } else {
+      for (int i = 0; i < _synthesized.positionalParameters.length; i++) {
+        VariableDeclaration synthesizedParameter =
+            _synthesized.positionalParameters[i];
+        if (i < _original.positionalParameters.length) {
+          cloneInitializer(
+              _original.positionalParameters[i], synthesizedParameter);
+        } else {
+          // Error case: use `null` as initializer.
+          synthesizedParameter.initializer = new NullLiteral()
+            ..parent = synthesizedParameter;
+        }
       }
-      for (int i = 0; i < _synthesized.namedParameters.length; i++) {
-        cloneInitializer(
-            originalParameters[_synthesized.namedParameters[i].name!]!,
-            _synthesized.namedParameters[i]);
+      if (_synthesized.namedParameters.isNotEmpty) {
+        Map<String, VariableDeclaration> originalParameters = {};
+        for (int i = 0; i < _original.namedParameters.length; i++) {
+          originalParameters[_original.namedParameters[i].name!] =
+              _original.namedParameters[i];
+        }
+        for (int i = 0; i < _synthesized.namedParameters.length; i++) {
+          VariableDeclaration synthesizedParameter =
+              _synthesized.namedParameters[i];
+          VariableDeclaration? originalParameter =
+              originalParameters[synthesizedParameter.name!];
+          if (originalParameter != null) {
+            cloneInitializer(originalParameter, synthesizedParameter);
+          } else {
+            // Error case: use `null` as initializer.
+            synthesizedParameter.initializer = new NullLiteral()
+              ..parent = synthesizedParameter;
+          }
+        }
       }
     }
   }

pkg/front_end/lib/src/fasta/source/source_type_alias_builder.dart

@@ -282,7 +282,8 @@ class SourceTypeAliasBuilder extends TypeAliasBuilderImpl {
     if (declaration is ClassBuilder &&
         targetType is InterfaceType &&
         typedef.typeParameters.isNotEmpty &&
-        !isProperRenameForClass(library.loader.typeEnvironment, typedef)) {
+        !isProperRenameForClass(
+            library.loader.typeEnvironment, typedef, library.library)) {
       tearOffs = {};
       _tearOffDependencies = {};
       declaration

pkg/front_end/test/spell_checking_list_tests.txt

@@ -144,6 +144,8 @@ c6c
 c7a
 c8a
 c8b
+c8c
+c8d
 ca
 cafebabe
 calloc
@@ -386,6 +388,7 @@ f6a
 f6b
 f7a
 f8a
+f8b
 fac
 faking
 falling

pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart

@@ -0,0 +1,20 @@
+// Copyright (c) 2021, 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.
+
+// From co19/Language/Types/Type_Aliases/scope_t02
+
+class A {}
+
+class C<T> {}
+
+typedef AAlias = A;
+typedef CAlias<T> = C<T>;
+
+typedef AAlias = A; //  error
+typedef AAlias = C<String>; // error
+typedef CAlias<T> = C<T>; //  error
+typedef CAlias = C<String>; //  error
+typedef CAlias<T1, T2> = C<T1>; //  error
+
+main() {}

pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart.strong.expect

@@ -0,0 +1,59 @@
+library /*isNonNullableByDefault*/;
+//
+// Problems in library:
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:14:9: Error: 'AAlias' is already declared in this scope.
+// typedef AAlias = A; //  error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:11:9: Context: Previous declaration of 'AAlias'.
+// typedef AAlias = A;
+//         ^^^^^^
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:15:9: Error: 'AAlias' is already declared in this scope.
+// typedef AAlias = C<String>; // error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:14:9: Context: Previous declaration of 'AAlias'.
+// typedef AAlias = A; //  error
+//         ^^^^^^
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:16:9: Error: 'CAlias' is already declared in this scope.
+// typedef CAlias<T> = C<T>; //  error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:12:9: Context: Previous declaration of 'CAlias'.
+// typedef CAlias<T> = C<T>;
+//         ^^^^^^
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:17:9: Error: 'CAlias' is already declared in this scope.
+// typedef CAlias = C<String>; //  error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:16:9: Context: Previous declaration of 'CAlias'.
+// typedef CAlias<T> = C<T>; //  error
+//         ^^^^^^
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:18:9: Error: 'CAlias' is already declared in this scope.
+// typedef CAlias<T1, T2> = C<T1>; //  error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:17:9: Context: Previous declaration of 'CAlias'.
+// typedef CAlias = C<String>; //  error
+//         ^^^^^^
+//
+import self as self;
+import "dart:core" as core;
+
+typedef AAlias = self::A;
+typedef CAlias<T extends core::Object? = dynamic> = self::C<T%>;
+class A extends core::Object {
+  synthetic constructor •() → self::A
+    : super core::Object::•()
+    ;
+  static method _#new#tearOff() → self::A
+    return new self::A::•();
+}
+class C<T extends core::Object? = dynamic> extends core::Object {
+  synthetic constructor •() → self::C<self::C::T%>
+    : super core::Object::•()
+    ;
+  static method _#new#tearOff<T extends core::Object? = dynamic>() → self::C<self::C::_#new#tearOff::T%>
+    return new self::C::•<self::C::_#new#tearOff::T%>();
+}
+static method main() → dynamic {}

pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart.strong.transformed.expect

@@ -0,0 +1,59 @@
+library /*isNonNullableByDefault*/;
+//
+// Problems in library:
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:14:9: Error: 'AAlias' is already declared in this scope.
+// typedef AAlias = A; //  error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:11:9: Context: Previous declaration of 'AAlias'.
+// typedef AAlias = A;
+//         ^^^^^^
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:15:9: Error: 'AAlias' is already declared in this scope.
+// typedef AAlias = C<String>; // error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:14:9: Context: Previous declaration of 'AAlias'.
+// typedef AAlias = A; //  error
+//         ^^^^^^
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:16:9: Error: 'CAlias' is already declared in this scope.
+// typedef CAlias<T> = C<T>; //  error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:12:9: Context: Previous declaration of 'CAlias'.
+// typedef CAlias<T> = C<T>;
+//         ^^^^^^
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:17:9: Error: 'CAlias' is already declared in this scope.
+// typedef CAlias = C<String>; //  error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:16:9: Context: Previous declaration of 'CAlias'.
+// typedef CAlias<T> = C<T>; //  error
+//         ^^^^^^
+//
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:18:9: Error: 'CAlias' is already declared in this scope.
+// typedef CAlias<T1, T2> = C<T1>; //  error
+//         ^^^^^^
+// pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart:17:9: Context: Previous declaration of 'CAlias'.
+// typedef CAlias = C<String>; //  error
+//         ^^^^^^
+//
+import self as self;
+import "dart:core" as core;
+
+typedef AAlias = self::A;
+typedef CAlias<T extends core::Object? = dynamic> = self::C<T%>;
+class A extends core::Object {
+  synthetic constructor •() → self::A
+    : super core::Object::•()
+    ;
+  static method _#new#tearOff() → self::A
+    return new self::A::•();
+}
+class C<T extends core::Object? = dynamic> extends core::Object {
+  synthetic constructor •() → self::C<self::C::T%>
+    : super core::Object::•()
+    ;
+  static method _#new#tearOff<T extends core::Object? = dynamic>() → self::C<self::C::_#new#tearOff::T%>
+    return new self::C::•<self::C::_#new#tearOff::T%>();
+}
+static method main() → dynamic {}

pkg/front_end/testcases/constructor_tearoffs/lowering/duplicate_typedefs.dart.textual_outline.expect

@@ -0,0 +1,12 @@
+class A {}
+
+class C<T> {}
+
+typedef AAlias = A;
+typedef CAlias<T> = C<T>;
+typedef AAlias = A;
+typedef AAlias = C<String>;
+typedef CAlias<T> = C<T>;
+typedef CAlias = C<String>;
+typedef CAlias<T1, T2> = C<T1>;
+main() {}