diff options
Diffstat (limited to 'crates/hir_ty/src')
-rw-r--r-- | crates/hir_ty/src/diagnostics.rs | 116 | ||||
-rw-r--r-- | crates/hir_ty/src/diagnostics/expr.rs | 85 |
2 files changed, 186 insertions, 15 deletions
diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index 247da43f2..323c5f963 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)] |
252 | pub struct BreakOutsideOfLoop { | 252 | pub 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)] |
276 | pub struct MissingUnsafe { | 276 | pub 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)] |
300 | pub struct MismatchedArgCount { | 300 | pub 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)] |
352 | pub struct IncorrectCase { | 352 | pub 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)] | ||
393 | pub struct ReplaceFilterMapNextWithFindMap { | ||
394 | pub file: HirFileId, | ||
395 | /// This expression is the whole method chain up to and including `.filter_map(..).next()`. | ||
396 | pub next_expr: AstPtr<ast::Expr>, | ||
397 | } | ||
398 | |||
399 | impl 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.next_expr.clone().into() } | ||
408 | } | ||
409 | fn as_any(&self) -> &(dyn Any + Send + 'static) { | ||
410 | self | ||
411 | } | ||
412 | } | ||
413 | |||
389 | #[cfg(test)] | 414 | #[cfg(test)] |
390 | mod tests { | 415 | mod tests { |
391 | use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; | 416 | use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; |
@@ -644,4 +669,87 @@ fn foo() { break; } | |||
644 | "#, | 669 | "#, |
645 | ); | 670 | ); |
646 | } | 671 | } |
672 | |||
673 | // Register the required standard library types to make the tests work | ||
674 | fn add_filter_map_with_find_next_boilerplate(body: &str) -> String { | ||
675 | let prefix = r#" | ||
676 | //- /main.rs crate:main deps:core | ||
677 | use core::iter::Iterator; | ||
678 | use core::option::Option::{self, Some, None}; | ||
679 | "#; | ||
680 | let suffix = r#" | ||
681 | //- /core/lib.rs crate:core | ||
682 | pub mod option { | ||
683 | pub enum Option<T> { Some(T), None } | ||
684 | } | ||
685 | pub mod iter { | ||
686 | pub trait Iterator { | ||
687 | type Item; | ||
688 | fn filter_map<B, F>(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option<B> { FilterMap } | ||
689 | fn next(&mut self) -> Option<Self::Item>; | ||
690 | } | ||
691 | pub struct FilterMap {} | ||
692 | impl Iterator for FilterMap { | ||
693 | type Item = i32; | ||
694 | fn next(&mut self) -> i32 { 7 } | ||
695 | } | ||
696 | } | ||
697 | "#; | ||
698 | format!("{}{}{}", prefix, body, suffix) | ||
699 | } | ||
700 | |||
701 | #[test] | ||
702 | fn replace_filter_map_next_with_find_map2() { | ||
703 | check_diagnostics(&add_filter_map_with_find_next_boilerplate( | ||
704 | r#" | ||
705 | fn foo() { | ||
706 | let m = [1, 2, 3].iter().filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); | ||
707 | //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ replace filter_map(..).next() with find_map(..) | ||
708 | } | ||
709 | "#, | ||
710 | )); | ||
711 | } | ||
712 | |||
713 | #[test] | ||
714 | fn replace_filter_map_next_with_find_map_no_diagnostic_without_next() { | ||
715 | check_diagnostics(&add_filter_map_with_find_next_boilerplate( | ||
716 | r#" | ||
717 | fn foo() { | ||
718 | let m = [1, 2, 3] | ||
719 | .iter() | ||
720 | .filter_map(|x| if *x == 2 { Some (4) } else { None }) | ||
721 | .len(); | ||
722 | } | ||
723 | "#, | ||
724 | )); | ||
725 | } | ||
726 | |||
727 | #[test] | ||
728 | fn replace_filter_map_next_with_find_map_no_diagnostic_with_intervening_methods() { | ||
729 | check_diagnostics(&add_filter_map_with_find_next_boilerplate( | ||
730 | r#" | ||
731 | fn foo() { | ||
732 | let m = [1, 2, 3] | ||
733 | .iter() | ||
734 | .filter_map(|x| if *x == 2 { Some (4) } else { None }) | ||
735 | .map(|x| x + 2) | ||
736 | .len(); | ||
737 | } | ||
738 | "#, | ||
739 | )); | ||
740 | } | ||
741 | |||
742 | #[test] | ||
743 | fn replace_filter_map_next_with_find_map_no_diagnostic_if_not_in_chain() { | ||
744 | check_diagnostics(&add_filter_map_with_find_next_boilerplate( | ||
745 | r#" | ||
746 | fn foo() { | ||
747 | let m = [1, 2, 3] | ||
748 | .iter() | ||
749 | .filter_map(|x| if *x == 2 { Some (4) } else { None }); | ||
750 | let n = m.next(); | ||
751 | } | ||
752 | "#, | ||
753 | )); | ||
754 | } | ||
647 | } | 755 | } |
diff --git a/crates/hir_ty/src/diagnostics/expr.rs b/crates/hir_ty/src/diagnostics/expr.rs index 107417c27..d740b7265 100644 --- a/crates/hir_ty/src/diagnostics/expr.rs +++ b/crates/hir_ty/src/diagnostics/expr.rs | |||
@@ -2,8 +2,10 @@ | |||
2 | 2 | ||
3 | use std::sync::Arc; | 3 | use std::sync::Arc; |
4 | 4 | ||
5 | use hir_def::{expr::Statement, path::path, resolver::HasResolver, AdtId, DefWithBodyId}; | 5 | use hir_def::{ |
6 | use hir_expand::diagnostics::DiagnosticSink; | 6 | expr::Statement, path::path, resolver::HasResolver, AdtId, AssocItemId, DefWithBodyId, |
7 | }; | ||
8 | use hir_expand::{diagnostics::DiagnosticSink, name}; | ||
7 | use rustc_hash::FxHashSet; | 9 | use rustc_hash::FxHashSet; |
8 | use syntax::{ast, AstPtr}; | 10 | use syntax::{ast, AstPtr}; |
9 | 11 | ||
@@ -24,6 +26,8 @@ pub(crate) use hir_def::{ | |||
24 | LocalFieldId, VariantId, | 26 | LocalFieldId, VariantId, |
25 | }; | 27 | }; |
26 | 28 | ||
29 | use super::ReplaceFilterMapNextWithFindMap; | ||
30 | |||
27 | pub(super) struct ExprValidator<'a, 'b: 'a> { | 31 | pub(super) struct ExprValidator<'a, 'b: 'a> { |
28 | owner: DefWithBodyId, | 32 | owner: DefWithBodyId, |
29 | infer: Arc<InferenceResult>, | 33 | infer: Arc<InferenceResult>, |
@@ -40,6 +44,8 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
40 | } | 44 | } |
41 | 45 | ||
42 | pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) { | 46 | pub(super) fn validate_body(&mut self, db: &dyn HirDatabase) { |
47 | self.check_for_filter_map_next(db); | ||
48 | |||
43 | let body = db.body(self.owner.into()); | 49 | let body = db.body(self.owner.into()); |
44 | 50 | ||
45 | for (id, expr) in body.exprs.iter() { | 51 | for (id, expr) in body.exprs.iter() { |
@@ -150,20 +156,76 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
150 | } | 156 | } |
151 | } | 157 | } |
152 | 158 | ||
153 | fn validate_call(&mut self, db: &dyn HirDatabase, call_id: ExprId, expr: &Expr) -> Option<()> { | 159 | fn check_for_filter_map_next(&mut self, db: &dyn HirDatabase) { |
160 | // Find the FunctionIds for Iterator::filter_map and Iterator::next | ||
161 | let iterator_path = path![core::iter::Iterator]; | ||
162 | let resolver = self.owner.resolver(db.upcast()); | ||
163 | let iterator_trait_id = match resolver.resolve_known_trait(db.upcast(), &iterator_path) { | ||
164 | Some(id) => id, | ||
165 | None => return, | ||
166 | }; | ||
167 | let iterator_trait_items = &db.trait_data(iterator_trait_id).items; | ||
168 | let filter_map_function_id = | ||
169 | match iterator_trait_items.iter().find(|item| item.0 == name![filter_map]) { | ||
170 | Some((_, AssocItemId::FunctionId(id))) => id, | ||
171 | _ => return, | ||
172 | }; | ||
173 | let next_function_id = match iterator_trait_items.iter().find(|item| item.0 == name![next]) | ||
174 | { | ||
175 | Some((_, AssocItemId::FunctionId(id))) => id, | ||
176 | _ => return, | ||
177 | }; | ||
178 | |||
179 | // Search function body for instances of .filter_map(..).next() | ||
180 | let body = db.body(self.owner.into()); | ||
181 | let mut prev = None; | ||
182 | for (id, expr) in body.exprs.iter() { | ||
183 | if let Expr::MethodCall { receiver, .. } = expr { | ||
184 | let function_id = match self.infer.method_resolution(id) { | ||
185 | Some(id) => id, | ||
186 | None => continue, | ||
187 | }; | ||
188 | |||
189 | if function_id == *filter_map_function_id { | ||
190 | prev = Some(id); | ||
191 | continue; | ||
192 | } | ||
193 | |||
194 | if function_id == *next_function_id { | ||
195 | if let Some(filter_map_id) = prev { | ||
196 | if *receiver == filter_map_id { | ||
197 | let (_, source_map) = db.body_with_source_map(self.owner.into()); | ||
198 | if let Ok(next_source_ptr) = source_map.expr_syntax(id) { | ||
199 | self.sink.push(ReplaceFilterMapNextWithFindMap { | ||
200 | file: next_source_ptr.file_id, | ||
201 | next_expr: next_source_ptr.value, | ||
202 | }); | ||
203 | } | ||
204 | } | ||
205 | } | ||
206 | } | ||
207 | } | ||
208 | prev = None; | ||
209 | } | ||
210 | } | ||
211 | |||
212 | 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. | 213 | // Check that the number of arguments matches the number of parameters. |
155 | 214 | ||
156 | // FIXME: Due to shortcomings in the current type system implementation, only emit this | 215 | // 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. | 216 | // diagnostic if there are no type mismatches in the containing function. |
158 | if self.infer.type_mismatches.iter().next().is_some() { | 217 | if self.infer.type_mismatches.iter().next().is_some() { |
159 | return None; | 218 | return; |
160 | } | 219 | } |
161 | 220 | ||
162 | let is_method_call = matches!(expr, Expr::MethodCall { .. }); | 221 | let is_method_call = matches!(expr, Expr::MethodCall { .. }); |
163 | let (sig, args) = match expr { | 222 | let (sig, args) = match expr { |
164 | Expr::Call { callee, args } => { | 223 | Expr::Call { callee, args } => { |
165 | let callee = &self.infer.type_of_expr[*callee]; | 224 | let callee = &self.infer.type_of_expr[*callee]; |
166 | let sig = callee.callable_sig(db)?; | 225 | let sig = match callee.callable_sig(db) { |
226 | Some(sig) => sig, | ||
227 | None => return, | ||
228 | }; | ||
167 | (sig, args.clone()) | 229 | (sig, args.clone()) |
168 | } | 230 | } |
169 | Expr::MethodCall { receiver, args, .. } => { | 231 | Expr::MethodCall { receiver, args, .. } => { |
@@ -175,22 +237,25 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
175 | // if the receiver is of unknown type, it's very likely we | 237 | // if the receiver is of unknown type, it's very likely we |
176 | // don't know enough to correctly resolve the method call. | 238 | // don't know enough to correctly resolve the method call. |
177 | // This is kind of a band-aid for #6975. | 239 | // This is kind of a band-aid for #6975. |
178 | return None; | 240 | return; |
179 | } | 241 | } |
180 | 242 | ||
181 | // FIXME: note that we erase information about substs here. This | 243 | // FIXME: note that we erase information about substs here. This |
182 | // is not right, but, luckily, doesn't matter as we care only | 244 | // is not right, but, luckily, doesn't matter as we care only |
183 | // about the number of params | 245 | // about the number of params |
184 | let callee = self.infer.method_resolution(call_id)?; | 246 | let callee = match self.infer.method_resolution(call_id) { |
247 | Some(callee) => callee, | ||
248 | None => return, | ||
249 | }; | ||
185 | let sig = db.callable_item_signature(callee.into()).value; | 250 | let sig = db.callable_item_signature(callee.into()).value; |
186 | 251 | ||
187 | (sig, args) | 252 | (sig, args) |
188 | } | 253 | } |
189 | _ => return None, | 254 | _ => return, |
190 | }; | 255 | }; |
191 | 256 | ||
192 | if sig.is_varargs { | 257 | if sig.is_varargs { |
193 | return None; | 258 | return; |
194 | } | 259 | } |
195 | 260 | ||
196 | let params = sig.params(); | 261 | let params = sig.params(); |
@@ -213,8 +278,6 @@ impl<'a, 'b> ExprValidator<'a, 'b> { | |||
213 | }); | 278 | }); |
214 | } | 279 | } |
215 | } | 280 | } |
216 | |||
217 | None | ||
218 | } | 281 | } |
219 | 282 | ||
220 | fn validate_match( | 283 | fn validate_match( |