PKL Config Scala#1076
Conversation
c101308 to
bb90a58
Compare
bioball
left a comment
There was a problem hiding this comment.
Made a first pass. Thanks for the contribution, this looks like a great start!
| rootProject.file("buildSrc/src/main/resources/license-header.star-block.txt"), | ||
| "package ", | ||
| ) | ||
| } |
There was a problem hiding this comment.
Move this to pklScalaLibrary.gradle.kts
| val libs = the<LibrariesForLibs>() | ||
|
|
||
| dependencies { | ||
| api(libs.scalaLibrary) |
There was a problem hiding this comment.
Remove (this is redundant when setting scala.scalaVersion)
| api(libs.scalaLibrary) |
There was a problem hiding this comment.
Removed api(libs.scalaLibrary) — redundant now that the plugin wires it in.
| private var classInfo: PClassInfo[Any] = | ||
| PClassInfo.Unavailable.asInstanceOf[PClassInfo[Any]] | ||
|
|
||
| // Holds an optional converter, cached upon first retrieval. | ||
| private var converter: Option[Converter[Any, Any]] = None |
There was a problem hiding this comment.
These should be thread-safe, because ValueMappers can possibly be shared across threads.
Maybe put inside AtomicReference?
There was a problem hiding this comment.
Done. Merged the two vars into a single AtomicReference[Entry(classInfo, converter)] so the paired state updates coherently — no risk of a reader seeing a new classInfo with a stale converter. Under contention, threads may each compute a fresh converter (wasted getConverter lookup), but never see a torn state.
| override def create( | ||
| sourceType: PClassInfo[_], | ||
| targetType: Type | ||
| ): Optional[Converter[_, _]] = { |
There was a problem hiding this comment.
Two comments:
- This should return
Optional.emptyifsourceTypeis notPClassInfo.String - The return type should be
Optional[Converter[String, Enumeration]]right? Is there any case where the mapped value wouldn't be anEnumeration?
There was a problem hiding this comment.
Both points addressed, but the final shape diverged from the initial refactor. Enum params no longer go through the generic ConverterFactory.create path at all — they're dispatched directly via pattern match on Param.Type.ScalaEnum inside ScalaPObjectToCaseClass (see #12). So the question of whether to tighten the Optional[Converter[_, _]] signature is moot: the wildcard never participates in enum resolution anymore. The inner lookup is strongly typed: String => Enumeration#Value.
| // .forParametrizedType1[java.util.Collection[_], Array[_]]( | ||
| // x => x == PClassInfo.Collection | x == PClassInfo.Set | x == PClassInfo.List, | ||
| // pCollectionToMutableCollectionConv[Array](_.iterator.toArray[Any]) | ||
| // ) |
There was a problem hiding this comment.
Removed — both the commented-out block and its entry in all.
| * Some(list of `Enumeration#Value`) if the enumeration can be resolved, | ||
| * None otherwise | ||
| */ | ||
| def asCustomEnum: Option[List[Enumeration#Value]] = { |
There was a problem hiding this comment.
I'm not totally following the code here.
Why do we need to care about case classes here? Can we expect enums to simply look like this instead?
object SimpleEnum extends Enumeration {
val Aaa, Bbb, Ccc = Value
}And we can just use .values to access enum members during conversion.
Pkl only turns string literal unions into enums in every other language; this means that we don't need our enum values to be parameterized.
There was a problem hiding this comment.
Done — and the annotation is deleted, not just optional. Both forms work now:
// plain — previously unsupported
object Color extends Enumeration { val Red, Green, Blue = Value }
case class Palette(primary: Color.Value)
// nested-Val form, still supported, no annotation needed
object Shape extends Enumeration {
case class V() extends Val(nextId)
val Circle, Square = V()
}
case class Drawing(shape: Shape.V)Getting there required more than .getDeclaringClass. For the plain form the runtime class is scala.Enumeration$Value, shared across every Enumeration in the JVM — Java reflection can't distinguish owners. I explored three workarounds before settling on the current approach:
- Pre-substitute resolved
Valueinstances into thePObject's property map and delegate. Dead —PClassInfo.forValuethrows on non-Pkl values. - Swap enum-param
Types for a sentinelParameterizedTypecarrying theEnumerationreference. Dead —ValueMapperImpl.getConvertercallsReflection.getExactSubtype(...)before factory dispatch, which normalises customTypes back to their raw class. - Upstream patch (make
createnon-final / add a per-param hook). Out of scope for this PR.
What we landed on:
ConstructorParamResolverusesscala.reflect.runtime.universeonly to walk ctor params and recover the path-dependentEnumeration#Valueprefix. It emits pure-dataParams with a sealedParam.Type { Jvm | ScalaEnum }.ScalaPObjectToCaseClassforksPObjectToDataObject.ConverterImpland dispatches onParam.Typedirectly. ForScalaEnumit bypassesValueMapper.getConverter(which can't see past the erasure) and does a direct member lookup.
The fork is the main cost — roughly 150 lines duplicated from PObjectToDataObject.ConverterImpl.convert. Happy to collapse it if you'd accept an upstream patch exposing a per-param hook; that would reduce this module to a slim subclass. Architectural details are in the updated PR description.
5a85162 to
023c5be
Compare
023c5be to
c8a7a01
Compare
|
Really excited to see some momentum on Scala support for pkl! I think it could be a good alternative to lightbend/config and pureconfig if there were first-class Scala support. I see this PR specifically targets Scala 2.13. One thing that stands out to me is that it isn't clear what would happen to code that won't compile under Scala 3, e.g. Given that Scala 3 was released ~5 years ago and most actively maintained Scala projects now cross-compile, it might be worth attempting a Scala 3 cross-build before merging the initial Scala 2 support to surface any surprises early and to have clarity on whether there are any known blockers. Either way, if Scala support is being prioritized for an upcoming pkl release, I'd love to see a Scala 3 variant simultaneously land as part of that same release 🙂 (I'd be happy to take a look and help with that!) |
|
Hello @ktham. The plan is though to than gradually drift to macro based solution. @bioball correct me if i'm wrong. |
|
On And yeah; I think we do want to move towards generated deserializers for JVM languages. I think this would be implemented as annotation processors on the Java/Kotlin side. Maybe macros for Scala, although I think that might also be challenging if we want to support both Scala 2 and 3. |
|
For a library first released in 2026, I’d focus on Scala 3 and wouldn’t support Scala 2 anymore. I’m also worried that adding pkl-scala to this repo could make the Gradle build much heavier, like the internal IntelliJ plugin did before being moved out. (I’d also love to see pkl-gradle moved out, which currently dominates build time.) |
cf31d37 to
436274c
Compare
|
hi folks. thanks for the feedbacks. appreciate it following your recommendations i have rewritten it to scala 3 with |
bioball
left a comment
There was a problem hiding this comment.
Did another pass! This is looking great! I think the biggest problem here is that ClassTag doesn't give us type arguments of generic types.
| * }}} | ||
| */ | ||
| implicit class ConfigSyntaxExtension(val x: Config) extends AnyVal { | ||
| def to[T](implicit ct: ClassTag[T]): T = x.as[T](ct.runtimeClass) |
There was a problem hiding this comment.
This will lose any type arguments to T; e.g. to[Seq[Int]].
I think there's a couple ways around this:
- We copy what we do in Java (see org.pkl.config.java.JavaType), and offer a
ScalaTypeAPI - We implement this as a macro (for both scala 2 and 3 if we want to ensure continued 2.13 support)
- We use izumi-reflect's
Tag
There was a problem hiding this comment.
Ah, this also doesn't handle union types, or string literal types, e.g.
config.to["hello"] // downgrades to `to[String]`
config.to[Int | Boolean] // downgrades to `to[Object]`There was a problem hiding this comment.
yes.. definitely. i missed that as we don't have cases for it. but it is definitely a gap
the easiest fix would be to incorporate izumi. but.. are we ok for external dependency?
we definitely can solve on our own. it will require some boilerplate though if we want to support 2.13 as the fix that will work for scala 3 will not be accepted by 2.13 even if it's a 3.7 sadly. so we will need cross-version compilation with all the corresponding "benefits"
let me create tests and solve it for just 3 at this point and we will see what to do next
| /** | ||
| * Provides conversions between Java types backing PKL and Scala types, enabling seamless | ||
| * interoperability for configuration values within PKL. | ||
| */ |
There was a problem hiding this comment.
| */ | |
| */ | |
| //noinspection ScalaWeakerAccess |
| Conversion.of( | ||
| PClassInfo.Duration, | ||
| classOf[Duration], | ||
| (v: PDuration, _) => Duration.fromNanos(v.inNanos()).toCoarsest |
There was a problem hiding this comment.
| (v: PDuration, _) => Duration.fromNanos(v.inNanos()).toCoarsest | |
| (v: PDuration, _) => { | |
| val unit = v.getUnit match { | |
| case DurationUnit.NANOS => TimeUnit.NANOSECONDS | |
| case DurationUnit.MICROS => TimeUnit.MICROSECONDS | |
| case DurationUnit.MILLIS => TimeUnit.MILLISECONDS | |
| case DurationUnit.SECONDS => TimeUnit.SECONDS | |
| case DurationUnit.MINUTES => TimeUnit.MINUTES | |
| case DurationUnit.HOURS => TimeUnit.HOURS | |
| case DurationUnit.DAYS => TimeUnit.DAYS | |
| } | |
| Duration(v.getValue, unit) | |
| } |
| case v: Option[_] => v.map(s1.updateAndGet(_, t1, vm)) | ||
| case v: Optional[_] => v.toScala.map(s1.updateAndGet(_, t1, vm)) | ||
| case v => Option(s1.updateAndGet(v, t1, vm)) |
There was a problem hiding this comment.
You won't get either Option or Optional from Pkl.
Also: Some constructor feels more idiomatic
| case v: Option[_] => v.map(s1.updateAndGet(_, t1, vm)) | |
| case v: Optional[_] => v.toScala.map(s1.updateAndGet(_, t1, vm)) | |
| case v => Option(s1.updateAndGet(v, t1, vm)) | |
| case v => Some(s1.updateAndGet(v, t1, vm)) |
| clazz.getDeclaredConstructors.headOption | ||
| .filter(_ => { | ||
| // case classes all implement Product | ||
| clazz.getInterfaces | ||
| .exists(i => classOf[scala.Product].isAssignableFrom(i)) | ||
| }) | ||
| .toJava |
There was a problem hiding this comment.
Better to delegate to Java implementation (find constructor whose argument count matches field count) in case Scala synthesizes multiple constructors per case class.
| clazz.getDeclaredConstructors.headOption | |
| .filter(_ => { | |
| // case classes all implement Product | |
| clazz.getInterfaces | |
| .exists(i => classOf[scala.Product].isAssignableFrom(i)) | |
| }) | |
| .toJava | |
| // case classes all implement Product | |
| if (!clazz.getInterfaces.exists(i => classOf[scala.Product].isAssignableFrom(i))) { | |
| return Optional.empty() | |
| } | |
| super.selectConstructor(clazz) |
| * | ||
| * Scala 3 enum cases implement `scala.reflect.Enum`. When the enum also extends `java.lang.Enum` | ||
| * (the opt-in Java-compat form), `Class.isEnum` returns true and `pkl-config-java`'s | ||
| * `PStringToEnum` already handles them; this factory defers in that case. For the plain |
There was a problem hiding this comment.
I don't see any code here that defers?
| * hand-author Scala classes with other collection shapes (Set, Vector, mutable collections, etc.) | ||
| * can register their own converter factories. | ||
| */ | ||
| object ScalaConverterFactories { |
There was a problem hiding this comment.
| object ScalaConverterFactories { | |
| //noinspection ScalaWeakerAccess | |
| object ScalaConverterFactories { |
| assert(result == Person("via-builder", 7)) | ||
| } finally { | ||
| evaluator.close() | ||
| } |
There was a problem hiding this comment.
[nit] share the same evaluator across all tests
If we can't, then do Using(ConfigEvaluatorBuilder.preconfigured().forScala().build()) { evaluator =>
| named<MavenPublication>("library") { | ||
| pom { | ||
| url.set("https://github.com/apple/pkl/tree/main/pkl-config-scala") | ||
| description.set("Scala config library based on the Pkl config language.") |
There was a problem hiding this comment.
| description.set("Scala config library based on the Pkl config language.") | |
| description.set("Scala extensions for pkl-config-java, a Java config library based on the Pkl config language.") |
There was a problem hiding this comment.
this is a nice point to discuss
if you remember well my very first version of this lib was built using scala macros (2.13 though)
it didn't require pkl-config-java. than there was ask to unify and now we're closing the loop. which is fine, but i want to emphasize that bringing two toolsets for solving one problem is even more confusing.
per my understanding it's either
- pkl-config-java extension and then it uses Conversions/ConversionFactories and the stuff all over the stack
- or it's pure scala with no reflection at all - macro/shapeless/magnolia/izumi - this kind of stack, no java reflection no dependency on pkl-config-java
what do you think?
| includeEngines("scalatest") | ||
| testLogging { events("passed", "skipped", "failed") } | ||
| } | ||
| } |
There was a problem hiding this comment.
Not necessarily blocking this PR: if our goal is to support both Scala 2.13 and Scala 3, we should be testing the library from both targets.
There was a problem hiding this comment.
i think we should produce scala 3..
scala 2.13 consumers can pull scala 3 library
https://www.scala-lang.org/blog/state-of-tasty-reader.html
so at this point we can lock at 3.7 and scale ro 2.13 and 3.8 later
| @file:Suppress("HttpUrlsUsage", "unused") | ||
|
|
There was a problem hiding this comment.
| @file:Suppress("HttpUrlsUsage", "unused") |
436274c to
9e7fae3
Compare
Drops the scala-reflect dependency and the bespoke ConstructorParamResolver / Param / ScalaPObjectToCaseClass plumbing that existed only to recover path-dependent Enumeration#Value types past Java reflection erasure. Reverts pObjectToCaseClass to the inline PObjectToDataObject anonymous subclass shape used by pkl-config-kotlin. Phase-1 scope; Scala 3 enum support follows in a later commit on this branch.
Build:
- Bump scala = "3.3.7" (LTS); switch scala-library, scalatest,
scalatestplus-junit, diffx artifacts to their _3 variants
- Drop -Xsource:3 (Scala 2 dialect flag); -release:N alone enforces
bytecode level on the Scala 3 compiler
Source: zero changes from Scala 2.13 source — the prior -Xsource:3
discipline already had the module on the Scala 3 dialect.
Adds PStringToScalaEnum factory for idiomatic `enum Foo { case A, B }`,
which Class.isEnum doesn't recognize (Scala 3 only sets ACC_ENUM when
the user opts in via `extends java.lang.Enum`). Looks up cases via the
companion module's synthetic values() method. Defers when Class.isEnum
is true so pkl-config-java's stock PStringToEnum handles the
Java-compat form, avoiding double-handling.
No scala-reflect dependency.
…ing enum lookup - Add forScala() extensions on ConfigDecoderBuilder and ConfigDecoder, matching pkl-config-kotlin's five-receiver surface. - Switch PStringToScalaEnum's internal store from Map to VectorMap so the error message lists candidate cases in enum-declaration order. - Extract dedicated `idiomatic Scala 3 enum routes through PStringToScalaEnum` test (was previously buried in the kitchen-sink test). - Strengthen the unknown-enum-value test to assert declaration order. - Add regression-pin test covering inherited Java conversions: BigInteger, BigDecimal, URI, URL, Path, File, Character, Bytes, DataSize, Listing, Mapping. Catches accidental factory-ordering regressions and documents that primitive Char isn't covered (only java.lang.Character is).
9e7fae3 to
446c417
Compare
…design objects ScalaConversions and ScalaConverterFactories expose val members that are only referenced internally from .all but are public-by-design (users may register a subset). IntelliJ flags these as ScalaWeakerAccess; suppress to silence the noise. The pklScalaLibrary @file:Suppress("HttpUrlsUsage", "unused") was carried over from a template — file has no HTTP URLs and no unused declarations.
Main module already uses Scala 3 idiom (`*` for import wildcards, `?` for type wildcards); the test files lagged. Sweep imports and `Tuple2[_, _]`.
The "this factory defers" wording read like a method call; rewrite to spell out the actual mechanism (return Optional.empty so the chain falls through). POM description now mirrors pkl-config-kotlin's, framing the artifact as extensions to pkl-config-java rather than a standalone library.
Pkl never emits Option or java.util.Optional values — nullability surfaces as PNull. The two reflective branches were copy-pasted defensively and could never fire. The remaining catch-all is reachable only after PNull/null is ruled out, so Option(...) was redundant null-checking; Some(...) makes the intent explicit.
getDeclaredConstructors.headOption picked an arbitrary constructor when a case class declares secondary constructors (user-written or synthesized). Filter on the Product interface, then defer to the Java base class's selectConstructor, which picks by argument count and matches the existing pkl-config-java policy. Drops the now-unused OptionConverters import.
…ation Both converters previously collapsed the value to nanos and let toCoarsest reverse-engineer a unit by checking divisibility. Use PDuration's own unit when the value fits in a Long (the common case for Pkl-emitted durations); fall back to nanos+toCoarsest only for fractional values. As a side effect, durations like `5000000.ns` now round-trip as NANOSECONDS instead of being re-coarsified to MILLISECONDS — equivalent length, faithful to the source.
Reviewer's suggested wording ("Scala extensions for pkl-config-java...")
was a partial framing — the artifact does delegate to pkl-config-java but
also exposes Scala-native API surface (case-class targeting, syntax
package, Scala 3 enum support). Keeping the original.
The Try { ... }.toOption pattern caught Throwable (including OOMEs and
StackOverflows) and swallowed the cause, leaving "no converter found"
upstream as the only signal of a misconfigured classpath. The block can
only realistically throw four things — ClassNotFoundException,
NoSuchFieldException, NoSuchMethodException, IllegalAccessException — all
ReflectiveOperationException. Catch that, then rethrow as
ConversionException with the failing class name and the original cause.
…FunSuite Both specs now build their evaluator (and dependent fixtures) inside withFixture per test, with deterministic close in finally. ScalaObjectMapperSpec's fixture is the ConfigEvaluator; PPairToScalaTupleSpec's bundles the evaluated PModule and the configured ValueMapper since every test consumes both. The ConfigEvaluatorBuilder-extension test stays separate — its purpose is to exercise the builder path, not the evaluator one — and now uses scala.util.Using for the same try-with-resources discipline as withFixture.
Purpose
Adds
pkl-config-scala— the binding/runtime layer that lets hand-authored Scala case classes consume Pkl configs. Mirrorspkl-config-kotlin. A follow-up will addpkl-codegen-scalaso users can either hand-write or generate their Scala types.Built on Scala 3.3.7 (LTS). Consumable from Scala 2.13 callers via
-Ytasty-reader— see Scala 2.13 cross-support below.What's covered
Scala-specific mappings added by this module:
class Foo { x: Int }case class Foo(x: Int)Durationscala.concurrent.duration.Duration,FiniteDurationInt(epoch) /String(ISO-8601)java.time.InstantString/Regexscala.util.matching.RegexPair<A, B>(A, B)/Tuple2[A, B]List<T>,Set<T>,Listing<T>,Collection<T>Seq[T]Map<K, V>,Mapping<K, V>immutable.Map[K, V]T?Option[T]enum Foo { case A, B }— both plain and Java-compatenum Foo extends java.lang.Enum[Foo]formsInherited transparently from
pkl-config-java(regression-pinned inScalaObjectMapperSpec):Int→java.math.BigInteger,java.math.BigDecimal,Byte,Short,Int,Float,DoubleFloat→Float,BigDecimalString→java.lang.Character,java.net.URI,java.net.URL,java.nio.file.Path,java.io.File,PatternBytes→Array[Byte]DataSize,Duration,Regexidentity passthroughsenumviapStringToEnumIntentionally not covered
Users with hand-authored types register their own converter factories:
scala.Enumeration— out of scope. The Scala 2.13 reflection plumbing it required is what motivated removing the earlier enum path; with Scala 3 enums supported natively, the Scala 2 form is dead weight.Set[T],Vector[T], mutable collections — matches the minimal default set inpkl-config-java/pkl-config-kotlin.Char— Java'spStringToCharacterregisters forjava.lang.Characteronly; case classes with aCharfield need to usejava.lang.Characterinstead, or register a(String, char.class)conversion.Module shape
Mirrors
pkl-config-kotlin:ScalaConverterFactories.pObjectToCaseClassisnew PObjectToDataObject { override selectConstructor }— same anonymous-subclass pattern asKotlinConverterFactories.pObjectToDataObject.ScalaConversionscarries scalar conversions (Instant / Duration / Regex), analogous toKotlinConversions.forScala()extensions onValueMapperBuilder/ConfigEvaluatorBuilder/ConfigEvaluator/ConfigDecoderBuilder/ConfigDecoder, plusConfig.to[T]. Matches the five-receiver surface offorKotlin()inConfigExtensions.kt.Option,Pair,Map,Seq) go through a smallCachedConverterFactorieshelper for per-type-arg caching. Same idea as Kotlin'sPPairToKotlinPair, factored to keep the four factories declarative.PStringToScalaEnumhandles idiomaticenum Foo { case A, B }by reading the companion module's syntheticvalues()method into an order-preservingVectorMap. For the opt-inenum Foo extends java.lang.Enum[Foo]form,Class.isEnumreturns true andpkl-config-java's stockPStringToEnumhandles it; our factory defers in that case to avoid double-handling.No
scala-reflectdependency.Scala 2.13 cross-support
Built for Scala 3.3.7. Consumable from Scala 2.13 callers via the TASTy reader:
Works for the 2.13 consumer:
forScala()builder extensions andConfig.to[T]— kept asimplicit class(pre-Scala 3 form), TASTy-readable.scala.Product, which is whatpObjectToCaseClasschecks for.Option/Pair/ regex / duration converters — usescala.collection.immutable.*, common to both Scala versions.Does not work:
Enumeration— never supported by this module.enum— 2.13 has noenumkeyword;PStringToScalaEnumis dead-but-harmless code on the consumer side.A follow-up could wire a smoke-test module that compiles against Scala 2.13 with
-Ytasty-readerand consumespkl-config-scala_3, to catch TASTy-reader regressions automatically.Notes
pkl-config-java/pkl-config-kotlin. Users register their own forSet/Vector/ mutable collections.TrieMap, ScalaOption+.toJavaat Java boundaries). Java types kept only where the API demands them (java.lang.reflect.*,MethodHandle,Optionalreturn).libraryvsfatJarambiguity thatpkl-config-kotlin.gradle.ktsworks around with manual XML injection) will be addressed in a follow-up; not required for this PR's scope.