Add new function_casts_as_integer lint
#141470
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,83 @@ | ||||||
| use rustc_hir as hir; | ||||||
| use rustc_macros::{LintDiagnostic, Subdiagnostic}; | ||||||
| use rustc_middle::ty::{self, Ty}; | ||||||
| use rustc_session::{declare_lint, declare_lint_pass}; | ||||||
| use rustc_span::{BytePos, Span}; | ||||||
|
|
||||||
| use crate::{LateContext, LateLintPass}; | ||||||
|
|
||||||
| declare_lint! { | ||||||
| /// The `function_casts_as_integer` lint detects cases where users cast a function into an | ||||||
| /// integer. | ||||||
| /// | ||||||
| /// ### Example | ||||||
| /// | ||||||
| /// ```rust | ||||||
| /// fn foo() {} | ||||||
| /// let x = foo as usize; | ||||||
| /// ``` | ||||||
| /// | ||||||
| /// {{produces}} | ||||||
| /// | ||||||
| /// ### Explanation | ||||||
| /// | ||||||
| /// You should never cast a function directly into an integer but go through | ||||||
| /// a cast as `fn` first to make it obvious what's going on. It also allows | ||||||
| /// to prevent confusion with (associated) constants. | ||||||
|
Comment on lines
+24
to
+26
There was a problem hiding this comment. This doesn't really explain what is the problem this lint is checking for, ie. why is it bad. What about saying that by casting a function item to an integer it's implicitly creating a function pointer and it's that one that is casted to an integer, and so by making it explicit it improves re-ability of the code and prevents bugs (in particular with enum variants). |
||||||
| pub FUNCTION_CASTS_AS_INTEGER, | ||||||
| Warn, | ||||||
|
There was a problem hiding this comment. Clippy has a few lints for There was a problem hiding this comment. I know, I implemented one myself. 😉 I think it highlights the fact that this is a big issue and that the compiler should warn about it and eventually even forbid this fn to integer cast (you need to cast to an fn pointer first). But in any case, it's up to the lang team. There was a problem hiding this comment. Agreed 👍 Just want to add this information as "prior art" for the lang team to make this decision. Even though it might've sounded like it, I'm not against adding this lint to rustc. Clippy question: Do you think if this lint gets added to rustc, we can (partially) deprecate Clippy lints? There was a problem hiding this comment. Hard to say. For example |
||||||
| "Casting a function into an integer", | ||||||
|
There was a problem hiding this comment. It's by convention always lowercase in the compiler.
Suggested change
|
||||||
| } | ||||||
|
|
||||||
| declare_lint_pass!( | ||||||
| /// Lint for casts of functions into integers. | ||||||
| FunctionCastsAsInteger => [FUNCTION_CASTS_AS_INTEGER] | ||||||
| ); | ||||||
|
|
||||||
| impl<'tcx> LateLintPass<'tcx> for FunctionCastsAsInteger { | ||||||
| fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>) { | ||||||
| let hir::ExprKind::Cast(cast_from_expr, cast_to_expr) = expr.kind else { return }; | ||||||
| let cast_to_ty = cx.typeck_results().expr_ty(expr); | ||||||
| // Casting to a function (pointer?), so all good. | ||||||
| if matches!(cast_to_ty.kind(), ty::FnDef(..) | ty::FnPtr(..)) { | ||||||
| return; | ||||||
| } | ||||||
| let cast_from_ty = cx.typeck_results().expr_ty(cast_from_expr); | ||||||
| if matches!(cast_from_ty.kind(), ty::FnDef(..)) { | ||||||
| cx.tcx.emit_node_span_lint( | ||||||
| FUNCTION_CASTS_AS_INTEGER, | ||||||
| expr.hir_id, | ||||||
| cast_to_expr.span.with_lo(cast_from_expr.span.hi() + BytePos(1)), | ||||||
| FunctionCastsAsIntegerMsg { | ||||||
| sugg: FunctionCastsAsIntegerSugg { | ||||||
| suggestion: cast_from_expr.span.shrink_to_hi(), | ||||||
| // We get the function pointer to have a nice display. | ||||||
| cast_from_ty: cx.typeck_results().expr_ty_adjusted(cast_from_expr), | ||||||
| cast_to_ty, | ||||||
| }, | ||||||
| }, | ||||||
| ); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| #[derive(LintDiagnostic)] | ||||||
| #[diag(lint_function_casts_as_integer)] | ||||||
| struct FunctionCastsAsIntegerMsg<'tcx> { | ||||||
|
There was a problem hiding this comment. By convention we name these with the
Suggested change
|
||||||
| #[subdiagnostic] | ||||||
| sugg: FunctionCastsAsIntegerSugg<'tcx>, | ||||||
| } | ||||||
|
|
||||||
| #[derive(Subdiagnostic)] | ||||||
| #[suggestion( | ||||||
| lint_cast_as_fn, | ||||||
| code = " as {cast_from_ty}", | ||||||
| applicability = "machine-applicable", | ||||||
| style = "verbose" | ||||||
| )] | ||||||
| struct FunctionCastsAsIntegerSugg<'tcx> { | ||||||
| #[primary_span] | ||||||
| pub suggestion: Span, | ||||||
| pub cast_from_ty: Ty<'tcx>, | ||||||
| pub cast_to_ty: Ty<'tcx>, | ||||||
| } | ||||||
|
Comment on lines
+64
to
+83
There was a problem hiding this comment. Should be moved to |
||||||
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.
No need to mention the "users", might be auto-generated code.