aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-01-07 19:53:05 +0000
committerGitHub <[email protected]>2021-01-07 19:53:05 +0000
commit5722d2b7b88cf04365037b0769316916e486de48 (patch)
tree89a4c3f6e8a2b41bb637f818defeb87a47fadd09
parent981a0d708ec352969f9ca075a3e0e50c6da48197 (diff)
parent7066bff9c323f3a02894f70728d22a4398d2e8bd (diff)
Merge #7097
7097: Add fix to wrap return expression in Some r=matklad a=theotherphil Fixes https://github.com/rust-analyzer/rust-analyzer/issues/7095 Co-authored-by: Phil Ellison <[email protected]>
-rw-r--r--crates/hir/src/diagnostics.rs2
-rw-r--r--crates/hir_def/src/path.rs1
-rw-r--r--crates/hir_expand/src/name.rs2
-rw-r--r--crates/hir_ty/src/diagnostics.rs15
-rw-r--r--crates/hir_ty/src/diagnostics/expr.rs27
-rw-r--r--crates/ide/src/diagnostics.rs53
-rw-r--r--crates/ide/src/diagnostics/fixes.rs10
7 files changed, 90 insertions, 20 deletions
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
index b1c924167..447faa04f 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@@ -4,6 +4,6 @@ pub use hir_expand::diagnostics::{
4 Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder, 4 Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder,
5}; 5};
6pub use hir_ty::diagnostics::{ 6pub use hir_ty::diagnostics::{
7 IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, 7 IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
8 NoSuchField, RemoveThisSemicolon, 8 NoSuchField, RemoveThisSemicolon,
9}; 9};
diff --git a/crates/hir_def/src/path.rs b/crates/hir_def/src/path.rs
index e2bf85bbc..3dd7c3cbb 100644
--- a/crates/hir_def/src/path.rs
+++ b/crates/hir_def/src/path.rs
@@ -305,6 +305,7 @@ pub use hir_expand::name as __name;
305macro_rules! __known_path { 305macro_rules! __known_path {
306 (core::iter::IntoIterator) => {}; 306 (core::iter::IntoIterator) => {};
307 (core::result::Result) => {}; 307 (core::result::Result) => {};
308 (core::option::Option) => {};
308 (core::ops::Range) => {}; 309 (core::ops::Range) => {};
309 (core::ops::RangeFrom) => {}; 310 (core::ops::RangeFrom) => {};
310 (core::ops::RangeFull) => {}; 311 (core::ops::RangeFull) => {};
diff --git a/crates/hir_expand/src/name.rs b/crates/hir_expand/src/name.rs
index 2f44876a8..95d853b6d 100644
--- a/crates/hir_expand/src/name.rs
+++ b/crates/hir_expand/src/name.rs
@@ -164,6 +164,7 @@ pub mod known {
164 future, 164 future,
165 result, 165 result,
166 boxed, 166 boxed,
167 option,
167 // Components of known path (type name) 168 // Components of known path (type name)
168 Iterator, 169 Iterator,
169 IntoIterator, 170 IntoIterator,
@@ -172,6 +173,7 @@ pub mod known {
172 Ok, 173 Ok,
173 Future, 174 Future,
174 Result, 175 Result,
176 Option,
175 Output, 177 Output,
176 Target, 178 Target,
177 Box, 179 Box,
diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs
index 14e18f5a1..c67a289f2 100644
--- a/crates/hir_ty/src/diagnostics.rs
+++ b/crates/hir_ty/src/diagnostics.rs
@@ -186,9 +186,10 @@ impl Diagnostic for MissingMatchArms {
186 } 186 }
187} 187}
188 188
189// Diagnostic: missing-ok-in-tail-expr 189// Diagnostic: missing-ok-or-some-in-tail-expr
190// 190//
191// This diagnostic is triggered if block that should return `Result` returns a value not wrapped in `Ok`. 191// This diagnostic is triggered if a block that should return `Result` returns a value not wrapped in `Ok`,
192// or if a block that should return `Option` returns a value not wrapped in `Some`.
192// 193//
193// Example: 194// Example:
194// 195//
@@ -198,17 +199,19 @@ impl Diagnostic for MissingMatchArms {
198// } 199// }
199// ``` 200// ```
200#[derive(Debug)] 201#[derive(Debug)]
201pub struct MissingOkInTailExpr { 202pub struct MissingOkOrSomeInTailExpr {
202 pub file: HirFileId, 203 pub file: HirFileId,
203 pub expr: AstPtr<ast::Expr>, 204 pub expr: AstPtr<ast::Expr>,
205 // `Some` or `Ok` depending on whether the return type is Result or Option
206 pub required: String,
204} 207}
205 208
206impl Diagnostic for MissingOkInTailExpr { 209impl Diagnostic for MissingOkOrSomeInTailExpr {
207 fn code(&self) -> DiagnosticCode { 210 fn code(&self) -> DiagnosticCode {
208 DiagnosticCode("missing-ok-in-tail-expr") 211 DiagnosticCode("missing-ok-or-some-in-tail-expr")
209 } 212 }
210 fn message(&self) -> String { 213 fn message(&self) -> String {
211 "wrap return expression in Ok".to_string() 214 format!("wrap return expression in {}", self.required)
212 } 215 }
213 fn display_source(&self) -> InFile<SyntaxNodePtr> { 216 fn display_source(&self) -> InFile<SyntaxNodePtr> {
214 InFile { file_id: self.file, value: self.expr.clone().into() } 217 InFile { file_id: self.file, value: self.expr.clone().into() }
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
index b4e453411..a1c484fdf 100644
--- a/crates/hir_ty/src/diagnostics/expr.rs
+++ b/crates/hir_ty/src/diagnostics/expr.rs
@@ -11,8 +11,8 @@ use crate::{
11 db::HirDatabase, 11 db::HirDatabase,
12 diagnostics::{ 12 diagnostics::{
13 match_check::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness}, 13 match_check::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness},
14 MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, 14 MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
15 RemoveThisSemicolon, 15 MissingPatFields, RemoveThisSemicolon,
16 }, 16 },
17 utils::variant_data, 17 utils::variant_data,
18 ApplicationTy, InferenceResult, Ty, TypeCtor, 18 ApplicationTy, InferenceResult, Ty, TypeCtor,
@@ -306,27 +306,40 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
306 }; 306 };
307 307
308 let core_result_path = path![core::result::Result]; 308 let core_result_path = path![core::result::Result];
309 let core_option_path = path![core::option::Option];
309 310
310 let resolver = self.owner.resolver(db.upcast()); 311 let resolver = self.owner.resolver(db.upcast());
311 let core_result_enum = match resolver.resolve_known_enum(db.upcast(), &core_result_path) { 312 let core_result_enum = match resolver.resolve_known_enum(db.upcast(), &core_result_path) {
312 Some(it) => it, 313 Some(it) => it,
313 _ => return, 314 _ => return,
314 }; 315 };
316 let core_option_enum = match resolver.resolve_known_enum(db.upcast(), &core_option_path) {
317 Some(it) => it,
318 _ => return,
319 };
315 320
316 let core_result_ctor = TypeCtor::Adt(AdtId::EnumId(core_result_enum)); 321 let core_result_ctor = TypeCtor::Adt(AdtId::EnumId(core_result_enum));
317 let params = match &mismatch.expected { 322 let core_option_ctor = TypeCtor::Adt(AdtId::EnumId(core_option_enum));
323
324 let (params, required) = match &mismatch.expected {
318 Ty::Apply(ApplicationTy { ctor, parameters }) if ctor == &core_result_ctor => { 325 Ty::Apply(ApplicationTy { ctor, parameters }) if ctor == &core_result_ctor => {
319 parameters 326 (parameters, "Ok".to_string())
327 }
328 Ty::Apply(ApplicationTy { ctor, parameters }) if ctor == &core_option_ctor => {
329 (parameters, "Some".to_string())
320 } 330 }
321 _ => return, 331 _ => return,
322 }; 332 };
323 333
324 if params.len() == 2 && params[0] == mismatch.actual { 334 if params.len() > 0 && params[0] == mismatch.actual {
325 let (_, source_map) = db.body_with_source_map(self.owner.into()); 335 let (_, source_map) = db.body_with_source_map(self.owner.into());
326 336
327 if let Ok(source_ptr) = source_map.expr_syntax(id) { 337 if let Ok(source_ptr) = source_map.expr_syntax(id) {
328 self.sink 338 self.sink.push(MissingOkOrSomeInTailExpr {
329 .push(MissingOkInTailExpr { file: source_ptr.file_id, expr: source_ptr.value }); 339 file: source_ptr.file_id,
340 expr: source_ptr.value,
341 required,
342 });
330 } 343 }
331 } 344 }
332 } 345 }
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs
index 6931a6190..055c0a79c 100644
--- a/crates/ide/src/diagnostics.rs
+++ b/crates/ide/src/diagnostics.rs
@@ -125,7 +125,7 @@ pub(crate) fn diagnostics(
125 .on::<hir::diagnostics::MissingFields, _>(|d| { 125 .on::<hir::diagnostics::MissingFields, _>(|d| {
126 res.borrow_mut().push(diagnostic_with_fix(d, &sema)); 126 res.borrow_mut().push(diagnostic_with_fix(d, &sema));
127 }) 127 })
128 .on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| { 128 .on::<hir::diagnostics::MissingOkOrSomeInTailExpr, _>(|d| {
129 res.borrow_mut().push(diagnostic_with_fix(d, &sema)); 129 res.borrow_mut().push(diagnostic_with_fix(d, &sema));
130 }) 130 })
131 .on::<hir::diagnostics::NoSuchField, _>(|d| { 131 .on::<hir::diagnostics::NoSuchField, _>(|d| {
@@ -305,6 +305,40 @@ mod tests {
305 } 305 }
306 306
307 #[test] 307 #[test]
308 fn test_wrap_return_type_option() {
309 check_fix(
310 r#"
311//- /main.rs crate:main deps:core
312use core::option::Option::{self, Some, None};
313
314fn div(x: i32, y: i32) -> Option<i32> {
315 if y == 0 {
316 return None;
317 }
318 x / y$0
319}
320//- /core/lib.rs crate:core
321pub mod result {
322 pub enum Result<T, E> { Ok(T), Err(E) }
323}
324pub mod option {
325 pub enum Option<T> { Some(T), None }
326}
327"#,
328 r#"
329use core::option::Option::{self, Some, None};
330
331fn div(x: i32, y: i32) -> Option<i32> {
332 if y == 0 {
333 return None;
334 }
335 Some(x / y)
336}
337"#,
338 );
339 }
340
341 #[test]
308 fn test_wrap_return_type() { 342 fn test_wrap_return_type() {
309 check_fix( 343 check_fix(
310 r#" 344 r#"
@@ -321,6 +355,9 @@ fn div(x: i32, y: i32) -> Result<i32, ()> {
321pub mod result { 355pub mod result {
322 pub enum Result<T, E> { Ok(T), Err(E) } 356 pub enum Result<T, E> { Ok(T), Err(E) }
323} 357}
358pub mod option {
359 pub enum Option<T> { Some(T), None }
360}
324"#, 361"#,
325 r#" 362 r#"
326use core::result::Result::{self, Ok, Err}; 363use core::result::Result::{self, Ok, Err};
@@ -352,6 +389,9 @@ fn div<T>(x: T) -> Result<T, i32> {
352pub mod result { 389pub mod result {
353 pub enum Result<T, E> { Ok(T), Err(E) } 390 pub enum Result<T, E> { Ok(T), Err(E) }
354} 391}
392pub mod option {
393 pub enum Option<T> { Some(T), None }
394}
355"#, 395"#,
356 r#" 396 r#"
357use core::result::Result::{self, Ok, Err}; 397use core::result::Result::{self, Ok, Err};
@@ -385,6 +425,9 @@ fn div(x: i32, y: i32) -> MyResult<i32> {
385pub mod result { 425pub mod result {
386 pub enum Result<T, E> { Ok(T), Err(E) } 426 pub enum Result<T, E> { Ok(T), Err(E) }
387} 427}
428pub mod option {
429 pub enum Option<T> { Some(T), None }
430}
388"#, 431"#,
389 r#" 432 r#"
390use core::result::Result::{self, Ok, Err}; 433use core::result::Result::{self, Ok, Err};
@@ -414,12 +457,15 @@ fn foo() -> Result<(), i32> { 0 }
414pub mod result { 457pub mod result {
415 pub enum Result<T, E> { Ok(T), Err(E) } 458 pub enum Result<T, E> { Ok(T), Err(E) }
416} 459}
460pub mod option {
461 pub enum Option<T> { Some(T), None }
462}
417"#, 463"#,
418 ); 464 );
419 } 465 }
420 466
421 #[test] 467 #[test]
422 fn test_wrap_return_type_not_applicable_when_return_type_is_not_result() { 468 fn test_wrap_return_type_not_applicable_when_return_type_is_not_result_or_option() {
423 check_no_diagnostics( 469 check_no_diagnostics(
424 r#" 470 r#"
425//- /main.rs crate:main deps:core 471//- /main.rs crate:main deps:core
@@ -433,6 +479,9 @@ fn foo() -> SomeOtherEnum { 0 }
433pub mod result { 479pub mod result {
434 pub enum Result<T, E> { Ok(T), Err(E) } 480 pub enum Result<T, E> { Ok(T), Err(E) }
435} 481}
482pub mod option {
483 pub enum Option<T> { Some(T), None }
484}
436"#, 485"#,
437 ); 486 );
438 } 487 }
diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs
index 71ec4df92..d7ad88ed5 100644
--- a/crates/ide/src/diagnostics/fixes.rs
+++ b/crates/ide/src/diagnostics/fixes.rs
@@ -3,7 +3,7 @@
3use hir::{ 3use hir::{
4 db::AstDatabase, 4 db::AstDatabase,
5 diagnostics::{ 5 diagnostics::{
6 Diagnostic, IncorrectCase, MissingFields, MissingOkInTailExpr, NoSuchField, 6 Diagnostic, IncorrectCase, MissingFields, MissingOkOrSomeInTailExpr, NoSuchField,
7 RemoveThisSemicolon, UnresolvedModule, 7 RemoveThisSemicolon, UnresolvedModule,
8 }, 8 },
9 HasSource, HirDisplay, InFile, Semantics, VariantDef, 9 HasSource, HirDisplay, InFile, Semantics, VariantDef,
@@ -94,15 +94,17 @@ impl DiagnosticWithFix for MissingFields {
94 } 94 }
95} 95}
96 96
97impl DiagnosticWithFix for MissingOkInTailExpr { 97impl DiagnosticWithFix for MissingOkOrSomeInTailExpr {
98 fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { 98 fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
99 let root = sema.db.parse_or_expand(self.file)?; 99 let root = sema.db.parse_or_expand(self.file)?;
100 let tail_expr = self.expr.to_node(&root); 100 let tail_expr = self.expr.to_node(&root);
101 let tail_expr_range = tail_expr.syntax().text_range(); 101 let tail_expr_range = tail_expr.syntax().text_range();
102 let edit = TextEdit::replace(tail_expr_range, format!("Ok({})", tail_expr.syntax())); 102 let replacement = format!("{}({})", self.required, tail_expr.syntax());
103 let edit = TextEdit::replace(tail_expr_range, replacement);
103 let source_change = 104 let source_change =
104 SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into(); 105 SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into();
105 Some(Fix::new("Wrap with ok", source_change, tail_expr_range)) 106 let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" };
107 Some(Fix::new(name, source_change, tail_expr_range))
106 } 108 }
107} 109}
108 110