mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Auto merge of #3561 - fuerstenau:master, r=oli-obk

Browse Source 
Suggest `.as_ref()?` instead of `?` in certain circumstances.
This commit is contained in:
bors 2018-12-28 20:32:42 +00:00
commit 3c4abb5b80
3 changed files with 107 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
clippy_lints/src/question_mark.rs
View File

@ -72,6 +72,8 @@ impl Pass {
if Self::is_option(cx, subject);
then {
let receiver_str = &Sugg::hir(cx, subject, "..");
let mut replacement: Option<String> = None;
if let Some(else_) = else_ {
if_chain! {
if let ExprKind::Block(block, None) = &else_.node;
@ -79,45 +81,41 @@ impl Pass {
if let Some(block_expr) = &block.expr;
if SpanlessEq::new(cx).ignore_fn().eq_expr(subject, block_expr);
then {
span_lint_and_then(
cx,
QUESTION_MARK,
expr.span,
"this block may be rewritten with the `?` operator",
|db| {
db.span_suggestion_with_applicability(
expr.span,
"replace_it_with",
format!("Some({}?)", Sugg::hir(cx, subject, "..")),
Applicability::MaybeIncorrect, // snippet
);
}
)
replacement = Some(format!("Some({}?)", receiver_str));
}
}
return;
} else if Self::moves_by_default(cx, subject) {
replacement = Some(format!("{}.as_ref()?;", receiver_str));
} else {
replacement = Some(format!("{}?;", receiver_str));
}
span_lint_and_then(
cx,
QUESTION_MARK,
expr.span,
"this block may be rewritten with the `?` operator",
|db| {
let receiver_str = &Sugg::hir(cx, subject, "..");
db.span_suggestion_with_applicability(
expr.span,
"replace_it_with",
format!("{}?;", receiver_str),
Applicability::MaybeIncorrect, // snippet
);
}
)
if let Some(replacement_str) = replacement {
span_lint_and_then(
cx,
QUESTION_MARK,
expr.span,
"this block may be rewritten with the `?` operator",
|db| {
db.span_suggestion_with_applicability(
expr.span,
"replace_it_with",
replacement_str,
Applicability::MaybeIncorrect, // snippet
);
}
)
}
}
}
}
fn moves_by_default(cx: &LateContext<'_, '_>, expression: &Expr) -> bool {
let expr_ty = cx.tables.expr_ty(expression);
expr_ty.moves_by_default(cx.tcx, cx.param_env, expression.span)
}
fn is_option(cx: &LateContext<'_, '_>, expression: &Expr) -> bool {
let expr_ty = cx.tables.expr_ty(expression);

54
tests/ui/question_mark.rs
View File

@ -15,6 +15,15 @@ fn some_func(a: Option<u32>) -> Option<u32> {
a
}
fn some_other_func(a: Option<u32>) -> Option<u32> {
if a.is_none() {
return None;
} else {
return Some(0);
}
unreachable!()
}
pub enum SeemsOption<T> {
Some(T),
None,
@ -37,11 +46,11 @@ fn returns_something_similar_to_option(a: SeemsOption<u32>) -> SeemsOption<u32>
a
}
pub struct SomeStruct {
pub struct CopyStruct {
pub opt: Option<u32>,
}
impl SomeStruct {
impl CopyStruct {
#[rustfmt::skip]
pub fn func(&self) -> Option<u32> {
if (self.opt).is_none() {
@ -62,12 +71,49 @@ impl SomeStruct {
}
}
#[derive(Clone)]
pub struct MoveStruct {
pub opt: Option<Vec<u32>>,
}
impl MoveStruct {
pub fn ref_func(&self) -> Option<Vec<u32>> {
if self.opt.is_none() {
return None;
}
self.opt.clone()
}
pub fn mov_func_reuse(self) -> Option<Vec<u32>> {
if self.opt.is_none() {
return None;
}
self.opt
}
pub fn mov_func_no_use(self) -> Option<Vec<u32>> {
if self.opt.is_none() {
return None;
}
Some(Vec::new())
}
}
fn main() {
some_func(Some(42));
some_func(None);
let some_struct = SomeStruct { opt: Some(54) };
some_struct.func();
let copy_struct = CopyStruct { opt: Some(54) };
copy_struct.func();
let move_struct = MoveStruct {
opt: Some(vec![42, 1337]),
};
move_struct.ref_func();
move_struct.clone().mov_func_reuse();
move_struct.clone().mov_func_no_use();
let so = SeemsOption::Some(45);
returns_something_similar_to_option(so);

32
tests/ui/question_mark.stderr
View File

@ -9,7 +9,7 @@ LL | | }
= note: `-D clippy::question-mark` implied by `-D warnings`
error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:47:9
--> $DIR/question_mark.rs:56:9
|
LL | / if (self.opt).is_none() {
LL | | return None;
@ -17,7 +17,7 @@ LL | | }
| |_________^ help: replace_it_with: `(self.opt)?;`
error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:51:9
--> $DIR/question_mark.rs:60:9
|
LL | / if self.opt.is_none() {
LL | | return None
@ -25,7 +25,7 @@ LL | | }
| |_________^ help: replace_it_with: `self.opt?;`
error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:55:17
--> $DIR/question_mark.rs:64:17
|
LL | let _ = if self.opt.is_none() {
| _________________^
@ -35,5 +35,29 @@ LL | | self.opt
LL | | };
| |_________^ help: replace_it_with: `Some(self.opt?)`
error: aborting due to 4 previous errors
error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:81:9
|
LL | / if self.opt.is_none() {
LL | | return None;
LL | | }
| |_________^ help: replace_it_with: `self.opt.as_ref()?;`
error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:89:9
|
LL | / if self.opt.is_none() {
LL | | return None;
LL | | }
| |_________^ help: replace_it_with: `self.opt.as_ref()?;`
error: this block may be rewritten with the `?` operator
--> $DIR/question_mark.rs:97:9
|
LL | / if self.opt.is_none() {
LL | | return None;
LL | | }
| |_________^ help: replace_it_with: `self.opt.as_ref()?;`
error: aborting due to 7 previous errors