Restore legacy (. ...) uncurried syntax with deprecation warning#8383
Restore legacy (. ...) uncurried syntax with deprecation warning#8383cknitt merged 5 commits intorescript-lang:masterfrom
(. ...) uncurried syntax with deprecation warning#8383Conversation
Accept the pre-v11 dotted uncurried syntax in function parameters, arguments, and type parameters so projects depending on libraries that still use it can parse again. Emit a `Warnings.Deprecated` on every occurrence, pointing at the leading dot, similar to the deprecation warnings rewatch emits for `bs-*` fields in rescript.json. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Rewatch suppresses warnings from external dependencies (users can't act on them), but the legacy `(. ...)` uncurried-syntax deprecation is a signal consumers need to see so they can report breakage upstream before the syntax is removed. Add a small allow-list in the stderr capture path for both the AST-parse phase and the compile phase that keeps warning blocks mentioning that specific deprecation while still dropping everything else from external packages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| #### :nail_care: Polish | ||
|
|
||
| - Allow builds while watchers are running. https://github.com/rescript-lang/rescript/pull/8349 | ||
| - Restore parsing of the legacy `(. ...)` uncurried syntax for backwards compatibility with libraries still on older ReScript versions; emit a deprecation warning when it is used. Rewatch also surfaces this specific deprecation when it originates from an external dependency so users can report breakage upstream. |
There was a problem hiding this comment.
| - Restore parsing of the legacy `(. ...)` uncurried syntax for backwards compatibility with libraries still on older ReScript versions; emit a deprecation warning when it is used. Rewatch also surfaces this specific deprecation when it originates from an external dependency so users can report breakage upstream. | |
| - Restore parsing of the legacy `(. ...)` uncurried syntax for backwards compatibility with libraries still on older ReScript versions; emit a deprecation warning when it is used. Rewatch also surfaces this specific deprecation when it originates from an external dependency so users can report breakage upstream. https://github.com/rescript-lang/rescript/pull/8383 |
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 129bf580ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .split("\n\n\n") | ||
| .filter(|block| block.contains(UNCURRIED_DOT_MARKER)) |
There was a problem hiding this comment.
Normalize line endings before splitting warning blocks
The filter assumes warning blocks are separated by "\n\n\n", but on Windows stderr commonly uses CRLF ("\r\n"), so this split does not break the stream into blocks. When an external dependency emits both the uncurried-dot deprecation and unrelated warnings, retain_critical_external_warnings will keep the entire stderr payload instead of only the critical block, effectively disabling suppression for those packages in Windows builds (this helper is used by both parse and compile warning paths).
Useful? React with 👍 / 👎.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
The "\n\n\n" block separator used by retain_critical_external_warnings assumes LF line endings. On Windows bsc emits CRLF, so the splitter would find no boundary and return the entire stderr — effectively disabling external-dep warning suppression for any package that emits the uncurried-dot deprecation alongside other warnings. Normalize CRLF → LF before splitting. Add a CRLF test that exercises the Windows-shaped input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
(. args)can parse. Every occurrence emitsWarnings.Deprecated(warning 3) pointing at the leading dot — similar in spirit to thebs-*field deprecations rewatch prints for legacy config.Background
rescript-lang/rescript#8211removed parser support for(. ...)because uncurried became the default. A lot of libraries (e.g.bs-css,rescript-nodejs, older packages) still ship that syntax, so consumers on v13 can't build them. This PR restores the parse (without restoring any runtime semantics — we still just ignore the dot) and leans on a deprecation warning to keep the incentive to migrate.Parser (compiler/syntax)
compiler/syntax/src/res_core.mlwarn_uncurried_dot_syntaxhelper that firesWarnings.DeprecatedviaLocation.prerr_warningat the location of the dot.parse_parameter,parse_parameters(Lparenbranch),parse_argument, andparse_type_parameteragain accept an optional leadingDot; each consumes it and emits the warning. AST output is identical to the dot-less form.compiler/syntax/src/res_grammar.ml: restoreDotinis_parameter_start,is_type_parameter_start, andis_argument_startso the delimited-list parsers still recognise it as a valid start token.Rewatch allow-list
Rewatch drops warnings from external (node_modules) packages in two places:
rewatch/src/build/parse.rs:130-190— stderr frombsc -bs-ast(where this parser warning actually fires).rewatch/src/build/compile.rs:941-950— stderr frombsccompile (defensive, for future type-check/codegen allow-list entries).Both branches now call
retain_critical_external_warnings: scans the stderr blob for the marker substring`(. ...)` uncurried syntax, splits on the\n\n\nblock boundary bsc uses between warnings, keeps only the matching blocks, and returns them to be printed. Unrelated warnings (unused values, etc.) in external deps remain hidden, so the blast radius is exactly the warning we care about.The layer that controls
-wflags (Config::get_warning_args) didn't need to change — it returnsvec![]for external deps, which leaves bsc on its built-in default where Warning 3 is active, so the warning fires on its own.Tests
tests/syntax_tests/data/parsing/grammar/expressions/uncurriedLegacyDot.res+…/typexpr/uncurriedLegacyDot.res— new snapshots covering(. x) => …,(. a, b) => …,(.) => …,f(. x),f(.), and(. int) => int.tests/syntax_tests/data/parsing/errors/other/expected/regionMissingComma.res.txt— snapshot refreshed: the stray.between params now also prints the deprecation warning before the "forgot a comma" error.rewatch/src/build/compile.rs— two unit tests for the allow-list filter.rewatch/tests/compile/18-external-dep-uncurried-dot.sh— integration test that stages a fakenode_modules/legacy-pkgcontaining both(. a, b) => …(must surface) and a stray unused binding (must stay suppressed); wired intosuite.sh.Test plan
make test-syntaxROUNDTRIP_TEST=1 make test-syntaxmake libcargo test --manifest-path rewatch/Cargo.tomlcargo clippy --manifest-path rewatch/Cargo.toml --all-targetsmake test-rewatch🤖 Generated with Claude Code