Add fix to wrap return expression in Some

This commit is contained in:
Phil Ellison 2020-12-30 17:23:00 +00:00
parent 981a0d708e
commit b2dbe6e43a
7 changed files with 90 additions and 24 deletions

View file

@ -4,6 +4,6 @@ pub use hir_expand::diagnostics::{
Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder, Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder,
}; };
pub use hir_ty::diagnostics::{ pub use hir_ty::diagnostics::{
IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
NoSuchField, RemoveThisSemicolon, NoSuchField, RemoveThisSemicolon,
}; };

View file

@ -305,6 +305,7 @@ pub use hir_expand::name as __name;
macro_rules! __known_path { macro_rules! __known_path {
(core::iter::IntoIterator) => {}; (core::iter::IntoIterator) => {};
(core::result::Result) => {}; (core::result::Result) => {};
(core::option::Option) => {};
(core::ops::Range) => {}; (core::ops::Range) => {};
(core::ops::RangeFrom) => {}; (core::ops::RangeFrom) => {};
(core::ops::RangeFull) => {}; (core::ops::RangeFull) => {};

View file

@ -164,6 +164,7 @@ pub mod known {
future, future,
result, result,
boxed, boxed,
option,
// Components of known path (type name) // Components of known path (type name)
Iterator, Iterator,
IntoIterator, IntoIterator,
@ -172,6 +173,7 @@ pub mod known {
Ok, Ok,
Future, Future,
Result, Result,
Option,
Output, Output,
Target, Target,
Box, Box,

View file

@ -186,9 +186,10 @@ impl Diagnostic for MissingMatchArms {
} }
} }
// Diagnostic: missing-ok-in-tail-expr // Diagnostic: missing-ok-or-some-in-tail-expr
// //
// This diagnostic is triggered if block that should return `Result` returns a value not wrapped in `Ok`. // This diagnostic is triggered if a block that should return `Result` returns a value not wrapped in `Ok`,
// or if a block that should return `Option` returns a value not wrapped in `Some`.
// //
// Example: // Example:
// //
@ -198,17 +199,19 @@ impl Diagnostic for MissingMatchArms {
// } // }
// ``` // ```
#[derive(Debug)] #[derive(Debug)]
pub struct MissingOkInTailExpr { pub struct MissingOkOrSomeInTailExpr {
pub file: HirFileId, pub file: HirFileId,
pub expr: AstPtr<ast::Expr>, pub expr: AstPtr<ast::Expr>,
// `Some` or `Ok` depending on whether the return type is Result or Option
pub required: String,
} }
impl Diagnostic for MissingOkInTailExpr { impl Diagnostic for MissingOkOrSomeInTailExpr {
fn code(&self) -> DiagnosticCode { fn code(&self) -> DiagnosticCode {
DiagnosticCode("missing-ok-in-tail-expr") DiagnosticCode("missing-ok-or-some-in-tail-expr")
} }
fn message(&self) -> String { fn message(&self) -> String {
"wrap return expression in Ok".to_string() format!("wrap return expression in {}", self.required)
} }
fn display_source(&self) -> InFile<SyntaxNodePtr> { fn display_source(&self) -> InFile<SyntaxNodePtr> {
InFile { file_id: self.file, value: self.expr.clone().into() } InFile { file_id: self.file, value: self.expr.clone().into() }

View file

@ -11,7 +11,7 @@ use crate::{
db::HirDatabase, db::HirDatabase,
diagnostics::{ diagnostics::{
match_check::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness}, match_check::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness},
MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, MissingPatFields,
RemoveThisSemicolon, RemoveThisSemicolon,
}, },
utils::variant_data, utils::variant_data,
@ -306,27 +306,37 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
}; };
let core_result_path = path![core::result::Result]; let core_result_path = path![core::result::Result];
let core_option_path = path![core::option::Option];
let resolver = self.owner.resolver(db.upcast()); let resolver = self.owner.resolver(db.upcast());
let core_result_enum = match resolver.resolve_known_enum(db.upcast(), &core_result_path) { let core_result_enum = match resolver.resolve_known_enum(db.upcast(), &core_result_path) {
Some(it) => it, Some(it) => it,
_ => return, _ => return,
}; };
let core_option_enum = match resolver.resolve_known_enum(db.upcast(), &core_option_path) {
let core_result_ctor = TypeCtor::Adt(AdtId::EnumId(core_result_enum)); Some(it) => it,
let params = match &mismatch.expected {
Ty::Apply(ApplicationTy { ctor, parameters }) if ctor == &core_result_ctor => {
parameters
}
_ => return, _ => return,
}; };
if params.len() == 2 && params[0] == mismatch.actual { let core_result_ctor = TypeCtor::Adt(AdtId::EnumId(core_result_enum));
let core_option_ctor = TypeCtor::Adt(AdtId::EnumId(core_option_enum));
let (params, required) = match &mismatch.expected {
Ty::Apply(ApplicationTy { ctor, parameters }) if ctor == &core_result_ctor => {
(parameters, "Ok".to_string())
},
Ty::Apply(ApplicationTy { ctor, parameters }) if ctor == &core_option_ctor => {
(parameters, "Some".to_string())
},
_ => return,
};
if params.len() > 0 && params[0] == mismatch.actual {
let (_, source_map) = db.body_with_source_map(self.owner.into()); let (_, source_map) = db.body_with_source_map(self.owner.into());
if let Ok(source_ptr) = source_map.expr_syntax(id) { if let Ok(source_ptr) = source_map.expr_syntax(id) {
self.sink self.sink
.push(MissingOkInTailExpr { file: source_ptr.file_id, expr: source_ptr.value }); .push(MissingOkOrSomeInTailExpr { file: source_ptr.file_id, expr: source_ptr.value, required });
} }
} }
} }

View file

@ -125,7 +125,7 @@ pub(crate) fn diagnostics(
.on::<hir::diagnostics::MissingFields, _>(|d| { .on::<hir::diagnostics::MissingFields, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema)); res.borrow_mut().push(diagnostic_with_fix(d, &sema));
}) })
.on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| { .on::<hir::diagnostics::MissingOkOrSomeInTailExpr, _>(|d| {
res.borrow_mut().push(diagnostic_with_fix(d, &sema)); res.borrow_mut().push(diagnostic_with_fix(d, &sema));
}) })
.on::<hir::diagnostics::NoSuchField, _>(|d| { .on::<hir::diagnostics::NoSuchField, _>(|d| {
@ -304,6 +304,40 @@ mod tests {
expect.assert_debug_eq(&diagnostics) expect.assert_debug_eq(&diagnostics)
} }
#[test]
fn test_wrap_return_type_option() {
check_fix(
r#"
//- /main.rs crate:main deps:core
use core::option::Option::{self, Some, None};
fn div(x: i32, y: i32) -> Option<i32> {
if y == 0 {
return None;
}
x / y<|>
}
//- /core/lib.rs crate:core
pub mod result {
pub enum Result<T, E> { Ok(T), Err(E) }
}
pub mod option {
pub enum Option<T> { Some(T), None }
}
"#,
r#"
use core::option::Option::{self, Some, None};
fn div(x: i32, y: i32) -> Option<i32> {
if y == 0 {
return None;
}
Some(x / y)
}
"#,
);
}
#[test] #[test]
fn test_wrap_return_type() { fn test_wrap_return_type() {
check_fix( check_fix(
@ -321,6 +355,9 @@ fn div(x: i32, y: i32) -> Result<i32, ()> {
pub mod result { pub mod result {
pub enum Result<T, E> { Ok(T), Err(E) } pub enum Result<T, E> { Ok(T), Err(E) }
} }
pub mod option {
pub enum Option<T> { Some(T), None }
}
"#, "#,
r#" r#"
use core::result::Result::{self, Ok, Err}; use core::result::Result::{self, Ok, Err};
@ -352,6 +389,9 @@ fn div<T>(x: T) -> Result<T, i32> {
pub mod result { pub mod result {
pub enum Result<T, E> { Ok(T), Err(E) } pub enum Result<T, E> { Ok(T), Err(E) }
} }
pub mod option {
pub enum Option<T> { Some(T), None }
}
"#, "#,
r#" r#"
use core::result::Result::{self, Ok, Err}; use core::result::Result::{self, Ok, Err};
@ -385,6 +425,9 @@ fn div(x: i32, y: i32) -> MyResult<i32> {
pub mod result { pub mod result {
pub enum Result<T, E> { Ok(T), Err(E) } pub enum Result<T, E> { Ok(T), Err(E) }
} }
pub mod option {
pub enum Option<T> { Some(T), None }
}
"#, "#,
r#" r#"
use core::result::Result::{self, Ok, Err}; use core::result::Result::{self, Ok, Err};
@ -414,12 +457,15 @@ fn foo() -> Result<(), i32> { 0 }
pub mod result { pub mod result {
pub enum Result<T, E> { Ok(T), Err(E) } pub enum Result<T, E> { Ok(T), Err(E) }
} }
pub mod option {
pub enum Option<T> { Some(T), None }
}
"#, "#,
); );
} }
#[test] #[test]
fn test_wrap_return_type_not_applicable_when_return_type_is_not_result() { fn test_wrap_return_type_not_applicable_when_return_type_is_not_result_or_option() {
check_no_diagnostics( check_no_diagnostics(
r#" r#"
//- /main.rs crate:main deps:core //- /main.rs crate:main deps:core
@ -433,6 +479,9 @@ fn foo() -> SomeOtherEnum { 0 }
pub mod result { pub mod result {
pub enum Result<T, E> { Ok(T), Err(E) } pub enum Result<T, E> { Ok(T), Err(E) }
} }
pub mod option {
pub enum Option<T> { Some(T), None }
}
"#, "#,
); );
} }

View file

@ -3,7 +3,7 @@
use hir::{ use hir::{
db::AstDatabase, db::AstDatabase,
diagnostics::{ diagnostics::{
Diagnostic, IncorrectCase, MissingFields, MissingOkInTailExpr, NoSuchField, Diagnostic, IncorrectCase, MissingFields, MissingOkOrSomeInTailExpr, NoSuchField,
RemoveThisSemicolon, UnresolvedModule, RemoveThisSemicolon, UnresolvedModule,
}, },
HasSource, HirDisplay, InFile, Semantics, VariantDef, HasSource, HirDisplay, InFile, Semantics, VariantDef,
@ -94,15 +94,16 @@ impl DiagnosticWithFix for MissingFields {
} }
} }
impl DiagnosticWithFix for MissingOkInTailExpr { impl DiagnosticWithFix for MissingOkOrSomeInTailExpr {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
let root = sema.db.parse_or_expand(self.file)?; let root = sema.db.parse_or_expand(self.file)?;
let tail_expr = self.expr.to_node(&root); let tail_expr = self.expr.to_node(&root);
let tail_expr_range = tail_expr.syntax().text_range(); let tail_expr_range = tail_expr.syntax().text_range();
let edit = TextEdit::replace(tail_expr_range, format!("Ok({})", tail_expr.syntax())); let replacement = format!("{}({})", self.required, tail_expr.syntax());
let source_change = let edit = TextEdit::replace(tail_expr_range, replacement);
SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into(); let source_change = SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into();
Some(Fix::new("Wrap with ok", source_change, tail_expr_range)) let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" };
Some(Fix::new(name, source_change, tail_expr_range))
} }
} }