Skip to content

Align two error message wordings with go-jsonnet and C++ jsonnet - #1111

Closed
He-Pin wants to merge 2 commits into
databricks:masterfrom
He-Pin:align-error-message-wording
Closed

Align two error message wordings with go-jsonnet and C++ jsonnet#1111
He-Pin wants to merge 2 commits into
databricks:masterfrom
He-Pin:align-error-message-wording

Conversation

@He-Pin

@He-Pin He-Pin commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Two small diagnostic-wording alignments, one commit each, found by four-way differential testing (sjsonnet vs go-jsonnet vs jrsonnet vs spec).

1. std.extVar unknown variable (commit 1)

message
sjsonnet before sjsonnet.Error: [std.extVar] Unknown extVar: UNKNOWN
sjsonnet after sjsonnet.Error: [std.extVar] Undefined external variable: UNKNOWN
go-jsonnet v0.22.0 RUNTIME ERROR: Undefined external variable: UNKNOWN

sjsonnet's [std.extVar] builtin prefix is retained.

2. Assert messages verbatim (commit 2)

The spec desugars assert e : msg to raising msg as the error value; go-jsonnet and C++ jsonnet report the bare message.

Program sjsonnet before sjsonnet after go-jsonnet
assert false : "Custom Message" Assertion failed: Custom Message Custom Message Custom Message
assert 1==2 : '%s != %s' % [x, y] Assertion failed: foo was not equal to bar foo was not equal to bar foo was not equal to bar
assert false : 42 (non-string) Assertion failed: 42 42 42
assert false (no message) Assertion failed Assertion failed (unchanged) Assertion failed

All 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.format too-few/too-many-values wording: go-jsonnet's messages there are path-dependent (Too many values to format: 2, expected 1 vs Not 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

He-Pin added 2 commits August 6, 2026 11:18
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
He-Pin marked this pull request as draft August 6, 2026 03:27
@He-Pin

He-Pin commented Aug 6, 2026

Copy link
Copy Markdown
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.

@He-Pin He-Pin closed this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant