-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Forbid pineapple on pizza #70645
Forbid pineapple on pizza #70645
Conversation
I work with a lot of international people and even some linguists so our code bases are always fully translated. For increased coverage, can you maybe also disallow "ananas"? |
I love Ananas on Pizza, can I invert the lint too?
|
| | | ||
| this is the pizza you ruined | ||
| | ||
= note: `#[forbid(pineapple_on_pizza)]` on by default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the atrocious crime against taste that has been committed here, I wonder if we should delete the file from the user's hard-drive. Although, I'm not sure how to test that... 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could get close by obtaining the whole span of the file and adding a removal suggestion to remove the complete span. Since we have rustfix tests this should be easily testable 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that's clever! @pietroalbini Can you adjust the PR to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is irresponsible to land these kinds of changes without an associated RFC. I, for one, find pineapple based toppings wonderful. For example, this would disrupt my workflow around creating my favorite pizza: #[allow(pineapple_on_pizza)] // UNACCEPTABLE!
let _: Pizza<(Pepperoni, Pineapple, Jalapeno)>; If I was forced to |
Please, let's merge this ASAP. I would make my life so much easier if this would become a de-facto standard. |
I think this feature is overly specific. For example, I'd also like to have warnings about And it doesn't work for |
We need an fcp for this. |
Does this need a fcp with all teams? This concerns preventing a crime against the taste of everybody. |
|
||
declare_lint! { | ||
pub PINEAPPLE_ON_PIZZA, | ||
Forbid, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs its own lint group. It should go with other similar lints, like allow(bad_style)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not allow(bad_style)
, but allow(bad_taste)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should allow pineapple pizza to be instantiated regardless of the pragma or directive. We shouldn't permit terrorism.
Unfortunately toppings are generally chosen at runtime, by the user, so a compile-time lint would catch almost nothing in practice. Clearly, the only proper solution here is a full-blown effect system with dependent typing (and probably generic modules just to be sure), so we can statically verify that e.g. calling .sort() on a VecWithPineappleAtTheEnd turns into a VecWithPineappleWherever. That will definitely require an RFC. Or fifty. |
You need to contact the localisation team for that. I've added it to their non-existent agenda. |
struct Pineapple; | ||
|
||
fn main() { | ||
let _: Pizza<Pineapple>; //~ERROR pineapple doesn't go on pizza |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A test is necessary to ensure that this lint fires regardless of the order in which – or count of – ingredients are applied to the base.
Removing Pineapple will reduce the cost of pizza as well. Isn't something zero cost something a thing here? |
I don't believe we allow negative cost abstractions. |
We should have accepted @Gankra's RFC (rust-lang/rfcs#1963) which enabled NCAs! |
Does this (and should this) PR see through type aliases? It would be good to document whether |
Speaking as a fan of both, that alias is an abomination. |
|
Folks, pineapple is UB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
continue; | ||
} | ||
for arg in args.args { | ||
if let hir::GenericArg::Type(hir::Ty { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be extended to support const generics:
Pizza<pineapple()>
should not be allowed, since there is no value to having pineapple on pizza.
I am uncomfortable with adding this to |
{ | ||
for pineapple_segment in path.segments { | ||
if pineapple_segment.ident.name.as_str().to_lowercase() | ||
== "pineapple" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't correctly interact with non_ascii_idents
, it should also account for 🍍 and 🍕. For future compatibility it should probably also support 🍕ZWJ🍍.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also account for people try to sneak things into the codebase with 🌲ZWJ🍎.
This needs to be more general to justify inclusion imo. Perhaps it be possible to include the Linnaean taxonomy into the Rust type system somehow? That would allow taxa to specified unambiguously and could scale to include whole branches. Implementation is left as an exercise for the reader. |
@estebank fork your own please |
I think that it would be way easier to whitelist only certain pizza ingredients and combinations, instead of blacklisting pineapples. That way we could remove a lot of corner cases like the kiwi. |
|
unacceptable, this would remove local ingredient, such as traditional cheese and salami, which did not gain international notoriety. Instead, we could disallow all ingredients implementing the |
I would like this feature in 30 minutes or less or I want my money back. |
I agree. Trying to prevent the user from constructing a logical error through identifying common patterns is a waste of time. Any solution should at least be able to solve the halting problem before we get half baked solutions like this. My approach to this problem is set up as follows: I'm exposing a Seed struct implementing a future that resolves to a Fruit enum. |
:triggered: |
Please add this! It’s the last thing stopping me from moving entirely to Rust from PHP! (We all know it really means Pizza Hates Pineapple, right?) |
I actually originally came to this thread expecting it to be about a parser change or something. I'm pretty sure the |
If we're going to have an option to |
This would break code showcased in Rustconf 2017: https://www.youtube.com/watch?v=wxPehGkoNOw&list=PL85XCvVPmGQhUSX_QBkxb4g1-o56cCqI9&index=7&t=527s |
what about people who write |
Clearly unsound behaviour. |
We should be writing lints for various illegal toppings, them being on pizza should be UB
ASAP. |
Now that entirety of the planet managed to survive April the 1st, closing. |
I agree with OP, but come on @Daksh14- kiwi pizza isn't that bad... in fact, I would propose kiwi pizza to be America's new pastime (since organized sports are a thing if the past now)! |
Dear Pineapple Prohibition Committee, Thank you for your meticulous review and subsequent rejection of the PR proposing a pineapple-free pizza policy in our source code. It seems our attempt to banish the tropical intruder has met with a resistance fiercer than a chef guarding his secret marinara recipe. While our intentions were noble, aiming to protect the sanctity of traditional pizza toppings, we recognize that our efforts have been in vain. We accept that pineapple on pizza is a matter of personal taste, much like coding styles or preferred text editors. We propose a truce. Let's embrace the diversity of pizza toppings as we do our coding languages. Whether you're a pineapple enthusiast or a purist, there's room for everyone at the pizza table. After all, it's the melting pot of flavors (and opinions) that makes our community so deliciously unique. May our code remain as versatile as our pizza choices. 🍕🍍 Sincerely, |
Eat rust, strawberry on pizza is awesome. |
I mean rust, like, that thing, not the programming language. |
This PR adds a new forbid-by-default lint to the Rust compiler, preventing users from writing wildly untasteful types such as
Pizza<Pineapple>
.r? @estebank
cc @steveklabnik, what do you think is the best way to document this feature?
This was inspired by @sgrif's awesome RustConf 2017 talk. Check it out!