Skip to content
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

Highlight only relevant parts of type path in type errors #57413

Closed
estebank opened this issue Jan 7, 2019 · 5 comments · Fixed by #65779
Closed

Highlight only relevant parts of type path in type errors #57413

estebank opened this issue Jan 7, 2019 · 5 comments · Fixed by #65779
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-papercut Diagnostics: An error or lint that needs small tweaks. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Jan 7, 2019

On E0308 type mismatch errors we already perform a basic check to type arguments in order to only highlight differences between the expected and found type. I recently came across a case where the two types had identifiers of the same length with no type arguments, but that were not local so the path was quite long. It took me some time to realize that the two types were actually different and not caused by different crate versions. It'd be nice if for the following, only the parts of the path that differ were highlighted/bolded in the CLI output:

expected: foo::bar::Baz
   found: foo::bar::Bar
                    ^^^ should highlight only this area

CC #43354

@estebank estebank added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-diagnostics Area: Messages for errors, warnings, and lints labels Jan 7, 2019
@estebank estebank added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority labels May 31, 2019
@kevgrasso
Copy link
Contributor

Hello, I'd like to implement this feature.

@estebank
Copy link
Contributor Author

estebank commented Oct 16, 2019

Hi @kevgrasso! Will you be needing a hand? To begin with, I would recommend searching for DiagnosticStyledString in order to find how it works today. Particularly, this method shows how the highlighting for specific type arguments is currently handled. It is extremely verbose, but the comments should help somewhat. Also, the rustc guide is a good primer on how to develop in the compiler's codebase. If you need a hand, don't hesitate to reach out.

@kevgrasso
Copy link
Contributor

Thanks, I'd found that struct and file, but I'd overlooked that method due to how complicated it seemed at a glance.

@estebank estebank added D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 18, 2019
@kevgrasso
Copy link
Contributor

@estebank Should I add a test for this? If so, how would I implement that? The standard UI testing method doesn't seem to track highlights.

@estebank
Copy link
Contributor Author

@kevgrasso sadly, we do not have any way to test correct application of colors in the test suite. Attach screenshots to the PR so that we can see the changes, but we have to rely on being extra careful when touching this part of the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-papercut Diagnostics: An error or lint that needs small tweaks. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants