From a23af89447d467c38ed2e5c9358cc81b47dc346d Mon Sep 17 00:00:00 2001 From: Michael Wright Date: Fri, 3 Sep 2021 08:34:34 +0200 Subject: [PATCH] Make `approx_const` MSRV aware Fixes #7623. --- clippy_lints/src/approx_const.rs | 127 +++++++++++++++----------- clippy_lints/src/lib.rs | 2 +- clippy_utils/src/msrvs.rs | 1 + tests/ui/min_rust_version_attr.rs | 5 + tests/ui/min_rust_version_attr.stderr | 8 +- 5 files changed, 83 insertions(+), 60 deletions(-) diff --git a/clippy_lints/src/approx_const.rs b/clippy_lints/src/approx_const.rs index b04c50e9d..069a10e9b 100644 --- a/clippy_lints/src/approx_const.rs +++ b/clippy_lints/src/approx_const.rs @@ -1,8 +1,10 @@ use clippy_utils::diagnostics::span_lint; +use clippy_utils::{meets_msrv, msrvs}; use rustc_ast::ast::{FloatTy, LitFloatType, LitKind}; use rustc_hir::{Expr, ExprKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; +use rustc_semver::RustcVersion; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::symbol; use std::f64::consts as f64; @@ -36,68 +38,83 @@ declare_clippy_lint! { "the approximate of a known float constant (in `std::fXX::consts`)" } -// Tuples are of the form (constant, name, min_digits) -const KNOWN_CONSTS: [(f64, &str, usize); 18] = [ - (f64::E, "E", 4), - (f64::FRAC_1_PI, "FRAC_1_PI", 4), - (f64::FRAC_1_SQRT_2, "FRAC_1_SQRT_2", 5), - (f64::FRAC_2_PI, "FRAC_2_PI", 5), - (f64::FRAC_2_SQRT_PI, "FRAC_2_SQRT_PI", 5), - (f64::FRAC_PI_2, "FRAC_PI_2", 5), - (f64::FRAC_PI_3, "FRAC_PI_3", 5), - (f64::FRAC_PI_4, "FRAC_PI_4", 5), - (f64::FRAC_PI_6, "FRAC_PI_6", 5), - (f64::FRAC_PI_8, "FRAC_PI_8", 5), - (f64::LN_2, "LN_2", 5), - (f64::LN_10, "LN_10", 5), - (f64::LOG2_10, "LOG2_10", 5), - (f64::LOG2_E, "LOG2_E", 5), - (f64::LOG10_2, "LOG10_2", 5), - (f64::LOG10_E, "LOG10_E", 5), - (f64::PI, "PI", 3), - (f64::SQRT_2, "SQRT_2", 5), +// Tuples are of the form (constant, name, min_digits, msrv) +const KNOWN_CONSTS: [(f64, &str, usize, Option); 18] = [ + (f64::E, "E", 4, None), + (f64::FRAC_1_PI, "FRAC_1_PI", 4, None), + (f64::FRAC_1_SQRT_2, "FRAC_1_SQRT_2", 5, None), + (f64::FRAC_2_PI, "FRAC_2_PI", 5, None), + (f64::FRAC_2_SQRT_PI, "FRAC_2_SQRT_PI", 5, None), + (f64::FRAC_PI_2, "FRAC_PI_2", 5, None), + (f64::FRAC_PI_3, "FRAC_PI_3", 5, None), + (f64::FRAC_PI_4, "FRAC_PI_4", 5, None), + (f64::FRAC_PI_6, "FRAC_PI_6", 5, None), + (f64::FRAC_PI_8, "FRAC_PI_8", 5, None), + (f64::LN_2, "LN_2", 5, None), + (f64::LN_10, "LN_10", 5, None), + (f64::LOG2_10, "LOG2_10", 5, Some(msrvs::LOG2_10)), + (f64::LOG2_E, "LOG2_E", 5, None), + (f64::LOG10_2, "LOG10_2", 5, Some(msrvs::LOG10_2)), + (f64::LOG10_E, "LOG10_E", 5, None), + (f64::PI, "PI", 3, None), + (f64::SQRT_2, "SQRT_2", 5, None), ]; -declare_lint_pass!(ApproxConstant => [APPROX_CONSTANT]); +pub struct ApproxConstant { + msrv: Option, +} + +impl ApproxConstant { + #[must_use] + pub fn new(msrv: Option) -> Self { + Self { msrv } + } + + fn check_lit(&self, cx: &LateContext<'_>, lit: &LitKind, e: &Expr<'_>) { + match *lit { + LitKind::Float(s, LitFloatType::Suffixed(fty)) => match fty { + FloatTy::F32 => self.check_known_consts(cx, e, s, "f32"), + FloatTy::F64 => self.check_known_consts(cx, e, s, "f64"), + }, + LitKind::Float(s, LitFloatType::Unsuffixed) => self.check_known_consts(cx, e, s, "f{32, 64}"), + _ => (), + } + } + + fn check_known_consts(&self, cx: &LateContext<'_>, e: &Expr<'_>, s: symbol::Symbol, module: &str) { + let s = s.as_str(); + if s.parse::().is_ok() { + for &(constant, name, min_digits, msrv) in &KNOWN_CONSTS { + if is_approx_const(constant, &s, min_digits) + && msrv.as_ref().map_or(true, |msrv| meets_msrv(self.msrv.as_ref(), msrv)) + { + span_lint( + cx, + APPROX_CONSTANT, + e.span, + &format!( + "approximate value of `{}::consts::{}` found. \ + Consider using it directly", + module, &name + ), + ); + return; + } + } + } + } +} + +impl_lint_pass!(ApproxConstant => [APPROX_CONSTANT]); impl<'tcx> LateLintPass<'tcx> for ApproxConstant { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { if let ExprKind::Lit(lit) = &e.kind { - check_lit(cx, &lit.node, e); + self.check_lit(cx, &lit.node, e); } } -} -fn check_lit(cx: &LateContext<'_>, lit: &LitKind, e: &Expr<'_>) { - match *lit { - LitKind::Float(s, LitFloatType::Suffixed(fty)) => match fty { - FloatTy::F32 => check_known_consts(cx, e, s, "f32"), - FloatTy::F64 => check_known_consts(cx, e, s, "f64"), - }, - LitKind::Float(s, LitFloatType::Unsuffixed) => check_known_consts(cx, e, s, "f{32, 64}"), - _ => (), - } -} - -fn check_known_consts(cx: &LateContext<'_>, e: &Expr<'_>, s: symbol::Symbol, module: &str) { - let s = s.as_str(); - if s.parse::().is_ok() { - for &(constant, name, min_digits) in &KNOWN_CONSTS { - if is_approx_const(constant, &s, min_digits) { - span_lint( - cx, - APPROX_CONSTANT, - e.span, - &format!( - "approximate value of `{}::consts::{}` found. \ - Consider using it directly", - module, &name - ), - ); - return; - } - } - } + extract_msrv_attr!(LateContext); } /// Returns `false` if the number of significant figures in `value` are diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 2b76eacb7..0d9b81e3d 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1865,7 +1865,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(needless_bool::NeedlessBool)); store.register_late_pass(|| Box::new(needless_bool::BoolComparison)); store.register_late_pass(|| Box::new(needless_for_each::NeedlessForEach)); - store.register_late_pass(|| Box::new(approx_const::ApproxConstant)); store.register_late_pass(|| Box::new(misc::MiscLints)); store.register_late_pass(|| Box::new(eta_reduction::EtaReduction)); store.register_late_pass(|| Box::new(identity_op::IdentityOp)); @@ -1894,6 +1893,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: }); let avoid_breaking_exported_api = conf.avoid_breaking_exported_api; + store.register_late_pass(move || Box::new(approx_const::ApproxConstant::new(msrv))); store.register_late_pass(move || Box::new(methods::Methods::new(avoid_breaking_exported_api, msrv))); store.register_late_pass(move || Box::new(matches::Matches::new(msrv))); store.register_early_pass(move || Box::new(manual_non_exhaustive::ManualNonExhaustive::new(msrv))); diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 14234d9c9..6fab17f07 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -17,6 +17,7 @@ msrv_aliases! { 1,50,0 { BOOL_THEN } 1,46,0 { CONST_IF_MATCH } 1,45,0 { STR_STRIP_PREFIX } + 1,43,0 { LOG2_10, LOG10_2 } 1,42,0 { MATCHES_MACRO } 1,41,0 { RE_REBALANCING_COHERENCE, RESULT_MAP_OR_ELSE } 1,40,0 { MEM_TAKE, NON_EXHAUSTIVE, OPTION_AS_DEREF } diff --git a/tests/ui/min_rust_version_attr.rs b/tests/ui/min_rust_version_attr.rs index 7f9f7ddc5..8d9fc5a86 100644 --- a/tests/ui/min_rust_version_attr.rs +++ b/tests/ui/min_rust_version_attr.rs @@ -4,6 +4,11 @@ use std::ops::{Deref, RangeFrom}; +fn approx_const() { + let log2_10 = 3.321928094887362; + let log10_2 = 0.301029995663981; +} + fn cloned_instead_of_copied() { let _ = [1].iter().cloned(); } diff --git a/tests/ui/min_rust_version_attr.stderr b/tests/ui/min_rust_version_attr.stderr index a2e4e86ed..360dcfb23 100644 --- a/tests/ui/min_rust_version_attr.stderr +++ b/tests/ui/min_rust_version_attr.stderr @@ -1,12 +1,12 @@ error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:160:24 + --> $DIR/min_rust_version_attr.rs:165:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | = note: `-D clippy::manual-strip` implied by `-D warnings` note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:159:9 + --> $DIR/min_rust_version_attr.rs:164:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -17,13 +17,13 @@ LL ~ assert_eq!(.to_uppercase(), "WORLD!"); | error: stripping a prefix manually - --> $DIR/min_rust_version_attr.rs:172:24 + --> $DIR/min_rust_version_attr.rs:177:24 | LL | assert_eq!(s["hello, ".len()..].to_uppercase(), "WORLD!"); | ^^^^^^^^^^^^^^^^^^^^ | note: the prefix was tested here - --> $DIR/min_rust_version_attr.rs:171:9 + --> $DIR/min_rust_version_attr.rs:176:9 | LL | if s.starts_with("hello, ") { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^