Skip to content

Commit fb82de5

Browse files
samueltardieusamueltardieu
authored
`manual_option_as_slice: improve diagnostics (#15926)
- Make the diagnostic message actually desribe the problem - Always give the suggestion, by using `snippet_with_context` - Make the suggestion verbose, because we sometimes lint multiline exprs like `match`es changelog: [`manual_option_as_slice`]: improve diagnostics
2 parents 039b40f + cb33ccd commit fb82de5

File tree

2 files changed

+84
-31
lines changed

2 files changed

+84
-31
lines changed

‎clippy_lints/src/manual_option_as_slice.rs‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
use clippy_config::Conf;
2-
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
2+
use clippy_utils::diagnostics::span_lint_and_then;
33
use clippy_utils::msrvs::Msrv;
44
use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath};
5+
use clippy_utils::source::snippet_with_context;
56
use clippy_utils::{is_none_pattern, msrvs, peel_hir_expr_refs, sym};
67
use rustc_errors::Applicability;
78
use rustc_hir::def::{DefKind, Res};
89
use rustc_hir::{Arm, Expr, ExprKind, LangItem, Pat, PatKind, QPath, is_range_literal};
910
use rustc_lint::{LateContext, LateLintPass};
10-
use rustc_middle::ty;
1111
use rustc_session::impl_lint_pass;
1212
use rustc_span::{Span, Symbol};
1313

@@ -124,8 +124,7 @@ fn check_map(cx: &LateContext<'_>, map: &Expr<'_>, span: Span, msrv: Msrv) {
124124
fn check_as_ref(cx: &LateContext<'_>, expr: &Expr<'_>, span: Span, msrv: Msrv) {
125125
if let ExprKind::MethodCall(seg, callee, [], _) = expr.kind
126126
&& seg.ident.name == sym::as_ref
127-
&& let ty::Adt(adtdef, ..) = cx.typeck_results().expr_ty(callee).kind()
128-
&& cx.tcx.is_diagnostic_item(sym::Option, adtdef.did())
127+
&& cx.typeck_results().expr_ty(callee).is_diag_item(cx, sym::Option)
129128
&& msrv.meets(
130129
cx,
131130
if clippy_utils::is_in_const_context(cx) {
@@ -135,19 +134,22 @@ fn check_as_ref(cx: &LateContext<'_>, expr: &Expr<'_>, span: Span, msrv: Msrv) {
135134
},
136135
)
137136
{
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-
}
137+
span_lint_and_then(
138+
cx,
139+
MANUAL_OPTION_AS_SLICE,
140+
span,
141+
"manual implementation of `Option::as_slice`",
142+
|diag| {
143+
let mut app = Applicability::MachineApplicable;
144+
let callee = snippet_with_context(cx, callee.span, expr.span.ctxt(), "_", &mut app).0;
145+
diag.span_suggestion_verbose(
146+
span,
147+
"use `Option::as_slice` directly",
148+
format!("{callee}.as_slice()"),
149+
app,
150+
);
151+
},
152+
);
151153
}
152154
}
153155

‎tests/ui/manual_option_as_slice.stderr‎

Lines changed: 65 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
error: use `Option::as_slice`
1+
error: manual implementation of `Option::as_slice`
22
--> tests/ui/manual_option_as_slice.rs:5:9
33
|
44
LL | _ = match x.as_ref() {
@@ -7,12 +7,21 @@ LL | |
77
LL | | Some(f) => std::slice::from_ref(f),
88
LL | | None => &[],
99
LL | | };
10-
| |_____^ help: use: `x.as_slice()`
10+
| |_____^
1111
|
1212
= note: `-D clippy::manual-option-as-slice` implied by `-D warnings`
1313
= help: to override `-D warnings` add `#[allow(clippy::manual_option_as_slice)]`
14+
help: use `Option::as_slice` directly
15+
|
16+
LL - _ = match x.as_ref() {
17+
LL -
18+
LL - Some(f) => std::slice::from_ref(f),
19+
LL - None => &[],
20+
LL - };
21+
LL + _ = x.as_slice();
22+
|
1423

15-
error: use `Option::as_slice`
24+
error: manual implementation of `Option::as_slice`
1625
--> tests/ui/manual_option_as_slice.rs:11:9
1726
|
1827
LL | _ = if let Some(f) = x.as_ref() {
@@ -23,37 +32,79 @@ LL | | std::slice::from_ref(f)
2332
LL | | } else {
2433
LL | | &[]
2534
LL | | };
26-
| |_____^ help: use: `x.as_slice()`
35+
| |_____^
36+
|
37+
help: use `Option::as_slice` directly
38+
|
39+
LL - _ = if let Some(f) = x.as_ref() {
40+
LL -
41+
LL -
42+
LL - std::slice::from_ref(f)
43+
LL - } else {
44+
LL - &[]
45+
LL - };
46+
LL + _ = x.as_slice();
47+
|
2748

28-
error: use `Option::as_slice`
49+
error: manual implementation of `Option::as_slice`
2950
--> tests/ui/manual_option_as_slice.rs:19:9
3051
|
3152
LL | _ = x.as_ref().map_or(&[][..], std::slice::from_ref);
32-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
53+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
54+
|
55+
help: use `Option::as_slice` directly
56+
|
57+
LL - _ = x.as_ref().map_or(&[][..], std::slice::from_ref);
58+
LL + _ = x.as_slice();
59+
|
3360

34-
error: use `Option::as_slice`
61+
error: manual implementation of `Option::as_slice`
3562
--> tests/ui/manual_option_as_slice.rs:22:9
3663
|
3764
LL | _ = x.as_ref().map_or_else(Default::default, std::slice::from_ref);
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
65+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66+
|
67+
help: use `Option::as_slice` directly
68+
|
69+
LL - _ = x.as_ref().map_or_else(Default::default, std::slice::from_ref);
70+
LL + _ = x.as_slice();
71+
|
3972

40-
error: use `Option::as_slice`
73+
error: manual implementation of `Option::as_slice`
4174
--> tests/ui/manual_option_as_slice.rs:25:9
4275
|
4376
LL | _ = x.as_ref().map(std::slice::from_ref).unwrap_or_default();
44-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
77+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
78+
|
79+
help: use `Option::as_slice` directly
80+
|
81+
LL - _ = x.as_ref().map(std::slice::from_ref).unwrap_or_default();
82+
LL + _ = x.as_slice();
83+
|
4584

46-
error: use `Option::as_slice`
85+
error: manual implementation of `Option::as_slice`
4786
--> tests/ui/manual_option_as_slice.rs:28:9
4887
|
4988
LL | _ = x.as_ref().map_or_else(|| &[42][..0], std::slice::from_ref);
50-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
89+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
90+
|
91+
help: use `Option::as_slice` directly
92+
|
93+
LL - _ = x.as_ref().map_or_else(|| &[42][..0], std::slice::from_ref);
94+
LL + _ = x.as_slice();
95+
|
5196

52-
error: use `Option::as_slice`
97+
error: manual implementation of `Option::as_slice`
5398
--> tests/ui/manual_option_as_slice.rs:33:13
5499
|
55100
LL | _ = x.as_ref().map_or_else(<&[_]>::default, from_ref);
56-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `x.as_slice()`
101+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
102+
|
103+
help: use `Option::as_slice` directly
104+
|
105+
LL - _ = x.as_ref().map_or_else(<&[_]>::default, from_ref);
106+
LL + _ = x.as_slice();
107+
|
57108

58109
error: aborting due to 7 previous errors
59110

0 commit comments

Comments
 (0)