Skip to content

Commit 521cbcd

Browse files
srujzsCommit Queue
srujzs
authored and
Commit Queue
committed
[pkg:js] Disallow external extension members with type parameters
Bug: #49350 Checks to see that external extension members on `@staticInterop` types do not declare or use a type parameter. Change-Id: Id8646b599094b748c5490810b64d872065676014 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254103 Reviewed-by: Sigmund Cherem <sigmund@google.com> Commit-Queue: Srujan Gaddam <srujzs@google.com> Reviewed-by: Joshua Litt <joshualitt@google.com>
1 parent b6526be commit 521cbcd

7 files changed

Lines changed: 231 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,17 @@
112112

113113
#### `package:js`
114114

115-
- **Breaking change**: Classes with the preview annotation `@staticInterop` are
116-
now disallowed from using `external` generative constructors. Use
117-
`external factory`s for these classes instead. See [#48730][] for more
118-
details.
115+
- **Breaking changes to the preview feature `@staticInterop`**:
116+
- Classes with this annotation are now disallowed from using `external`
117+
generative constructors. Use `external factory`s for these classes instead,
118+
and the behavior should be identical. See [#48730][] for more details.
119+
- Classes with this annotation's external extension members are now disallowed
120+
from using type parameters e.g. `external void method<T>(T t)`. Use a
121+
non-`external` extension method for type parameters instead. See [#49350][]
122+
for more details.
119123

120124
[#48730]: https://github.com/dart-lang/sdk/issues/48730
125+
[#49350]: https://github.com/dart-lang/sdk/issues/49350
121126

122127
### Tools
123128

‎pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7121,6 +7121,22 @@ const MessageCode messageJsInteropOperatorsNotSupported = const MessageCode(
71217121
problemMessage: r"""JS interop classes do not support operator methods.""",
71227122
correctionMessage: r"""Try replacing this with a normal method.""");
71237123

7124+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
7125+
const Code<Null>
7126+
codeJsInteropStaticInteropExternalExtensionMembersWithTypeParameters =
7127+
messageJsInteropStaticInteropExternalExtensionMembersWithTypeParameters;
7128+
7129+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
7130+
const MessageCode
7131+
messageJsInteropStaticInteropExternalExtensionMembersWithTypeParameters =
7132+
const MessageCode(
7133+
"JsInteropStaticInteropExternalExtensionMembersWithTypeParameters",
7134+
problemMessage:
7135+
r"""`@staticInterop` classes cannot have external extension members with type parameters.""",
7136+
correctionMessage:
7137+
r"""Try using a Dart extension member if you need type parameters instead.""");
7138+
7139+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
71247140
const Code<Null> codeJsInteropStaticInteropGenerativeConstructor =
71257141
messageJsInteropStaticInteropGenerativeConstructor;
71267142

‎pkg/_js_interop_checks/lib/js_interop_checks.dart‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart'
1919
messageJsInteropNonExternalConstructor,
2020
messageJsInteropNonExternalMember,
2121
messageJsInteropOperatorsNotSupported,
22+
messageJsInteropStaticInteropExternalExtensionMembersWithTypeParameters,
2223
messageJsInteropStaticInteropGenerativeConstructor,
2324
templateJsInteropDartClassExtendsJSClass,
2425
templateJsInteropStaticInteropWithInstanceMembers,
@@ -34,6 +35,7 @@ class JsInteropChecks extends RecursiveVisitor {
3435
final CoreTypes _coreTypes;
3536
final DiagnosticReporter<Message, LocatedMessage> _diagnosticsReporter;
3637
final Map<String, Class> _nativeClasses;
38+
final _TypeParameterVisitor _typeParameterVisitor = _TypeParameterVisitor();
3739
bool _classHasJSAnnotation = false;
3840
bool _classHasAnonymousAnnotation = false;
3941
bool _classHasStaticInteropAnnotation = false;
@@ -301,6 +303,26 @@ class JsInteropChecks extends RecursiveVisitor {
301303
procedure.name.text.length,
302304
procedure.fileUri);
303305
}
306+
307+
if (procedure.isExternal &&
308+
procedure.isExtensionMember &&
309+
_isStaticInteropExtensionMember(procedure)) {
310+
// If the extension has type parameters of its own, it copies those type
311+
// parameters to the procedure's type parameters (in the front) as well.
312+
// Ignore these for the analysis.
313+
var extensionTypeParams =
314+
_libraryExtensionsIndex![procedure.reference]!.typeParameters;
315+
var procedureTypeParams = List.from(procedure.function.typeParameters);
316+
procedureTypeParams.removeRange(0, extensionTypeParams.length);
317+
if (procedureTypeParams.isNotEmpty ||
318+
_typeParameterVisitor.usesTypeParameters(procedure)) {
319+
_diagnosticsReporter.report(
320+
messageJsInteropStaticInteropExternalExtensionMembersWithTypeParameters,
321+
procedure.fileOffset,
322+
procedure.name.text.length,
323+
procedure.fileUri);
324+
}
325+
}
304326
}
305327

306328
@override
@@ -450,6 +472,12 @@ class JsInteropChecks extends RecursiveVisitor {
450472
return _checkExtensionMember(member, hasJSInteropAnnotation);
451473
}
452474

475+
/// Returns whether given extension [member] is in an extension that is on a
476+
/// `@staticInterop` class.
477+
bool _isStaticInteropExtensionMember(Member member) {
478+
return _checkExtensionMember(member, hasStaticInteropAnnotation);
479+
}
480+
453481
/// Returns whether given extension [member] is in an extension on a Native
454482
/// class.
455483
bool _isNativeExtensionMember(Member member) {
@@ -471,3 +499,18 @@ class JsInteropChecks extends RecursiveVisitor {
471499
return onType is InterfaceType && validateExtensionClass(onType.classNode);
472500
}
473501
}
502+
503+
class _TypeParameterVisitor extends RecursiveVisitor {
504+
bool _visitedTypeParameterType = false;
505+
506+
bool usesTypeParameters(Node node) {
507+
_visitedTypeParameterType = false;
508+
node.accept(this);
509+
return _visitedTypeParameterType;
510+
}
511+
512+
@override
513+
void visitTypeParameterType(TypeParameterType node) {
514+
_visitedTypeParameterType = true;
515+
}
516+
}

‎pkg/front_end/messages.status‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,8 @@ JsInteropNonExternalMember/analyzerCode: Fail # Web compiler specific
579579
JsInteropNonExternalMember/example: Fail # Web compiler specific
580580
JsInteropOperatorsNotSupported/analyzerCode: Fail # Web compiler specific
581581
JsInteropOperatorsNotSupported/example: Fail # Web compiler specific
582+
JsInteropStaticInteropExternalExtensionMembersWithTypeParameters/analyzerCode: Fail # Web compiler specific
583+
JsInteropStaticInteropExternalExtensionMembersWithTypeParameters/example: Fail # Web compiler specific
582584
JsInteropStaticInteropGenerativeConstructor/analyzerCode: Fail # Web compiler specific
583585
JsInteropStaticInteropGenerativeConstructor/example: Fail # Web compiler specific
584586
JsInteropStaticInteropMockExternalExtensionMemberConflict/analyzerCode: Fail # Web compiler specific

‎pkg/front_end/messages.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5266,6 +5266,10 @@ JsInteropStaticInteropMockNotStaticInteropType:
52665266
problemMessage: "First type argument '#type' is not a `@staticInterop` type."
52675267
correctionMessage: "Use a `@staticInterop` class instead."
52685268

5269+
JsInteropStaticInteropExternalExtensionMembersWithTypeParameters:
5270+
problemMessage: "`@staticInterop` classes cannot have external extension members with type parameters."
5271+
correctionMessage: "Try using a Dart extension member if you need type parameters instead."
5272+
52695273
JsInteropStaticInteropWithInstanceMembers:
52705274
problemMessage: "JS interop class '#name' with `@staticInterop` annotation cannot declare instance members."
52715275
correctionMessage: "Try moving the instance member to a static extension."

‎tests/lib/js/static_interop_test/external_extension_member_type_parameters_static_test.dart‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
@JS()
6+
library external_extension_member_type_parameters_static_test;
7+
8+
import 'package:js/js.dart';
9+
10+
@JS()
11+
@staticInterop
12+
class Uninstantiated {}
13+
14+
typedef TypedefT<T> = T Function();
15+
16+
extension E1<T> on Uninstantiated {
17+
// Test simple type parameters.
18+
external T fieldT;
19+
// ^
20+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
21+
external T get getT;
22+
// ^
23+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
24+
external set setT(T t);
25+
// ^
26+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
27+
external T returnT();
28+
// ^
29+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
30+
external void consumeT(T t);
31+
// ^
32+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
33+
34+
// Test type parameters in a nested type context.
35+
external List<T> fieldNestedT;
36+
// ^
37+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
38+
external void Function(T) get getNestedT;
39+
// ^
40+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
41+
external set setNestedT(TypedefT<T> nestedT);
42+
// ^
43+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
44+
external List<Map<T, T>> returnNestedT();
45+
// ^
46+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
47+
external void consumeNestedT(Set<TypedefT<T>> nestedT);
48+
// ^
49+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
50+
51+
// Test type parameters that are declared by the member.
52+
external U returnU<U>();
53+
// ^
54+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
55+
external void consumeU<U>(U u);
56+
// ^
57+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
58+
}
59+
60+
@JS()
61+
@staticInterop
62+
class Instantiated {}
63+
64+
extension E2 on Instantiated {
65+
// Test generic types where there all the type parameters are instantiated.
66+
external List<int> fieldList;
67+
external List<int> get getList;
68+
external set setList(List<int> list);
69+
external List<int> returnList();
70+
external void consumeList(List<int> list);
71+
}
72+
73+
// Extension members that don't declare or use type parameters should not be
74+
// affected by whether their extension declares a type parameter.
75+
@JS()
76+
@staticInterop
77+
class ExtensionWithTypeParams {}
78+
79+
extension E3<T> on ExtensionWithTypeParams {
80+
external void noTypeParams();
81+
external void declareTypeParam<U>(U u);
82+
// ^
83+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
84+
external void useTypeParam(T t);
85+
// ^
86+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
87+
}
88+
89+
void main() {}

‎tests/lib_2/js/static_interop_test/external_extension_member_type_parameters_static_test.dart‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// @dart = 2.9
6+
7+
@JS()
8+
library external_extension_member_type_parameters_static_test;
9+
10+
import 'package:js/js.dart';
11+
12+
@JS()
13+
@staticInterop
14+
class Uninstantiated {}
15+
16+
typedef TypedefT<T> = T Function();
17+
18+
extension E1<T> on Uninstantiated {
19+
// Test simple type parameters.
20+
external T get getT;
21+
// [error column 18]
22+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
23+
external set setT(T t);
24+
// [error column 16]
25+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
26+
external T returnT();
27+
// [error column 14]
28+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
29+
external void consumeT(T t);
30+
// [error column 17]
31+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
32+
33+
// Test type parameters in a nested type context.
34+
external void Function(T) get getNestedT;
35+
// [error column 33]
36+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
37+
external set setNestedT(TypedefT<T> nestedT);
38+
// [error column 16]
39+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
40+
external List<Map<T, T>> returnNestedT();
41+
// [error column 28]
42+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
43+
external void consumeNestedT(Set<TypedefT<T>> nestedT);
44+
// [error column 17]
45+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
46+
47+
// Test type parameters that are declared by the member.
48+
external U returnU<U>();
49+
// [error column 14]
50+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
51+
external void consumeU<U>(U u);
52+
// [error column 17]
53+
// [web] `@staticInterop` classes cannot have external extension members with type parameters.
54+
}
55+
56+
@JS()
57+
@staticInterop
58+
class Instantiated {}
59+
60+
extension E2 on Instantiated {
61+
// Test generic types where there all the type parameters are instantiated.
62+
external List<int> get getList;
63+
external set setList(List<int> list);
64+
external List<int> returnList();
65+
external void consumeList(List<int> list);
66+
}
67+
68+
void main() {}

0 commit comments

Comments
 (0)