Skip to content

Commit a319876

Browse files
sjrdsjrd
committed
Address the remaining review comments.
1 parent c4d0acf commit a319876

10 files changed

+47
-64
lines changed

‎compiler/src/dotty/tools/dotc/config/SJSPlatform.scala

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ class SJSPlatform()(using Context) extends JavaPlatform {
2626
|| jsDefinitions.isJSFunctionClass(cls)
2727
|| jsDefinitions.isJSThisFunctionClass(cls)
2828

29+
/** Is the given class symbol eligible for Java serialization-specific methods?
30+
*
31+
* This is not simply false because we still want to add them to Scala classes
32+
* and objects. They might be transitively used by macros and other compile-time
33+
* code. It feels safer to have them be somewhat equivalent to the ones we would
34+
* get in a JVM project. The JVM back-end will slap an extends `java.io.Serializable`
35+
* to them, so we should be consistent and also emit the proper serialization methods.
36+
*/
2937
override def shouldReceiveJavaSerializationMethods(sym: ClassSymbol)(using Context): Boolean =
3038
!sym.isSubClass(jsDefinitions.JSAnyClass)
3139
}

‎compiler/src/dotty/tools/dotc/transform/sjs/PrepJSInterop.scala

Lines changed: 13 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -98,20 +98,6 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
9898
}
9999
}
100100

101-
/** Whether to check that we have proper literals in some crucial places.
102-
*
103-
* This is always true in dotc. We keep the definition so that the code
104-
* code can be as similar as possible to the scalac phase.
105-
*/
106-
private final val shouldCheckLiterals = true
107-
108-
/** Whether to check and prepare exports.
109-
*
110-
* This is always true in dotc. We keep the definition so that the code
111-
* code can be as similar as possible to the scalac phase.
112-
*/
113-
private final val shouldPrepareExports = true
114-
115101
/** DefDefs in class templates that export methods to JavaScript */
116102
private val exporters = mutable.Map.empty[Symbol, mutable.ListBuffer[Tree]]
117103

@@ -155,9 +141,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
155141

156142
tree match {
157143
case tree: TypeDef if tree.isClassDef =>
158-
if (shouldPrepareExports)
159-
registerClassOrModuleExports(sym)
160-
144+
registerClassOrModuleExports(sym)
161145
if (isJSAny(sym))
162146
transformJSClassDef(tree)
163147
else
@@ -173,10 +157,8 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
173157
* (Note that local-to-block can never have exports, but the error
174158
* messages for that are handled by genExportMember).
175159
*/
176-
if (shouldPrepareExports && (sym.is(Method) || sym.isLocalToBlock)) {
177-
exporters.getOrElseUpdate(sym.owner, mutable.ListBuffer.empty) ++=
178-
genExportMember(sym)
179-
}
160+
if (sym.is(Method) || sym.isLocalToBlock)
161+
exporters.getOrElseUpdate(sym.owner, mutable.ListBuffer.empty) ++= genExportMember(sym)
180162

181163
if (sym.isLocalToBlock)
182164
super.transform(tree)
@@ -532,7 +514,6 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
532514
} else {
533515
// The symbol can be annotated with @js.native. Now check its JS native loading spec.
534516
if (sym.is(Trait)) {
535-
assert(sym.is(Trait), sym) // just tested in the previous `if`
536517
for (annot <- sym.annotations) {
537518
val annotSym = annot.symbol
538519
if (isJSNativeLoadingSpecAnnot(annotSym))
@@ -604,8 +585,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
604585
}
605586

606587
case Some(annot) if annot.symbol == jsdefn.JSGlobalAnnot =>
607-
if (shouldCheckLiterals)
608-
checkJSGlobalLiteral(annot)
588+
checkJSGlobalLiteral(annot)
609589
val pathName = annot.argumentConstantString(0).getOrElse {
610590
if ((enclosingOwner is OwnerKind.ScalaMod) && !sym.owner.isPackageObject) {
611591
report.error(
@@ -617,8 +597,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
617597
checkGlobalRefPath(pathName)
618598

619599
case Some(annot) if annot.symbol == jsdefn.JSImportAnnot =>
620-
if (shouldCheckLiterals)
621-
checkJSImportLiteral(annot)
600+
checkJSImportLiteral(annot)
622601
annot.argumentConstantString(2).foreach { globalPathName =>
623602
checkGlobalRefPath(globalPathName)
624603
}
@@ -644,7 +623,6 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
644623
else if (sym.isJSBracketCall)
645624
report.error("@JSBracketCall is not allowed on @js.native vals and defs", annotPos(jsdefn.JSBracketCallAnnot))
646625

647-
//if (!sym.is(Accessor))
648626
checkRHSCallsJSNative(tree, "@js.native members")
649627

650628
// Check that we do not override or implement anything from a superclass
@@ -873,21 +851,18 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
873851

874852
for (annot <- allJSNameAnnots.headOption) {
875853
// Check everything about the first @JSName annotation
876-
if (sym.isLocalToBlock || (enclosingOwner isnt OwnerKind.JSType)) {
854+
if (sym.isLocalToBlock || (enclosingOwner isnt OwnerKind.JSType))
877855
report.error("@JSName can only be used on members of JS types.", annot.tree)
878-
} else if (sym.is(Trait)) {
856+
else if (sym.is(Trait))
879857
report.error("@JSName cannot be used on traits.", annot.tree)
880-
} else if (isPrivateMaybeWithin(sym)) {
858+
else if (isPrivateMaybeWithin(sym))
881859
report.error("@JSName cannot be used on private members.", annot.tree)
882-
} else {
883-
if (shouldCheckLiterals)
884-
checkJSNameArgument(sym, annot)
885-
}
860+
else
861+
checkJSNameArgument(sym, annot)
886862

887863
// Check that there is at most one @JSName annotation.
888-
for (duplicate <- allJSNameAnnots.tail) {
864+
for (duplicate <- allJSNameAnnots.tail)
889865
report.error("Duplicate @JSName annotation.", duplicate.tree)
890-
}
891866
}
892867
}
893868

@@ -924,7 +899,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
924899
* module class symbol.
925900
*/
926901
private def markExposedIfRequired(sym: Symbol)(using Context): Unit = {
927-
def shouldBeExposed: Boolean = {
902+
val shouldBeExposed: Boolean = {
928903
// it is a member of a non-native JS class
929904
(enclosingOwner is OwnerKind.JSNonNative) && !sym.isLocalToBlock &&
930905
// it is a term member, and it is not synthetic
@@ -935,7 +910,7 @@ class PrepJSInterop extends MacroTransform with IdentityDenotTransformer { thisP
935910
!sym.isConstructor //&& !sym.isValueParameter && !sym.isParamWithDefault
936911
}
937912

938-
if (shouldPrepareExports && shouldBeExposed)
913+
if (shouldBeExposed)
939914
sym.addAnnotation(jsdefn.ExposedJSMemberAnnot)
940915
}
941916
}

‎tests/neg-scalajs/jsconstructorof2.check renamed to ‎tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.check

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,40 @@
1-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:13:27 ---------------------------------------------------------------
1+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:13:27 -----------------------------------------
22
13 | val a = js.constructorOf[NativeJSTrait] // error
33
| ^^^^^^^^^^^^^
44
| non-trait class type required but NativeJSTrait found
5-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:14:27 ---------------------------------------------------------------
5+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:14:27 -----------------------------------------
66
14 | val b = js.constructorOf[NativeJSObject.type] // error
77
| ^^^^^^^^^^^^^^^^^^^
88
| NativeJSObject.type is not a class type
9-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:16:27 ---------------------------------------------------------------
9+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:16:27 -----------------------------------------
1010
16 | val c = js.constructorOf[NativeJSClass with NativeJSTrait] // error
1111
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1212
| NativeJSClass & NativeJSTrait is not a class type
13-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:17:27 ---------------------------------------------------------------
13+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:17:27 -----------------------------------------
1414
17 | val d = js.constructorOf[NativeJSClass { def bar: Int }] // error
1515
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1616
| NativeJSClass{bar: => Int} is not a class type
17-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:19:27 ---------------------------------------------------------------
17+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:19:27 -----------------------------------------
1818
19 | val e = js.constructorOf[JSTrait] // error
1919
| ^^^^^^^
2020
| non-trait class type required but JSTrait found
21-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:20:27 ---------------------------------------------------------------
21+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:20:27 -----------------------------------------
2222
20 | val f = js.constructorOf[JSObject.type] // error
2323
| ^^^^^^^^^^^^^
2424
| JSObject.type is not a class type
25-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:22:27 ---------------------------------------------------------------
25+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:22:27 -----------------------------------------
2626
22 | val g = js.constructorOf[JSClass with JSTrait] // error
2727
| ^^^^^^^^^^^^^^^^^^^^
2828
| JSClass & JSTrait is not a class type
29-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:23:27 ---------------------------------------------------------------
29+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:23:27 -----------------------------------------
3030
23 | val h = js.constructorOf[JSClass { def bar: Int }] // error
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^
3232
| JSClass{bar: => Int} is not a class type
33-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:25:42 ---------------------------------------------------------------
33+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:25:42 -----------------------------------------
3434
25 | def foo[A <: js.Any] = js.constructorOf[A] // error
3535
| ^
3636
| A is not a class type
37-
-- Error: tests/neg-scalajs/jsconstructorof2.scala:26:66 ---------------------------------------------------------------
37+
-- Error: tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala:26:66 -----------------------------------------
3838
26 | def bar[A <: js.Any: scala.reflect.ClassTag] = js.constructorOf[A] // error
3939
| ^
4040
| A is not a class type

‎tests/neg-scalajs/jsconstructorof2.scala renamed to ‎tests/neg-scalajs/jsconstructorof-error-in-prepjsinterop.scala

File renamed without changes.

‎tests/neg-scalajs/jsconstructorof1.check renamed to ‎tests/neg-scalajs/jsconstructorof-error-in-typer.check

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
-- [E057] Type Mismatch Error: tests/neg-scalajs/jsconstructorof1.scala:9:27 -------------------------------------------
1+
-- [E057] Type Mismatch Error: tests/neg-scalajs/jsconstructorof-error-in-typer.scala:9:27 -----------------------------
22
9 | val a = js.constructorOf[ScalaClass] // error
33
| ^
44
| Type argument ScalaClass does not conform to upper bound scala.scalajs.js.Any
5-
-- [E057] Type Mismatch Error: tests/neg-scalajs/jsconstructorof1.scala:10:27 ------------------------------------------
5+
-- [E057] Type Mismatch Error: tests/neg-scalajs/jsconstructorof-error-in-typer.scala:10:27 ----------------------------
66
10 | val b = js.constructorOf[ScalaTrait] // error
77
| ^
88
| Type argument ScalaTrait does not conform to upper bound scala.scalajs.js.Any
9-
-- [E057] Type Mismatch Error: tests/neg-scalajs/jsconstructorof1.scala:11:27 ------------------------------------------
9+
-- [E057] Type Mismatch Error: tests/neg-scalajs/jsconstructorof-error-in-typer.scala:11:27 ----------------------------
1010
11 | val c = js.constructorOf[ScalaObject.type] // error
1111
| ^
1212
| Type argument ScalaObject.type does not conform to upper bound scala.scalajs.js.Any

‎tests/neg-scalajs/jsconstructorof1.scala renamed to ‎tests/neg-scalajs/jsconstructorof-error-in-typer.scala

File renamed without changes.

‎tests/neg-scalajs/jsconstructortag2.check renamed to ‎tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.check

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,40 @@
1-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:13:42 --------------------------------------------------------------
1+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:13:42 ----------------------------------------
22
13 | val a = js.constructorTag[NativeJSTrait] // error
33
| ^
44
| non-trait class type required but NativeJSTrait found
5-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:14:48 --------------------------------------------------------------
5+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:14:48 ----------------------------------------
66
14 | val b = js.constructorTag[NativeJSObject.type] // error
77
| ^
88
| NativeJSObject.type is not a class type
9-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:16:61 --------------------------------------------------------------
9+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:16:61 ----------------------------------------
1010
16 | val c = js.constructorTag[NativeJSClass with NativeJSTrait] // error
1111
| ^
1212
| (NativeJSClass & NativeJSTrait) is not a class type
13-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:17:59 --------------------------------------------------------------
13+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:17:59 ----------------------------------------
1414
17 | val d = js.constructorTag[NativeJSClass { def bar: Int }] // error
1515
| ^
1616
| NativeJSClass{bar: => Int} is not a class type
17-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:19:36 --------------------------------------------------------------
17+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:19:36 ----------------------------------------
1818
19 | val e = js.constructorTag[JSTrait] // error
1919
| ^
2020
| non-trait class type required but JSTrait found
21-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:20:42 --------------------------------------------------------------
21+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:20:42 ----------------------------------------
2222
20 | val f = js.constructorTag[JSObject.type] // error
2323
| ^
2424
| JSObject.type is not a class type
25-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:22:49 --------------------------------------------------------------
25+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:22:49 ----------------------------------------
2626
22 | val g = js.constructorTag[JSClass with JSTrait] // error
2727
| ^
2828
| (JSClass & JSTrait) is not a class type
29-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:23:53 --------------------------------------------------------------
29+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:23:53 ----------------------------------------
3030
23 | val h = js.constructorTag[JSClass { def bar: Int }] // error
3131
| ^
3232
| JSClass{bar: => Int} is not a class type
33-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:25:45 --------------------------------------------------------------
33+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:25:45 ----------------------------------------
3434
25 | def foo[A <: js.Any] = js.constructorTag[A] // error
3535
| ^
3636
| A is not a class type
37-
-- Error: tests/neg-scalajs/jsconstructortag2.scala:26:69 --------------------------------------------------------------
37+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala:26:69 ----------------------------------------
3838
26 | def bar[A <: js.Any: scala.reflect.ClassTag] = js.constructorTag[A] // error
3939
| ^
4040
| A is not a class type

‎tests/neg-scalajs/jsconstructortag2.scala renamed to ‎tests/neg-scalajs/jsconstructortag-error-in-prepjsinterop.scala

File renamed without changes.

‎tests/neg-scalajs/jsconstructortag1.check renamed to ‎tests/neg-scalajs/jsconstructortag-error-in-typer.check

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
-- Error: tests/neg-scalajs/jsconstructortag1.scala:9:39 ---------------------------------------------------------------
1+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-typer.scala:9:39 -------------------------------------------------
22
9 | val a = js.constructorTag[ScalaClass] // error
33
| ^
44
|no implicit argument of type scala.scalajs.js.ConstructorTag[ScalaClass] was found for parameter tag of method constructorTag in package scala.scalajs.js.
@@ -7,7 +7,7 @@
77
| scala.scalajs.js.ConstructorTag.materialize[Nothing]
88
|
99
|But method materialize in object ConstructorTag does not match type scala.scalajs.js.ConstructorTag[ScalaClass].
10-
-- Error: tests/neg-scalajs/jsconstructortag1.scala:10:39 --------------------------------------------------------------
10+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-typer.scala:10:39 ------------------------------------------------
1111
10 | val b = js.constructorTag[ScalaTrait] // error
1212
| ^
1313
|no implicit argument of type scala.scalajs.js.ConstructorTag[ScalaTrait] was found for parameter tag of method constructorTag in package scala.scalajs.js.
@@ -16,7 +16,7 @@
1616
| scala.scalajs.js.ConstructorTag.materialize[Nothing]
1717
|
1818
|But method materialize in object ConstructorTag does not match type scala.scalajs.js.ConstructorTag[ScalaTrait].
19-
-- Error: tests/neg-scalajs/jsconstructortag1.scala:11:45 --------------------------------------------------------------
19+
-- Error: tests/neg-scalajs/jsconstructortag-error-in-typer.scala:11:45 ------------------------------------------------
2020
11 | val c = js.constructorTag[ScalaObject.type] // error
2121
| ^
2222
|no implicit argument of type scala.scalajs.js.ConstructorTag[ScalaObject.type] was found for parameter tag of method constructorTag in package scala.scalajs.js.

‎tests/neg-scalajs/jsconstructortag1.scala renamed to ‎tests/neg-scalajs/jsconstructortag-error-in-typer.scala

File renamed without changes.

0 commit comments

Comments
 (0)