Align two error message wordings with go-jsonnet and C++ jsonnet - #1111
Closed
He-Pin wants to merge 2 commits into
Closed
Align two error message wordings with go-jsonnet and C++ jsonnet#1111He-Pin wants to merge 2 commits into
He-Pin wants to merge 2 commits into
Conversation
Motivation:
sjsonnet reported "Unknown extVar: <name>" while go-jsonnet and the C++
reference report "Undefined external variable: <name>". Tools and users
porting between implementations see divergent diagnostics for the same
program.
Modification:
Reword the failure in StdLibModule.extVarFunction, keeping sjsonnet's
[std.extVar] builtin prefix. Update the go_test_suite golden.
Result:
std.extVar("UNKNOWN") now fails with "[std.extVar] Undefined external
variable: UNKNOWN", matching the reference implementations' wording.
Motivation:
The spec desugars `assert e : msg` to raising `msg` as the error value,
and go-jsonnet / C++ jsonnet report the bare message ("RUNTIME ERROR:
<msg>"). sjsonnet prepended "Assertion failed: ", so the same program
produced divergent diagnostics across implementations. Asserts without a
message already reported "Assertion failed" everywhere and are unchanged.
Modification:
Drop the prefix at the three assert failure sites in Evaluator.scala
(top-level assert, object assert, object-comprehension assert) and
refresh the eight affected goldens across test_suite, go_test_suite and
new_test_suite.
Result:
`assert false : "Custom Message"` now fails with "Custom Message"
exactly, matching the reference implementations; non-string messages are
still manifested (e.g. `assert false : 42` fails with "42").
He-Pin
marked this pull request as draft
August 6, 2026 03:27
Contributor
Author
|
Closing: after discussion, sjsonnet's existing wording is preferred — the messages carry more context (the [std.extVar] builtin prefix, the 'Assertion failed:' marker) and wording parity with go-jsonnet is not a goal in itself. No change needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two small diagnostic-wording alignments, one commit each, found by four-way differential testing (sjsonnet vs go-jsonnet vs jrsonnet vs spec).
1.
std.extVarunknown variable (commit 1)sjsonnet.Error: [std.extVar] Unknown extVar: UNKNOWNsjsonnet.Error: [std.extVar] Undefined external variable: UNKNOWNRUNTIME ERROR: Undefined external variable: UNKNOWNsjsonnet's
[std.extVar]builtin prefix is retained.2. Assert messages verbatim (commit 2)
The spec desugars
assert e : msgto raisingmsgas the error value; go-jsonnet and C++ jsonnet report the bare message.assert false : "Custom Message"Assertion failed: Custom MessageCustom MessageCustom Messageassert 1==2 : '%s != %s' % [x, y]Assertion failed: foo was not equal to barfoo was not equal to barfoo was not equal to barassert false : 42(non-string)Assertion failed: 424242assert false(no message)Assertion failedAssertion failed(unchanged)Assertion failedAll three assert failure sites are updated (top-level assert, object assert, object-comprehension assert); eight goldens refreshed across
test_suite,go_test_suite,new_test_suite.Deliberately not included
std.formattoo-few/too-many-values wording: go-jsonnet's messages there are path-dependent (Too many values to format: 2, expected 1vsNot enough values to format: 1, expected more than 1), so exact alignment is not meaningful, and sjsonnet's current messages (too few values to format: 1, expected at least 2) are at least as precise. Left as is.Test plan
./mill 'sjsonnet.jvm[_].test'— all Scala versions (2.12.21 / 2.13.18 / 3.3.8) pass./mill __.checkFormat— clean