[pkg:js] Disallow staticInterop generative constructors

Fixes https://github.com/dart-lang/sdk/issues/48730

Change-Id: I4c7f687ec8d2724de0e031aa5ebe887f93843761
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/254101
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Johnni Winther <johnniwinther@google.com>
This commit is contained in:
Srujan Gaddam 2022-10-18 17:47:29 +00:00 committed by Commit Queue
parent ea541423d0
commit b6526beeab
27 changed files with 163 additions and 77 deletions

View file

@ -108,6 +108,17 @@
[`MirrorsUsed`]: https://api.dart.dev/stable/dart-mirrors/MirrorsUsed-class.html
[`Comment`]: https://api.dart.dev/stable/dart-mirrors/Comment-class.html
### Other libraries
#### `package:js`
- **Breaking change**: Classes with the preview annotation `@staticInterop` are
now disallowed from using `external` generative constructors. Use
`external factory`s for these classes instead. See [#48730][] for more
details.
[#48730]: https://github.com/dart-lang/sdk/issues/48730
### Tools
#### Analyzer

View file

@ -7121,6 +7121,16 @@ const MessageCode messageJsInteropOperatorsNotSupported = const MessageCode(
problemMessage: r"""JS interop classes do not support operator methods.""",
correctionMessage: r"""Try replacing this with a normal method.""");
const Code<Null> codeJsInteropStaticInteropGenerativeConstructor =
messageJsInteropStaticInteropGenerativeConstructor;
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const MessageCode messageJsInteropStaticInteropGenerativeConstructor =
const MessageCode("JsInteropStaticInteropGenerativeConstructor",
problemMessage:
r"""`@staticInterop` classes should not contain any generative constructors.""",
correctionMessage: r"""Use factory constructors instead.""");
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String name, String string)>
templateJsInteropStaticInteropMockExternalExtensionMemberConflict =

View file

@ -19,6 +19,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart'
messageJsInteropNonExternalConstructor,
messageJsInteropNonExternalMember,
messageJsInteropOperatorsNotSupported,
messageJsInteropStaticInteropGenerativeConstructor,
templateJsInteropDartClassExtendsJSClass,
templateJsInteropStaticInteropWithInstanceMembers,
templateJsInteropStaticInteropWithNonStaticSupertype,
@ -318,15 +319,22 @@ class JsInteropChecks extends RecursiveVisitor {
@override
void visitConstructor(Constructor constructor) {
_checkInstanceMemberJSAnnotation(constructor);
if (_classHasJSAnnotation &&
!constructor.isExternal &&
!constructor.isSynthetic) {
// Non-synthetic constructors must be annotated with `external`.
_diagnosticsReporter.report(
messageJsInteropNonExternalConstructor,
constructor.fileOffset,
constructor.name.text.length,
constructor.fileUri);
if (!constructor.isSynthetic) {
if (_classHasJSAnnotation && !constructor.isExternal) {
// Non-synthetic constructors must be annotated with `external`.
_diagnosticsReporter.report(
messageJsInteropNonExternalConstructor,
constructor.fileOffset,
constructor.name.text.length,
constructor.fileUri);
}
if (_classHasStaticInteropAnnotation) {
_diagnosticsReporter.report(
messageJsInteropStaticInteropGenerativeConstructor,
constructor.fileOffset,
constructor.name.text.length,
constructor.fileUri);
}
}
if (!_isJSInteropMember(constructor)) {

View file

@ -579,6 +579,8 @@ JsInteropNonExternalMember/analyzerCode: Fail # Web compiler specific
JsInteropNonExternalMember/example: Fail # Web compiler specific
JsInteropOperatorsNotSupported/analyzerCode: Fail # Web compiler specific
JsInteropOperatorsNotSupported/example: Fail # Web compiler specific
JsInteropStaticInteropGenerativeConstructor/analyzerCode: Fail # Web compiler specific
JsInteropStaticInteropGenerativeConstructor/example: Fail # Web compiler specific
JsInteropStaticInteropMockExternalExtensionMemberConflict/analyzerCode: Fail # Web compiler specific
JsInteropStaticInteropMockExternalExtensionMemberConflict/example: Fail # Web compiler specific
JsInteropStaticInteropMockMemberNotSubtype/analyzerCode: Fail # Web compiler specific

View file

@ -5242,6 +5242,10 @@ JsInteropOperatorsNotSupported:
JsInteropInvalidStaticClassMemberName:
problemMessage: "JS interop static class members cannot have '.' in their JS name."
JsInteropStaticInteropGenerativeConstructor:
problemMessage: "`@staticInterop` classes should not contain any generative constructors."
correctionMessage: "Use factory constructors instead."
JsInteropStaticInteropMockExternalExtensionMemberConflict:
problemMessage: "External extension member with name '#name' is defined in the following extensions and none are more specific: #string."
correctionMessage: "Try using the `@JS` annotation to rename conflicting members."

View file

@ -21,17 +21,16 @@ import "package:js/js.dart";
@#C4
@#C5
class StaticJSClass extends core::Object {
external constructor •() → sta::StaticJSClass
;
external static factory •() → sta::StaticJSClass;
static method _#new#tearOff() → _in::JavaScriptObject
return new sta::StaticJSClass::•() as _in::JavaScriptObject;
return sta::StaticJSClass::•() as _in::JavaScriptObject;
static factory factory() → sta::StaticJSClass {
return new sta::StaticJSClass::•();
return sta::StaticJSClass::•();
}
static method _#factory#tearOff() → _in::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
static method /*isLegacy*/ factory|staticInteropFactoryStub() → _in::JavaScriptObject {
return (new sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
return (sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
}
}
@#C2

View file

@ -22,17 +22,16 @@ import "package:js/js.dart";
@#C4
@#C5
class StaticJSClass extends core::Object {
external constructor •() → sta::StaticJSClass
;
external static factory •() → sta::StaticJSClass;
static method _#new#tearOff() → _in::JavaScriptObject
return new sta::StaticJSClass::•() as _in::JavaScriptObject;
return sta::StaticJSClass::•() as _in::JavaScriptObject;
static factory factory() → sta::StaticJSClass {
return new sta::StaticJSClass::•();
return sta::StaticJSClass::•();
}
static method _#factory#tearOff() → _in::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
static method /*isLegacy*/ factory|staticInteropFactoryStub() → _in::JavaScriptObject {
return (new sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
return (sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
}
}
@#C2

View file

@ -21,17 +21,16 @@ import "package:js/js.dart";
@#C4
@#C5
class StaticJSClass extends core::Object {
external constructor •() → sta::StaticJSClass
;
external static factory •() → sta::StaticJSClass;
static method _#new#tearOff() → _in::JavaScriptObject
return new sta::StaticJSClass::•() as _in::JavaScriptObject;
return sta::StaticJSClass::•() as _in::JavaScriptObject;
static factory factory() → sta::StaticJSClass {
return new sta::StaticJSClass::•();
return sta::StaticJSClass::•();
}
static method _#factory#tearOff() → _in::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
static method /*isLegacy*/ factory|staticInteropFactoryStub() → _in::JavaScriptObject {
return (new sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
return (sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
}
}
@#C2

View file

@ -18,10 +18,9 @@ import "package:js/js.dart";
@#C4
@#C5
class StaticJSClass extends core::Object {
external constructor •() → self2::StaticJSClass
;
external static factory •() → self2::StaticJSClass;
static method _#new#tearOff() → _in::JavaScriptObject
return new self2::StaticJSClass::•() as _in::JavaScriptObject;
return self2::StaticJSClass::•() as _in::JavaScriptObject;
static factory factory() → self2::StaticJSClass
;
static method _#factory#tearOff() → _in::JavaScriptObject

View file

@ -22,17 +22,16 @@ import "package:js/js.dart";
@#C4
@#C5
class StaticJSClass extends core::Object {
external constructor •() → sta::StaticJSClass
;
external static factory •() → sta::StaticJSClass;
static method _#new#tearOff() → _in::JavaScriptObject
return new sta::StaticJSClass::•() as _in::JavaScriptObject;
return sta::StaticJSClass::•() as _in::JavaScriptObject;
static factory factory() → sta::StaticJSClass {
return new sta::StaticJSClass::•();
return sta::StaticJSClass::•();
}
static method _#factory#tearOff() → _in::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
static method /*isLegacy*/ factory|staticInteropFactoryStub() → _in::JavaScriptObject {
return (new sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
return (sta::StaticJSClass::•() as _in::JavaScriptObject) as _in::JavaScriptObject;
}
}
@#C2

View file

@ -13,7 +13,7 @@ external void eval(String code);
@JS('JSClass')
@staticInterop
class StaticJSClass {
external StaticJSClass();
external factory StaticJSClass();
factory StaticJSClass.factory() {
return StaticJSClass();
}

View file

@ -68,7 +68,7 @@ worlds:
@JS('JSClass')
@staticInterop
class StaticJSClass {
external StaticJSClass();
external factory StaticJSClass();
factory StaticJSClass.factory() {
return StaticJSClass();
}

View file

@ -29,20 +29,20 @@ library from "org-dartlang-test:///lib1.dart" as lib1 {
method instanceMethod() → dart._interceptors::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
get instanceGetter() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
set instanceSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
static method staticMethod() → dart._interceptors::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
static get staticGetter() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
static set staticSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
static method _#new#tearOff() → lib1::Class
return new lib1::Class::•();
}
static method topLevelMethod() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
static get topLevelGetter() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
static set topLevelSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
}
@#C3
@ -53,17 +53,16 @@ library static_interop from "org-dartlang-test:///lib2.dart" as sta {
@#C5
@#C2
class StaticJSClass extends dart.core::Object {
external constructor •() → sta::StaticJSClass
;
external static factory •() → sta::StaticJSClass;
static method _#new#tearOff() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
static factory factory() → sta::StaticJSClass {
return new sta::StaticJSClass::•();
return sta::StaticJSClass::•();
}
static method _#factory#tearOff() → dart._interceptors::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
static method /*isLegacy*/ factory|staticInteropFactoryStub() → dart._interceptors::JavaScriptObject {
return (new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject) as dart._interceptors::JavaScriptObject;
return (sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject) as dart._interceptors::JavaScriptObject;
}
}
@#C3

View file

@ -29,20 +29,20 @@ library from "org-dartlang-test:///lib1.dart" as lib1 {
method instanceMethod() → dart._interceptors::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
get instanceGetter() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
set instanceSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
static method staticMethod() → dart._interceptors::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
static get staticGetter() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
static set staticSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
static method _#new#tearOff() → lib1::Class
return new lib1::Class::•();
}
static method topLevelMethod() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
static get topLevelGetter() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
static set topLevelSetter((dart._interceptors::JavaScriptObject) → void f) → void {}
}
@#C3
@ -53,17 +53,16 @@ library static_interop from "org-dartlang-test:///lib2.dart" as sta {
@#C5
@#C2
class StaticJSClass extends dart.core::Object {
external constructor •() → sta::StaticJSClass
;
external static factory •() → sta::StaticJSClass;
static method _#new#tearOff() → dart._interceptors::JavaScriptObject
return new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
return sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject;
static factory factory() → sta::StaticJSClass {
return new sta::StaticJSClass::•();
return sta::StaticJSClass::•();
}
static method _#factory#tearOff() → dart._interceptors::JavaScriptObject
return sta::StaticJSClass::factory|staticInteropFactoryStub();
static method /*isLegacy*/ factory|staticInteropFactoryStub() → dart._interceptors::JavaScriptObject {
return (new sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject) as dart._interceptors::JavaScriptObject;
return (sta::StaticJSClass::•() as dart._interceptors::JavaScriptObject) as dart._interceptors::JavaScriptObject;
}
}
@#C3

View file

@ -0,0 +1,25 @@
// Copyright (c) 2022, 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.
@JS()
library generative_constructor_static_test;
import 'package:js/js.dart';
@JS()
@staticInterop
class JSClass {
external JSClass();
// ^
// [web] `@staticInterop` classes should not contain any generative constructors.
external JSClass.namedConstructor();
// ^
// [web] `@staticInterop` classes should not contain any generative constructors.
}
@JS()
@staticInterop
class SyntheticConstructor {}
void main() {}

View file

@ -12,8 +12,7 @@ import 'package:js/js.dart';
@JS()
@staticInterop
class StaticJSClass {
external StaticJSClass();
external StaticJSClass.namedConstructor();
external factory StaticJSClass();
external factory StaticJSClass.externalFactory();
factory StaticJSClass.redirectingFactory() = StaticJSClass;
factory StaticJSClass.factory() => StaticJSClass();

View file

@ -0,0 +1,27 @@
// Copyright (c) 2022, 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.
// @dart = 2.9
@JS()
library generative_constructor_static_test;
import 'package:js/js.dart';
@JS()
@staticInterop
class JSClass {
external JSClass();
// ^
// [web] `@staticInterop` classes should not contain any generative constructors.
external JSClass.namedConstructor();
// ^
// [web] `@staticInterop` classes should not contain any generative constructors.
}
@JS()
@staticInterop
class SyntheticConstructor {}
void main() {}

View file

@ -12,8 +12,7 @@ import 'package:js/js.dart';
@JS()
@staticInterop
class StaticJSClass {
external StaticJSClass();
external StaticJSClass.namedConstructor();
external factory StaticJSClass();
external factory StaticJSClass.externalFactory();
factory StaticJSClass.redirectingFactory() = StaticJSClass;
factory StaticJSClass.factory() => StaticJSClass();

View file

@ -13,7 +13,7 @@ external void eval(String code);
@JS('JSClass')
@staticInterop
class StaticJSClass {
external StaticJSClass();
external factory StaticJSClass();
factory StaticJSClass.factory(StaticJSClass _) {
return StaticJSClass();
}

View file

@ -12,7 +12,7 @@ import 'factory_stub_test.dart';
@JS('NativeClass')
@staticInterop
class StaticNativeClassCopy {
external StaticNativeClassCopy();
external factory StaticNativeClassCopy();
factory StaticNativeClassCopy.nestedFactory() {
StaticNativeClass.nestedFactory();
return StaticNativeClassCopy();

View file

@ -26,7 +26,7 @@ class NativeClass extends JavaScriptObject {
@JS('NativeClass')
@staticInterop
class StaticNativeClass {
external StaticNativeClass();
external factory StaticNativeClass();
factory StaticNativeClass.redirectingFactory() = StaticNativeClass;
factory StaticNativeClass.simpleFactory() => StaticNativeClass();
factory StaticNativeClass.factoryWithParam(

View file

@ -27,7 +27,7 @@ class NativeClass extends JavaScriptObject {
@JS('NativeClass')
@staticInterop
class StaticNativeClass {
external StaticNativeClass();
external factory StaticNativeClass();
}
@JS()
@ -38,13 +38,13 @@ class JSClass {
@JS('JSClass')
@staticInterop
class StaticJSClass {
external StaticJSClass();
external factory StaticJSClass();
}
@JS()
@anonymous
class AnonymousClass {
external factory();
external factory AnonymousClass();
}
@JS()
@ -149,11 +149,17 @@ void main() {
// object or a native object.
expect(staticNativeClass is JSClass, false);
expect(confuse(staticNativeClass) is JSClass, false);
expect(() => staticNativeClass as JSClass, throws);
// TODO(srujzs): This should throw in dart2js without the `confuse`, but it
// does not. This does throw without `confuse` when we use an external
// generative constructor for `StaticNativeClass`, but not when we use an
// external factory constructor. Investigate why dart2js' type check
// optimizations have this discrepancy.
expect(() => confuse(staticNativeClass) as JSClass, throws);
expect(staticNativeClass is AnonymousClass, false);
expect(confuse(staticNativeClass) is AnonymousClass, false);
expect(() => staticNativeClass as AnonymousClass, throws);
// TODO(srujzs): Same comment as above.
expect(() => confuse(staticNativeClass) as AnonymousClass, throws);
expect(staticJsClass is NativeClass, false);
expect(confuse(staticJsClass) is NativeClass, false);

View file

@ -56,8 +56,7 @@ extension InterfaceExtension on Interface {
@JS('NativeClass')
@staticInterop
class StaticJSClass extends Parent implements Interface {
external StaticJSClass();
external StaticJSClass.namedConstructor();
external factory StaticJSClass();
external factory StaticJSClass.externalFactory();
factory StaticJSClass.redirectingFactory() = StaticJSClass;
@ -115,7 +114,6 @@ void main() {
NativeClass();
// Invoke constructors and ensure they're typed correctly.
StaticJSClass staticJs = StaticJSClass();
staticJs = StaticJSClass.namedConstructor();
staticJs = StaticJSClass.externalFactory();
staticJs = StaticJSClass.redirectingFactory();

View file

@ -14,7 +14,7 @@ import 'factory_stub_test.dart';
@JS('NativeClass')
@staticInterop
class StaticNativeClassCopy {
external StaticNativeClassCopy();
external factory StaticNativeClassCopy();
factory StaticNativeClassCopy.nestedFactory() {
StaticNativeClass.nestedFactory();
return StaticNativeClassCopy();

View file

@ -28,7 +28,7 @@ class NativeClass extends JavaScriptObject {
@JS('NativeClass')
@staticInterop
class StaticNativeClass {
external StaticNativeClass();
external factory StaticNativeClass();
factory StaticNativeClass.redirectingFactory() = StaticNativeClass;
factory StaticNativeClass.simpleFactory() => StaticNativeClass();
factory StaticNativeClass.factoryWithParam(

View file

@ -28,7 +28,7 @@ class NativeClass extends JavaScriptObject {
@JS('NativeClass')
@staticInterop
class StaticNativeClass {
external StaticNativeClass();
external factory StaticNativeClass();
}
@JS()
@ -39,13 +39,13 @@ class JSClass {
@JS('JSClass')
@staticInterop
class StaticJSClass {
external StaticJSClass();
external factory StaticJSClass();
}
@JS()
@anonymous
class AnonymousClass {
external factory();
external factory AnonymousClass();
}
@JS()
@ -139,11 +139,17 @@ void main() {
// object or a native object.
expect(staticNativeClass is JSClass, false);
expect(confuse(staticNativeClass) is JSClass, false);
expect(() => staticNativeClass as JSClass, throws);
// TODO(srujzs): This should throw in dart2js without the `confuse`, but it
// does not. This does throw without `confuse` when we use an external
// generative constructor for `StaticNativeClass`, but not when we use an
// external factory constructor. Investigate why dart2js' type check
// optimizations have this discrepancy.
expect(() => confuse(staticNativeClass) as JSClass, throws);
expect(staticNativeClass is AnonymousClass, false);
expect(confuse(staticNativeClass) is AnonymousClass, false);
expect(() => staticNativeClass as AnonymousClass, throws);
// TODO(srujzs): Same comment as above.
expect(() => confuse(staticNativeClass) as AnonymousClass, throws);
expect(staticJsClass is NativeClass, false);
expect(confuse(staticJsClass) is NativeClass, false);

View file

@ -56,8 +56,7 @@ extension InterfaceExtension on Interface {
@JS('NativeClass')
@staticInterop
class StaticJSClass extends Parent implements Interface {
external StaticJSClass();
external StaticJSClass.namedConstructor();
external factory StaticJSClass();
external factory StaticJSClass.externalFactory();
factory StaticJSClass.redirectingFactory() = StaticJSClass;
@ -110,7 +109,6 @@ void main() {
NativeClass();
// Invoke constructors and ensure they're typed correctly.
StaticJSClass staticJs = StaticJSClass();
staticJs = StaticJSClass.namedConstructor();
staticJs = StaticJSClass.externalFactory();
staticJs = StaticJSClass.redirectingFactory();