Skip to content

Commit 82be9dc

Browse files
committed
Auto merge of #5481 - sinkuu:no_as_ref, r=phansch
question_mark: don't add `as_ref()` for a call expression If a call returns a `!Copy` value, it does so regardless of whether `as_ref()` is added. For example, `foo.into_option().as_ref()?` can be simplified to `foo.into_option()?`. --- changelog: Improved `question_mark` lint suggestion so that it doesn't add redundant `as_ref()`
2 parents 8ae143f + f58bb5b commit 82be9dc

File tree

4 files changed

+40
-4
lines changed

4 files changed

+40
-4
lines changed

clippy_lints/src/question_mark.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,12 @@ impl QuestionMark {
7070
replacement = Some(format!("Some({}?)", receiver_str));
7171
}
7272
}
73-
} else if Self::moves_by_default(cx, subject) {
74-
replacement = Some(format!("{}.as_ref()?;", receiver_str));
73+
} else if Self::moves_by_default(cx, subject)
74+
&& !matches!(subject.kind, ExprKind::Call(..) | ExprKind::MethodCall(..))
75+
{
76+
replacement = Some(format!("{}.as_ref()?;", receiver_str));
7577
} else {
76-
replacement = Some(format!("{}?;", receiver_str));
78+
replacement = Some(format!("{}?;", receiver_str));
7779
}
7880

7981
if let Some(replacement_str) = replacement {

tests/ui/question_mark.fixed

+12
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,16 @@ impl MoveStruct {
9393
}
9494
}
9595

96+
fn func() -> Option<i32> {
97+
fn f() -> Option<String> {
98+
Some(String::new())
99+
}
100+
101+
f()?;
102+
103+
Some(0)
104+
}
105+
96106
fn main() {
97107
some_func(Some(42));
98108
some_func(None);
@@ -110,4 +120,6 @@ fn main() {
110120

111121
let so = SeemsOption::Some(45);
112122
returns_something_similar_to_option(so);
123+
124+
func();
113125
}

tests/ui/question_mark.rs

+14
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ impl MoveStruct {
121121
}
122122
}
123123

124+
fn func() -> Option<i32> {
125+
fn f() -> Option<String> {
126+
Some(String::new())
127+
}
128+
129+
if f().is_none() {
130+
return None;
131+
}
132+
133+
Some(0)
134+
}
135+
124136
fn main() {
125137
some_func(Some(42));
126138
some_func(None);
@@ -138,4 +150,6 @@ fn main() {
138150

139151
let so = SeemsOption::Some(45);
140152
returns_something_similar_to_option(so);
153+
154+
func();
141155
}

tests/ui/question_mark.stderr

+9-1
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,13 @@ LL | | return None;
9292
LL | | };
9393
| |_________^ help: replace it with: `self.opt?`
9494

95-
error: aborting due to 10 previous errors
95+
error: this block may be rewritten with the `?` operator
96+
--> $DIR/question_mark.rs:129:5
97+
|
98+
LL | / if f().is_none() {
99+
LL | | return None;
100+
LL | | }
101+
| |_____^ help: replace it with: `f()?;`
102+
103+
error: aborting due to 11 previous errors
96104

0 commit comments

Comments
 (0)