Skip to content

Comments

[ty] Add ty.inlayHints.variableTypes server option#19780

Merged
dhruvmanila merged 4 commits intomainfrom
dhruv/inlay-hint-variable-types
Aug 7, 2025
Merged

[ty] Add ty.inlayHints.variableTypes server option#19780
dhruvmanila merged 4 commits intomainfrom
dhruv/inlay-hint-variable-types

Conversation

@dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Aug 6, 2025 •
edited
Loading

Summary

This PR adds a new ty.inlayHints.variableTypes server setting to configure ty to include / exclude inlay hints at variable position.

Currently, we only support inlay hints at this position so this option basically translates to enabling / disabling inlay hints for now :)

The VS Code extension PR is astral-sh/ty-vscode#112.

closes: astral-sh/ty#472

Test Plan

Add E2E tests.

@dhruvmanila dhruvmanila added server Related to the LSP server ty Multi-file analysis & type inference labels Aug 6, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025 •
edited
Loading

Diagnostic diff on typing conformance tests

No changes detected when running ty on typing conformance tests ✅

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2025 •
edited
Loading

mypy_primer results

No ecosystem changes detected ✅
No memory usage changes detected ✅

@dhruvmanila dhruvmanila force-pushed the dhruv/warn-unknown-options branch from 125dcaa to 2b934cd Compare August 6, 2025 13:07
Base automatically changed from dhruv/warn-unknown-options to main August 6, 2025 13:11
@dhruvmanila dhruvmanila force-pushed the dhruv/inlay-hint-variable-types branch 2 times, most recently from bf11983 to 20af384 Compare August 7, 2025 11:07
@dhruvmanila
Copy link
Member Author

dhruvmanila commented Aug 7, 2025

One thing that I've noticed with other server is that they start off with ty.inlayHints.variableTypes.enable option so that any other related option under the "variableTypes" namespace can be accommodated in the future. This PR doesn't do that but we can do it if that's more useful.

@dhruvmanila dhruvmanila force-pushed the dhruv/inlay-hint-variable-types branch from 20af384 to 5c8297d Compare August 7, 2025 11:15
@dhruvmanila dhruvmanila force-pushed the dhruv/inlay-hint-variable-types branch from 5c8297d to 12d2847 Compare August 7, 2025 11:15
@dhruvmanila dhruvmanila changed the title [ty] Add ty.inlayHints.variablesTypes server option [ty] Add ty.inlayHints.variableTypes server option Aug 7, 2025
@dhruvmanila dhruvmanila added the configuration Related to settings and configuration label Aug 7, 2025
@dhruvmanila dhruvmanila marked this pull request as ready for review August 7, 2025 11:17
MichaReiser
MichaReiser approved these changes Aug 7, 2025
View reviewed changes
crates/ty_ide/src/inlay_hints.rs
Comment on lines +126 to +128
if !self.settings.variable_types {
return;
}
Copy link
Member

@MichaReiser MichaReiser Aug 7, 2025

Choose a reason for hiding this comment

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

It seems unnecessary to traverse the entire tree only to bail out at the assignment level. Should we add a InlayHintSettings::any_enabled and bail out in the inlay hint request if it returns false to avoid doing all this unnecessary work?

Copy link
Member Author

@dhruvmanila dhruvmanila Aug 7, 2025

Choose a reason for hiding this comment

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

Yeah, that makes sense.

Copy link
Member Author

@dhruvmanila dhruvmanila Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Actually, I don't think we should add that today because there will be (in the future) multiple positions for which we won't provide a way to disable. I don't think the server should provide configuring every position of inlay hints. So, I'm going to avoid adding this today.

Copy link
Member

@MichaReiser MichaReiser Aug 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Can you give an example of inlays that the user can't disable and we'd always provide (I'm not disagreeing, just curious)?

Copy link
Member Author

@dhruvmanila dhruvmanila Aug 7, 2025

Choose a reason for hiding this comment

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

Actually, I don't have an example on top of my mind and it might be that it doesn't exists 😅

@MichaReiser MichaReiser mentioned this pull request Aug 7, 2025
Merged
@dhruvmanila dhruvmanila mentioned this pull request Aug 7, 2025
Merged
@dhruvmanila dhruvmanila merged commit b22586f into main Aug 7, 2025
36 of 37 checks passed
@dhruvmanila dhruvmanila deleted the dhruv/inlay-hint-variable-types branch August 7, 2025 13:46
dhruvmanila added a commit to astral-sh/ty-vscode that referenced this pull request Aug 7, 2025
@dhruvmanila
Add ty.inlayHints.variableTypes setting (#112)
38d25d5 
## Summary

This PR adds the `ty.inlayHints.variableTypes` as added in the server in
astral-sh/ruff#19780.

## Test Plan



https://github.com/user-attachments/assets/2ef2fc4e-cbad-4a83-acab-6950bdb0d806
@dhruvmanila dhruvmanila mentioned this pull request Aug 7, 2025
Merged
dhruvmanila added a commit to astral-sh/ty that referenced this pull request Aug 7, 2025
@dhruvmanila
Add ty.experimental.rename, ty.inlayHints.variableTypes (#951)
daf8e91 
## Summary

This PR adds reference documentation for the two new editor settings
that were added in astral-sh/ruff#19800 and
astral-sh/ruff#19780.

## Test Plan

<img width="2491" height="1374" alt="Screenshot 2025-08-07 at 19 24 44"
src="https://github.com/user-attachments/assets/f0fc32c5-1045-4fd9-900d-c4e7e429d54f"
/>
dcreager added a commit that referenced this pull request Aug 7, 2025
@dcreager
Merge branch 'main' into dcreager/bound-typevar
6650090 
* main:
  Update Rust toolchain to 1.89 (#19807)
  [ty] Add `ty.inlayHints.variableTypes` server option (#19780)
  [ty] Add failing tests for tuple subclasses (#19803)
  [ty] Add `ty.experimental.rename` server setting (#19800)
dcreager added a commit that referenced this pull request Aug 7, 2025
@dcreager
Merge branch 'dcreager/bound-typevar' into dcreager/inferrable
152330e 
* dcreager/bound-typevar: (24 commits)
  more comment fix
  this isn't true anymore
  fix inner function in overload implementation ecosystem error
  Update Rust toolchain to 1.89 (#19807)
  [ty] Add `ty.inlayHints.variableTypes` server option (#19780)
  synthetic typevars for type materializations are bound
  [ty] Add failing tests for tuple subclasses (#19803)
  [ty] Add `ty.experimental.rename` server setting (#19800)
  clippy
  make BoundTypeVarInstance interned
  [ty] Implemented support for "rename" language server feature (#19551)
  [ty] Reduce size of member table (#19572)
  [ty] Move server capabilities creation (#19798)
  separate types!
  tmp
  allow KnownInstance::TypeVar in type expressions
  bind typevar in Generic/Protocol base class reference
  [ty] Repurpose `FunctionType.into_bound_method_type` to return `BoundMethodType` (#19793)
  remove unneeded GenericContext::with_binding_context
  create KnownInstanceType::TypeVar for PEP 695 too
  ...
@dhruvmanila dhruvmanila mentioned this pull request Aug 14, 2025
Merged
epitech314 added a commit to epitech314/ty that referenced this pull request Nov 12, 2025
@epitech314
Add ty.experimental.rename, ty.inlayHints.variableTypes (#951)
5400f92 
## Summary

This PR adds reference documentation for the two new editor settings
that were added in astral-sh/ruff#19800 and
astral-sh/ruff#19780.

## Test Plan

<img width="2491" height="1374" alt="Screenshot 2025-08-07 at 19 24 44"
src="https://github.com/user-attachments/assets/f0fc32c5-1045-4fd9-900d-c4e7e429d54f"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

configuration Related to settings and configuration server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add config option to disable inlay type hints for variable assignment targets

2 participants

@dhruvmanila @MichaReiser