Skip to content

Commit 283f684

Browse files
nik-revnik-rev
committed
feat: Implement concealed_obvious_default lint
1 parent 9e7782b commit 283f684

38 files changed

+654
-275
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5694,6 +5694,7 @@ Released 2018-09-13
56945694
[`collection_is_never_read`]: https://rust-lang.github.io/rust-clippy/master/index.html#collection_is_never_read
56955695
[`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
56965696
[`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
5697+
[`concealed_obvious_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#concealed_obvious_default
56975698
[`confusing_method_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#confusing_method_to_numeric_cast
56985699
[`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
56995700
[`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime

‎clippy_config/src/conf.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,7 +1097,7 @@ impl serde::de::Error for FieldError {
10971097
writeln!(msg).unwrap();
10981098
for (column, column_width) in column_widths.iter().copied().enumerate() {
10991099
let index = column * rows + row;
1100-
let field = expected.get(index).copied().unwrap_or_default();
1100+
let field = expected.get(index).copied().unwrap_or("");
11011101
write!(msg, "{:SEPARATOR_WIDTH$}{field:column_width$}", " ").unwrap();
11021102
}
11031103
}
@@ -1128,7 +1128,7 @@ fn calculate_dimensions(fields: &[&str]) -> (usize, Vec<usize>) {
11281128
(0..rows)
11291129
.map(|row| {
11301130
let index = column * rows + row;
1131-
let field = fields.get(index).copied().unwrap_or_default();
1131+
let field = fields.get(index).copied().unwrap_or("");
11321132
field.len()
11331133
})
11341134
.max()

‎clippy_lints/src/arbitrary_source_item_ordering.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering {
414414
let module_level_order = self
415415
.module_item_order_groupings
416416
.module_level_order_of(&item_kind)
417-
.unwrap_or_default();
417+
.unwrap_or(0);
418418

419419
if let Some(cur_t) = cur_t.as_ref() {
420420
use std::cmp::Ordering; // Better legibility.

‎clippy_lints/src/casts/cast_possible_truncation.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
4949
.min(apply_reductions(cx, nbits, right, signed))
5050
.min(apply_reductions(cx, nbits, left, signed)),
5151
BinOpKind::Shr => apply_reductions(cx, nbits, left, signed)
52-
.saturating_sub(constant_int(cx, right).map_or(0, |s| u64::try_from(s).unwrap_or_default())),
52+
.saturating_sub(constant_int(cx, right).map_or(0, |s| u64::try_from(s).unwrap_or(0))),
5353
_ => nbits,
5454
},
5555
ExprKind::MethodCall(method, left, [right], _) => {

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
359359
crate::methods::CLONE_ON_COPY_INFO,
360360
crate::methods::CLONE_ON_REF_PTR_INFO,
361361
crate::methods::COLLAPSIBLE_STR_REPLACE_INFO,
362+
crate::methods::CONCEALED_OBVIOUS_DEFAULT_INFO,
362363
crate::methods::CONST_IS_EMPTY_INFO,
363364
crate::methods::DOUBLE_ENDED_ITERATOR_LAST_INFO,
364365
crate::methods::DRAIN_COLLECT_INFO,

‎clippy_lints/src/manual_strip.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ fn find_stripping<'tcx>(
269269

270270
fn visit_pat(&mut self, pat: &'tcx rustc_hir::Pat<'tcx>) -> Self::Result {
271271
if let PatKind::Binding(_, _, ident, _) = pat.kind {
272-
*self.bindings.entry(ident.name).or_default() += 1;
272+
*self.bindings.entry(ident.name).or_insert(0) += 1;
273273
}
274274
walk_pat(self, pat);
275275
}

‎clippy_lints/src/methods/concealed_obvious_default.rs

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
use std::borrow::Cow;
2+
3+
use rustc_span::Symbol;
4+
5+
use clippy_utils::diagnostics::span_lint_and_sugg;
6+
use clippy_utils::sym;
7+
use clippy_utils::ty::is_type_diagnostic_item;
8+
use rustc_errors::Applicability;
9+
use rustc_hir as hir;
10+
use rustc_lint::LateContext;
11+
use rustc_middle::ty::{self, Ty};
12+
use rustc_span::Span;
13+
14+
use super::CONCEALED_OBVIOUS_DEFAULT;
15+
16+
pub(super) fn check(cx: &LateContext<'_>, recv: &hir::Expr<'_>, method_name: Symbol, call_span: Span) {
17+
// Type of the expression which invoked the method
18+
let recv_ty = cx.typeck_results().expr_ty(recv);
19+
20+
// Only consider algebraic data types e.g. an `Option`.
21+
// Their generics are represented by `generic_args`
22+
if let ty::Adt(adt, generic_args) = recv_ty.kind()
23+
// `name_of_generic`, is e.g. a `sym::Option`
24+
&& let Some(name_of_generic) = cx.tcx.get_diagnostic_name(adt.did())
25+
&& let Some((message, suggestion)) = CONCEALING_METHODS.into_iter().find_map(|concealing| {
26+
(name_of_generic == concealing.ty && method_name == concealing.method)
27+
.then(|| {
28+
generic_args.get(concealing.generic_index).and_then(|entry| {
29+
entry.as_type().and_then(|ty| {
30+
extract_obvious_default(cx, ty).map(|(default, ty)| {
31+
let method = (concealing.fmt_msg)(ty);
32+
(
33+
format!("method {method} conceals the underlying type"),
34+
(concealing.fmt_sugg)(default),
35+
)
36+
})
37+
})
38+
})
39+
})
40+
.flatten()
41+
})
42+
{
43+
span_lint_and_sugg(
44+
cx,
45+
CONCEALED_OBVIOUS_DEFAULT,
46+
call_span,
47+
message,
48+
"write the default type explicitly".to_string(),
49+
suggestion,
50+
Applicability::MachineApplicable,
51+
);
52+
}
53+
}
54+
55+
/// Method which conceals an underlying type of a generic
56+
struct ConcealingMethod {
57+
/// Generic which contains the concealing method, e.g. `Option<T>`
58+
ty: Symbol,
59+
/// Index of the concealed generic type, e.g. `0` for `Option<T>`
60+
generic_index: usize,
61+
/// The concealing method, e.g. `unwrap_or_default`
62+
method: Symbol,
63+
/// Format the lint's message
64+
///
65+
/// Receives the concealed type, e.g. for `Option<bool>` receives `bool`
66+
fmt_msg: fn(Cow<'static, str>) -> String,
67+
/// Format the lint's suggestion
68+
///
69+
/// Receives the default of the concealed type, e.g. for `Option<bool>` receives `false`,
70+
/// as `bool::default()` is `false`
71+
fmt_sugg: fn(&'static str) -> String,
72+
}
73+
74+
/// List of methods which use `Default` trait under the hood,
75+
/// but they have an alternative non-`Default` method
76+
///
77+
/// For example, there is `Option::unwrap_or_default` which is almost the same
78+
/// as `Option::unwrap_or`, but the type does not have to be provided and the
79+
/// `Default` implementation is used.
80+
const CONCEALING_METHODS: [ConcealingMethod; 4] = [
81+
ConcealingMethod {
82+
ty: sym::Result,
83+
// Result<T, E>
84+
// ^ want
85+
generic_index: 0,
86+
method: sym::unwrap_or_default,
87+
fmt_msg: |ty| format!("Result::<{ty}, _>::unwrap_or_default()"),
88+
fmt_sugg: |val| format!("unwrap_or({val})"),
89+
},
90+
ConcealingMethod {
91+
ty: sym::Option,
92+
// Option<T>
93+
// ^ want
94+
generic_index: 0,
95+
method: sym::unwrap_or_default,
96+
fmt_msg: |ty| format!("Option::<{ty}>::unwrap_or_default()"),
97+
fmt_sugg: |val| format!("unwrap_or({val})"),
98+
},
99+
ConcealingMethod {
100+
ty: sym::HashMapEntry,
101+
// Entry<'a, K, V, A = Global>
102+
// ^ want
103+
generic_index: 2,
104+
method: sym::or_default,
105+
fmt_msg: |ty| format!("hash_map::Entry::<'_, _, {ty}>::or_default()"),
106+
fmt_sugg: |val| format!("or_insert({val})"),
107+
},
108+
ConcealingMethod {
109+
ty: sym::BTreeEntry,
110+
// Entry<'a, K, V, A = Global>
111+
// ^ want
112+
generic_index: 2,
113+
method: sym::or_default,
114+
fmt_msg: |ty| format!("btree_map::Entry::<'_, _, {ty}>::or_default()"),
115+
fmt_sugg: |val| format!("or_insert({val})"),
116+
},
117+
];
118+
119+
/// Get default value of a type with an obvious default.
120+
///
121+
/// # Returns
122+
///
123+
/// If the type has an obvious default:
124+
///
125+
/// - Default for the type
126+
/// - The type as it should be displayed in the lint message
127+
///
128+
/// If the type is not considered to have an obvious default, return `None`.
129+
fn extract_obvious_default(cx: &LateContext<'_>, ty: Ty<'_>) -> Option<(&'static str, Cow<'static, str>)> {
130+
let ty = ty.peel_refs();
131+
Some(if ty.is_char() {
132+
("'\\0'", "char".into())
133+
} else if ty.is_floating_point() {
134+
("0.0", ty.to_string().into())
135+
} else if ty.is_integral() {
136+
("0", ty.to_string().into())
137+
} else if ty.is_str() {
138+
("\"\"", "&str".into())
139+
} else if ty.is_bool() {
140+
("false", "bool".into())
141+
} else if ty.is_unit() {
142+
("()", "()".into())
143+
} else if is_type_diagnostic_item(cx, ty, sym::Option) {
144+
("None", "Option<_>".into())
145+
} else {
146+
return None;
147+
})
148+
}

‎clippy_lints/src/methods/mod.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ mod clone_on_copy;
1414
mod clone_on_ref_ptr;
1515
mod cloned_instead_of_copied;
1616
mod collapsible_str_replace;
17+
mod concealed_obvious_default;
1718
mod double_ended_iterator_last;
1819
mod drain_collect;
1920
mod err_expect;
@@ -4565,6 +4566,46 @@ declare_clippy_lint! {
45654566
"hardcoded localhost IP address"
45664567
}
45674568

4569+
declare_clippy_lint! {
4570+
/// ### What it does
4571+
///
4572+
/// Checks for usages of methods like `Option::unwrap_or_default`,
4573+
/// for types where the `Default` implementation is obvious and can be written literally
4574+
///
4575+
/// ### Why is this bad?
4576+
///
4577+
/// You have to know what the underlying type is, instead of it being obvious from a glance
4578+
///
4579+
/// ### Example
4580+
/// ```no_run
4581+
/// fn f(
4582+
/// a: Option<&str>,
4583+
/// b: Option<usize>,
4584+
/// c: Option<Option<bool>>
4585+
/// ) {
4586+
/// let a = a.unwrap_or_default();
4587+
/// let b = b.unwrap_or_default();
4588+
/// let c = c.unwrap_or_default();
4589+
/// }
4590+
/// ```
4591+
/// Use instead:
4592+
/// ```no_run
4593+
/// fn f(
4594+
/// a: Option<&str>,
4595+
/// b: Option<usize>,
4596+
/// c: Option<Option<bool>>
4597+
/// ) {
4598+
/// let a = a.unwrap_or("");
4599+
/// let b = b.unwrap_or(0);
4600+
/// let c = c.unwrap_or(None);
4601+
/// }
4602+
/// ```
4603+
#[clippy::version = "1.89.0"]
4604+
pub CONCEALED_OBVIOUS_DEFAULT,
4605+
complexity,
4606+
".*or_default() obscures the underlying type"
4607+
}
4608+
45684609
#[expect(clippy::struct_excessive_bools)]
45694610
pub struct Methods {
45704611
avoid_breaking_exported_api: bool,
@@ -4744,6 +4785,7 @@ impl_lint_pass!(Methods => [
47444785
IO_OTHER_ERROR,
47454786
SWAP_WITH_TEMPORARY,
47464787
IP_CONSTANT,
4788+
CONCEALED_OBVIOUS_DEFAULT
47474789
]);
47484790

47494791
/// Extracts a method call name, args, and `Span` of the method name.
@@ -5505,6 +5547,10 @@ impl Methods {
55055547
_ => {},
55065548
}
55075549
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
5550+
concealed_obvious_default::check(cx, recv, name, call_span);
5551+
},
5552+
(sym::or_default, []) => {
5553+
concealed_obvious_default::check(cx, recv, name, call_span);
55085554
},
55095555
(sym::unwrap_or_else, [u_arg]) => {
55105556
match method_call(recv) {

‎clippy_lints/src/operators/arithmetic_side_effects.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,9 @@ impl ArithmeticSideEffects {
6464
/// "multiplication" are present in the inner set of allowed types.
6565
fn has_allowed_binary(&self, lhs_ty: Ty<'_>, rhs_ty: Ty<'_>) -> bool {
6666
let lhs_ty_string = lhs_ty.to_string();
67-
let lhs_ty_string_elem = lhs_ty_string.split('<').next().unwrap_or_default();
67+
let lhs_ty_string_elem = lhs_ty_string.split('<').next().unwrap_or("");
6868
let rhs_ty_string = rhs_ty.to_string();
69-
let rhs_ty_string_elem = rhs_ty_string.split('<').next().unwrap_or_default();
69+
let rhs_ty_string_elem = rhs_ty_string.split('<').next().unwrap_or("");
7070
if let Some(rhs_from_specific) = self.allowed_binary.get(lhs_ty_string_elem)
7171
&& {
7272
let rhs_has_allowed_ty = rhs_from_specific.contains(rhs_ty_string_elem);
@@ -85,7 +85,7 @@ impl ArithmeticSideEffects {
8585
/// allowed types.
8686
fn has_allowed_unary(&self, ty: Ty<'_>) -> bool {
8787
let ty_string = ty.to_string();
88-
let ty_string_elem = ty_string.split('<').next().unwrap_or_default();
88+
let ty_string_elem = ty_string.split('<').next().unwrap_or("");
8989
self.allowed_unary.contains(ty_string_elem)
9090
}
9191

‎clippy_lints/src/utils/author.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
216216

217217
fn next(&self, s: &'static str) -> String {
218218
let mut ids = self.ids.take();
219-
let out = match *ids.entry(s).and_modify(|n| *n += 1).or_default() {
219+
let out = match *ids.entry(s).and_modify(|n| *n += 1).or_insert(0) {
220220
// first usage of the name, use it as is
221221
0 => s.to_string(),
222222
// append a number starting with 1

‎clippy_utils/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2770,7 +2770,7 @@ pub fn tokenize_with_text(s: &str) -> impl Iterator<Item = (TokenKind, &str, Inn
27702770
let range = pos as usize..end as usize;
27712771
let inner = InnerSpan::new(range.start, range.end);
27722772
pos = end;
2773-
(t.kind, s.get(range).unwrap_or_default(), inner)
2773+
(t.kind, s.get(range).unwrap_or(""), inner)
27742774
})
27752775
}
27762776

‎clippy_utils/src/ty/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,7 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
934934
match (cx.layout_of(ty).map(|layout| layout.size.bytes()), ty.kind()) {
935935
(Ok(size), _) => size,
936936
(Err(_), ty::Tuple(list)) => list.iter().map(|t| approx_ty_size(cx, t)).sum(),
937-
(Err(_), ty::Array(t, n)) => n.try_to_target_usize(cx.tcx).unwrap_or_default() * approx_ty_size(cx, *t),
937+
(Err(_), ty::Array(t, n)) => n.try_to_target_usize(cx.tcx).unwrap_or(0) * approx_ty_size(cx, *t),
938938
(Err(_), ty::Adt(def, subst)) if def.is_struct() => def
939939
.variants()
940940
.iter()
@@ -955,7 +955,7 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
955955
.sum::<u64>()
956956
})
957957
.max()
958-
.unwrap_or_default(),
958+
.unwrap_or(0),
959959
(Err(_), ty::Adt(def, subst)) if def.is_union() => def
960960
.variants()
961961
.iter()
@@ -964,10 +964,10 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
964964
.iter()
965965
.map(|field| approx_ty_size(cx, field.ty(cx.tcx, subst)))
966966
.max()
967-
.unwrap_or_default()
967+
.unwrap_or(0)
968968
})
969969
.max()
970-
.unwrap_or_default(),
970+
.unwrap_or(0),
971971
(Err(_), _) => 0,
972972
}
973973
}

‎src/driver.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ pub fn main() {
267267
let clippy_args_var = env::var("CLIPPY_ARGS").ok();
268268
let clippy_args = clippy_args_var
269269
.as_deref()
270-
.unwrap_or_default()
270+
.unwrap_or("")
271271
.split("__CLIPPY_HACKERY__")
272272
.filter_map(|s| match s {
273273
"" => None,

0 commit comments

Comments
 (0)