New [suspicious_arguments] lint for possibly swapped arguments

CHANGELOG.md

@@ -3923,6 +3923,7 @@ Released 2018-09-13
 [`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools
 [`stutter`]: https://rust-lang.github.io/rust-clippy/master/index.html#stutter
 [`suboptimal_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#suboptimal_flops
+[`suspicious_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arguments
 [`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl
 [`suspicious_assignment_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_assignment_formatting
 [`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting

clippy_lints/src/checked_conversions.rs

@@ -215,6 +215,7 @@ fn check_lower_bound<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<Conversion<'tcx>> {
 
     // First of we need a binary containing the expression & the cast
     if let ExprKind::Binary(ref op, left, right) = &expr.kind {
+        #[expect(clippy::suspicious_arguments, reason = "these are intentionally reversed")]
         normalize_le_ge(op, right, left).and_then(|(l, r)| check_function(l, r))
     } else {
         None

clippy_lints/src/lib.register_all.rs

@@ -297,6 +297,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(strings::STRING_FROM_UTF8_AS_BYTES),
     LintId::of(strings::TRIM_SPLIT_WHITESPACE),
     LintId::of(strlen_on_c_strings::STRLEN_ON_C_STRINGS),
+    LintId::of(suspicious_arguments::SUSPICIOUS_ARGUMENTS),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
     LintId::of(swap::ALMOST_SWAPPED),

clippy_lints/src/lib.register_lints.rs

@@ -514,6 +514,7 @@ store.register_lints(&[
     strings::STR_TO_STRING,
     strings::TRIM_SPLIT_WHITESPACE,
     strlen_on_c_strings::STRLEN_ON_C_STRINGS,
+    suspicious_arguments::SUSPICIOUS_ARGUMENTS,
     suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS,
     suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,
     suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL,

clippy_lints/src/lib.register_suspicious.rs

@@ -29,6 +29,7 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
     LintId::of(operators::FLOAT_EQUALITY_WITHOUT_ABS),
     LintId::of(operators::MISREFACTORED_ASSIGN_OP),
     LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT),
+    LintId::of(suspicious_arguments::SUSPICIOUS_ARGUMENTS),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
     LintId::of(swap_ptr_to_ref::SWAP_PTR_TO_REF),

clippy_lints/src/lib.rs

@@ -370,6 +370,7 @@ mod stable_sort_primitive;
 mod std_instead_of_core;
 mod strings;
 mod strlen_on_c_strings;
+mod suspicious_arguments;
 mod suspicious_operation_groupings;
 mod suspicious_trait_impl;
 mod swap;
@@ -933,6 +934,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(std_instead_of_core::StdReexports::default()));
     store.register_late_pass(|| Box::new(manual_instant_elapsed::ManualInstantElapsed));
     store.register_late_pass(|| Box::new(partialeq_to_none::PartialeqToNone));
+    store.register_late_pass(|| Box::new(suspicious_arguments::SuspiciousArguments));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/suspicious_arguments.rs

@@ -0,0 +1,140 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::path_res;
+use rustc_data_structures::fx::FxHashMap;
+use rustc_hir::{Expr, ExprKind, QPath};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::Span;
+
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for calls to a function where the parameters look swapped
+    ///
+    /// ### Why is this bad?
+    ///
+    /// This likely indicates an error, where the arguments were reversed.
+    ///
+    /// ### Example
+    /// ```rust
+    /// fn resize(width: usize, height: usize) {}
+    ///
+    /// let height = 100;
+    /// let width = 200;
+    /// resize(height, width);
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// fn resize(width: usize, height: usize) {}
+    ///
+    /// let height = 100;
+    /// let width = 200;
+    /// resize(width, height);
+    /// ```
+    #[clippy::version = "1.64.0"]
+    pub SUSPICIOUS_ARGUMENTS,
+    suspicious,
+    "function call with probably swapped arguments"
+}
+declare_lint_pass!(SuspiciousArguments => [SUSPICIOUS_ARGUMENTS]);
+
+fn arguments_are_sus(cx: &LateContext<'_>, definition: &[(String, Span)], call: &[Option<(String, Span)>]) {
+    let idxs: FxHashMap<&String, usize> = definition
+        .iter()
+        .enumerate()
+        .map(|(idx, (item, _))| (item, idx))
+        .collect();
+
+    for (call_idx, arg_and_span) in call.iter().enumerate() {
+        if let Some((arg, call_span)) = arg_and_span {
+            if let Some(&def_idx) = idxs.get(arg) {
+                if call_idx != def_idx {
+                    if let Some((reverse_call, reverse_call_span)) = &call[def_idx] {
+                        let def_for_call = &definition[call_idx];
+                        if reverse_call == &def_for_call.0 {
+                            // This is technically being called twice, but it's being
+                            // deduplicated?
+                            span_lint_and_then(
+                                cx,
+                                SUSPICIOUS_ARGUMENTS,
+                                vec![*call_span, *reverse_call_span],
+                                "these arguments are possibly swapped",
+                                |diag| {
+                                    let second_span = definition[def_idx].1;
+                                    diag.span_note(vec![def_for_call.1, second_span], "the arguments are defined here");
+                                },
+                            );
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
+
+/// Try and guess an ident for `expr`. If they give something like `my_struct.height`
+/// or `height()`, use that.
+fn guess_ident(expr: &Expr<'_>) -> Option<(String, Span)> {
+    match &expr.kind {
+        ExprKind::Path(qp) => {
+            if let QPath::Resolved(_, p) = qp && let Some(segment) = p.segments.last() {
+                return Some((segment.ident.to_string(), segment.ident.span));
+            }
+        }
+        ExprKind::Field(_, ident) => {
+            return Some((ident.to_string(), ident.span));
+        }
+        ExprKind::Call(func, _) => {
+            return guess_ident(func);
+        }
+        _ => {},
+    }
+
+    None
+}
+
+impl<'tcx> LateLintPass<'tcx> for SuspiciousArguments {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if expr.span.from_expansion() {
+            return;
+        }
+
+        if let ExprKind::Call(f, args) = expr.kind
+            && let Some(def_id) = path_res(cx, f).opt_def_id() {
+
+            // fn_arg_names will ICE on a tuple struct being constructed.
+            //
+            // We can't usefully lint on this anyways, tuple struct constructors have no parameter
+            // names.
+            if cx.tcx.is_constructor(def_id) {
+                return;
+            }
+
+            // fn_arg_names will ICE on a variadic function.
+            //
+            // We *could* usefully lint on this (and it seems to work for *foreign* items, just not
+            // local ones?)
+            if cx.tcx.fn_sig(def_id).c_variadic() {
+                return;
+            }
+
+            let mut def_args = Vec::new();
+            for ident in cx.tcx.fn_arg_names(def_id) {
+                def_args.push((ident.to_string(), ident.span));
+            }
+
+            let mut call_args = Vec::new();
+
+            for call_arg in args {
+                call_args.push(guess_ident(call_arg));
+            }
+
+            if def_args.len() != call_args.len() {
+                let def_args_query = cx.tcx.fn_arg_names(def_id);
+                rustc_middle::span_bug!(expr.span, "{def_args_query:?}\n{args:#?}");
+            }
+
+            arguments_are_sus(cx, &def_args, &call_args);
+        }
+    }
+}

tests/ui/suspicious_arguments.rs

@@ -0,0 +1,145 @@
+// edition:2015
+#![warn(clippy::suspicious_arguments)]
+#![allow(anonymous_parameters, clippy::no_effect, dead_code)]
+
+trait AnonymousArgs {
+    fn scale(&self, usize, usize) {}
+}
+
+fn scale_this<A: AnonymousArgs>(value: A) {
+    // We don't expect a lint, we just don't want an ICE here.
+    let width = 1;
+    let height = 2;
+    value.scale(width, height);
+    A::scale(&value, width, height);
+}
+
+fn resize(width: usize, height: usize) {}
+
+struct Bitmap;
+
+impl Bitmap {
+    fn new(width: usize, height: usize) -> Self {
+        Bitmap
+    }
+}
+
+struct TupleStruct(usize, usize);
+
+enum TupleEnum {
+    Hello,
+    World(usize, usize),
+}
+
+#[derive(Default)]
+struct Dimensions {
+    width: usize,
+    height: usize,
+}
+
+fn function_names() {
+    fn height() -> usize { 0 }
+    fn width() -> usize { 0 }
+
+    resize(height(), width());
+}
+
+fn pathed_function_names() {
+    mod uwu {
+        pub fn height() -> usize { 0 }
+        pub fn width() -> usize { 0 }
+    }
+
+    resize(uwu::height(), uwu::width());
+}
+
+fn variable_names() {
+    let width = 0;
+    let height = 0;
+
+    resize(height, width);
+    Bitmap::new(height, width);
+}
+
+#[rustfmt::ignore]
+fn struct_names() {
+    let dims = Dimensions::default();
+    resize(dims.height, dims.width);
+
+    resize(
+        dims
+        .height,
+        // Very long!
+        dims
+        .width);
+}
+
+fn should_not_lint() {
+    let width = 0;
+    let height = 0;
+    TupleStruct(width, height);
+    TupleEnum::World(width, height);
+
+    resize(0, width);
+    resize(height, 0);
+    resize(height, height);
+    resize(width, width);
+}
+
+fn cross_crate() {
+    let f = vec![0_u8];
+    let iterable = |_| { true };
+
+    itertools::all(f, iterable);
+}
+
+fn cross_std() {
+    let mut xvalue = 42;
+    let mut yvalue = 42;
+    let x = &mut xvalue;
+    let y = &mut yvalue;
+
+    std::mem::swap(y, x);
+}
+
+fn varargs() {
+    extern "C" {
+        fn test_var_args(width: usize, height: usize, ...);
+    }
+
+    if false {
+        unsafe {
+            let width = 0;
+            let height = 0;
+            let not_foo = 0;
+            test_var_args(height, width, not_foo, width, height);
+        }
+    }
+}
+
+fn tri_rotate() {
+    fn large_resize(width: usize, height: usize, depth: usize) {}
+
+    let width = 0;
+    let height = 0;
+    let depth = 0;
+
+    large_resize(height, depth, width);
+}
+
+fn patterns() {
+    fn array([width, height]: [usize; 2]) {
+
+    }
+
+    fn tuple((width, height): (usize, usize)) {
+    }
+
+    let width = 0;
+    let height = 0;
+
+    array([height, width]);
+    tuple((height, width));
+}
+
+fn main() {}