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

Suggest invalid expression when use public #100165

Closed
TheAwiteb opened this issue Aug 5, 2022 · 8 comments · Fixed by #100188
Closed

Suggest invalid expression when use public #100165

TheAwiteb opened this issue Aug 5, 2022 · 8 comments · Fixed by #100188
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@TheAwiteb
Copy link

TheAwiteb commented Aug 5, 2022

When writing public let some = 3; it will suggest pub let some = 3; and I think nothing should be suggested before it is validated.

Note:
The statement to be suggested must be checked if it is true or not, if true it is suggested and if not it is not.

The issue from: #99903

Given the following code:

fn main() {
    public let some = 3;
}

The current output is:

   Compiling playground v0.0.1 (/playground)
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found keyword `let`
 --> src/main.rs:2:12
  |
2 |     public let some = 3;
  |            ^^^ expected one of 8 possible tokens
  |
help: write `pub` instead of `public` to make the item public
  |
2 |     pub let some = 3;
  |     ~~~

error: could not compile `playground` due to previous error

Ideally the output should look like:

   Compiling playground v0.0.1 (/playground)
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found keyword `let`
 --> src/main.rs:2:12
  |
2 |     public let some = 3;
  |            ^^^ expected one of 8 possible tokens

error: could not compile `playground` due to previous error
@TheAwiteb TheAwiteb added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 5, 2022
@TheAwiteb
Copy link
Author

@rustbot claim

@gimbling-away
Copy link
Contributor

Oh dear, I didn't think of that while making the PR. 😅

A possible solution could be checking if the current token is for initializing an item? Like, struct/enum/macro, etc.

@TheAwiteb
Copy link
Author

TheAwiteb commented Aug 5, 2022

A possible solution could be checking if the current token is for initializing an item? Like, struct/enum/macro, etc.

I wrote this solution, but I don't know if I can use it in rustc, it's currently building 😁

let is_valid_stmt: bool = syn::parse_str::<syn::Stmt>("pub let some = 3;")
    .is_ok(); // false

@TheAwiteb
Copy link
Author

I can't work on the problem unfortunately, whoever will assign it to him you can solve it this way

can_be_public = ["enum", "struct", "fn", "extern", "trait"];
if can_be_public.contains(&self.token.as_str()) {
    // ...
}

The block

if let TokenKind::Ident(symbol, _) = &self.prev_token.kind {
if symbol.as_str() == "public" {
err.span_suggestion_short(
self.prev_token.span,
"write `pub` instead of `public` to make the item public",
"pub",
appl,
);
}
}

@TheAwiteb
Copy link
Author

@rustbot release-assignment

@chenyukang
Copy link
Member

@rustbot claim

@chenyukang
Copy link
Member

Trying to add a method in token:

    /// Returns `true` if the token the start of a item 
    pub fn can_begin_item(&self) -> bool {
        self.is_keyword(kw::Use)
        || self.is_keyword(kw::Fn)
        || self.is_keyword(kw::Extern)
        || self.is_keyword(kw::Crate)
        || self.is_keyword(kw::Mod)
        || self.is_keyword(kw::Const)
        || self.is_keyword(kw::Static)
        || self.is_keyword(kw::Trait)
        || self.is_keyword(kw::Impl)
        || self.is_keyword(kw::Mod)
        || self.is_keyword(kw::Type)
        || self.is_keyword(kw::Enum)
        || self.is_keyword(kw::Struct)
        || self.is_keyword(kw::Union)
    }

And add a check:

 if self.prev_token.is_ident_named(sym::public) && self.token.can_begin_item() {
            err.span_suggestion_short(
                self.prev_token.span,
                "write `pub` instead of `public` to make the item public",
                "pub",
                appl,
            );
        }

Any other better idea? @gimbles

@gimbling-away
Copy link
Contributor

  1. This could be the macro keyword, as well.
  2. That looks alright, ideally the repeated block could be prevented using a macro? I'm afraid that might slightly overengineer it, though. 😂

ChrisDenton added a commit to ChrisDenton/rust that referenced this issue Aug 20, 2022
…ebank

Parser will not suggest invalid expression when use public

Fixes rust-lang#100165
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Aug 20, 2022
…ebank

Parser will not suggest invalid expression when use public

Fixes rust-lang#100165
JohnTitor added a commit to JohnTitor/rust that referenced this issue Aug 24, 2022
…ebank

Parser will not suggest invalid expression when use public

Fixes rust-lang#100165
@bors bors closed this as completed in ed8cfc8 Aug 25, 2022
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 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.

3 participants