Skip to content

Show a functor's parameters on hover - #9083

Open
cknitt wants to merge 1 commit into
fix-9072-applied-functor-typesfrom
fix-9071-functor-hover
Open

cknitt wants to merge 1 commit into
fix-9072-applied-functor-typesfrom
fix-9071-functor-hover

Conversation

@cknitt

@cknitt cknitt commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Fixes #9071. Stacked on #9084, which models an applied functor by its result signature.

Hover on a functor now shows its parameters, for example module MakeSet: (O: Ord) => {...}. Analysis models a functor by its result structure; the parameters are recorded on that structure, and only hover displays them.

An applied functor such as module IntSet = MakeSet(IntOrd) is a plain module and shows no parameters; a partial application shows only the remaining ones, and a generative functor shows () =>.

New analysis test: HoverFunctor (named, multi-parameter, inline-signature, applied, partially applied and generative functors).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Changelog

🐛 Bug fix

  • Hover on a functor shows its parameters, such as module MakeSet: (O: Ord) => {...}, instead of only its body.

Semantic Diff

1 commit · Full page

Show a functor's parameters on hover

aac2e68 · cknitt + Claude · Oct 27, 2026, 12:54 UTC

Why

Editor analysis models a functor by the structure of its result, so hover on a functor showed its body as if it were a plain module, and nothing said it takes arguments. The result structure now records the parameters, and hover prints them in front of the body.

Diagnostic modified (hover on a 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])
}

rescript · tests/analysis_tests/tests/src/HoverFunctor.res

-module MakeSet: {
+module MakeSet: (O: Ord) => {
   type t = array<O.t>
   let add: (t, O.t) => array<O.t>
 }

Review

  • Why not add a functor case to the analysis module type? Completion, references and path resolution all treat a functor as its result and would each need the new case; only hover displays the parameters, so they are recorded on the result structure.
  • What does hover show for an applied functor? A plain module: an application is modelled by its signature, so module IntSet = MakeSet(IntOrd) shows no parameters, and a partial application such as Pair(IntOrd) shows only the remaining one.

@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.47%. Comparing base (c989f87) to head (aac2e68).

Files with missing lines Patch % Lines
analysis/src/process_cmt.ml 83.33% 4 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                       @@
##           fix-9072-applied-functor-types    #9083   +/-   ##
===============================================================
  Coverage                           81.46%   81.47%           
===============================================================
  Files                                 460      460           
  Lines                               64688    64711   +23     
===============================================================
+ Hits                                52700    52721   +21     
- Misses                              11988    11990    +2     
Files with missing lines Coverage Δ
analysis/src/hover.ml 77.70% <100.00%> (+0.43%) ⬆️
analysis/src/shared_types.ml 80.41% <ø> (ø)
analysis/src/process_cmt.ml 83.42% <83.33%> (+0.39%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@9083

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@9083

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@9083

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@9083

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@9083

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@9083

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@9083

commit: aac2e68

@github-actions

Copy link
Copy Markdown

@cknitt
cknitt marked this pull request as ready for review October 27, 2026 14:20
@cknitt

cknitt commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T08:01:22.502913Z aac2e68 Manual request
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 71fcc36a3f

ℹ️ 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".

Comment thread analysis/src/hover.ml
@cknitt
cknitt force-pushed the fix-9071-functor-hover branch from 71fcc36 to 518b1df Compare October 27, 2026 19:58
@cknitt
cknitt changed the base branch from master to fix-9072-applied-functor-types October 27, 2026 19:58
@cknitt
cknitt added this pull request to stack #9093 October 27, 2026 19:58
@cknitt

cknitt commented Oct 27, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 518b1df249

ℹ️ 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".

@cknitt
cknitt requested a review from cristianoc October 27, 2026 20:18

@cristianoc cristianoc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the hover parameter metadata, applied-signature dependency, and named/multiple/inline/applied/partial/generative fixtures. The reported structural-result case is fixed, and no blocking regression was found. Named-result Ident functors remain a pre-existing limitation. CI analysis tests inspected; tests were not rerun locally.

Why:
    Editor analysis models a functor by the structure of its result, so
    hover on a functor showed its body as if it were a plain module, and
    nothing said it takes arguments. The result structure now records the
    parameters, and hover prints them in front of the body.

Diagnostic modified (hover on a 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])
    }
Before (rescript): tests/analysis_tests/tests/src/HoverFunctor.res
    module MakeSet: {
      type t = array<O.t>
      let add: (t, O.t) => array<O.t>
    }
After (rescript): tests/analysis_tests/tests/src/HoverFunctor.res
    module MakeSet: (O: Ord) => {
      type t = array<O.t>
      let add: (t, O.t) => array<O.t>
    }

Review: Why not add a functor case to the analysis module type?
    Completion, references and path resolution all treat a functor as its
    result and would each need the new case; only hover displays the
    parameters, so they are recorded on the result structure.

Review: What does hover show for an applied functor?
    A plain module: an application is modelled by its signature, so
    `module IntSet = MakeSet(IntOrd)` shows no parameters, and a partial
    application such as `Pair(IntOrd)` shows only the remaining one.

Fixes: #9071
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>
@cknitt
cknitt force-pushed the fix-9071-functor-hover branch from 518b1df to aac2e68 Compare October 28, 2026 07:58
@cknitt

cknitt commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aac2e68c57

ℹ️ 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".

| Structure s -> Structure {s with functor_params = param :: s.functor_params}
| Constraint (m1, m2) ->
Constraint (add_functor_param param m1, add_functor_param param m2)
| Ident _ -> m

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve functor parameters on identifier results

For a functor with a named result signature, such as module Make = (O: Ord): Set => {...} or an interface declaration module Make: (O: Ord) => Set, the result becomes Ident Set, and this branch discards the parameter. Hover.show_module subsequently resolves that identifier to Set, so hover still describes Make as a plain module without (O: Ord) =>. The same loss occurs for functors whose body returns a module identifier. Preserve the parameters independently of the result's representation and carry them through identifier resolution.

Useful? React with 👍 / 👎.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hover on a functor drops its parameter

2 participants