Skip to content

Commit b6526be

Browse files
srujzsCommit Queue
srujzs
authored and
Commit Queue
committed
[pkg:js] Disallow staticInterop generative constructors
Fixes #48730 Change-Id: I4c7f687ec8d2724de0e031aa5ebe887f93843761 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254101 Reviewed-by: Sigmund Cherem <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent ea54142 commit b6526be

27 files changed

+163
-77
lines changed

‎CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,17 @@
108108
[`MirrorsUsed`]: https://api.dart.dev/stable/dart-mirrors/MirrorsUsed-class.html
109109
[`Comment`]: https://api.dart.dev/stable/dart-mirrors/Comment-class.html
110110

111+
### Other libraries
112+
113+
#### `package:js`
114+
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.
119+
120+
[#48730]: https://github.com/dart-lang/sdk/issues/48730
121+
111122
### Tools
112123

113124
#### Analyzer

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7121,6 +7121,16 @@ 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+
const Code<Null> codeJsInteropStaticInteropGenerativeConstructor =
7125+
messageJsInteropStaticInteropGenerativeConstructor;
7126+
7127+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
7128+
const MessageCode messageJsInteropStaticInteropGenerativeConstructor =
7129+
const MessageCode("JsInteropStaticInteropGenerativeConstructor",
7130+
problemMessage:
7131+
r"""`@staticInterop` classes should not contain any generative constructors.""",
7132+
correctionMessage: r"""Use factory constructors instead.""");
7133+
71247134
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
71257135
const Template<Message Function(String name, String string)>
71267136
templateJsInteropStaticInteropMockExternalExtensionMemberConflict =

‎pkg/_js_interop_checks/lib/js_interop_checks.dart

Lines changed: 17 additions & 9 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+
messageJsInteropStaticInteropGenerativeConstructor,
2223
templateJsInteropDartClassExtendsJSClass,
2324
templateJsInteropStaticInteropWithInstanceMembers,
2425
templateJsInteropStaticInteropWithNonStaticSupertype,
@@ -318,15 +319,22 @@ class JsInteropChecks extends RecursiveVisitor {
318319
@override
319320
void visitConstructor(Constructor constructor) {
320321
_checkInstanceMemberJSAnnotation(constructor);
321-
if (_classHasJSAnnotation &&
322-
!constructor.isExternal &&
323-
!constructor.isSynthetic) {
324-
// Non-synthetic constructors must be annotated with `external`.
325-
_diagnosticsReporter.report(
326-
messageJsInteropNonExternalConstructor,
327-
constructor.fileOffset,
328-
constructor.name.text.length,
329-
constructor.fileUri);
322+
if (!constructor.isSynthetic) {
323+
if (_classHasJSAnnotation && !constructor.isExternal) {
324+
// Non-synthetic constructors must be annotated with `external`.
325+
_diagnosticsReporter.report(
326+
messageJsInteropNonExternalConstructor,
327+
constructor.fileOffset,
328+
constructor.name.text.length,
329+
constructor.fileUri);
330+
}
331+
if (_classHasStaticInteropAnnotation) {
332+
_diagnosticsReporter.report(
333+
messageJsInteropStaticInteropGenerativeConstructor,
334+
constructor.fileOffset,
335+
constructor.name.text.length,
336+
constructor.fileUri);
337+
}
330338
}
331339

332340
if (!_isJSInteropMember(constructor)) {

‎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+
JsInteropStaticInteropGenerativeConstructor/analyzerCode: Fail # Web compiler specific
583+
JsInteropStaticInteropGenerativeConstructor/example: Fail # Web compiler specific
582584
JsInteropStaticInteropMockExternalExtensionMemberConflict/analyzerCode: Fail # Web compiler specific
583585
JsInteropStaticInteropMockExternalExtensionMemberConflict/example: Fail # Web compiler specific
584586
JsInteropStaticInteropMockMemberNotSubtype/analyzerCode: Fail # Web compiler specific

‎pkg/front_end/messages.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5242,6 +5242,10 @@ JsInteropOperatorsNotSupported:
52425242
JsInteropInvalidStaticClassMemberName:
52435243
problemMessage: "JS interop static class members cannot have '.' in their JS name."
52445244

5245+
JsInteropStaticInteropGenerativeConstructor:
5246+
problemMessage: "`@staticInterop` classes should not contain any generative constructors."
5247+
correctionMessage: "Use factory constructors instead."
5248+
52455249
JsInteropStaticInteropMockExternalExtensionMemberConflict:
52465250
problemMessage: "External extension member with name '#name' is defined in the following extensions and none are more specific: #string."
52475251
correctionMessage: "Try using the `@JS` annotation to rename conflicting members."

‎pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.strong.expect

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,16 @@ import "package:js/js.dart";
2121
@#C4
2222
@#C5
2323
class StaticJSClass extends core::Object {
24-
external constructor •() → sta::StaticJSClass
25-
;
24+
external static factory •() → sta::StaticJSClass;
2625
static method _#new#tearOff() → _in::JavaScriptObject
27-
return new sta::StaticJSClass::•() as _in::JavaScriptObject;
26+
return sta::StaticJSClass::•() as _in::JavaScriptObject;
2827
static factory factory() → sta::StaticJSClass {
29-
return new sta::StaticJSClass::•();
28+
return sta::StaticJSClass::•();
3029
}
3130
static method _#factory#tearOff() → _in::JavaScriptObject
3231
return sta::StaticJSClass::factory|staticInteropFactoryStub();
3332
static method /*isLegacy*/ factory|staticInteropFactoryStub() → _in::JavaScriptObject {
34-
return (new sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
33+
return (sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
3534
}
3635
}
3736
@#C2

‎pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.strong.transformed.expect

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,16 @@ import "package:js/js.dart";
2222
@#C4
2323
@#C5
2424
class StaticJSClass extends core::Object {
25-
external constructor •() → sta::StaticJSClass
26-
;
25+
external static factory •() → sta::StaticJSClass;
2726
static method _#new#tearOff() → _in::JavaScriptObject
28-
return new sta::StaticJSClass::•() as _in::JavaScriptObject;
27+
return sta::StaticJSClass::•() as _in::JavaScriptObject;
2928
static factory factory() → sta::StaticJSClass {
30-
return new sta::StaticJSClass::•();
29+
return sta::StaticJSClass::•();
3130
}
3231
static method _#factory#tearOff() → _in::JavaScriptObject
3332
return sta::StaticJSClass::factory|staticInteropFactoryStub();
3433
static method /*isLegacy*/ factory|staticInteropFactoryStub() → _in::JavaScriptObject {
35-
return (new sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
34+
return (sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
3635
}
3736
}
3837
@#C2

‎pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.weak.expect

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,16 @@ import "package:js/js.dart";
2121
@#C4
2222
@#C5
2323
class StaticJSClass extends core::Object {
24-
external constructor •() → sta::StaticJSClass
25-
;
24+
external static factory •() → sta::StaticJSClass;
2625
static method _#new#tearOff() → _in::JavaScriptObject
27-
return new sta::StaticJSClass::•() as _in::JavaScriptObject;
26+
return sta::StaticJSClass::•() as _in::JavaScriptObject;
2827
static factory factory() → sta::StaticJSClass {
29-
return new sta::StaticJSClass::•();
28+
return sta::StaticJSClass::•();
3029
}
3130
static method _#factory#tearOff() → _in::JavaScriptObject
3231
return sta::StaticJSClass::factory|staticInteropFactoryStub();
3332
static method /*isLegacy*/ factory|staticInteropFactoryStub() → _in::JavaScriptObject {
34-
return (new sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
33+
return (sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
3534
}
3635
}
3736
@#C2

‎pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.weak.outline.expect

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,9 @@ import "package:js/js.dart";
1818
@#C4
1919
@#C5
2020
class StaticJSClass extends core::Object {
21-
external constructor •() → self2::StaticJSClass
22-
;
21+
external static factory •() → self2::StaticJSClass;
2322
static method _#new#tearOff() → _in::JavaScriptObject
24-
return new self2::StaticJSClass::•() as _in::JavaScriptObject;
23+
return self2::StaticJSClass::•() as _in::JavaScriptObject;
2524
static factory factory() → self2::StaticJSClass
2625
;
2726
static method _#factory#tearOff() → _in::JavaScriptObject

‎pkg/front_end/testcases/dartdevc/static_interop_erasure/main.dart.weak.transformed.expect

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,16 @@ import "package:js/js.dart";
2222
@#C4
2323
@#C5
2424
class StaticJSClass extends core::Object {
25-
external constructor •() → sta::StaticJSClass
26-
;
25+
external static factory •() → sta::StaticJSClass;
2726
static method _#new#tearOff() → _in::JavaScriptObject
28-
return new sta::StaticJSClass::•() as _in::JavaScriptObject;
27+
return sta::StaticJSClass::•() as _in::JavaScriptObject;
2928
static factory factory() → sta::StaticJSClass {
30-
return new sta::StaticJSClass::•();
29+
return sta::StaticJSClass::•();
3130
}
3231
static method _#factory#tearOff() → _in::JavaScriptObject
3332
return sta::StaticJSClass::factory|staticInteropFactoryStub();
3433
static method /*isLegacy*/ factory|staticInteropFactoryStub() → _in::JavaScriptObject {
35-
return (new sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
34+
return (sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
3635
}
3736
}
3837
@#C2

‎pkg/front_end/testcases/dartdevc/static_interop_erasure/main_lib.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ external void eval(String code);
1313
@JS('JSClass')
1414
@staticInterop
1515
class StaticJSClass {
16-
external StaticJSClass();
16+
external factory StaticJSClass();
1717
factory StaticJSClass.factory() {
1818
return StaticJSClass();
1919
}

‎pkg/front_end/testcases/incremental/js_interop_change.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ worlds:
6868
@JS('JSClass')
6969
@staticInterop
7070
class StaticJSClass {
71-
external StaticJSClass();
71+
external factory StaticJSClass();
7272
factory StaticJSClass.factory() {
7373
return StaticJSClass();
7474
}

‎pkg/front_end/testcases/incremental/js_interop_change.yaml.world.1.expect

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,20 @@ library from "org-dartlang-test:///lib1.dart" as lib1 {
2929
method instanceMethod() → dart._interceptors::JavaScriptObject
3030
return sta::StaticJSClass::factory|staticInteropFactoryStub();
3131
get instanceGetter() → dart._interceptors::JavaScriptObject
32-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
32+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
3333
set instanceSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
3434
static method staticMethod() → dart._interceptors::JavaScriptObject
3535
return sta::StaticJSClass::factory|staticInteropFactoryStub();
3636
static get staticGetter() → dart._interceptors::JavaScriptObject
37-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
37+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
3838
static set staticSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
3939
static method _#new#tearOff() → lib1::Class
4040
return new lib1::Class::•();
4141
}
4242
static method topLevelMethod() → dart._interceptors::JavaScriptObject
43-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
43+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
4444
static get topLevelGetter() → dart._interceptors::JavaScriptObject
45-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
45+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
4646
static set topLevelSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
4747
}
4848
@#C3
@@ -53,17 +53,16 @@ library static_interop from "org-dartlang-test:///lib2.dart" as sta {
5353
@#C5
5454
@#C2
5555
class StaticJSClass extends dart.core::Object {
56-
external constructor •() → sta::StaticJSClass
57-
;
56+
external static factory •() → sta::StaticJSClass;
5857
static method _#new#tearOff() → dart._interceptors::JavaScriptObject
59-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
58+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
6059
static factory factory() → sta::StaticJSClass {
61-
return new sta::StaticJSClass::•();
60+
return sta::StaticJSClass::•();
6261
}
6362
static method _#factory#tearOff() → dart._interceptors::JavaScriptObject
6463
return sta::StaticJSClass::factory|staticInteropFactoryStub();
6564
static method /*isLegacy*/ factory|staticInteropFactoryStub() → dart._interceptors::JavaScriptObject {
66-
return (new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject) as dart._interceptors::JavaScriptObject;
65+
return (sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject) as dart._interceptors::JavaScriptObject;
6766
}
6867
}
6968
@#C3

‎pkg/front_end/testcases/incremental/js_interop_change.yaml.world.2.expect

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,20 @@ library from "org-dartlang-test:///lib1.dart" as lib1 {
2929
method instanceMethod() → dart._interceptors::JavaScriptObject
3030
return sta::StaticJSClass::factory|staticInteropFactoryStub();
3131
get instanceGetter() → dart._interceptors::JavaScriptObject
32-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
32+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
3333
set instanceSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
3434
static method staticMethod() → dart._interceptors::JavaScriptObject
3535
return sta::StaticJSClass::factory|staticInteropFactoryStub();
3636
static get staticGetter() → dart._interceptors::JavaScriptObject
37-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
37+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
3838
static set staticSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
3939
static method _#new#tearOff() → lib1::Class
4040
return new lib1::Class::•();
4141
}
4242
static method topLevelMethod() → dart._interceptors::JavaScriptObject
43-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
43+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
4444
static get topLevelGetter() → dart._interceptors::JavaScriptObject
45-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
45+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
4646
static set topLevelSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
4747
}
4848
@#C3
@@ -53,17 +53,16 @@ library static_interop from "org-dartlang-test:///lib2.dart" as sta {
5353
@#C5
5454
@#C2
5555
class StaticJSClass extends dart.core::Object {
56-
external constructor •() → sta::StaticJSClass
57-
;
56+
external static factory •() → sta::StaticJSClass;
5857
static method _#new#tearOff() → dart._interceptors::JavaScriptObject
59-
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
58+
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
6059
static factory factory() → sta::StaticJSClass {
61-
return new sta::StaticJSClass::•();
60+
return sta::StaticJSClass::•();
6261
}
6362
static method _#factory#tearOff() → dart._interceptors::JavaScriptObject
6463
return sta::StaticJSClass::factory|staticInteropFactoryStub();
6564
static method /*isLegacy*/ factory|staticInteropFactoryStub() → dart._interceptors::JavaScriptObject {
66-
return (new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject) as dart._interceptors::JavaScriptObject;
65+
return (sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject) as dart._interceptors::JavaScriptObject;
6766
}
6867
}
6968
@#C3

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
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 generative_constructor_static_test;
7+
8+
import 'package:js/js.dart';
9+
10+
@JS()
11+
@staticInterop
12+
class JSClass {
13+
external JSClass();
14+
// ^
15+
// [web] `@staticInterop` classes should not contain any generative constructors.
16+
external JSClass.namedConstructor();
17+
// ^
18+
// [web] `@staticInterop` classes should not contain any generative constructors.
19+
}
20+
21+
@JS()
22+
@staticInterop
23+
class SyntheticConstructor {}
24+
25+
void main() {}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ import 'package:js/js.dart';
1212
@JS()
1313
@staticInterop
1414
class StaticJSClass {
15-
external StaticJSClass();
16-
external StaticJSClass.namedConstructor();
15+
external factory StaticJSClass();
1716
external factory StaticJSClass.externalFactory();
1817
factory StaticJSClass.redirectingFactory() = StaticJSClass;
1918
factory StaticJSClass.factory() => StaticJSClass();

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
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 generative_constructor_static_test;
9+
10+
import 'package:js/js.dart';
11+
12+
@JS()
13+
@staticInterop
14+
class JSClass {
15+
external JSClass();
16+
// ^
17+
// [web] `@staticInterop` classes should not contain any generative constructors.
18+
external JSClass.namedConstructor();
19+
// ^
20+
// [web] `@staticInterop` classes should not contain any generative constructors.
21+
}
22+
23+
@JS()
24+
@staticInterop
25+
class SyntheticConstructor {}
26+
27+
void main() {}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ import 'package:js/js.dart';
1212
@JS()
1313
@staticInterop
1414
class StaticJSClass {
15-
external StaticJSClass();
16-
external StaticJSClass.namedConstructor();
15+
external factory StaticJSClass();
1716
external factory StaticJSClass.externalFactory();
1817
factory StaticJSClass.redirectingFactory() = StaticJSClass;
1918
factory StaticJSClass.factory() => StaticJSClass();

‎tests/modular/static_interop_erasure/static_interop.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ external void eval(String code);
1313
@JS('JSClass')
1414
@staticInterop
1515
class StaticJSClass {
16-
external StaticJSClass();
16+
external factory StaticJSClass();
1717
factory StaticJSClass.factory(StaticJSClass _) {
1818
return StaticJSClass();
1919
}

‎tests/web/native/static_interop_erasure/factory_stub_lib.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import 'factory_stub_test.dart';
1212
@JS('NativeClass')
1313
@staticInterop
1414
class StaticNativeClassCopy {
15-
external StaticNativeClassCopy();
15+
external factory StaticNativeClassCopy();
1616
factory StaticNativeClassCopy.nestedFactory() {
1717
StaticNativeClass.nestedFactory();
1818
return StaticNativeClassCopy();

0 commit comments

Comments
 (0)