Forbid pineapple on pizza by emilyalbini · Pull Request #70645 · rust-lang/rust

14 min read Original article ↗

@emilyalbini

This PR adds a new forbid-by-default lint to the Rust compiler, preventing users from writing wildly untasteful types such as Pizza<Pineapple>.

error

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!

Enet4, imjasonmiller, chqoot-vgxu, nightmared, Centril, Mirabellensaft, Dexterp37, f2koi, Nerzal, terry90, and 409 more reacted with thumbs up emoji xurtis, kornelski, huitseeker, yaahc, jyn514, Ixrec, acanthite1855, cuviper, tomhoule, matthunz, and 111 more reacted with thumbs down emoji phansch, alexbool, mati865, MarcoBuster, Riey, killercup, bnjjj, Enet4, jrouaix, somniumism, and 298 more reacted with laugh emoji JayceFayne, Virgiel, mcronce, DavidHusicka, DasEtwas, zoechi, Xunjin, Thor99, 4dam7, benbrittain, and 48 more reacted with hooray emoji skibz, valters-tomsons, Fussmatte, qthree, Aliath, benbrittain, lauromoura, szbergeron, peterdelevoryas, darakian, and 13 more reacted with confused emoji AlecsFerra, Riey, SnowyCoder, Enet4, somniumism, skade, Centril, ljedrz, Nerzal, terry90, and 126 more reacted with heart emoji Enet4, Nerzal, playXE, miguelraz, Patryk27, hug-dev, Virgiel, hobofan, rigma, nmuldavin, and 48 more reacted with rocket emoji Enet4, fbstj, playXE, miguelraz, Virgiel, louy2, William-Edward-zz, jonhoo, Xunjin, GreeFine, and 30 more reacted with eyes emoji

@emilyalbini

@killercup

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"?

Enet4, jrouaix, Jake-Shadle, skade, kennytm, chqoot-vgxu, Centril, Dexterp37, Evrey, Limeth, and 114 more reacted with thumbs up emoji zoten, jrouaix, mati865, skade, Eijebong, Dexterp37, vishalol, Dylan-DPC-zz, Limeth, 95th, and 46 more reacted with laugh emoji mimoo, anhosh, and karimamer77 reacted with confused emoji eilgin, jasoncarr0, zoomix, GermanDZ, RustyRaptor, xleelz, DocJade, and brookelew reacted with heart emoji

@dns2utf8

I love Ananas on Pizza, can I invert the lint too?

#[allow(pinapple_on_pizza)]  // for liberals
#[enforce(pinapple_on_pizza)] // for me ;)
jamesmunns, snowfrogdev, ars9, 1b15, jebrosen, Virgiel, werner, Proximyst, amadeusine, sakex, and 62 more reacted with thumbs up emoji robinfriedli, L0ry-git, EvanCarroll, jakesarjeant, and k26pl reacted with thumbs down emoji 0x08088405, Xaeroxe, abreis, CYBAI, joseluis, safinsingh, PawelPerek, and kennytm reacted with laugh emoji das-g, Evrey, 95th, Virgiel, amadeusine, valters-tomsons, estebank, ignacygrudzinski, pheki, iredelmeier, and 18 more reacted with heart emoji justinas, Eraden, deb0ch, and thomasantony reacted with rocket emoji

Centril

| |
| this is the pizza you ruined
|
= note: `#[forbid(pineapple_on_pizza)]` on by default

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... 🤔

phansch, Dexterp37, killercup, DianaNites, 95th, YaLTeR, natemcintosh, Virgiel, jeandudey, AdminXVII, and 34 more reacted with thumbs up emoji DenialAdams, aheart, 6543, sunjay, WaffleLapkin, LeSeulArtichaut, Litarvan, ebkalderon, dgsantana, naftulikay, and 13 more reacted with laugh emoji

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 🤔

Centril, DianaNites, 95th, jyn514, YaLTeR, tux3, cramertj, Virgiel, Aaron1011, estebank, and 10 more reacted with heart emoji

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?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you get the span of the whole crate?

Sigourney Weaver in Aliens saying, "It's the only way to be sure."

ebkalderon, pmeinhardt, OpenUAS, Nashenas88, thomasantony, Centril, thomaseizinger, vexx32, RustyRaptor, s0lst1ce, and 7 more reacted with laugh emoji

@jamesmunns

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 allow this lint every time I instantiate my favorite pizza, I think this would be a total blocker for my use of Rust.

huitseeker, yaahc, jyn514, jebrosen, 95th, wackbyte, CJKay, jrdngr, cramertj, Virgiel, and 50 more reacted with thumbs up emoji Enet4, Dowwie, hckr, EvanCarroll, bowlofeggs, mkhan45, Vrabbers, lovesegfault, Neightro, brendanzab, and 5 more reacted with thumbs down emoji Centril, Dexterp37, Enet4, mark-i-m, insanitybit, ratijas, and RustyRaptor reacted with confused emoji cramertj, Virgiel, tjkirch, guswynn, torch2424, szbergeron, SimonWoodburyForget, DianeLooney, Peter-JanGootzen, adam-mcdaniel, and 5 more reacted with heart emoji

@Dexterp37

Please, let's merge this ASAP. I would make my life so much easier if this would become a de-facto standard.

EvanCarroll, scooter-dangle, adevore, AlexAegis, torch2424, szbergeron, Xaeroxe, mark-i-m, Neightro, optozorax, and 11 more reacted with thumbs up emoji

@kornelski

I think this feature is overly specific. For example, I'd also like to have warnings about Anchovies.

And it doesn't work for Marmite/Vegemite types.

munik, arguablykomodo, najamelan, CJKay, SystemNeoSpl, Virgiel, amadeusine, Masterchef365, sakex, qthree, and 25 more reacted with thumbs up emoji OpenUAS and RustyRaptor reacted with laugh emoji

@Dylan-DPC-zz

@bjorn3

Does this need a fcp with all teams? This concerns preventing a crime against the taste of everybody.

jyn514


declare_lint! {
pub PINEAPPLE_ON_PIZZA,
Forbid,

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).

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).

awortman-fastly, arguablykomodo, SystemNeoSpl, Virgiel, amadeusine, haze, nico-abram, BatmanAoD, crides, guswynn, and 20 more reacted with thumbs up emoji joshtriplett reacted with thumbs down emoji jyn514, iancormac84, wesleywiser, RustyYato, arguablykomodo, prestontw, SystemNeoSpl, Virgiel, amadeusine, Nemikolh, and 35 more reacted with laugh emoji

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.

@Ixrec

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.

RustyYato, cramertj, werner, amadeusine, qthree, wnoise, hckr, evaporei, schirtze, diamondburned, and 49 more reacted with thumbs up emoji

@Dylan-DPC-zz

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"?

You need to contact the localisation team for that. I've added it to their non-existent agenda.

@cuviper cuviper added the D-incorrect

Diagnostics: A diagnostic that is giving misleading or incorrect information.

label

Apr 1, 2020

nagisa

struct Pineapple;

fn main() {
let _: Pizza<Pineapple>; //~ERROR pineapple doesn't go on pizza

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.

@amanjeev

Removing Pineapple will reduce the cost of pizza as well. Isn't something zero cost something a thing here?

gdf8gdn8, fedenator, diamondburned, mkhan45, akwodkiewicz, zebp, reillysiemens, Neightro, akshatagarwl, optozorax, and 13 more reacted with thumbs up emoji

@bjorn3

I don't believe we allow negative cost abstractions.

jyn514, arguablykomodo, jebrosen, SystemNeoSpl, cramertj, Viranchee, hombit, pheki, qthree, wnoise, and 39 more reacted with laugh emoji

@Centril

@jebrosen

Does this (and should this) PR see through type aliases? It would be good to document whether type Mushroom = Pineapple; is a suitable workaround for this lint.

@cuviper

It would be good to document whether type Mushroom = Pineapple; is a suitable workaround for this lint.

Speaking as a fan of both, that alias is an abomination.

matiaskotlik, maksimil, hapenia, and k26pl reacted with thumbs up emoji jyn514, AdminXVII, estebank, pheki, szbergeron, tidux, mark-i-m, matiaskotlik, icew4ll, fbstj, and 7 more reacted with laugh emoji matiaskotlik reacted with hooray emoji matiaskotlik and davidwneary reacted with heart emoji matiaskotlik reacted with rocket emoji matiaskotlik reacted with eyes emoji

@Dowwie

Pizza<Pineapple>, one of the most unsound combinations, deserves its own CVE.

@Dylan-DPC-zz

amanjeev, Nemikolh, bdelmas, lachlansneff, mcronce, eira-fransham, kontheocharis, arguablykomodo, BigRedEye, szbergeron, and 28 more reacted with thumbs up emoji

kush-patel-hs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Aaron1011

continue;
}
for arg in args.args {
if let hir::GenericArg::Type(hir::Ty {

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:

should not be allowed, since there is no value to having pineapple on pizza.

estebank, VictorKoenders, killercup, evaporei, TimDiekmann, arutr, itsmontoya, juliavdkris, eiennohi, brycesteinhoff, and 31 more reacted with laugh emoji

@estebank

I am uncomfortable with adding this to rustc, this clearly belongs in clippy for many reasons, not the least of which that I like pineapple in pizza.

benbrittain, WaffleLapkin, myrrlyn, LPGhatguy, cacampbell, martinrlilja, tidux, peterdelevoryas, cardiffman, spacekookie, and 13 more reacted with thumbs up emoji Centril, amanjeev, Dowwie, mkhan45, szbergeron, zebp, quat1024, ForsakenHarmony, and ratijas reacted with thumbs down emoji emilyalbini, Centril, amanjeev, killercup, peterdelevoryas, CYBAI, janriemer, and Pairman reacted with laugh emoji jamesmunns, peterdelevoryas, luqmana, jasoncarr0, yasiupl, and Pairman reacted with hooray emoji ratijas reacted with confused emoji emilyalbini, jebrosen, cuviper, pthariensflame, amanjeev, Centril, Fussmatte, iredelmeier, benbrittain, tjkirch, and 10 more reacted with heart emoji

estebank

{
for pineapple_segment in path.segments {
if pineapple_segment.ident.name.as_str().to_lowercase()
== "pineapple"

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🍍.

jonhoo, Aaron1011, pheki, itsmontoya, garrettmaring, bpartridge, myrrlyn, crides, BigRedEye, szbergeron, and 15 more reacted with thumbs up emoji

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🍎.

DasEtwas, eira-fransham, jamesmunns, bpartridge, jdonszelmann, fbstj, thomasaiman, timotree3, xarvic, leetfin, and 3 more reacted with thumbs up emoji DasEtwas, jyn514, bjorn3, oriontvv, sgrif, Erutuon, cole-h, joefarebrother, jasoncarr0, RalfJung, and 8 more reacted with laugh emoji

@timClicks

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.

@amanjeev

@jdonszelmann

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.

@hollmmax

What happens if you transmute Pineapple to any other topping?

fn main() {
    unsafe {
        let topping = std::mem::transmute::<Pineapple, Topping>(Pineapple);
    }
    let pizza = Pizza(Topping);
}

One solution would be to forbid all instances of Pineapple. But I think the fruit market won't be happy with that decision

This would be undefined behavior, since Pineapple is not a Topping.

Pineapple is a valid Topping, just not for any food implementing the Salty trait.
Instead the problem is in the unsafe wrapping, he's just putting pineapples into tortillas, and pretending that's fine. Highly unsafe.

@hollmmax

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 Sweet trait, which would also prevent other abominations such as chocolate pizza.

@randysecrist

I would like this feature in 30 minutes or less or I want my money back.

@AZon8

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.

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.
A user can await it for multiple years. However, i plan to build my API such that when the plant is pending , i.e. planding, the API prevents the construction of a Pizza struct as this would allow the user to get a mutilated reference by unpinning the basis of a civilized world.

rochacbruno

@thefallentree

@jvantuyl

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?)
🚫 🍕 🍍

@mark-i-m

I actually originally came to this thread expecting it to be about a parser change or something. I'm pretty sure the A-pizza label should be added.

@ccfb3ee765a58cae

If we're going to have an option to #[allow(pinapple_on_pizza)], can we at least have pineapple_on_pizza.await!() syntax?

wusyong

@Havvy

@DutchGhost

what about people who write Pizza<Banana> ? Banana doesn't belong on Pizza either!

@trondhe

Clearly unsound behaviour.

@Daksh14

We should be writing lints for various illegal toppings, them being on pizza should be UB

  1. Pineapples
  2. Bananas
  3. Strawberries
  4. Kiwis

ASAP.

@nagisa

Now that entirety of the planet managed to survive April the 1st, closing.

EvanCarroll, playXE, mark-i-m, dsluijk, Rekkonnect, Snoop05, Suya1671, oleggtro, maksimil, apppppppple, and 17 more reacted with thumbs down emoji Newbytee, locnide, lunabunn, seandewar, coolshaurya, pro465, Pairman, and flippette reacted with laugh emoji Daksh14, terinjokes, mrchief, dns2utf8, literalplus, nathanfranke, locnide, lunabunn, coolshaurya, RedL0tus, and 6 more reacted with hooray emoji ebolyen, estebank, HadrienG2, Daksh14, TheButlah, CodingKoopa, Rekkonnect, locnide, lunabunn, Anders429, and 3 more reacted with heart emoji

@JimLynchCodes

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)!

@mr-cheffy

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,
The Anti-Pineapple Pizza Task Force (Retired)

@fgclue

We should be writing lints for various illegal toppings, them being on pizza should be UB

1. Pineapples

2. Bananas

3. Strawberries

4. Kiwis

ASAP.

Eat rust, strawberry on pizza is awesome.

@fgclue

I mean rust, like, that thing, not the programming language.