user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

new manual_option_as_slice lint

Browse source
This commit is contained in:
Andre Bogus 2024-12-29 17:42:33 +01:00
parent 913592373d
commit 13be95ab11
10 changed files with 432 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -5764,6 +5764,7 @@ Released 2018-09-13
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
[`manual_ok_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_err
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
[`manual_option_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_option_as_slice
[`manual_pattern_char_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns

1
clippy_lints/src/declared_lints.rs
View file

@ -320,6 +320,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
crate::manual_let_else::MANUAL_LET_ELSE_INFO,
crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
crate::manual_option_as_slice::MANUAL_OPTION_AS_SLICE_INFO,
crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO,
crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
crate::manual_retain::MANUAL_RETAIN_INFO,

2
clippy_lints/src/lib.rs
View file

@ -217,6 +217,7 @@ mod manual_is_power_of_two;
mod manual_let_else;
mod manual_main_separator_str;
mod manual_non_exhaustive;
mod manual_option_as_slice;
mod manual_range_patterns;
mod manual_rem_euclid;
mod manual_retain;
@ -976,5 +977,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(unneeded_struct_pattern::UnneededStructPattern));
store.register_late_pass(|_| Box::<unnecessary_semicolon::UnnecessarySemicolon>::default());
store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf)));
store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf)));
// add lints here, do not remove this comment, it's used in `new_lint`
}

225
clippy_lints/src/manual_option_as_slice.rs Normal file
View file

@ -0,0 +1,225 @@
use clippy_config::Conf;
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
use clippy_utils::{is_none_arm, msrvs, peel_hir_expr_refs};
use rustc_errors::Applicability;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::{Arm, Expr, ExprKind, LangItem, Pat, PatKind, QPath, is_range_literal};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::impl_lint_pass;
use rustc_span::{Span, Symbol, sym};
declare_clippy_lint! {
/// ### What it does
/// This detects various manual reimplementations of `Option::as_slice`.
///
/// ### Why is this bad?
/// Those implementations are both more complex than calling `as_slice`
/// and unlike that incur a branch, pessimizing performance and leading
/// to more generated code.
///
/// ### Example
/// ```no_run
///# let opt = Some(1);
/// _ = opt.as_ref().map_or(&[][..], std::slice::from_ref);
/// _ = match opt.as_ref() {
/// Some(f) => std::slice::from_ref(f),
/// None => &[],
/// };
/// ```
/// Use instead:
/// ```no_run
///# let opt = Some(1);
/// _ = opt.as_slice();
/// _ = opt.as_slice();
/// ```
#[clippy::version = "1.85.0"]
pub MANUAL_OPTION_AS_SLICE,
complexity,
"manual `Option::as_slice`"
}
pub struct ManualOptionAsSlice {
msrv: msrvs::Msrv,
}
impl ManualOptionAsSlice {
pub fn new(conf: &Conf) -> Self {
Self {
msrv: conf.msrv.clone(),
}
}
}
impl_lint_pass!(ManualOptionAsSlice => [MANUAL_OPTION_AS_SLICE]);
impl LateLintPass<'_> for ManualOptionAsSlice {
extract_msrv_attr!(LateContext);
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
let span = expr.span;
if span.from_expansion()
|| !self.msrv.meets(if clippy_utils::is_in_const_context(cx) {
msrvs::CONST_OPTION_AS_SLICE
} else {
msrvs::OPTION_AS_SLICE
})
{
return;
}
match expr.kind {
ExprKind::Match(scrutinee, [arm1, arm2], _) => {
if is_none_arm(cx, arm2) && check_arms(cx, arm2, arm1)
|| is_none_arm(cx, arm1) && check_arms(cx, arm1, arm2)
{
check_as_ref(cx, scrutinee, span);
}
},
ExprKind::If(cond, then, Some(other)) => {
if let ExprKind::Let(let_expr) = cond.kind
&& let Some(binding) = extract_ident_from_some_pat(cx, let_expr.pat)
&& check_some_body(cx, binding, then)
&& is_empty_slice(cx, other.peel_blocks())
{
check_as_ref(cx, let_expr.init, span);
}
},
ExprKind::MethodCall(seg, callee, [], _) => {
if seg.ident.name.as_str() == "unwrap_or_default" {
check_map(cx, callee, span);
}
},
ExprKind::MethodCall(seg, callee, [or], _) => match seg.ident.name.as_str() {
"unwrap_or" => {
if is_empty_slice(cx, or) {
check_map(cx, callee, span);
}
},
"unwrap_or_else" => {
if returns_empty_slice(cx, or) {
check_map(cx, callee, span);
}
},
_ => {},
},
ExprKind::MethodCall(seg, callee, [or_else, map], _) => match seg.ident.name.as_str() {
"map_or" => {
if is_empty_slice(cx, or_else) && is_slice_from_ref(cx, map) {
check_as_ref(cx, callee, span);
}
},
"map_or_else" => {
if returns_empty_slice(cx, or_else) && is_slice_from_ref(cx, map) {
check_as_ref(cx, callee, span);
}
},
_ => {},
},
_ => {},
}
}
}
fn check_map(cx: &LateContext<'_>, map: &Expr<'_>, span: Span) {
if let ExprKind::MethodCall(seg, callee, [mapping], _) = map.kind
&& seg.ident.name == sym::map
&& is_slice_from_ref(cx, mapping)
{
check_as_ref(cx, callee, span);
}
}
fn check_as_ref(cx: &LateContext<'_>, expr: &Expr<'_>, span: Span) {
if let ExprKind::MethodCall(seg, callee, [], _) = expr.kind
&& seg.ident.name == sym::as_ref
&& let ty::Adt(adtdef, ..) = cx.typeck_results().expr_ty(callee).kind()
&& cx.tcx.is_diagnostic_item(sym::Option, adtdef.did())
{
if let Some(snippet) = clippy_utils::source::snippet_opt(cx, callee.span) {
span_lint_and_sugg(
cx,
MANUAL_OPTION_AS_SLICE,
span,
"use `Option::as_slice`",
"use",
format!("{snippet}.as_slice()"),
Applicability::MachineApplicable,
);
} else {
span_lint(cx, MANUAL_OPTION_AS_SLICE, span, "use `Option_as_slice`");
}
}
}
fn extract_ident_from_some_pat(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option<Symbol> {
if let PatKind::TupleStruct(QPath::Resolved(None, path), [binding], _) = pat.kind
&& let Res::Def(DefKind::Ctor(..), def_id) = path.res
&& let PatKind::Binding(_mode, _hir_id, ident, _inner_pat) = binding.kind
&& clippy_utils::is_lang_item_or_ctor(cx, def_id, LangItem::OptionSome)
{
Some(ident.name)
} else {
None
}
}
/// Returns true if `expr` is `std::slice::from_ref(<name>)`. Used in `if let`s.
fn check_some_body(cx: &LateContext<'_>, name: Symbol, expr: &Expr<'_>) -> bool {
if let ExprKind::Call(slice_from_ref, [arg]) = expr.peel_blocks().kind
&& is_slice_from_ref(cx, slice_from_ref)
&& let ExprKind::Path(QPath::Resolved(None, path)) = arg.kind
&& let [seg] = path.segments
{
seg.ident.name == name
} else {
false
}
}
fn check_arms(cx: &LateContext<'_>, none_arm: &Arm<'_>, some_arm: &Arm<'_>) -> bool {
if none_arm.guard.is_none()
&& some_arm.guard.is_none()
&& is_empty_slice(cx, none_arm.body)
&& let Some(name) = extract_ident_from_some_pat(cx, some_arm.pat)
{
check_some_body(cx, name, some_arm.body)
} else {
false
}
}
fn returns_empty_slice(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
match expr.kind {
ExprKind::Path(_) => clippy_utils::is_path_diagnostic_item(cx, expr, sym::default_fn),
ExprKind::Closure(cl) => is_empty_slice(cx, cx.tcx.hir().body(cl.body).value),
_ => false,
}
}
/// Returns if expr returns an empty slice. If:
/// - An indexing operation to an empty array with a built-in range. `[][..]`
/// - An indexing operation with a zero-ended range. `expr[..0]`
/// - A reference to an empty array. `&[]`
/// - Or a call to `Default::default`.
fn is_empty_slice(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
let expr = peel_hir_expr_refs(expr.peel_blocks()).0;
match expr.kind {
ExprKind::Index(arr, range, _) => match arr.kind {
ExprKind::Array([]) => is_range_literal(range),
ExprKind::Array(_) => {
let Some(range) = clippy_utils::higher::Range::hir(range) else {
return false;
};
range.end.is_some_and(|e| clippy_utils::is_integer_const(cx, e, 0))
},
_ => false,
},
ExprKind::Array([]) => true,
ExprKind::Call(def, []) => clippy_utils::is_path_diagnostic_item(cx, def, sym::default_fn),
_ => false,
}
}
fn is_slice_from_ref(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
clippy_utils::is_expr_path_def_path(cx, expr, &["core", "slice", "raw", "from_ref"])
}

10
clippy_lints/src/matches/match_as_ref.rs
View file

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{is_res_lang_ctor, path_res, peel_blocks};
use clippy_utils::{is_none_arm, is_res_lang_ctor, path_res, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::{Arm, BindingMode, ByRef, Expr, ExprKind, LangItem, Mutability, PatKind, QPath};
use rustc_lint::LateContext;
@ -55,14 +55,6 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
}
}
// Checks if arm has the form `None => None`
fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
matches!(
arm.pat.kind,
PatKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), LangItem::OptionNone)
)
}
// Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`)
fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option<Mutability> {
if let PatKind::TupleStruct(ref qpath, [first_pat, ..], _) = arm.pat.kind

8
clippy_utils/src/lib.rs
View file

@ -342,6 +342,14 @@ pub fn is_wild(pat: &Pat<'_>) -> bool {
matches!(pat.kind, PatKind::Wild)
}
// Checks if arm has the form `None => _`
pub fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
matches!(
arm.pat.kind,
PatKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id),OptionNone)
)
}
/// Checks if the given `QPath` belongs to a type alias.
pub fn is_ty_alias(qpath: &QPath<'_>) -> bool {
match *qpath {

3
clippy_utils/src/msrvs.rs
View file

@ -1,4 +1,5 @@
use rustc_ast::attr::AttributeExt;
use rustc_attr_parsing::{RustcVersion, parse_version};
use rustc_session::Session;
use rustc_span::{Symbol, sym};
@ -18,12 +19,14 @@ macro_rules! msrv_aliases {
// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,84,0 { CONST_OPTION_AS_SLICE }
1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP }
1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION }
1,80,0 { BOX_INTO_ITER, LAZY_CELL }
1,77,0 { C_STR_LITERALS }
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
1,75,0 { OPTION_AS_SLICE }
1,74,0 { REPR_RUST }
1,73,0 { MANUAL_DIV_CEIL }
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }

62
tests/ui/manual_option_as_slice.fixed Normal file
View file

@ -0,0 +1,62 @@
#![warn(clippy::manual_option_as_slice)]
#![allow(clippy::redundant_closure, clippy::unwrap_or_default)]
fn check(x: Option<u32>) {
_ = x.as_slice();
_ = x.as_slice();
_ = x.as_slice();
//~^ manual_option_as_slice
_ = x.as_slice();
//~^ manual_option_as_slice
_ = x.as_slice();
//~^ manual_option_as_slice
_ = x.as_slice();
//~^ manual_option_as_slice
{
use std::slice::from_ref;
_ = x.as_slice();
//~^ manual_option_as_slice
}
// possible false positives
let y = x.as_ref();
_ = match y {
// as_ref outside
Some(f) => &[f][..],
None => &[][..],
};
_ = match x.as_ref() {
Some(f) => std::slice::from_ref(f),
None => &[0],
};
_ = match x.as_ref() {
Some(42) => &[23],
Some(f) => std::slice::from_ref(f),
None => &[],
};
let b = &[42];
_ = if let Some(_f) = x.as_ref() {
std::slice::from_ref(b)
} else {
&[]
};
_ = x.as_ref().map_or(&[42][..], std::slice::from_ref);
_ = x.as_ref().map_or_else(|| &[42][..1], std::slice::from_ref);
_ = x.as_ref().map(|f| std::slice::from_ref(f)).unwrap_or_default();
}
#[clippy::msrv = "1.74"]
fn check_msrv(x: Option<u32>) {
_ = x.as_ref().map_or(&[][..], std::slice::from_ref);
}
fn main() {
check(Some(1));
check_msrv(Some(175));
}

71
tests/ui/manual_option_as_slice.rs Normal file
View file

@ -0,0 +1,71 @@
#![warn(clippy::manual_option_as_slice)]
#![allow(clippy::redundant_closure, clippy::unwrap_or_default)]
fn check(x: Option<u32>) {
_ = match x.as_ref() {
//~^ manual_option_as_slice
Some(f) => std::slice::from_ref(f),
None => &[],
};
_ = if let Some(f) = x.as_ref() {
//~^ manual_option_as_slice
std::slice::from_ref(f)
} else {
&[]
};
_ = x.as_ref().map_or(&[][..], std::slice::from_ref);
//~^ manual_option_as_slice
_ = x.as_ref().map_or_else(Default::default, std::slice::from_ref);
//~^ manual_option_as_slice
_ = x.as_ref().map(std::slice::from_ref).unwrap_or_default();
//~^ manual_option_as_slice
_ = x.as_ref().map_or_else(|| &[42][..0], std::slice::from_ref);
//~^ manual_option_as_slice
{
use std::slice::from_ref;
_ = x.as_ref().map_or_else(<&[_]>::default, from_ref);
//~^ manual_option_as_slice
}
// possible false positives
let y = x.as_ref();
_ = match y {
// as_ref outside
Some(f) => &[f][..],
None => &[][..],
};
_ = match x.as_ref() {
Some(f) => std::slice::from_ref(f),
None => &[0],
};
_ = match x.as_ref() {
Some(42) => &[23],
Some(f) => std::slice::from_ref(f),
None => &[],
};
let b = &[42];
_ = if let Some(_f) = x.as_ref() {
std::slice::from_ref(b)
} else {
&[]
};
_ = x.as_ref().map_or(&[42][..], std::slice::from_ref);
_ = x.as_ref().map_or_else(|| &[42][..1], std::slice::from_ref);
_ = x.as_ref().map(|f| std::slice::from_ref(f)).unwrap_or_default();
}
#[clippy::msrv = "1.74"]
fn check_msrv(x: Option<u32>) {
_ = x.as_ref().map_or(&[][..], std::slice::from_ref);
}
fn main() {
check(Some(1));
check_msrv(Some(175));
}

58
tests/ui/manual_option_as_slice.stderr Normal file
View file

@ -0,0 +1,58 @@
error: use `Option::as_slice`
--> tests/ui/manual_option_as_slice.rs:5:9
|
LL | _ = match x.as_ref() {
| _________^
LL | |
LL | | Some(f) => std::slice::from_ref(f),
LL | | None => &[],
LL | | };
| |_____^ help: use: `x.as_slice()`
|
= note: `-D clippy::manual-option-as-slice` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_option_as_slice)]`
error: use `Option::as_slice`
--> tests/ui/manual_option_as_slice.rs:11:9
|
LL | _ = if let Some(f) = x.as_ref() {
| _________^
LL | |
LL | | std::slice::from_ref(f)
LL | | } else {
LL | | &[]
LL | | };
| |_____^ help: use: `x.as_slice()`
error: use `Option::as_slice`
--> tests/ui/manual_option_as_slice.rs:18:9
|
LL | _ = x.as_ref().map_or(&[][..], std::slice::from_ref);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
error: use `Option::as_slice`
--> tests/ui/manual_option_as_slice.rs:21:9
|
LL | _ = x.as_ref().map_or_else(Default::default, std::slice::from_ref);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
error: use `Option::as_slice`
--> tests/ui/manual_option_as_slice.rs:24:9
|
LL | _ = x.as_ref().map(std::slice::from_ref).unwrap_or_default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
error: use `Option::as_slice`
--> tests/ui/manual_option_as_slice.rs:27:9
|
LL | _ = x.as_ref().map_or_else(|| &[42][..0], std::slice::from_ref);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
error: use `Option::as_slice`
--> tests/ui/manual_option_as_slice.rs:32:13
|
LL | _ = x.as_ref().map_or_else(<&[_]>::default, from_ref);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
error: aborting due to 7 previous errors