Skip to content

"remove this" suggestions should take spacing into account #46614

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
zackmdavis opened this issue Dec 10, 2017 · 6 comments
Closed

"remove this" suggestions should take spacing into account #46614

zackmdavis opened this issue Dec 10, 2017 · 6 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-diagnostics Working group: Diagnostics

Comments

@zackmdavis
Copy link
Member

rls_remove_this

@sfackler
Copy link
Member

@zackmdavis
Copy link
Member Author

RLS is just plumbing through the suggestions we (the compiler) give it; any improvements would need to happen here.

@sfackler
Copy link
Member

But it seems right at the diagnostic level that the mut span only covers that token, and not whitespace - it's the job of the library performing automated source transforms to do any stylistic cleanup.

@zackmdavis
Copy link
Member Author

That's a good point; I guess the deeper issue is that the compiler's "replace this span with this text" concept of a "suggestion" is limited? (On the other hand, I'm not eager to try to teach external tools about the AST ...)

@oli-obk
Copy link
Contributor

oli-obk commented Dec 10, 2017

We do suggest inserting spaces, I don't see why we can't suggest removing them either.

Especially in the mut case, we always know that it's preceded and followed by a space

@estebank estebank added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. WG-diagnostics Working group: Diagnostics labels Dec 11, 2017
@estebank
Copy link
Contributor

As a stop-gap measure we can use span_through_char instead of span_until_char in any suggestion that we know ends with a space separator to do the right thing on correctly formatted code, but the suggestion engine will need a revamp to be able to account for this kind of things on a more consistent and less ad-hoc manner so that it can handle, for example, multiple spaces between mut and the identifier.

bors added a commit that referenced this issue Feb 2, 2018
Include space in suggestion `mut` in bindings

Fix #46614.
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. WG-diagnostics Working group: Diagnostics
Projects
None yet
Development

No branches or pull requests

4 participants