From cb66bb8ff9609d4fc3702ac1ed6197802b54e473 Mon Sep 17 00:00:00 2001 From: ivan770 Date: Tue, 8 Dec 2020 20:47:20 +0200 Subject: [PATCH 1/7] Remove this semicolon --- crates/hir/src/diagnostics.rs | 2 +- crates/hir_ty/src/diagnostics.rs | 24 ++++++++++++++++++++++++ crates/hir_ty/src/diagnostics/expr.rs | 27 ++++++++++++++++++++++++++- crates/ide/src/diagnostics.rs | 3 +++ crates/ide/src/diagnostics/fixes.rs | 26 ++++++++++++++++++++------ 5 files changed, 74 insertions(+), 8 deletions(-) diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index eaf1a14ec34..0f2ed4bb1c8 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -5,5 +5,5 @@ pub use hir_expand::diagnostics::{ }; pub use hir_ty::diagnostics::{ IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, - NoSuchField, + NoSuchField, RemoveThisSemicolon }; diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index b58fe0ed772..e59487e544b 100644 --- a/crates/hir_ty/src/diagnostics.rs +++ b/crates/hir_ty/src/diagnostics.rs @@ -216,6 +216,30 @@ impl Diagnostic for MissingOkInTailExpr { } } +#[derive(Debug)] +pub struct RemoveThisSemicolon { + pub file: HirFileId, + pub expr: AstPtr, +} + +impl Diagnostic for RemoveThisSemicolon { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("remove-this-semicolon") + } + + fn message(&self) -> String { + "Remove this semicolon".to_string() + } + + fn display_source(&self) -> InFile { + InFile { file_id: self.file, value: self.expr.clone().into() } + } + + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } +} + // Diagnostic: break-outside-of-loop // // This diagnostic is triggered if `break` keyword is used outside of a loop. diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 434b19354f2..313422968c7 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -2,7 +2,7 @@ use std::sync::Arc; -use hir_def::{path::path, resolver::HasResolver, AdtId, DefWithBodyId}; +use hir_def::{AdtId, DefWithBodyId, expr::Statement, path::path, resolver::HasResolver}; use hir_expand::diagnostics::DiagnosticSink; use rustc_hash::FxHashSet; use syntax::{ast, AstPtr}; @@ -23,6 +23,8 @@ pub(crate) use hir_def::{ LocalFieldId, VariantId, }; +use super::RemoveThisSemicolon; + pub(super) struct ExprValidator<'a, 'b: 'a> { owner: DefWithBodyId, infer: Arc, @@ -78,6 +80,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> { let body_expr = &body[body.body_expr]; if let Expr::Block { tail: Some(t), .. } = body_expr { self.validate_results_in_tail_expr(body.body_expr, *t, db); + } else { + if let Expr::Block { statements, .. } = body_expr { + if let Some(Statement::Expr(id)) = statements.last() { + self.validate_missing_tail_expr(body.body_expr, *id, db); + } + } } } @@ -317,6 +325,23 @@ impl<'a, 'b> ExprValidator<'a, 'b> { } } } + + fn validate_missing_tail_expr(&mut self, body_id: ExprId, possible_tail_id: ExprId, db: &dyn HirDatabase) { + let mismatch = match self.infer.type_mismatch_for_expr(body_id) { + Some(m) => m, + None => return, + }; + + if let Some(possible_tail_ty) = self.infer.type_of_expr.get(possible_tail_id) { + if mismatch.actual == Ty::unit() && mismatch.expected == *possible_tail_ty { + let (_, source_map) = db.body_with_source_map(self.owner.into()); + + if let Ok(source_ptr) = source_map.expr_syntax(possible_tail_id) { + self.sink.push(RemoveThisSemicolon { file: source_ptr.file_id, expr: source_ptr.value }); + } + } + } + } } pub fn record_literal_missing_fields( diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index c8453edb308..9157704dca4 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -131,6 +131,9 @@ pub(crate) fn diagnostics( .on::(|d| { res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) + .on::(|d| { + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); + }) .on::(|d| { res.borrow_mut().push(warning_with_fix(d, &sema)); }) diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index d275dd75ba7..cb4d49ad90d 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -4,7 +4,7 @@ use hir::{ db::AstDatabase, diagnostics::{ Diagnostic, IncorrectCase, MissingFields, MissingOkInTailExpr, NoSuchField, - UnresolvedModule, + RemoveThisSemicolon, UnresolvedModule, }, HasSource, HirDisplay, Semantics, VariantDef, }; @@ -13,11 +13,7 @@ use ide_db::{ source_change::{FileSystemEdit, SourceFileEdit}, RootDatabase, }; -use syntax::{ - algo, - ast::{self, edit::IndentLevel, make}, - AstNode, -}; +use syntax::{AstNode, Direction, T, algo, ast::{self, ExprStmt, edit::IndentLevel, make}}; use text_edit::TextEdit; use crate::{diagnostics::Fix, references::rename::rename_with_semantics, FilePosition}; @@ -102,6 +98,24 @@ impl DiagnosticWithFix for MissingOkInTailExpr { } } +impl DiagnosticWithFix for RemoveThisSemicolon { + fn fix(&self, sema: &Semantics) -> Option { + let root = sema.db.parse_or_expand(self.file)?; + + let semicolon = self.expr.to_node(&root) + .syntax() + .siblings_with_tokens(Direction::Next) + .filter_map(|it| it.into_token()) + .find(|it| it.kind() == T![;])? + .text_range(); + + let edit = TextEdit::delete(semicolon); + let source_change = SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into(); + + Some(Fix::new("Remove this semicolon", source_change, semicolon)) + } +} + impl DiagnosticWithFix for IncorrectCase { fn fix(&self, sema: &Semantics) -> Option { let root = sema.db.parse_or_expand(self.file)?; From 581567a4c8f418588bab5dbc760076c660111f81 Mon Sep 17 00:00:00 2001 From: ivan770 Date: Tue, 8 Dec 2020 20:50:13 +0200 Subject: [PATCH 2/7] Remove use via super --- crates/hir_ty/src/diagnostics/expr.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 313422968c7..6ec15c1808d 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -11,7 +11,7 @@ use crate::{ db::HirDatabase, diagnostics::{ match_check::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness}, - MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, + MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, RemoveThisSemicolon }, utils::variant_data, ApplicationTy, InferenceResult, Ty, TypeCtor, @@ -23,8 +23,6 @@ pub(crate) use hir_def::{ LocalFieldId, VariantId, }; -use super::RemoveThisSemicolon; - pub(super) struct ExprValidator<'a, 'b: 'a> { owner: DefWithBodyId, infer: Arc, From f2950a135036a608ccdcff3f5da77561927d6952 Mon Sep 17 00:00:00 2001 From: ivan770 Date: Tue, 8 Dec 2020 20:50:40 +0200 Subject: [PATCH 3/7] Remove unused use --- crates/ide/src/diagnostics/fixes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index cb4d49ad90d..7b05e87f202 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -13,7 +13,7 @@ use ide_db::{ source_change::{FileSystemEdit, SourceFileEdit}, RootDatabase, }; -use syntax::{AstNode, Direction, T, algo, ast::{self, ExprStmt, edit::IndentLevel, make}}; +use syntax::{AstNode, Direction, T, algo, ast::{self, edit::IndentLevel, make}}; use text_edit::TextEdit; use crate::{diagnostics::Fix, references::rename::rename_with_semantics, FilePosition}; From 7738467e0a11c9878d9e9486daeb0dc18d93b8e8 Mon Sep 17 00:00:00 2001 From: ivan770 Date: Tue, 8 Dec 2020 19:25:21 +0000 Subject: [PATCH 4/7] Format code --- crates/hir/src/diagnostics.rs | 2 +- crates/hir_ty/src/diagnostics/expr.rs | 17 +++++++++++++---- crates/ide/src/diagnostics/fixes.rs | 13 ++++++++++--- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 0f2ed4bb1c8..b1c9241670b 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -5,5 +5,5 @@ pub use hir_expand::diagnostics::{ }; pub use hir_ty::diagnostics::{ IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, - NoSuchField, RemoveThisSemicolon + NoSuchField, RemoveThisSemicolon, }; diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 6ec15c1808d..98b3cade215 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -2,7 +2,7 @@ use std::sync::Arc; -use hir_def::{AdtId, DefWithBodyId, expr::Statement, path::path, resolver::HasResolver}; +use hir_def::{expr::Statement, path::path, resolver::HasResolver, AdtId, DefWithBodyId}; use hir_expand::diagnostics::DiagnosticSink; use rustc_hash::FxHashSet; use syntax::{ast, AstPtr}; @@ -11,7 +11,8 @@ use crate::{ db::HirDatabase, diagnostics::{ match_check::{is_useful, MatchCheckCtx, Matrix, PatStack, Usefulness}, - MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, RemoveThisSemicolon + MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, MissingPatFields, + RemoveThisSemicolon, }, utils::variant_data, ApplicationTy, InferenceResult, Ty, TypeCtor, @@ -324,7 +325,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> { } } - fn validate_missing_tail_expr(&mut self, body_id: ExprId, possible_tail_id: ExprId, db: &dyn HirDatabase) { + fn validate_missing_tail_expr( + &mut self, + body_id: ExprId, + possible_tail_id: ExprId, + db: &dyn HirDatabase, + ) { let mismatch = match self.infer.type_mismatch_for_expr(body_id) { Some(m) => m, None => return, @@ -335,7 +341,10 @@ impl<'a, 'b> ExprValidator<'a, 'b> { let (_, source_map) = db.body_with_source_map(self.owner.into()); if let Ok(source_ptr) = source_map.expr_syntax(possible_tail_id) { - self.sink.push(RemoveThisSemicolon { file: source_ptr.file_id, expr: source_ptr.value }); + self.sink.push(RemoveThisSemicolon { + file: source_ptr.file_id, + expr: source_ptr.value, + }); } } } diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 7b05e87f202..c235b5bf443 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -13,7 +13,11 @@ use ide_db::{ source_change::{FileSystemEdit, SourceFileEdit}, RootDatabase, }; -use syntax::{AstNode, Direction, T, algo, ast::{self, edit::IndentLevel, make}}; +use syntax::{ + algo, + ast::{self, edit::IndentLevel, make}, + AstNode, Direction, T, +}; use text_edit::TextEdit; use crate::{diagnostics::Fix, references::rename::rename_with_semantics, FilePosition}; @@ -102,7 +106,9 @@ impl DiagnosticWithFix for RemoveThisSemicolon { fn fix(&self, sema: &Semantics) -> Option { let root = sema.db.parse_or_expand(self.file)?; - let semicolon = self.expr.to_node(&root) + let semicolon = self + .expr + .to_node(&root) .syntax() .siblings_with_tokens(Direction::Next) .filter_map(|it| it.into_token()) @@ -110,7 +116,8 @@ impl DiagnosticWithFix for RemoveThisSemicolon { .text_range(); let edit = TextEdit::delete(semicolon); - let source_change = 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("Remove this semicolon", source_change, semicolon)) } From 86c183716cc6dfd44da15c86a7726d0a117c214e Mon Sep 17 00:00:00 2001 From: ivan770 Date: Wed, 9 Dec 2020 10:17:28 +0200 Subject: [PATCH 5/7] Apply suggestions from code review Co-authored-by: bjorn3 --- crates/hir_ty/src/diagnostics/expr.rs | 37 +++++++++++++++------------ 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 98b3cade215..27ffcf29108 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -77,13 +77,11 @@ impl<'a, 'b> ExprValidator<'a, 'b> { } } let body_expr = &body[body.body_expr]; - if let Expr::Block { tail: Some(t), .. } = body_expr { - self.validate_results_in_tail_expr(body.body_expr, *t, db); - } else { - if let Expr::Block { statements, .. } = body_expr { - if let Some(Statement::Expr(id)) = statements.last() { - self.validate_missing_tail_expr(body.body_expr, *id, db); - } + if let Expr::Block { statements, tail, .. } = body_expr { + if let Some(t) = tail { + self.validate_results_in_tail_expr(body.body_expr, *t, db); + } else if let Some(Statement::Expr(id)) = statements.last() { + self.validate_missing_tail_expr(body.body_expr, *id, db); } } } @@ -336,17 +334,22 @@ impl<'a, 'b> ExprValidator<'a, 'b> { None => return, }; - if let Some(possible_tail_ty) = self.infer.type_of_expr.get(possible_tail_id) { - if mismatch.actual == Ty::unit() && mismatch.expected == *possible_tail_ty { - let (_, source_map) = db.body_with_source_map(self.owner.into()); + let possible_tail_ty = if let Some(possible_tail_ty) = self.infer.type_of_expr.get(possible_tail_id) { + possible_tail_ty + } else { + return; + }; - if let Ok(source_ptr) = source_map.expr_syntax(possible_tail_id) { - self.sink.push(RemoveThisSemicolon { - file: source_ptr.file_id, - expr: source_ptr.value, - }); - } - } + if mismatch.actual != Ty::unit() || mismatch.expected != *possible_tail_ty { + return; + } + + let (_, source_map) = db.body_with_source_map(self.owner.into()); + if let Ok(source_ptr) = source_map.expr_syntax(possible_tail_id) { + self.sink.push(RemoveThisSemicolon { + file: source_ptr.file_id, + expr: source_ptr.value, + }); } } } From 35006eba79b9a445c4ebec66f1a93b3170398e0e Mon Sep 17 00:00:00 2001 From: ivan770 Date: Wed, 9 Dec 2020 08:22:13 +0000 Subject: [PATCH 6/7] Apply rustfmt changes --- crates/hir_ty/src/diagnostics/expr.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 27ffcf29108..9e461e0b05a 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -334,11 +334,12 @@ impl<'a, 'b> ExprValidator<'a, 'b> { None => return, }; - let possible_tail_ty = if let Some(possible_tail_ty) = self.infer.type_of_expr.get(possible_tail_id) { - possible_tail_ty - } else { - return; - }; + let possible_tail_ty = + if let Some(possible_tail_ty) = self.infer.type_of_expr.get(possible_tail_id) { + possible_tail_ty + } else { + return; + }; if mismatch.actual != Ty::unit() || mismatch.expected != *possible_tail_ty { return; @@ -346,10 +347,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> { let (_, source_map) = db.body_with_source_map(self.owner.into()); if let Ok(source_ptr) = source_map.expr_syntax(possible_tail_id) { - self.sink.push(RemoveThisSemicolon { - file: source_ptr.file_id, - expr: source_ptr.value, - }); + self.sink + .push(RemoveThisSemicolon { file: source_ptr.file_id, expr: source_ptr.value }); } } } From bbb0bc7b041278480edbfaa7c3cdadc5a704fc03 Mon Sep 17 00:00:00 2001 From: ivan770 Date: Thu, 10 Dec 2020 18:10:39 +0200 Subject: [PATCH 7/7] Cast to ExprStmt, style fixes --- crates/hir_ty/src/diagnostics/expr.rs | 11 +++++------ crates/ide/src/diagnostics/fixes.rs | 8 ++++---- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 9e461e0b05a..84941570630 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs @@ -334,18 +334,17 @@ impl<'a, 'b> ExprValidator<'a, 'b> { None => return, }; - let possible_tail_ty = - if let Some(possible_tail_ty) = self.infer.type_of_expr.get(possible_tail_id) { - possible_tail_ty - } else { - return; - }; + let possible_tail_ty = match self.infer.type_of_expr.get(possible_tail_id) { + Some(ty) => ty, + None => return, + }; if mismatch.actual != Ty::unit() || mismatch.expected != *possible_tail_ty { return; } let (_, source_map) = db.body_with_source_map(self.owner.into()); + if let Ok(source_ptr) = source_map.expr_syntax(possible_tail_id) { self.sink .push(RemoveThisSemicolon { file: source_ptr.file_id, expr: source_ptr.value }); diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index c235b5bf443..ba046232aa0 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -16,7 +16,7 @@ use ide_db::{ use syntax::{ algo, ast::{self, edit::IndentLevel, make}, - AstNode, Direction, T, + AstNode, }; use text_edit::TextEdit; @@ -110,9 +110,9 @@ impl DiagnosticWithFix for RemoveThisSemicolon { .expr .to_node(&root) .syntax() - .siblings_with_tokens(Direction::Next) - .filter_map(|it| it.into_token()) - .find(|it| it.kind() == T![;])? + .parent() + .and_then(ast::ExprStmt::cast) + .and_then(|expr| expr.semicolon_token())? .text_range(); let edit = TextEdit::delete(semicolon);