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

Confusing suggestion when if is missing in else if #49361

Closed
stokhos opened this issue Mar 25, 2018 · 4 comments · Fixed by #97298
Closed

Confusing suggestion when if is missing in else if #49361

stokhos opened this issue Mar 25, 2018 · 4 comments · Fixed by #97298
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST C-enhancement Category: An issue proposing an enhancement or a PR with one. 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.

Comments

@stokhos
Copy link

stokhos commented Mar 25, 2018

pub fn nth(n:u32)->Option<u32>{
    let mut x:u32 = 2;
    let mut m:u32 = 1;
    let mut p:Vec<u32> = vec![2];
    'outer: loop {
        x += 1;
        'inner: for i in p.iter() {
            if x % i ==0{
                continue 'outer;
            }
        }
        p.push(x);
        m+=1;
        if n < 1 {
            return None;        
        } else if n == 1 {
            return Some(2);
        } else m == n { // should be else if here
            return Some(x);
        }
    }
}

fn main(){
    println!("{:?}", nth(1));
}

The error here is if is missing in else if

but compiler reports: error:

error: expected identifier, found keyword `return`
  --> src/main.rs:19:13
   |
19 |             return Some(x);
   |             ^^^^^^
@estebank
Copy link
Contributor

The output in the current beta already points out at the place where the else if should be, even if it doesn't suggest it:

error: expected identifier, found keyword `return`
  --> src/main.rs:19:13
   |
18 |         } else m == n { // should be else if here
   |                     - while parsing this struct
19 |             return Some(x);
   |             ^^^^^^ expected identifier, found keyword

error: expected `{`, found `m`
  --> src/main.rs:18:16
   |
18 |           } else m == n { // should be else if here
   |  ________________-
19 | |             return Some(x);
20 | |         }
   | |_________- help: try placing this code inside a block: `{ m == n{}; }`

error: aborting due to 2 previous errors

@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 Mar 26, 2018
@estebank estebank added A-parser Area: The parsing of Rust source code to an AST 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
@stokhos stokhos closed this as completed May 20, 2022
@estebank
Copy link
Contributor

The current output is:

error: expected identifier, found keyword `return`
  --> src/main.rs:19:13
   |
18 |         } else m == n { // should be else if here
   |                     - while parsing this struct
19 |             return Some(x);
   |             ^^^^^^ expected identifier, found keyword

error: expected `{`, found `m`
  --> src/main.rs:18:16
   |
18 |         } else m == n { // should be else if here
   |                ^ expected `{`
   |
help: try placing this code inside a block
   |
18 ~         } else { m == n { // should be else if here
19 |             return Some(x);
20 ~         } }
   |

I still wish the parser to detect that the code immediately following else is a binary operation followed by a block, so that we can instead emit

error: expected `{` or `if`, found `m`
  --> src/main.rs:18:16
   |
18 |         } else m == n { // should be else if here
   |                ^ expected `{`
   |
help: you might be missing an `if` keyword here
   |
18 |         } else if m == n { // should be else if here
   |                ++

@estebank estebank reopened this May 20, 2022
@estebank
Copy link
Contributor

CC @compiler-errors, in case you might be interested in looking at the parser :)

@compiler-errors
Copy link
Member

this seems like a neat fix, i'll take a stab at it

@compiler-errors compiler-errors self-assigned this May 20, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 24, 2022
…, r=davidtwco

Parse expression after `else` as a condition if followed by `{`

Fixes rust-lang#49361.

Two things:
1. This wording needs help. I can never find a natural/intuitive phrasing when I write diagnostics 😅
2. Do we even want to show the "wrap in braces" case? I would assume most of the time the "add an `if`" case is the right one.
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 24, 2022
…, r=davidtwco

Parse expression after `else` as a condition if followed by `{`

Fixes rust-lang#49361.

Two things:
1. This wording needs help. I can never find a natural/intuitive phrasing when I write diagnostics 😅
2. Do we even want to show the "wrap in braces" case? I would assume most of the time the "add an `if`" case is the right one.
@bors bors closed this as completed in 0531521 May 24, 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 A-parser Area: The parsing of Rust source code to an AST C-enhancement Category: An issue proposing an enhancement or a PR with one. 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants