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.67% 81.68% +0.01%
==========================================
Files 455 455
Lines 64859 64891 +32
==========================================
+ Hits 52974 53009 +35
+ Misses 11885 11882 -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 Diff1 commit · Full page ✏️ Stop warning 20 on calls of open JS object methods86453bb · cknitt + Claude · Nov 01, 2026, 16:59 UTC · edited
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
|
|
@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". |
5e83724 to
afec64d
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>
ffd35ca to
86453bb
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. Callees that cannot return still warn, including field reads on them:
throw(e)(1),throw(e)["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, 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