This addresses many IDE warnings resulting from the switch to JSpecify.
Also, this changes the behavior of exporting VmObject; there's no place
in our code that does not force a VmObject prior to export, so
the existing logic around handling nullable values has been removed.
This adjusts the doc comments in ref.pkl
* Fix incorrect code snippets
* Clean up examples
* Remove some sections
* Make phrasing consistent with the rest of the stdlib
## What changed
This updates the GraalVM install task so Windows installs use ZIP extraction instead of the Unix tar path. Windows now publishes the extracted directory by moving it into place, while macOS and Linux keep the existing tar extraction and symlink-based install flow.
## Why
BuildInfo.GraalVm downloads .zip archives on Windows, but InstallGraalVm always ran tar --strip-components=1 -xzf and then tried to install by creating a symbolic link. That combination is fragile on Windows: the archive format does not match the extraction command, and symlink creation can require special privileges.
The changes made in this commit are good, but we're going to kick this out
to the next release.
This is because:
1. There's a couple more issues around the `abstract` modifier that is not
implemented yet, and need design considerations
2. These are breaking changes, and we want to minimize the amount of breakages
for users.
3. The main branch is still the develop branch for Pkl 0.32
We will apply a re-revert of this commit after Pkl 0.32 is released.
The eval task, unlike other tasks, creates the evaluator when the property is evaluated, not just when the task is executed (in the `getEffective*` properties). The problem here is that property evaluation can and will happen before the task is executed, because Gradle needs property values for caching and sometimes task dependency information.
Therefore, if due to misconfiguration or due to intentional configuration the evaluator cannot be created - for example, when the `PklProject` module doesn't exist, but the project directory is configured - then the task will fail with an exception which looks _very_ similar to a regular execution exception, but which actually is not, because it is thrown not during task execution, but during preparation for the execution.
This distinction matters a lot when you rely on conditional task execution. If you use the `onlyIf` predicate on tasks to define a condition for the task execution, this predicate will be evaluated before the task actions are run, but *after* properties are evaluated. Thus, if property evaluation fails with an exception, it will *look* as if the `onlyIf` predicate is completely ignored and not even evaluated, and the task action is run regardless of the predicate.
This error mode is extremely confusing and is actually wrong: if I use `onlyIf` to gate the task execution, I don't want *any* of its logic to run. In fact, in my case I specifically use `onlyIf` on the evaluation task to only execute it if a prerequisite is met, and the same predicate guards a bunch of other tasks which prepare the Pkl project, so my Pkl project isn't even generated if the predicate fails. But due to this behavior of properties evaluation which are not controlled by `onlyIf`, the project is still attempted to be evaluated, and this results in a very unexpected build failure.
The solution is to ignore the evaluation exception, because for all intents and purposes, if the project can't be properly evaluated, it will fail during the task execution as it should, so the values of output properties don't really matter as the task will never be run.
These methods aren't implemented in `Set`, and don't really make sense because `Set` types can't be accessed by index.
Note: although this removes methods, this actually isn't a breaking change:
1. Calling `Set.findIndex()` currently throws an error around "cannot invoke abstract method"
2. `List` and `Set` are the only subclasses of `Collection`.
The following code isn't breaking at runtime, although static analysis tooling (like our IDE plugins) will now flag this as an error:
```pkl
myCollection: Collection
idx = myCollection.indexOf(1)
```
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>
This reverts the commits that enabled Gradle's configuration cache
feature.
IMO: this feature is too hard to use. We don't know if a task is valid
for the configuration cache until it runs, and it's very hard to tell if
something is safe when authoring Gradle code.
For example, our publish tasks are currently failing; I don't know how I
would fix this without running the publish task again on my dev machine.
Also, some of our build scripts become more brittle because of this; for
example, see
https://github.com/apple/pkl/blob/bb07589eae0b3195a589559a3245cbc12c29b394/build-logic/src/main/kotlin/BuildInfo.kt#L291-L296
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.
dependabot[bot]