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

Warn when match variables start with an uppercase letter #10304

Closed
kmcallister opened this issue Nov 6, 2013 · 8 comments
Closed

Warn when match variables start with an uppercase letter #10304

kmcallister opened this issue Nov 6, 2013 · 8 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-lints Area: Lints (warnings about flaws in source code) such as unused_mut.

Comments

@kmcallister
Copy link
Contributor

I had code like

match e.kind {
    EndOfFile => return Ok(()),
    _         => return Err(()),
}

and got error: unreachable pattern on the second pattern. I hadn't imported std::rt::io::EndOfFile so rustc interpreted EndOfFile as a variable rather than an enum constructor.

Most Rust code follows the stylistic rule of capitalizing enum constructors and not capitalizing variables. So I think rustc should warn when binding a capitalized variable in a match — at least when it makes another pattern unreachable, but probably always.

@nikomatsakis
Copy link
Contributor

This has been proposed numerous times. I think it's a great idea. I couldn't find another issue though I'm sure one or two exists -- or maybe not. Here is a generalization: generate a warning if you create a variable that has the same name as a variant of the enum being matched (note that this variant name may be lower case). I actually think it'd possibly make sense to have both warnings, but checking for variables with the same name as an (unimported) variant seems more targeted.

@klutzy
Copy link
Contributor

klutzy commented Nov 12, 2013

I like this suggestion (I've got similar problem several times), but I feel uppercase check is lint job. We already have unused variable lint so it should emit an warning for EndOfFile variable, but unreachable pattern error came first and stopped.

@huonw
Copy link
Member

huonw commented Nov 12, 2013

Couldn't we just go full-haskell and enforce* locals are snake_case (or mixedCase ... anything not starting with a capital) and statics and enum variants are UPPERCASE and/or CamelCase (well anything with an initial capital)?

This would solve #7526 too.

(* Possibly as lints on deny by default, but lints are only checked after the other errors happen.)

@pnkfelix
Copy link
Member

cc @chris-morgan who was discussing related issues in the chat room yesterday. (I.e. the problem where users get surprised that a catch-all _ pattern is unreachable because it is preceded by a Var pattern that the user thought was already bound as a static.)

@pnkfelix
Copy link
Member

Some of the previously filed issues for this are #4639 and #3070

@steveklabnik
Copy link
Member

Triage: still no lint, still same error message.

@wesleywiser
Copy link
Member

Is this still an issue? I tried the repro from #13995 and I got this

test.rs:9:9: 9:11 warning: pattern binding `V1` is named the same as one of the variants of the type `A::E` [E0170]
test.rs:9         V1 => 1,
                  ^~
test.rs:9:9: 9:11 help: run `rustc --explain E0170` to see a detailed explanation
test.rs:9:9: 9:11 help: if you meant to match on a variant, consider making the path in the pattern qualified: `A::E::V1`
test.rs:10:9: 10:11 warning: pattern binding `V2` is named the same as one of the variants of the type `A::E` [E0170]
test.rs:10         V2 => 2,
                   ^~
test.rs:10:9: 10:11 help: run `rustc --explain E0170` to see a detailed explanation
test.rs:10:9: 10:11 help: if you meant to match on a variant, consider making the path in the pattern qualified: `A::E::V2`
test.rs:10:9: 10:11 error: unreachable pattern [E0001]
test.rs:10         V2 => 2,
                   ^~
test.rs:10:9: 10:11 help: run `rustc --explain E0001` to see a detailed explanation
error: aborting due to previous error

@alexcrichton
Copy link
Member

I also believe this is fixed, thanks @wesleywiser!

Jarcho pushed a commit to Jarcho/rust that referenced this issue Feb 26, 2023
[never_loop] Fix rust-lang#10304

It is not sufficient to ignore break from a block inside the loop. Instructions after the break must be ignored, as they are unreachable. This is also true for all instructions in outer blocks and loops until the right block is reached.

Fixes rust-lang#10304

---

changelog: FP: [`never_loop`]: No longer lints, for statements following break statements for outer blocks.
[rust-lang#10311](rust-lang/rust-clippy#10311)
<!-- changelog_checked-->
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-lints Area: Lints (warnings about flaws in source code) such as unused_mut.
Projects
None yet
Development

No branches or pull requests

8 participants