lint “non_upper_case_globals” wrongly warns the consumer

[nwin]

The lint “non_upper_case_globals” wrongly warns the consumer about a ill-named constant (playpen). It only seems to happen if the constant is imported into the current namespace and if you’re using it in a match statement:

mod consts {
    #![allow(non_upper_case_globals)]
    pub const fooBAR: usize = 23;
}

fn main() {
    use consts::*;
    println!("{}", match 23 {
        fooBAR => "yes",
        _ => "no"
    });
}

[pnkfelix]

I think this is by design; someone reading the match arm fooBAR => ... is likely to interpret fooBAR as a binding, not a constant that is being matched against...

[nwin]

I don’t think so. You have the same ambiguity problem when matching against lower-case enum variants and there is no warning emitted for that.

[pnkfelix]

I think I can agree that there is an "if-and-only-if" relationship here: if (and only if) we warn about fooBAR => ... when fooBAR is a const-item, then we should warn about fooBAR => ... when fooBAR is an enum-variant.

[nwin]

I think there are two things to agree on:

1. What is the purpose of non_upper_case_globals? Warning about naming conventions or ambiguity?
2. Do we want to warn about ambiguity (because of expectations) in this case?

If it is the purpose of non_upper_case_globals to warning about naming conventions this lint should not warn about this case because you cannot adhere to naming conventions on the consumer side.

On the other hand, if it’s purpose is to warn about ambiguity, and we want to warn about ambiguity in this case it should give a better hint (e.g. "use full qualifier").

Furthermore we would also have to warn about this case (because it is not a constant):

fn main() {
    println!("{}", match 23 {
        FOO_BAR => "yes",
        _ => "no"
    });
}

This is why I oppose. The consequence would would be that would also have to warn about every variable not following naming conventions.

[steveklabnik]

Triage: no changes

[steveklabnik]

Triage: no change

[Lokathor]

This seems to have been filed a second time: #39371

I presume one of the two issues can be merged into the other.

[Spoonbender]

Triage: no change

[traviscross]

@rustbot labels +I-lang-nominated

Nominating this on behalf of @Lokathor. The main issue here seems to be that the person whose code is getting the warning is not necessarily well positioned to change the name, and the upstream who is well placed to change the name may have already silenced this warning but we still warn in the downstream code.

[joshtriplett]

We discussed this in today's @rust-lang/lang meeting.

We agreed that the current lint should not fire on uses, only on declarations. However, we also felt cases that are ambiguous between being a constant or a binding (e.g. bare names like xyzABC without a ::), we should have a different lint to flag those and suggest that they be made unambiguously either a binding or a constant.

A pattern could be made unambiguously a binding with an bound_ident @ _, or made unambiguously a constant by giving a path that includes a ::.