skip warning when generic evolved;

J-ZhengLi · J-ZhengLi · commit 936d19c924a6 · 2023-12-20T16:55:44.000+08:00
suggest explicit type when its inferred in closure
diff --git a/clippy_lints/src/manual_is_ascii_check.rs b/clippy_lints/src/manual_is_ascii_check.rs
@@ -1,13 +1,15 @@
 use clippy_config::msrvs::{self, Msrv};
-use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::macros::root_macro_call;
 use clippy_utils::sugg::Sugg;
-use clippy_utils::{higher, in_constant};
+use clippy_utils::{higher, in_constant, path_to_local, peel_ref_operators};
 use rustc_ast::ast::RangeLimits;
-use rustc_ast::LitKind::{Byte, Char};
+use rustc_ast::LitKind::{self, Byte, Char};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::Applicability;
-use rustc_hir::{BorrowKind, Expr, ExprKind, PatKind, RangeEnd};
+use rustc_hir::{BodyId, Closure, Expr, ExprKind, FnDecl, HirId, PatKind, RangeEnd, TyKind};
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
 use rustc_session::impl_lint_pass;
 use rustc_span::def_id::DefId;
 use rustc_span::{sym, Span};
@@ -59,12 +61,16 @@ impl_lint_pass!(ManualIsAsciiCheck => [MANUAL_IS_ASCII_CHECK]);
 
 pub struct ManualIsAsciiCheck {
     msrv: Msrv,
+    parent_closure_params_map: FxHashMap<HirId, Span>,
 }
 
 impl ManualIsAsciiCheck {
     #[must_use]
     pub fn new(msrv: Msrv) -> Self {
-        Self { msrv }
+        Self {
+            msrv,
+            parent_closure_params_map: FxHashMap::default(),
+        }
     }
 }
 
@@ -97,12 +103,16 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
             return;
         }
 
+        if let ExprKind::Closure(Closure { fn_decl, body, .. }) = expr.kind {
+            self.parent_closure_params_map = params_with_inferred_ty_map(cx, *body, fn_decl);
+        }
+
         if let Some(macro_call) = root_macro_call(expr.span)
             && is_matches_macro(cx, macro_call.def_id)
         {
             if let ExprKind::Match(recv, [arm, ..], _) = expr.kind {
                 let range = check_pat(&arm.pat.kind);
-                check_is_ascii(cx, macro_call.span, recv, &range);
+                check_is_ascii(cx, macro_call.span, recv, &range, None);
             }
         } else if let ExprKind::MethodCall(path, receiver, [arg], ..) = expr.kind
             && path.ident.name == sym!(contains)
@@ -112,19 +122,58 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
                 limits: RangeLimits::Closed,
             }) = higher::Range::hir(receiver)
         {
-            let range = check_range(start, end);
-            if let ExprKind::AddrOf(BorrowKind::Ref, _, e) = arg.kind {
-                check_is_ascii(cx, expr.span, e, &range);
-            } else {
-                check_is_ascii(cx, expr.span, arg, &range);
+            let arg_ty = cx.typeck_results().expr_ty(arg).peel_refs();
+            if matches!(arg_ty.kind(), ty::Param(_)) {
+                return;
             }
+
+            let arg = peel_ref_operators(cx, arg);
+            let ty_sugg = get_ty_sugg(arg, &self.parent_closure_params_map, start);
+            let range = check_range(start, end);
+            check_is_ascii(cx, expr.span, peel_ref_operators(cx, arg), &range, ty_sugg);
+        }
+    }
+
+    fn check_expr_post(&mut self, _: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        if let ExprKind::Closure(_) = expr.kind {
+            self.parent_closure_params_map = FxHashMap::default();
         }
     }
 
     extract_msrv_attr!(LateContext);
 }
 
-fn check_is_ascii(cx: &LateContext<'_>, span: Span, recv: &Expr<'_>, range: &CharRange) {
+fn get_ty_sugg(arg: &Expr<'_>, map: &FxHashMap<HirId, Span>, range_expr: &Expr<'_>) -> Option<(Span, &'static str)> {
+    let ExprKind::Lit(lit) = range_expr.kind else {
+        return None;
+    };
+    let sugg_ty_span = path_to_local(arg).and_then(|id| map.get(&id)).copied()?;
+    let sugg_ty_str = match lit.node {
+        LitKind::Char(_) => "char",
+        LitKind::Byte(_) => "u8",
+        _ => return None,
+    };
+
+    Some((sugg_ty_span, sugg_ty_str))
+}
+
+fn params_with_inferred_ty_map(cx: &LateContext<'_>, body_id: BodyId, fn_decl: &FnDecl<'_>) -> FxHashMap<HirId, Span> {
+    let params = cx.tcx.hir().body(body_id).params.iter();
+    let fn_decl_input_tys = fn_decl.inputs.iter();
+    // Could just collect params with `span` == `ty_span` but this should be safer
+    params
+        .zip(fn_decl_input_tys)
+        .filter_map(|(param, ty)| {
+            if let TyKind::Infer = ty.kind {
+                Some((param.pat.hir_id, param.span))
+            } else {
+                None
+            }
+        })
+        .collect()
+}
+
+fn check_is_ascii(cx: &LateContext<'_>, span: Span, recv: &Expr<'_>, range: &CharRange, ty_sugg: Option<(Span, &str)>) {
     if let Some(sugg) = match range {
         CharRange::UpperChar => Some("is_ascii_uppercase"),
         CharRange::LowerChar => Some("is_ascii_lowercase"),
@@ -137,14 +186,22 @@ fn check_is_ascii(cx: &LateContext<'_>, span: Span, recv: &Expr<'_>, range: &Cha
         let mut app = Applicability::MachineApplicable;
         let recv = Sugg::hir_with_context(cx, recv, span.ctxt(), default_snip, &mut app).maybe_par();
 
-        span_lint_and_sugg(
+        span_lint_and_then(
             cx,
             MANUAL_IS_ASCII_CHECK,
             span,
             "manual check for common ascii range",
-            "try",
-            format!("{recv}.{sugg}()"),
-            app,
+            |diag| {
+                diag.span_suggestion(span, "try", format!("{recv}.{sugg}()"), app);
+                if let Some((ty_span, ty_str)) = ty_sugg {
+                    diag.span_suggestion(
+                        ty_span,
+                        "also make sure to label the correct type",
+                        format!("{recv}: {ty_str}"),
+                        app,
+                    );
+                }
+            },
         );
     }
 }
diff --git a/tests/ui/manual_is_ascii_check.fixed b/tests/ui/manual_is_ascii_check.fixed
@@ -55,3 +55,29 @@ fn msrv_1_47() {
     const FOO: bool = 'x'.is_ascii_digit();
     const BAR: bool = 'x'.is_ascii_hexdigit();
 }
+
+#[allow(clippy::deref_addrof, clippy::needless_borrow)]
+fn with_refs() {
+    let cool_letter = &&'g';
+    cool_letter.is_ascii_digit();
+    cool_letter.is_ascii_lowercase();
+}
+
+fn generics() {
+    fn a<U>(u: &U) -> bool
+    where
+        char: PartialOrd<U>,
+        U: PartialOrd<char> + ?Sized,
+    {
+        ('A'..='Z').contains(u)
+    }
+
+    fn take_while<Item, F>(cond: F)
+    where
+        Item: Sized,
+        F: Fn(Item) -> bool,
+    {
+    }
+    take_while(|c: char| c.is_ascii_uppercase());
+    take_while(|c: u8| c.is_ascii_uppercase());
+}
diff --git a/tests/ui/manual_is_ascii_check.rs b/tests/ui/manual_is_ascii_check.rs
@@ -55,3 +55,29 @@ fn msrv_1_47() {
     const FOO: bool = matches!('x', '0'..='9');
     const BAR: bool = matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F');
 }
+
+#[allow(clippy::deref_addrof, clippy::needless_borrow)]
+fn with_refs() {
+    let cool_letter = &&'g';
+    ('0'..='9').contains(&&cool_letter);
+    ('a'..='z').contains(*cool_letter);
+}
+
+fn generics() {
+    fn a<U>(u: &U) -> bool
+    where
+        char: PartialOrd<U>,
+        U: PartialOrd<char> + ?Sized,
+    {
+        ('A'..='Z').contains(u)
+    }
+
+    fn take_while<Item, F>(cond: F)
+    where
+        Item: Sized,
+        F: Fn(Item) -> bool,
+    {
+    }
+    take_while(|c| ('A'..='Z').contains(&c));
+    take_while(|c| (b'A'..=b'Z').contains(&c));
+}
diff --git a/tests/ui/manual_is_ascii_check.stderr b/tests/ui/manual_is_ascii_check.stderr
@@ -133,5 +133,47 @@ error: manual check for common ascii range
 LL |     const BAR: bool = matches!('x', '0'..='9' | 'a'..='f' | 'A'..='F');
    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `'x'.is_ascii_hexdigit()`
 
-error: aborting due to 22 previous errors
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:62:5
+   |
+LL |     ('0'..='9').contains(&&cool_letter);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cool_letter.is_ascii_digit()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:63:5
+   |
+LL |     ('a'..='z').contains(*cool_letter);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cool_letter.is_ascii_lowercase()`
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:81:20
+   |
+LL |     take_while(|c| ('A'..='Z').contains(&c));
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL |     take_while(|c| c.is_ascii_uppercase());
+   |                    ~~~~~~~~~~~~~~~~~~~~~~
+help: also make sure to label the correct type
+   |
+LL |     take_while(|c: char| ('A'..='Z').contains(&c));
+   |                 ~~~~~~~
+
+error: manual check for common ascii range
+  --> $DIR/manual_is_ascii_check.rs:82:20
+   |
+LL |     take_while(|c| (b'A'..=b'Z').contains(&c));
+   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL |     take_while(|c| c.is_ascii_uppercase());
+   |                    ~~~~~~~~~~~~~~~~~~~~~~
+help: also make sure to label the correct type
+   |
+LL |     take_while(|c: u8| (b'A'..=b'Z').contains(&c));
+   |                 ~~~~~
+
+error: aborting due to 26 previous errors
 
