fix suggestion for [len_zero] with macros

This commit is contained in:
J-ZhengLi 2024-04-02 01:27:17 +08:00
parent 41e814a554
commit b456ed31e4
4 changed files with 114 additions and 4 deletions

View file

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then};
use clippy_utils::source::snippet_with_context; use clippy_utils::source::{snippet_opt, snippet_with_context};
use clippy_utils::sugg::Sugg; use clippy_utils::sugg::{has_enclosing_paren, Sugg};
use clippy_utils::{get_item_name, get_parent_as_impl, is_lint_allowed, peel_ref_operators}; use clippy_utils::{get_item_name, get_parent_as_impl, is_lint_allowed, peel_ref_operators};
use rustc_ast::ast::LitKind; use rustc_ast::ast::LitKind;
use rustc_errors::Applicability; use rustc_errors::Applicability;
@ -192,7 +192,7 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
if let ExprKind::Binary(Spanned { node: cmp, .. }, left, right) = expr.kind { if let ExprKind::Binary(Spanned { node: cmp, .. }, left, right) = expr.kind {
// expr.span might contains parenthesis, see issue #10529 // expr.span might contains parenthesis, see issue #10529
let actual_span = left.span.with_hi(right.span.hi()); let actual_span = span_without_enclosing_paren(cx, expr.span);
match cmp { match cmp {
BinOpKind::Eq => { BinOpKind::Eq => {
check_cmp(cx, actual_span, left, right, "", 0); // len == 0 check_cmp(cx, actual_span, left, right, "", 0); // len == 0
@ -218,6 +218,20 @@ impl<'tcx> LateLintPass<'tcx> for LenZero {
} }
} }
fn span_without_enclosing_paren(cx: &LateContext<'_>, span: Span) -> Span {
let Some(snippet) = snippet_opt(cx, span) else {
return span;
};
if has_enclosing_paren(snippet) {
let source_map = cx.tcx.sess.source_map();
let left_paren = source_map.start_point(span);
let right_parent = source_map.end_point(span);
left_paren.between(right_parent)
} else {
span
}
}
fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items: &[TraitItemRef]) { fn check_trait_items(cx: &LateContext<'_>, visited_trait: &Item<'_>, trait_items: &[TraitItemRef]) {
fn is_named_self(cx: &LateContext<'_>, item: &TraitItemRef, name: Symbol) -> bool { fn is_named_self(cx: &LateContext<'_>, item: &TraitItemRef, name: Symbol) -> bool {
item.ident.name == name item.ident.name == name
@ -495,6 +509,10 @@ fn check_for_is_empty(
} }
fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>, op: &str, compare_to: u32) { fn check_cmp(cx: &LateContext<'_>, span: Span, method: &Expr<'_>, lit: &Expr<'_>, op: &str, compare_to: u32) {
if method.span.from_expansion() {
return;
}
if let (&ExprKind::MethodCall(method_path, receiver, args, _), ExprKind::Lit(lit)) = (&method.kind, &lit.kind) { if let (&ExprKind::MethodCall(method_path, receiver, args, _), ExprKind::Lit(lit)) = (&method.kind, &lit.kind) {
// check if we are in an is_empty() method // check if we are in an is_empty() method
if let Some(name) = get_item_name(cx, method) { if let Some(name) = get_item_name(cx, method) {

View file

@ -189,3 +189,40 @@ fn main() {
fn test_slice(b: &[u8]) { fn test_slice(b: &[u8]) {
if !b.is_empty() {} if !b.is_empty() {}
} }
// issue #11992
fn binop_with_macros() {
macro_rules! len {
($seq:ident) => {
$seq.len()
};
}
macro_rules! compare_to {
($val:literal) => {
$val
};
($val:expr) => {{ $val }};
}
macro_rules! zero {
() => {
0
};
}
let has_is_empty = HasIsEmpty;
// Don't lint, suggesting changes might break macro compatibility.
(len!(has_is_empty) > 0).then(|| println!("This can happen."));
// Don't lint, suggesting changes might break macro compatibility.
if len!(has_is_empty) == 0 {}
// Don't lint
if has_is_empty.len() == compare_to!(if true { 0 } else { 1 }) {}
// This is fine
if has_is_empty.len() == compare_to!(1) {}
if has_is_empty.is_empty() {}
if has_is_empty.is_empty() {}
(!has_is_empty.is_empty()).then(|| println!("This can happen."));
}

View file

@ -189,3 +189,40 @@ fn main() {
fn test_slice(b: &[u8]) { fn test_slice(b: &[u8]) {
if b.len() != 0 {} if b.len() != 0 {}
} }
// issue #11992
fn binop_with_macros() {
macro_rules! len {
($seq:ident) => {
$seq.len()
};
}
macro_rules! compare_to {
($val:literal) => {
$val
};
($val:expr) => {{ $val }};
}
macro_rules! zero {
() => {
0
};
}
let has_is_empty = HasIsEmpty;
// Don't lint, suggesting changes might break macro compatibility.
(len!(has_is_empty) > 0).then(|| println!("This can happen."));
// Don't lint, suggesting changes might break macro compatibility.
if len!(has_is_empty) == 0 {}
// Don't lint
if has_is_empty.len() == compare_to!(if true { 0 } else { 1 }) {}
// This is fine
if has_is_empty.len() == compare_to!(1) {}
if has_is_empty.len() == compare_to!(0) {}
if has_is_empty.len() == zero!() {}
(compare_to!(0) < has_is_empty.len()).then(|| println!("This can happen."));
}

View file

@ -142,5 +142,23 @@ error: length comparison to zero
LL | if b.len() != 0 {} LL | if b.len() != 0 {}
| ^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!b.is_empty()` | ^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!b.is_empty()`
error: aborting due to 23 previous errors error: length comparison to zero
--> tests/ui/len_zero.rs:224:8
|
LL | if has_is_empty.len() == compare_to!(0) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
error: length comparison to zero
--> tests/ui/len_zero.rs:225:8
|
LL | if has_is_empty.len() == zero!() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()`
error: length comparison to zero
--> tests/ui/len_zero.rs:227:6
|
LL | (compare_to!(0) < has_is_empty.len()).then(|| println!("This can happen."));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()`
error: aborting due to 26 previous errors