From e493fdb869c3418f035258ff6a89bd68737a515a Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 30 Aug 2026 18:59:01 +0100 Subject: [PATCH 1/5] fix(scala): support case classes declared inside objects Scala 2 emits the `apply` and constructor-default static forwarders on a case class only for a top-level companion, so `fory-json-scala` did not recognize a case class declared inside an `object`. Such a type fell through to the generic Java object model, which discovered its fields for writing but had no creator for reading, so every property silently decoded to its default. Scala 3 emits those forwarders for any statically owned module and was unaffected. Resolve `apply` and `$lessinit$greater$default$N` from the companion singleton when the case class carries no static forwarders, keeping the static path as the fast path. `JsonObjectModel` now carries the receiver of instance constructor defaults, `JsonCreatorInfo` binds it into the default invoker, and both reader codegen paths invoke the default on that receiver. Reject, instead of silently decoding, a case class that cannot be reconstructed: one declared inside a class, which needs an outer instance, and one declared inside a method, whose companion is not reachable. Co-Authored-By: Claude Opus 5 (1M context) --- docs/json/scala.md | 4 + .../fory/json/codec/JsonObjectModel.java | 60 ++++++++++++ .../fory/json/codec/ObjectCodecBuilder.java | 1 + .../fory/json/codegen/JsonReaderCodegen.java | 44 ++++++--- .../fory/json/meta/JsonCreatorInfo.java | 33 +++++-- .../fory/json/ForyJsonGraalVMFeature.java | 9 +- .../scala/internal/ScalaObjectModels.scala | 93 ++++++++++++++++--- .../fory/json/scala/ScalaJsonSuite.scala | 39 ++++++++ 8 files changed, 252 insertions(+), 31 deletions(-) diff --git a/docs/json/scala.md b/docs/json/scala.md index a988157bff..cfc7fdc819 100644 --- a/docs/json/scala.md +++ b/docs/json/scala.md @@ -50,6 +50,10 @@ or mutate constructor `val` fields. Defaults in later parameter lists receive th constructor arguments exactly as Scala defines them. A missing parameter without a default is an error. Mutable body properties are applied after construction. +A case class may be declared at the top level or inside an `object`, at any nesting depth. A case +class declared inside a `class` or inside a method is rejected, because Fory cannot reach the +enclosing instance or companion it needs to rebuild the value. + Fory JSON annotations can be placed directly on Scala constructor properties: ```scala diff --git a/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java b/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java index 64c2422b86..871335deeb 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java @@ -42,6 +42,7 @@ public final class JsonObjectModel { private final String[] parameterNames; private final Method[] accessors; private final Method[] defaultMethods; + private final Object defaultsReceiver; private final int[] defaultMaskBits; private final boolean[] parameterNullable; private final TypeRef[] parameterTypes; @@ -68,6 +69,43 @@ public JsonObjectModel( Method[] propertyGetters, Method[] propertySetters, TypeRef[] propertyTypes) { + this( + constructor, + defaultConstructor, + parameterNames, + accessors, + defaultMethods, + null, + defaultMaskBits, + parameterNullable, + parameterTypes, + propertyNames, + propertyGetters, + propertySetters, + propertyTypes); + } + + /** + * Creates one ordinary language object model whose constructor defaults are instance methods on + * {@code defaultsReceiver}. Scala emits {@code $lessinit$greater$default$N} on the companion + * singleton and mirrors it as a static forwarder on the case class only for a top-level + * companion, so a case class declared inside an {@code object} binds its defaults on that + * singleton. Pass {@code null} when the defaults are static members of the created type. + */ + public JsonObjectModel( + Constructor constructor, + Constructor defaultConstructor, + String[] parameterNames, + Method[] accessors, + Method[] defaultMethods, + Object defaultsReceiver, + int[] defaultMaskBits, + boolean[] parameterNullable, + TypeRef[] parameterTypes, + String[] propertyNames, + Method[] propertyGetters, + Method[] propertySetters, + TypeRef[] propertyTypes) { this( (Executable) constructor, constructor, @@ -75,6 +113,7 @@ public JsonObjectModel( parameterNames, accessors, defaultMethods, + defaultsReceiver, defaultMaskBits, parameterNullable, parameterTypes, @@ -108,6 +147,7 @@ public JsonObjectModel( parameterNames, accessors, defaultMethods, + null, defaultMaskBits, parameterNullable, parameterTypes, @@ -127,6 +167,7 @@ public JsonObjectModel( String[] parameterNames, Method[] accessors, Method[] defaultMethods, + Object defaultsReceiver, int[] defaultMaskBits, boolean[] parameterNullable, TypeRef[] parameterTypes, @@ -142,6 +183,7 @@ public JsonObjectModel( this.parameterNames = parameterNames.clone(); this.accessors = accessors.clone(); this.defaultMethods = defaultMethods.clone(); + this.defaultsReceiver = defaultsReceiver; this.defaultMaskBits = defaultMaskBits.clone(); this.parameterNullable = parameterNullable.clone(); this.parameterTypes = parameterTypes.clone(); @@ -169,6 +211,7 @@ private JsonObjectModel( parameterNames = new String[0]; accessors = new Method[0]; defaultMethods = new Method[0]; + defaultsReceiver = null; defaultMaskBits = new int[0]; parameterNullable = new boolean[0]; parameterTypes = new TypeRef[0]; @@ -303,6 +346,18 @@ private void validate() { if (defaultMethods[i] != null && defaultMaskBits[i] >= 0) { throw new IllegalArgumentException("A constructor parameter has two default mechanisms"); } + if (defaultMethods[i] != null + && Modifier.isStatic(defaultMethods[i].getModifiers()) == (defaultsReceiver != null)) { + throw new IllegalArgumentException( + "A JSON constructor default receiver is required exactly for instance defaults " + + defaultMethods[i]); + } + if (defaultsReceiver != null + && defaultMethods[i] != null + && !defaultMethods[i].getDeclaringClass().isInstance(defaultsReceiver)) { + throw new IllegalArgumentException( + "JSON constructor default receiver does not own " + defaultMethods[i]); + } } names.clear(); for (int i = 0; i < propertyNames.length; i++) { @@ -390,6 +445,11 @@ public Method[] defaultMethods() { return defaultMethods.clone(); } + /** Returns the receiver of instance constructor-default methods, or null when they are static. */ + public Object defaultsReceiver() { + return defaultsReceiver; + } + public int[] defaultMaskBits() { return defaultMaskBits.clone(); } diff --git a/java/fory-json/src/main/java/org/apache/fory/json/codec/ObjectCodecBuilder.java b/java/fory-json/src/main/java/org/apache/fory/json/codec/ObjectCodecBuilder.java index eba59e9e3f..0a8120e6d9 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/codec/ObjectCodecBuilder.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/codec/ObjectCodecBuilder.java @@ -1720,6 +1720,7 @@ private static JsonCreatorInfo buildObjectModelCreatorInfo( creatorDefaults(rawTypes), generatedCodec, defaultMethods, + objectModel.defaultsReceiver(), names, objectModel.defaultConstructor(), defaultMaskBits, diff --git a/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java b/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java index 1f150c290a..fb131d02ef 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java @@ -1307,14 +1307,30 @@ private Expression creatorDefaultValue( inputs[i] = new Expression.Cast(arguments.values[i], TypeRef.of(dependencies[i])).inline(); } Expression value = - new Expression.StaticInvoke( - method.getDeclaringClass(), - method.getName(), - TypeRef.of(method.getReturnType()), - inputs); + Modifier.isStatic(method.getModifiers()) + ? new Expression.StaticInvoke( + method.getDeclaringClass(), + method.getName(), + TypeRef.of(method.getReturnType()), + inputs) + : new Expression.Invoke( + defaultsReceiver(method), + method.getName(), + TypeRef.of(method.getReturnType()), + inputs); return new Expression.Cast(value, TypeRef.of(parameterType)); } + /** Reads the language singleton that owns instance constructor defaults, such as a companion. */ + private Expression defaultsReceiver(Method method) { + return new Expression.Cast( + new Expression.Invoke( + fieldRef("creator", JsonCreatorInfo.class), + "defaultsReceiver", + TypeRef.of(Object.class)), + TypeRef.of(method.getDeclaringClass())); + } + private Expression finishCreator( JsonGeneratedCodecBuilder builder, Class type, @@ -1485,13 +1501,17 @@ private void appendWorkspaceDefaults( .append(";\n"); } } else { - body.append("arguments[") - .append(i) - .append("] = ") - .append(ctx.type(method.getDeclaringClass())) - .append('.') - .append(method.getName()) - .append('('); + body.append("arguments[").append(i).append("] = "); + if (Modifier.isStatic(method.getModifiers())) { + body.append(ctx.type(method.getDeclaringClass())); + } else { + body.append("((") + .append(ctx.type(method.getDeclaringClass())) + .append(") ") + .append(creatorExpression) + .append(".defaultsReceiver())"); + } + body.append('.').append(method.getName()).append('('); Class[] dependencies = method.getParameterTypes(); for (int j = 0; j < dependencies.length; j++) { if (j != 0) { diff --git a/java/fory-json/src/main/java/org/apache/fory/json/meta/JsonCreatorInfo.java b/java/fory-json/src/main/java/org/apache/fory/json/meta/JsonCreatorInfo.java index 06ae336461..f93cfea9f6 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/meta/JsonCreatorInfo.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/meta/JsonCreatorInfo.java @@ -67,6 +67,7 @@ public final class JsonCreatorInfo { private final MethodHandle invoker; private final GeneratedJsonCodec generatedCodec; private final Method[] defaultMethods; + private final Object defaultsReceiver; private final MethodHandle[] defaultInvokers; private final Constructor defaultConstructor; private final MethodHandle defaultConstructorInvoker; @@ -97,6 +98,7 @@ public JsonCreatorInfo( null, null, null, + null, null); } @@ -109,6 +111,7 @@ public JsonCreatorInfo( Object[] defaults, GeneratedJsonCodec generatedCodec, Method[] defaultMethods, + Object defaultsReceiver, String[] parameterNames, Constructor defaultConstructor, int[] defaultMaskBits, @@ -121,6 +124,7 @@ public JsonCreatorInfo( defaults, generatedCodec, defaultMethods, + defaultsReceiver, parameterNames, defaultConstructor, defaultMaskBits, @@ -142,6 +146,7 @@ public static JsonCreatorInfo fixedInstance(Class ownerType, Object instance) null, null, null, + null, instance); } @@ -159,6 +164,7 @@ private JsonCreatorInfo( Object[] defaults, GeneratedJsonCodec generatedCodec, Method[] defaultMethods, + Object defaultsReceiver, String[] parameterNames, Constructor defaultConstructor, int[] defaultMaskBits, @@ -178,10 +184,11 @@ private JsonCreatorInfo( this.fixedInstance = fixedInstance; this.parameterNames = parameterNames == null ? null : parameterNames.clone(); this.defaultMethods = defaultMethods == null ? null : defaultMethods.clone(); + this.defaultsReceiver = defaultsReceiver; defaultInvokers = this.defaultMethods == null ? null - : buildDefaultInvokers(ownerType, executable, this.defaultMethods); + : buildDefaultInvokers(ownerType, executable, this.defaultMethods, defaultsReceiver); defaultConstructorInvoker = defaultConstructor == null ? null @@ -210,6 +217,7 @@ private JsonCreatorInfo( defaults = source.defaults; generatedCodec = source.generatedCodec; defaultMethods = source.defaultMethods; + defaultsReceiver = source.defaultsReceiver; defaultInvokers = source.defaultInvokers; defaultConstructor = source.defaultConstructor; defaultConstructorInvoker = source.defaultConstructorInvoker; @@ -409,6 +417,12 @@ public Method defaultMethod(int index) { return defaultMethods == null ? null : defaultMethods[index]; } + /** Returns the receiver of instance constructor defaults, or null when they are static. */ + @Internal + public Object defaultsReceiver() { + return defaultsReceiver; + } + /** Evaluates one prevalidated language-defined constructor default. */ @Internal public Object defaultValue(int index, Object[] arguments) { @@ -596,7 +610,7 @@ private void applyDeferred(Object value, Object[] arguments) { } private static MethodHandle[] buildDefaultInvokers( - Class ownerType, Executable executable, Method[] defaultMethods) { + Class ownerType, Executable executable, Method[] defaultMethods, Object defaultsReceiver) { if (defaultMethods.length != executable.getParameterCount()) { throw new ForyJsonException("Constructor default count does not match " + executable); } @@ -607,8 +621,13 @@ private static MethodHandle[] buildDefaultInvokers( if (method == null) { continue; } - if ((method.getDeclaringClass() != ownerType - || !java.lang.reflect.Modifier.isStatic(method.getModifiers())) + // A default is either a static member of the created type or an instance member of the + // language singleton that owns it, such as a Scala companion of a nested case class. + boolean instanceDefault = !java.lang.reflect.Modifier.isStatic(method.getModifiers()); + Class declaringClass = method.getDeclaringClass(); + if ((instanceDefault + ? defaultsReceiver == null || !declaringClass.isInstance(defaultsReceiver) + : defaultsReceiver != null || declaringClass != ownerType) || !method.getName().equals("$lessinit$greater$default$" + (i + 1)) || method.getParameterCount() > i || !java.lang.reflect.Modifier.isPublic(method.getModifiers()) @@ -622,8 +641,10 @@ private static MethodHandle[] buildDefaultInvokers( } } try { - MethodHandle target = - _JDKAccess._trustedLookup(method.getDeclaringClass()).unreflect(method); + MethodHandle target = _JDKAccess._trustedLookup(declaringClass).unreflect(method); + if (instanceDefault) { + target = target.bindTo(defaultsReceiver); + } invokers[i] = workspaceInvoker(target, dependencyTypes); } catch (IllegalAccessException e) { throw new ForyJsonException("Cannot access JSON constructor default " + method, e); diff --git a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java index 69f47379bc..a5d445d4ba 100644 --- a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java +++ b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java @@ -437,8 +437,15 @@ private void registerObjectModel(DuringAnalysisAccess access, ObjectCodec obj } for (int i = 0; i < creator.argumentCount(); i++) { Method defaultMethod = creator.defaultMethod(i); - if (defaultMethod != null) { + if (defaultMethod == null) { + continue; + } + if (Modifier.isStatic(defaultMethod.getModifiers())) { registerCreator(defaultMethod); + } else if (processedCreators.add(defaultMethod)) { + // An instance default is invoked on the language singleton that owns it, so its bound + // invoker already lives in the creator metadata. Only reflection access is needed. + RuntimeReflection.register(defaultMethod); } } } diff --git a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala index 71cb62dbdb..7953bceb0c 100644 --- a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala +++ b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala @@ -28,15 +28,35 @@ import org.apache.fory.reflect.TypeRef private[scala] object ScalaObjectModels { def isCaseClass(typeClass: Class[_]): Boolean = { - if (!classOf[Product].isAssignableFrom(typeClass) || typeClass.getName.startsWith("scala.Tuple")) { + val name = typeClass.getName + if (!classOf[Product].isAssignableFrom(typeClass) || name.startsWith("scala.Tuple")) { return false } - findPrimaryConstructor(typeClass) != null + val companion = companionOwner(typeClass) + if (companion != null && findPrimaryConstructor(typeClass, companion) != null) return true + // A case class that cannot reach its companion, such as one declared inside a class or a + // method, is still a case class. Claim it so the codec reports the exact reason instead of + // leaving it to a generic object model that silently drops every property. `copy` returning + // the declaring class is the compiler marker; standard-library types keep their own mapping. + !name.startsWith("scala.") && copyMethod(typeClass) != null } def caseClassCodec(typeRef: TypeRef[_], resolver: JsonTypeResolver): ObjectCodec[_] = { val typeClass = typeRef.getRawType - val constructor = findPrimaryConstructor(typeClass) + if (outerField(typeClass) != null) { + throw ScalaTypeSupport.unsupported( + typeRef, + "case class declared inside a class cannot be reconstructed without its outer instance" + ) + } + val companion = companionOwner(typeClass) + if (companion == null) { + throw ScalaTypeSupport.unsupported( + typeRef, + "case class companion is not reachable, such as a case class declared in a method" + ) + } + val constructor = findPrimaryConstructor(typeClass, companion) if (constructor == null) { throw ScalaTypeSupport.unsupported(typeRef, "case class has no supported public primary constructor") } @@ -78,7 +98,7 @@ private[scala] object ScalaObjectModels { // Constructor properties and their accessors are one logical occurrence. In particular, // @JsonEnumeration binds an erased Enumeration.Value parameter to the exact MODULE$ owner. val logicalParameterTypes = propertyTypes.take(names.length) - val defaults = constructorDefaults(typeClass, parameterTypes) + val defaults = constructorDefaults(typeClass, companion, parameterTypes) resolver.createObjectCodec( typeRef, new JsonObjectModel( @@ -87,6 +107,7 @@ private[scala] object ScalaObjectModels { names, accessors, defaults, + companion.receiver, Array.fill(names.length)(-1), Array.fill(names.length)(true), logicalParameterTypes, @@ -165,6 +186,21 @@ private[scala] object ScalaObjectModels { } } + private def copyMethod(typeClass: Class[_]): Method = { + typeClass.getMethods + .find(method => + method.getName == "copy" && !Modifier.isStatic(method.getModifiers) && + !method.isBridge && !method.isSynthetic && method.getReturnType == typeClass + ) + .orNull + } + + private def outerField(typeClass: Class[_]): Field = { + typeClass.getDeclaredFields + .find(field => field.getName == "$outer" && !Modifier.isStatic(field.getModifiers)) + .orNull + } + private def productFields(typeClass: Class[_]): Array[Field] = { typeClass.getDeclaredFields.filter { field => val modifiers = field.getModifiers @@ -172,17 +208,47 @@ private[scala] object ScalaObjectModels { } } - private def findPrimaryConstructor(typeClass: Class[_]): Constructor[_] = { - val constructors = typeClass.getConstructors + /** + * Owner of the compiler-generated `apply` and `$lessinit$greater$default$N` members of a case + * class. Scala mirrors those companion members as static forwarders on the case class itself + * only for a top-level companion, so a case class declared inside an `object` keeps them as + * instance members of the companion singleton. `receiver` is null for the static form. + */ + private final class CompanionOwner(val owner: Class[_], val receiver: AnyRef) + + private def companionOwner(typeClass: Class[_]): CompanionOwner = { val methods = typeClass.getMethods + var index = 0 + while (index < methods.length) { + val method = methods(index) + if ( + method.getName == "apply" && Modifier.isStatic(method.getModifiers) && + method.getReturnType == typeClass + ) return new CompanionOwner(typeClass, null) + index += 1 + } + val companionClass = + try Class.forName(typeClass.getName + "$", false, typeClass.getClassLoader) + catch { case _: ClassNotFoundException | _: LinkageError => return null } + val field = singletonField(companionClass) + if (field == null) null else new CompanionOwner(companionClass, field.get(null)) + } + + private def findPrimaryConstructor( + typeClass: Class[_], + companion: CompanionOwner + ): Constructor[_] = { + val constructors = typeClass.getConstructors + val methods = companion.owner.getMethods + val staticApply = companion.receiver == null var selected: Constructor[_] = null var index = 0 while (index < constructors.length) { val constructor = constructors(index) val parameterTypes = constructor.getParameterTypes val matchingApply = methods.exists { method => - method.getName == "apply" && Modifier.isStatic(method.getModifiers) && !method.isBridge && - !method.isSynthetic && method.getReturnType == typeClass && + method.getName == "apply" && Modifier.isStatic(method.getModifiers) == staticApply && + !method.isBridge && !method.isSynthetic && method.getReturnType == typeClass && sameTypes(method.getParameterTypes, parameterTypes) } if (!constructor.isSynthetic && !constructor.isVarArgs && matchingApply) { @@ -237,18 +303,21 @@ private[scala] object ScalaObjectModels { private def constructorDefaults( typeClass: Class[_], + companion: CompanionOwner, parameterTypes: Array[Class[_]] ): Array[Method] = { - // Scala emits constructor-default forwarders on the case-class owner. Using those exact - // methods keeps construction metadata owner-bound and avoids loading the companion singleton. + // Constructor defaults live on the same owner as `apply`: static forwarders on the case-class + // owner for a top-level companion, otherwise instance members of the companion singleton. // A default in a later parameter list receives the preceding parameter lists as arguments. val defaults = new Array[Method](parameterTypes.length) + val staticDefault = companion.receiver == null var index = 0 while (index < defaults.length) { val name = "$lessinit$greater$default$" + (index + 1) - val candidates = typeClass.getMethods.filter { method => + val candidates = companion.owner.getMethods.filter { method => val modifiers = method.getModifiers - method.getName == name && Modifier.isPublic(modifiers) && Modifier.isStatic(modifiers) && + method.getName == name && Modifier.isPublic(modifiers) && + Modifier.isStatic(modifiers) == staticDefault && method.getParameterCount <= index && compatibleDefaultParameters(method.getParameterTypes, parameterTypes) && compatibleDefaultResult(method.getReturnType, parameterTypes(index)) diff --git a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala index 95b7ce0ab9..9ffaf461f0 100644 --- a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala +++ b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala @@ -25,6 +25,7 @@ import org.apache.fory.json.ForyJsonException import org.apache.fory.json.annotation.{JsonIgnore, JsonProperty, JsonUnwrapped} import org.apache.fory.json.codec.AbstractJsonValueCodec import org.apache.fory.json.reader.JsonReader +import org.apache.fory.json.resolver.UnsupportedJsonTypeException import org.apache.fory.json.writer.JsonWriter import org.apache.fory.reflect.TypeRef import org.scalatest.funsuite.AnyFunSuite @@ -56,6 +57,20 @@ case class UnwrappedState( var label: String = "default-label" } +object NestedModels { + case class Point(x: Int, y: String) + + case class Region(origin: Point, size: Int = 2) + + object Inner { + case class Depth(level: Int) + } +} + +class OuterHolder { + case class Bound(id: Int) +} + case class NullableRequired(value: String) case class UserId(value: Int) extends AnyVal @@ -173,6 +188,30 @@ class ScalaJsonSuite extends AnyFunSuite { } } + test("case class declared inside an object") { + for (json <- Seq( + ForyJsonScala.builder().withCodegen(false).build(), + ForyJsonScala.builder().withAsyncCompilation(false).build() + )) { + val region = NestedModels.Region(NestedModels.Point(1, "a"), 4) + val encoded = json.toJson(region) + assert(encoded.contains("\"origin\"")) + assert(json.fromJson(encoded, classOf[NestedModels.Region]) == region) + // Scala 2 keeps `apply` and the constructor defaults on the companion singleton because it + // emits static forwarders only for a top-level companion. + val defaulted = json.fromJson("{\"origin\":{\"x\":1,\"y\":\"a\"}}", classOf[NestedModels.Region]) + assert(defaulted.size == 2) + val depth = NestedModels.Inner.Depth(3) + assert(json.fromJson(json.toJson(depth), classOf[NestedModels.Inner.Depth]) == depth) + } + } + + test("case class declared inside a class is rejected") { + val json = ForyJsonScala.builder().withCodegen(false).build() + val holder = new OuterHolder + assertThrows[UnsupportedJsonTypeException](json.toJson(holder.Bound(1))) + } + test("required constructor values cannot be omitted as null") { for (json <- Seq( ForyJsonScala.builder().withCodegen(false).build(), From ec34ed31b086bddfe1171057f6143602ad8e7adf Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 30 Aug 2026 19:31:24 +0100 Subject: [PATCH 2/5] fix(scala): resolve nested companions in native images and deep nesting Addresses AI review findings on the previous commit. `ReflectionUtils.getLiteralName` skipped its nested-Scala-object correction whenever the canonical name ended with `$`, which is true of every companion module class. A companion declared two or more levels inside an object produced `a.B$Inner$.Depth$`, which names no resolvable type, so a generated reader for such a case class failed to compile. The correction now covers module classes. `ForyJsonGraalVMFeature` registers the companion class, its `MODULE$` field, and the `apply` and constructor-default methods that the Scala module queries when it rebuilds the object model at image runtime. Without this a nested case class resolved on the JVM but not in a native image. The comment justifying why an instance default skips `registerCreator` was wrong: no bound invoker is retained from build time; the real reason is that `creatorHandle` spreads an argument array over the exact parameter count and cannot describe a receiver. Recognition no longer initializes the companion: resolving the owner leaves the singleton unloaded, so deciding whether a type is a supported case class never runs a user object body, and `MODULE$` is read only once the model is built. `CompanionOwner` carries an explicit static-forwarder flag instead of inferring it from a null receiver, and the case-class marker also requires a declared `productPrefix` so fewer hand-written `Product` types are claimed. An instance default must now be declared by the created type's companion, and the model carries a receiver only when a default is actually bound to it. Tests cover the workspace codegen path through `@JsonUnwrapped`, a default that consumes a preceding constructor argument, a doubly nested companion, and the method-local rejection. The native-image main round-trips a nested case class. Co-Authored-By: Claude Opus 5 (1M context) --- docs/json/scala.md | 4 +- .../apache/fory/reflect/ReflectionUtils.java | 7 +- .../apache/fory/json/codegen/JsonCodegen.java | 7 +- .../fory/json/codegen/JsonReaderCodegen.java | 50 +++++++++++--- .../fory/json/meta/JsonCreatorInfo.java | 4 +- .../fory/json/ForyJsonGraalVMFeature.java | 69 ++++++++++++++++++- .../scala/internal/ScalaObjectModels.scala | 58 ++++++++++++---- .../ScalaJsonEnumerationNativeImageMain.scala | 10 +++ .../fory/json/scala/ScalaJsonSuite.scala | 45 +++++++++++- 9 files changed, 219 insertions(+), 35 deletions(-) diff --git a/docs/json/scala.md b/docs/json/scala.md index cfc7fdc819..178a000756 100644 --- a/docs/json/scala.md +++ b/docs/json/scala.md @@ -51,8 +51,8 @@ constructor arguments exactly as Scala defines them. A missing parameter without error. Mutable body properties are applied after construction. A case class may be declared at the top level or inside an `object`, at any nesting depth. A case -class declared inside a `class` or inside a method is rejected, because Fory cannot reach the -enclosing instance or companion it needs to rebuild the value. +class declared inside a `class` or inside a method is rejected for both reading and writing, +because Fory cannot reach the enclosing instance or companion it needs to rebuild the value. Fory JSON annotations can be placed directly on Scala constructor properties: diff --git a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java index 82647de3ae..94a014fed4 100644 --- a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java +++ b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java @@ -630,8 +630,11 @@ public static String getLiteralName(Class cls) { // qualifier name of scala object type will ends with `.` canonicalName = clsName.substring(0, clsName.length() - 1).replace("$", ".") + "$"; } else { - if (!canonicalName.endsWith("$") && canonicalName.contains("$")) { - // nested scala object type can't be accessed in java by using canonicalName + if (canonicalName.contains("$")) { + // nested scala object type can't be accessed in java by using canonicalName. This includes + // a nested module class, whose own name ends with `$`: the canonical name of a companion + // declared two or more levels inside an object mixes `.` and `$` separators and names no + // resolvable type. // see more detailed in // https://stackoverflow.com/questions/30809070/accessing-scala-nested-classes-from-java int nestedLevels = 0; diff --git a/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonCodegen.java b/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonCodegen.java index 1c3dde20b2..5ff4c69a45 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonCodegen.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonCodegen.java @@ -1134,9 +1134,10 @@ private boolean canCompileCreator(JsonCreatorInfo creator) { return false; } Method defaultMethod = creator.defaultMethod(i); - // JsonCreatorInfo guarantees that a default method belongs to the creator owner and that its - // dependency types are the preceding creator parameters. The generated reader still invokes - // that exact method, so validate its access from the final definition context as well. + // JsonCreatorInfo guarantees that a default method belongs to the creator owner, or to the + // language singleton that owns instance defaults, and that its dependency types are the + // preceding creator parameters. The generated reader invokes that exact method on that exact + // declaring class, so validate its access from the final definition context as well. if (defaultMethod != null && !canCall(defaultMethod)) { return false; } diff --git a/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java b/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java index fb131d02ef..c029108035 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java @@ -74,6 +74,7 @@ abstract class JsonReaderCodegen { private static final boolean LITTLE_ENDIAN = NativeByteOrder.IS_LITTLE_ENDIAN; private static final long UTF16_PAIR_MASK = 0x0000FFFF0000FFFFL; private static final long UTF16_BYTE_MASK = 0x00FF00FF00FF00FFL; + private static final String DEFAULTS_OWNER_LOCAL = "defaultsOwner"; final JsonCodegen codegen; final JsonTypeResolver resolver; @@ -1321,7 +1322,12 @@ private Expression creatorDefaultValue( return new Expression.Cast(value, TypeRef.of(parameterType)); } - /** Reads the language singleton that owns instance constructor defaults, such as a companion. */ + /** + * Reads the language singleton that owns instance constructor defaults, such as a companion. + * Each defaulted parameter needs its own expression: generated code for one expression instance + * is emitted once, at its first use site, and every use site here is a separate missing-argument + * block, so a shared instance would reference a local declared in a sibling block. + */ private Expression defaultsReceiver(Method method) { return new Expression.Cast( new Expression.Invoke( @@ -1468,12 +1474,42 @@ private void addWorkspaceCreatorMethod( ctx.addMethod("private final", methodName, body.toString(), type, Object[].class, "arguments"); } + /** Declares the shared instance-default receiver local when the creator binds one. */ + private void appendDefaultsOwnerLocal( + CodegenContext ctx, + StringBuilder body, + JsonCreatorInfo creator, + String creatorExpression, + int parameterCount) { + Class owner = null; + for (int i = 0; i < parameterCount; i++) { + Method method = creator.defaultMethod(i); + if (method != null && !Modifier.isStatic(method.getModifiers())) { + owner = method.getDeclaringClass(); + break; + } + } + if (owner == null) { + return; + } + String ownerType = ctx.type(owner); + body.append(ownerType) + .append(' ') + .append(DEFAULTS_OWNER_LOCAL) + .append(" = ((") + .append(ownerType) + .append(") ") + .append(creatorExpression) + .append(".defaultsReceiver());\n"); + } + private void appendWorkspaceDefaults( CodegenContext ctx, StringBuilder body, JsonCreatorInfo creator, String creatorExpression, Class[] parameterTypes) { + appendDefaultsOwnerLocal(ctx, body, creator, creatorExpression, parameterTypes.length); for (int i = 0; i < parameterTypes.length; i++) { Method method = creator.defaultMethod(i); body.append("if (") @@ -1502,15 +1538,9 @@ private void appendWorkspaceDefaults( } } else { body.append("arguments[").append(i).append("] = "); - if (Modifier.isStatic(method.getModifiers())) { - body.append(ctx.type(method.getDeclaringClass())); - } else { - body.append("((") - .append(ctx.type(method.getDeclaringClass())) - .append(") ") - .append(creatorExpression) - .append(".defaultsReceiver())"); - } + body.append(Modifier.isStatic(method.getModifiers()) + ? ctx.type(method.getDeclaringClass()) + : DEFAULTS_OWNER_LOCAL); body.append('.').append(method.getName()).append('('); Class[] dependencies = method.getParameterTypes(); for (int j = 0; j < dependencies.length; j++) { diff --git a/java/fory-json/src/main/java/org/apache/fory/json/meta/JsonCreatorInfo.java b/java/fory-json/src/main/java/org/apache/fory/json/meta/JsonCreatorInfo.java index f93cfea9f6..b2a4b0b06d 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/meta/JsonCreatorInfo.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/meta/JsonCreatorInfo.java @@ -626,7 +626,9 @@ private static MethodHandle[] buildDefaultInvokers( boolean instanceDefault = !java.lang.reflect.Modifier.isStatic(method.getModifiers()); Class declaringClass = method.getDeclaringClass(); if ((instanceDefault - ? defaultsReceiver == null || !declaringClass.isInstance(defaultsReceiver) + ? defaultsReceiver == null + || !declaringClass.isInstance(defaultsReceiver) + || !declaringClass.getName().equals(ownerType.getName() + "$") : defaultsReceiver != null || declaringClass != ownerType) || !method.getName().equals("$lessinit$greater$default$" + (i + 1)) || method.getParameterCount() > i diff --git a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java index a5d445d4ba..655b002d74 100644 --- a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java +++ b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java @@ -114,6 +114,10 @@ final class ForyJsonGraalVMFeature implements Feature { private final Set> processedCodecs = ConcurrentHashMap.newKeySet(); private final Set> processedContainers = ConcurrentHashMap.newKeySet(); private final Set processedCreators = new LinkedHashSet<>(); + // Reflection-only registrations are tracked apart from processedCreators, whose membership also + // means a creator handle was retained. + private final Set processedReflectiveMethods = new LinkedHashSet<>(); + private final Set> languageSingletonOwners = new LinkedHashSet<>(); private final Set> processedObjectModels = Collections.newSetFromMap(new IdentityHashMap<>()); private final ArrayList hostedConfigurations = new ArrayList<>(); @@ -442,12 +446,15 @@ private void registerObjectModel(DuringAnalysisAccess access, ObjectCodec obj } if (Modifier.isStatic(defaultMethod.getModifiers())) { registerCreator(defaultMethod); - } else if (processedCreators.add(defaultMethod)) { - // An instance default is invoked on the language singleton that owns it, so its bound - // invoker already lives in the creator metadata. Only reflection access is needed. + } else if (processedReflectiveMethods.add(defaultMethod)) { + // An instance default is bound to its owning singleton when the creator metadata is + // rebuilt at image runtime. creatorHandle spreads an argument array over the exact + // parameter count, which does not describe a method that also takes a receiver, so the + // method is only made reflectively available here. RuntimeReflection.register(defaultMethod); } } + registerLanguageSingletonOwner(creator.executable().getDeclaringClass()); } for (JsonFieldInfo field : objectModel.writeFields()) { registerFieldAccessor(access, field.writeField(), field.writeGetter(), null); @@ -866,6 +873,62 @@ private void registerRecord(Class type) { registerCreator(constructor); } + /** + * Registers the singleton that owns a type's constructor metadata when the type does not carry + * static forwarders for it. A Scala case class declared inside an object keeps `apply` and its + * constructor defaults on the companion singleton, and the language module resolves that + * singleton reflectively while rebuilding the object model at image runtime. + */ + private void registerLanguageSingletonOwner(Class type) { + if (!languageSingletonOwners.add(type) || hasStaticFactory(type)) { + return; + } + Class companion; + try { + companion = Class.forName(type.getName() + "$", false, type.getClassLoader()); + } catch (ClassNotFoundException | LinkageError e) { + return; + } + Field field; + try { + field = companion.getField("MODULE$"); + } catch (NoSuchFieldException e) { + return; + } + int modifiers = field.getModifiers(); + if (field.getType() != companion + || !Modifier.isPublic(companion.getModifiers()) + || !Modifier.isPublic(modifiers) + || !Modifier.isStatic(modifiers) + || !Modifier.isFinal(modifiers)) { + return; + } + RuntimeReflection.register(companion); + RuntimeReflection.register(field); + for (Method method : companion.getMethods()) { + if (method.getDeclaringClass() == companion + && !Modifier.isStatic(method.getModifiers()) + && (method.getReturnType() == type + || method.getName().startsWith("$lessinit$greater$default$")) + && processedReflectiveMethods.add(method)) { + RuntimeReflection.register(method); + } + } + } + + private static boolean hasStaticFactory(Class type) { + for (Method method : type.getMethods()) { + if (Modifier.isStatic(method.getModifiers()) + && method.getReturnType() == type + && !method.isBridge() + && !method.isSynthetic() + && "apply".equals(method.getName())) { + return true; + } + } + return false; + } + private void registerCreator(Executable executable) { if (processedCreators.add(executable)) { RuntimeReflection.register(executable); diff --git a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala index 7953bceb0c..10aab33864 100644 --- a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala +++ b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala @@ -36,9 +36,11 @@ private[scala] object ScalaObjectModels { if (companion != null && findPrimaryConstructor(typeClass, companion) != null) return true // A case class that cannot reach its companion, such as one declared inside a class or a // method, is still a case class. Claim it so the codec reports the exact reason instead of - // leaving it to a generic object model that silently drops every property. `copy` returning - // the declaring class is the compiler marker; standard-library types keep their own mapping. - !name.startsWith("scala.") && copyMethod(typeClass) != null + // leaving it to a generic object model that silently drops every property. A generated `copy` + // returning the declaring class together with a declared `productPrefix`, which `Product` + // otherwise supplies by default, is the compiler marker of a case class. Standard-library + // types keep their own mapping. + !name.startsWith("scala.") && copyMethod(typeClass) != null && declaresProductPrefix(typeClass) } def caseClassCodec(typeRef: TypeRef[_], resolver: JsonTypeResolver): ObjectCodec[_] = { @@ -99,6 +101,11 @@ private[scala] object ScalaObjectModels { // @JsonEnumeration binds an erased Enumeration.Value parameter to the exact MODULE$ owner. val logicalParameterTypes = propertyTypes.take(names.length) val defaults = constructorDefaults(typeClass, companion, parameterTypes) + // The receiver belongs to the model only when a default is actually bound to it, so a nested + // case class without defaults keeps a receiver-free model. + val defaultsReceiver = + if (companion.staticForwarders || defaults.forall(_ == null)) null + else companionInstance(typeRef, companion.owner) resolver.createObjectCodec( typeRef, new JsonObjectModel( @@ -107,7 +114,7 @@ private[scala] object ScalaObjectModels { names, accessors, defaults, - companion.receiver, + defaultsReceiver, Array.fill(names.length)(-1), Array.fill(names.length)(true), logicalParameterTypes, @@ -186,6 +193,13 @@ private[scala] object ScalaObjectModels { } } + private def declaresProductPrefix(typeClass: Class[_]): Boolean = { + try { + val method = typeClass.getDeclaredMethod("productPrefix") + method.getReturnType == classOf[String] && !method.isSynthetic + } catch { case _: NoSuchMethodException => false } + } + private def copyMethod(typeClass: Class[_]): Method = { typeClass.getMethods .find(method => @@ -212,10 +226,13 @@ private[scala] object ScalaObjectModels { * Owner of the compiler-generated `apply` and `$lessinit$greater$default$N` members of a case * class. Scala mirrors those companion members as static forwarders on the case class itself * only for a top-level companion, so a case class declared inside an `object` keeps them as - * instance members of the companion singleton. `receiver` is null for the static form. + * instance members of the companion singleton. */ - private final class CompanionOwner(val owner: Class[_], val receiver: AnyRef) + private final class CompanionOwner(val owner: Class[_], val staticForwarders: Boolean) + // Recognition must not initialize the companion. Resolving the owner keeps the singleton + // unloaded so deciding whether a type is a supported case class never runs a user object body; + // caseClassCodec reads MODULE$ only once it commits to building the model. private def companionOwner(typeClass: Class[_]): CompanionOwner = { val methods = typeClass.getMethods var index = 0 @@ -223,15 +240,32 @@ private[scala] object ScalaObjectModels { val method = methods(index) if ( method.getName == "apply" && Modifier.isStatic(method.getModifiers) && - method.getReturnType == typeClass - ) return new CompanionOwner(typeClass, null) + !method.isBridge && !method.isSynthetic && method.getReturnType == typeClass + ) return new CompanionOwner(typeClass, true) index += 1 } val companionClass = try Class.forName(typeClass.getName + "$", false, typeClass.getClassLoader) catch { case _: ClassNotFoundException | _: LinkageError => return null } - val field = singletonField(companionClass) - if (field == null) null else new CompanionOwner(companionClass, field.get(null)) + if (!Modifier.isPublic(companionClass.getModifiers) || singletonField(companionClass) == null) { + null + } else new CompanionOwner(companionClass, false) + } + + private def companionInstance(typeRef: TypeRef[_], companionClass: Class[_]): AnyRef = { + val instance = + try singletonField(companionClass).get(null) + catch { + case error: ReflectiveOperationException => + throw new ForyJsonException( + s"Cannot read Scala companion singleton ${companionClass.getName}", + error + ) + } + if (instance == null) { + throw ScalaTypeSupport.unsupported(typeRef, "case class companion singleton is not initialized") + } + instance } private def findPrimaryConstructor( @@ -240,7 +274,7 @@ private[scala] object ScalaObjectModels { ): Constructor[_] = { val constructors = typeClass.getConstructors val methods = companion.owner.getMethods - val staticApply = companion.receiver == null + val staticApply = companion.staticForwarders var selected: Constructor[_] = null var index = 0 while (index < constructors.length) { @@ -310,7 +344,7 @@ private[scala] object ScalaObjectModels { // owner for a top-level companion, otherwise instance members of the companion singleton. // A default in a later parameter list receives the preceding parameter lists as arguments. val defaults = new Array[Method](parameterTypes.length) - val staticDefault = companion.receiver == null + val staticDefault = companion.staticForwarders var index = 0 while (index < defaults.length) { val name = "$lessinit$greater$default$" + (index + 1) diff --git a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonEnumerationNativeImageMain.scala b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonEnumerationNativeImageMain.scala index aa9abd2168..691cb86eb9 100644 --- a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonEnumerationNativeImageMain.scala +++ b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonEnumerationNativeImageMain.scala @@ -26,6 +26,11 @@ object NativeWeekday extends Enumeration { val Monday, Tuesday = Value } +object NativeNested { + @JsonType + case class Reading(value: Int, unit: String = "px") +} + @JsonType case class NativeEnumerationSchedule( @JsonEnumeration(classOf[NativeWeekday.type]) day: NativeWeekday.Value, @@ -45,6 +50,11 @@ object ScalaJsonEnumerationNativeImageMain { List(NativeWeekday.Monday, NativeWeekday.Tuesday) ) require(json.fromJson(json.toJson(value), classOf[NativeEnumerationSchedule]) == value) + // A case class declared inside an object binds `apply` and its constructor defaults on the + // companion singleton, which the image must reach reflectively at runtime. + val reading = NativeNested.Reading(3, "em") + require(json.fromJson(json.toJson(reading), classOf[NativeNested.Reading]) == reading) + require(json.fromJson("{\"value\":3}", classOf[NativeNested.Reading]).unit == "px") println("Fory Scala 2 Enumeration native image succeeded") } } diff --git a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala index 9ffaf461f0..b6099b622e 100644 --- a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala +++ b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala @@ -62,8 +62,19 @@ object NestedModels { case class Region(origin: Point, size: Int = 2) + case class Span(from: Int)(val to: Int = from + 1) + + case class UnwrappedNested(code: Int = 5) { + var note: String = "default-note" + } + + case class UnwrappedOwner( + id: Int = 3, + @JsonUnwrapped nested: UnwrappedNested = UnwrappedNested() + ) + object Inner { - case class Depth(level: Int) + case class Depth(level: Int, unit: String = "px") } } @@ -201,8 +212,32 @@ class ScalaJsonSuite extends AnyFunSuite { // emits static forwarders only for a top-level companion. val defaulted = json.fromJson("{\"origin\":{\"x\":1,\"y\":\"a\"}}", classOf[NestedModels.Region]) assert(defaulted.size == 2) - val depth = NestedModels.Inner.Depth(3) + // A doubly nested companion must also be spelled correctly by generated readers. + val depth = NestedModels.Inner.Depth(3, "em") assert(json.fromJson(json.toJson(depth), classOf[NestedModels.Inner.Depth]) == depth) + assert(json.fromJson("{\"level\":3}", classOf[NestedModels.Inner.Depth]).unit == "px") + } + } + + test("nested case class defaults use preceding parameter lists") { + for (json <- Seq( + ForyJsonScala.builder().withCodegen(false).build(), + ForyJsonScala.builder().withAsyncCompilation(false).build() + )) { + assert(json.fromJson("{\"from\":4}", classOf[NestedModels.Span]).to == 5) + } + } + + test("nested unwrapped creators apply defaults") { + for (json <- Seq( + ForyJsonScala.builder().withCodegen(false).build(), + ForyJsonScala.builder().withAsyncCompilation(false).build() + )) { + val value = + json.fromJson("{\"note\":\"child\"}", classOf[NestedModels.UnwrappedOwner]) + assert(value.id == 3) + assert(value.nested.code == 5) + assert(value.nested.note == "child") } } @@ -212,6 +247,12 @@ class ScalaJsonSuite extends AnyFunSuite { assertThrows[UnsupportedJsonTypeException](json.toJson(holder.Bound(1))) } + test("case class declared inside a method is rejected") { + case class MethodLocal(id: Int) + val json = ForyJsonScala.builder().withCodegen(false).build() + assertThrows[UnsupportedJsonTypeException](json.toJson(MethodLocal(1))) + } + test("required constructor values cannot be omitted as null") { for (json <- Seq( ForyJsonScala.builder().withCodegen(false).build(), From 637b5063faa75871698353e97ab7c2bbff059912 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 30 Aug 2026 19:52:56 +0100 Subject: [PATCH 3/5] fix(scala): narrow the case-class claim and keep default fetches conditional Addresses the second and third AI review rounds. The receiver belongs only to the model shapes that can carry instance defaults. The full constructor is now private and its public form keeps its previous signature, so `fory-json-kotlin`, whose call site uses that shape, is untouched. `JsonObjectModel` also rejects a receiver that no default is bound to, so the invariant is total rather than checked per parameter. The unreconstructible-case-class claim applies only when the companion is unreachable. A reachable companion whose primary constructor this module does not support, such as a varargs or non-public one, keeps its previous handling instead of becoming a hard failure. A companion that exists but cannot be linked is no longer reported as an unreachable companion: only absence means the type has no companion, and a `LinkageError` now surfaces with the companion it failed to load. The workspace creator no longer hoists the defaults receiver into a local: that local ran on every construction, while the value is needed only on a missing argument. Both codegen paths now fetch it on the branch that uses it. GraalVM registration is narrowed to the members the Scala module looks up, and its helpers are named for the Scala companion they actually detect. Both rejection tests now assert their message. An outer-bound case class also has no reachable companion, so without that assertion either branch alone satisfied both tests, and the method-local case must be declared inside an object to reach the companion check at all. Co-Authored-By: Claude Opus 5 (1M context) --- docs/json/scala.md | 7 ++- .../apache/fory/reflect/ReflectionUtils.java | 4 +- .../fory/json/codec/JsonObjectModel.java | 41 ++++++++++++++ .../fory/json/codegen/JsonReaderCodegen.java | 54 ++++++------------ .../fory/json/ForyJsonGraalVMFeature.java | 22 ++++---- .../scala/internal/ScalaObjectModels.scala | 56 ++++++++++++------- .../fory/json/scala/ScalaJsonSuite.scala | 19 ++++++- 7 files changed, 127 insertions(+), 76 deletions(-) diff --git a/docs/json/scala.md b/docs/json/scala.md index 178a000756..be98df03a0 100644 --- a/docs/json/scala.md +++ b/docs/json/scala.md @@ -50,9 +50,10 @@ or mutate constructor `val` fields. Defaults in later parameter lists receive th constructor arguments exactly as Scala defines them. A missing parameter without a default is an error. Mutable body properties are applied after construction. -A case class may be declared at the top level or inside an `object`, at any nesting depth. A case -class declared inside a `class` or inside a method is rejected for both reading and writing, -because Fory cannot reach the enclosing instance or companion it needs to rebuild the value. +A case class may be declared at the top level, or inside an `object` at any nesting depth, as long +as every enclosing scope is itself an `object`. A case class enclosed by a `class` or by a method +is rejected for both reading and writing, because Fory cannot reach the enclosing instance or the +companion it needs to rebuild the value. Fory JSON annotations can be placed directly on Scala constructor properties: diff --git a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java index 94a014fed4..09b615196f 100644 --- a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java +++ b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java @@ -634,7 +634,9 @@ public static String getLiteralName(Class cls) { // nested scala object type can't be accessed in java by using canonicalName. This includes // a nested module class, whose own name ends with `$`: the canonical name of a companion // declared two or more levels inside an object mixes `.` and `$` separators and names no - // resolvable type. + // resolvable type. A module class only one level deep keeps its canonical name: only its + // last segment ends with `$`, which resolves, so the nesting-level bound below is load + // bearing in both directions. // see more detailed in // https://stackoverflow.com/questions/30809070/accessing-scala-nested-classes-from-java int nestedLevels = 0; diff --git a/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java b/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java index 871335deeb..90ca2a55cb 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java @@ -161,6 +161,41 @@ public JsonObjectModel( /** Creates a model with exact reconstructibility and deferred-required facts. */ public JsonObjectModel( + Executable creator, + Executable invocationCreator, + Constructor defaultConstructor, + String[] parameterNames, + Method[] accessors, + Method[] defaultMethods, + int[] defaultMaskBits, + boolean[] parameterNullable, + TypeRef[] parameterTypes, + String[] propertyNames, + Method[] propertyGetters, + Method[] propertySetters, + TypeRef[] propertyTypes, + boolean[] propertyReconstructible, + boolean[] propertyRequired) { + this( + creator, + invocationCreator, + defaultConstructor, + parameterNames, + accessors, + defaultMethods, + null, + defaultMaskBits, + parameterNullable, + parameterTypes, + propertyNames, + propertyGetters, + propertySetters, + propertyTypes, + propertyReconstructible, + propertyRequired); + } + + private JsonObjectModel( Executable creator, Executable invocationCreator, Constructor defaultConstructor, @@ -337,6 +372,7 @@ private void validate() { } } HashSet names = new HashSet<>(); + boolean hasInstanceDefault = false; for (int i = 0; i < parameterNames.length; i++) { String name = parameterNames[i]; if (name == null || name.isEmpty() || !names.add(name)) { @@ -358,6 +394,11 @@ private void validate() { throw new IllegalArgumentException( "JSON constructor default receiver does not own " + defaultMethods[i]); } + hasInstanceDefault |= defaultMethods[i] != null; + } + if (defaultsReceiver != null && !hasInstanceDefault) { + throw new IllegalArgumentException( + "A JSON constructor default receiver requires at least one instance default"); } names.clear(); for (int i = 0; i < propertyNames.length; i++) { diff --git a/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java b/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java index c029108035..4499f77bf1 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/codegen/JsonReaderCodegen.java @@ -74,7 +74,6 @@ abstract class JsonReaderCodegen { private static final boolean LITTLE_ENDIAN = NativeByteOrder.IS_LITTLE_ENDIAN; private static final long UTF16_PAIR_MASK = 0x0000FFFF0000FFFFL; private static final long UTF16_BYTE_MASK = 0x00FF00FF00FF00FFL; - private static final String DEFAULTS_OWNER_LOCAL = "defaultsOwner"; final JsonCodegen codegen; final JsonTypeResolver resolver; @@ -1323,10 +1322,11 @@ private Expression creatorDefaultValue( } /** - * Reads the language singleton that owns instance constructor defaults, such as a companion. - * Each defaulted parameter needs its own expression: generated code for one expression instance - * is emitted once, at its first use site, and every use site here is a separate missing-argument - * block, so a shared instance would reference a local declared in a sibling block. + * Reads the language singleton that owns instance constructor defaults, such as a Scala + * companion. Each defaulted parameter needs its own expression, because generated code for one + * expression instance is emitted once at its first use site, and every use site here is a + * separate missing-argument block, so a shared instance would reference a local declared in a + * sibling block. */ private Expression defaultsReceiver(Method method) { return new Expression.Cast( @@ -1474,42 +1474,12 @@ private void addWorkspaceCreatorMethod( ctx.addMethod("private final", methodName, body.toString(), type, Object[].class, "arguments"); } - /** Declares the shared instance-default receiver local when the creator binds one. */ - private void appendDefaultsOwnerLocal( - CodegenContext ctx, - StringBuilder body, - JsonCreatorInfo creator, - String creatorExpression, - int parameterCount) { - Class owner = null; - for (int i = 0; i < parameterCount; i++) { - Method method = creator.defaultMethod(i); - if (method != null && !Modifier.isStatic(method.getModifiers())) { - owner = method.getDeclaringClass(); - break; - } - } - if (owner == null) { - return; - } - String ownerType = ctx.type(owner); - body.append(ownerType) - .append(' ') - .append(DEFAULTS_OWNER_LOCAL) - .append(" = ((") - .append(ownerType) - .append(") ") - .append(creatorExpression) - .append(".defaultsReceiver());\n"); - } - private void appendWorkspaceDefaults( CodegenContext ctx, StringBuilder body, JsonCreatorInfo creator, String creatorExpression, Class[] parameterTypes) { - appendDefaultsOwnerLocal(ctx, body, creator, creatorExpression, parameterTypes.length); for (int i = 0; i < parameterTypes.length; i++) { Method method = creator.defaultMethod(i); body.append("if (") @@ -1538,9 +1508,17 @@ private void appendWorkspaceDefaults( } } else { body.append("arguments[").append(i).append("] = "); - body.append(Modifier.isStatic(method.getModifiers()) - ? ctx.type(method.getDeclaringClass()) - : DEFAULTS_OWNER_LOCAL); + if (Modifier.isStatic(method.getModifiers())) { + body.append(ctx.type(method.getDeclaringClass())); + } else { + // Fetched on the missing-argument branch, not once per construction: a creator whose + // properties are all present must not pay for a receiver it never reads. + body.append("((") + .append(ctx.type(method.getDeclaringClass())) + .append(") ") + .append(creatorExpression) + .append(".defaultsReceiver())"); + } body.append('.').append(method.getName()).append('('); Class[] dependencies = method.getParameterTypes(); for (int j = 0; j < dependencies.length; j++) { diff --git a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java index 655b002d74..5c37f95159 100644 --- a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java +++ b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java @@ -117,7 +117,7 @@ final class ForyJsonGraalVMFeature implements Feature { // Reflection-only registrations are tracked apart from processedCreators, whose membership also // means a creator handle was retained. private final Set processedReflectiveMethods = new LinkedHashSet<>(); - private final Set> languageSingletonOwners = new LinkedHashSet<>(); + private final Set> scalaCompanionOwners = new LinkedHashSet<>(); private final Set> processedObjectModels = Collections.newSetFromMap(new IdentityHashMap<>()); private final ArrayList hostedConfigurations = new ArrayList<>(); @@ -454,7 +454,7 @@ private void registerObjectModel(DuringAnalysisAccess access, ObjectCodec obj RuntimeReflection.register(defaultMethod); } } - registerLanguageSingletonOwner(creator.executable().getDeclaringClass()); + registerScalaCompanion(creator.executable().getDeclaringClass()); } for (JsonFieldInfo field : objectModel.writeFields()) { registerFieldAccessor(access, field.writeField(), field.writeGetter(), null); @@ -874,13 +874,13 @@ private void registerRecord(Class type) { } /** - * Registers the singleton that owns a type's constructor metadata when the type does not carry - * static forwarders for it. A Scala case class declared inside an object keeps `apply` and its - * constructor defaults on the companion singleton, and the language module resolves that - * singleton reflectively while rebuilding the object model at image runtime. + * Registers the Scala companion that owns a type's constructor metadata when the type carries no + * static forwarders for it. A case class declared inside an object keeps `apply` and its + * constructor defaults on the companion singleton, and the Scala module resolves that singleton + * reflectively while rebuilding the object model at image runtime. */ - private void registerLanguageSingletonOwner(Class type) { - if (!languageSingletonOwners.add(type) || hasStaticFactory(type)) { + private void registerScalaCompanion(Class type) { + if (!scalaCompanionOwners.add(type) || hasScalaStaticFactory(type)) { return; } Class companion; @@ -906,9 +906,11 @@ private void registerLanguageSingletonOwner(Class type) { RuntimeReflection.register(companion); RuntimeReflection.register(field); for (Method method : companion.getMethods()) { + // Exactly the members the language module looks up on the singleton: the factory it matches + // against the primary constructor, and the constructor defaults. if (method.getDeclaringClass() == companion && !Modifier.isStatic(method.getModifiers()) - && (method.getReturnType() == type + && (("apply".equals(method.getName()) && method.getReturnType() == type) || method.getName().startsWith("$lessinit$greater$default$")) && processedReflectiveMethods.add(method)) { RuntimeReflection.register(method); @@ -916,7 +918,7 @@ private void registerLanguageSingletonOwner(Class type) { } } - private static boolean hasStaticFactory(Class type) { + private static boolean hasScalaStaticFactory(Class type) { for (Method method : type.getMethods()) { if (Modifier.isStatic(method.getModifiers()) && method.getReturnType() == type diff --git a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala index 10aab33864..74846e8eba 100644 --- a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala +++ b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala @@ -33,14 +33,15 @@ private[scala] object ScalaObjectModels { return false } val companion = companionOwner(typeClass) - if (companion != null && findPrimaryConstructor(typeClass, companion) != null) return true + if (companion != null) return findPrimaryConstructor(typeClass, companion) != null // A case class that cannot reach its companion, such as one declared inside a class or a // method, is still a case class. Claim it so the codec reports the exact reason instead of // leaving it to a generic object model that silently drops every property. A generated `copy` // returning the declaring class together with a declared `productPrefix`, which `Product` // otherwise supplies by default, is the compiler marker of a case class. Standard-library - // types keep their own mapping. - !name.startsWith("scala.") && copyMethod(typeClass) != null && declaresProductPrefix(typeClass) + // types keep their own mapping. A reachable companion whose constructor this module does not + // support, such as a varargs or non-public primary constructor, keeps its previous handling. + !name.startsWith("scala.") && declaresCopy(typeClass) && declaresProductPrefix(typeClass) } def caseClassCodec(typeRef: TypeRef[_], resolver: JsonTypeResolver): ObjectCodec[_] = { @@ -105,7 +106,7 @@ private[scala] object ScalaObjectModels { // case class without defaults keeps a receiver-free model. val defaultsReceiver = if (companion.staticForwarders || defaults.forall(_ == null)) null - else companionInstance(typeRef, companion.owner) + else companionInstance(typeRef, companion) resolver.createObjectCodec( typeRef, new JsonObjectModel( @@ -200,13 +201,11 @@ private[scala] object ScalaObjectModels { } catch { case _: NoSuchMethodException => false } } - private def copyMethod(typeClass: Class[_]): Method = { - typeClass.getMethods - .find(method => - method.getName == "copy" && !Modifier.isStatic(method.getModifiers) && - !method.isBridge && !method.isSynthetic && method.getReturnType == typeClass - ) - .orNull + private def declaresCopy(typeClass: Class[_]): Boolean = { + typeClass.getMethods.exists(method => + method.getName == "copy" && !Modifier.isStatic(method.getModifiers) && + !method.isBridge && !method.isSynthetic && method.getReturnType == typeClass + ) } private def outerField(typeClass: Class[_]): Field = { @@ -228,8 +227,15 @@ private[scala] object ScalaObjectModels { * only for a top-level companion, so a case class declared inside an `object` keeps them as * instance members of the companion singleton. */ - private final class CompanionOwner(val owner: Class[_], val staticForwarders: Boolean) + private final class CompanionOwner( + val owner: Class[_], + val singleton: Field, + val staticForwarders: Boolean + ) + // `fory-json` mirrors this companion rule in two places that must stay in sync: the + // `ownerType + "$"` check in JsonCreatorInfo.buildDefaultInvokers, and the native-image + // registration in ForyJsonGraalVMFeature. // Recognition must not initialize the companion. Resolving the owner keeps the singleton // unloaded so deciding whether a type is a supported case class never runs a user object body; // caseClassCodec reads MODULE$ only once it commits to building the model. @@ -241,24 +247,32 @@ private[scala] object ScalaObjectModels { if ( method.getName == "apply" && Modifier.isStatic(method.getModifiers) && !method.isBridge && !method.isSynthetic && method.getReturnType == typeClass - ) return new CompanionOwner(typeClass, true) + ) return new CompanionOwner(typeClass, null, true) index += 1 } + val companionName = typeClass.getName + "$" val companionClass = - try Class.forName(typeClass.getName + "$", false, typeClass.getClassLoader) - catch { case _: ClassNotFoundException | _: LinkageError => return null } - if (!Modifier.isPublic(companionClass.getModifiers) || singletonField(companionClass) == null) { - null - } else new CompanionOwner(companionClass, false) + try Class.forName(companionName, false, typeClass.getClassLoader) + catch { + // Absence means the type has no companion. A companion that exists but cannot be linked, + // including one missing native-image reflection metadata, is a real failure and must not + // be reported as an unreachable companion. + case _: ClassNotFoundException => return null + case error: LinkageError => + throw new ForyJsonException(s"Cannot load Scala companion $companionName", error) + } + val field = singletonField(companionClass) + if (!Modifier.isPublic(companionClass.getModifiers) || field == null) null + else new CompanionOwner(companionClass, field, false) } - private def companionInstance(typeRef: TypeRef[_], companionClass: Class[_]): AnyRef = { + private def companionInstance(typeRef: TypeRef[_], companion: CompanionOwner): AnyRef = { val instance = - try singletonField(companionClass).get(null) + try companion.singleton.get(null) catch { case error: ReflectiveOperationException => throw new ForyJsonException( - s"Cannot read Scala companion singleton ${companionClass.getName}", + s"Cannot read Scala companion singleton ${companion.owner.getName}", error ) } diff --git a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala index b6099b622e..18ca60d32c 100644 --- a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala +++ b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonSuite.scala @@ -82,6 +82,16 @@ class OuterHolder { case class Bound(id: Int) } +// Declared in a method of an object, so it captures no outer instance and its companion is a +// local module with no MODULE$. A method-local case class inside a class hits the outer check +// instead. +object MethodLocalHolder { + def create(): Any = { + case class MethodLocal(id: Int) + MethodLocal(1) + } +} + case class NullableRequired(value: String) case class UserId(value: Int) extends AnyVal @@ -244,13 +254,16 @@ class ScalaJsonSuite extends AnyFunSuite { test("case class declared inside a class is rejected") { val json = ForyJsonScala.builder().withCodegen(false).build() val holder = new OuterHolder - assertThrows[UnsupportedJsonTypeException](json.toJson(holder.Bound(1))) + // Both rejections assert their message: an outer-bound case class also has no reachable + // companion, so only the message distinguishes the outer check from the companion check. + val error = intercept[UnsupportedJsonTypeException](json.toJson(holder.Bound(1))) + assert(error.getMessage.contains("without its outer instance")) } test("case class declared inside a method is rejected") { - case class MethodLocal(id: Int) val json = ForyJsonScala.builder().withCodegen(false).build() - assertThrows[UnsupportedJsonTypeException](json.toJson(MethodLocal(1))) + val error = intercept[UnsupportedJsonTypeException](json.toJson(MethodLocalHolder.create())) + assert(error.getMessage.contains("companion is not reachable")) } test("required constructor values cannot be omitted as null") { From a86a870b4ffebfe8cc01102ea069fb3732426ec2 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 30 Aug 2026 21:21:13 +0100 Subject: [PATCH 4/5] fix(scala): keep case-class recognition free of linkage failures Addresses the fourth AI review round. Recognition answers a predicate for every Product that reaches this module, including types it does not own and never claims. Reporting a companion that cannot link belongs to the owning path, so a `LinkageError` now fails only once `caseClassCodec` has committed to the type. Before this change an unrelated Product with a stale companion could fail resolution that previously succeeded through the core object model. The nested-module literal-name comment named the wrong cause: the mixed canonical name resolves for javac, and it is the generated-code compiler that mangles it into a name that resolves to nothing. The outer-instance rejection also fires for a case class enclosed by a trait, so its message and the doc say so. The native-image main also round-trips a nested case class with no defaults, which still needs the companion to match `apply`, and a doubly nested one, which additionally depends on the literal-name fix. Co-Authored-By: Claude Opus 5 (1M context) --- docs/json/scala.md | 2 +- .../org/apache/fory/reflect/ReflectionUtils.java | 9 +++++---- .../apache/fory/json/codec/JsonObjectModel.java | 6 +++--- .../apache/fory/json/ForyJsonGraalVMFeature.java | 4 ++-- .../json/scala/internal/ScalaObjectModels.scala | 12 ++++++++---- .../ScalaJsonEnumerationNativeImageMain.scala | 15 +++++++++++++++ 6 files changed, 34 insertions(+), 14 deletions(-) diff --git a/docs/json/scala.md b/docs/json/scala.md index be98df03a0..6828cdc3d8 100644 --- a/docs/json/scala.md +++ b/docs/json/scala.md @@ -51,7 +51,7 @@ constructor arguments exactly as Scala defines them. A missing parameter without error. Mutable body properties are applied after construction. A case class may be declared at the top level, or inside an `object` at any nesting depth, as long -as every enclosing scope is itself an `object`. A case class enclosed by a `class` or by a method +as every enclosing scope is itself an `object`. A case class enclosed by a `class`, a trait, or a method is rejected for both reading and writing, because Fory cannot reach the enclosing instance or the companion it needs to rebuild the value. diff --git a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java index 09b615196f..99e49339b2 100644 --- a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java +++ b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java @@ -633,10 +633,11 @@ public static String getLiteralName(Class cls) { if (canonicalName.contains("$")) { // nested scala object type can't be accessed in java by using canonicalName. This includes // a nested module class, whose own name ends with `$`: the canonical name of a companion - // declared two or more levels inside an object mixes `.` and `$` separators and names no - // resolvable type. A module class only one level deep keeps its canonical name: only its - // last segment ends with `$`, which resolves, so the nesting-level bound below is load - // bearing in both directions. + // declared two or more levels inside an object mixes `.` and `$` separators, which the + // generated-code compiler mangles back into a name that resolves to nothing + // ("pkg.A$B$ declares no member type C$"). A module class only one level deep keeps its + // canonical name, because only its last segment ends with `$`, so the nesting-level bound + // below is load bearing in both directions. // see more detailed in // https://stackoverflow.com/questions/30809070/accessing-scala-nested-classes-from-java int nestedLevels = 0; diff --git a/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java b/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java index 90ca2a55cb..05a12bf67a 100644 --- a/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java +++ b/java/fory-json/src/main/java/org/apache/fory/json/codec/JsonObjectModel.java @@ -372,7 +372,7 @@ private void validate() { } } HashSet names = new HashSet<>(); - boolean hasInstanceDefault = false; + boolean hasDefaultMethod = false; for (int i = 0; i < parameterNames.length; i++) { String name = parameterNames[i]; if (name == null || name.isEmpty() || !names.add(name)) { @@ -394,9 +394,9 @@ private void validate() { throw new IllegalArgumentException( "JSON constructor default receiver does not own " + defaultMethods[i]); } - hasInstanceDefault |= defaultMethods[i] != null; + hasDefaultMethod |= defaultMethods[i] != null; } - if (defaultsReceiver != null && !hasInstanceDefault) { + if (defaultsReceiver != null && !hasDefaultMethod) { throw new IllegalArgumentException( "A JSON constructor default receiver requires at least one instance default"); } diff --git a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java index 5c37f95159..b462d98562 100644 --- a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java +++ b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java @@ -117,7 +117,7 @@ final class ForyJsonGraalVMFeature implements Feature { // Reflection-only registrations are tracked apart from processedCreators, whose membership also // means a creator handle was retained. private final Set processedReflectiveMethods = new LinkedHashSet<>(); - private final Set> scalaCompanionOwners = new LinkedHashSet<>(); + private final Set> scalaCompanionsConsidered = new LinkedHashSet<>(); private final Set> processedObjectModels = Collections.newSetFromMap(new IdentityHashMap<>()); private final ArrayList hostedConfigurations = new ArrayList<>(); @@ -880,7 +880,7 @@ private void registerRecord(Class type) { * reflectively while rebuilding the object model at image runtime. */ private void registerScalaCompanion(Class type) { - if (!scalaCompanionOwners.add(type) || hasScalaStaticFactory(type)) { + if (!scalaCompanionsConsidered.add(type) || hasScalaStaticFactory(type)) { return; } Class companion; diff --git a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala index 74846e8eba..cc146c41f0 100644 --- a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala +++ b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala @@ -32,7 +32,7 @@ private[scala] object ScalaObjectModels { if (!classOf[Product].isAssignableFrom(typeClass) || name.startsWith("scala.Tuple")) { return false } - val companion = companionOwner(typeClass) + val companion = companionOwner(typeClass, committed = false) if (companion != null) return findPrimaryConstructor(typeClass, companion) != null // A case class that cannot reach its companion, such as one declared inside a class or a // method, is still a case class. Claim it so the codec reports the exact reason instead of @@ -49,10 +49,10 @@ private[scala] object ScalaObjectModels { if (outerField(typeClass) != null) { throw ScalaTypeSupport.unsupported( typeRef, - "case class declared inside a class cannot be reconstructed without its outer instance" + "case class declared inside a class or trait cannot be reconstructed without its outer instance" ) } - val companion = companionOwner(typeClass) + val companion = companionOwner(typeClass, committed = true) if (companion == null) { throw ScalaTypeSupport.unsupported( typeRef, @@ -239,7 +239,10 @@ private[scala] object ScalaObjectModels { // Recognition must not initialize the companion. Resolving the owner keeps the singleton // unloaded so deciding whether a type is a supported case class never runs a user object body; // caseClassCodec reads MODULE$ only once it commits to building the model. - private def companionOwner(typeClass: Class[_]): CompanionOwner = { + // `committed` separates recognition from binding. Recognition answers a predicate for every + // Product that reaches this module, including types it does not own, so a companion that exists + // but cannot link must not fail that type there. Only the owning path reports it. + private def companionOwner(typeClass: Class[_], committed: Boolean): CompanionOwner = { val methods = typeClass.getMethods var index = 0 while (index < methods.length) { @@ -259,6 +262,7 @@ private[scala] object ScalaObjectModels { // be reported as an unreachable companion. case _: ClassNotFoundException => return null case error: LinkageError => + if (!committed) return null throw new ForyJsonException(s"Cannot load Scala companion $companionName", error) } val field = singletonField(companionClass) diff --git a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonEnumerationNativeImageMain.scala b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonEnumerationNativeImageMain.scala index 691cb86eb9..1830ea67c9 100644 --- a/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonEnumerationNativeImageMain.scala +++ b/scala/fory-json-scala/src/test/scala/org/apache/fory/json/scala/ScalaJsonEnumerationNativeImageMain.scala @@ -29,6 +29,16 @@ object NativeWeekday extends Enumeration { object NativeNested { @JsonType case class Reading(value: Int, unit: String = "px") + + // No default: the companion is still needed to match `apply` against the primary constructor. + @JsonType + case class Plain(value: Int) + + object Deep { + // Two levels inside an object: also depends on the nested-module literal name. + @JsonType + case class Nested(value: Int, unit: String = "em") + } } @JsonType @@ -55,6 +65,11 @@ object ScalaJsonEnumerationNativeImageMain { val reading = NativeNested.Reading(3, "em") require(json.fromJson(json.toJson(reading), classOf[NativeNested.Reading]) == reading) require(json.fromJson("{\"value\":3}", classOf[NativeNested.Reading]).unit == "px") + val plain = NativeNested.Plain(4) + require(json.fromJson(json.toJson(plain), classOf[NativeNested.Plain]) == plain) + val deep = NativeNested.Deep.Nested(5, "rem") + require(json.fromJson(json.toJson(deep), classOf[NativeNested.Deep.Nested]) == deep) + require(json.fromJson("{\"value\":5}", classOf[NativeNested.Deep.Nested]).unit == "em") println("Fory Scala 2 Enumeration native image succeeded") } } From 13a62665cb5145ad4c44aed0f39114edb1fb00cd Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Sun, 30 Aug 2026 21:48:30 +0100 Subject: [PATCH 5/5] 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) --- docs/json/scala.md | 6 +-- .../apache/fory/reflect/ReflectionUtils.java | 10 ++--- .../fory/json/ForyJsonGraalVMFeature.java | 4 +- .../scala/internal/ScalaObjectModels.scala | 42 +++++++++++-------- 4 files changed, 35 insertions(+), 27 deletions(-) diff --git a/docs/json/scala.md b/docs/json/scala.md index 6828cdc3d8..04b9637f31 100644 --- a/docs/json/scala.md +++ b/docs/json/scala.md @@ -51,9 +51,9 @@ constructor arguments exactly as Scala defines them. A missing parameter without error. Mutable body properties are applied after construction. A case class may be declared at the top level, or inside an `object` at any nesting depth, as long -as every enclosing scope is itself an `object`. A case class enclosed by a `class`, a trait, or a method -is rejected for both reading and writing, because Fory cannot reach the enclosing instance or the -companion it needs to rebuild the value. +as every enclosing scope is itself an `object`. A case class enclosed by a `class`, a trait, or a +method is rejected for both reading and writing, because Fory cannot reach the enclosing instance +or the companion it needs to rebuild the value. Fory JSON annotations can be placed directly on Scala constructor properties: diff --git a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java index 99e49339b2..b201f05756 100644 --- a/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java +++ b/java/fory-core/src/main/java/org/apache/fory/reflect/ReflectionUtils.java @@ -633,11 +633,11 @@ public static String getLiteralName(Class cls) { if (canonicalName.contains("$")) { // nested scala object type can't be accessed in java by using canonicalName. This includes // a nested module class, whose own name ends with `$`: the canonical name of a companion - // declared two or more levels inside an object mixes `.` and `$` separators, which the - // generated-code compiler mangles back into a name that resolves to nothing - // ("pkg.A$B$ declares no member type C$"). A module class only one level deep keeps its - // canonical name, because only its last segment ends with `$`, so the nesting-level bound - // below is load bearing in both directions. + // declared two or more levels inside an object has a `$`-terminated segment in the + // middle of its canonical name, one per enclosing module class, and the generated-code + // compiler cannot resolve through those ("pkg.A$B$ declares no member type C$"). One level + // deep is enclosed by the mirror class instead, so `pkg.A.C$` ends with the only `$` and + // still resolves: the nesting-level bound below is load bearing in both directions. // see more detailed in // https://stackoverflow.com/questions/30809070/accessing-scala-nested-classes-from-java int nestedLevels = 0; diff --git a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java index b462d98562..d5de194e4b 100644 --- a/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java +++ b/java/fory-json/src/main/java17/org/apache/fory/json/ForyJsonGraalVMFeature.java @@ -117,7 +117,7 @@ final class ForyJsonGraalVMFeature implements Feature { // Reflection-only registrations are tracked apart from processedCreators, whose membership also // means a creator handle was retained. private final Set processedReflectiveMethods = new LinkedHashSet<>(); - private final Set> scalaCompanionsConsidered = new LinkedHashSet<>(); + private final Set> typesConsideredForScalaCompanion = new LinkedHashSet<>(); private final Set> processedObjectModels = Collections.newSetFromMap(new IdentityHashMap<>()); private final ArrayList hostedConfigurations = new ArrayList<>(); @@ -880,7 +880,7 @@ private void registerRecord(Class type) { * reflectively while rebuilding the object model at image runtime. */ private void registerScalaCompanion(Class type) { - if (!scalaCompanionsConsidered.add(type) || hasScalaStaticFactory(type)) { + if (!typesConsideredForScalaCompanion.add(type) || hasScalaStaticFactory(type)) { return; } Class companion; diff --git a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala index cc146c41f0..1974ced6f3 100644 --- a/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala +++ b/scala/fory-json-scala/src/main/scala/org/apache/fory/json/scala/internal/ScalaObjectModels.scala @@ -32,16 +32,26 @@ private[scala] object ScalaObjectModels { if (!classOf[Product].isAssignableFrom(typeClass) || name.startsWith("scala.Tuple")) { return false } - val companion = companionOwner(typeClass, committed = false) - if (companion != null) return findPrimaryConstructor(typeClass, companion) != null - // A case class that cannot reach its companion, such as one declared inside a class or a - // method, is still a case class. Claim it so the codec reports the exact reason instead of - // leaving it to a generic object model that silently drops every property. A generated `copy` - // returning the declaring class together with a declared `productPrefix`, which `Product` - // otherwise supplies by default, is the compiler marker of a case class. Standard-library - // types keep their own mapping. A reachable companion whose constructor this module does not - // support, such as a varargs or non-public primary constructor, keeps its previous handling. - !name.startsWith("scala.") && declaresCopy(typeClass) && declaresProductPrefix(typeClass) + // Recognition answers a predicate for every Product reaching this module, including types it + // does not own, and reflecting over a companion or a case class resolves member descriptors. + // A type whose members reference absent classes must simply be declined here; the owning path + // reports the failure. Ambiguity stays loud: it means this module does own the type and cannot + // pick a constructor. + try { + val companion = companionOwner(typeClass, committed = false) + if (companion != null) findPrimaryConstructor(typeClass, companion) != null + else { + // A case class that cannot reach its companion, such as one declared inside a class or a + // method, is still a case class. Claim it so the codec reports the exact reason instead of + // leaving it to a generic object model that silently drops every property. A generated + // `copy` returning the declaring class together with a declared `productPrefix`, which + // `Product` otherwise supplies by default, is the compiler marker of a case class. + // Standard-library types keep their own mapping. A reachable companion whose constructor + // this module does not support, such as a varargs or non-public primary constructor, keeps + // its previous handling. + !name.startsWith("scala.") && declaresCopy(typeClass) && declaresProductPrefix(typeClass) + } + } catch { case _: LinkageError => false } } def caseClassCodec(typeRef: TypeRef[_], resolver: JsonTypeResolver): ObjectCodec[_] = { @@ -227,11 +237,9 @@ private[scala] object ScalaObjectModels { * only for a top-level companion, so a case class declared inside an `object` keeps them as * instance members of the companion singleton. */ - private final class CompanionOwner( - val owner: Class[_], - val singleton: Field, - val staticForwarders: Boolean - ) + private final class CompanionOwner(val owner: Class[_], val singleton: Field) { + def staticForwarders: Boolean = singleton == null + } // `fory-json` mirrors this companion rule in two places that must stay in sync: the // `ownerType + "$"` check in JsonCreatorInfo.buildDefaultInvokers, and the native-image @@ -250,7 +258,7 @@ private[scala] object ScalaObjectModels { if ( method.getName == "apply" && Modifier.isStatic(method.getModifiers) && !method.isBridge && !method.isSynthetic && method.getReturnType == typeClass - ) return new CompanionOwner(typeClass, null, true) + ) return new CompanionOwner(typeClass, null) index += 1 } val companionName = typeClass.getName + "$" @@ -267,7 +275,7 @@ private[scala] object ScalaObjectModels { } val field = singletonField(companionClass) if (!Modifier.isPublic(companionClass.getModifiers) || field == null) null - else new CompanionOwner(companionClass, field, false) + else new CompanionOwner(companionClass, field) } private def companionInstance(typeRef: TypeRef[_], companion: CompanionOwner): AnyRef = {