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

Changed extern crate foo as bar; error message #17757

Closed
wants to merge 2 commits into from

Conversation

gamazeps
Copy link
Contributor

@gamazeps gamazeps commented Oct 4, 2014

I did not put the crate name in the error note, if that's necessary I'll look into it.

Closes #17709

let path = if self.token == token::EQ {
self.bump();
let path = self.parse_str();
let span = self.span;
self.obsolete(span, ObsoleteExternCrateRenaming);
Some(path)
} else if self.eat_keyword(keywords::As){
let last_span = self.last_span;
self.span_note(last_span, "expected `;`, found `as`; perhaps you \
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be an error (span_err).

Copy link
Member

Choose a reason for hiding this comment

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

Or, maybe it hits the expect below? In which case, doesn't this print something weird?

E.g.

extern crate foo as bar;

possibly prints something like:

note: expected `;`, found `as`; perhaps you meant to enclose the crate name in a string
...
error: expected `;`, found `bar`

(i.e. two messages)

Maybe you could adjust the branches a little more, which also allows for a nice error message:

let path = if self.eat(&token::EQ) {
    let path = self.parse_str();
    let span = self.span;
    self.obsolete(span, ObsoleteExternCrateRenaming);
    Some(path)
} else if self.eat_keyword(keywords::As) {
    // skip the ident if there is one
    if is_ident(&self.token) { self.bump(); }

    let last_span = self.last_span;
    self.span_err(last_span,
                  format!("expected `;`, found `as`; perhaps you meant \
                           to enclose the crate name (`{}`) in a string?",
                          the_ident.as_str())));
    None
} else {
    None
};
self.expect(&token::SEMI);
(path, the_ident)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it did hit two messages, but it didn't seem horrible to have both a note and an error (but it's my first diagnostic issue, so it may be why ^^).
On it, I'll commit you a new one in about an hour :)

@huonw
Copy link
Member

huonw commented Oct 4, 2014

Could you add a test for this? E.g. create src/test/compile-fail/extern-crate-as-no-string-help.rs

LICENSE

extern crate foo as bar;
//~^ ERROR expected `;`, found `as`; perhaps you meant to enclose the crate name (`foo`) in a string?

(where LICENSE can just be copied from one of the other compile-fail tests.)

@gamazeps
Copy link
Contributor Author

gamazeps commented Oct 4, 2014

Done :)


let last_span = self.last_span;
Copy link
Member

Choose a reason for hiding this comment

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

Oh, wait, this should move to be before the if statement above (we want the span to point at the original ident, not at the as keyword).

// Tests that the proper help is displayed in the error message

extern crate foo as bar;
//~^ ERROR expected `;`, found `as`; perhaps you meant to enclose the crate name `foo` in a string ?
Copy link
Member

Choose a reason for hiding this comment

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

The space before ? will need to be removed.

@huonw
Copy link
Member

huonw commented Oct 7, 2014

It looks like the test src/test/compile-fail/extern-foreign-crate.rs needs to have it's ERROR directive updated, since = is no longer legal to follow extern crate foo.

@gamazeps
Copy link
Contributor Author

gamazeps commented Oct 7, 2014

on it :)

@gamazeps
Copy link
Contributor Author

@huonw Done (sorry if it took so long for so few changes)

bors added a commit that referenced this pull request Oct 13, 2014
I did not put the crate name in the error note, if that's necessary I'll look into it.

Closes #17709
@bors bors closed this Oct 13, 2014
@gamazeps gamazeps deleted the issue17709 branch May 7, 2017 14:52
lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 29, 2024
…Veykril

assist: Add new assist toggle_macro_delimiter

Closes rust-lang#17716
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.

extern crate foo as bar; gives outdated & unhelpful error message
5 participants