Suggest cfg(false) instead of cfg(FALSE)

[clubby789]

Split from #149791

cc @Urgau

[rustbot]

Some changes occurred in check-cfg diagnostics

cc @Urgau

[rustbot]

r? @madsmtm

rustbot has assigned @madsmtm.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[joshtriplett]

LGTM. r=me once CI passes.

@bors delegate+

[bors]

✌️ @clubby789, you can now approve this pull request!

If @joshtriplett told you to "r=me" after making some further change, please make that change, then do @bors r=@joshtriplett

[jdonszelmann]

hm, have you seen @scrabsha's work on uppercase/lowercase suggestions in the parser? I think it'd be cool if we could suggest against all mis-capitalizations here and maybe do that in a general enough way that all attribute parsers can benefit from it instead of a cfg-specific lint. At least, could you take a quick look if we can re-use the parsing infra for it? I like it when the error messages are consistent :)

[jdonszelmann]

if that doesn't help let me know and I'll approve this version. The change is a good one.

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[jdonszelmann]

@bors delegate-

[clubby789]

I've spent a while looking into this and I'm not sure it makes sense to factor out the exact machinery from parsing, especially as the attr parsing machinery is some distance from the check-cfg lint.
This check-cfg special case is to address the specific pattern of people using an obviously non-existent FALSE cfg, and to inform them that the new boolean literal syntax exists. Similarly, fake false and true expected cfg names to use the existing 'similar name' machinery in check-cfg would give us a more consistent but less specific error:

warning: unexpected `cfg` condition name: `FALSE`
--> $DIR/false.rs:8:7
|
LL | #[cfg(FALSE)]
| ^^^^^ help: there is a config with a similar name: `false`
|
= help: to expect this configuration use `--check-cfg=cfg(FALSE)`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default
warning: 1 warning emitted

[jdonszelmann]

ok, but this wouldn't trigger on any other mis-capitalization of false. I think it's reasonable to at least also detect False since it's pythonesque. I think it's ok to leave it specific, but I would like to see other capitalizations (just by checking .lower()) and emitting the same diagnostic.

[jdonszelmann]

@rustbot author

[clubby789]

@rustbot ready

[jdonszelmann]

one last thing, could you explain that replacing it with false doesn't mean the cfg named false, but like, it literally means false. like, with this it seems like false is just another defined cfg name. Something like:

note: you may have meant to use false (notice the capitalization). Doing so makes this predicate evaluate to false unconditionally

and similar for true.

[Urgau]

We may want to have a test for r#False.

[clubby789]

This currently raises the suggestion on r#False. I switched to just checking if the source code has r# to prevent this.

[clubby789]

Extended to support true as well

[jdonszelmann]

I like it. Thanks @clubby789 ! Sorry I kept having smol things to fix.

@bors r+ rollup

[bors]

📌 Commit 0349359 has been approved by jdonszelmann

It is now in the queue for this repository.