Several `Facts:` examples in the `pkl:base` standard library docs assert
statements that are false when evaluated. Since these examples are not
run by any test, the mistakes went unnoticed and are rendered verbatim
into the generated API docs, where they mislead readers.
The corrected examples:
- `String.isNotBlank`: `"\t\n\r".isNotBlank` was listed as holding, but
a string of only whitespace is blank, so it is false. Negated it to
match the neighboring `!"".isNotBlank` and `!" ".isNotBlank` examples.
- `DataSize.toBinaryUnit` / `toDecimalUnit`: the `mb`/`mib` lines
mirrored the `kb`/`kib` lines, but the identity only holds for adjacent
units. `1024.kb == 1000.kib` (both 1,024,000 b), whereas `1024.mb` is
1,024,000,000 b and `1000.mib` is 1,048,576,000 b, so they are not
equal. There is no clean round-number equivalent at this magnitude, so I
removed the two false lines; the remaining examples still demonstrate
the conversion.
- `Collection.any`: `!List(1, 2, 3).any((n) -> n.isEven)` is false
because 2 is even. Changed the list to `List(1, 3, 5)` so the negation
holds.
- `IntSeq.end`: the example read `IntSeq(2, 5).start == 5`, which
documents the wrong property and is false (`start` is 2). Corrected it
to `IntSeq(2, 5).end == 5`.
- `List.isDistinctBy` / `distinctBy`: `List("a", "b", "abc")` is not
distinct by length, since `"a"` and `"b"` both have length 1. Switched
to `List("a", "bb", "ccc")` so the distinctness examples hold.
- `Map`: `Map(...).values` returns a `List`, not a `Set`. Corrected the
expected type.
I verified that each corrected example evaluates to `true`, and that the
neighboring examples I kept still pass, using the released Pkl 0.31.1
binary. The changes are confined to doc-comment text, so formatting is
unaffected.
---------
Signed-off-by: Aditya Singh <adisin650@gmail.com>
This makes various improvements to the handling of frame slot vars, and
includes some bug fixes introduced by
https://github.com/apple/pkl/pull/1622
* Refactor SymbolTable to track for-generator and parameter slots in
each scope
* Execute let expressions in their own root node in some places
* Unify how frame slots are managed; they are all represented as
`FrameSlotVariable`, created in `AstBuilder`, and passed into
`SymbolTable`.
* Fix how let expressions are executed in custom this scopes (introduce
a new root node when needed)
Fixes#1614.
## Context
A non-abstract `class` (or `module`) was allowed to declare `abstract`
properties and methods.
Because such an enclosing type is instantiable, an `abstract` member
there can never be guaranteed
an implementation — so the contradiction surfaced only as a runtime
error when the member was
accessed (`Cannot invoke abstract method`), or not at all.
This makes it a compile-time error to declare an `abstract` member
unless its enclosing class or
module is also `abstract`. This is consistent with how Pkl already
rejects instantiating an abstract
class, and mirrors how Java and Kotlin treat abstract members.
## Before
```pkl
class Foo {
abstract bar: Int
}
res = new Foo { bar = 5 } // evaluated successfully (should fail)
```
```pkl
class Foo {
abstract function bar(): Int
}
res = new Foo {} // evaluated successfully; res.bar() failed only at runtime
```
## After
```
–– Pkl Error ––
Cannot define an abstract member in a non-abstract class.
2 | abstract bar: Int
^^^^^^^^
at Foo
A member can only be `abstract` if its enclosing class is also `abstract`.
```
## Implementation
- `AstBuilder` now validates, while building the AST, that a
non-abstract class/module declares no
`abstract` members. The check runs in both `visitClass` and
`visitModule`, and the error points at
the `abstract` keyword.
- Adds the `abstractMemberInNonAbstractClass` error message.
## Scope: classes and modules
The issue describes classes; I applied the same rule to modules as well,
since a module is a class
in Pkl and a non-abstract module is likewise directly evaluatable. Happy
to narrow this to classes
only if you'd prefer — it's a one-line change either way.
The `moduleMethodModifiers` pkl-doc test fixture declared an abstract
method at non-abstract module
level (relying on the old behavior); it's updated to an `abstract
module`, and its expected
documentation output is regenerated.
## Tests
- New `LanguageSnippetTests` error cases: abstract property in a class,
abstract method in a class,
and abstract member in a module.
- `./gradlew build` passes (`pkl-core` and `pkl-doc` included).
---------
Co-authored-by: Vinayak <vinayak@vama.app>
Co-authored-by: Daniel Chao <daniel.h.chao@gmail.com>
This implements HTTP redirect following ourselves.
The goal is:
1. All I/O is checked against `--allowed-resources` and
`--allowed-modules`, including HTTP redirects
2. HTTP rewrite rules can affect redirect following
3. HTTP headers can affect redirect following
---------
Co-authored-by: Islon Scherer <islonscherer@gmail.com>
The added snippet test originally produced error "A value of type
`Function2` cannot be exported."
This PR actually fixes the bug twice:
* By marking `ConvertProperty.render` as `hidden` so that it is skipped
when the enclosing object is exported. This broke any attempts to obtain
the module schema because this requires exporting all annotations on all
class properties.
* By changing the way that `VmUndefinedValueException.fillInHint()`
obtains the module URI to avoid obtaining the module schema (and
triggering the more expensive module schema generation process).
It also makes function-typed annotation properties in `pkl:Command` hidden to avoid similar issues there.
This introduces breaking changes for external readers are loaded:
1. In PklProject, relative paths are resolved relative to the enclosing
PklProject file (make behavior consistent with how other settings work)
2. Make CLI flags blow away any settings set on a PklProject
3. Introduce a new `workingDir` property, which defaults to the
PklProject dir
The overall goal is to make this behavior consistent with how other
settings work.
For example, relative paths for other evaluator settings are already
relative to the project directory.
Additionally, in every other case, CLI flags will overwrite any setting
set within PklProject.
Previously, `VmDynamic.isHiddenOrLocalProperty` didn't correctly
identify locals whose cache key is an `ObjectMember` instead of an
`Identifier`, causing `VmDynamic. getRegularMemberCount` to return an
incorrect value. This caused some renderers to produce incorrect output.
Resolves#1631
This introduces an optimization to write let expressions to frame slots
of the current frame, rather than transforming them into lambdas. As a
result, let expressions are _much_ faster to execute.
This replaces `ResolveVariableNode` and `ResolveMethodNode` with their resolution.
When we build the truffle node tree, we determine whether names resolve to:
* lexical scope
* base module
* implicit this
Then, we use this information to directly construct the underlying nodes (`ReadPropertyNode`, `ReadLocalPropertyNode`, etc).
Additionally, `AstBuilder` determines whether the property access must be const or not.
This introduces a `BaseModuleMembers` registry, which gets generated as part of Java compilation.
Use Locale.ROOT to apply the lowecase format. For URI scheme and host
locale-neutral casing is the semantically the correct choice. Added a
unit test that sets the default locale to tr-TR and that would fail
without the fix.
* Relax forbidden headers constraints
- remove restriction on browser-related headers
- allow any glob pattern (no need to end with `/` or `*`, because glob
patterns already require users to explicitly declare prefix matches if
that's the intention)
* Replace `List<Pair<, ...>>`; use `Map<String, ...>` instead
* Use glob pattern strings as an API throughout, instead of `Pattern`
(e.g. in `HttpClientBuilder`)
* Add HTTP headers to message passing API
* Add HTTP headers to executor API (introduces `ExecutorSpiOptions4`)
* Add tests for Gradle, CLI, and pkl-executor invocations
* Improve documentation
* Add `isGlobPattern` API to class `String` for in-language validation
of http headers
* Behavior change: make sure explicitly configured `User-Agent` in
`HttpClientBuilder` can be shadowed by headers (allows users to set
`--http-header "**=User-Agent: My User Agent"` and for this to be the
only user agent).
CC @kyokuping
Replace pkl-core's local nullness annotations with JSpecify annotations.
Enable NullAway checking for pkl-core packages except org.pkl.core.ast
and org.pkl.core.stdlib.
Notable code changes:
- Add a dedicated late-init constructor to VmTyped
- Move VmExceptionBuilder's fallback message derivation from withCause()
to build()
- Split VmException rendering between builder-provided messages and
string-backed messages
- Initialize MessageTransport handlers with default throwing handlers
- Update JSON helper collection types to allow nullable values JSON
arrays and objects can contain JSON null,
so the Java Map/List element types need to model nullable elements
explicitly
- Make public command transform APIs accept nullable transformed values
Command transforms can produce null for optional/default handling,
so the BiFunction and options-map element types now model that
explicitly
- Make ExecutorSpiException accept nullable message and cause
Existing call sites can pass nullable causes from Throwable.getCause()
- Remove JSR-305 semantics from `@LateInit`
JSpecify does not support the same type-qualifier-nickname pattern,
so `@LateInit` is now documentation plus a NullAway
constructor-initialization exemption
Out of scope:
- NullAway checking of org.pkl.core.ast and org.pkl.core.stdlib
- IntelliJ warnings related to `@LateInit` fields
- Removing the JSR-305 dependency, since concurrency annotations are
still in use
The version of local project dependencies should _always_ exactly match
up with what's declared in a PklProject.deps.json; any package in the
transitive dependency tree should always be delcaring the same import
too.
Closes#1591
Power assertions only work when the source section is available. If it
is unavailable, power assertions throw a ParserError (unexpected EOF on
an empty input) when re-parsing the expression for presentation.
ktfmt has much improved how it formats Kotlin code. Unfortunately, this
means that whenever we touch a single line in a Kotlin file, we get a
_lot_ more changes thanks to ratcheting now picking up this file for
formatting.
This PR just reformats every single Kotlin file so we don't have to deal
with this churn in future PRs that touch Kotlin code.
Motivation
- Enable correct NullAway analysis
- Pick up toolchain fixes and improvements
Toolchains
- Require JDK 25 for JVM toolchain (keep Java 17 runtime compatibility)
- Require Kotlin 2.3.20 for Kotlin toolchain (keep Kotlin 2.2 runtime
compatibility)
- Require JDK 25 for Gradle daemon JVM (via
gradle-daemon-jvm.properties)
- Fix javac and kotlinc warnings from toolchain upgrades
CI
- Bump GitHub workflows to JDK 25
Building Kotlin
- Bump Kotlin language level to 2.2 to match stdlib version
- Consolidate build logic into pklKotlinBase.gradle.kts
- Adopt modern Kotlin plugin syntax
- Fix new kotlinc warnings
- Update ktfmt to 0.62
- first version compatible with Kotlin 2.3.20
- changes formatting compared to 0.61
- Replace dependency resolution rule with BOM alignment
- rule was too broad and interfered with toolchain/runtime separation
Testing
- Expand matrix to JDK 25 (LTS) and 26
- Ensure each matrix task can be run independently
- Fix KotlinCodeGeneratorsTest and EmbeddedExecutorsTest on affected
JDKs
- Disable one test in CliCommandTest on affected JDKs (failure cause
unknown)
Compatibility fixes
- Fix reflective access in DocGenerator on affected JDKs
Build fixes
- Fix misuse of `task.enabled` vs. `report.required`
- Fix `gradlew tasks` on Windows
- Downgrade Spotless to 8.3.0 to (hopefully) work around sporadic
NoClassDefFoundError
Result
- NullAway runs correctly
- Broader JDK test coverage
- More reproducible and potentially faster builds
Prior to this change, this code would activate powers assertions /
instrumentation permanently:
```pkl
foo: String(contains("a")) | String(contains("b")) = "boo"
```
This is because the `contains("a")` constraint would fail, triggering
power assertions, but the subsequent check of the union's
`contains("b")` branch would succeed.
As observed in #1419, once instrumentation is enabled, all subsequent
evaluation slows significantly.
As with #1419, the fix here is to disable power assertions via
`VmLocalContext` until we know that all union members failed type
checking; then, each member is re-executed with power assertions allowed
to provide the improved user-facing error.
The loop unwraps nullables and constraints but breaks straight away
after a `typealias`. This means the nullable is missed. Removing the
`break` fixes it.
## Exception
```
org.pkl.core.PklException: –– Pkl Error ––
Command option property `foo` has unsupported type `String?`.
11 | foo: OptionalString
^^^^^^^^^^^^^^^^^^^
at <unknown> (file:///var/folders/xh/lmp1n6qj4m13t53cfmbqnkwh0000gn/T/junit-1378070630576324311/cmd.pkl)
Use a supported type or define a transformEach and/or transformAll function
```
The `choices` stream was consumed eagerly for metavar construction, then
captured in a lambda for later validation—which promptly fell over with
`IllegalStateException`. Materialise to a `List` straightaway.
* Forbid overlap of built-in and command-defined flag names
* Allow interleaving built-in and command-defined flags on the command
line
* List abbreviated flag names first, matching the behavior of built-in
flags
This enables defining declarative key and/or value transformations in
cases where neither `Class`- nor path-based converters can be applied
gracefully. It is also the only way to express transforming the
resulting property names in `Typed` objects without applying a converter
to the entire containing type, which is cumbersome at best.
SPICE: https://github.com/apple/pkl-evolution/pull/26
This adds power assertions to Pkl!
This implements the SPICE described in
https://github.com/apple/pkl-evolution/pull/29
This follows the power assertions style of reporting also found in
Groovy, Kotlin, and others.
* Literal values are not emitted in the diagram
* Stdlib constructors of literals like `List(1, 2)` are also considered
literals
Power assertions are added to:
* Failing type constraints
* Failing test facts
Power assertions are implemented as a truffle instrument to observe
execution.
When an assertion fails, the instrument is created and the assertion is
run again to observe facts.
This incurs runtime overhead to collect facts, but has no impact on code
in the non-error case.
---------
Co-authored-by: Islon Scherer <islonscherer@gmail.com>
Fixes#1309
The issue was that super calls were blocked inside let expressions
because:
1. The compiler's isClassMemberScope() check didn't skip over lambda
scopes created by let expressions
2. The runtime's findSupermethod() didn't traverse past VmFunction
owners to find the actual class prototype
Changes:
- SymbolTable.java: Updated isClassMemberScope() to skip lambda scopes
before checking if the parent is a class or module scope
- InvokeSuperMethodNode.java: Updated findSupermethod() to skip
VmFunction owners when looking for the class prototype
Added regression tests covering:
- Super method calls inside let expressions
- Super property access inside let expressions
- Nested let expressions with super calls
---------
Co-authored-by: Jen Basch <jbasch@apple.com>
Adds convenience methods `isNotEmpty` and `isNotBlank`. This borrows the
same methods from Kotlin.
This helps users write more fluent constraints, for example,
`foo.isNotEmpty.implies(bar)`.
Adds:
* List#isNotEmpty
* Map#isNotEmpty
* Set#isNotEmpty
* Mapping#isNotEmpty
* Listing#isNotEmpty
* String#isNotEmpty
* String#isNotBlank
Islon Scherer