diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c2801cfb5e..17ff182c7be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4203,6 +4203,7 @@ Released 2018-09-13 [`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const [`float_equality_without_abs`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_equality_without_abs [`fn_address_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_address_comparisons +[`fn_null_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_null_check [`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast [`fn_to_numeric_cast_any`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_any diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5bae62ce24f..480a65ac70c 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -161,6 +161,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::float_literal::LOSSY_FLOAT_LITERAL_INFO, crate::floating_point_arithmetic::IMPRECISE_FLOPS_INFO, crate::floating_point_arithmetic::SUBOPTIMAL_FLOPS_INFO, + crate::fn_null_check::FN_NULL_CHECK_INFO, crate::format::USELESS_FORMAT_INFO, crate::format_args::FORMAT_IN_FORMAT_ARGS_INFO, crate::format_args::TO_STRING_IN_FORMAT_ARGS_INFO, diff --git a/clippy_lints/src/fn_null_check.rs b/clippy_lints/src/fn_null_check.rs new file mode 100644 index 00000000000..d0e82500dd0 --- /dev/null +++ b/clippy_lints/src/fn_null_check.rs @@ -0,0 +1,129 @@ +use clippy_utils::consts::{constant, Constant}; +use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::{is_integer_literal, is_path_diagnostic_item}; +use if_chain::if_chain; +use rustc_hir::{BinOpKind, Expr, ExprKind, TyKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Checks for comparing a function pointer to null. + /// + /// ### Why is this bad? + /// Function pointers are assumed to not be null. + /// + /// ### Example + /// ```rust,ignore + /// let fn_ptr: fn() = /* somehow obtained nullable function pointer */ + /// + /// if (fn_ptr as *const ()).is_null() { ... } + /// ``` + /// Use instead: + /// ```rust + /// let fn_ptr: Option = /* somehow obtained nullable function pointer */ + /// + /// if fn_ptr.is_none() { ... } + /// ``` + #[clippy::version = "1.67.0"] + pub FN_NULL_CHECK, + correctness, + "`fn()` type assumed to be nullable" +} +declare_lint_pass!(FnNullCheck => [FN_NULL_CHECK]); + +const LINT_MSG: &str = "function pointer assumed to be nullable, even though it isn't"; +const HELP_MSG: &str = "try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value"; + +fn is_fn_ptr_cast(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { + if_chain! { + if let ExprKind::Cast(cast_expr, cast_ty) = expr.kind; + if let TyKind::Ptr(_) = cast_ty.kind; + if cx.typeck_results().expr_ty_adjusted(cast_expr).is_fn(); + then { + true + } else { + false + } + } +} + +impl<'tcx> LateLintPass<'tcx> for FnNullCheck { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { + // Catching: + // (fn_ptr as * ).is_null() + if_chain! { + if let ExprKind::MethodCall(method_name, receiver, _, _) = expr.kind; + if method_name.ident.as_str() == "is_null"; + if is_fn_ptr_cast(cx, receiver); + then { + span_lint_and_help( + cx, + FN_NULL_CHECK, + expr.span, + LINT_MSG, + None, + HELP_MSG + ); + } + } + + if let ExprKind::Binary(op, left, right) = expr.kind + && let BinOpKind::Eq = op.node + { + let to_check: &Expr<'_>; + if is_fn_ptr_cast(cx, left) { + to_check = right; + } else if is_fn_ptr_cast(cx, right) { + to_check = left; + } else { + return; + } + + // Catching: + // (fn_ptr as * ) == + let c = constant(cx, cx.typeck_results(), to_check); + if let Some((Constant::RawPtr(0), _)) = c { + span_lint_and_help( + cx, + FN_NULL_CHECK, + expr.span, + LINT_MSG, + None, + HELP_MSG + ); + return; + } + + // Catching: + // (fn_ptr as * ) == (0 as ) + if let ExprKind::Cast(cast_expr, _) = to_check.kind && is_integer_literal(cast_expr, 0) { + span_lint_and_help( + cx, + FN_NULL_CHECK, + expr.span, + LINT_MSG, + None, + HELP_MSG + ); + return; + } + + // Catching: + // (fn_ptr as * ) == std::ptr::null() + if let ExprKind::Call(func, []) = to_check.kind && + is_path_diagnostic_item(cx, func, sym::ptr_null) + { + span_lint_and_help( + cx, + FN_NULL_CHECK, + expr.span, + LINT_MSG, + None, + HELP_MSG + ); + } + } + } +} diff --git a/clippy_lints/src/transmute/transmute_null_to_fn.rs b/clippy_lints/src/transmute/transmute_null_to_fn.rs index db89078f657..032b4c1e2d2 100644 --- a/clippy_lints/src/transmute/transmute_null_to_fn.rs +++ b/clippy_lints/src/transmute/transmute_null_to_fn.rs @@ -9,7 +9,7 @@ use rustc_span::symbol::sym; use super::TRANSMUTE_NULL_TO_FN; const LINT_MSG: &str = "transmuting a known null pointer into a function pointer"; -const NOTE_MSG: &str = "this transmute results in a null function pointer"; +const NOTE_MSG: &str = "this transmute results in undefined behavior"; const HELP_MSG: &str = "try wrapping your function pointer type in `Option` instead, and using `None` as a null pointer value"; diff --git a/tests/ui/fn_null_check.rs b/tests/ui/fn_null_check.rs new file mode 100644 index 00000000000..df5bc8420d5 --- /dev/null +++ b/tests/ui/fn_null_check.rs @@ -0,0 +1,21 @@ +#![allow(unused)] +#![warn(clippy::fn_null_check)] +#![allow(clippy::cmp_null)] +#![allow(clippy::ptr_eq)] +#![allow(clippy::zero_ptr)] + +pub const ZPTR: *const () = 0 as *const _; +pub const NOT_ZPTR: *const () = 1 as *const _; + +fn main() { + let fn_ptr = main; + + if (fn_ptr as *mut ()).is_null() {} + if (fn_ptr as *const u8).is_null() {} + if (fn_ptr as *const ()) == std::ptr::null() {} + if (fn_ptr as *const ()) == (0 as *const ()) {} + if (fn_ptr as *const ()) == ZPTR {} + + // no lint + if (fn_ptr as *const ()) == NOT_ZPTR {} +} diff --git a/tests/ui/fn_null_check.stderr b/tests/ui/fn_null_check.stderr new file mode 100644 index 00000000000..660dd323979 --- /dev/null +++ b/tests/ui/fn_null_check.stderr @@ -0,0 +1,43 @@ +error: function pointer assumed to be nullable, even though it isn't + --> $DIR/fn_null_check.rs:13:8 + | +LL | if (fn_ptr as *mut ()).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value + = note: `-D clippy::fn-null-check` implied by `-D warnings` + +error: function pointer assumed to be nullable, even though it isn't + --> $DIR/fn_null_check.rs:14:8 + | +LL | if (fn_ptr as *const u8).is_null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value + +error: function pointer assumed to be nullable, even though it isn't + --> $DIR/fn_null_check.rs:15:8 + | +LL | if (fn_ptr as *const ()) == std::ptr::null() {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value + +error: function pointer assumed to be nullable, even though it isn't + --> $DIR/fn_null_check.rs:16:8 + | +LL | if (fn_ptr as *const ()) == (0 as *const ()) {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value + +error: function pointer assumed to be nullable, even though it isn't + --> $DIR/fn_null_check.rs:17:8 + | +LL | if (fn_ptr as *const ()) == ZPTR {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: try wrapping your function pointer type in `Option` instead, and using `is_none` to check for null pointer value + +error: aborting due to 5 previous errors +