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

[resolve] use rename suggestion is not well-formatted #45799

Closed
behnam opened this issue Nov 6, 2017 · 4 comments
Closed

[resolve] use rename suggestion is not well-formatted #45799

behnam opened this issue Nov 6, 2017 · 4 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Path resolution C-bug Category: This is a bug.

Comments

@behnam
Copy link
Contributor

behnam commented Nov 6, 2017

if let Ok(snippet) = cm.span_to_snippet(binding.span) {
err.span_suggestion(binding.span,
rename_msg,
format!("{} as Other{}", snippet, name));

Trying to re-import std or any other crate, I get a suggestion like this:

14 | extern crate std;
   | ^^^^^^^^^^^^^^^^^ `std` reimported here
   |
   = note: `std` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
   |
14 | extern crate std; as Otherstd
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The problem is in the formatting of the suggestion, where SEMICOLON (;) is placed before the as keyword, like this extern crate std; as Otherstd instead of extern crate std as Otherstd;.

The suggestion doesn't exist in stable or beta, so this affects only nightly at the moment.

Original commit: ff83240
Original PR: #45660

/cc @Cldfire

@behnam
Copy link
Contributor Author

behnam commented Nov 6, 2017

Looks like the feature works fine for use statements, but has this bug for extern crate ones. Maybe worth adding a test case for extern crate, too.

@petrochenkov petrochenkov added A-diagnostics Area: Messages for errors, warnings, and lints A-resolve Area: Path resolution labels Nov 6, 2017
@Cldfire
Copy link
Contributor

Cldfire commented Nov 6, 2017

Whoops! Thanks for noticing that.

I should be able to look into fixing this tonight (~12 hours from now) or tomorrow. @estebank, may I ask if you have any comments or input as to how this should be fixed before I do so?

@estebank
Copy link
Contributor

estebank commented Nov 6, 2017

@Cldfire what I would do is change the binding.span for extern crate statements to not include the semicolon. To do so, it is probably enough to modify parse_item_extern_crate in libsyntax/parse/parser.rs to take the self.prev_span before parsing the semicolon. That change would probably affect other ui tests as it would change underlines involving extern crate statements, and you should check if there're any regressions doing that change.

@Cldfire
Copy link
Contributor

Cldfire commented Nov 6, 2017

Gotcha, thanks for the advice.

@TimNN TimNN added the C-bug Category: This is a bug. label Nov 7, 2017
bors added a commit that referenced this issue Jan 28, 2018
Correctly format `extern crate` conflict resolution help

Closes #45799. Follow up to @Cldfire's #45820.

If the `extern` statement that will have a suggestion ends on a `;`, synthesize a new span that doesn't include it.
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 A-resolve Area: Path resolution C-bug Category: This is a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants