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

unfriendly error diagnostic when using expressions in pattern #112593

Closed
ShE3py opened this issue Jun 13, 2023 · 5 comments · Fixed by #118625
Closed

unfriendly error diagnostic when using expressions in pattern #112593

ShE3py opened this issue Jun 13, 2023 · 5 comments · Fixed by #118625
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-parser Area: The parsing of Rust source code to an AST. A-patterns Relating to patterns and pattern matching C-enhancement Category: An issue proposing an enhancement or a PR with one. D-papercut Diagnostics: An error or lint that needs small tweaks. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ShE3py
Copy link
Contributor

ShE3py commented Jun 13, 2023

Code

struct Foo(String);

fn bar(foo: Foo) -> bool {
    match foo {
        Foo("hi".to_owned()) => true,
        _ => false
    }
}

Current output

Compiling playground v0.0.1 (/playground)
error: expected one of `)`, `,`, `...`, `..=`, `..`, or `|`, found `.`
 --> src/lib.rs:5:17
  |
5 |         Foo("hi".to_owned()) => true,
  |                 ^
  |                 |
  |                 expected one of `)`, `,`, `...`, `..=`, `..`, or `|`
  |                 help: missing `,`

error[E0531]: cannot find tuple struct or tuple variant `to_owned` in this scope
 --> src/lib.rs:5:18
  |
5 |         Foo("hi".to_owned()) => true,
  |                  ^^^^^^^^ not found in this scope

error[E0023]: this pattern has 2 fields, but the corresponding tuple struct has 1 field
 --> src/lib.rs:5:13
  |
1 | struct Foo(String);
  |            ------ tuple struct has 1 field
...
5 |         Foo("hi".to_owned()) => true,
  |             ^^^^ ^^^^^^^^^^ expected 1 field, found 2

Some errors have detailed explanations: E0023, E0531.
For more information about an error, try `rustc --explain E0023`.
error: could not compile `playground` (lib) due to 3 previous errors

Desired output

Compiling playground v0.0.1 (/playground)
error[E0164]: expected tuple struct or tuple variant, found associated function `<&str as ToOwned>::to_owned`
 --> src/lib.rs:5:13
  |
5 |         Foo("hi".to_owned()) => true,
  |             ^^^^^^^^^^^^^^^ `fn` calls are not allowed in patterns
  |
  = help: for more information, visit https://doc.rust-lang.org/reference/patterns.html

For more information about this error, try `rustc --explain E0164`.
error: could not compile `playground` (lib) due to previous error

Rationale and extra context

This error occured while I was doing an assert!(matches!(...)) for a struct that doesn't implement Eq.

Using String::new gives better messages;

error[E0164]: expected tuple struct or tuple variant, found associated function `String::new`
 --> src/lib.rs:5:20
  |
5 |         Foo { bar: String::new("hi") } => true,
  |                    ^^^^^^^^^^^^^^^^^ `fn` calls are not allowed in patterns
  |
  = help: for more information, visit https://doc.rust-lang.org/book/ch18-00-patterns.html

Clippy gives the same error messages.

Other cases

struct Foo { bar: String }

fn bar(foo: Foo) -> bool {
    match foo {
        Foo { bar: "hi".to_owned() } => true,
        _ => false
    }
}

rustc output:

   Compiling playground v0.0.1 (/playground)
error: expected `,`
 --> src/lib.rs:5:24
  |
5 |         Foo { bar: "hi".to_owned() } => true,
  |         ---            ^
  |         |
  |         while parsing the fields for this pattern

error: could not compile `playground` (lib) due to previous error

Similar errors when using expressions such as Foo(1 + 2).

Anything else?

No response

@ShE3py ShE3py 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 Jun 13, 2023
@ShE3py
Copy link
Contributor Author

ShE3py commented Jun 13, 2023

@rustbot label +A-patterns +C-enhancement

@rustbot rustbot added A-patterns Relating to patterns and pattern matching C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Jun 13, 2023
@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. labels Jun 17, 2023
@TaKO8Ki TaKO8Ki self-assigned this Jun 18, 2023
@clubby789
Copy link
Contributor

Looks like the same issue as #112593 (dot being recovered as a comma)

@Nadrieril
Copy link
Member

Nadrieril commented Dec 1, 2023

I have limited experience with parsing but I know where I'd start on this.

This is where the pattern of a match arm is parsed. The idea would be, in the error case, to try to parse an expression and emit a better error message if that succeeds. This code is an example of "try to parse X and emit better error if that works". You want to do something similar, but try to parse an expression instead (I'm guessing with self.parse_expr_res). You'll also have to define a diagnostic struct (like MissingCommaAfterMatchArm in the example) with an appropriate message in messages.ftl. (diagnostic structs are documented in the dev-guide).

@clubby789 which issue did you mean? You accidentally linked to this one. EDIT: I'm guessing #90121

@TaKO8Ki I'm unassigining you because it's been a few months, feel free to reassign yourself

@Nadrieril Nadrieril added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. labels Dec 1, 2023
@Nadrieril Nadrieril added the E-help-wanted Call for participation: Help is requested to fix this issue. label Dec 1, 2023
@ShE3py
Copy link
Contributor Author

ShE3py commented Dec 4, 2023

@rustbot claim

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 28, 2024
Improve handling of expressions in patterns

Closes rust-lang#112593.

Methodcalls' dots in patterns are silently recovered as commas (e.g. `Foo("".len())` -> `Foo("", len())`) so extra diagnostics are emitted:
```rs
struct Foo(u8, String, u8);

fn bar(foo: Foo) -> bool {
    match foo {
        Foo(4, "yippee".yeet(), 7) => true,
        _ => false
    }
}
```
```
error: expected one of `)`, `,`, `...`, `..=`, `..`, or `|`, found `.`
 --> main.rs:5:24
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                        ^
  |                        |
  |                        expected one of `)`, `,`, `...`, `..=`, `..`, or `|`
  |                        help: missing `,`

error[E0531]: cannot find tuple struct or tuple variant `yeet` in this scope
 --> main.rs:5:25
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                         ^^^^ not found in this scope

error[E0023]: this pattern has 4 fields, but the corresponding tuple struct has 3 fields
 --> main.rs:5:13
  |
1 | struct Foo(u8, String, u8);
  |            --  ------  -- tuple struct has 3 fields
...
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |             ^  ^^^^^^^^ ^^^^^^  ^ expected 3 fields, found 4

error: aborting due to 3 previous errors
```

This PR checks for patterns that ends with a dot and a lowercase ident (as structs/variants should be uppercase):
```
error: expected a pattern, found a method call
 --> main.rs:5:16
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                ^^^^^^^^^^^^^^^ method calls are not allowed in patterns

error: aborting due to 1 previous error
```

Also check for expressions:
```rs
fn is_idempotent(x: f32) -> bool {
    match x {
        x * x => true,
        _ => false,
    }
}

fn main() {
    let mut t: [i32; 5];
    let t[0] = 1;
}
```
```
error: expected a pattern, found an expression
 --> main.rs:3:9
  |
3 |         x * x => true,
  |         ^^^^^ arbitrary expressions are not allowed in patterns

error: expected a pattern, found an expression
  --> main.rs:10:9
   |
10 |     let t[0] = 1;
   |         ^^^^ arbitrary expressions are not allowed in patterns
```

Would be cool if the compiler could suggest adding a guard for `match`es, but I've no idea how to do it.

---
`@rustbot` label +A-diagnostics +A-parser +A-patterns +C-enhancement
@bors bors closed this as completed in 0138151 Jan 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 29, 2024
Rollup merge of rust-lang#118625 - ShE3py:expr-in-pats, r=WaffleLapkin

Improve handling of expressions in patterns

Closes rust-lang#112593.

Methodcalls' dots in patterns are silently recovered as commas (e.g. `Foo("".len())` -> `Foo("", len())`) so extra diagnostics are emitted:
```rs
struct Foo(u8, String, u8);

fn bar(foo: Foo) -> bool {
    match foo {
        Foo(4, "yippee".yeet(), 7) => true,
        _ => false
    }
}
```
```
error: expected one of `)`, `,`, `...`, `..=`, `..`, or `|`, found `.`
 --> main.rs:5:24
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                        ^
  |                        |
  |                        expected one of `)`, `,`, `...`, `..=`, `..`, or `|`
  |                        help: missing `,`

error[E0531]: cannot find tuple struct or tuple variant `yeet` in this scope
 --> main.rs:5:25
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                         ^^^^ not found in this scope

error[E0023]: this pattern has 4 fields, but the corresponding tuple struct has 3 fields
 --> main.rs:5:13
  |
1 | struct Foo(u8, String, u8);
  |            --  ------  -- tuple struct has 3 fields
...
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |             ^  ^^^^^^^^ ^^^^^^  ^ expected 3 fields, found 4

error: aborting due to 3 previous errors
```

This PR checks for patterns that ends with a dot and a lowercase ident (as structs/variants should be uppercase):
```
error: expected a pattern, found a method call
 --> main.rs:5:16
  |
5 |         Foo(4, "yippee".yeet(), 7) => true,
  |                ^^^^^^^^^^^^^^^ method calls are not allowed in patterns

error: aborting due to 1 previous error
```

Also check for expressions:
```rs
fn is_idempotent(x: f32) -> bool {
    match x {
        x * x => true,
        _ => false,
    }
}

fn main() {
    let mut t: [i32; 5];
    let t[0] = 1;
}
```
```
error: expected a pattern, found an expression
 --> main.rs:3:9
  |
3 |         x * x => true,
  |         ^^^^^ arbitrary expressions are not allowed in patterns

error: expected a pattern, found an expression
  --> main.rs:10:9
   |
10 |     let t[0] = 1;
   |         ^^^^ arbitrary expressions are not allowed in patterns
```

Would be cool if the compiler could suggest adding a guard for `match`es, but I've no idea how to do it.

---
`@rustbot` label +A-diagnostics +A-parser +A-patterns +C-enhancement
@Nadrieril
Copy link
Member

Great work @ShE3py! Thanks for taking the time to fix this

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. A-patterns Relating to patterns and pattern matching C-enhancement Category: An issue proposing an enhancement or a PR with one. D-papercut Diagnostics: An error or lint that needs small tweaks. E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. 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.

6 participants