Skip to content

Commit

Permalink
Fix type suggestion for manual_is_ascii_check (rust-lang#13916)
Browse files Browse the repository at this point in the history
Fixes rust-lang#13913 .

changelog: [`manual_is_ascii_check`]: fix type suggestions for
references

Previously it only derived `char` and `u8` types, now it should always
annotate the lambda parameter with the correct type (e.g. `&char`).

I'm quite new to Rust and this is my first contact with clippy, so I'm
open for suggetions :)
  • Loading branch information
Alexendoo authored Jan 8, 2025
2 parents f5ca68f + 8461d3f commit 894e87c
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 16 deletions.
25 changes: 10 additions & 15 deletions clippy_lints/src/manual_is_ascii_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_ast::ast::RangeLimits;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Node, Param, PatKind, RangeEnd};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_middle::ty::{self, Ty};
use rustc_session::impl_lint_pass;
use rustc_span::{Span, sym};

Expand Down Expand Up @@ -114,7 +114,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
&& !matches!(cx.typeck_results().expr_ty(arg).peel_refs().kind(), ty::Param(_))
{
let arg = peel_ref_operators(cx, arg);
let ty_sugg = get_ty_sugg(cx, arg, start);
let ty_sugg = get_ty_sugg(cx, arg);
let range = check_range(start, end);
check_is_ascii(cx, expr.span, arg, &range, ty_sugg);
}
Expand All @@ -123,19 +123,14 @@ impl<'tcx> LateLintPass<'tcx> for ManualIsAsciiCheck {
extract_msrv_attr!(LateContext);
}

fn get_ty_sugg(cx: &LateContext<'_>, arg: &Expr<'_>, bound_expr: &Expr<'_>) -> Option<(Span, &'static str)> {
if let ExprKind::Lit(lit) = bound_expr.kind
&& let local_hid = path_to_local(arg)?
&& let Node::Param(Param { ty_span, span, .. }) = cx.tcx.parent_hir_node(local_hid)
fn get_ty_sugg<'tcx>(cx: &LateContext<'tcx>, arg: &Expr<'_>) -> Option<(Span, Ty<'tcx>)> {
let local_hid = path_to_local(arg)?;
if let Node::Param(Param { ty_span, span, .. }) = cx.tcx.parent_hir_node(local_hid)
// `ty_span` and `span` are the same for inferred type, thus a type suggestion must be given
&& ty_span == span
{
let ty_str = match lit.node {
Char(_) => "char",
Byte(_) => "u8",
_ => return None,
};
return Some((*ty_span, ty_str));
let arg_type = cx.typeck_results().expr_ty(arg);
return Some((*ty_span, arg_type));
}
None
}
Expand All @@ -145,7 +140,7 @@ fn check_is_ascii(
span: Span,
recv: &Expr<'_>,
range: &CharRange,
ty_sugg: Option<(Span, &'_ str)>,
ty_sugg: Option<(Span, Ty<'_>)>,
) {
let sugg = match range {
CharRange::UpperChar => "is_ascii_uppercase",
Expand All @@ -159,8 +154,8 @@ fn check_is_ascii(
let mut app = Applicability::MachineApplicable;
let recv = Sugg::hir_with_context(cx, recv, span.ctxt(), default_snip, &mut app).maybe_par();
let mut suggestion = vec![(span, format!("{recv}.{sugg}()"))];
if let Some((ty_span, ty_str)) = ty_sugg {
suggestion.push((ty_span, format!("{recv}: {ty_str}")));
if let Some((ty_span, ty)) = ty_sugg {
suggestion.push((ty_span, format!("{recv}: {ty}")));
}

span_lint_and_then(
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/manual_is_ascii_check.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,8 @@ fn generics() {
take_while(|c: u8| c.is_ascii_uppercase());
take_while(|c: char| c.is_ascii_uppercase());
}

fn adds_type_reference() {
let digits: Vec<&char> = ['1', 'A'].iter().take_while(|c: &&char| c.is_ascii_digit()).collect();
let digits: Vec<&mut char> = ['1', 'A'].iter_mut().take_while(|c: &&mut char| c.is_ascii_digit()).collect();
}
5 changes: 5 additions & 0 deletions tests/ui/manual_is_ascii_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,8 @@ fn generics() {
take_while(|c| (b'A'..=b'Z').contains(&c));
take_while(|c: char| ('A'..='Z').contains(&c));
}

fn adds_type_reference() {
let digits: Vec<&char> = ['1', 'A'].iter().take_while(|c| ('0'..='9').contains(c)).collect();
let digits: Vec<&mut char> = ['1', 'A'].iter_mut().take_while(|c| ('0'..='9').contains(c)).collect();
}
24 changes: 23 additions & 1 deletion tests/ui/manual_is_ascii_check.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -173,5 +173,27 @@ error: manual check for common ascii range
LL | take_while(|c: char| ('A'..='Z').contains(&c));
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `c.is_ascii_uppercase()`

error: aborting due to 27 previous errors
error: manual check for common ascii range
--> tests/ui/manual_is_ascii_check.rs:87:63
|
LL | let digits: Vec<&char> = ['1', 'A'].iter().take_while(|c| ('0'..='9').contains(c)).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^
|
help: try
|
LL | let digits: Vec<&char> = ['1', 'A'].iter().take_while(|c: &&char| c.is_ascii_digit()).collect();
| ~~~~~~~~~ ~~~~~~~~~~~~~~~~~~

error: manual check for common ascii range
--> tests/ui/manual_is_ascii_check.rs:88:71
|
LL | let digits: Vec<&mut char> = ['1', 'A'].iter_mut().take_while(|c| ('0'..='9').contains(c)).collect();
| ^^^^^^^^^^^^^^^^^^^^^^^
|
help: try
|
LL | let digits: Vec<&mut char> = ['1', 'A'].iter_mut().take_while(|c: &&mut char| c.is_ascii_digit()).collect();
| ~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~

error: aborting due to 29 previous errors

0 comments on commit 894e87c

Please sign in to comment.