Repository navigation
Conversation
Changelog🐛 Bug fix
Semantic Diff6 commits · Full page Use the applied signature for modules created by functor applicationcd32c86 · cknitt + Claude · Oct 27, 2026, 12:59 UTC
Diagnostic modified (completion on an applied functor) module type Ord = {
type t
let compare: (t, t) => int
}
module MakeSet = (O: Ord) => {
type t = array<O.t>
let add = (s: t, x: O.t) => s->Array.concat([x])
}
module IntSet = MakeSet({
type t = int
let compare = (a: int, b) => a - b
})
// IntSet.
-"detail": "(t, O.t) => array<O.t>"
+"detail": "(t, int) => array<int>"Diagnostic modified (hover on a type of an applied functor)
-type IntSet.t = array<O.t>
+type IntSet.t = array<int>Review
Keep the functor's file for members of an applied functor8e3cbb2 · cknitt + Claude · Oct 27, 2026, 17:45 UTC Codex review on #9084: the members of Link signature help type definitions to their declaring file934e2ec · cknitt + Claude · Oct 27, 2026, 20:22 UTC Codex review on #9084: signature help built its Type definition link from the caller's file, so for a type of an applied functor from another file it pointed into the caller. It now takes the file from the type's declaration, as hover does; the link function requires it, so no caller can fall back to the wrong file. Connect renamed members of an applied functor to their declarations0e4be65 · cknitt + Claude · Oct 27, 2026, 20:22 UTC Codex review on #9084: the members of an applied functor are registered under fresh stamps, so references to Connect nested modules of an applied functor to their declarations39c5bf6 · cknitt + Claude · Oct 27, 2026, 21:23 UTC Codex review on #9084: uses of a module path such as ✨ Resolve applied fields and constructors to the functor's declarationsc989f87 · cknitt + Claude · Oct 28, 2026, 07:45 UTC · new Codex review on #9084: an applied functor's record fields and constructors are read from its signature, where they have no location. Go to Definition returned that missing location, and rename added it as a reference, producing edits at character -1. Their lookups now go through the type they were renamed from, which has the body's locations, and fields and constructors without a location are not registered as references. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9084 +/- ##
==========================================
+ Coverage 81.45% 81.46% +0.01%
==========================================
Files 460 460
Lines 64645 64688 +43
==========================================
+ Hits 52657 52700 +43
Misses 11988 11988
🚀 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-9084 |
|
@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: eeb4965510
ℹ️ 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 on #9084: the members of `module M = Other.Make(X)` were registered in the current file with the locations of Other's functor body, so Go to Definition and hover's type-definition link combined Other's location with the current file. Their module path now records the functor, as it does for included modules, so both resolve to the functor's file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4cc7a1a84
ℹ️ 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 on #9084: signature help built its Type definition link from the caller's file, so for a type of an applied functor from another file it pointed into the caller. It now takes the file from the type's declaration, as hover does; the link function requires it, so no caller can fall back to the wrong file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9084: the members of an applied functor are registered under fresh stamps, so references to `IntSet.add` were no longer references to `MakeSet.add`; Find References missed them and renaming the declaration left them unchanged. Each renamed stamp now maps to the stamp it was renamed from: a use of the member is also recorded as a use of the original, and references starting from the member start from the original. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ca5862e19
ℹ️ 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 on #9084: uses of a module path such as `M.Nested` were recorded only under the renamed stamp. They are now also recorded under the original, like value uses. The originals are taken from the functor's result signature: when the argument is a path, the application's signature was already renamed by the compiler and doesn't carry the body's stamps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec52147ce9
ℹ️ 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".
Why:
Editor analysis modelled `module M = F(X)` by F's own body, so hover
and completion on M's items showed the types of the functor's body,
with the parameter's types such as `O.t`, instead of the argument's.
M is now modelled by the signature the compiler computed for the
application. That signature keeps the identifiers of F's body, whose
stamps already name the body's items, so it is renamed first.
Diagnostic modified (completion on an applied functor):
Example:
module type Ord = {
type t
let compare: (t, t) => int
}
module MakeSet = (O: Ord) => {
type t = array<O.t>
let add = (s: t, x: O.t) => s->Array.concat([x])
}
module IntSet = MakeSet({
type t = int
let compare = (a: int, b) => a - b
})
// IntSet.
Before (text): tests/analysis_tests/tests/src/AppliedFunctor.res
"detail": "(t, O.t) => array<O.t>"
After (text): tests/analysis_tests/tests/src/AppliedFunctor.res
"detail": "(t, int) => array<int>"
Diagnostic modified (hover on a type of an applied functor):
Before (text):
type IntSet.t = array<O.t>
After (text):
type IntSet.t = array<int>
Review: Why raise the stamp counter before renaming?
The renamed identifiers get stamps from the analysis process's own
counter, which starts low and would hand out stamps that the typed
tree already uses; above 2^40 they cannot meet one.
Fixes: #9072
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3
Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9084: the members of `module M = Other.Make(X)` were registered in the current file with the locations of Other's functor body, so Go to Definition and hover's type-definition link combined Other's location with the current file. Their module path now records the functor, as it does for included modules, so both resolve to the functor's file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9084: signature help built its Type definition link from the caller's file, so for a type of an applied functor from another file it pointed into the caller. It now takes the file from the type's declaration, as hover does; the link function requires it, so no caller can fall back to the wrong file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9084: the members of an applied functor are registered under fresh stamps, so references to `IntSet.add` were no longer references to `MakeSet.add`; Find References missed them and renaming the declaration left them unchanged. Each renamed stamp now maps to the stamp it was renamed from: a use of the member is also recorded as a use of the original, and references starting from the member start from the original. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9084: uses of a module path such as `M.Nested` were recorded only under the renamed stamp. They are now also recorded under the original, like value uses. The originals are taken from the functor's result signature: when the argument is a path, the application's signature was already renamed by the compiler and doesn't carry the body's stamps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9084: an applied functor's record fields and constructors are read from its signature, where they have no location. Go to Definition returned that missing location, and rename added it as a reference, producing edits at character -1. Their lookups now go through the type they were renamed from, which has the body's locations, and fields and constructors without a location are not registered as references. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Signed-off-by: Christoph Knittel <ck@cca.io>
ec52147 to
c989f87
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c989f879df
ℹ️ 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".
| | mty -> mty | ||
| in | ||
| record_renamed_stamps env.renamed_stamps functor_result renamed; | ||
| for_type_module ~name:module_name ~env renamed |
There was a problem hiding this comment.
When a functor result declares a module type, the new signature-based processing drops it: for_type_signature_item handles Sig_module but ignores Sig_modtype. For example, a functor body containing module type S = { let value: int } and module Nested: S = { let value = 1 } produces a renamed Nested whose Mty_ident points to an unregistered renamed S. Consequently, completion, hover, and definition lookup for Applied.Nested.value stop resolving, and Applied.S itself is unavailable to analysis. Previously, traversing the functor body registered S through Tstr_modtype. Handle module-type declarations when materializing the applied signature.
Useful? React with 👍 / 👎.
Fixes #9072.
Analysis modelled
module M = F(X)by F's own body, so hover and completion on M's items showed the body's parameter types (array<O.t>). M is now modelled by the signature the compiler computed for the application.That signature keeps the identifiers of F's body, whose stamps already name the body's items in the file's stamp table. So it is renamed with
Substfirst, from a stamp counter raised far above the stamps in a typed tree.New analysis test:
AppliedFunctor.Members of the applied module are also connected to their declarations in the functor's body, so Find References and rename cover both within a file, including nested modules.
Known limitation: across files, references and rename don't connect a use such as
StrSet.addtoaddin the functor's file. That needs external references for renamed members; go to definition and hover links do resolve to the functor's file.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3