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

Add alias 'nonstandard_style' for 'bad_style' #41646

Closed
Manishearth opened this issue Apr 30, 2017 · 82 comments
Closed

Add alias 'nonstandard_style' for 'bad_style' #41646

Manishearth opened this issue Apr 30, 2017 · 82 comments
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Manishearth
Copy link
Member

@ashleygwilliams mentioned this in her talk today; #[allow(bad_style)] is a rather rebuke-y shame-y annotation on code.

While we do want people to use proper style, it's a useful tool whilst learning to be able to turn off all the extra friction that you deal with that you don't have to. Let's be nicer about it; could we rename this to nonstandard_style and keep bad_style as a backcompat alias?

Unsure if this requires an RFC.

@Manishearth Manishearth added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Apr 30, 2017
@Manishearth
Copy link
Member Author

(Actual name of alias very bikesheddable, I just picked the first thing that came to mind.)

@skade
Copy link
Contributor

skade commented Apr 30, 2017

Bikeshed: "relaxed_style" sounds good.

@leonardo-m
Copy link

"bad_style" sounds better than "nonstandard_style" because "nonstandard" sounds un-opinionated. The point of a "bad_style" annotation is to help people feel guilty.

@Manishearth
Copy link
Member Author

Right, I'm questioning if that's a good thing. Do we need to? Even nonstandard_style (or in general, any warning-allow) does make you feel guilty, bad_style is just very much like a shaming and feels bad.

@mgattozzi
Copy link
Contributor

Her talk was great. I think using something like relaxed_style or permissive_style would be a better choice then bad_style

@tumdum
Copy link

tumdum commented Apr 30, 2017

I think that 'any_style' is short and is not passive aggressive at all :)

@steveklabnik steveklabnik added I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Apr 30, 2017
@steveklabnik
Copy link
Member

marking as both @rust-lang/compiler and @rust-lang/lang ; not sure who this falls under, really. Also nominating for discussion; I'm not sure if this requires an RFC or not.

@retep998
Copy link
Member

retep998 commented Apr 30, 2017

As one of the heaviest users of bad_style (https://github.com/retep998/winapi-rs/blob/dev/src/lib.rs#L9), I'd like to voice my lack of opinion on this matter.

@ashleygwilliams
Copy link
Member

i have no opinion on what we alias it as, but i would be very interested in an alias that is less shame-y. i am also very happy to implement once we decide what a good alias is!

@nikomatsakis
Copy link
Contributor

For those readers who (like me) were not familiar with the bad_style group, it refers to a collection of 3 lints that enforce various capitalization conventions (e.g., snake_case methods, SHOUTY_GLOBALS, etc).

I'm in favor of renaming. I like any_style because it's short and easy to spell, but #[deny(any_style)] doesn't make any sense to me. =)

Regarding whether this ought to have an RFC -- I could go either way; if we opt for no RFC, then we ought to at least do a "mini-FCP" here in the issue. TBH I'm not sure what team has ownership of lints though. =) I guess @rust-lang/lang?

@aturon
Copy link
Member

aturon commented May 3, 2017

I definitely don't feel this needs an RFC. I also wholeheartedly agree that we should change the name.

nonstandard_style or alternative_style seem good to me.

@whitequark
Copy link
Member

alternative_style seem good to me

Do we really want the "alternative facts" connotation? I can already see someone writing that awful snark and the HN frontpage it'll hit and I'd really rather not have it.

@aturon
Copy link
Member

aturon commented May 3, 2017

@whitequark

Heh, I hadn't realized that the term "alternative" had gotten quite so poisoned! Anyway my preferences here aren't very strong.

@withoutboats
Copy link
Contributor

withoutboats commented May 3, 2017

I'm happy with any non-judgmental name, we can FCP the PR (I also don't care if we don't)

@whitequark
Copy link
Member

FWIW I like the any_style suggestion as it makes no statement about the particular style in question. I am not worried at all about this change leading to more deviation from conventions because:

  • humans are very good on exerting conformance pressure on their own, no need for the compiler assist;
  • the fact that warnings are emitted by default already make anyone new to Rust stop and think;
  • most Rust code is already uniform to a degree that I have not seen elsewhere, and there's no clear reason why that would suddenly change;
  • rustfmt isn't going to be very configurable, further fossilizing the conventional style.

If something I'm worried there's too much friction for even minor deviations; I don't think I'll ever use rustfmt, for example.

@joshtriplett
Copy link
Member

joshtriplett commented May 3, 2017

Given that this comes up often in code translators (such as corrode), and potentially in binding generators as well, then having a more neutral alias for this seems appropriate. I also don't think adding an alias needs an RFC.

I'm not going to add to the bikeshedding on the name itself. 🚳

@jseyfried
Copy link
Contributor

any_style seems too broad -- it includes the standard style, so #![deny(any_style)] is weird.

I prefer nonstandard_style -- I think it's clearer than alternative_style since the latter could sound like we have an alternative in mind and isn't clear about to which it is alternative.

@est31
Copy link
Member

est31 commented May 3, 2017

As someone who has been annoyed by some C++ codebases that despite having official casing rules use different casing styles in parallel (including libraries using different casing styles), I am glad of those checks to be in the compiler and default on, because they create consistency. I believe this wish of consistency in this issue to be universal, so this isn't just a style issue where everyone can have its own, as your choice of casing gets forced upon the users of your library as well. I use tabs in my codebase and am writing let v :Type = try!(foo()) instead of let v: Type = foo()? so I'm not "mainstream" regarding style, but those choices don't escape my codebase so I'm comfortable doing them.

This check has many positive side effects. Like when reading code, you get accustomed to differentiate things inside lists of use clauses by their cases.

Looking at the issue from the other perspective, renaming the lint to nonstandard_style makes you feel intolerant when you type #![forbid(nonstandard_style)]. So one group gets always shamed.

Also, for beginners its important to see that the style they are doing is not fine, but rather something that is bad for the community of crates, especially their users, due to the inconsistency to the other crates on crates.io.

Overall though I don't feel very strongly about a rename, as long as the lint stays a warning by default.

@Manishearth
Copy link
Member Author

renaming the lint to nonstandard_style makes you feel intolerant when you type #![forbid(nonstandard_style)]. So one group gets always shamed.

No more than #![forbid(bad_style)].

Beginners already see that the style is not great. This is just about making it possible to design your own beginner experience without feeling bad about it. This doesn't change the warning's existence, it just changes the name so that it isn't discouraging when disabled.

@est31
Copy link
Member

est31 commented May 22, 2017

One example for how the snake case style helps you to identify a bug: https://is.gd/XArdpj

You need to look twice to see that the last case in the match is creating a new variable instead of being an actual enum variant.

@skade
Copy link
Contributor

skade commented May 23, 2017

@est31 I find this example rather contrived (it switches off multiple layers of safety and imports the matching shorthand).

Sure, "nonstandard_style" brings you half-way there, but the option is in the compiler and supported, this discussion is about an alias only.

@nikomatsakis nikomatsakis removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-nominated labels May 24, 2017
@nikomatsakis
Copy link
Contributor

@rfcbot fcp merge

I agree we don't need an RFC. I'm moving that we agree to change the name to nonstandard_style (and deprecate -- but naturally not remove! -- the old name bad_style).

@est31
Copy link
Member

est31 commented May 24, 2017

contrived

Its not a contrived example: I have encountered it in one of my programs. Luckily I had the bad_style lint turned on, so I didn't actually encounter the bug itself.

Sure, "nonstandard_style" brings you half-way there, but the option is in the compiler and supported, this discussion is about an alias only.

No, its about deprecation as well, and probably a Rust 2.0 or something will remove the name.

@Manishearth
Copy link
Member Author

Most lints prevent bugs. That isn't an argument for giving it a name like that.

its about deprecation as well

deprecating the name, not the lint.

@rust-lang rust-lang unlocked this conversation Aug 30, 2017
@rust-lang rust-lang deleted a comment Sep 6, 2017
@rust-lang rust-lang deleted a comment Sep 6, 2017
@rust-lang rust-lang locked and limited conversation to collaborators Sep 13, 2017
@kennytm kennytm added C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Feb 22, 2018
@Centril Centril added finished-final-comment-period The final comment period is finished for this PR / Issue. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 24, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Aug 29, 2018

This was fixed in #48386

(looks like we forgot to unlock)

@rust-lang rust-lang unlocked this conversation Aug 29, 2018
@carols10cents
Copy link
Member

@Manishearth #48386 has a comment from you saying that it doesn't fix this issue completely, reopening because I don't see any other linked PRs

@carols10cents carols10cents reopened this Aug 29, 2018
@Manishearth
Copy link
Member Author

Manishearth commented Aug 29, 2018 via email

pietroalbini pushed a commit to pietroalbini/rust that referenced this issue Aug 29, 2018
`bad_style` is being deprecated in favor of `nonstandard_style`:

- rust-lang#41646
pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 29, 2018
…shearth

Replace usages of 'bad_style' with 'nonstandard_style'.

`bad_style` is being deprecated in favor of `nonstandard_style`:

- rust-lang#41646
pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 29, 2018
…shearth

Replace usages of 'bad_style' with 'nonstandard_style'.

`bad_style` is being deprecated in favor of `nonstandard_style`:

- rust-lang#41646
pietroalbini added a commit to pietroalbini/rust that referenced this issue Aug 30, 2018
…shearth

Replace usages of 'bad_style' with 'nonstandard_style'.

`bad_style` is being deprecated in favor of `nonstandard_style`:

- rust-lang#41646
@WiSaGaN

This comment has been minimized.

@retep998

This comment has been minimized.

@mbrubeck
Copy link
Contributor

mbrubeck commented Sep 8, 2018

Please direct comments about moderation to the moderators or to new forum threads.

bors added a commit that referenced this issue Oct 16, 2018
Make `bad_style` a silent alias for `nonstandard_style`

Now only `nonstandard_style` is suggested in `rustc -W help`, but `bad_style` will not produce a warning. Closes #41646.

r? @Manishearth
djrenren pushed a commit to djrenren/libtest that referenced this issue Jan 22, 2019
`bad_style` is being deprecated in favor of `nonstandard_style`:

- rust-lang/rust#41646
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests