Repository navigation
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8921 +/- ##
==========================================
+ Coverage 81.48% 81.49% +0.01%
==========================================
Files 460 460
Lines 64728 64760 +32
==========================================
+ Hits 52744 52779 +35
+ Misses 11984 11981 -3
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8921 |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a085ae910f
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b9449a508
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc2b5c9ccf
ℹ️ 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".
Changelog🐛 Bug fix
Semantic Diff6 commits · Full page ✏️ Stop warning 20 on calls of open JS object methods9d4f953 · cknitt + Claude · Oct 19, 2026, 19:49 UTC · edited
Diagnostic removed (warning 20 on open object methods) @get external clipboardData: string => {..} = "clipboardData"
let text = clipboardData("event")["getData"]("text/plain")
-Warning number 20
-2:46-57
-this argument will not be used by the function.
+none✏️ Keep warning 20 on object fields that cannot returnee77791 · cknitt + Claude · Oct 20, 2026, 13:31 UTC · edited
Diagnostic added (warning 20 on fields that cannot return) @get external clipboardData: string => {..} = "clipboardData"
let text = clipboardData("event")["getData"]("text/plain")
let fromFailwith = () => failwith("boom")["m"](Console.log("b"))
-none, on either call
+Warning number 20
+3:48-63
+this argument will not be used by the function.Review
✏️ Test object field reads in arguments of open JS object methods0ba40e7 · cknitt + Claude · Oct 20, 2026, 13:32 UTC · edited Arguments that read object fields, from closed or open objects, in first or last position or through a nested method call, keep the call of an open JS object method free of warning 20. The exemption is decided before any argument is typed, so their reads cannot replace the callee's. let closedFirst = clipboardData("e")["m"](closed["x"], "a")
let nested = clipboardData("e")["m"](clipboardData("f")["n"]("y"))✏️ Use throw in the unused-argument object method fixtureb17a067 · cknitt + Claude · Oct 20, 2026, 13:46 UTC · edited The fixture writes its never-returning expressions as let retained = clipboardData("e")
let first = retained["m"]
let second = retained["m"]("used")Review
✏️ Pass the open-row method check to type_application explicitlya38596e · cknitt + Claude · Oct 21, 2026, 04:32 UTC · edited The Pexp_apply case resets open_row_field_read before typing the callee, reads it right after and passes the result to type_application as ~open_row_method, so the global is no longer read where other expressions may have been typed in between. Behavior is unchanged. Keep the open-row field read marker request-local5e83724 · cknitt + Claude · Oct 21, 2026, 10:56 UTC Requests type-checked concurrently on separate domains shared the open_row_field_read ref, so one request could overwrite another's marker between typing a callee and reading it, and report warning 20 on an open JS object method call. The marker is now a Request_local key. A compiler request test compiles open-object method calls in two concurrent requests with warning 20 enabled and expects no warning; it failed on every run with the shared ref. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Why:
Calling a method read from an open JS object type, such as
`obj["getData"]("text/plain")` with `obj: {..}`, reported warning 20
"this argument will not be used by the function". The method's type is a
fresh type variable, which the warning took as a sign that the function
never returns, but the call passes its arguments to the JS method. Calls
whose function is a fresh type variable for other reasons, such as
`failwith("x")(1)`, still warn.
Diagnostic removed (warning 20 on open object methods):
Example:
@get external clipboardData: string => {..} = "clipboardData"
let text = clipboardData("event")["getData"]("text/plain")
Before (diagnostic):
Warning number 20
2:46-57
this argument will not be used by the function.
After (diagnostic): none
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
Why:
The exemption for open JS object methods covered every object field
read, so calls of fields that cannot return, such as
`failwith("boom")["m"](x)` or `{"m": failwith("boom")}["m"](x)`, lost
warning 20. The read now records the fresh variable it creates for a
field added to an open row, and the application skips the warning only
when its callee is that variable.
Diagnostic added (warning 20 on fields that cannot return):
Example:
@get external clipboardData: string => {..} = "clipboardData"
let text = clipboardData("event")["getData"]("text/plain")
let fromFailwith = () => failwith("boom")["m"](Console.log("b"))
Before (diagnostic): none, on either call
After (diagnostic):
Warning number 20
3:48-63
this argument will not be used by the function.
Review: Why a global slot rather than a typed-tree flag? The callee's read
is the last read typed before its application, and the slot is set on
every read and compared by physical equality with the callee's type, so
it only matches the callee's own read. A typed-tree field would change
Texp_object_get and the .cmt format for a warning.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
Arguments that read object fields, from closed or open objects, in first or
last position or through a nested method call, keep the call of an open JS
object method free of warning 20. The exemption is decided before any
argument is typed, so their reads cannot replace the callee's.
let closedFirst = clipboardData("e")["m"](closed["x"], "a")
let nested = clipboardData("e")["m"](clipboardData("f")["n"]("y"))
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
The fixture writes its never-returning expressions as `throw(Failure(...))`
instead of the deprecated `failwith`, so its expected output holds only
warning 20 once `failwith` reports warning 3. It also covers an open object
kept in a binding whose method is read before it is called: that call
reports no warning 20 either.
let retained = clipboardData("e")
let first = retained["m"]
let second = retained["m"]("used")
Review: Does the retained case need a record of every open-row field
variable? No: a binding keeps the object type at the binding's level,
below the level of the later call, so the call's fresh-variable check
does not apply and no warning is reported.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
The Pexp_apply case resets open_row_field_read before typing the callee, reads it right after and passes the result to type_application as ~open_row_method, so the global is no longer read where other expressions may have been typed in between. Behavior is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
Requests type-checked concurrently on separate domains shared the open_row_field_read ref, so one request could overwrite another's marker between typing a callee and reading it, and report warning 20 on an open JS object method call. The marker is now a Request_local key. A compiler request test compiles open-object method calls in two concurrent requests with warning 20 enabled and expects no warning; it failed on every run with the shared ref. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
8f403db to
5e83724
Compare
Calling a method of an open JS object type, e.g.
cd(e)["getData"]("text/plain")withcd: string => {..}, reported warning 20 "this argument will not be used by the function". The method's type is a fresh type variable, which the warning reads as "the function never returns", but the JS method does receive the argument. The warning is now skipped when the applied expression is an object property read (Texp_object_get); other fresh-variable cases such asfailwith("x")(1)still warn.Fixes #8159.
New super_errors fixture
unused_argument_object_method.rescovers both the object method call (no warning) and thefailwithcase (warning kept).🤖 Generated with Claude Code
https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo