Repository navigation
Conversation
Changelog🐛 Bug fix
Semantic Diff18 commits · Full page Complete the constructors of extensible variantse751edb · cknitt + Claude · Oct 27, 2026, 13:06 UTC
Diagnostic modified (completion of an extensible variant) type attr = ..
type attr += Str(string) | Num(int)
// let a: attr = Nu
-Nullable, Null
+Num(_), Nullable, NullDiagnostic modified (exhaustive switch snippet) let value: attr = Str("s")
// switch valu
switch value {
| Str(_) => %todo
| Num(_) => %todo
| Flag => %todo
| _ => %todo
}Review
Match extension constructors by type and name them by module path5639dff · cknitt + Claude · Oct 27, 2026, 17:50 UTC Codex review on #9085: extensions were matched to an open type by its last name only, so Name extension constructors relative to where completion runsf2fd425 · cknitt + Claude · Oct 27, 2026, 20:29 UTC Codex review on #9085: extension constructors were named relative to the root of their file and to the file of the completed value. Inside the declaring module that inserted Qualify extension constructors of namespaced modules by source patha62bfe5 · cknitt + Claude · Oct 27, 2026, 21:45 UTC Codex review on #9085: the module name of another file comes from its compiled name, which in a namespaced project is Offer extensions in scope at the cursor and from interface modulese6a28af · cknitt + Claude · Oct 28, 2026, 07:47 UTC Codex review on #9085: an extension of the edited file declared after the cursor was offered although it isn't in scope there, and an extension inside a module of an interface ( Qualify extensions of nested interface modules and complete their payloads52ee62d · cknitt + Claude · Oct 29, 2026, 06:34 UTC Codex review on #9085: an extension two modules deep in an interface got only the outer module in its path, because the signature walk didn't extend the module path when entering a module, and completion inside the payload of a qualified constructor such as Type extension payloads in their own file and match qualified constructorsb03db7b · cknitt + Claude · Oct 29, 2026, 10:10 UTC Codex review on #9085: a payload type declared next to the extension, as in Look up unqualified extension payloads in the scope at the cursord56bea1 · cknitt + Claude · Oct 29, 2026, 10:33 UTC Codex review on #9085: ranking same-named extension constructors let an open outrank a local extension declared after it, so after Keep a catch-all when expanding or switching on an open typefb97e50 · cknitt + Claude · Oct 29, 2026, 18:02 UTC
Codeaction modified (Expand catch-all) type attr = ..
type attr += Str(string) | Num(int)
let f = (value: attr) =>
switch value {
| Str(_) => "str"
| _ => "other"
}
-Num(_)
+Num(_) | _Codeaction modified (Exhaustive switch) type attr = ..
type attr += Str(string) | Num(int)
let value: attr = Str("s")
value
switch value {
| Str(_) => %todo
| Num(_) => %todo
+| _ => %todo
}Review
Resolve an open's extension constructor only through its module347aa66 · cknitt + Claude · Oct 29, 2026, 18:03 UTC
Completion modified (extension payload after an open) type attr = ..
module A = {
type attr += Value(bool)
}
type attr += Value(option<int>)
open A
let x: attr = Value()
-option<int>, the payload of the local Value
+bool, the payload of A.Value: true, falseResolve module aliases in the path of an extension constructorfbbb47d · cknitt + Claude · Nov 01, 2026, 06:37 UTC
Completion modified (extension payload through a module alias) type attr = ..
module B = {
type attr += Value(option<int>)
}
type attr += Value(bool)
module Alias = B
let x: attr = Alias.Value()
-bool, the payload of the local Value
+option<int>, the payload of B.Value: None, Some(_)Completion modified (extension payload after an open of an alias) type attr = ..
module B = {
type attr += Value(option<int>)
}
module Alias = B
let v: attr = B.Value(None)
open Alias
switch v { | Value() }
-Fatal error: exception Invalid_argument("List.nth")
+option<int>, the payload of B.Value: None, Some(_)Review
Name extension constructors in code actions from the action's scope7735c23 · cknitt + Claude · Nov 01, 2026, 06:37 UTC
Codeaction modified (Exhaustive switch) type attr = ..
type attr += Str(string)
let value: attr = Str("s")
module Nested = {
type attr += Inner
value
type attr += AfterAction
}
switch value {
| Str(_) => %todo
-| Nested.Inner => %todo
-| Nested.AfterAction => %todo
+| Inner => %todo
| _ => %todo
}Codeaction modified (Expand catch-all) type attr = ..
type attr += Str(string) | Num(int)
let value: attr = Str("s")
let f = () =>
switch value {
| Str(_) => "str"
| _ => "other"
}
module Nested = {
type attr += Inner
}
-Num(_) | Nested.Inner | _
+Num(_) | _Review
Look up every extension constructor path in one walk of the scopea2c632c · cknitt + Claude · Nov 01, 2026, 07:10 UTC
Completion modified (qualified extension payload after an open) type attr = ..
type color = Red | Blue
module Outer = {
module Inner = {
type attr += Deep(bool)
}
}
module Inner = {
type attr += Deep(option<color>)
}
open Outer
let x: attr = Inner.Deep()
-option<color>, the payload of the local Inner.Deep
+bool, the payload of Outer.Inner.Deep: true, falseReview
Show signature help for qualified extension constructorsed70932 · cknitt + Claude · Nov 01, 2026, 07:10 UTC
Signaturehelp modified (qualified extension constructor) type attr = ..
module Sub = {
type attr += Toggle(bool)
}
let toggled: attr = Sub.Toggle(true)
-null
+Sub.Toggle(bool), parameter 0 activeKeep the completion scope and an open type's catch-all in Type_utilsaeda3bf · cknitt + Claude · Nov 01, 2026, 07:11 UTC
Test extensions of interface modules in one fixture284953f · cknitt + Claude · Nov 01, 2026, 07:12 UTC
✨ Resolve the written constructor path in signature helpc41f3c8 · cknitt + Claude · Nov 01, 2026, 07:54 UTC · new
Signaturehelp modified (same-named extension constructors) type attr = ..
module A = {
type attr += Value(bool)
}
module B = {
type attr += Value(option<int>)
}
open B
let b: attr = B.Value(None)
let unqualified: attr = Value(None)
-A.Value(bool)
+B.Value(option<int>)✨ Offer extensions of expression-local modules only in their block0e45dd0 · cknitt + Claude · Nov 01, 2026, 07:57 UTC · new
Completion modified (extension of an expression-local module) type attr = ..
type attr += Str(string)
let make = () => {
module Local = {
type attr += InLocal
}
let inBlock: attr = InL
Local.InLocal
}
let afterMake: attr = InL
-Local.InLocal, Local.InLocal
+Local.InLocal, nothingCodeaction modified (Expand catch-all after the block) let w = switch value {
| Str(_) => 1
| _ => 2
}
-Local.InLocal | _
+_Review
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9085 +/- ##
==========================================
+ Coverage 81.72% 81.80% +0.08%
==========================================
Files 460 460
Lines 64817 65145 +328
==========================================
+ Hits 52969 53291 +322
- Misses 11848 11854 +6
🚀 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-9085 |
|
@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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98ecc79d93
ℹ️ 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 #9085: extensions were matched to an open type by its last name only, so `A.t` got the constructors of `B.t`, and a constructor declared in a nested module was inserted without its module, which doesn't compile because an extension constructor can't be found from the expected type. An extension now records the type it extends and is matched by that type's file and stamp, and it is named by its module path, for example `Sub.Flag`; a prefix may match the last part. 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: f678bb32e5
ℹ️ 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 #9085: extension constructors were named relative to the root of their file and to the file of the completed value. Inside the declaring module that inserted `Sub.Flag`, which is not in scope there, and for a value imported from another file the constructors were not qualified with its module, while extensions in the edited file were missed. A completion request now records the edited file and the module path at the cursor; constructors of that file drop the module path shared with the cursor, constructors of other files get their module's name, and extensions are collected from the edited 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: 56cff12ba2
ℹ️ 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 #9085: the module name of another file comes from its compiled name, which in a namespaced project is `Base-MyNamespace`, so completion inserted `Base-MyNamespace.Flag`. The name is turned into its source path `MyNamespace.Base` first. 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: 3408b27a6e
ℹ️ 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 #9085: extensions were matched to an open type by its last name only, so `A.t` got the constructors of `B.t`, and a constructor declared in a nested module was inserted without its module, which doesn't compile because an extension constructor can't be found from the expected type. An extension now records the type it extends and is matched by that type's file and stamp, and it is named by its module path, for example `Sub.Flag`; a prefix may match the last part. 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>
3408b27 to
84e1386
Compare
Codex review on #9085: extension constructors were named relative to the root of their file and to the file of the completed value. Inside the declaring module that inserted `Sub.Flag`, which is not in scope there, and for a value imported from another file the constructors were not qualified with its module, while extensions in the edited file were missed. A completion request now records the edited file and the module path at the cursor; constructors of that file drop the module path shared with the cursor, constructors of other files get their module's name, and extensions are collected from the edited 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 #9085: the module name of another file comes from its compiled name, which in a namespaced project is `Base-MyNamespace`, so completion inserted `Base-MyNamespace.Flag`. The name is turned into its source path `MyNamespace.Base` first. 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 #9085: an extension of the edited file declared after the cursor was offered although it isn't in scope there, and an extension inside a module of an interface (`module Sub: { type attr += Flag }`) was never registered, because only typed-tree extensions were. The completion scope now records the cursor position and skips later extensions of the edited file, and extensions in signatures are registered too. 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: 84e1386175
ℹ️ 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".
…loads Codex review on #9085: an extension two modules deep in an interface got only the outer module in its path, because the signature walk didn't extend the module path when entering a module, and completion inside the payload of a qualified constructor such as `Sub.Toggle(...)` found no constructor, because the payload position keeps only `Toggle`. Signature modules now extend the path, and payload lookup also matches the last part of a qualified constructor name. 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 |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cb8834566
ℹ️ 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".
…ctors Codex review on #9085: a payload type declared next to the extension, as in `type Base.attr += Color(color)` with a local `color`, was resolved in the open type's file, so payload completion lost it; and of two same-named extensions, `B.Value(...)` completed the payload of the first one, `A.Value`, because the payload position kept only `Value`. Each extension constructor now carries the environment of its file, and the payload position keeps the written path: the constructor sharing the longest suffix with it is meant, ties going to the most recent open, then to the last declared one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Signed-off-by: Christoph Knittel <ck@cca.io>
…loads Codex review on #9085: an extension two modules deep in an interface got only the outer module in its path, because the signature walk didn't extend the module path when entering a module, and completion inside the payload of a qualified constructor such as `Sub.Toggle(...)` found no constructor, because the payload position keeps only `Toggle`. Signature modules now extend the path, and payload lookup also matches the last part of a qualified constructor name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
…ctors Codex review on #9085: a payload type declared next to the extension, as in `type Base.attr += Color(color)` with a local `color`, was resolved in the open type's file, so payload completion lost it; and of two same-named extensions, `B.Value(...)` completed the payload of the first one, `A.Value`, because the payload position kept only `Value`. Each extension constructor now carries the environment of its file, and the payload position keeps the written path: the constructor sharing the longest suffix with it is meant, ties going to the most recent open, then to the last declared one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9085: ranking same-named extension constructors let an open outrank a local extension declared after it, so after `open A` and a later local `Value(...)`, completion used `A.Value`'s payload; and an extension inside a module type, `module type S = { type attr += Phantom }`, was offered as `S.Phantom` and added to exhaustive switches. A qualified constructor is now the one its written path names, and an unqualified one is looked up in the completion scope as the compiler does, which now records extension constructors, the most recent declaration or open winning. Extensions in module types are skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
Why:
Codex review on #9085: now that an open type extracts as a variant,
the Expand catch-all and Exhaustive switch code actions replaced or
built the switch from the constructors known here, dropping the `_`
that matches constructors added in other files.
Codeaction modified (Expand catch-all):
Example:
type attr = ..
type attr += Str(string) | Num(int)
let f = (value: attr) =>
switch value {
| Str(_) => "str"
| _ => "other"
}
Before (text): the `_` is replaced by
Num(_)
After (text): the `_` is replaced by
Num(_) | _
Codeaction modified (Exhaustive switch):
Example:
type attr = ..
type attr += Str(string) | Num(int)
let value: attr = Str("s")
value
Before (rescript):
switch value {
| Str(_) => %todo
| Num(_) => %todo
}
After (rescript):
switch value {
| Str(_) => %todo
| Num(_) => %todo
| _ => %todo
}
Review: Why keep the `_` as an alternative instead of a separate case?
Replacing only the pattern needs no copy of the case body, and the
expanded or-pattern lists the known constructors for splitting by hand.
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:
Codex review on #9085: an `open A` after a local extension `Value`
resolved the payload of an unqualified `Value(...)` to the local
constructor. Matching `A.Value` against the constructors also accepted
the bare local `Value` as a suffix, and the later declaration won. A
constructor named without a module is declared around the cursor, so
an open no longer brings it into scope.
Completion modified (extension payload after an open):
Example:
type attr = ..
module A = {
type attr += Value(bool)
}
type attr += Value(option<int>)
open A
let x: attr = Value()
Before (text): payload completion inside `Value()` completes
option<int>, the payload of the local Value
After (text): payload completion inside `Value()` completes
bool, the payload of A.Value: true, false
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>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 417ad95d74
ℹ️ 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 didn't record the constructors added by `type t += ...`,
and an open type has none in its declaration, so completion offered no
constructor where a value of the type was expected. They are now
registered as constructors of an open type, and typed completion of an
open type offers those declared in the type's file and in the current
file. Since other files can extend the type, the exhaustive switch
snippet for one adds a `_` case.
Diagnostic modified (completion of an extensible variant):
Example:
type attr = ..
type attr += Str(string) | Num(int)
// let a: attr = Nu
Before (text): tests/analysis_tests/tests/src/CompletionExtensibleVariant.res
Nullable, Null
After (text): tests/analysis_tests/tests/src/CompletionExtensibleVariant.res
Num(_), Nullable, Null
Diagnostic modified (exhaustive switch snippet):
Example:
let value: attr = Str("s")
// switch valu
After (rescript):
switch value {
| Str(_) => %todo
| Num(_) => %todo
| Flag => %todo
| _ => %todo
}
Review: Which extensions are offered?
Those in the file that declares the type and in the current file;
an extension in a third file is not known to the current file's
analysis.
Fixes: #9073
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9085: extensions were matched to an open type by its last name only, so `A.t` got the constructors of `B.t`, and a constructor declared in a nested module was inserted without its module, which doesn't compile because an extension constructor can't be found from the expected type. An extension now records the type it extends and is matched by that type's file and stamp, and it is named by its module path, for example `Sub.Flag`; a prefix may match the last part. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9085: extension constructors were named relative to the root of their file and to the file of the completed value. Inside the declaring module that inserted `Sub.Flag`, which is not in scope there, and for a value imported from another file the constructors were not qualified with its module, while extensions in the edited file were missed. A completion request now records the edited file and the module path at the cursor; constructors of that file drop the module path shared with the cursor, constructors of other files get their module's name, and extensions are collected from the edited file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9085: the module name of another file comes from its compiled name, which in a namespaced project is `Base-MyNamespace`, so completion inserted `Base-MyNamespace.Flag`. The name is turned into its source path `MyNamespace.Base` first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9085: an extension of the edited file declared after the cursor was offered although it isn't in scope there, and an extension inside a module of an interface (`module Sub: { type attr += Flag }`) was never registered, because only typed-tree extensions were. The completion scope now records the cursor position and skips later extensions of the edited file, and extensions in signatures are registered too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
…loads Codex review on #9085: an extension two modules deep in an interface got only the outer module in its path, because the signature walk didn't extend the module path when entering a module, and completion inside the payload of a qualified constructor such as `Sub.Toggle(...)` found no constructor, because the payload position keeps only `Toggle`. Signature modules now extend the path, and payload lookup also matches the last part of a qualified constructor name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
…ctors Codex review on #9085: a payload type declared next to the extension, as in `type Base.attr += Color(color)` with a local `color`, was resolved in the open type's file, so payload completion lost it; and of two same-named extensions, `B.Value(...)` completed the payload of the first one, `A.Value`, because the payload position kept only `Value`. Each extension constructor now carries the environment of its file, and the payload position keeps the written path: the constructor sharing the longest suffix with it is meant, ties going to the most recent open, then to the last declared one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
Codex review on #9085: ranking same-named extension constructors let an open outrank a local extension declared after it, so after `open A` and a later local `Value(...)`, completion used `A.Value`'s payload; and an extension inside a module type, `module type S = { type attr += Phantom }`, was offered as `S.Phantom` and added to exhaustive switches. A qualified constructor is now the one its written path names, and an unqualified one is looked up in the completion scope as the compiler does, which now records extension constructors, the most recent declaration or open winning. Extensions in module types are skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gGkPma9LAZimuQjnrP Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
Why:
Codex review on #9085: now that an open type extracts as a variant,
the Expand catch-all and Exhaustive switch code actions replaced or
built the switch from the constructors known here, dropping the `_`
that matches constructors added in other files.
Codeaction modified (Expand catch-all):
Example:
type attr = ..
type attr += Str(string) | Num(int)
let f = (value: attr) =>
switch value {
| Str(_) => "str"
| _ => "other"
}
Before (text): the `_` is replaced by
Num(_)
After (text): the `_` is replaced by
Num(_) | _
Codeaction modified (Exhaustive switch):
Example:
type attr = ..
type attr += Str(string) | Num(int)
let value: attr = Str("s")
value
Before (rescript):
switch value {
| Str(_) => %todo
| Num(_) => %todo
}
After (rescript):
switch value {
| Str(_) => %todo
| Num(_) => %todo
| _ => %todo
}
Review: Why keep the `_` as an alternative instead of a separate case?
Replacing only the pattern needs no copy of the case body, and the
expanded or-pattern lists the known constructors for splitting by hand.
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:
Codex review on #9085: an `open A` after a local extension `Value`
resolved the payload of an unqualified `Value(...)` to the local
constructor. Matching `A.Value` against the constructors also accepted
the bare local `Value` as a suffix, and the later declaration won. A
constructor named without a module is declared around the cursor, so
an open no longer brings it into scope.
Completion modified (extension payload after an open):
Example:
type attr = ..
module A = {
type attr += Value(bool)
}
type attr += Value(option<int>)
open A
let x: attr = Value()
Before (text): payload completion inside `Value()` completes
option<int>, the payload of the local Value
After (text): payload completion inside `Value()` completes
bool, the payload of A.Value: true, false
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>
417ad95 to
347aa66
Compare
Why:
Codex review on #9085: an extension constructor written through a
module alias, `Alias.Value(...)`, was matched by its text against the
recorded `B.Value`, so the payload of another constructor, or none,
was completed. After `open Alias` the lookup found no constructor of
the alias and, with nothing left to pick from, crashed on an empty
list. The written path, qualified or from an open, now has its leading
module alias declared in the scope at the cursor replaced by the path
it aliases before the constructor is selected.
Completion modified (extension payload through a module alias):
Example:
type attr = ..
module B = {
type attr += Value(option<int>)
}
type attr += Value(bool)
module Alias = B
let x: attr = Alias.Value()
Before (text): payload completion inside `Alias.Value()` completes
bool, the payload of the local Value
After (text): payload completion inside `Alias.Value()` completes
option<int>, the payload of B.Value: None, Some(_)
Completion modified (extension payload after an open of an alias):
Example:
type attr = ..
module B = {
type attr += Value(option<int>)
}
module Alias = B
let v: attr = B.Value(None)
open Alias
switch v { | Value() }
Before (text): payload completion inside `Value()`
Fatal error: exception Invalid_argument("List.nth")
After (text): payload completion inside `Value()` completes
option<int>, the payload of B.Value: None, Some(_)
Review: Why only a leading alias? The path is named from the cursor, so
its first module is the one the scope there declares; an alias deeper in
the path would need the enclosing module's structure, which the naming of
extension constructors doesn't walk.
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:
Codex review on #9085: the Expand catch-all and Exhaustive switch code
actions found the type of the switched expression through completion,
whose scope ended before they extracted the constructors of an open
type. Extension constructors were then named from the top of the file
and not filtered by position, so inside `module Nested` the action
wrote `Nested.Inner`, which isn't in scope there, and offered
constructors declared after the action. The scope now lasts through
the whole action: `Xform.with_type_of_expr` runs the action in it.
Codeaction modified (Exhaustive switch):
Example:
type attr = ..
type attr += Str(string)
let value: attr = Str("s")
module Nested = {
type attr += Inner
value
type attr += AfterAction
}
Before (rescript): on `value`
switch value {
| Str(_) => %todo
| Nested.Inner => %todo
| Nested.AfterAction => %todo
| _ => %todo
}
After (rescript): on `value`
switch value {
| Str(_) => %todo
| Inner => %todo
| _ => %todo
}
Codeaction modified (Expand catch-all):
Example:
type attr = ..
type attr += Str(string) | Num(int)
let value: attr = Str("s")
let f = () =>
switch value {
| Str(_) => "str"
| _ => "other"
}
module Nested = {
type attr += Inner
}
Before (text): the `_` is replaced by
Num(_) | Nested.Inner | _
After (text): the `_` is replaced by
Num(_) | _
Review: Why keep the scope in the global instead of passing it to
`Type_utils.extract_type`? Extension constructors are named deep inside
type extraction, which completion, hover and code generation reach
through many callers; a continuation around the code action keeps the
existing mechanism and changes only its three call sites.
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:
Review of #9085: after several review rounds, payload constructors
were found in three ways, a scope walk for unqualified names, a
text match with suffixes in both directions for qualified paths, and
module alias resolution on top. A qualified path ignored the opens in
scope, so after `open Outer` the path `Inner.Deep` still named a local
`Inner` declared before the open. One walk of the scope now resolves
every path as the compiler does: the most recent declaration of the
constructor, of the path's first module (unaliased), or open naming
it decides; a path that names its modules from further out than the
cursor stays the fallback.
Completion modified (qualified extension payload after an open):
Example:
type attr = ..
type color = Red | Blue
module Outer = {
module Inner = {
type attr += Deep(bool)
}
}
module Inner = {
type attr += Deep(option<color>)
}
open Outer
let x: attr = Inner.Deep()
Before (text): payload completion inside `Inner.Deep()` completes
option<color>, the payload of the local Inner.Deep
After (text): payload completion inside `Inner.Deep()` completes
bool, the payload of Outer.Inner.Deep: true, false
Review: Why does an open try only the exact path? Constructors are
named from the cursor, so the opened module's path followed by the
written one is the name of the constructor it brings into scope; a
suffix match accepted unrelated constructors, and recursing into the
outer opens would be exponential in their number.
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:
Review of #9085: signature help looked up the constructor of an open
type by its last name, `Toggle`, among constructors named by their
module path, `Sub.Toggle`, and found none. It now compares the last
part of the path.
Signaturehelp modified (qualified extension constructor):
Example:
type attr = ..
module Sub = {
type attr += Toggle(bool)
}
let toggled: attr = Sub.Toggle(true)
Before (text): signature help inside `Sub.Toggle(true)`
null
After (text): signature help inside `Sub.Toggle(true)`
Sub.Toggle(bool), parameter 0 active
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:
Review of #9085: the scope that names extension constructors was set
through two wrappers, Type_utils' and one in Completion_back_end that
only added the module path at the cursor, and the catch-all an open
type adds to a switch was decided in Xform and again in
Completion_back_end. `Type_utils.with_completion_scope` now computes
the module path itself and is the one wrapper, used by completion and
by code actions, and `Type_utils.open_type_catch_all` serves the
exhaustive switch completion and both catch-all expansions. Analysis
output 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>
Why:
Review of #9085: CompletionExtensibleIface and
CompletionExtensiblePayloadIface both declared an open type with
extensions in modules of an interface, one to test their names, the
other the payload of one of them. The payload case moves into
CompletionExtensibleIface, whose `Sub` now also declares
`Toned(tone)`, and the second fixture is removed. The exhaustive
switch of the remaining fixture gains a `Toned(_)` case; the payload
completion is the one the removed fixture had.
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>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 284953f3d9
ℹ️ 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:
Codex review on #9085: signature help kept only the last name of the
constructor and took the first extension constructor of that name, so
inside `B.Value(...)` it showed `A.Value(bool)`. It now resolves the
written path with the lookup payload completion uses, in the scope at
the cursor. That scope comes from `Completion_front_end.scope_at`, the
scope the completion parser has reached at the cursor, since a
payload being typed usually offers nothing to complete.
Signaturehelp modified (same-named extension constructors):
Example:
type attr = ..
module A = {
type attr += Value(bool)
}
module B = {
type attr += Value(option<int>)
}
open B
let b: attr = B.Value(None)
let unqualified: attr = Value(None)
Before (text): signature help inside `B.Value(None)` and `Value(None)`
A.Value(bool)
After (text): signature help inside `B.Value(None)` and `Value(None)`
B.Value(option<int>)
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:
Codex review on #9085: an extension declared in a module inside an
expression, such as `module Local = {...}` in a function body, was
offered after the function, and from other files, where `Local` is
not in scope. The contents of such a module now get a module path
that starts out of sight (`NotVisible`, as the module itself already
had), and its extensions are offered only in the current file where
the scope's most recent module of that name is that local module,
declared before the extension.
Completion modified (extension of an expression-local module):
Example:
type attr = ..
type attr += Str(string)
let make = () => {
module Local = {
type attr += InLocal
}
let inBlock: attr = InL
Local.InLocal
}
let afterMake: attr = InL
Before (text): completion of `InL` inside the block and after `make`
Local.InLocal, Local.InLocal
After (text): completion of `InL` inside the block and after `make`
Local.InLocal, nothing
Codeaction modified (Expand catch-all after the block):
Example:
let w = switch value {
| Str(_) => 1
| _ => 2
}
Before (text): the `_` is replaced by
Local.InLocal | _
After (text): the `_` is replaced by
_
Review: Why match the local module by name and position instead of its
block's extent? The completion scope already holds the modules in scope
at the cursor with their locations; requiring the most recent one of the
name to be a local module declared before the extension needs no new
data from the typed tree. Inside the local module's own structure the
module isn't in scope yet, so its extensions are not offered there.
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>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e45dd058d
ℹ️ 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".
| let opened = | ||
| List.filteri (fun i _ -> i < List.length opened - 1) opened | ||
| in | ||
| match exact (unalias scope (opened @ path)) with |
There was a problem hiding this comment.
Resolve opened module paths through preceding opens
When a module is opened through an earlier open, payload lookup cannot resolve its constructors. For example, with Outer.Inner.Value(bool) extending the open type, followed by open Outer and open Inner, completing inside unqualified Value(...) finds no constructor: this branch compares Inner.Value against the recorded Outer.Inner.Value, while unalias only follows module aliases and ignores preceding opens. Expression and pattern payload completion consequently lose boolean suggestions, and signature help also fails. Resolve the opened module in its declaration scope before matching its constructor.
Useful? React with 👍 / 👎.
| | Some loc -> | ||
| is_local_module loc && Loc.start loc <= Loc.start declared.extent_loc | ||
| | None -> false) |
There was a problem hiding this comment.
Retain local extensions inside their declaring module body
For an expression-local module containing both an extension and code using it, this check excludes the extension even inside that module. For example, inside module Local = { type attr += Flag; ... } within a function, typed completion of an attr expression after the declaration cannot offer Flag. The Pexp_letmodule traversal in completion_front_end.ml adds Local to the scope only after traversing its module expression, so no matching Module scope item exists while visiting its body and this branch returns false. Recognize positions inside the declaring local module as visible as well as positions in the subsequent binding body.
Useful? React with 👍 / 👎.
Fixes #9073.
Analysis didn't record constructors added by
type t += ..., so completion offered none for an extensible variant. They are now registered as constructors of an open type. Typed completion of an open type, in both expressions and patterns, offers the ones declared in the type's file and in the current file. The exhaustive-switch snippet for an open type ends with a_case, since other files can extend it.New analysis test:
CompletionExtensibleVariant.Constructors are named so that they are in scope where completion runs: unqualified inside their declaring module,
Sub.Flagelsewhere in the file, andModule.Flagfrom another file, using source paths in namespaced projects.Known limitation: an extension declared inside a module whose signature hides it (
module Hidden: {} = { type attr += Flag }) is still offered. Filtering through the visible signature is left for a follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3