From 4336396e262e84545b1f4fb484bfdd783b338ade Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Mon, 17 Jun 2019 07:02:04 +0200 Subject: [PATCH 1/5] Add lint for use of ^ operator as exponentiation. --- CHANGELOG.md | 1 + clippy_lints/src/lib.rs | 5 ++ clippy_lints/src/xor_used_as_pow.rs | 77 +++++++++++++++++++++++++++++ src/lintlist/mod.rs | 7 +++ tests/ui/xor_used_as_pow.rs | 15 ++++++ tests/ui/xor_used_as_pow.stderr | 24 +++++++++ 6 files changed, 129 insertions(+) create mode 100644 clippy_lints/src/xor_used_as_pow.rs create mode 100644 tests/ui/xor_used_as_pow.rs create mode 100644 tests/ui/xor_used_as_pow.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fd79deb56c6..76d13e2ea69d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2047,6 +2047,7 @@ Released 2018-09-13 [`wrong_pub_self_convention`]: https://rust-lang.github.io/rust-clippy/master/index.html#wrong_pub_self_convention [`wrong_self_convention`]: https://rust-lang.github.io/rust-clippy/master/index.html#wrong_self_convention [`wrong_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#wrong_transmute +[`xor_used_as_pow`]: https://rust-lang.github.io/rust-clippy/master/index.html#xor_used_as_pow [`zero_divided_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_divided_by_zero [`zero_prefixed_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_prefixed_literal [`zero_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#zero_ptr diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index b330b66776c1..feac024c9395 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -335,6 +335,7 @@ mod verbose_file_reads; mod wildcard_dependencies; mod wildcard_imports; mod write; +mod xor_used_as_pow; mod zero_div_zero; // end lints modules, do not remove this comment, it’s used in `update_lints` @@ -909,6 +910,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &write::WRITELN_EMPTY_STRING, &write::WRITE_LITERAL, &write::WRITE_WITH_NEWLINE, + &xor_used_as_pow::XOR_USED_AS_POW, &zero_div_zero::ZERO_DIVIDED_BY_ZERO, ]); // end register lints, do not remove this comment, it’s used in `update_lints` @@ -1148,6 +1150,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box asm_syntax::InlineAsmX86AttSyntax); store.register_early_pass(|| box asm_syntax::InlineAsmX86IntelSyntax); store.register_late_pass(|| box undropped_manually_drops::UndroppedManuallyDrops); + store.register_early_pass(|| box xor_used_as_pow::XorUsedAsPow); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ @@ -1557,6 +1560,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&write::WRITELN_EMPTY_STRING), LintId::of(&write::WRITE_LITERAL), LintId::of(&write::WRITE_WITH_NEWLINE), + LintId::of(&xor_used_as_pow::XOR_USED_AS_POW), LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO), ]); @@ -1820,6 +1824,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT), LintId::of(&unwrap::PANICKING_UNWRAP), LintId::of(&vec_resize_to_zero::VEC_RESIZE_TO_ZERO), + LintId::of(&xor_used_as_pow::XOR_USED_AS_POW), ]); store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![ diff --git a/clippy_lints/src/xor_used_as_pow.rs b/clippy_lints/src/xor_used_as_pow.rs new file mode 100644 index 000000000000..a685d82426b1 --- /dev/null +++ b/clippy_lints/src/xor_used_as_pow.rs @@ -0,0 +1,77 @@ +use crate::utils::{span_help_and_lint, span_lint_and_sugg}; +use if_chain::if_chain; +use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintPass}; +use rustc::{declare_lint_pass, declare_tool_lint}; +use rustc_errors::Applicability; +use syntax::ast::{BinOpKind, Expr, ExprKind, LitKind}; + +declare_clippy_lint! { + /// **What it does:** Checks for use of `^` operator when exponentiation was intended. + /// + /// **Why is this bad?** This is most probably a typo. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust,ignore + /// // Bad + /// 2 ^ 16; + /// + /// // Good + /// 1 << 16; + /// 2i32.pow(16); + /// ``` + pub XOR_USED_AS_POW, + correctness, + "use of `^` operator when exponentiation was intended" +} + +declare_lint_pass!(XorUsedAsPow => [XOR_USED_AS_POW]); + +impl EarlyLintPass for XorUsedAsPow { + fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { + if_chain! { + if !in_external_macro(cx.sess, expr.span); + if let ExprKind::Binary(op, left, right) = &expr.node; + if BinOpKind::BitXor == op.node; + if let ExprKind::Lit(lit) = &left.node; + if let LitKind::Int(lhs, _) = lit.node; + if let ExprKind::Lit(lit) = &right.node; + if let LitKind::Int(rhs, _) = lit.node; + then { + if lhs == 2 { + if rhs == 8 || rhs == 16 || rhs == 32 || rhs == 64 || rhs == 128 { + span_lint_and_sugg( + cx, + XOR_USED_AS_POW, + expr.span, + "it appears you are trying to get the maximum value of an integer, but `^` is not an exponentiation operator", + "try", + format!("std::u{}::MAX", rhs), + Applicability::MaybeIncorrect, + ) + } else { + span_lint_and_sugg( + cx, + XOR_USED_AS_POW, + expr.span, + "it appears you are trying to get a power of two, but `^` is not an exponentiation operator", + "use a bitshift instead", + format!("1 << {}", rhs), + Applicability::MaybeIncorrect, + ) + } + } else { + span_help_and_lint( + cx, + XOR_USED_AS_POW, + expr.span, + "`^` is not an exponentiation operator but appears to have been used as one", + "did you mean to use .pow()?" + ) + } + } + } + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 4bf77dae6377..0de2953aa7c1 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2853,6 +2853,13 @@ vec![ deprecation: None, module: "transmute", }, + Lint { + name: "xor_used_as_pow", + group: "correctness", + desc: "use of `^` operator when exponentiation was intended", + deprecation: None, + module: "xor_used_as_pow", + }, Lint { name: "zero_divided_by_zero", group: "complexity", diff --git a/tests/ui/xor_used_as_pow.rs b/tests/ui/xor_used_as_pow.rs new file mode 100644 index 000000000000..4e0305d32e47 --- /dev/null +++ b/tests/ui/xor_used_as_pow.rs @@ -0,0 +1,15 @@ +#![warn(clippy::xor_used_as_pow)] + +fn main() { + // These should succeed + // With variables, it's not as clear whether the intention was exponentiation or not + let x = 15; + println!("{}", 2 ^ x); + let y = 2; + println!("{}", y ^ 16); + + // These should fail + println!("{}", 2 ^ 16); + println!("{}", 2 ^ 7); + println!("{}", 9 ^ 3); +} diff --git a/tests/ui/xor_used_as_pow.stderr b/tests/ui/xor_used_as_pow.stderr new file mode 100644 index 000000000000..dc3d6351908f --- /dev/null +++ b/tests/ui/xor_used_as_pow.stderr @@ -0,0 +1,24 @@ +error: it appears you are trying to get the maximum value of an integer, but `^` is not an exponentiation operator + --> $DIR/xor_used_as_pow.rs:12:20 + | +LL | println!("{}", 2 ^ 16); + | ^^^^^^ help: try: `std::u16::MAX` + | + = note: `-D clippy::xor-used-as-pow` implied by `-D warnings` + +error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator + --> $DIR/xor_used_as_pow.rs:13:20 + | +LL | println!("{}", 2 ^ 7); + | ^^^^^ help: use a bitshift instead: `1 << 7` + +error: `^` is not an exponentiation operator but appears to have been used as one + --> $DIR/xor_used_as_pow.rs:14:20 + | +LL | println!("{}", 9 ^ 3); + | ^^^^^ + | + = help: did you mean to use .pow()? + +error: aborting due to 3 previous errors + From 773dc7624837ba57636421a3e8f90e6042c6975f Mon Sep 17 00:00:00 2001 From: cgm616 Date: Fri, 16 Oct 2020 12:03:05 -0400 Subject: [PATCH 2/5] Update lint to current clippy API --- clippy_lints/src/xor_used_as_pow.rs | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/xor_used_as_pow.rs b/clippy_lints/src/xor_used_as_pow.rs index a685d82426b1..66631c5c2a30 100644 --- a/clippy_lints/src/xor_used_as_pow.rs +++ b/clippy_lints/src/xor_used_as_pow.rs @@ -1,9 +1,10 @@ -use crate::utils::{span_help_and_lint, span_lint_and_sugg}; +use crate::utils::{span_lint_and_help, span_lint_and_sugg}; use if_chain::if_chain; -use rustc::lint::{in_external_macro, EarlyContext, EarlyLintPass, LintArray, LintPass}; -use rustc::{declare_lint_pass, declare_tool_lint}; +use rustc_ast::{BinOpKind, Expr, ExprKind, LitKind}; use rustc_errors::Applicability; -use syntax::ast::{BinOpKind, Expr, ExprKind, LitKind}; +use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_middle::lint::in_external_macro; +use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// **What it does:** Checks for use of `^` operator when exponentiation was intended. @@ -33,12 +34,12 @@ impl EarlyLintPass for XorUsedAsPow { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { if_chain! { if !in_external_macro(cx.sess, expr.span); - if let ExprKind::Binary(op, left, right) = &expr.node; + if let ExprKind::Binary(op, left, right) = &expr.kind; if BinOpKind::BitXor == op.node; - if let ExprKind::Lit(lit) = &left.node; - if let LitKind::Int(lhs, _) = lit.node; - if let ExprKind::Lit(lit) = &right.node; - if let LitKind::Int(rhs, _) = lit.node; + if let ExprKind::Lit(lit) = &left.kind; + if let LitKind::Int(lhs, _) = lit.kind; + if let ExprKind::Lit(lit) = &right.kind; + if let LitKind::Int(rhs, _) = lit.kind; then { if lhs == 2 { if rhs == 8 || rhs == 16 || rhs == 32 || rhs == 64 || rhs == 128 { @@ -63,11 +64,12 @@ impl EarlyLintPass for XorUsedAsPow { ) } } else { - span_help_and_lint( + span_lint_and_help( cx, XOR_USED_AS_POW, expr.span, "`^` is not an exponentiation operator but appears to have been used as one", + None, "did you mean to use .pow()?" ) } From 282aafebf96947b9970cb4e4581fc8c913d2fa0e Mon Sep 17 00:00:00 2001 From: Cole Graber-Mitchell Date: Sat, 17 Oct 2020 23:09:31 -0400 Subject: [PATCH 3/5] Apply suggestions from code review Minor docs and description changes Co-authored-by: Eduardo Broto --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/xor_used_as_pow.rs | 194 +++++++++++++++++++++------- src/lintlist/mod.rs | 2 +- tests/ui/xor_used_as_pow.fixed | 33 +++++ tests/ui/xor_used_as_pow.rs | 36 ++++-- tests/ui/xor_used_as_pow.stderr | 64 +++++++-- 6 files changed, 262 insertions(+), 69 deletions(-) create mode 100644 tests/ui/xor_used_as_pow.fixed diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index feac024c9395..364839f1d89e 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1150,7 +1150,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| box asm_syntax::InlineAsmX86AttSyntax); store.register_early_pass(|| box asm_syntax::InlineAsmX86IntelSyntax); store.register_late_pass(|| box undropped_manually_drops::UndroppedManuallyDrops); - store.register_early_pass(|| box xor_used_as_pow::XorUsedAsPow); + store.register_late_pass(|| box xor_used_as_pow::XorUsedAsPow); store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ diff --git a/clippy_lints/src/xor_used_as_pow.rs b/clippy_lints/src/xor_used_as_pow.rs index 66631c5c2a30..b2e5837863ba 100644 --- a/clippy_lints/src/xor_used_as_pow.rs +++ b/clippy_lints/src/xor_used_as_pow.rs @@ -1,79 +1,179 @@ -use crate::utils::{span_lint_and_help, span_lint_and_sugg}; +use crate::utils::{ + last_path_segment, numeric_literal::NumericLiteral, qpath_res, snippet_opt, span_lint_and_help, span_lint_and_sugg, +}; use if_chain::if_chain; -use rustc_ast::{BinOpKind, Expr, ExprKind, LitKind}; +use rustc_ast::{LitIntType, LitKind}; use rustc_errors::Applicability; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_hir::{ + def::{DefKind, Res}, + BinOpKind, BindingAnnotation, Expr, ExprKind, ItemKind, Lit, Node, PatKind, QPath, +}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::Span; declare_clippy_lint! { - /// **What it does:** Checks for use of `^` operator when exponentiation was intended. + /// **What it does:** Checks for use of `^` operator when exponentiation was probably intended. + /// A caret is commonly an ASCII-compatible/keyboard-accessible way to write down exponentiation in docs, + /// readmes, and comments, and copying and pasting a formula can inadvertedly introduce this error. + /// Moreover, `^` means exponentiation in other programming languages. /// - /// **Why is this bad?** This is most probably a typo. + /// **Why is this bad?** This is most probably a mistake. /// /// **Known problems:** None. /// /// **Example:** /// - /// ```rust,ignore + /// ```rust /// // Bad - /// 2 ^ 16; + /// let a = 2 ^ 16; + /// let b = 10 ^ 4; /// /// // Good - /// 1 << 16; - /// 2i32.pow(16); + /// let a = 1 << 16; + /// let b = 10i32.pow(4); /// ``` pub XOR_USED_AS_POW, correctness, - "use of `^` operator when exponentiation was intended" + "use of `^` operator when exponentiation was probably intended" } declare_lint_pass!(XorUsedAsPow => [XOR_USED_AS_POW]); -impl EarlyLintPass for XorUsedAsPow { - fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { +impl LateLintPass<'_> for XorUsedAsPow { + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) { + let parent_id = cx.tcx.hir().get_parent_item(expr.hir_id); + if let Some(Node::Item(parent_item)) = cx.tcx.hir().find(parent_id) { + if let ItemKind::Enum(_, _) = parent_item.kind { + return; + } + } + if_chain! { - if !in_external_macro(cx.sess, expr.span); + if !in_external_macro(cx.sess(), expr.span); if let ExprKind::Binary(op, left, right) = &expr.kind; if BinOpKind::BitXor == op.node; - if let ExprKind::Lit(lit) = &left.kind; - if let LitKind::Int(lhs, _) = lit.kind; - if let ExprKind::Lit(lit) = &right.kind; - if let LitKind::Int(rhs, _) = lit.kind; + if let ExprKind::Lit(lhs) = &left.kind; + if let Some((lhs_val, lhs_type)) = unwrap_dec_int_literal(cx, lhs); then { - if lhs == 2 { - if rhs == 8 || rhs == 16 || rhs == 32 || rhs == 64 || rhs == 128 { - span_lint_and_sugg( - cx, - XOR_USED_AS_POW, - expr.span, - "it appears you are trying to get the maximum value of an integer, but `^` is not an exponentiation operator", - "try", - format!("std::u{}::MAX", rhs), - Applicability::MaybeIncorrect, - ) - } else { - span_lint_and_sugg( - cx, - XOR_USED_AS_POW, - expr.span, - "it appears you are trying to get a power of two, but `^` is not an exponentiation operator", - "use a bitshift instead", - format!("1 << {}", rhs), - Applicability::MaybeIncorrect, - ) + match &right.kind { + ExprKind::Lit(rhs) => { + if let Some((rhs_val, _)) = unwrap_dec_int_literal(cx, rhs) { + report_with_lit(cx, lhs_val, rhs_val, expr.span); + } } - } else { - span_lint_and_help( - cx, - XOR_USED_AS_POW, - expr.span, - "`^` is not an exponentiation operator but appears to have been used as one", - None, - "did you mean to use .pow()?" - ) + ExprKind::Path(qpath) => { + match qpath_res(cx, qpath, right.hir_id) { + Res::Local(hir_id) => { + if_chain! { + let node = cx.tcx.hir().get(hir_id); + if let Node::Binding(pat) = node; + if let PatKind::Binding(bind_ann, ..) = pat.kind; + if !matches!(bind_ann, BindingAnnotation::RefMut | + BindingAnnotation::Mutable); + let parent_node = cx.tcx.hir().get_parent_node(hir_id); + if let Some(Node::Local(parent_let_expr)) = cx.tcx.hir().find(parent_node); + if let Some(init) = parent_let_expr.init; + then { + match init.kind { + // immutable bindings that are initialized with literal + ExprKind::Lit(..) => report_with_ident(cx, lhs_val, qpath, expr.span), + // immutable bindings that are initialized with constant + ExprKind::Path(ref path) => { + let res = qpath_res(cx, path, init.hir_id); + if let Res::Def(DefKind::Const, ..) = res { + report_with_ident(cx, lhs_val, qpath, expr.span); + } + } + _ => {}, + } + } + } + }, + // constant + Res::Def(DefKind::Const, ..) => report_with_ident(cx, lhs_val, qpath, expr.span), + _ => {}, + } + } + _ => {} } } } } } + +fn unwrap_dec_int_literal(cx: &LateContext<'_>, lit: &Lit) -> Option<(u128, LitIntType)> { + if_chain! { + if let LitKind::Int(val, val_type) = lit.node; + if let Some(snippet) = snippet_opt(cx, lit.span); + if let Some(decoded) = NumericLiteral::from_lit_kind(&snippet, &lit.node); + if decoded.is_decimal(); + then { + return Some((val, val_type)); + } + else { + return None; + } + } +} + +fn report_with_ident(cx: &LateContext<'_>, lhs: u128, rhs: &QPath<'_>, span: Span) { + match lhs { + 2 => { + let ident = last_path_segment(rhs).ident.name.to_ident_string(); + report_pow_of_two(cx, format!("1 << {}", ident), span); + }, + 10 => report_pow_of_ten(cx, span), + _ => {}, + } +} + +fn report_with_lit(cx: &LateContext<'_>, lhs: u128, rhs: u128, span: Span) { + if rhs > 127 { + return; + } + match lhs { + 2 => { + if rhs == 0 { + report_pow_of_two(cx, format!("1"), span); + return; + } + + let lhs_str = if rhs <= 31 { + "1_u32" + } else if rhs <= 63 { + "1_u64" + } else { + "1_u127" + }; + + report_pow_of_two(cx, format!("{} << {}", lhs_str, rhs), span); + }, + 10 => report_pow_of_ten(cx, span), + _ => {}, + } +} + +fn report_pow_of_two(cx: &LateContext<'_>, sugg: String, span: Span) { + span_lint_and_sugg( + cx, + XOR_USED_AS_POW, + span, + "it appears you are trying to get a power of two, but `^` is not an exponentiation operator", + "use a bitshift or constant instead", + sugg, + Applicability::MaybeIncorrect, + ) +} + +fn report_pow_of_ten(cx: &LateContext<'_>, span: Span) { + span_lint_and_help( + cx, + XOR_USED_AS_POW, + span, + "`^` is not an exponentiation operator but appears to have been used as one", + None, + "did you mean to use .pow()?", + ) +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 0de2953aa7c1..44af5f431d62 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -2856,7 +2856,7 @@ vec![ Lint { name: "xor_used_as_pow", group: "correctness", - desc: "use of `^` operator when exponentiation was intended", + desc: "use of `^` operator when exponentiation was probably intended", deprecation: None, module: "xor_used_as_pow", }, diff --git a/tests/ui/xor_used_as_pow.fixed b/tests/ui/xor_used_as_pow.fixed new file mode 100644 index 000000000000..a252f5505ea7 --- /dev/null +++ b/tests/ui/xor_used_as_pow.fixed @@ -0,0 +1,33 @@ +// run-rustfix +#![warn(clippy::xor_used_as_pow)] + +// Should not be linted +#[allow(dead_code)] +enum E { + First = 1 ^ 8, + Second = 2 ^ 8, + Third = 3 ^ 8, + Tenth = 10 ^ 8, +} + +fn main() { + // These should succeed: + let _ = 9 ^ 3; // lhs other than 2 or 10 + let _ = 0x02 ^ 6; // lhs not decimal + let _ = 2 ^ 0x10; // rhs hexadecimal + let _ = 10 ^ 0b0101; // rhs binary + let _ = 2 ^ 0o1; // rhs octal + let _ = 10 ^ -18; // negative rhs + + // These should fail + let _ = 1_u32 << 3; + let _ = 10 ^ 4; + let _ = 1_u64 << 32; + let _ = 1; + let _ = 10 ^ 0; + { + let x = 15; + let _ = 1 << x; + let _ = 10 ^ x; + } +} diff --git a/tests/ui/xor_used_as_pow.rs b/tests/ui/xor_used_as_pow.rs index 4e0305d32e47..766396a82dda 100644 --- a/tests/ui/xor_used_as_pow.rs +++ b/tests/ui/xor_used_as_pow.rs @@ -1,15 +1,33 @@ +// run-rustfix #![warn(clippy::xor_used_as_pow)] +// Should not be linted +#[allow(dead_code)] +enum E { + First = 1 ^ 8, + Second = 2 ^ 8, + Third = 3 ^ 8, + Tenth = 10 ^ 8, +} + fn main() { - // These should succeed - // With variables, it's not as clear whether the intention was exponentiation or not - let x = 15; - println!("{}", 2 ^ x); - let y = 2; - println!("{}", y ^ 16); + // These should succeed: + let _ = 9 ^ 3; // lhs other than 2 or 10 + let _ = 0x02 ^ 6; // lhs not decimal + let _ = 2 ^ 0x10; // rhs hexadecimal + let _ = 10 ^ 0b0101; // rhs binary + let _ = 2 ^ 0o1; // rhs octal + let _ = 10 ^ -18; // negative rhs // These should fail - println!("{}", 2 ^ 16); - println!("{}", 2 ^ 7); - println!("{}", 9 ^ 3); + let _ = 2 ^ 3; + let _ = 10 ^ 4; + let _ = 2 ^ 32; + let _ = 2 ^ 0; + let _ = 10 ^ 0; + { + let x = 15; + let _ = 2 ^ x; + let _ = 10 ^ x; + } } diff --git a/tests/ui/xor_used_as_pow.stderr b/tests/ui/xor_used_as_pow.stderr index dc3d6351908f..826a179af92f 100644 --- a/tests/ui/xor_used_as_pow.stderr +++ b/tests/ui/xor_used_as_pow.stderr @@ -1,24 +1,66 @@ -error: it appears you are trying to get the maximum value of an integer, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:12:20 +error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator + --> $DIR/xor_used_as_pow.rs:23:13 | -LL | println!("{}", 2 ^ 16); - | ^^^^^^ help: try: `std::u16::MAX` +LL | let _ = 2 ^ 3; + | ^^^^^ help: use a bitshift or constant instead: `1_u32 << 3` | = note: `-D clippy::xor-used-as-pow` implied by `-D warnings` +error: `^` is not an exponentiation operator but appears to have been used as one + --> $DIR/xor_used_as_pow.rs:24:13 + | +LL | let _ = 10 ^ 4; + | ^^^^^^ + | + = help: did you mean to use .pow()? + +error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator + --> $DIR/xor_used_as_pow.rs:25:13 + | +LL | let _ = 2 ^ 32; + | ^^^^^^ help: use a bitshift or constant instead: `1_u64 << 32` + +error: the operation is ineffective. Consider reducing it to `2` + --> $DIR/xor_used_as_pow.rs:26:13 + | +LL | let _ = 2 ^ 0; + | ^^^^^ + | + = note: `-D clippy::identity-op` implied by `-D warnings` + +error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator + --> $DIR/xor_used_as_pow.rs:26:13 + | +LL | let _ = 2 ^ 0; + | ^^^^^ help: use a bitshift or constant instead: `1` + +error: the operation is ineffective. Consider reducing it to `10` + --> $DIR/xor_used_as_pow.rs:27:13 + | +LL | let _ = 10 ^ 0; + | ^^^^^^ + +error: `^` is not an exponentiation operator but appears to have been used as one + --> $DIR/xor_used_as_pow.rs:27:13 + | +LL | let _ = 10 ^ 0; + | ^^^^^^ + | + = help: did you mean to use .pow()? + error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:13:20 + --> $DIR/xor_used_as_pow.rs:30:17 | -LL | println!("{}", 2 ^ 7); - | ^^^^^ help: use a bitshift instead: `1 << 7` +LL | let _ = 2 ^ x; + | ^^^^^ help: use a bitshift or constant instead: `1 << x` error: `^` is not an exponentiation operator but appears to have been used as one - --> $DIR/xor_used_as_pow.rs:14:20 + --> $DIR/xor_used_as_pow.rs:31:17 | -LL | println!("{}", 9 ^ 3); - | ^^^^^ +LL | let _ = 10 ^ x; + | ^^^^^^ | = help: did you mean to use .pow()? -error: aborting due to 3 previous errors +error: aborting due to 9 previous errors From 12f150ea03c7668a3b5875d46168bbbee1680bb1 Mon Sep 17 00:00:00 2001 From: cgm616 Date: Thu, 22 Oct 2020 15:00:51 -0400 Subject: [PATCH 4/5] Remove unrelated lints from tests --- tests/ui/xor_used_as_pow.fixed | 1 + tests/ui/xor_used_as_pow.rs | 1 + tests/ui/xor_used_as_pow.stderr | 30 ++++++++---------------------- 3 files changed, 10 insertions(+), 22 deletions(-) diff --git a/tests/ui/xor_used_as_pow.fixed b/tests/ui/xor_used_as_pow.fixed index a252f5505ea7..eac4d1ba1353 100644 --- a/tests/ui/xor_used_as_pow.fixed +++ b/tests/ui/xor_used_as_pow.fixed @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::xor_used_as_pow)] +#![allow(clippy::identity_op)] // Should not be linted #[allow(dead_code)] diff --git a/tests/ui/xor_used_as_pow.rs b/tests/ui/xor_used_as_pow.rs index 766396a82dda..2ba4d8cb3595 100644 --- a/tests/ui/xor_used_as_pow.rs +++ b/tests/ui/xor_used_as_pow.rs @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::xor_used_as_pow)] +#![allow(clippy::identity_op)] // Should not be linted #[allow(dead_code)] diff --git a/tests/ui/xor_used_as_pow.stderr b/tests/ui/xor_used_as_pow.stderr index 826a179af92f..4441fdef94e9 100644 --- a/tests/ui/xor_used_as_pow.stderr +++ b/tests/ui/xor_used_as_pow.stderr @@ -1,5 +1,5 @@ error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:23:13 + --> $DIR/xor_used_as_pow.rs:24:13 | LL | let _ = 2 ^ 3; | ^^^^^ help: use a bitshift or constant instead: `1_u32 << 3` @@ -7,7 +7,7 @@ LL | let _ = 2 ^ 3; = note: `-D clippy::xor-used-as-pow` implied by `-D warnings` error: `^` is not an exponentiation operator but appears to have been used as one - --> $DIR/xor_used_as_pow.rs:24:13 + --> $DIR/xor_used_as_pow.rs:25:13 | LL | let _ = 10 ^ 4; | ^^^^^^ @@ -15,33 +15,19 @@ LL | let _ = 10 ^ 4; = help: did you mean to use .pow()? error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:25:13 + --> $DIR/xor_used_as_pow.rs:26:13 | LL | let _ = 2 ^ 32; | ^^^^^^ help: use a bitshift or constant instead: `1_u64 << 32` -error: the operation is ineffective. Consider reducing it to `2` - --> $DIR/xor_used_as_pow.rs:26:13 - | -LL | let _ = 2 ^ 0; - | ^^^^^ - | - = note: `-D clippy::identity-op` implied by `-D warnings` - error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:26:13 + --> $DIR/xor_used_as_pow.rs:27:13 | LL | let _ = 2 ^ 0; | ^^^^^ help: use a bitshift or constant instead: `1` -error: the operation is ineffective. Consider reducing it to `10` - --> $DIR/xor_used_as_pow.rs:27:13 - | -LL | let _ = 10 ^ 0; - | ^^^^^^ - error: `^` is not an exponentiation operator but appears to have been used as one - --> $DIR/xor_used_as_pow.rs:27:13 + --> $DIR/xor_used_as_pow.rs:28:13 | LL | let _ = 10 ^ 0; | ^^^^^^ @@ -49,18 +35,18 @@ LL | let _ = 10 ^ 0; = help: did you mean to use .pow()? error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:30:17 + --> $DIR/xor_used_as_pow.rs:31:17 | LL | let _ = 2 ^ x; | ^^^^^ help: use a bitshift or constant instead: `1 << x` error: `^` is not an exponentiation operator but appears to have been used as one - --> $DIR/xor_used_as_pow.rs:31:17 + --> $DIR/xor_used_as_pow.rs:32:17 | LL | let _ = 10 ^ x; | ^^^^^^ | = help: did you mean to use .pow()? -error: aborting due to 9 previous errors +error: aborting due to 7 previous errors From ebb71f26d4a222734e1523c6787695d2b651bb3c Mon Sep 17 00:00:00 2001 From: cgm616 Date: Thu, 22 Oct 2020 18:15:29 -0400 Subject: [PATCH 5/5] Prevent linting on zero values --- clippy_lints/src/xor_used_as_pow.rs | 13 ++---- tests/ui/xor_used_as_pow.rs | 7 +--- tests/ui/xor_used_as_pow.stderr | 41 ++----------------- ...ow.fixed => xor_used_as_pow_fixable.fixed} | 5 +-- tests/ui/xor_used_as_pow_fixable.rs | 31 ++++++++++++++ tests/ui/xor_used_as_pow_fixable.stderr | 22 ++++++++++ 6 files changed, 63 insertions(+), 56 deletions(-) rename tests/ui/{xor_used_as_pow.fixed => xor_used_as_pow_fixable.fixed} (88%) create mode 100644 tests/ui/xor_used_as_pow_fixable.rs create mode 100644 tests/ui/xor_used_as_pow_fixable.stderr diff --git a/clippy_lints/src/xor_used_as_pow.rs b/clippy_lints/src/xor_used_as_pow.rs index b2e5837863ba..81fbe01240fd 100644 --- a/clippy_lints/src/xor_used_as_pow.rs +++ b/clippy_lints/src/xor_used_as_pow.rs @@ -55,7 +55,7 @@ impl LateLintPass<'_> for XorUsedAsPow { if let ExprKind::Binary(op, left, right) = &expr.kind; if BinOpKind::BitXor == op.node; if let ExprKind::Lit(lhs) = &left.kind; - if let Some((lhs_val, lhs_type)) = unwrap_dec_int_literal(cx, lhs); + if let Some((lhs_val, _)) = unwrap_dec_int_literal(cx, lhs); then { match &right.kind { ExprKind::Lit(rhs) => { @@ -110,10 +110,10 @@ fn unwrap_dec_int_literal(cx: &LateContext<'_>, lit: &Lit) -> Option<(u128, LitI if let Some(decoded) = NumericLiteral::from_lit_kind(&snippet, &lit.node); if decoded.is_decimal(); then { - return Some((val, val_type)); + Some((val, val_type)) } else { - return None; + None } } } @@ -130,16 +130,11 @@ fn report_with_ident(cx: &LateContext<'_>, lhs: u128, rhs: &QPath<'_>, span: Spa } fn report_with_lit(cx: &LateContext<'_>, lhs: u128, rhs: u128, span: Span) { - if rhs > 127 { + if rhs > 127 || rhs == 0 { return; } match lhs { 2 => { - if rhs == 0 { - report_pow_of_two(cx, format!("1"), span); - return; - } - let lhs_str = if rhs <= 31 { "1_u32" } else if rhs <= 63 { diff --git a/tests/ui/xor_used_as_pow.rs b/tests/ui/xor_used_as_pow.rs index 2ba4d8cb3595..80d90f310803 100644 --- a/tests/ui/xor_used_as_pow.rs +++ b/tests/ui/xor_used_as_pow.rs @@ -1,4 +1,3 @@ -// run-rustfix #![warn(clippy::xor_used_as_pow)] #![allow(clippy::identity_op)] @@ -19,16 +18,12 @@ fn main() { let _ = 10 ^ 0b0101; // rhs binary let _ = 2 ^ 0o1; // rhs octal let _ = 10 ^ -18; // negative rhs + let _ = 2 ^ 0; // zero rhs // These should fail - let _ = 2 ^ 3; let _ = 10 ^ 4; - let _ = 2 ^ 32; - let _ = 2 ^ 0; - let _ = 10 ^ 0; { let x = 15; - let _ = 2 ^ x; let _ = 10 ^ x; } } diff --git a/tests/ui/xor_used_as_pow.stderr b/tests/ui/xor_used_as_pow.stderr index 4441fdef94e9..77e7bc1221ac 100644 --- a/tests/ui/xor_used_as_pow.stderr +++ b/tests/ui/xor_used_as_pow.stderr @@ -1,52 +1,19 @@ -error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:24:13 - | -LL | let _ = 2 ^ 3; - | ^^^^^ help: use a bitshift or constant instead: `1_u32 << 3` - | - = note: `-D clippy::xor-used-as-pow` implied by `-D warnings` - error: `^` is not an exponentiation operator but appears to have been used as one - --> $DIR/xor_used_as_pow.rs:25:13 + --> $DIR/xor_used_as_pow.rs:24:13 | LL | let _ = 10 ^ 4; | ^^^^^^ | + = note: `-D clippy::xor-used-as-pow` implied by `-D warnings` = help: did you mean to use .pow()? -error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:26:13 - | -LL | let _ = 2 ^ 32; - | ^^^^^^ help: use a bitshift or constant instead: `1_u64 << 32` - -error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:27:13 - | -LL | let _ = 2 ^ 0; - | ^^^^^ help: use a bitshift or constant instead: `1` - -error: `^` is not an exponentiation operator but appears to have been used as one - --> $DIR/xor_used_as_pow.rs:28:13 - | -LL | let _ = 10 ^ 0; - | ^^^^^^ - | - = help: did you mean to use .pow()? - -error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator - --> $DIR/xor_used_as_pow.rs:31:17 - | -LL | let _ = 2 ^ x; - | ^^^^^ help: use a bitshift or constant instead: `1 << x` - error: `^` is not an exponentiation operator but appears to have been used as one - --> $DIR/xor_used_as_pow.rs:32:17 + --> $DIR/xor_used_as_pow.rs:27:17 | LL | let _ = 10 ^ x; | ^^^^^^ | = help: did you mean to use .pow()? -error: aborting due to 7 previous errors +error: aborting due to 2 previous errors diff --git a/tests/ui/xor_used_as_pow.fixed b/tests/ui/xor_used_as_pow_fixable.fixed similarity index 88% rename from tests/ui/xor_used_as_pow.fixed rename to tests/ui/xor_used_as_pow_fixable.fixed index eac4d1ba1353..26e32126a8a9 100644 --- a/tests/ui/xor_used_as_pow.fixed +++ b/tests/ui/xor_used_as_pow_fixable.fixed @@ -19,16 +19,13 @@ fn main() { let _ = 10 ^ 0b0101; // rhs binary let _ = 2 ^ 0o1; // rhs octal let _ = 10 ^ -18; // negative rhs + let _ = 2 ^ 0; // zero rhs // These should fail let _ = 1_u32 << 3; - let _ = 10 ^ 4; let _ = 1_u64 << 32; - let _ = 1; - let _ = 10 ^ 0; { let x = 15; let _ = 1 << x; - let _ = 10 ^ x; } } diff --git a/tests/ui/xor_used_as_pow_fixable.rs b/tests/ui/xor_used_as_pow_fixable.rs new file mode 100644 index 000000000000..9a020f8601b1 --- /dev/null +++ b/tests/ui/xor_used_as_pow_fixable.rs @@ -0,0 +1,31 @@ +// run-rustfix +#![warn(clippy::xor_used_as_pow)] +#![allow(clippy::identity_op)] + +// Should not be linted +#[allow(dead_code)] +enum E { + First = 1 ^ 8, + Second = 2 ^ 8, + Third = 3 ^ 8, + Tenth = 10 ^ 8, +} + +fn main() { + // These should succeed: + let _ = 9 ^ 3; // lhs other than 2 or 10 + let _ = 0x02 ^ 6; // lhs not decimal + let _ = 2 ^ 0x10; // rhs hexadecimal + let _ = 10 ^ 0b0101; // rhs binary + let _ = 2 ^ 0o1; // rhs octal + let _ = 10 ^ -18; // negative rhs + let _ = 2 ^ 0; // zero rhs + + // These should fail + let _ = 2 ^ 3; + let _ = 2 ^ 32; + { + let x = 15; + let _ = 2 ^ x; + } +} diff --git a/tests/ui/xor_used_as_pow_fixable.stderr b/tests/ui/xor_used_as_pow_fixable.stderr new file mode 100644 index 000000000000..64a612d7a72c --- /dev/null +++ b/tests/ui/xor_used_as_pow_fixable.stderr @@ -0,0 +1,22 @@ +error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator + --> $DIR/xor_used_as_pow_fixable.rs:25:13 + | +LL | let _ = 2 ^ 3; + | ^^^^^ help: use a bitshift or constant instead: `1_u32 << 3` + | + = note: `-D clippy::xor-used-as-pow` implied by `-D warnings` + +error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator + --> $DIR/xor_used_as_pow_fixable.rs:26:13 + | +LL | let _ = 2 ^ 32; + | ^^^^^^ help: use a bitshift or constant instead: `1_u64 << 32` + +error: it appears you are trying to get a power of two, but `^` is not an exponentiation operator + --> $DIR/xor_used_as_pow_fixable.rs:29:17 + | +LL | let _ = 2 ^ x; + | ^^^^^ help: use a bitshift or constant instead: `1 << x` + +error: aborting due to 3 previous errors +