Skip to content

Commit c5218d5

Browse files
authored
new manual_option_as_slice lint (#13901)
Hey folks. It's been a while since I added the `as_slice` method to `Option`, and I totally forgot about a lint to suggest it. Well, I had some time around Christmas, so here it is now. --- changelog: add [`manual_option_as_slice`] lint
2 parents 2c51951 + 13be95a commit c5218d5

10 files changed

+432
-9
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5765,6 +5765,7 @@ Released 2018-09-13
57655765
[`manual_non_exhaustive`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive
57665766
[`manual_ok_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_err
57675767
[`manual_ok_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_ok_or
5768+
[`manual_option_as_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_option_as_slice
57685769
[`manual_pattern_char_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison
57695770
[`manual_range_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains
57705771
[`manual_range_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_patterns

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
322322
crate::manual_let_else::MANUAL_LET_ELSE_INFO,
323323
crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
324324
crate::manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE_INFO,
325+
crate::manual_option_as_slice::MANUAL_OPTION_AS_SLICE_INFO,
325326
crate::manual_range_patterns::MANUAL_RANGE_PATTERNS_INFO,
326327
crate::manual_rem_euclid::MANUAL_REM_EUCLID_INFO,
327328
crate::manual_retain::MANUAL_RETAIN_INFO,

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ mod manual_is_power_of_two;
217217
mod manual_let_else;
218218
mod manual_main_separator_str;
219219
mod manual_non_exhaustive;
220+
mod manual_option_as_slice;
220221
mod manual_range_patterns;
221222
mod manual_rem_euclid;
222223
mod manual_retain;
@@ -976,5 +977,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
976977
store.register_late_pass(|_| Box::new(unneeded_struct_pattern::UnneededStructPattern));
977978
store.register_late_pass(|_| Box::<unnecessary_semicolon::UnnecessarySemicolon>::default());
978979
store.register_late_pass(move |_| Box::new(non_std_lazy_statics::NonStdLazyStatic::new(conf)));
980+
store.register_late_pass(|_| Box::new(manual_option_as_slice::ManualOptionAsSlice::new(conf)));
979981
// add lints here, do not remove this comment, it's used in `new_lint`
980982
}
+225
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
use clippy_config::Conf;
2+
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
3+
use clippy_utils::{is_none_arm, msrvs, peel_hir_expr_refs};
4+
use rustc_errors::Applicability;
5+
use rustc_hir::def::{DefKind, Res};
6+
use rustc_hir::{Arm, Expr, ExprKind, LangItem, Pat, PatKind, QPath, is_range_literal};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_middle::ty;
9+
use rustc_session::impl_lint_pass;
10+
use rustc_span::{Span, Symbol, sym};
11+
12+
declare_clippy_lint! {
13+
/// ### What it does
14+
/// This detects various manual reimplementations of `Option::as_slice`.
15+
///
16+
/// ### Why is this bad?
17+
/// Those implementations are both more complex than calling `as_slice`
18+
/// and unlike that incur a branch, pessimizing performance and leading
19+
/// to more generated code.
20+
///
21+
/// ### Example
22+
/// ```no_run
23+
///# let opt = Some(1);
24+
/// _ = opt.as_ref().map_or(&[][..], std::slice::from_ref);
25+
/// _ = match opt.as_ref() {
26+
/// Some(f) => std::slice::from_ref(f),
27+
/// None => &[],
28+
/// };
29+
/// ```
30+
/// Use instead:
31+
/// ```no_run
32+
///# let opt = Some(1);
33+
/// _ = opt.as_slice();
34+
/// _ = opt.as_slice();
35+
/// ```
36+
#[clippy::version = "1.85.0"]
37+
pub MANUAL_OPTION_AS_SLICE,
38+
complexity,
39+
"manual `Option::as_slice`"
40+
}
41+
42+
pub struct ManualOptionAsSlice {
43+
msrv: msrvs::Msrv,
44+
}
45+
46+
impl ManualOptionAsSlice {
47+
pub fn new(conf: &Conf) -> Self {
48+
Self {
49+
msrv: conf.msrv.clone(),
50+
}
51+
}
52+
}
53+
54+
impl_lint_pass!(ManualOptionAsSlice => [MANUAL_OPTION_AS_SLICE]);
55+
56+
impl LateLintPass<'_> for ManualOptionAsSlice {
57+
extract_msrv_attr!(LateContext);
58+
59+
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
60+
let span = expr.span;
61+
if span.from_expansion()
62+
|| !self.msrv.meets(if clippy_utils::is_in_const_context(cx) {
63+
msrvs::CONST_OPTION_AS_SLICE
64+
} else {
65+
msrvs::OPTION_AS_SLICE
66+
})
67+
{
68+
return;
69+
}
70+
match expr.kind {
71+
ExprKind::Match(scrutinee, [arm1, arm2], _) => {
72+
if is_none_arm(cx, arm2) && check_arms(cx, arm2, arm1)
73+
|| is_none_arm(cx, arm1) && check_arms(cx, arm1, arm2)
74+
{
75+
check_as_ref(cx, scrutinee, span);
76+
}
77+
},
78+
ExprKind::If(cond, then, Some(other)) => {
79+
if let ExprKind::Let(let_expr) = cond.kind
80+
&& let Some(binding) = extract_ident_from_some_pat(cx, let_expr.pat)
81+
&& check_some_body(cx, binding, then)
82+
&& is_empty_slice(cx, other.peel_blocks())
83+
{
84+
check_as_ref(cx, let_expr.init, span);
85+
}
86+
},
87+
ExprKind::MethodCall(seg, callee, [], _) => {
88+
if seg.ident.name.as_str() == "unwrap_or_default" {
89+
check_map(cx, callee, span);
90+
}
91+
},
92+
ExprKind::MethodCall(seg, callee, [or], _) => match seg.ident.name.as_str() {
93+
"unwrap_or" => {
94+
if is_empty_slice(cx, or) {
95+
check_map(cx, callee, span);
96+
}
97+
},
98+
"unwrap_or_else" => {
99+
if returns_empty_slice(cx, or) {
100+
check_map(cx, callee, span);
101+
}
102+
},
103+
_ => {},
104+
},
105+
ExprKind::MethodCall(seg, callee, [or_else, map], _) => match seg.ident.name.as_str() {
106+
"map_or" => {
107+
if is_empty_slice(cx, or_else) && is_slice_from_ref(cx, map) {
108+
check_as_ref(cx, callee, span);
109+
}
110+
},
111+
"map_or_else" => {
112+
if returns_empty_slice(cx, or_else) && is_slice_from_ref(cx, map) {
113+
check_as_ref(cx, callee, span);
114+
}
115+
},
116+
_ => {},
117+
},
118+
_ => {},
119+
}
120+
}
121+
}
122+
123+
fn check_map(cx: &LateContext<'_>, map: &Expr<'_>, span: Span) {
124+
if let ExprKind::MethodCall(seg, callee, [mapping], _) = map.kind
125+
&& seg.ident.name == sym::map
126+
&& is_slice_from_ref(cx, mapping)
127+
{
128+
check_as_ref(cx, callee, span);
129+
}
130+
}
131+
132+
fn check_as_ref(cx: &LateContext<'_>, expr: &Expr<'_>, span: Span) {
133+
if let ExprKind::MethodCall(seg, callee, [], _) = expr.kind
134+
&& seg.ident.name == sym::as_ref
135+
&& let ty::Adt(adtdef, ..) = cx.typeck_results().expr_ty(callee).kind()
136+
&& cx.tcx.is_diagnostic_item(sym::Option, adtdef.did())
137+
{
138+
if let Some(snippet) = clippy_utils::source::snippet_opt(cx, callee.span) {
139+
span_lint_and_sugg(
140+
cx,
141+
MANUAL_OPTION_AS_SLICE,
142+
span,
143+
"use `Option::as_slice`",
144+
"use",
145+
format!("{snippet}.as_slice()"),
146+
Applicability::MachineApplicable,
147+
);
148+
} else {
149+
span_lint(cx, MANUAL_OPTION_AS_SLICE, span, "use `Option_as_slice`");
150+
}
151+
}
152+
}
153+
154+
fn extract_ident_from_some_pat(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option<Symbol> {
155+
if let PatKind::TupleStruct(QPath::Resolved(None, path), [binding], _) = pat.kind
156+
&& let Res::Def(DefKind::Ctor(..), def_id) = path.res
157+
&& let PatKind::Binding(_mode, _hir_id, ident, _inner_pat) = binding.kind
158+
&& clippy_utils::is_lang_item_or_ctor(cx, def_id, LangItem::OptionSome)
159+
{
160+
Some(ident.name)
161+
} else {
162+
None
163+
}
164+
}
165+
166+
/// Returns true if `expr` is `std::slice::from_ref(<name>)`. Used in `if let`s.
167+
fn check_some_body(cx: &LateContext<'_>, name: Symbol, expr: &Expr<'_>) -> bool {
168+
if let ExprKind::Call(slice_from_ref, [arg]) = expr.peel_blocks().kind
169+
&& is_slice_from_ref(cx, slice_from_ref)
170+
&& let ExprKind::Path(QPath::Resolved(None, path)) = arg.kind
171+
&& let [seg] = path.segments
172+
{
173+
seg.ident.name == name
174+
} else {
175+
false
176+
}
177+
}
178+
179+
fn check_arms(cx: &LateContext<'_>, none_arm: &Arm<'_>, some_arm: &Arm<'_>) -> bool {
180+
if none_arm.guard.is_none()
181+
&& some_arm.guard.is_none()
182+
&& is_empty_slice(cx, none_arm.body)
183+
&& let Some(name) = extract_ident_from_some_pat(cx, some_arm.pat)
184+
{
185+
check_some_body(cx, name, some_arm.body)
186+
} else {
187+
false
188+
}
189+
}
190+
191+
fn returns_empty_slice(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
192+
match expr.kind {
193+
ExprKind::Path(_) => clippy_utils::is_path_diagnostic_item(cx, expr, sym::default_fn),
194+
ExprKind::Closure(cl) => is_empty_slice(cx, cx.tcx.hir().body(cl.body).value),
195+
_ => false,
196+
}
197+
}
198+
199+
/// Returns if expr returns an empty slice. If:
200+
/// - An indexing operation to an empty array with a built-in range. `[][..]`
201+
/// - An indexing operation with a zero-ended range. `expr[..0]`
202+
/// - A reference to an empty array. `&[]`
203+
/// - Or a call to `Default::default`.
204+
fn is_empty_slice(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
205+
let expr = peel_hir_expr_refs(expr.peel_blocks()).0;
206+
match expr.kind {
207+
ExprKind::Index(arr, range, _) => match arr.kind {
208+
ExprKind::Array([]) => is_range_literal(range),
209+
ExprKind::Array(_) => {
210+
let Some(range) = clippy_utils::higher::Range::hir(range) else {
211+
return false;
212+
};
213+
range.end.is_some_and(|e| clippy_utils::is_integer_const(cx, e, 0))
214+
},
215+
_ => false,
216+
},
217+
ExprKind::Array([]) => true,
218+
ExprKind::Call(def, []) => clippy_utils::is_path_diagnostic_item(cx, def, sym::default_fn),
219+
_ => false,
220+
}
221+
}
222+
223+
fn is_slice_from_ref(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
224+
clippy_utils::is_expr_path_def_path(cx, expr, &["core", "slice", "raw", "from_ref"])
225+
}

clippy_lints/src/matches/match_as_ref.rs

+1-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_with_applicability;
3-
use clippy_utils::{is_res_lang_ctor, path_res, peel_blocks};
3+
use clippy_utils::{is_none_arm, is_res_lang_ctor, path_res, peel_blocks};
44
use rustc_errors::Applicability;
55
use rustc_hir::{Arm, BindingMode, ByRef, Expr, ExprKind, LangItem, Mutability, PatKind, QPath};
66
use rustc_lint::LateContext;
@@ -55,14 +55,6 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr:
5555
}
5656
}
5757

58-
// Checks if arm has the form `None => None`
59-
fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
60-
matches!(
61-
arm.pat.kind,
62-
PatKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), LangItem::OptionNone)
63-
)
64-
}
65-
6658
// Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`)
6759
fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option<Mutability> {
6860
if let PatKind::TupleStruct(ref qpath, [first_pat, ..], _) = arm.pat.kind

clippy_utils/src/lib.rs

+8
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,14 @@ pub fn is_wild(pat: &Pat<'_>) -> bool {
341341
matches!(pat.kind, PatKind::Wild)
342342
}
343343

344+
// Checks if arm has the form `None => _`
345+
pub fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
346+
matches!(
347+
arm.pat.kind,
348+
PatKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id),OptionNone)
349+
)
350+
}
351+
344352
/// Checks if the given `QPath` belongs to a type alias.
345353
pub fn is_ty_alias(qpath: &QPath<'_>) -> bool {
346354
match *qpath {

clippy_utils/src/msrvs.rs

+3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use rustc_ast::attr::AttributeExt;
2+
23
use rustc_attr_parsing::{RustcVersion, parse_version};
34
use rustc_session::Session;
45
use rustc_span::{Symbol, sym};
@@ -18,12 +19,14 @@ macro_rules! msrv_aliases {
1819

1920
// names may refer to stabilized feature flags or library items
2021
msrv_aliases! {
22+
1,84,0 { CONST_OPTION_AS_SLICE }
2123
1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP }
2224
1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
2325
1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION }
2426
1,80,0 { BOX_INTO_ITER, LAZY_CELL }
2527
1,77,0 { C_STR_LITERALS }
2628
1,76,0 { PTR_FROM_REF, OPTION_RESULT_INSPECT }
29+
1,75,0 { OPTION_AS_SLICE }
2730
1,74,0 { REPR_RUST }
2831
1,73,0 { MANUAL_DIV_CEIL }
2932
1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }

tests/ui/manual_option_as_slice.fixed

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
#![warn(clippy::manual_option_as_slice)]
2+
#![allow(clippy::redundant_closure, clippy::unwrap_or_default)]
3+
4+
fn check(x: Option<u32>) {
5+
_ = x.as_slice();
6+
7+
_ = x.as_slice();
8+
9+
_ = x.as_slice();
10+
//~^ manual_option_as_slice
11+
12+
_ = x.as_slice();
13+
//~^ manual_option_as_slice
14+
15+
_ = x.as_slice();
16+
//~^ manual_option_as_slice
17+
18+
_ = x.as_slice();
19+
//~^ manual_option_as_slice
20+
21+
{
22+
use std::slice::from_ref;
23+
_ = x.as_slice();
24+
//~^ manual_option_as_slice
25+
}
26+
27+
// possible false positives
28+
let y = x.as_ref();
29+
_ = match y {
30+
// as_ref outside
31+
Some(f) => &[f][..],
32+
None => &[][..],
33+
};
34+
_ = match x.as_ref() {
35+
Some(f) => std::slice::from_ref(f),
36+
None => &[0],
37+
};
38+
_ = match x.as_ref() {
39+
Some(42) => &[23],
40+
Some(f) => std::slice::from_ref(f),
41+
None => &[],
42+
};
43+
let b = &[42];
44+
_ = if let Some(_f) = x.as_ref() {
45+
std::slice::from_ref(b)
46+
} else {
47+
&[]
48+
};
49+
_ = x.as_ref().map_or(&[42][..], std::slice::from_ref);
50+
_ = x.as_ref().map_or_else(|| &[42][..1], std::slice::from_ref);
51+
_ = x.as_ref().map(|f| std::slice::from_ref(f)).unwrap_or_default();
52+
}
53+
54+
#[clippy::msrv = "1.74"]
55+
fn check_msrv(x: Option<u32>) {
56+
_ = x.as_ref().map_or(&[][..], std::slice::from_ref);
57+
}
58+
59+
fn main() {
60+
check(Some(1));
61+
check_msrv(Some(175));
62+
}

0 commit comments

Comments
 (0)