Skip to content

Commit 13a6266

Browse files
pjfanningclaude
andcommitted
fix(scala): decline, rather than fail, types with unresolvable members
Addresses the fifth AI review round, where both reviewers independently found the same gap. Guarding only `Class.forName` was not enough to keep recognition free of linkage failures. `getField` and `getMethods` resolve member descriptors, so a companion or case class declaring a member whose type is absent at runtime threw a `NoClassDefFoundError` out of `isCaseClass` for a type this module does not own. Recognition now declines such a type; the owning path still reports it. Ambiguity stays loud, and the comment says so rather than promising blanket quiet. `CompanionOwner` keeps one piece of state: static forwarders are exactly the absence of a companion singleton. The literal-name comment named the wrong mechanism again. Verified against the compiled classes: a one-level companion is enclosed by the mirror class, so its canonical name ends with its only `$` and resolves; two or more levels put a `$`-terminated segment in the middle, which the generated-code compiler cannot resolve through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a86a870 commit 13a6266

4 files changed

Lines changed: 35 additions & 27 deletions

File tree

docs/json/scala.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ constructor arguments exactly as Scala defines them. A missing parameter without
5151
error. Mutable body properties are applied after construction.
5252

5353
A case class may be declared at the top level, or inside an `object` at any nesting depth, as long
54-
as every enclosing scope is itself an `object`. A case class enclosed by a `class`, a trait, or a method
55-
is rejected for both reading and writing, because Fory cannot reach the enclosing instance or the
56-
companion it needs to rebuild the value.
54+
as every enclosing scope is itself an `object`. A case class enclosed by a `class`, a trait, or a
55+
method is rejected for both reading and writing, because Fory cannot reach the enclosing instance
56+
or the companion it needs to rebuild the value.
5757

5858
Fory JSON annotations can be placed directly on Scala constructor properties:
5959

java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -633,11 +633,11 @@ public static String getLiteralName(Class<?> cls) {
633633
if (canonicalName.contains("$")) {
634634
// nested scala object type can't be accessed in java by using canonicalName. This includes
635635
// a nested module class, whose own name ends with `$`: the canonical name of a companion
636-
// declared two or more levels inside an object mixes `.` and `$` separators, which the
637-
// generated-code compiler mangles back into a name that resolves to nothing
638-
// ("pkg.A$B$ declares no member type C$"). A module class only one level deep keeps its
639-
// canonical name, because only its last segment ends with `$`, so the nesting-level bound
640-
// below is load bearing in both directions.
636+
// declared two or more levels inside an object has a `$`-terminated segment in the
637+
// middle of its canonical name, one per enclosing module class, and the generated-code
638+
// compiler cannot resolve through those ("pkg.A$B$ declares no member type C$"). One level
639+
// deep is enclosed by the mirror class instead, so `pkg.A.C$` ends with the only `$` and
640+
// still resolves: the nesting-level bound below is load bearing in both directions.
641641
// see more detailed in
642642
// https://stackoverflow.com/questions/30809070/accessing-scala-nested-classes-from-java
643643
int nestedLevels = 0;

java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ final class ForyJsonGraalVMFeature implements Feature {
117117
// Reflection-only registrations are tracked apart from processedCreators, whose membership also
118118
// means a creator handle was retained.
119119
private final Set<Method> processedReflectiveMethods = new LinkedHashSet<>();
120-
private final Set<Class<?>> scalaCompanionsConsidered = new LinkedHashSet<>();
120+
private final Set<Class<?>> typesConsideredForScalaCompanion = new LinkedHashSet<>();
121121
private final Set<ObjectCodec<?>> processedObjectModels =
122122
Collections.newSetFromMap(new IdentityHashMap<>());
123123
private final ArrayList<HostedConfiguration> hostedConfigurations = new ArrayList<>();
@@ -880,7 +880,7 @@ private void registerRecord(Class<?> type) {
880880
* reflectively while rebuilding the object model at image runtime.
881881
*/
882882
private void registerScalaCompanion(Class<?> type) {
883-
if (!scalaCompanionsConsidered.add(type) || hasScalaStaticFactory(type)) {
883+
if (!typesConsideredForScalaCompanion.add(type) || hasScalaStaticFactory(type)) {
884884
return;
885885
}
886886
Class<?> companion;

scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala

Lines changed: 25 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,16 +32,26 @@ private[scala] object ScalaObjectModels {
3232
if (!classOf[Product].isAssignableFrom(typeClass) || name.startsWith("scala.Tuple")) {
3333
return false
3434
}
35-
val companion = companionOwner(typeClass, committed = false)
36-
if (companion != null) return findPrimaryConstructor(typeClass, companion) != null
37-
// A case class that cannot reach its companion, such as one declared inside a class or a
38-
// method, is still a case class. Claim it so the codec reports the exact reason instead of
39-
// leaving it to a generic object model that silently drops every property. A generated `copy`
40-
// returning the declaring class together with a declared `productPrefix`, which `Product`
41-
// otherwise supplies by default, is the compiler marker of a case class. Standard-library
42-
// types keep their own mapping. A reachable companion whose constructor this module does not
43-
// support, such as a varargs or non-public primary constructor, keeps its previous handling.
44-
!name.startsWith("scala.") && declaresCopy(typeClass) && declaresProductPrefix(typeClass)
35+
// Recognition answers a predicate for every Product reaching this module, including types it
36+
// does not own, and reflecting over a companion or a case class resolves member descriptors.
37+
// A type whose members reference absent classes must simply be declined here; the owning path
38+
// reports the failure. Ambiguity stays loud: it means this module does own the type and cannot
39+
// pick a constructor.
40+
try {
41+
val companion = companionOwner(typeClass, committed = false)
42+
if (companion != null) findPrimaryConstructor(typeClass, companion) != null
43+
else {
44+
// A case class that cannot reach its companion, such as one declared inside a class or a
45+
// method, is still a case class. Claim it so the codec reports the exact reason instead of
46+
// leaving it to a generic object model that silently drops every property. A generated
47+
// `copy` returning the declaring class together with a declared `productPrefix`, which
48+
// `Product` otherwise supplies by default, is the compiler marker of a case class.
49+
// Standard-library types keep their own mapping. A reachable companion whose constructor
50+
// this module does not support, such as a varargs or non-public primary constructor, keeps
51+
// its previous handling.
52+
!name.startsWith("scala.") && declaresCopy(typeClass) && declaresProductPrefix(typeClass)
53+
}
54+
} catch { case _: LinkageError => false }
4555
}
4656

4757
def caseClassCodec(typeRef: TypeRef[_], resolver: JsonTypeResolver): ObjectCodec[_] = {
@@ -227,11 +237,9 @@ private[scala] object ScalaObjectModels {
227237
* only for a top-level companion, so a case class declared inside an `object` keeps them as
228238
* instance members of the companion singleton.
229239
*/
230-
private final class CompanionOwner(
231-
val owner: Class[_],
232-
val singleton: Field,
233-
val staticForwarders: Boolean
234-
)
240+
private final class CompanionOwner(val owner: Class[_], val singleton: Field) {
241+
def staticForwarders: Boolean = singleton == null
242+
}
235243

236244
// `fory-json` mirrors this companion rule in two places that must stay in sync: the
237245
// `ownerType + "$"` check in JsonCreatorInfo.buildDefaultInvokers, and the native-image
@@ -250,7 +258,7 @@ private[scala] object ScalaObjectModels {
250258
if (
251259
method.getName == "apply" && Modifier.isStatic(method.getModifiers) &&
252260
!method.isBridge && !method.isSynthetic && method.getReturnType == typeClass
253-
) return new CompanionOwner(typeClass, null, true)
261+
) return new CompanionOwner(typeClass, null)
254262
index += 1
255263
}
256264
val companionName = typeClass.getName + "$"
@@ -267,7 +275,7 @@ private[scala] object ScalaObjectModels {
267275
}
268276
val field = singletonField(companionClass)
269277
if (!Modifier.isPublic(companionClass.getModifiers) || field == null) null
270-
else new CompanionOwner(companionClass, field, false)
278+
else new CompanionOwner(companionClass, field)
271279
}
272280

273281
private def companionInstance(typeRef: TypeRef[_], companion: CompanionOwner): AnyRef = {

0 commit comments

Comments
 (0)