Skip to content

Report an error for a format at the end of the string - #15

Merged
DIGarnier merged 6 commits into
mainfrom
fix-trailing-format
Nov 12, 2026
Merged

DIGarnier merged 6 commits into
mainfrom
fix-trailing-format

Conversation

@DIGarnier

@DIGarnier DIGarnier commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

A format specifier with nothing after it to apply to was dropped without
any message, cutting off the rest of the string:

  • [%string "%d"] produced ""
  • [%string "Sale: 50% off today"] produced "Sale: 50"

The same mistake in the middle of a string already reports
"Format is not followed by variable/expression". This reports it at the
end of the string as well, pointing at the %. Use %% for a literal
percent sign.

Strings that previously compiled with a trailing bare % will now fail
to compile; CHANGES.md has a note under Unreleased.

Tests: added test/ppx_string_interpolation_errors_test.ml, which runs
the rewriter on source snippets and checks the reported error.

Also in this PR, as separate commits:

  • Sync the opam file with opam-repository: require OCaml >= 4.11.0 as the
    published 1.1.0 does (raised there after the lower-bounds CI failed on
    OCaml 4.08 and 4.09 because of sedlex, see
    Package ppx_string_interpolation.1.1.0 ocaml/opam-repository#28114 (comment)),
    add ounit as a test dependency and run the tests with {with-test}.
  • Prepare the 1.2.0 release: CHANGES.md entries for 1.2.0, plus the
    missing 1.0.1 and 1.1.0 entries.

@DIGarnier
DIGarnier force-pushed the fix-trailing-format branch 3 times, most recently from 6cc5a36 to 45d6cc9 Compare November 12, 2026 23:10
@bn-d

bn-d commented Nov 12, 2026

Copy link
Copy Markdown

Add CI please https://github.com/bn-d/ppx_make/blob/main/.github/workflows/build.yml

let valid_test1 _ =
assert_expands
~expected:{|let _ = "Sale: 50% off today"|}
{|let _ = [%string "Sale: 50%% off today"]|}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use exp

A format specifier with nothing after it to apply to, such as
[%string "%d"] or [%string "50% off"], was dropped without any
message, cutting off the rest of the string. The same mistake in
the middle of a string already reports "Format is not followed by
variable/expression"; report it at the end of the string as well.

Add tests that run the rewriter on snippets and check its errors.

Signed-off-by: David Garnier <dgarnier4@bloomberg.net>
Require OCaml >= 4.11.0 as the published 1.1.0 package does. The bound
was raised in opam-repository after the lower-bounds CI jobs failed on
OCaml 4.08 and 4.09, where older sedlex versions do not provide a
compatible ppx driver:
ocaml/opam-repository#28114 (comment)

Also add ounit as a test dependency and run the tests with {with-test}.

Signed-off-by: David Garnier <dgarnier4@bloomberg.net>
Add CHANGES.md entries for 1.2.0 and the earlier 1.0.1 and 1.1.0
releases.

Signed-off-by: David Garnier <dgarnier4@bloomberg.net>
Build and test on Linux with OCaml 4.11, 4.14 and 5, and on macOS with
OCaml 5, and check formatting with ocamlformat 0.27.0. Actions are
pinned to commit SHAs and kept up to date by Dependabot.

Signed-off-by: David Garnier <dgarnier4@bloomberg.net>
Signed-off-by: David Garnier <dgarnier4@bloomberg.net>
ubuntu-latest now ships GCC 15, which compiles C as C23 by default,
and the OCaml 4.11 runtime does not build under C23.

Signed-off-by: David Garnier <dgarnier4@bloomberg.net>
@DIGarnier
DIGarnier marked this pull request as ready for review November 12, 2026 23:32

@bn-d bn-d left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@DIGarnier
DIGarnier merged commit f9b5866 into main Nov 12, 2026
6 checks passed
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