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

Detect incorrect ; in Option::ok_or_else and Result::map_err

Browse Source 
Fix #72124.
This commit is contained in:
Esteban Küber 2023-10-07 03:47:02 +00:00
parent 53817963ed
commit 98e5317173
4 changed files with 86 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
compiler/rustc_span/src/symbol.rs
View File

@ -973,6 +973,7 @@
managed_boxes,
manually_drop,
map,
map_err,
marker,
marker_trait_attr,
masked,
@ -1137,6 +1138,7 @@
offset,
offset_of,
offset_of_enum,
ok_or_else,
omit_gdb_pretty_printer_section,
on,
on_unimplemented,

70
compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
View File

@ -41,7 +41,7 @@
use rustc_session::Limit;
use rustc_span::def_id::LOCAL_CRATE;
use rustc_span::symbol::sym;
use rustc_span::{BytePos, ExpnKind, Span, DUMMY_SP};
use rustc_span::{BytePos, ExpnKind, Span, Symbol, DUMMY_SP};
use std::borrow::Cow;
use std::fmt;
use std::iter;
@ -1045,6 +1045,7 @@ fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
return;
}
let self_ty = trait_ref.self_ty();
let found_ty = trait_ref.args.get(1).and_then(|a| a.as_type());
let mut prev_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
@ -1070,17 +1071,80 @@ fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) {
// The following logic is simlar to `point_at_chain`, but that's focused on associated types
let mut expr = expr;
while let hir::ExprKind::MethodCall(_path_segment, rcvr_expr, _args, span) = expr.kind {
while let hir::ExprKind::MethodCall(path_segment, rcvr_expr, args, span) = expr.kind {
// Point at every method call in the chain with the `Result` type.
// let foo = bar.iter().map(mapper)?;
// ------ -----------
expr = rcvr_expr;
chain.push((span, prev_ty));
prev_ty = self.resolve_vars_if_possible(
let next_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr).unwrap_or(Ty::new_misc_error(self.tcx)),
);
let is_diagnostic_item = |symbol: Symbol, ty: Ty<'tcx>| {
let ty::Adt(def, _) = ty.kind() else {
return false;
};
self.tcx.is_diagnostic_item(symbol, def.did())
};
// For each method in the chain, see if this is `Result::map_err` or
// `Option::ok_or_else` and if it is, see if the closure passed to it has an incorrect
// trailing `;`.
if let Some(ty) = get_e_type(prev_ty)
&& let Some(found_ty) = found_ty
// Ideally we would instead use `FnCtxt::lookup_method_for_diagnostic` for 100%
// accurate check, but we are in the wrong stage to do that and looking for
// `Result::map_err` by checking the Self type and the path segment is enough.
// sym::ok_or_else
&& (
( // Result::map_err
path_segment.ident.name == sym::map_err
&& is_diagnostic_item(sym::Result, next_ty)
) || ( // Option::ok_or_else
path_segment.ident.name == sym::ok_or_else
&& is_diagnostic_item(sym::Option, next_ty)
)
)
// Found `Result<_, ()>?`
&& let ty::Tuple(tys) = found_ty.kind()
&& tys.is_empty()
// The current method call returns `Result<_, ()>`
&& self.can_eq(obligation.param_env, ty, found_ty)
// There's a single argument in the method call and it is a closure
&& args.len() == 1
&& let Some(arg) = args.get(0)
&& let hir::ExprKind::Closure(closure) = arg.kind
// The closure has a block for its body with no tail expression
&& let body = self.tcx.hir().body(closure.body)
&& let hir::ExprKind::Block(block, _) = body.value.kind
&& let None = block.expr
// The last statement is of a type that can be converted to the return error type
&& let [.., stmt] = block.stmts
&& let hir::StmtKind::Semi(expr) = stmt.kind
&& let expr_ty = self.resolve_vars_if_possible(
typeck.expr_ty_adjusted_opt(expr)
.unwrap_or(Ty::new_misc_error(self.tcx)),
)
&& self
.infcx
.type_implements_trait(
self.tcx.get_diagnostic_item(sym::From).unwrap(),
[self_ty, expr_ty],
obligation.param_env,
)
.must_apply_modulo_regions()
{
err.span_suggestion_short(
stmt.span.with_lo(expr.span.hi()),
"remove this semicolon",
String::new(),
Applicability::MachineApplicable,
);
}
prev_ty = next_ty;
if let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = expr.kind
&& let hir::Path { res: hir::def::Res::Local(hir_id), .. } = path
&& let Some(hir::Node::Pat(binding)) = self.tcx.hir().find(*hir_id)

9
tests/ui/traits/question-mark-result-err-mismatch.rs
View File

@ -8,7 +8,9 @@ fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
});
let one = x
.map(|s| ())
.map_err(|_| ()) //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
.map_err(|e| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
e; //~ HELP remove this semicolon
})
.map(|()| "")?; //~ ERROR `?` couldn't convert the error to `String`
//~^ NOTE in this expansion of desugaring of operator `?`
//~| NOTE in this expansion of desugaring of operator `?`
@ -17,6 +19,7 @@ fn foo() -> Result<String, String> { //~ NOTE expected `String` because of this
//~| NOTE the trait `From<()>` is not implemented for `String`
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
//~| NOTE required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
//~| HELP the following other types implement trait `From<T>`:
Ok(one.to_string())
}
@ -33,6 +36,7 @@ fn bar() -> Result<(), String> { //~ NOTE expected `String` because of this
//~| NOTE the trait `From<()>` is not implemented for `String`
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
//~| NOTE required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
//~| HELP the following other types implement trait `From<T>`:
Ok(one)
}
@ -42,7 +46,7 @@ fn baz() -> Result<String, String> { //~ NOTE expected `String` because of this
.split_whitespace()
.next()
.ok_or_else(|| { //~ NOTE this can't be annotated with `?` because it has type `Result<_, ()>`
"Couldn't split the test string";
"Couldn't split the test string"; //~ HELP remove this semicolon
})?;
//~^ ERROR `?` couldn't convert the error to `String`
//~| NOTE in this expansion of desugaring of operator `?`
@ -52,6 +56,7 @@ fn baz() -> Result<String, String> { //~ NOTE expected `String` because of this
//~| NOTE the trait `From<()>` is not implemented for `String`
//~| NOTE the question mark operation (`?`) implicitly performs a conversion on the error value using the `From` trait
//~| NOTE required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
//~| HELP the following other types implement trait `From<T>`:
Ok(one.to_string())
}

15
tests/ui/traits/question-mark-result-err-mismatch.stderr
View File

@ -1,5 +1,5 @@
error[E0277]: `?` couldn't convert the error to `String`
--> $DIR/question-mark-result-err-mismatch.rs:12:22
--> $DIR/question-mark-result-err-mismatch.rs:14:22
|
LL | fn foo() -> Result<String, String> {
| ---------------------- expected `String` because of this
@ -10,8 +10,12 @@ LL | | "Couldn't split the test string"
LL | | });
| |__________- this has type `Result<_, &str>`
...
LL | .map_err(|_| ())
| --------------- this can't be annotated with `?` because it has type `Result<_, ()>`
LL | .map_err(|e| {
| __________-
LL | | e;
| | - help: remove this semicolon
LL | | })
| |__________- this can't be annotated with `?` because it has type `Result<_, ()>`
LL | .map(|()| "")?;
| ^ the trait `From<()>` is not implemented for `String`
|
@ -26,7 +30,7 @@ LL | .map(|()| "")?;
= note: required for `Result<String, String>` to implement `FromResidual<Result<Infallible, ()>>`
error[E0277]: `?` couldn't convert the error to `String`
--> $DIR/question-mark-result-err-mismatch.rs:27:25
--> $DIR/question-mark-result-err-mismatch.rs:30:25
|
LL | fn bar() -> Result<(), String> {
| ------------------ expected `String` because of this
@ -49,7 +53,7 @@ LL | .map_err(|_| ())?;
= note: required for `Result<(), String>` to implement `FromResidual<Result<Infallible, ()>>`
error[E0277]: `?` couldn't convert the error to `String`
--> $DIR/question-mark-result-err-mismatch.rs:46:11
--> $DIR/question-mark-result-err-mismatch.rs:50:11
|
LL | fn baz() -> Result<String, String> {
| ---------------------- expected `String` because of this
@ -57,6 +61,7 @@ LL | fn baz() -> Result<String, String> {
LL | .ok_or_else(|| {
| __________-
LL | | "Couldn't split the test string";
| | - help: remove this semicolon
LL | | })?;
| | -^ the trait `From<()>` is not implemented for `String`
| |__________|