diff options
author | Aleksey Kladov <[email protected]> | 2021-06-13 18:19:11 +0100 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2021-06-13 18:19:11 +0100 |
commit | 949a6ec469507db5e79578da94e17cb63cb54d19 (patch) | |
tree | 5e4ec22d50715d9855e751ed3f296e90e5bfaa73 /crates | |
parent | 74f3cca85ab870614f314c6180e2fbb883ad4fe3 (diff) |
internal: refactor missing or or some diagnostic
Diffstat (limited to 'crates')
-rw-r--r-- | crates/hir/src/diagnostics.rs | 31 | ||||
-rw-r--r-- | crates/hir/src/lib.rs | 6 | ||||
-rw-r--r-- | crates/ide/src/diagnostics.rs | 5 | ||||
-rw-r--r-- | crates/ide/src/diagnostics/fixes.rs | 1 | ||||
-rw-r--r-- | crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs (renamed from crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs) | 59 |
5 files changed, 44 insertions, 58 deletions
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index b144bb335..9afee0b90 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs | |||
@@ -37,6 +37,7 @@ diagnostics![ | |||
37 | MacroError, | 37 | MacroError, |
38 | MismatchedArgCount, | 38 | MismatchedArgCount, |
39 | MissingFields, | 39 | MissingFields, |
40 | MissingOkOrSomeInTailExpr, | ||
40 | MissingUnsafe, | 41 | MissingUnsafe, |
41 | NoSuchField, | 42 | NoSuchField, |
42 | RemoveThisSemicolon, | 43 | RemoveThisSemicolon, |
@@ -157,41 +158,13 @@ pub struct RemoveThisSemicolon { | |||
157 | pub expr: InFile<AstPtr<ast::Expr>>, | 158 | pub expr: InFile<AstPtr<ast::Expr>>, |
158 | } | 159 | } |
159 | 160 | ||
160 | // Diagnostic: missing-ok-or-some-in-tail-expr | ||
161 | // | ||
162 | // This diagnostic is triggered if a block that should return `Result` returns a value not wrapped in `Ok`, | ||
163 | // or if a block that should return `Option` returns a value not wrapped in `Some`. | ||
164 | // | ||
165 | // Example: | ||
166 | // | ||
167 | // ```rust | ||
168 | // fn foo() -> Result<u8, ()> { | ||
169 | // 10 | ||
170 | // } | ||
171 | // ``` | ||
172 | #[derive(Debug)] | 161 | #[derive(Debug)] |
173 | pub struct MissingOkOrSomeInTailExpr { | 162 | pub struct MissingOkOrSomeInTailExpr { |
174 | pub file: HirFileId, | 163 | pub expr: InFile<AstPtr<ast::Expr>>, |
175 | pub expr: AstPtr<ast::Expr>, | ||
176 | // `Some` or `Ok` depending on whether the return type is Result or Option | 164 | // `Some` or `Ok` depending on whether the return type is Result or Option |
177 | pub required: String, | 165 | pub required: String, |
178 | } | 166 | } |
179 | 167 | ||
180 | impl Diagnostic for MissingOkOrSomeInTailExpr { | ||
181 | fn code(&self) -> DiagnosticCode { | ||
182 | DiagnosticCode("missing-ok-or-some-in-tail-expr") | ||
183 | } | ||
184 | fn message(&self) -> String { | ||
185 | format!("wrap return expression in {}", self.required) | ||
186 | } | ||
187 | fn display_source(&self) -> InFile<SyntaxNodePtr> { | ||
188 | InFile { file_id: self.file, value: self.expr.clone().into() } | ||
189 | } | ||
190 | fn as_any(&self) -> &(dyn Any + Send + 'static) { | ||
191 | self | ||
192 | } | ||
193 | } | ||
194 | |||
195 | // Diagnostic: missing-match-arm | 168 | // Diagnostic: missing-match-arm |
196 | // | 169 | // |
197 | // This diagnostic is triggered if `match` block is missing one or more match arms. | 170 | // This diagnostic is triggered if `match` block is missing one or more match arms. |
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index cb9bf60b8..aaab5336a 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs | |||
@@ -1190,11 +1190,7 @@ impl Function { | |||
1190 | } | 1190 | } |
1191 | BodyValidationDiagnostic::MissingOkOrSomeInTailExpr { expr, required } => { | 1191 | BodyValidationDiagnostic::MissingOkOrSomeInTailExpr { expr, required } => { |
1192 | match source_map.expr_syntax(expr) { | 1192 | match source_map.expr_syntax(expr) { |
1193 | Ok(source_ptr) => sink.push(MissingOkOrSomeInTailExpr { | 1193 | Ok(expr) => acc.push(MissingOkOrSomeInTailExpr { expr, required }.into()), |
1194 | file: source_ptr.file_id, | ||
1195 | expr: source_ptr.value, | ||
1196 | required, | ||
1197 | }), | ||
1198 | Err(SyntheticSyntax) => (), | 1194 | Err(SyntheticSyntax) => (), |
1199 | } | 1195 | } |
1200 | } | 1196 | } |
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 3ced08f30..af282db0c 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs | |||
@@ -9,6 +9,7 @@ mod inactive_code; | |||
9 | mod macro_error; | 9 | mod macro_error; |
10 | mod mismatched_arg_count; | 10 | mod mismatched_arg_count; |
11 | mod missing_fields; | 11 | mod missing_fields; |
12 | mod missing_ok_or_some_in_tail_expr; | ||
12 | mod missing_unsafe; | 13 | mod missing_unsafe; |
13 | mod no_such_field; | 14 | mod no_such_field; |
14 | mod remove_this_semicolon; | 15 | mod remove_this_semicolon; |
@@ -163,9 +164,6 @@ pub(crate) fn diagnostics( | |||
163 | } | 164 | } |
164 | let res = RefCell::new(res); | 165 | let res = RefCell::new(res); |
165 | let sink_builder = DiagnosticSinkBuilder::new() | 166 | let sink_builder = DiagnosticSinkBuilder::new() |
166 | .on::<hir::diagnostics::MissingOkOrSomeInTailExpr, _>(|d| { | ||
167 | res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); | ||
168 | }) | ||
169 | .on::<hir::diagnostics::IncorrectCase, _>(|d| { | 167 | .on::<hir::diagnostics::IncorrectCase, _>(|d| { |
170 | res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); | 168 | res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); |
171 | }) | 169 | }) |
@@ -223,6 +221,7 @@ pub(crate) fn diagnostics( | |||
223 | AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), | 221 | AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), |
224 | AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), | 222 | AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), |
225 | AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), | 223 | AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), |
224 | AnyDiagnostic::MissingOkOrSomeInTailExpr(d) => missing_ok_or_some_in_tail_expr::missing_ok_or_some_in_tail_expr(&ctx, &d), | ||
226 | AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), | 225 | AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), |
227 | AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), | 226 | AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), |
228 | AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), | 227 | AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), |
diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 70f17881e..350575b3a 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs | |||
@@ -2,7 +2,6 @@ | |||
2 | //! The same module also has all curret custom fixes for the diagnostics implemented. | 2 | //! The same module also has all curret custom fixes for the diagnostics implemented. |
3 | mod change_case; | 3 | mod change_case; |
4 | mod replace_with_find_map; | 4 | mod replace_with_find_map; |
5 | mod wrap_tail_expr; | ||
6 | 5 | ||
7 | use hir::{diagnostics::Diagnostic, Semantics}; | 6 | use hir::{diagnostics::Diagnostic, Semantics}; |
8 | use ide_assists::AssistResolveStrategy; | 7 | use ide_assists::AssistResolveStrategy; |
diff --git a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs b/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs index dcb21e037..e27b54e66 100644 --- a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs +++ b/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs | |||
@@ -1,26 +1,45 @@ | |||
1 | use hir::{db::AstDatabase, diagnostics::MissingOkOrSomeInTailExpr, Semantics}; | 1 | use hir::{db::AstDatabase, Semantics}; |
2 | use ide_assists::{Assist, AssistResolveStrategy}; | 2 | use ide_assists::Assist; |
3 | use ide_db::{source_change::SourceChange, RootDatabase}; | 3 | use ide_db::source_change::SourceChange; |
4 | use syntax::AstNode; | 4 | use syntax::AstNode; |
5 | use text_edit::TextEdit; | 5 | use text_edit::TextEdit; |
6 | 6 | ||
7 | use crate::diagnostics::{fix, DiagnosticWithFixes}; | 7 | use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext}; |
8 | 8 | ||
9 | impl DiagnosticWithFixes for MissingOkOrSomeInTailExpr { | 9 | // Diagnostic: missing-ok-or-some-in-tail-expr |
10 | fn fixes( | 10 | // |
11 | &self, | 11 | // This diagnostic is triggered if a block that should return `Result` returns a value not wrapped in `Ok`, |
12 | sema: &Semantics<RootDatabase>, | 12 | // or if a block that should return `Option` returns a value not wrapped in `Some`. |
13 | _resolve: &AssistResolveStrategy, | 13 | // |
14 | ) -> Option<Vec<Assist>> { | 14 | // Example: |
15 | let root = sema.db.parse_or_expand(self.file)?; | 15 | // |
16 | let tail_expr = self.expr.to_node(&root); | 16 | // ```rust |
17 | let tail_expr_range = tail_expr.syntax().text_range(); | 17 | // fn foo() -> Result<u8, ()> { |
18 | let replacement = format!("{}({})", self.required, tail_expr.syntax()); | 18 | // 10 |
19 | let edit = TextEdit::replace(tail_expr_range, replacement); | 19 | // } |
20 | let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); | 20 | // ``` |
21 | let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" }; | 21 | pub(super) fn missing_ok_or_some_in_tail_expr( |
22 | Some(vec![fix("wrap_tail_expr", name, source_change, tail_expr_range)]) | 22 | ctx: &DiagnosticsContext<'_>, |
23 | } | 23 | d: &hir::MissingOkOrSomeInTailExpr, |
24 | ) -> Diagnostic { | ||
25 | Diagnostic::new( | ||
26 | "missing-ok-or-some-in-tail-expr", | ||
27 | format!("wrap return expression in {}", d.required), | ||
28 | ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, | ||
29 | ) | ||
30 | .with_fixes(fixes(ctx, d)) | ||
31 | } | ||
32 | |||
33 | fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingOkOrSomeInTailExpr) -> Option<Vec<Assist>> { | ||
34 | let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?; | ||
35 | let tail_expr = d.expr.value.to_node(&root); | ||
36 | let tail_expr_range = tail_expr.syntax().text_range(); | ||
37 | let replacement = format!("{}({})", d.required, tail_expr.syntax()); | ||
38 | let edit = TextEdit::replace(tail_expr_range, replacement); | ||
39 | let source_change = | ||
40 | SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit); | ||
41 | let name = if d.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" }; | ||
42 | Some(vec![fix("wrap_tail_expr", name, source_change, tail_expr_range)]) | ||
24 | } | 43 | } |
25 | 44 | ||
26 | #[cfg(test)] | 45 | #[cfg(test)] |