aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPhil Ellison <[email protected]>2020-12-28 13:41:15 +0000
committerPhil Ellison <[email protected]>2021-01-23 07:40:24 +0000
commit1316422a7c2ef26e9da78fa23f170407b1cb39bb (patch)
tree1eea156207bcc920f0573ef51d7f6ad4fe03299d
parenteab5db20edd9604ba5d489fa8c6430eb7bac6610 (diff)
Add diagnostic for filter_map followed by next
-rw-r--r--crates/hir/src/diagnostics.rs11
-rw-r--r--crates/hir_ty/src/diagnostics.rs48
-rw-r--r--crates/hir_ty/src/diagnostics/expr.rs70
-rw-r--r--crates/ide/src/diagnostics.rs3
-rw-r--r--crates/ide/src/diagnostics/fixes.rs33
5 files changed, 150 insertions, 15 deletions
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
index 447faa04f..f7fd3e237 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@@ -5,5 +5,14 @@ pub use hir_expand::diagnostics::{
5}; 5};
6pub use hir_ty::diagnostics::{ 6pub use hir_ty::diagnostics::{
7 IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, 7 IncorrectCase, MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr,
8 NoSuchField, RemoveThisSemicolon, 8 NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap,
9}; 9};
10
11// PHIL:
12// hir/src/diagnostics.rs - just pub uses the type from hir_ty::diagnostics (DONE)
13// hir_ty/src/diagnostics.rs - defines the type (DONE)
14// hir_ty/src/diagnostics.rs - plus a test (DONE) <--- one example found, need to copy the not-applicable tests from the assist version
15// ide/src/diagnostics.rs - define handler for when this diagnostic is raised (DONE)
16
17// ide/src/diagnostics/fixes.rs - pulls in type from hir, and impls DiagnosticWithFix (TODO)
18// hir_ty/src/diagnostics/expr.rs - do the real work (TODO)
diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs
index 247da43f2..b4cf81c10 100644
--- a/crates/hir_ty/src/diagnostics.rs
+++ b/crates/hir_ty/src/diagnostics.rs
@@ -247,7 +247,7 @@ impl Diagnostic for RemoveThisSemicolon {
247 247
248// Diagnostic: break-outside-of-loop 248// Diagnostic: break-outside-of-loop
249// 249//
250// This diagnostic is triggered if `break` keyword is used outside of a loop. 250// This diagnostic is triggered if the `break` keyword is used outside of a loop.
251#[derive(Debug)] 251#[derive(Debug)]
252pub struct BreakOutsideOfLoop { 252pub struct BreakOutsideOfLoop {
253 pub file: HirFileId, 253 pub file: HirFileId,
@@ -271,7 +271,7 @@ impl Diagnostic for BreakOutsideOfLoop {
271 271
272// Diagnostic: missing-unsafe 272// Diagnostic: missing-unsafe
273// 273//
274// This diagnostic is triggered if operation marked as `unsafe` is used outside of `unsafe` function or block. 274// This diagnostic is triggered if an operation marked as `unsafe` is used outside of an `unsafe` function or block.
275#[derive(Debug)] 275#[derive(Debug)]
276pub struct MissingUnsafe { 276pub struct MissingUnsafe {
277 pub file: HirFileId, 277 pub file: HirFileId,
@@ -295,7 +295,7 @@ impl Diagnostic for MissingUnsafe {
295 295
296// Diagnostic: mismatched-arg-count 296// Diagnostic: mismatched-arg-count
297// 297//
298// This diagnostic is triggered if function is invoked with an incorrect amount of arguments. 298// This diagnostic is triggered if a function is invoked with an incorrect amount of arguments.
299#[derive(Debug)] 299#[derive(Debug)]
300pub struct MismatchedArgCount { 300pub struct MismatchedArgCount {
301 pub file: HirFileId, 301 pub file: HirFileId,
@@ -347,7 +347,7 @@ impl fmt::Display for CaseType {
347 347
348// Diagnostic: incorrect-ident-case 348// Diagnostic: incorrect-ident-case
349// 349//
350// This diagnostic is triggered if item name doesn't follow https://doc.rust-lang.org/1.0.0/style/style/naming/README.html[Rust naming convention]. 350// This diagnostic is triggered if an item name doesn't follow https://doc.rust-lang.org/1.0.0/style/style/naming/README.html[Rust naming convention].
351#[derive(Debug)] 351#[derive(Debug)]
352pub struct IncorrectCase { 352pub struct IncorrectCase {
353 pub file: HirFileId, 353 pub file: HirFileId,
@@ -386,6 +386,31 @@ impl Diagnostic for IncorrectCase {
386 } 386 }
387} 387}
388 388
389// Diagnostic: replace-filter-map-next-with-find-map
390//
391// This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`.
392#[derive(Debug)]
393pub struct ReplaceFilterMapNextWithFindMap {
394 pub file: HirFileId,
395 pub filter_map_expr: AstPtr<ast::Expr>,
396 pub next_expr: AstPtr<ast::Expr>,
397}
398
399impl Diagnostic for ReplaceFilterMapNextWithFindMap {
400 fn code(&self) -> DiagnosticCode {
401 DiagnosticCode("replace-filter-map-next-with-find-map")
402 }
403 fn message(&self) -> String {
404 "replace filter_map(..).next() with find_map(..)".to_string()
405 }
406 fn display_source(&self) -> InFile<SyntaxNodePtr> {
407 InFile { file_id: self.file, value: self.filter_map_expr.clone().into() }
408 }
409 fn as_any(&self) -> &(dyn Any + Send + 'static) {
410 self
411 }
412}
413
389#[cfg(test)] 414#[cfg(test)]
390mod tests { 415mod tests {
391 use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; 416 use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt};
@@ -644,4 +669,19 @@ fn foo() { break; }
644 "#, 669 "#,
645 ); 670 );
646 } 671 }
672
673 #[test]
674 fn replace_missing_filter_next_with_find_map() {
675 check_diagnostics(
676 r#"
677 fn foo() {
678 let m = [1, 2, 3]
679 .iter()
680 .filter_map(|x| if *x == 2 { Some (4) } else { None })
681 .next();
682 //^^^ Replace .filter_map(..).next() with .find_map(..)
683 }
684 "#,
685 );
686 }
647} 687}
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs
index 107417c27..170d23178 100644
--- a/crates/hir_ty/src/diagnostics/expr.rs
+++ b/crates/hir_ty/src/diagnostics/expr.rs
@@ -24,6 +24,8 @@ pub(crate) use hir_def::{
24 LocalFieldId, VariantId, 24 LocalFieldId, VariantId,
25}; 25};
26 26
27use super::ReplaceFilterMapNextWithFindMap;
28
27pub(super) struct ExprValidator<'a, 'b: 'a> { 29pub(super) struct ExprValidator<'a, 'b: 'a> {
28 owner: DefWithBodyId, 30 owner: DefWithBodyId,
29 infer: Arc<InferenceResult>, 31 infer: Arc<InferenceResult>,
@@ -39,7 +41,18 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
39 ExprValidator { owner, infer, sink } 41 ExprValidator { owner, infer, sink }
40 } 42 }
41 43
44 fn bar() {
45 // LOOK FOR THIS
46 let m = [1, 2, 3]
47 .iter()
48 .filter_map(|x| if *x == 2 { Some(4) } else { None })
49 .next();
50 }
51
42 pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) { 52 pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) {
53 // DO NOT MERGE: just getting something working for now
54 self.check_for_filter_map_next(db);
55
43 let body = db.body(self.owner.into()); 56 let body = db.body(self.owner.into());
44 57
45 for (id, expr) in body.exprs.iter() { 58 for (id, expr) in body.exprs.iter() {
@@ -150,20 +163,58 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
150 } 163 }
151 } 164 }
152 165
153 fn validate_call(&mut self, db: &dyn HirDatabase, call_id: ExprId, expr: &Expr) -> Option<()> { 166 fn check_for_filter_map_next(&mut self, db: &dyn HirDatabase) {
167 let body = db.body(self.owner.into());
168 let mut prev = None;
169
170 for (id, expr) in body.exprs.iter() {
171 if let Expr::MethodCall { receiver, method_name, args, .. } = expr {
172 let method_name_hack_do_not_merge = format!("{}", method_name);
173
174 if method_name_hack_do_not_merge == "filter_map" && args.len() == 1 {
175 prev = Some((id, args[0]));
176 continue;
177 }
178
179 if method_name_hack_do_not_merge == "next" {
180 if let Some((filter_map_id, filter_map_args)) = prev {
181 if *receiver == filter_map_id {
182 let (_, source_map) = db.body_with_source_map(self.owner.into());
183 if let (Ok(filter_map_source_ptr), Ok(next_source_ptr)) = (
184 source_map.expr_syntax(filter_map_id),
185 source_map.expr_syntax(id),
186 ) {
187 self.sink.push(ReplaceFilterMapNextWithFindMap {
188 file: filter_map_source_ptr.file_id,
189 filter_map_expr: filter_map_source_ptr.value,
190 next_expr: next_source_ptr.value,
191 });
192 }
193 }
194 }
195 }
196 }
197 prev = None;
198 }
199 }
200
201 fn validate_call(&mut self, db: &dyn HirDatabase, call_id: ExprId, expr: &Expr) {
154 // Check that the number of arguments matches the number of parameters. 202 // Check that the number of arguments matches the number of parameters.
155 203
156 // FIXME: Due to shortcomings in the current type system implementation, only emit this 204 // FIXME: Due to shortcomings in the current type system implementation, only emit this
157 // diagnostic if there are no type mismatches in the containing function. 205 // diagnostic if there are no type mismatches in the containing function.
158 if self.infer.type_mismatches.iter().next().is_some() { 206 if self.infer.type_mismatches.iter().next().is_some() {
159 return None; 207 return;
160 } 208 }
161 209
162 let is_method_call = matches!(expr, Expr::MethodCall { .. }); 210 let is_method_call = matches!(expr, Expr::MethodCall { .. });
163 let (sig, args) = match expr { 211 let (sig, args) = match expr {
164 Expr::Call { callee, args } => { 212 Expr::Call { callee, args } => {
165 let callee = &self.infer.type_of_expr[*callee]; 213 let callee = &self.infer.type_of_expr[*callee];
166 let sig = callee.callable_sig(db)?; 214 let sig = match callee.callable_sig(db) {
215 Some(sig) => sig,
216 None => return,
217 };
167 (sig, args.clone()) 218 (sig, args.clone())
168 } 219 }
169 Expr::MethodCall { receiver, args, .. } => { 220 Expr::MethodCall { receiver, args, .. } => {
@@ -175,22 +226,25 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
175 // if the receiver is of unknown type, it's very likely we 226 // if the receiver is of unknown type, it's very likely we
176 // don't know enough to correctly resolve the method call. 227 // don't know enough to correctly resolve the method call.
177 // This is kind of a band-aid for #6975. 228 // This is kind of a band-aid for #6975.
178 return None; 229 return;
179 } 230 }
180 231
181 // FIXME: note that we erase information about substs here. This 232 // FIXME: note that we erase information about substs here. This
182 // is not right, but, luckily, doesn't matter as we care only 233 // is not right, but, luckily, doesn't matter as we care only
183 // about the number of params 234 // about the number of params
184 let callee = self.infer.method_resolution(call_id)?; 235 let callee = match self.infer.method_resolution(call_id) {
236 Some(callee) => callee,
237 None => return,
238 };
185 let sig = db.callable_item_signature(callee.into()).value; 239 let sig = db.callable_item_signature(callee.into()).value;
186 240
187 (sig, args) 241 (sig, args)
188 } 242 }
189 _ => return None, 243 _ => return,
190 }; 244 };
191 245
192 if sig.is_varargs { 246 if sig.is_varargs {
193 return None; 247 return;
194 } 248 }
195 249
196 let params = sig.params(); 250 let params = sig.params();
@@ -213,8 +267,6 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
213 }); 267 });
214 } 268 }
215 } 269 }
216
217 None
218 } 270 }
219 271
220 fn validate_match( 272 fn validate_match(
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs
index b35bc2bae..8607139ba 100644
--- a/crates/ide/src/diagnostics.rs
+++ b/crates/ide/src/diagnostics.rs
@@ -136,6 +136,9 @@ pub(crate) fn diagnostics(
136 .on::<hir::diagnostics::IncorrectCase, _>(|d| { 136 .on::<hir::diagnostics::IncorrectCase, _>(|d| {
137 res.borrow_mut().push(warning_with_fix(d, &sema)); 137 res.borrow_mut().push(warning_with_fix(d, &sema));
138 }) 138 })
139 .on::<hir::diagnostics::ReplaceFilterMapNextWithFindMap, _>(|d| {
140 res.borrow_mut().push(warning_with_fix(d, &sema));
141 })
139 .on::<hir::diagnostics::InactiveCode, _>(|d| { 142 .on::<hir::diagnostics::InactiveCode, _>(|d| {
140 // If there's inactive code somewhere in a macro, don't propagate to the call-site. 143 // If there's inactive code somewhere in a macro, don't propagate to the call-site.
141 if d.display_source().file_id.expansion_info(db).is_some() { 144 if d.display_source().file_id.expansion_info(db).is_some() {
diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs
index 579d5a308..eafbac43a 100644
--- a/crates/ide/src/diagnostics/fixes.rs
+++ b/crates/ide/src/diagnostics/fixes.rs
@@ -4,7 +4,7 @@ use hir::{
4 db::AstDatabase, 4 db::AstDatabase,
5 diagnostics::{ 5 diagnostics::{
6 Diagnostic, IncorrectCase, MissingFields, MissingOkOrSomeInTailExpr, NoSuchField, 6 Diagnostic, IncorrectCase, MissingFields, MissingOkOrSomeInTailExpr, NoSuchField,
7 RemoveThisSemicolon, UnresolvedModule, 7 RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, UnresolvedModule,
8 }, 8 },
9 HasSource, HirDisplay, InFile, Semantics, VariantDef, 9 HasSource, HirDisplay, InFile, Semantics, VariantDef,
10}; 10};
@@ -144,6 +144,37 @@ impl DiagnosticWithFix for IncorrectCase {
144 } 144 }
145} 145}
146 146
147// Bugs:
148// * Action is applicable for both iter() and filter_map() rows
149// * Action deletes the entire method chain
150impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap {
151 fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
152 let root = sema.db.parse_or_expand(self.file)?;
153
154 let next_expr = self.next_expr.to_node(&root);
155 let next_expr_range = next_expr.syntax().text_range();
156
157 let filter_map_expr = self.filter_map_expr.to_node(&root);
158 let filter_map_expr_range = filter_map_expr.syntax().text_range();
159
160 let edit = TextEdit::delete(next_expr_range);
161
162 // This is the entire method chain, including the array literal
163 eprintln!("NEXT EXPR: {:#?}", next_expr);
164 // This is the entire method chain except for the final next()
165 eprintln!("FILTER MAP EXPR: {:#?}", filter_map_expr);
166
167 let source_change =
168 SourceFileEdit { file_id: self.file.original_file(sema.db), edit }.into();
169
170 Some(Fix::new(
171 "Replace filter_map(..).next() with find_map()",
172 source_change,
173 filter_map_expr_range,
174 ))
175 }
176}
177
147fn missing_record_expr_field_fix( 178fn missing_record_expr_field_fix(
148 sema: &Semantics<RootDatabase>, 179 sema: &Semantics<RootDatabase>,
149 usage_file_id: FileId, 180 usage_file_id: FileId,