Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #8921 +/- ##
==========================================
+ Coverage 81.74% 81.75% +0.01%
==========================================
Files 457 457
Lines 65713 65751 +38
==========================================
+ Hits 53715 53757 +42
+ Misses 11998 11994 -4
🚀 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 Diff2 commits · Full page Stop warning 20 on calls of open JS object methods872642a · cknitt + Claude · Nov 01, 2026, 16:59 UTC
Diagnostic removed (warning 20 on open object methods) @get external clipboardData: string => {..} = "clipboardData"
let text = clipboardData("event")["getData"]("text/plain")
let fromThrow = () => throw(Failure("boom"))["m"](Console.log("b"))
Warning number 20
-Ex.res:2:46-57
-this argument will not be used by the function.
-
-Warning number 20
Ex.res:3:51-66
this argument will not be used by the function.Review
✨ Stop warning 20 on chained calls of open JS object methods4dbf462 · cknitt + Claude · Nov 07, 2026, 06:35 UTC · new
Diagnostic removed (warning 20 on chained open object methods) @get external clipboardData: string => {..} = "clipboardData"
let text = clipboardData("event")["getData"]("text/plain")
let focus = clipboardData("event")["target"]["focus"]()
let fromThrow = () => throw(Failure("boom"))["m"](Console.log("b"))
Warning number 20
-Ex.res:3:55
-this argument will not be used by the function.
-
-Warning number 20
Ex.res:4:51-66
this argument will not be used by the function.Review
|
|
@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". |
ffd35ca to
86453bb
Compare
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 read adds the
method to the object's open row with a fresh type variable as its
type, and type_application takes a callee whose type is a fresh
variable as a function that never returns. The JS method does receive
its arguments. Fixes #8159.
The read of a field that the open row gains now records that field's
type, and the application skips the warning only when its callee is
that read. Callees that cannot return still warn, including field reads
on them: `throw(e)(1)`, `throw(e)["m"](x)`, `{"m": throw(e)}["m"](x)`.
Diagnostic removed (warning 20 on open object methods):
Example:
@get external clipboardData: string => {..} = "clipboardData"
let text = clipboardData("event")["getData"]("text/plain")
let fromThrow = () => throw(Failure("boom"))["m"](Console.log("b"))
Before (diagnostic):
Warning number 20
Ex.res:2:46-57
this argument will not be used by the function.
Warning number 20
Ex.res:3:51-66
this argument will not be used by the function.
After (diagnostic):
Warning number 20
Ex.res:3:51-66
this argument will not be used by the function.
Review: Why does the read record the field type instead of the typed tree
marking the callee? The Pexp_apply case clears the record, types the
callee and reads the record right away, comparing it with the callee's
type by physical equality, so only the callee's own read can match. It
passes the result to type_application as ~open_row_method. A typed-tree
flag would change Texp_object_get and the .cmt format for a warning.
Review: Can a read inside an argument replace the callee's record? No:
the check runs before any argument is typed. The fixture covers
arguments that read closed and open object fields, first or last, and
nested method calls.
Review: Why a Request_local key rather than a plain ref? Requests
type-checked concurrently on separate domains would share a ref, and
one could overwrite another's record between typing a callee and
checking it. An ounit test compiles open-object method calls in two
concurrent requests with warning 20 on and expects none; it failed on
every run with a shared ref.
Review: Does a method read before the call, `let o = f("e")` then
`o["m"]` and `o["m"]("used")`, need a record of every field? No: the
binding keeps the object type at the binding's level, below the level
of the later call, so the fresh-variable check does not apply.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gJavp7rWUAfzV9rjFj
Signed-off-by: Christoph Knittel <ck@cca.io>
Why:
Only the first read on an open object was exempt, so a chain still
reported warning 20: in `obj["target"]["focus"]()` the read of `target`
gives a fresh variable, and the read of `focus` on that variable looked
like a read on a value that cannot return (`throw(e)["m"]`). The same
held for a method call on a method's result,
`obj["querySelector"]("x")["focus"]()`.
A call of an open-row method now records its result type, and a read
whose object type is still a variable counts as an open-row read when
that variable is the recorded one. `throw(e)["a"]["m"](x)` still warns.
Diagnostic removed (warning 20 on chained open object methods):
Example:
@get external clipboardData: string => {..} = "clipboardData"
let text = clipboardData("event")["getData"]("text/plain")
let focus = clipboardData("event")["target"]["focus"]()
let fromThrow = () => throw(Failure("boom"))["m"](Console.log("b"))
Before (diagnostic):
Warning number 20
Ex.res:3:55
this argument will not be used by the function.
Warning number 20
Ex.res:4:51-66
this argument will not be used by the function.
After (diagnostic):
Warning number 20
Ex.res:4:51-66
this argument will not be used by the function.
Review: Can an argument's read leave a stale record for a read on the
call's result? No: the application sets the record after typing its
arguments, to its result type or None. The fixture covers chains whose
calls read open and closed object fields in their arguments.
Review: Why look at the record only when the object type is a variable?
A variable object is unknown either because it came from an open row
or because its expression cannot return. Only the first matches the
record, compared by physical equality right after typing the object.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gJnAkDvw6bdwGNa1pW
Signed-off-by: Christoph Knittel <ck@cca.io>
86453bb to
4dbf462
Compare
Calling a method of an open JS object type, e.g.
clipboardData(e)["getData"]("text/plain")withclipboardData: string => {..}, reported warning 20 "this argument will not be used by the function". The read adds the method to the open row with a fresh type variable as its type. The warning took that variable to mean "this function never returns", but the JS method does receive its arguments.A field read that adds a field to an open row now records that field's type. The application skips the warning only when its callee is that read. A call of such a method records its result type too, and a read on an object whose type is that recorded variable counts as an open-row read, so chains like
obj["target"]["focus"]()andobj["querySelector"]("x")["focus"]()do not warn either. Callees that cannot return still warn, including field reads on them:throw(e)(1),throw(e)["m"](x),throw(e)["a"]["m"](x),{"m": throw(e)}["m"](x). The record is request-local, so requests type-checked at the same time on other domains cannot overwrite it.Fixes #8159.
New tests:
unused_argument_object_method.res: open object method calls (also with field reads in the arguments, nested calls, chains of reads and calls, and a method read before the call) report nothing. The never-returning cases above keep warning 20.concurrent_open_object_method: two concurrent requests compile open object method calls with-w +20and expect no warning.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kbf3gJavp7rWUAfzV9rjFj