Skip to content

fix show variance when hovering over generics #3540#3545

Open
asukaminato0721 wants to merge 1 commit into
facebook:mainfrom
asukaminato0721:3540
Open

fix show variance when hovering over generics #3540#3545
asukaminato0721 wants to merge 1 commit into
facebook:mainfrom
asukaminato0721:3540

Conversation

@asukaminato0721
Copy link
Copy Markdown
Contributor

Summary

Fixes #3540

now class type-parameter hovers find the owning class, query the existing variance solver, and render T@Foo (covariant) without the redundant (type parameter) T: ... prefix.

Test Plan

add test

@meta-cla meta-cla Bot added the cla signed label May 22, 2026
@asukaminato0721 asukaminato0721 marked this pull request as ready for review May 22, 2026 07:32
Copilot AI review requested due to automatic review settings May 22, 2026 07:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions
Copy link
Copy Markdown

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

fn type_parameter_hover_display(
solver: &AnswersSolver<TransactionHandle<'_>>,
type_: &Type,
owner: &Class,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can't the type param be owned by a function definition too?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and perhaps another question is, is it important/useful to display the owner at all? we didn't do it previously, and IDK how much people understand the @ notation, even if Pyright does use it

"Expected class type parameter hover to show inferred variance, got: {report}"
);
assert!(
!report.contains("(type parameter) T: T"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

our format seems somewhat standardized with the kind of symbol in parentheses in front of the type, i agree that maybe it's redundant, but I wonder if this is introducing inconsistencies

@meta-codesync
Copy link
Copy Markdown
Contributor

meta-codesync Bot commented May 22, 2026

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D106091156.

@yangdanny97 yangdanny97 self-assigned this May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

show variance when hovering over generics

3 participants