Skip to content

Worked issue #10446 hide "obvious" inlays #10864

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

emeryc
Copy link

@emeryc emeryc commented Nov 25, 2021

The suggested method in the issue didn't work because that code path appears not to be hit during the generation of these types. Instead I introduced a new concept of unambigious types as part of let statements and filtered out cases that the issue seemed to be pointing at.

There were two tests that seemed to depend on the old behavior that I cleaned up in order to bring into alignment with the new behavior, this

This should solve #10446

The suggested method in the issue didn't work because that code path
appears not to be hit during the generation of these types. Instead I
introduced a new concept of unambigious types as part of let statements
and filtered out cases that the issue seemed to be pointing at.

There were two tests that seemed to depend on the old behavior that I
cleaned up in order to bring into alignment with the new behavior, this
This cleans up several if liet statements but...
Comment on lines 1208 to -1157
let test = Some(Test { a: Some(3), b: 1 });
//^^^^ Option<Test>
Copy link
Member

Choose a reason for hiding this comment

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

These changes are undesirable, the idea behind this "hiding" is to only hide info if it is redundant. In this case here specifically, only Some is mentioned, so the fact that this local is of the type Option<Test> is not redundant and as such should always be shown.

Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

The suggested method in the issue didn't work because that code path appears not to be hit during the generation of these types.

This sounds surprising to me, is_named_constructor should definitely be hit as well as the branch shown in the issue.

@emeryc
Copy link
Author

emeryc commented Nov 26, 2021

call point is:

            if config.hide_named_constructor_hints
                && is_named_constructor(sema, pat, &ty_name).is_some()

hide_named_constructor_hints is false, and so we never evaluate the is_named_constructor due to short circuiting.

@Veykril
Copy link
Member

Veykril commented Nov 26, 2021

Ah you need to enable the config for your test cases specifically as done here https://github.com/rust-analyzer/rust-analyzer/blob/9f447ad522a2e7464d827d5b51cdeb53a90c91f5/crates/ide/src/inlay_hints.rs#L1340-L1386

@Veykril
Copy link
Member

Veykril commented Dec 17, 2021

@emeryc any updates here?

@emeryc
Copy link
Author

emeryc commented Dec 18, 2021

Actual solution is going to be different enough for this PR to be not worth it. Sorry I should have just closed this sooner.

@emeryc emeryc closed this Dec 18, 2021
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.

2 participants