Skip to content

Commit

Permalink
Fix sbt#355: Handle malformed class name
Browse files Browse the repository at this point in the history
The binary name encoding for Scala has been broken for a while. It does
not follow the Java source name rules because it does not append a `$`
after the mirror class of an object, which already ends in `$`.

Zinc works around this fix in the only way it can: it gets the enclosing
class, which is obtained from the `InnerClasses` java classfile
metadata, and it reconstructs the path from there. It circumvents, by
design, the core issue: it doesn't call Java's name parser at all.

This issue has been fixed in the JDK9, but unfortunately it's going to
be tormenting us for a while because 2.13 does not target Java 9, and
we're still at 2.12.x.

This is the fix in JDK9:
http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8057919

In order to fully fix this issue, the Scala compiler should do the
desired behaviour in 2.13.x. 2.12.x is already lost since this fix is
binary incompatible. The appropriate JIRA ticket is here:
scala/bug#2034.
  • Loading branch information
jvican committed Jul 24, 2017
1 parent 19d54fe commit b040f0b
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,50 @@ object ClassToAPI {
new mutable.HashSet,
new mutable.HashSet)

def classCanonicalName(c: Class[_]): String =
Option(c.getCanonicalName) getOrElse { c.getName }
/**
* The binary name encoding for Scala has been broken for a while. It does not
* follow the Java source name rules because it does not append a `$` after the
* mirror class of an object, which already ends in `$`.
*
* Zinc works around this fix in the only way it can: it gets the enclosing class,
* which is obtained from the `InnerClasses` java classfile metadata, and it
* reconstructs the path from there. It circumvents, by design, the core issue:
* it doesn't call Java's name parser at all.
*
* This issue has been fixed in the JDK9, but unfortunately it's going to be tormenting
* us for a while because 2.13 does not target Java 9, and we're still at 2.12.x.
*
* <a href="http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8057919">This is the fix in Java 9.</a>
*
* In order to fully fix this issue, the Scala compiler should do the desired behaviour
* in 2.13.x. 2.12.x is already lost since this fix is binary incompatible. The appropriate
* JIRA ticket is here: https://github.com/scala/bug/issues/2034.
*
* @return The canonical name if not null, the full name otherwise.
*/
def handleMalformedNameOf(c: Class[_], isRecursive: Boolean = false): String = {
if (c == null) "" // Return nothing if it hits the top-level class
else {
val className = c.getName
// Adds a `dollar` if it's a recursive call or if the original class is an object
val atEnd: String =
if (isRecursive) "$" else if (className.endsWith("$")) "$" else ""
try {
val canonicalName = c.getCanonicalName
if (canonicalName == null) className
else canonicalName + atEnd
} catch {
case malformedError: java.lang.InternalError
if malformedError.getMessage.contains("Malformed class name") =>
val enclosingClass = c.getEnclosingClass
val enclosingName = enclosingClass.getName
val restOfName = c.getName.stripPrefix(enclosingName)
handleMalformedNameOf(enclosingClass, true) + restOfName + atEnd
}
}
}

def classCanonicalName(c: Class[_]): String = handleMalformedNameOf(c)

def toDefinitions(cmap: ClassMap)(c: Class[_]): Seq[api.ClassLikeDef] =
cmap.memo.getOrElseUpdate(classCanonicalName(c), toDefinitions0(c, cmap))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,48 @@
package repro

abstract class Boo {
// Pretend use of `Boo`'s companion
val b = Boo
}

// Tests simple class in nested objects
object Boo {
object Foo {
class Impl
}
}

abstract class Boo2 {
val b2 = Boo2
}

// Tests simple class in object, special-cased (by Scala)
// It does and should not trigger malformed class name.
object Boo2 {
class Impl
}

abstract class Boo3 {
val b3 = Boo3
}

// Tests three nested object + class
object Boo3 {
object Foo2 {
object Bar {
class Impl
}
}
}

abstract class Boo4 {
val b4 = Boo4
}

// Tests nested classes inside nested objects
object Boo4 {
object Foo3 {
class BarClass {
class Impl
}
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,32 @@
package repro;

import repro.Boo;
import repro.Boo2;
import repro.Boo3;
import repro.Boo4;

public class BooUser {
private static Boo getOwnBuf() {
return new Boo() {};
}

private static Boo2 getOwnBuf2() {
return new Boo2() {};
}

private static Boo3 getOwnBuf3() {
return new Boo3() {};
}

private static Boo4 getOwnBuf4() {
return new Boo4() {};
}

public static void main(String[] args) {
Boo boo = getOwnBuf();
Boo2 boo2 = getOwnBuf2();
Boo3 boo3 = getOwnBuf3();
Boo4 boo4 = getOwnBuf4();
System.out.println("Made a buf: " + boo);
}
}

0 comments on commit b040f0b

Please sign in to comment.