@@ -13,26 +13,57 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart'
1313 messageJsInteropEnclosingClassJSAnnotationContext,
1414 messageJsInteropIndexNotSupported,
1515 messageJsInteropNamedParameters,
16- messageJsInteropNonExternalConstructor;
16+ messageJsInteropNonExternalConstructor,
17+ messageJsInteropNonExternalMember;
1718
1819import 'src/js_interop.dart' ;
1920
2021class JsInteropChecks extends RecursiveVisitor <void > {
2122 final DiagnosticReporter <Message , LocatedMessage > _diagnosticsReporter;
23+ bool _classHasJSAnnotation = false ;
24+ bool _libraryHasJSAnnotation = false ;
2225
2326 JsInteropChecks (this ._diagnosticsReporter);
2427
2528 @override
2629 void defaultMember (Member member) {
27- _checkMemberJSInteropAnnotation (member);
30+ _checkJSInteropAnnotation (member);
31+ // TODO(43530): Disallow having JS interop annotations on non-external
32+ // members (class members or otherwise). Currently, they're being ignored.
2833 super .defaultMember (member);
2934 }
3035
3136 @override
32- void visitProcedure (Procedure procedure) {
33- _checkMemberJSInteropAnnotation (procedure);
37+ void visitClass (Class cls) {
38+ _classHasJSAnnotation = hasJSInteropAnnotation (cls);
39+ super .visitClass (cls);
40+ _classHasJSAnnotation = false ;
41+ }
3442
35- if (! procedure.isExternal || ! isJSInteropMember (procedure)) return ;
43+ @override
44+ void visitLibrary (Library lib) {
45+ _libraryHasJSAnnotation = hasJSInteropAnnotation (lib);
46+ super .visitLibrary (lib);
47+ _libraryHasJSAnnotation = false ;
48+ }
49+
50+ @override
51+ void visitProcedure (Procedure procedure) {
52+ _checkJSInteropAnnotation (procedure);
53+ if (_classHasJSAnnotation && ! procedure.isExternal) {
54+ // If not one of few exceptions, member is not allowed to exclude
55+ // `external` inside of a JS interop class.
56+ if (! (procedure.isAbstract ||
57+ procedure.isFactory ||
58+ procedure.isStatic)) {
59+ _diagnosticsReporter.report (
60+ messageJsInteropNonExternalMember,
61+ procedure.fileOffset,
62+ procedure.name.text.length,
63+ procedure.location.file);
64+ }
65+ }
66+ if (! _isJSInteropMember (procedure)) return ;
3667
3768 if (! procedure.isStatic &&
3869 (procedure.name.text == '[]=' || procedure.name.text == '[]' )) {
@@ -65,17 +96,18 @@ class JsInteropChecks extends RecursiveVisitor<void> {
6596
6697 @override
6798 void visitConstructor (Constructor constructor) {
68- _checkMemberJSInteropAnnotation (constructor);
69-
70- if ( ! isJSInteropMember ( constructor)) return ;
71-
72- if ( ! constructor.isExternal && ! constructor.isSynthetic) {
99+ _checkJSInteropAnnotation (constructor);
100+ if (_classHasJSAnnotation &&
101+ ! constructor.isExternal &&
102+ ! constructor.isSynthetic) {
103+ // Non-synthetic constructors must be annotated with `external`.
73104 _diagnosticsReporter.report (
74105 messageJsInteropNonExternalConstructor,
75106 constructor.fileOffset,
76107 constructor.name.text.length,
77108 constructor.location.file);
78109 }
110+ if (! _isJSInteropMember (constructor)) return ;
79111
80112 _checkNoNamedParameters (constructor.function);
81113 }
@@ -92,14 +124,18 @@ class JsInteropChecks extends RecursiveVisitor<void> {
92124 }
93125 }
94126
95- /// Reports an error if [m] has a JS interop annotation and is part of a class
96- /// that does not.
97- void _checkMemberJSInteropAnnotation (Member m) {
98- if (! hasJSInteropAnnotation (m)) return ;
99- var enclosingClass = m.enclosingClass;
100- if (enclosingClass != null && ! hasJSInteropAnnotation (enclosingClass)) {
127+ /// Reports an error if [member] does not correctly use the JS interop
128+ /// annotation or the keyword `external` .
129+ void _checkJSInteropAnnotation (Member member) {
130+ var enclosingClass = member.enclosingClass;
131+
132+ if (! _classHasJSAnnotation &&
133+ enclosingClass != null &&
134+ hasJSInteropAnnotation (member)) {
135+ // If in a class that is not JS interop, this member is not allowed to be
136+ // JS interop.
101137 _diagnosticsReporter.report (messageJsInteropEnclosingClassJSAnnotation,
102- m .fileOffset, m .name.text.length, m .location.file,
138+ member .fileOffset, member .name.text.length, member .location.file,
103139 context: < LocatedMessage > [
104140 messageJsInteropEnclosingClassJSAnnotationContext.withLocation (
105141 enclosingClass.location.file,
@@ -108,4 +144,15 @@ class JsInteropChecks extends RecursiveVisitor<void> {
108144 ]);
109145 }
110146 }
147+
148+ /// Returns whether [member] is considered to be a JS interop member.
149+ bool _isJSInteropMember (Member member) {
150+ if (! member.isExternal) return false ;
151+ if (_classHasJSAnnotation) return true ;
152+ if (! _classHasJSAnnotation && member.enclosingClass != null ) return false ;
153+ // In the case where the member does not belong to any class, a JS
154+ // annotation is not needed on the library to be considered JS interop as
155+ // long as the member has an annotation.
156+ return hasJSInteropAnnotation (member) || _libraryHasJSAnnotation;
157+ }
111158}
0 commit comments