From a4b413d4fd8244e8a54ada9ef6b99160d8f7f01c Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Mon, 25 Dec 2023 20:53:01 +0000 Subject: [PATCH 001/278] Add asm label support to AST and HIR --- clippy_lints/src/loops/never_loop.rs | 3 +++ clippy_utils/src/hir_utils.rs | 1 + 2 files changed, 4 insertions(+) diff --git a/clippy_lints/src/loops/never_loop.rs b/clippy_lints/src/loops/never_loop.rs index 245a903f998..65d922f03df 100644 --- a/clippy_lints/src/loops/never_loop.rs +++ b/clippy_lints/src/loops/never_loop.rs @@ -255,6 +255,9 @@ fn never_loop_expr<'tcx>( InlineAsmOperand::Const { .. } | InlineAsmOperand::SymFn { .. } | InlineAsmOperand::SymStatic { .. } => { NeverLoopResult::Normal }, + InlineAsmOperand::Label { block } => { + never_loop_block(cx, block, local_labels, main_loop_id) + } })), ExprKind::OffsetOf(_, _) | ExprKind::Yield(_, _) diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index d50332e82da..643852c1c54 100644 --- a/clippy_utils/src/hir_utils.rs +++ b/clippy_utils/src/hir_utils.rs @@ -835,6 +835,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { self.hash_body(anon_const.body); }, InlineAsmOperand::SymStatic { path, def_id: _ } => self.hash_qpath(path), + InlineAsmOperand::Label { block } => self.hash_block(block), } } }, From dbfbd0e77fd12a46ac81d0ec6fca4012aad0ea2d Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Mon, 19 Feb 2024 23:30:17 +0100 Subject: [PATCH 002/278] feat: add `const_is_empty` lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/methods/is_empty.rs | 40 +++++++++ clippy_lints/src/methods/mod.rs | 35 +++++++- tests/ui/const_is_empty.rs | 52 +++++++++++ tests/ui/const_is_empty.stderr | 124 +++++++++++++++++++++++++++ 6 files changed, 252 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/methods/is_empty.rs create mode 100644 tests/ui/const_is_empty.rs create mode 100644 tests/ui/const_is_empty.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 26dce1f9265..9182e61f6d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5109,6 +5109,7 @@ Released 2018-09-13 [`collection_is_never_read`]: https://rust-lang.github.io/rust-clippy/master/index.html#collection_is_never_read [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain [`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty +[`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime [`copy_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#copy_iterator [`crate_in_macro_def`]: https://rust-lang.github.io/rust-clippy/master/index.html#crate_in_macro_def diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index b6a35bb3e03..fe9bc77ce55 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -350,6 +350,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::CLONE_ON_COPY_INFO, crate::methods::CLONE_ON_REF_PTR_INFO, crate::methods::COLLAPSIBLE_STR_REPLACE_INFO, + crate::methods::CONST_IS_EMPTY_INFO, crate::methods::DRAIN_COLLECT_INFO, crate::methods::ERR_EXPECT_INFO, crate::methods::EXPECT_FUN_CALL_INFO, diff --git a/clippy_lints/src/methods/is_empty.rs b/clippy_lints/src/methods/is_empty.rs new file mode 100644 index 00000000000..4713c99d33b --- /dev/null +++ b/clippy_lints/src/methods/is_empty.rs @@ -0,0 +1,40 @@ +use clippy_utils::diagnostics::span_lint_and_note; +use clippy_utils::expr_or_init; +use rustc_ast::LitKind; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LintContext}; +use rustc_middle::lint::in_external_macro; +use rustc_span::source_map::Spanned; + +use super::CONST_IS_EMPTY; + +pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, receiver: &Expr<'_>) { + if in_external_macro(cx.sess(), expr.span) || !receiver.span.eq_ctxt(expr.span) { + return; + } + let init_expr = expr_or_init(cx, receiver); + if let Some(init_is_empty) = is_empty(init_expr) + && init_expr.span.eq_ctxt(receiver.span) + { + span_lint_and_note( + cx, + CONST_IS_EMPTY, + expr.span, + &format!("this expression always evaluates to {init_is_empty:?}"), + Some(init_expr.span), + "because its initialization value is constant", + ); + } +} + +fn is_empty(expr: &'_ rustc_hir::Expr<'_>) -> Option { + if let ExprKind::Lit(Spanned { node, .. }) = expr.kind { + match node { + LitKind::Str(sym, _) => Some(sym.is_empty()), + LitKind::ByteStr(value, _) | LitKind::CStr(value, _) => Some(value.is_empty()), + _ => None, + } + } else { + None + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 820f4c85890..b6894971acc 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -36,6 +36,7 @@ mod inefficient_to_string; mod inspect_for_each; mod into_iter_on_ref; mod is_digit_ascii_radix; +mod is_empty; mod iter_cloned_collect; mod iter_count; mod iter_filter; @@ -4041,6 +4042,31 @@ declare_clippy_lint! { "calling `.get().is_some()` or `.get().is_none()` instead of `.contains()` or `.contains_key()`" } +declare_clippy_lint! { + /// ### What it does + /// It identifies calls to `.is_empty()` on constant values. + /// + /// ### Why is this bad? + /// String literals and constant values are known at compile time. Checking if they + /// are empty will always return the same value. This might not be the intention of + /// the expression. + /// + /// ### Example + /// ```no_run + /// let value = ""; + /// if value.is_empty() { + /// println!("the string is empty"); + /// } + /// ``` + /// Use instead: + /// ```no_run + /// println!("the string is empty"); + /// ``` + #[clippy::version = "1.78.0"] + pub CONST_IS_EMPTY, + suspicious, + "is_empty() called on strings known at compile time" +} pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -4089,6 +4115,7 @@ impl_lint_pass!(Methods => [ CLONE_ON_COPY, CLONE_ON_REF_PTR, COLLAPSIBLE_STR_REPLACE, + CONST_IS_EMPTY, ITER_OVEREAGER_CLONED, CLONED_INSTEAD_OF_COPIED, FLAT_MAP_OPTION, @@ -4442,7 +4469,7 @@ impl Methods { ("as_deref" | "as_deref_mut", []) => { needless_option_as_deref::check(cx, expr, recv, name); }, - ("as_bytes" | "is_empty", []) => { + ("as_bytes", []) => { if let Some(("as_str", recv, [], as_str_span, _)) = method_call(recv) { redundant_as_str::check(cx, expr, recv, as_str_span, span); } @@ -4616,6 +4643,12 @@ impl Methods { ("hash", [arg]) => { unit_hash::check(cx, expr, recv, arg); }, + ("is_empty", []) => { + if let Some(("as_str", recv, [], as_str_span, _)) = method_call(recv) { + redundant_as_str::check(cx, expr, recv, as_str_span, span); + } + is_empty::check(cx, expr, recv); + }, ("is_file", []) => filetype_is_file::check(cx, expr, recv), ("is_digit", [radix]) => is_digit_ascii_radix::check(cx, expr, recv, radix, &self.msrv), ("is_none", []) => check_is_some_is_none(cx, expr, recv, call_span, false), diff --git a/tests/ui/const_is_empty.rs b/tests/ui/const_is_empty.rs new file mode 100644 index 00000000000..d61cdbb0424 --- /dev/null +++ b/tests/ui/const_is_empty.rs @@ -0,0 +1,52 @@ +#![warn(clippy::const_is_empty)] + +fn test_literal() { + if "".is_empty() { + //~^ERROR: this expression always evaluates to true + } + if "foobar".is_empty() { + //~^ERROR: this expression always evaluates to false + } +} + +fn test_byte_literal() { + if b"".is_empty() { + //~^ERROR: this expression always evaluates to true + } + if b"foobar".is_empty() { + //~^ERROR: this expression always evaluates to false + } +} + +fn test_no_mut() { + let mut empty = ""; + if empty.is_empty() { + // No lint because it is mutable + } +} + +fn test_propagated() { + let empty = ""; + let non_empty = "foobar"; + let empty2 = empty; + let non_empty2 = non_empty; + if empty2.is_empty() { + //~^ERROR: this expression always evaluates to true + } + if non_empty2.is_empty() { + //~^ERROR: this expression always evaluates to false + } +} + +fn main() { + let value = "foobar"; + let _ = value.is_empty(); + //~^ ERROR: this expression always evaluates to false + let x = value; + let _ = x.is_empty(); + //~^ ERROR: this expression always evaluates to false + let _ = "".is_empty(); + //~^ ERROR: this expression always evaluates to true + let _ = b"".is_empty(); + //~^ ERROR: this expression always evaluates to true +} diff --git a/tests/ui/const_is_empty.stderr b/tests/ui/const_is_empty.stderr new file mode 100644 index 00000000000..5a89e686242 --- /dev/null +++ b/tests/ui/const_is_empty.stderr @@ -0,0 +1,124 @@ +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:4:8 + | +LL | if "".is_empty() { + | ^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:4:8 + | +LL | if "".is_empty() { + | ^^ + = note: `-D clippy::const-is-empty` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::const_is_empty)]` + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:7:8 + | +LL | if "foobar".is_empty() { + | ^^^^^^^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:7:8 + | +LL | if "foobar".is_empty() { + | ^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:13:8 + | +LL | if b"".is_empty() { + | ^^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:13:8 + | +LL | if b"".is_empty() { + | ^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:16:8 + | +LL | if b"foobar".is_empty() { + | ^^^^^^^^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:16:8 + | +LL | if b"foobar".is_empty() { + | ^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:33:8 + | +LL | if empty2.is_empty() { + | ^^^^^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:29:17 + | +LL | let empty = ""; + | ^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:36:8 + | +LL | if non_empty2.is_empty() { + | ^^^^^^^^^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:30:21 + | +LL | let non_empty = "foobar"; + | ^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:43:13 + | +LL | let _ = value.is_empty(); + | ^^^^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:42:17 + | +LL | let value = "foobar"; + | ^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:46:13 + | +LL | let _ = x.is_empty(); + | ^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:42:17 + | +LL | let value = "foobar"; + | ^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:48:13 + | +LL | let _ = "".is_empty(); + | ^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:48:13 + | +LL | let _ = "".is_empty(); + | ^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:50:13 + | +LL | let _ = b"".is_empty(); + | ^^^^^^^^^^^^^^ + | +note: because its initialization value is constant + --> tests/ui/const_is_empty.rs:50:13 + | +LL | let _ = b"".is_empty(); + | ^^^ + +error: aborting due to 10 previous errors + From 89b334d47cf3326af349e2a6a747e707f47bdd30 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Sun, 18 Feb 2024 15:39:34 +0100 Subject: [PATCH 003/278] chore: update some tests to allow `const_is_empty` --- tests/ui/bool_assert_comparison.fixed | 2 +- tests/ui/bool_assert_comparison.rs | 2 +- tests/ui/len_zero.fixed | 8 ++++- tests/ui/len_zero.rs | 8 ++++- tests/ui/len_zero.stderr | 46 +++++++++++++-------------- tests/ui/needless_bitwise_bool.fixed | 1 + tests/ui/needless_bitwise_bool.rs | 1 + tests/ui/needless_bitwise_bool.stderr | 2 +- tests/ui/redundant_as_str.fixed | 1 + tests/ui/redundant_as_str.rs | 1 + tests/ui/redundant_as_str.stderr | 4 +-- 11 files changed, 46 insertions(+), 30 deletions(-) diff --git a/tests/ui/bool_assert_comparison.fixed b/tests/ui/bool_assert_comparison.fixed index 63b8e27e1c6..b05166a055e 100644 --- a/tests/ui/bool_assert_comparison.fixed +++ b/tests/ui/bool_assert_comparison.fixed @@ -1,4 +1,4 @@ -#![allow(unused, clippy::assertions_on_constants)] +#![allow(unused, clippy::assertions_on_constants, clippy::const_is_empty)] #![warn(clippy::bool_assert_comparison)] use std::ops::Not; diff --git a/tests/ui/bool_assert_comparison.rs b/tests/ui/bool_assert_comparison.rs index 58f81fedb79..dc51fcf1d36 100644 --- a/tests/ui/bool_assert_comparison.rs +++ b/tests/ui/bool_assert_comparison.rs @@ -1,4 +1,4 @@ -#![allow(unused, clippy::assertions_on_constants)] +#![allow(unused, clippy::assertions_on_constants, clippy::const_is_empty)] #![warn(clippy::bool_assert_comparison)] use std::ops::Not; diff --git a/tests/ui/len_zero.fixed b/tests/ui/len_zero.fixed index 745fc7e1a8b..c16d7a26616 100644 --- a/tests/ui/len_zero.fixed +++ b/tests/ui/len_zero.fixed @@ -1,5 +1,11 @@ #![warn(clippy::len_zero)] -#![allow(dead_code, unused, clippy::needless_if, clippy::len_without_is_empty)] +#![allow( + dead_code, + unused, + clippy::needless_if, + clippy::len_without_is_empty, + clippy::const_is_empty +)] extern crate core; use core::ops::Deref; diff --git a/tests/ui/len_zero.rs b/tests/ui/len_zero.rs index 048ad2f4fd3..5c49a5abf81 100644 --- a/tests/ui/len_zero.rs +++ b/tests/ui/len_zero.rs @@ -1,5 +1,11 @@ #![warn(clippy::len_zero)] -#![allow(dead_code, unused, clippy::needless_if, clippy::len_without_is_empty)] +#![allow( + dead_code, + unused, + clippy::needless_if, + clippy::len_without_is_empty, + clippy::const_is_empty +)] extern crate core; use core::ops::Deref; diff --git a/tests/ui/len_zero.stderr b/tests/ui/len_zero.stderr index b1f04c94de6..dd07a85d62c 100644 --- a/tests/ui/len_zero.stderr +++ b/tests/ui/len_zero.stderr @@ -1,5 +1,5 @@ error: length comparison to zero - --> tests/ui/len_zero.rs:82:8 + --> tests/ui/len_zero.rs:88:8 | LL | if x.len() == 0 { | ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `x.is_empty()` @@ -8,13 +8,13 @@ LL | if x.len() == 0 { = help: to override `-D warnings` add `#[allow(clippy::len_zero)]` error: length comparison to zero - --> tests/ui/len_zero.rs:86:8 + --> tests/ui/len_zero.rs:92:8 | LL | if "".len() == 0 {} | ^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `"".is_empty()` error: comparison to empty slice - --> tests/ui/len_zero.rs:95:20 + --> tests/ui/len_zero.rs:101:20 | LL | println!("{}", *s1 == ""); | ^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s1.is_empty()` @@ -23,121 +23,121 @@ LL | println!("{}", *s1 == ""); = help: to override `-D warnings` add `#[allow(clippy::comparison_to_empty)]` error: comparison to empty slice - --> tests/ui/len_zero.rs:96:20 + --> tests/ui/len_zero.rs:102:20 | LL | println!("{}", **s2 == ""); | ^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s2.is_empty()` error: comparison to empty slice - --> tests/ui/len_zero.rs:97:20 + --> tests/ui/len_zero.rs:103:20 | LL | println!("{}", ***s3 == ""); | ^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s3.is_empty()` error: comparison to empty slice - --> tests/ui/len_zero.rs:98:20 + --> tests/ui/len_zero.rs:104:20 | LL | println!("{}", ****s4 == ""); | ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s4.is_empty()` error: comparison to empty slice - --> tests/ui/len_zero.rs:99:20 + --> tests/ui/len_zero.rs:105:20 | LL | println!("{}", *****s5 == ""); | ^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `s5.is_empty()` error: comparison to empty slice - --> tests/ui/len_zero.rs:100:20 + --> tests/ui/len_zero.rs:106:20 | LL | println!("{}", ******(s6) == ""); | ^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(s6).is_empty()` error: comparison to empty slice - --> tests/ui/len_zero.rs:103:20 + --> tests/ui/len_zero.rs:109:20 | LL | println!("{}", &**d2s == ""); | ^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `(**d2s).is_empty()` error: length comparison to zero - --> tests/ui/len_zero.rs:118:8 + --> tests/ui/len_zero.rs:124:8 | LL | if has_is_empty.len() == 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:121:8 + --> tests/ui/len_zero.rs:127:8 | LL | if has_is_empty.len() != 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:124:8 + --> tests/ui/len_zero.rs:130:8 | LL | if has_is_empty.len() > 0 { | ^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()` error: length comparison to one - --> tests/ui/len_zero.rs:127:8 + --> tests/ui/len_zero.rs:133:8 | LL | if has_is_empty.len() < 1 { | ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()` error: length comparison to one - --> tests/ui/len_zero.rs:130:8 + --> tests/ui/len_zero.rs:136:8 | LL | if has_is_empty.len() >= 1 { | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()` error: length comparison to zero - --> tests/ui/len_zero.rs:141:8 + --> tests/ui/len_zero.rs:147:8 | LL | if 0 == has_is_empty.len() { | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()` error: length comparison to zero - --> tests/ui/len_zero.rs:144:8 + --> tests/ui/len_zero.rs:150:8 | LL | if 0 != has_is_empty.len() { | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()` error: length comparison to zero - --> tests/ui/len_zero.rs:147:8 + --> tests/ui/len_zero.rs:153:8 | LL | if 0 < has_is_empty.len() { | ^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()` error: length comparison to one - --> tests/ui/len_zero.rs:150:8 + --> tests/ui/len_zero.rs:156:8 | LL | if 1 <= has_is_empty.len() { | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()` error: length comparison to one - --> tests/ui/len_zero.rs:153:8 + --> tests/ui/len_zero.rs:159:8 | LL | if 1 > has_is_empty.len() { | ^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()` error: length comparison to zero - --> tests/ui/len_zero.rs:167:8 + --> tests/ui/len_zero.rs:173:8 | LL | if with_is_empty.len() == 0 { | ^^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `with_is_empty.is_empty()` error: length comparison to zero - --> tests/ui/len_zero.rs:179:6 + --> tests/ui/len_zero.rs:185:6 | LL | (has_is_empty.len() > 0).then(|| println!("This can happen.")); | ^^^^^^^^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!has_is_empty.is_empty()` error: length comparison to zero - --> tests/ui/len_zero.rs:180:6 + --> tests/ui/len_zero.rs:186:6 | LL | (has_is_empty.len() == 0).then(|| println!("Or this!")); | ^^^^^^^^^^^^^^^^^^^^^^^ help: using `is_empty` is clearer and more explicit: `has_is_empty.is_empty()` error: length comparison to zero - --> tests/ui/len_zero.rs:184:8 + --> tests/ui/len_zero.rs:190:8 | LL | if b.len() != 0 {} | ^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!b.is_empty()` diff --git a/tests/ui/needless_bitwise_bool.fixed b/tests/ui/needless_bitwise_bool.fixed index 201f8a4c19d..a8176618c1f 100644 --- a/tests/ui/needless_bitwise_bool.fixed +++ b/tests/ui/needless_bitwise_bool.fixed @@ -1,4 +1,5 @@ #![warn(clippy::needless_bitwise_bool)] +#![allow(clippy::const_is_empty)] fn returns_bool() -> bool { true diff --git a/tests/ui/needless_bitwise_bool.rs b/tests/ui/needless_bitwise_bool.rs index b0e5014b74b..f190eb2b76e 100644 --- a/tests/ui/needless_bitwise_bool.rs +++ b/tests/ui/needless_bitwise_bool.rs @@ -1,4 +1,5 @@ #![warn(clippy::needless_bitwise_bool)] +#![allow(clippy::const_is_empty)] fn returns_bool() -> bool { true diff --git a/tests/ui/needless_bitwise_bool.stderr b/tests/ui/needless_bitwise_bool.stderr index f29d4492540..9f14646c3e5 100644 --- a/tests/ui/needless_bitwise_bool.stderr +++ b/tests/ui/needless_bitwise_bool.stderr @@ -1,5 +1,5 @@ error: use of bitwise operator instead of lazy operator between booleans - --> tests/ui/needless_bitwise_bool.rs:22:8 + --> tests/ui/needless_bitwise_bool.rs:23:8 | LL | if y & !x { | ^^^^^^ help: try: `y && !x` diff --git a/tests/ui/redundant_as_str.fixed b/tests/ui/redundant_as_str.fixed index 4185b402226..708a1cc9150 100644 --- a/tests/ui/redundant_as_str.fixed +++ b/tests/ui/redundant_as_str.fixed @@ -1,4 +1,5 @@ #![warn(clippy::redundant_as_str)] +#![allow(clippy::const_is_empty)] fn main() { let string = "Hello, world!".to_owned(); diff --git a/tests/ui/redundant_as_str.rs b/tests/ui/redundant_as_str.rs index 7a74d8a55de..257af591cef 100644 --- a/tests/ui/redundant_as_str.rs +++ b/tests/ui/redundant_as_str.rs @@ -1,4 +1,5 @@ #![warn(clippy::redundant_as_str)] +#![allow(clippy::const_is_empty)] fn main() { let string = "Hello, world!".to_owned(); diff --git a/tests/ui/redundant_as_str.stderr b/tests/ui/redundant_as_str.stderr index f086de5fede..f5379d701db 100644 --- a/tests/ui/redundant_as_str.stderr +++ b/tests/ui/redundant_as_str.stderr @@ -1,5 +1,5 @@ error: this `as_str` is redundant and can be removed as the method immediately following exists on `String` too - --> tests/ui/redundant_as_str.rs:7:29 + --> tests/ui/redundant_as_str.rs:8:29 | LL | let _redundant = string.as_str().as_bytes(); | ^^^^^^^^^^^^^^^^^ help: try: `as_bytes` @@ -8,7 +8,7 @@ LL | let _redundant = string.as_str().as_bytes(); = help: to override `-D warnings` add `#[allow(clippy::redundant_as_str)]` error: this `as_str` is redundant and can be removed as the method immediately following exists on `String` too - --> tests/ui/redundant_as_str.rs:8:29 + --> tests/ui/redundant_as_str.rs:9:29 | LL | let _redundant = string.as_str().is_empty(); | ^^^^^^^^^^^^^^^^^ help: try: `is_empty` From 288497093b3bba07d8ab994913135ad6b0cccbc8 Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Mon, 19 Feb 2024 23:52:09 +0100 Subject: [PATCH 004/278] feat: extend `const_is_empty` with many kinds of constants --- clippy_lints/src/methods/is_empty.rs | 51 +++++---- clippy_utils/src/consts.rs | 96 ++++++++++++++-- tests/ui/const_is_empty.rs | 49 +++++++++ tests/ui/const_is_empty.stderr | 157 ++++++++++++++++----------- 4 files changed, 262 insertions(+), 91 deletions(-) diff --git a/clippy_lints/src/methods/is_empty.rs b/clippy_lints/src/methods/is_empty.rs index 4713c99d33b..7fe66062251 100644 --- a/clippy_lints/src/methods/is_empty.rs +++ b/clippy_lints/src/methods/is_empty.rs @@ -1,40 +1,49 @@ -use clippy_utils::diagnostics::span_lint_and_note; -use clippy_utils::expr_or_init; -use rustc_ast::LitKind; -use rustc_hir::{Expr, ExprKind}; +use clippy_utils::consts::constant_is_empty; +use clippy_utils::diagnostics::span_lint; +use clippy_utils::{find_binding_init, path_to_local}; +use rustc_hir::{Expr, HirId}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::lint::in_external_macro; -use rustc_span::source_map::Spanned; +use rustc_span::sym; use super::CONST_IS_EMPTY; +/// Expression whose initialization depend on a constant conditioned by a `#[cfg(…)]` directive will +/// not trigger the lint. pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, receiver: &Expr<'_>) { if in_external_macro(cx.sess(), expr.span) || !receiver.span.eq_ctxt(expr.span) { return; } let init_expr = expr_or_init(cx, receiver); - if let Some(init_is_empty) = is_empty(init_expr) - && init_expr.span.eq_ctxt(receiver.span) - { - span_lint_and_note( + if !receiver.span.eq_ctxt(init_expr.span) { + return; + } + if let Some(init_is_empty) = constant_is_empty(cx, init_expr) { + span_lint( cx, CONST_IS_EMPTY, expr.span, &format!("this expression always evaluates to {init_is_empty:?}"), - Some(init_expr.span), - "because its initialization value is constant", ); } } -fn is_empty(expr: &'_ rustc_hir::Expr<'_>) -> Option { - if let ExprKind::Lit(Spanned { node, .. }) = expr.kind { - match node { - LitKind::Str(sym, _) => Some(sym.is_empty()), - LitKind::ByteStr(value, _) | LitKind::CStr(value, _) => Some(value.is_empty()), - _ => None, - } - } else { - None - } +fn is_under_cfg(cx: &LateContext<'_>, id: HirId) -> bool { + cx.tcx + .hir() + .parent_id_iter(id) + .any(|id| cx.tcx.hir().attrs(id).iter().any(|attr| attr.has_name(sym::cfg))) +} + +/// Similar to [`clippy_utils::expr_or_init`], but does not go up the chain if the initialization +/// value depends on a `#[cfg(…)]` directive. +fn expr_or_init<'a, 'b, 'tcx: 'b>(cx: &LateContext<'tcx>, mut expr: &'a Expr<'b>) -> &'a Expr<'b> { + while let Some(init) = path_to_local(expr) + .and_then(|id| find_binding_init(cx, id)) + .filter(|init| cx.typeck_results().expr_adjustments(init).is_empty()) + .filter(|init| !is_under_cfg(cx, init.hir_id)) + { + expr = init; + } + expr } diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 79c691992a8..774f3f99d46 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -10,6 +10,7 @@ use rustc_hir::{BinOp, BinOpKind, Block, ConstBlock, Expr, ExprKind, HirId, Item use rustc_lexer::tokenize; use rustc_lint::LateContext; use rustc_middle::mir::interpret::{alloc_range, Scalar}; +use rustc_middle::mir::ConstValue; use rustc_middle::ty::{self, EarlyBinder, FloatTy, GenericArgsRef, IntTy, List, ScalarInt, Ty, TyCtxt, UintTy}; use rustc_middle::{bug, mir, span_bug}; use rustc_span::symbol::{Ident, Symbol}; @@ -303,6 +304,12 @@ impl ConstantSource { } } +/// Attempts to check whether the expression is a constant representing an empty slice, str, array, +/// etc… +pub fn constant_is_empty(lcx: &LateContext<'_>, e: &Expr<'_>) -> Option { + ConstEvalLateContext::new(lcx, lcx.typeck_results()).expr_is_empty(e) +} + /// Attempts to evaluate the expression as a constant. pub fn constant<'tcx>( lcx: &LateContext<'tcx>, @@ -402,7 +409,13 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { match e.kind { ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr(self.lcx.tcx.hir().body(body).value), ExprKind::DropTemps(e) => self.expr(e), - ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id, self.typeck_results.expr_ty(e)), + ExprKind::Path(ref qpath) => { + self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| { + let result = mir_to_const(this.lcx, result)?; + this.source = ConstantSource::Constant; + Some(result) + }) + }, ExprKind::Block(block, _) => self.block(block), ExprKind::Lit(lit) => { if is_direct_expn_of(e.span, "cfg").is_some() { @@ -468,6 +481,39 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { } } + /// Simple constant folding to determine if an expression is an empty slice, str, array, … + pub fn expr_is_empty(&mut self, e: &Expr<'_>) -> Option { + match e.kind { + ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr_is_empty(self.lcx.tcx.hir().body(body).value), + ExprKind::DropTemps(e) => self.expr_is_empty(e), + ExprKind::Path(ref qpath) => { + self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| { + mir_is_empty(this.lcx, result) + }) + }, + ExprKind::Lit(lit) => { + if is_direct_expn_of(e.span, "cfg").is_some() { + None + } else { + match &lit.node { + LitKind::Str(is, _) => Some(is.is_empty()), + LitKind::ByteStr(s, _) | LitKind::CStr(s, _) => Some(s.is_empty()), + _ => None, + } + } + }, + ExprKind::Array(vec) => self.multi(vec).map(|v| v.is_empty()), + ExprKind::Repeat(..) => { + if let ty::Array(_, n) = self.typeck_results.expr_ty(e).kind() { + Some(n.try_eval_target_usize(self.lcx.tcx, self.lcx.param_env)? == 0) + } else { + span_bug!(e.span, "typeck error"); + } + }, + _ => None, + } + } + #[expect(clippy::cast_possible_wrap)] fn constant_not(&self, o: &Constant<'tcx>, ty: Ty<'_>) -> Option> { use self::Constant::{Bool, Int}; @@ -515,8 +561,11 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { vec.iter().map(|elem| self.expr(elem)).collect::>() } - /// Lookup a possibly constant expression from an `ExprKind::Path`. - fn fetch_path(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>) -> Option> { + /// Lookup a possibly constant expression from an `ExprKind::Path` and apply a function on it. + fn fetch_path_and_apply(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>, f: F) -> Option + where + F: FnOnce(&mut Self, rustc_middle::mir::Const<'tcx>) -> Option, + { let res = self.typeck_results.qpath_res(qpath, id); match res { Res::Def(DefKind::Const | DefKind::AssocConst, def_id) => { @@ -549,9 +598,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { .const_eval_resolve(self.param_env, mir::UnevaluatedConst::new(def_id, args), None) .ok() .map(|val| rustc_middle::mir::Const::from_value(val, ty))?; - let result = mir_to_const(self.lcx, result)?; - self.source = ConstantSource::Constant; - Some(result) + f(self, result) }, _ => None, } @@ -742,7 +789,6 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { } pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> Option> { - use rustc_middle::mir::ConstValue; let mir::Const::Val(val, _) = result else { // We only work on evaluated consts. return None; @@ -788,6 +834,42 @@ pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> } } +fn mir_is_empty<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> Option { + let mir::Const::Val(val, _) = result else { + // We only work on evaluated consts. + return None; + }; + match (val, result.ty().kind()) { + (_, ty::Ref(_, inner_ty, _)) => match inner_ty.kind() { + ty::Str | ty::Slice(_) => { + if let ConstValue::Indirect { alloc_id, offset } = val { + // Get the length from the slice, using the same formula as + // [`ConstValue::try_get_slice_bytes_for_diagnostics`]. + let a = lcx.tcx.global_alloc(alloc_id).unwrap_memory().inner(); + let ptr_size = lcx.tcx.data_layout.pointer_size; + if a.size() < offset + 2 * ptr_size { + // (partially) dangling reference + return None; + } + let len = a + .read_scalar(&lcx.tcx, alloc_range(offset + ptr_size, ptr_size), false) + .ok()? + .to_target_usize(&lcx.tcx) + .ok()?; + Some(len == 0) + } else { + None + } + }, + ty::Array(_, len) => Some(len.try_to_target_usize(lcx.tcx)? == 0), + _ => None, + }, + (ConstValue::Indirect { .. }, ty::Array(_, len)) => Some(len.try_to_target_usize(lcx.tcx)? == 0), + (ConstValue::ZeroSized, _) => Some(true), + _ => None, + } +} + fn field_of_struct<'tcx>( adt_def: ty::AdtDef<'tcx>, lcx: &LateContext<'tcx>, diff --git a/tests/ui/const_is_empty.rs b/tests/ui/const_is_empty.rs index d61cdbb0424..bd7e3a7c5d5 100644 --- a/tests/ui/const_is_empty.rs +++ b/tests/ui/const_is_empty.rs @@ -38,6 +38,55 @@ fn test_propagated() { } } +const EMPTY_STR: &str = ""; +const NON_EMPTY_STR: &str = "foo"; +const EMPTY_BSTR: &[u8] = b""; +const NON_EMPTY_BSTR: &[u8] = b"foo"; +const EMPTY_U8_SLICE: &[u8] = &[]; +const NON_EMPTY_U8_SLICE: &[u8] = &[1, 2]; +const EMPTY_SLICE: &[u32] = &[]; +const NON_EMPTY_SLICE: &[u32] = &[1, 2]; +const NON_EMPTY_SLICE_REPEAT: &[u32] = &[1; 2]; +const EMPTY_ARRAY: [u32; 0] = []; +const EMPTY_ARRAY_REPEAT: [u32; 0] = [1; 0]; +const NON_EMPTY_ARRAY: [u32; 2] = [1, 2]; +const NON_EMPTY_ARRAY_REPEAT: [u32; 2] = [1; 2]; +const EMPTY_REF_ARRAY: &[u32; 0] = &[]; +const NON_EMPTY_REF_ARRAY: &[u32; 3] = &[1, 2, 3]; + +fn test_from_const() { + let _ = EMPTY_STR.is_empty(); + //~^ ERROR: this expression always evaluates to true + let _ = NON_EMPTY_STR.is_empty(); + //~^ ERROR: this expression always evaluates to false + let _ = EMPTY_BSTR.is_empty(); + //~^ ERROR: this expression always evaluates to true + let _ = NON_EMPTY_BSTR.is_empty(); + //~^ ERROR: this expression always evaluates to false + let _ = EMPTY_ARRAY.is_empty(); + //~^ ERROR: this expression always evaluates to true + let _ = EMPTY_ARRAY_REPEAT.is_empty(); + //~^ ERROR: this expression always evaluates to true + let _ = EMPTY_U8_SLICE.is_empty(); + //~^ ERROR: this expression always evaluates to true + let _ = NON_EMPTY_U8_SLICE.is_empty(); + //~^ ERROR: this expression always evaluates to false + let _ = NON_EMPTY_ARRAY.is_empty(); + //~^ ERROR: this expression always evaluates to false + let _ = NON_EMPTY_ARRAY_REPEAT.is_empty(); + //~^ ERROR: this expression always evaluates to false + let _ = EMPTY_REF_ARRAY.is_empty(); + //~^ ERROR: this expression always evaluates to true + let _ = NON_EMPTY_REF_ARRAY.is_empty(); + //~^ ERROR: this expression always evaluates to false + let _ = EMPTY_SLICE.is_empty(); + //~^ ERROR: this expression always evaluates to true + let _ = NON_EMPTY_SLICE.is_empty(); + //~^ ERROR: this expression always evaluates to false + let _ = NON_EMPTY_SLICE_REPEAT.is_empty(); + //~^ ERROR: this expression always evaluates to false +} + fn main() { let value = "foobar"; let _ = value.is_empty(); diff --git a/tests/ui/const_is_empty.stderr b/tests/ui/const_is_empty.stderr index 5a89e686242..ef5aa556e6a 100644 --- a/tests/ui/const_is_empty.stderr +++ b/tests/ui/const_is_empty.stderr @@ -4,11 +4,6 @@ error: this expression always evaluates to true LL | if "".is_empty() { | ^^^^^^^^^^^^^ | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:4:8 - | -LL | if "".is_empty() { - | ^^ = note: `-D clippy::const-is-empty` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::const_is_empty)]` @@ -17,108 +12,144 @@ error: this expression always evaluates to false | LL | if "foobar".is_empty() { | ^^^^^^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:7:8 - | -LL | if "foobar".is_empty() { - | ^^^^^^^^ error: this expression always evaluates to true --> tests/ui/const_is_empty.rs:13:8 | LL | if b"".is_empty() { | ^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:13:8 - | -LL | if b"".is_empty() { - | ^^^ error: this expression always evaluates to false --> tests/ui/const_is_empty.rs:16:8 | LL | if b"foobar".is_empty() { | ^^^^^^^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:16:8 - | -LL | if b"foobar".is_empty() { - | ^^^^^^^^^ error: this expression always evaluates to true --> tests/ui/const_is_empty.rs:33:8 | LL | if empty2.is_empty() { | ^^^^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:29:17 - | -LL | let empty = ""; - | ^^ error: this expression always evaluates to false --> tests/ui/const_is_empty.rs:36:8 | LL | if non_empty2.is_empty() { | ^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:58:13 | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:30:21 - | -LL | let non_empty = "foobar"; - | ^^^^^^^^ +LL | let _ = EMPTY_STR.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:43:13 + --> tests/ui/const_is_empty.rs:60:13 + | +LL | let _ = NON_EMPTY_STR.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:62:13 + | +LL | let _ = EMPTY_BSTR.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:64:13 + | +LL | let _ = NON_EMPTY_BSTR.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:66:13 + | +LL | let _ = EMPTY_ARRAY.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:68:13 + | +LL | let _ = EMPTY_ARRAY_REPEAT.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:70:13 + | +LL | let _ = EMPTY_U8_SLICE.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:72:13 + | +LL | let _ = NON_EMPTY_U8_SLICE.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:74:13 + | +LL | let _ = NON_EMPTY_ARRAY.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:76:13 + | +LL | let _ = NON_EMPTY_ARRAY_REPEAT.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:78:13 + | +LL | let _ = EMPTY_REF_ARRAY.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:80:13 + | +LL | let _ = NON_EMPTY_REF_ARRAY.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:82:13 + | +LL | let _ = EMPTY_SLICE.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:84:13 + | +LL | let _ = NON_EMPTY_SLICE.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:86:13 + | +LL | let _ = NON_EMPTY_SLICE_REPEAT.is_empty(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: this expression always evaluates to false + --> tests/ui/const_is_empty.rs:92:13 | LL | let _ = value.is_empty(); | ^^^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:42:17 - | -LL | let value = "foobar"; - | ^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:46:13 + --> tests/ui/const_is_empty.rs:95:13 | LL | let _ = x.is_empty(); | ^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:42:17 - | -LL | let value = "foobar"; - | ^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:48:13 + --> tests/ui/const_is_empty.rs:97:13 | LL | let _ = "".is_empty(); | ^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:48:13 - | -LL | let _ = "".is_empty(); - | ^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:50:13 + --> tests/ui/const_is_empty.rs:99:13 | LL | let _ = b"".is_empty(); | ^^^^^^^^^^^^^^ - | -note: because its initialization value is constant - --> tests/ui/const_is_empty.rs:50:13 - | -LL | let _ = b"".is_empty(); - | ^^^ -error: aborting due to 10 previous errors +error: aborting due to 25 previous errors From 1159e2c00fdc97dede1dca0464d57750283ac3df Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Tue, 20 Feb 2024 00:05:59 +0100 Subject: [PATCH 005/278] feat: add more tests for the `const_is_empty` lint Suggested by @xFredNet and @matthiaskrgr. --- tests/ui/const_is_empty.rs | 68 ++++++++++++++++++++++++++++++++++ tests/ui/const_is_empty.stderr | 58 ++++++++++++++++------------- 2 files changed, 100 insertions(+), 26 deletions(-) diff --git a/tests/ui/const_is_empty.rs b/tests/ui/const_is_empty.rs index bd7e3a7c5d5..df7817e4b5f 100644 --- a/tests/ui/const_is_empty.rs +++ b/tests/ui/const_is_empty.rs @@ -1,4 +1,6 @@ +#![feature(inline_const)] #![warn(clippy::const_is_empty)] +#![allow(clippy::needless_late_init, unused_must_use)] fn test_literal() { if "".is_empty() { @@ -99,3 +101,69 @@ fn main() { let _ = b"".is_empty(); //~^ ERROR: this expression always evaluates to true } + +fn str_from_arg(var: &str) { + var.is_empty(); + // Do not lint, we know nothiny about var +} + +fn update_str() { + let mut value = "duck"; + value = "penguin"; + + let _ = value.is_empty(); + // Do not lint since value is mutable +} + +fn macros() { + // Content from Macro + let file = include_str!("const_is_empty.rs"); + let _ = file.is_empty(); + // No lint because initializer comes from a macro result + + let var = env!("PATH"); + let _ = var.is_empty(); + // No lint because initializer comes from a macro result +} + +fn conditional_value() { + let value; + + if true { + value = "hey"; + } else { + value = "hej"; + } + + let _ = value.is_empty(); + // Do not lint, current constant folding is too simple to detect this +} + +fn cfg_conditioned() { + #[cfg(test)] + let val = ""; + #[cfg(not(test))] + let val = "foo"; + + let _ = val.is_empty(); + // Do not lint, value depend on a #[cfg(…)] directive +} + +fn not_cfg_conditioned() { + let val = ""; + #[cfg(not(target_os = "inexistent"))] + let _ = val.is_empty(); + //~^ ERROR: this expression always evaluates to true +} + +const fn const_rand() -> &'static str { + "17" +} + +fn const_expressions() { + let _ = const { if true { "1" } else { "2" } }.is_empty(); + // Do not lint, we do not recurse into boolean expressions + + let _ = const_rand().is_empty(); + // Do not lint, we do not recurse into functions +} diff --git a/tests/ui/const_is_empty.stderr b/tests/ui/const_is_empty.stderr index ef5aa556e6a..0e09da77bb4 100644 --- a/tests/ui/const_is_empty.stderr +++ b/tests/ui/const_is_empty.stderr @@ -1,5 +1,5 @@ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:4:8 + --> tests/ui/const_is_empty.rs:6:8 | LL | if "".is_empty() { | ^^^^^^^^^^^^^ @@ -8,148 +8,154 @@ LL | if "".is_empty() { = help: to override `-D warnings` add `#[allow(clippy::const_is_empty)]` error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:7:8 + --> tests/ui/const_is_empty.rs:9:8 | LL | if "foobar".is_empty() { | ^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:13:8 + --> tests/ui/const_is_empty.rs:15:8 | LL | if b"".is_empty() { | ^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:16:8 + --> tests/ui/const_is_empty.rs:18:8 | LL | if b"foobar".is_empty() { | ^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:33:8 + --> tests/ui/const_is_empty.rs:35:8 | LL | if empty2.is_empty() { | ^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:36:8 + --> tests/ui/const_is_empty.rs:38:8 | LL | if non_empty2.is_empty() { | ^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:58:13 + --> tests/ui/const_is_empty.rs:60:13 | LL | let _ = EMPTY_STR.is_empty(); | ^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:60:13 + --> tests/ui/const_is_empty.rs:62:13 | LL | let _ = NON_EMPTY_STR.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:62:13 + --> tests/ui/const_is_empty.rs:64:13 | LL | let _ = EMPTY_BSTR.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:64:13 + --> tests/ui/const_is_empty.rs:66:13 | LL | let _ = NON_EMPTY_BSTR.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:66:13 + --> tests/ui/const_is_empty.rs:68:13 | LL | let _ = EMPTY_ARRAY.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:68:13 + --> tests/ui/const_is_empty.rs:70:13 | LL | let _ = EMPTY_ARRAY_REPEAT.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:70:13 + --> tests/ui/const_is_empty.rs:72:13 | LL | let _ = EMPTY_U8_SLICE.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:72:13 + --> tests/ui/const_is_empty.rs:74:13 | LL | let _ = NON_EMPTY_U8_SLICE.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:74:13 + --> tests/ui/const_is_empty.rs:76:13 | LL | let _ = NON_EMPTY_ARRAY.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:76:13 + --> tests/ui/const_is_empty.rs:78:13 | LL | let _ = NON_EMPTY_ARRAY_REPEAT.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:78:13 + --> tests/ui/const_is_empty.rs:80:13 | LL | let _ = EMPTY_REF_ARRAY.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:80:13 + --> tests/ui/const_is_empty.rs:82:13 | LL | let _ = NON_EMPTY_REF_ARRAY.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:82:13 + --> tests/ui/const_is_empty.rs:84:13 | LL | let _ = EMPTY_SLICE.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:84:13 + --> tests/ui/const_is_empty.rs:86:13 | LL | let _ = NON_EMPTY_SLICE.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:86:13 + --> tests/ui/const_is_empty.rs:88:13 | LL | let _ = NON_EMPTY_SLICE_REPEAT.is_empty(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:92:13 + --> tests/ui/const_is_empty.rs:94:13 | LL | let _ = value.is_empty(); | ^^^^^^^^^^^^^^^^ error: this expression always evaluates to false - --> tests/ui/const_is_empty.rs:95:13 + --> tests/ui/const_is_empty.rs:97:13 | LL | let _ = x.is_empty(); | ^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:97:13 + --> tests/ui/const_is_empty.rs:99:13 | LL | let _ = "".is_empty(); | ^^^^^^^^^^^^^ error: this expression always evaluates to true - --> tests/ui/const_is_empty.rs:99:13 + --> tests/ui/const_is_empty.rs:101:13 | LL | let _ = b"".is_empty(); | ^^^^^^^^^^^^^^ -error: aborting due to 25 previous errors +error: this expression always evaluates to true + --> tests/ui/const_is_empty.rs:155:13 + | +LL | let _ = val.is_empty(); + | ^^^^^^^^^^^^^^ + +error: aborting due to 26 previous errors From 898ed8825d610d4f441ccb7c84c770261ce45ded Mon Sep 17 00:00:00 2001 From: Samuel Tardieu Date: Mon, 26 Feb 2024 09:47:13 +0100 Subject: [PATCH 006/278] feat: make `const_is_empty` lint ignore external constants --- clippy_utils/src/consts.rs | 11 +++++++++++ tests/ui/const_is_empty.rs | 5 +++++ 2 files changed, 16 insertions(+) diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs index 774f3f99d46..a4cb4ee93ca 100644 --- a/clippy_utils/src/consts.rs +++ b/clippy_utils/src/consts.rs @@ -13,6 +13,7 @@ use rustc_middle::mir::interpret::{alloc_range, Scalar}; use rustc_middle::mir::ConstValue; use rustc_middle::ty::{self, EarlyBinder, FloatTy, GenericArgsRef, IntTy, List, ScalarInt, Ty, TyCtxt, UintTy}; use rustc_middle::{bug, mir, span_bug}; +use rustc_span::def_id::DefId; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::SyntaxContext; use rustc_target::abi::Size; @@ -482,11 +483,21 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> { } /// Simple constant folding to determine if an expression is an empty slice, str, array, … + /// `None` will be returned if the constness cannot be determined, or if the resolution + /// leaves the local crate. pub fn expr_is_empty(&mut self, e: &Expr<'_>) -> Option { match e.kind { ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr_is_empty(self.lcx.tcx.hir().body(body).value), ExprKind::DropTemps(e) => self.expr_is_empty(e), ExprKind::Path(ref qpath) => { + if !self + .typeck_results + .qpath_res(qpath, e.hir_id) + .opt_def_id() + .is_some_and(DefId::is_local) + { + return None; + } self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| { mir_is_empty(this.lcx, result) }) diff --git a/tests/ui/const_is_empty.rs b/tests/ui/const_is_empty.rs index df7817e4b5f..ae37a82e4f9 100644 --- a/tests/ui/const_is_empty.rs +++ b/tests/ui/const_is_empty.rs @@ -167,3 +167,8 @@ fn const_expressions() { let _ = const_rand().is_empty(); // Do not lint, we do not recurse into functions } + +fn constant_from_external_crate() { + let _ = std::env::consts::EXE_EXTENSION.is_empty(); + // Do not lint, `exe_ext` comes from the `std` crate +} From 301d293ef6cc1aeb341b31a5339f7d857a38b930 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Mon, 4 Mar 2024 16:38:29 +0000 Subject: [PATCH 007/278] Move `iter_nth` to `style`, add machine applicable suggestion --- clippy_lints/src/methods/iter_nth.rs | 49 +++++++++++++---------- clippy_lints/src/methods/mod.rs | 16 ++++---- tests/ui/iter_nth.fixed | 60 ++++++++++++++++++++++++++++ tests/ui/iter_nth.rs | 3 ++ tests/ui/iter_nth.stderr | 48 ++++++++++++++++++---- 5 files changed, 139 insertions(+), 37 deletions(-) create mode 100644 tests/ui/iter_nth.fixed diff --git a/clippy_lints/src/methods/iter_nth.rs b/clippy_lints/src/methods/iter_nth.rs index 12104310405..5b0b70b4b96 100644 --- a/clippy_lints/src/methods/iter_nth.rs +++ b/clippy_lints/src/methods/iter_nth.rs @@ -1,10 +1,10 @@ -use super::utils::derefs_to_slice; -use crate::methods::iter_nth_zero; -use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::ty::get_type_diagnostic_name; +use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_span::symbol::sym; +use rustc_span::Span; use super::ITER_NTH; @@ -12,28 +12,33 @@ pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, expr: &hir::Expr<'_>, iter_recv: &'tcx hir::Expr<'tcx>, - nth_recv: &hir::Expr<'_>, - nth_arg: &hir::Expr<'_>, - is_mut: bool, -) { - let mut_str = if is_mut { "_mut" } else { "" }; - let caller_type = if derefs_to_slice(cx, iter_recv, cx.typeck_results().expr_ty(iter_recv)).is_some() { - "slice" - } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::Vec) { - "`Vec`" - } else if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(iter_recv), sym::VecDeque) { - "`VecDeque`" - } else { - iter_nth_zero::check(cx, expr, nth_recv, nth_arg); - return; // caller is not a type that we want to lint + iter_method: &str, + iter_span: Span, + nth_span: Span, +) -> bool { + let caller_type = match get_type_diagnostic_name(cx, cx.typeck_results().expr_ty(iter_recv).peel_refs()) { + Some(sym::Vec) => "`Vec`", + Some(sym::VecDeque) => "`VecDeque`", + _ if cx.typeck_results().expr_ty_adjusted(iter_recv).peel_refs().is_slice() => "slice", + // caller is not a type that we want to lint + _ => return false, }; - span_lint_and_help( + span_lint_and_then( cx, ITER_NTH, expr.span, - &format!("called `.iter{mut_str}().nth()` on a {caller_type}"), - None, - &format!("calling `.get{mut_str}()` is both faster and more readable"), + &format!("called `.{iter_method}().nth()` on a {caller_type}"), + |diag| { + let get_method = if iter_method == "iter_mut" { "get_mut" } else { "get" }; + diag.span_suggestion_verbose( + iter_span.to(nth_span), + format!("`{get_method}` is equivalent but more concise"), + get_method, + Applicability::MachineApplicable, + ); + }, ); + + true } diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 8a24ccea3a1..5445391c035 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1235,12 +1235,11 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for usage of `.iter().nth()` (and the related - /// `.iter_mut().nth()`) on standard library types with *O*(1) element access. + /// Checks for usage of `.iter().nth()`/`.iter_mut().nth()` on standard library types that have + /// equivalent `.get()`/`.get_mut()` methods. /// /// ### Why is this bad? - /// `.get()` and `.get_mut()` are more efficient and more - /// readable. + /// `.get()` and `.get_mut()` are equivalent but more concise. /// /// ### Example /// ```no_run @@ -1256,7 +1255,7 @@ declare_clippy_lint! { /// ``` #[clippy::version = "pre 1.29.0"] pub ITER_NTH, - perf, + style, "using `.iter().nth()` on a standard library type with O(1) element access" } @@ -4723,8 +4722,11 @@ impl Methods { iter_overeager_cloned::Op::LaterCloned, false, ), - Some(("iter", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, false), - Some(("iter_mut", recv2, [], _, _)) => iter_nth::check(cx, expr, recv2, recv, n_arg, true), + Some((iter_method @ ("iter" | "iter_mut"), iter_recv, [], iter_span, _)) => { + if !iter_nth::check(cx, expr, iter_recv, iter_method, iter_span, span) { + iter_nth_zero::check(cx, expr, recv, n_arg); + } + }, _ => iter_nth_zero::check(cx, expr, recv, n_arg), }, ("ok_or_else", [arg]) => unnecessary_lazy_eval::check(cx, expr, recv, arg, "ok_or"), diff --git a/tests/ui/iter_nth.fixed b/tests/ui/iter_nth.fixed new file mode 100644 index 00000000000..aff3731a883 --- /dev/null +++ b/tests/ui/iter_nth.fixed @@ -0,0 +1,60 @@ +//@aux-build:option_helpers.rs + +#![warn(clippy::iter_nth)] +#![allow(clippy::useless_vec)] + +#[macro_use] +extern crate option_helpers; + +use option_helpers::IteratorFalsePositives; +use std::collections::VecDeque; + +/// Struct to generate false positives for things with `.iter()`. +#[derive(Copy, Clone)] +struct HasIter; + +impl HasIter { + fn iter(self) -> IteratorFalsePositives { + IteratorFalsePositives { foo: 0 } + } + + fn iter_mut(self) -> IteratorFalsePositives { + IteratorFalsePositives { foo: 0 } + } +} + +/// Checks implementation of `ITER_NTH` lint. +fn iter_nth() { + let mut some_vec = vec![0, 1, 2, 3]; + let mut boxed_slice: Box<[u8]> = Box::new([0, 1, 2, 3]); + let mut some_vec_deque: VecDeque<_> = some_vec.iter().cloned().collect(); + + { + // Make sure we lint `.iter()` for relevant types. + let bad_vec = some_vec.get(3); + let bad_slice = &some_vec[..].get(3); + let bad_boxed_slice = boxed_slice.get(3); + let bad_vec_deque = some_vec_deque.get(3); + } + + { + // Make sure we lint `.iter_mut()` for relevant types. + let bad_vec = some_vec.get_mut(3); + } + { + let bad_slice = &some_vec[..].get_mut(3); + } + { + let bad_vec_deque = some_vec_deque.get_mut(3); + } + + let vec_ref = &Vec::::new(); + vec_ref.get(3); + + // Make sure we don't lint for non-relevant types. + let false_positive = HasIter; + let ok = false_positive.iter().nth(3); + let ok_mut = false_positive.iter_mut().nth(3); +} + +fn main() {} diff --git a/tests/ui/iter_nth.rs b/tests/ui/iter_nth.rs index 7c567bb81d8..89d68044ddd 100644 --- a/tests/ui/iter_nth.rs +++ b/tests/ui/iter_nth.rs @@ -48,6 +48,9 @@ fn iter_nth() { let bad_vec_deque = some_vec_deque.iter_mut().nth(3); } + let vec_ref = &Vec::::new(); + vec_ref.iter().nth(3); + // Make sure we don't lint for non-relevant types. let false_positive = HasIter; let ok = false_positive.iter().nth(3); diff --git a/tests/ui/iter_nth.stderr b/tests/ui/iter_nth.stderr index c5dd0c99727..178463f5347 100644 --- a/tests/ui/iter_nth.stderr +++ b/tests/ui/iter_nth.stderr @@ -4,9 +4,12 @@ error: called `.iter().nth()` on a `Vec` LL | let bad_vec = some_vec.iter().nth(3); | ^^^^^^^^^^^^^^^^^^^^^^ | - = help: calling `.get()` is both faster and more readable = note: `-D clippy::iter-nth` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::iter_nth)]` +help: `get` is equivalent but more concise + | +LL | let bad_vec = some_vec.get(3); + | ~~~ error: called `.iter().nth()` on a slice --> tests/ui/iter_nth.rs:35:26 @@ -14,7 +17,10 @@ error: called `.iter().nth()` on a slice LL | let bad_slice = &some_vec[..].iter().nth(3); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: calling `.get()` is both faster and more readable +help: `get` is equivalent but more concise + | +LL | let bad_slice = &some_vec[..].get(3); + | ~~~ error: called `.iter().nth()` on a slice --> tests/ui/iter_nth.rs:36:31 @@ -22,7 +28,10 @@ error: called `.iter().nth()` on a slice LL | let bad_boxed_slice = boxed_slice.iter().nth(3); | ^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: calling `.get()` is both faster and more readable +help: `get` is equivalent but more concise + | +LL | let bad_boxed_slice = boxed_slice.get(3); + | ~~~ error: called `.iter().nth()` on a `VecDeque` --> tests/ui/iter_nth.rs:37:29 @@ -30,7 +39,10 @@ error: called `.iter().nth()` on a `VecDeque` LL | let bad_vec_deque = some_vec_deque.iter().nth(3); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: calling `.get()` is both faster and more readable +help: `get` is equivalent but more concise + | +LL | let bad_vec_deque = some_vec_deque.get(3); + | ~~~ error: called `.iter_mut().nth()` on a `Vec` --> tests/ui/iter_nth.rs:42:23 @@ -38,7 +50,10 @@ error: called `.iter_mut().nth()` on a `Vec` LL | let bad_vec = some_vec.iter_mut().nth(3); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: calling `.get_mut()` is both faster and more readable +help: `get_mut` is equivalent but more concise + | +LL | let bad_vec = some_vec.get_mut(3); + | ~~~~~~~ error: called `.iter_mut().nth()` on a slice --> tests/ui/iter_nth.rs:45:26 @@ -46,7 +61,10 @@ error: called `.iter_mut().nth()` on a slice LL | let bad_slice = &some_vec[..].iter_mut().nth(3); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: calling `.get_mut()` is both faster and more readable +help: `get_mut` is equivalent but more concise + | +LL | let bad_slice = &some_vec[..].get_mut(3); + | ~~~~~~~ error: called `.iter_mut().nth()` on a `VecDeque` --> tests/ui/iter_nth.rs:48:29 @@ -54,7 +72,21 @@ error: called `.iter_mut().nth()` on a `VecDeque` LL | let bad_vec_deque = some_vec_deque.iter_mut().nth(3); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: calling `.get_mut()` is both faster and more readable +help: `get_mut` is equivalent but more concise + | +LL | let bad_vec_deque = some_vec_deque.get_mut(3); + | ~~~~~~~ -error: aborting due to 7 previous errors +error: called `.iter().nth()` on a `Vec` + --> tests/ui/iter_nth.rs:52:5 + | +LL | vec_ref.iter().nth(3); + | ^^^^^^^^^^^^^^^^^^^^^ + | +help: `get` is equivalent but more concise + | +LL | vec_ref.get(3); + | ~~~ + +error: aborting due to 8 previous errors From 68a58f255ac7b7487769667fdd5fe2536f952c15 Mon Sep 17 00:00:00 2001 From: Ross Smyth Date: Thu, 15 Feb 2024 19:54:35 -0500 Subject: [PATCH 008/278] Add postfix-match experimental feature Co-authored-by: Josh Stone --- compiler/rustc_ast_passes/src/feature_gate.rs | 1 + compiler/rustc_feature/src/unstable.rs | 2 + compiler/rustc_parse/src/parser/expr.rs | 19 +++++- compiler/rustc_span/src/symbol.rs | 1 + .../src/language-features/postfix-match.md | 22 +++++++ .../feature-gate-postfix_match.rs | 17 +++++ .../feature-gate-postfix_match.stderr | 23 +++++++ .../ui/match/postfix-match/pf-match-chain.rs | 16 +++++ .../postfix-match/pf-match-exhaustiveness.rs | 7 +++ .../pf-match-exhaustiveness.stderr | 21 +++++++ .../ui/match/postfix-match/pf-match-types.rs | 15 +++++ .../match/postfix-match/pf-match-types.stderr | 21 +++++++ tests/ui/match/postfix-match/postfix-match.rs | 62 +++++++++++++++++++ 13 files changed, 226 insertions(+), 1 deletion(-) create mode 100644 src/doc/unstable-book/src/language-features/postfix-match.md create mode 100644 tests/ui/feature-gates/feature-gate-postfix_match.rs create mode 100644 tests/ui/feature-gates/feature-gate-postfix_match.stderr create mode 100644 tests/ui/match/postfix-match/pf-match-chain.rs create mode 100644 tests/ui/match/postfix-match/pf-match-exhaustiveness.rs create mode 100644 tests/ui/match/postfix-match/pf-match-exhaustiveness.stderr create mode 100644 tests/ui/match/postfix-match/pf-match-types.rs create mode 100644 tests/ui/match/postfix-match/pf-match-types.stderr create mode 100644 tests/ui/match/postfix-match/postfix-match.rs diff --git a/compiler/rustc_ast_passes/src/feature_gate.rs b/compiler/rustc_ast_passes/src/feature_gate.rs index a28fcb00779..296f237763e 100644 --- a/compiler/rustc_ast_passes/src/feature_gate.rs +++ b/compiler/rustc_ast_passes/src/feature_gate.rs @@ -565,6 +565,7 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session, features: &Features) { gate_all!(generic_const_items, "generic const items are experimental"); gate_all!(unnamed_fields, "unnamed fields are not yet fully implemented"); gate_all!(fn_delegation, "functions delegation is not yet fully implemented"); + gate_all!(postfix_match, "postfix match is experimental"); if !visitor.features.never_patterns { if let Some(spans) = spans.get(&sym::never_patterns) { diff --git a/compiler/rustc_feature/src/unstable.rs b/compiler/rustc_feature/src/unstable.rs index 17c4d81474e..89f471d21bb 100644 --- a/compiler/rustc_feature/src/unstable.rs +++ b/compiler/rustc_feature/src/unstable.rs @@ -555,6 +555,8 @@ declare_features! ( (unstable, offset_of_nested, "1.77.0", Some(120140)), /// Allows using `#[optimize(X)]`. (unstable, optimize_attribute, "1.34.0", Some(54882)), + /// Allows postfix match `expr.match { ... }` + (unstable, postfix_match, "CURRENT_RUSTC_VERSION", Some(121618)), /// Allows macro attributes on expressions, statements and non-inline modules. (unstable, proc_macro_hygiene, "1.30.0", Some(54727)), /// Allows `&raw const $place_expr` and `&raw mut $place_expr` expressions. diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 6cc358db9fc..02ff452473d 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1375,6 +1375,12 @@ impl<'a> Parser<'a> { return Ok(self.mk_await_expr(self_arg, lo)); } + if self.eat_keyword(kw::Match) { + let match_span = self.prev_token.span; + self.psess.gated_spans.gate(sym::postfix_match, match_span); + return self.parse_match_block(lo, match_span, self_arg); + } + let fn_span_lo = self.token.span; let mut seg = self.parse_path_segment(PathStyle::Expr, None)?; self.check_trailing_angle_brackets(&seg, &[&token::OpenDelim(Delimiter::Parenthesis)]); @@ -2889,8 +2895,19 @@ impl<'a> Parser<'a> { /// Parses a `match ... { ... }` expression (`match` token already eaten). fn parse_expr_match(&mut self) -> PResult<'a, P> { let match_span = self.prev_token.span; - let lo = self.prev_token.span; let scrutinee = self.parse_expr_res(Restrictions::NO_STRUCT_LITERAL, None)?; + + self.parse_match_block(match_span, match_span, scrutinee) + } + + /// Parses a `match expr { ... }` or a `expr.match { ... }` expression. + /// This is after the match token and scrutinee are eaten + fn parse_match_block( + &mut self, + lo: Span, + match_span: Span, + scrutinee: P, + ) -> PResult<'a, P> { if let Err(mut e) = self.expect(&token::OpenDelim(Delimiter::Brace)) { if self.token == token::Semi { e.span_suggestion_short( diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 3784a08b1b7..2474189fef6 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1319,6 +1319,7 @@ symbols! { poll, poll_next, post_dash_lto: "post-lto", + postfix_match, powerpc_target_feature, powf128, powf16, diff --git a/src/doc/unstable-book/src/language-features/postfix-match.md b/src/doc/unstable-book/src/language-features/postfix-match.md new file mode 100644 index 00000000000..cd6b6a7442c --- /dev/null +++ b/src/doc/unstable-book/src/language-features/postfix-match.md @@ -0,0 +1,22 @@ +# `postfix-match` + +`postfix-match` adds the feature for matching upon values postfix +the expressions that generate the values. + +```rust,edition2021 +#![feature(postfix_match)] + +enum Foo { + Bar, + Baz +} + +fn get_foo() -> Foo { + Foo::Bar +} + +get_foo().match { + Foo::Bar => {}, + Foo::Baz => panic!(), +} +``` diff --git a/tests/ui/feature-gates/feature-gate-postfix_match.rs b/tests/ui/feature-gates/feature-gate-postfix_match.rs new file mode 100644 index 00000000000..dce7e81a9ae --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-postfix_match.rs @@ -0,0 +1,17 @@ +// Testing that postfix match doesn't work without enabling the feature + +fn main() { + let val = Some(42); + + val.match { //~ ERROR postfix match is experimental + Some(42) => "the answer to life, the universe, and everything", + _ => "might be the answer to something" + }; + + // Test that the gate works behind a cfg + #[cfg(FALSE)] + val.match { //~ ERROR postfix match is experimental + Some(42) => "the answer to life, the universe, and everything", + _ => "might be the answer to something" + }; +} diff --git a/tests/ui/feature-gates/feature-gate-postfix_match.stderr b/tests/ui/feature-gates/feature-gate-postfix_match.stderr new file mode 100644 index 00000000000..136838788dd --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-postfix_match.stderr @@ -0,0 +1,23 @@ +error[E0658]: postfix match is experimental + --> $DIR/feature-gate-postfix_match.rs:6:9 + | +LL | val.match { + | ^^^^^ + | + = note: see issue #121618 for more information + = help: add `#![feature(postfix_match)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error[E0658]: postfix match is experimental + --> $DIR/feature-gate-postfix_match.rs:13:9 + | +LL | val.match { + | ^^^^^ + | + = note: see issue #121618 for more information + = help: add `#![feature(postfix_match)]` to the crate attributes to enable + = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0658`. diff --git a/tests/ui/match/postfix-match/pf-match-chain.rs b/tests/ui/match/postfix-match/pf-match-chain.rs new file mode 100644 index 00000000000..80546e1963b --- /dev/null +++ b/tests/ui/match/postfix-match/pf-match-chain.rs @@ -0,0 +1,16 @@ +//@ run-pass + +#![feature(postfix_match)] + +fn main() { + 1.match { + 2 => Some(0), + _ => None, + }.match { + None => Ok(true), + Some(_) => Err("nope") + }.match { + Ok(_) => (), + Err(_) => panic!() + } +} diff --git a/tests/ui/match/postfix-match/pf-match-exhaustiveness.rs b/tests/ui/match/postfix-match/pf-match-exhaustiveness.rs new file mode 100644 index 00000000000..f4cac46f7cd --- /dev/null +++ b/tests/ui/match/postfix-match/pf-match-exhaustiveness.rs @@ -0,0 +1,7 @@ +#![feature(postfix_match)] + +fn main() { + Some(1).match { //~ non-exhaustive patterns + None => {}, + } +} diff --git a/tests/ui/match/postfix-match/pf-match-exhaustiveness.stderr b/tests/ui/match/postfix-match/pf-match-exhaustiveness.stderr new file mode 100644 index 00000000000..f458218bb5d --- /dev/null +++ b/tests/ui/match/postfix-match/pf-match-exhaustiveness.stderr @@ -0,0 +1,21 @@ +error[E0004]: non-exhaustive patterns: `Some(_)` not covered + --> $DIR/pf-match-exhaustiveness.rs:4:5 + | +LL | Some(1).match { + | ^^^^^^^ pattern `Some(_)` not covered + | +note: `Option` defined here + --> $SRC_DIR/core/src/option.rs:LL:COL + ::: $SRC_DIR/core/src/option.rs:LL:COL + | + = note: not covered + = note: the matched value is of type `Option` +help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown + | +LL ~ None => {}, +LL ~ Some(_) => todo!(), + | + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0004`. diff --git a/tests/ui/match/postfix-match/pf-match-types.rs b/tests/ui/match/postfix-match/pf-match-types.rs new file mode 100644 index 00000000000..af205926fb6 --- /dev/null +++ b/tests/ui/match/postfix-match/pf-match-types.rs @@ -0,0 +1,15 @@ +#![feature(postfix_match)] + +fn main() { + Some(10).match { + //~^ NOTE `match` arms have incompatible types + Some(5) => false, + //~^ NOTE this is found to be of type `bool` + Some(2) => true, + //~^ NOTE this is found to be of type `bool` + None => (), + //~^ ERROR `match` arms have incompatible types + //~| NOTE expected `bool`, found `()` + _ => true + } +} diff --git a/tests/ui/match/postfix-match/pf-match-types.stderr b/tests/ui/match/postfix-match/pf-match-types.stderr new file mode 100644 index 00000000000..0cfc1363d5f --- /dev/null +++ b/tests/ui/match/postfix-match/pf-match-types.stderr @@ -0,0 +1,21 @@ +error[E0308]: `match` arms have incompatible types + --> $DIR/pf-match-types.rs:10:20 + | +LL | / Some(10).match { +LL | | +LL | | Some(5) => false, + | | ----- this is found to be of type `bool` +LL | | +LL | | Some(2) => true, + | | ---- this is found to be of type `bool` +LL | | +LL | | None => (), + | | ^^ expected `bool`, found `()` +... | +LL | | _ => true +LL | | } + | |_____- `match` arms have incompatible types + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/match/postfix-match/postfix-match.rs b/tests/ui/match/postfix-match/postfix-match.rs new file mode 100644 index 00000000000..03c4e8ab545 --- /dev/null +++ b/tests/ui/match/postfix-match/postfix-match.rs @@ -0,0 +1,62 @@ +//@ run-pass + +#![feature(postfix_match)] + +struct Bar { + foo: u8, + baz: u8, +} + +pub fn main() { + let thing = Some("thing"); + + thing.match { + Some("nothing") => {}, + Some(text) if text.eq_ignore_ascii_case("tapir") => {}, + Some("true") | Some("false") => {}, + Some("thing") => {}, + Some(_) => {}, + None => {} + }; + + let num = 2u8; + + num.match { + 0 => {}, + 1..=5 => {}, + _ => {}, + }; + + let slic = &[1, 2, 3, 4][..]; + + slic.match { + [1] => {}, + [2, _tail @ ..] => {}, + [1, _] => {}, + _ => {}, + }; + + slic[0].match { + 1 => 0, + i => i, + }; + + let out = (1, 2).match { + (1, 3) => 0, + (_, 1) => 0, + (1, i) => i, + _ => 3, + }; + assert!(out == 2); + + let strct = Bar { + foo: 3, + baz: 4 + }; + + strct.match { + Bar { foo: 1, .. } => {}, + Bar { baz: 2, .. } => {}, + _ => (), + }; +} From 78b3bf98c3425d139b42a6326701573ff98b1a1f Mon Sep 17 00:00:00 2001 From: Ross Smyth Date: Sat, 17 Feb 2024 12:43:54 -0500 Subject: [PATCH 009/278] Add MatchKind member to the Match expr for pretty printing & fmt --- compiler/rustc_ast/src/ast.rs | 11 +++++++++- compiler/rustc_ast/src/mut_visit.rs | 2 +- compiler/rustc_ast/src/visit.rs | 2 +- compiler/rustc_ast_lowering/src/expr.rs | 2 +- .../rustc_ast_pretty/src/pprust/state/expr.rs | 20 +++++++++++++----- .../src/assert/context.rs | 2 +- .../src/deriving/cmp/partial_ord.rs | 2 +- .../src/deriving/debug.rs | 3 +-- compiler/rustc_expand/src/build.rs | 4 ++-- compiler/rustc_lint/src/unused.rs | 4 ++-- compiler/rustc_parse/src/parser/expr.rs | 18 +++++++++------- .../clippy/clippy_lints/src/redundant_else.rs | 2 +- .../src/suspicious_operation_groupings.rs | 2 +- .../clippy/clippy_utils/src/ast_utils.rs | 2 +- src/tools/rustfmt/src/expr.rs | 8 +++---- src/tools/rustfmt/src/matches.rs | 4 +++- tests/pretty/postfix-match.rs | 21 +++++++++++++++++++ 17 files changed, 76 insertions(+), 33 deletions(-) create mode 100644 tests/pretty/postfix-match.rs diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 13b1a589d9b..6ffb3d65abc 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1436,7 +1436,7 @@ pub enum ExprKind { /// `'label: loop { block }` Loop(P, Option