-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,52 @@ | ||||||
use crate::{LateContext, LateLintPass, LintContext}; | ||||||
use rustc_hir as hir; | ||||||
|
||||||
declare_lint! { | ||||||
pub PINEAPPLE_ON_PIZZA, | ||||||
Forbid, | ||||||
"pineapple doesn't go on pizza" | ||||||
} | ||||||
|
||||||
declare_lint_pass!(PineappleOnPizza => [PINEAPPLE_ON_PIZZA]); | ||||||
|
||||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PineappleOnPizza { | ||||||
fn check_ty(&mut self, cx: &LateContext<'_, '_>, ty: &hir::Ty<'_>) { | ||||||
if let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = &ty.kind { | ||||||
for pizza_segment in path.segments { | ||||||
if let Some(args) = pizza_segment.args { | ||||||
if pizza_segment.ident.name.as_str().to_lowercase() != "pizza" { | ||||||
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 commentThe 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. |
||||||
kind: hir::TyKind::Path(hir::QPath::Resolved(_, path)), | ||||||
.. | ||||||
}) = arg | ||||||
{ | ||||||
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 commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't correctly interact with There was a problem hiding this comment. Choose a reason for hiding this commentThe 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🍎. |
||||||
{ | ||||||
cx.struct_span_lint( | ||||||
PINEAPPLE_ON_PIZZA, | ||||||
pineapple_segment.ident.span, | ||||||
|lint| { | ||||||
let mut err = | ||||||
lint.build("pineapple doesn't go on pizza"); | ||||||
err.span_label( | ||||||
pizza_segment.ident.span, | ||||||
"this is the pizza you ruined", | ||||||
); | ||||||
err.note("you're a monster"); // Yep Esteban, you are. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Remove salty comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nah. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok with it. |
||||||
err.emit(); | ||||||
}, | ||||||
); | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is way too little nesting. Please use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, fancy! Thankyou! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
struct Pizza<T>(T); | ||
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 commentThe 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. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
error: pineapple doesn't go on pizza | ||
--> $DIR/pineapple-on-pizza.rs:5:18 | ||
| | ||
LL | let _: Pizza<Pineapple>; | ||
| ----- ^^^^^^^^^ | ||
| | | ||
| 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 commentThe 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 commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. |
||
= note: you're a monster | ||
|
||
error: aborting due to previous error | ||
|
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)
, butallow(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.