Skip to content

fix: std.format %c rejects multi-character strings - #1015

Merged
stephenamar-db merged 2 commits into
databricks:masterfrom
He-Pin:fix/format-c-single-char
Jun 24, 2026
Merged

stephenamar-db merged 2 commits into
databricks:masterfrom
He-Pin:fix/format-c-single-char

Conversation

@He-Pin

@He-Pin He-Pin commented Jun 24, 2026 •

Copy link
Copy Markdown
Contributor

fix: std.format %c rejects multi-character strings

Motivation

The %c format specifier should only accept a single-character string or a numeric codepoint. sjsonnet silently accepted multi-character strings (e.g. "%c" % "AB" → "AB"), while C++ jsonnet, go-jsonnet, and jrsonnet all correctly reject them.

Modification

  • Format.scala: When %c receives a Val.Str, validate codepoint count is exactly 1 using codePointCount. Multi-char or empty strings raise "%c expected 1-sized string got: N", matching C++ jsonnet's error format.
  • Added regression tests: error.format_c_multi_char_string.jsonnet, error.format_c_empty_string.jsonnet

Result

"%c" % "AB" and "%c" % "" now correctly error, matching all reference implementations.

Expression cpp-jsonnet go-jsonnet jrsonnet sjsonnet (before) sjsonnet (after)
"%c" % "A" "A" "A" "A" "A" ✅ "A" ✅
"%c" % "AB" ERROR ERROR ERROR "AB" ❌ ERROR ✅
"%c" % "" ERROR ERROR ERROR "" ❌ ERROR ✅
"%c" % 65 "A" "A" "A" "A" ✅ "A" ✅

He-Pin added 2 commits June 24, 2026 11:16
Motivation:
The %c format specifier should only accept a single-character string
or a numeric codepoint. sjsonnet silently accepted multi-character
strings (e.g. "%c" % "AB" → "AB"), while C++ jsonnet, go-jsonnet,
and jrsonnet all correctly reject them with an error.

Modification:
- Format.scala: When %c receives a Val.Str, validate it contains
  exactly 1 codepoint. Raise "%c expected 1-sized string got: N"
  if the codepoint count is != 1, matching C++ jsonnet's error
  format.

Result:
"%c" % "AB" now correctly errors instead of silently returning "AB".

Cross-implementation comparison:
| Expression        | C++ jsonnet           | go-jsonnet | jrsonnet                     | sjsonnet (before) | sjsonnet (after)            |
|-------------------|-----------------------|------------|------------------------------|-------------------|-----------------------------|
| "%c" % "A"        | "A"                   | "A"        | "A"                          | "A"               | "A"                         |
| "%c" % "AB"       | ERROR: 1-sized string | ERROR      | ERROR: 1 char string got 2   | "AB" ❌           | ERROR: 1-sized string ✅    |
| "%c" % ""         | ERROR                 | ERROR      | ERROR: 1 char string got 0   | "" ❌             | ERROR: 1-sized string ✅    |
| "%c" % 65         | "A"                   | "A"        | "A"                          | "A"               | "A"                         |
| "%c" % "世"       | "世"                  | "世"       | "世"                         | "世"              | "世"                        |
@stephenamar-db
stephenamar-db merged commit 84fb15c into databricks:master Jun 24, 2026
5 checks passed
He-Pin added a commit to He-Pin/sjsonnet that referenced this pull request Jun 24, 2026
…ged code behavior

PR databricks#1022 added format_c error test files (intended as part of PR databricks#1015's
%c validation coverage). However, PR databricks#1022's CI ran at 08:07 UTC, before
PR databricks#1015 was merged at 16:51 UTC. The golden files were generated from
code without %c rejection, so they contained success output instead of
error messages.

Two minutes after databricks#1015 merged, databricks#1022 was squash-merged — the stale
golden files entered master without a CI re-run on the combined state.

Fix both golden files to contain the actual sjsonnet.Error output:

- error.format_c_multichar.jsonnet.golden: "AB" → error message
- error.format_c_empty_string.jsonnet.golden: "" → error message
stephenamar-db pushed a commit that referenced this pull request Jun 24, 2026
…ehavior (#1031)

## Summary

PR #1022 added format_c error test files intended as part of PR #1015
`%c` validation coverage. However, PR #1022 CI ran at 08:07 UTC, before
PR #1015 was merged at 16:51 UTC. The golden files were generated from
code without `%c` rejection, so they contained success output instead of
error messages.

Two minutes after #1015 merged, #1022 was squash-merged. The stale
golden files entered master without a CI re-run on the combined state.

## Timeline

```
08:07  PR #1022 CI runs, master has NO %c rejection code
       "%c" % ""   outputs ""     golden correct at this time
       "%c" % "AB" outputs "AB"   golden correct at this time
       CI all green

16:51  PR #1015 merged, %c rejection code enters master
16:53  PR #1022 merged, stale golden files enter master with no CI re-run

After  All PRs branch from master with both %c rejection and stale goldens
       "%c" % ""   errors, golden "" is now wrong
       "%c" % "AB" errors, golden "AB" is now wrong
```

## Fix

Update both golden files to contain the actual `sjsonnet.Error` output:

| File | Before stale | After correct |
|---|---|---|
| `error.format_c_multichar.jsonnet.golden` | `"AB"` | `sjsonnet.Error:
[std.format] %c expected 1-sized string got: 2` |
| `error.format_c_empty_string.jsonnet.golden` | `""` | `sjsonnet.Error:
[std.format] %c expected 1-sized string got: 0` |

## Implementation behavior comparison

Behavior verified with go-jsonnet and jrsonnet latest local binary,
`jrsonnet 0.5.0-pre99`. Both implementations reject empty and
multi-character strings for `%c`, and accept single Unicode code points.

| Expression | go-jsonnet 0.21.0 | jrsonnet 0.5.0-pre99 | Expected
sjsonnet behavior |
|---|---|---|---|
| `std.format("%c", "")` | exit 1, `%c expected 1-sized string got: 0` |
exit 1, `%c expected 1 char string, got 0` | exit 1, `%c expected
1-sized string got: 0` |
| `std.format("%c", "AB")` | exit 1, `%c expected 1-sized string got: 2`
| exit 1, `%c expected 1 char string, got 2` | exit 1, `%c expected
1-sized string got: 2` |
| `std.format("%c", "A")` | exit 0, `"A"` | exit 0, `"A"` | exit 0,
`"A"` |
| `std.format("%c", "世")` | exit 0, `"世"` | exit 0, `"世"` | exit 0,
`"世"` |
| `std.format("%c", 127757)` | exit 0, `"🌍"` | exit 0, `"🌍"` | exit 0,
`"🌍"` |

## Verification

```
./mill "sjsonnet.jvm.2_13_18.test" -- "*FileTests*"
Tests: 102, Passed: 102, Failed: 0

rtk ./mill "sjsonnet.jvm[3.3.7]".test.testOnly sjsonnet.FileTests
Tests: 3, Passed: 3, Failed: 0

rtk ./mill "sjsonnet.jvm[3.3.7]".test
SUCCESS
```

## Test plan

- [x] FileTests.new_test_suite passes with corrected golden files
- [x] Full JVM Scala 3.3.7 test suite passes locally
- [x] go-jsonnet and jrsonnet behavior compared for `%c` empty,
multi-character, single ASCII, single Unicode, and numeric codepoint
cases
- [ ] CI green on all 4 build targets: js, jvm, native, wasm
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.

2 participants