diff --git a/clippy_lints/src/loops.rs b/clippy_lints/src/loops.rs index 058d9adcb51..fcf6a879b91 100644 --- a/clippy_lints/src/loops.rs +++ b/clippy_lints/src/loops.rs @@ -27,7 +27,7 @@ use syntax_pos::BytePos; use crate::utils::paths; use crate::utils::{ - get_enclosing_block, get_parent_expr, higher, is_integer_literal, is_refutable, last_path_segment, + get_enclosing_block, get_parent_expr, has_iter_method, higher, is_integer_literal, is_refutable, last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, snippet, snippet_opt, snippet_with_applicability, span_help_and_lint, span_lint, span_lint_and_sugg, span_lint_and_then, SpanlessEq, }; @@ -1118,6 +1118,12 @@ fn check_for_loop_range<'a, 'tcx>( } } + // don't lint if the container that is indexed does not have .iter() method + let has_iter = has_iter_method(cx, indexed_ty); + if has_iter.is_none() { + return; + } + // don't lint if the container that is indexed into is also used without // indexing if visitor.referenced.contains(&indexed) { diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 739f6e3990a..4b147cf82a6 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -1,11 +1,11 @@ use crate::utils::paths; use crate::utils::sugg; use crate::utils::{ - get_arg_name, get_parent_expr, get_trait_def_id, implements_trait, in_macro, is_copy, is_expn_of, is_self, - is_self_ty, iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, match_trait_method, - match_type, match_var, method_calls, method_chain_args, remove_blocks, return_ty, same_tys, single_segment_path, - snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_sugg, - span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq, + get_arg_name, get_parent_expr, get_trait_def_id, has_iter_method, implements_trait, in_macro, is_copy, is_expn_of, + is_self, is_self_ty, iter_input_pats, last_path_segment, match_def_path, match_path, match_qpath, + match_trait_method, match_type, match_var, method_calls, method_chain_args, remove_blocks, return_ty, same_tys, + single_segment_path, snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, + span_lint_and_sugg, span_lint_and_then, span_note_and_lint, walk_ptrs_ty, walk_ptrs_ty_depth, SpanlessEq, }; use if_chain::if_chain; use matches::matches; @@ -2237,47 +2237,23 @@ fn ty_has_iter_method( cx: &LateContext<'_, '_>, self_ref_ty: ty::Ty<'_>, ) -> Option<(&'static Lint, &'static str, &'static str)> { - // FIXME: instead of this hard-coded list, we should check if `::iter` - // exists and has the desired signature. Unfortunately FnCtxt is not exported - // so we can't use its `lookup_method` method. - static INTO_ITER_COLLECTIONS: [(&Lint, &[&str]); 13] = [ - (INTO_ITER_ON_REF, &paths::VEC), - (INTO_ITER_ON_REF, &paths::OPTION), - (INTO_ITER_ON_REF, &paths::RESULT), - (INTO_ITER_ON_REF, &paths::BTREESET), - (INTO_ITER_ON_REF, &paths::BTREEMAP), - (INTO_ITER_ON_REF, &paths::VEC_DEQUE), - (INTO_ITER_ON_REF, &paths::LINKED_LIST), - (INTO_ITER_ON_REF, &paths::BINARY_HEAP), - (INTO_ITER_ON_REF, &paths::HASHSET), - (INTO_ITER_ON_REF, &paths::HASHMAP), - (INTO_ITER_ON_ARRAY, &["std", "path", "PathBuf"]), - (INTO_ITER_ON_REF, &["std", "path", "Path"]), - (INTO_ITER_ON_REF, &["std", "sync", "mpsc", "Receiver"]), - ]; - - let (self_ty, mutbl) = match self_ref_ty.sty { - ty::Ref(_, self_ty, mutbl) => (self_ty, mutbl), - _ => unreachable!(), - }; - let method_name = match mutbl { - hir::MutImmutable => "iter", - hir::MutMutable => "iter_mut", - }; - - let def_id = match self_ty.sty { - ty::Array(..) => return Some((INTO_ITER_ON_ARRAY, "array", method_name)), - ty::Slice(..) => return Some((INTO_ITER_ON_REF, "slice", method_name)), - ty::Adt(adt, _) => adt.did, - _ => return None, - }; - - for (lint, path) in &INTO_ITER_COLLECTIONS { - if match_def_path(cx.tcx, def_id, path) { - return Some((lint, path.last().unwrap(), method_name)); - } + if let Some(ty_name) = has_iter_method(cx, self_ref_ty) { + let lint = match ty_name { + "array" | "PathBuf" => INTO_ITER_ON_ARRAY, + _ => INTO_ITER_ON_REF, + }; + let mutbl = match self_ref_ty.sty { + ty::Ref(_, _, mutbl) => mutbl, + _ => unreachable!(), + }; + let method_name = match mutbl { + hir::MutImmutable => "iter", + hir::MutMutable => "iter_mut", + }; + Some((lint, ty_name, method_name)) + } else { + None } - None } fn lint_into_iter(cx: &LateContext<'_, '_>, expr: &hir::Expr, self_ref_ty: ty::Ty<'_>, method_span: Span) { diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs index 0816c209a42..d959d7490eb 100644 --- a/clippy_lints/src/utils/mod.rs +++ b/clippy_lints/src/utils/mod.rs @@ -1155,6 +1155,47 @@ pub fn any_parent_is_automatically_derived(tcx: TyCtxt<'_, '_, '_>, node: NodeId false } +/// Returns true if ty has `iter` or `iter_mut` methods +pub fn has_iter_method(cx: &LateContext<'_, '_>, probably_ref_ty: ty::Ty<'_>) -> Option<&'static str> { + // FIXME: instead of this hard-coded list, we should check if `::iter` + // exists and has the desired signature. Unfortunately FnCtxt is not exported + // so we can't use its `lookup_method` method. + static INTO_ITER_COLLECTIONS: [&[&str]; 13] = [ + &paths::VEC, + &paths::OPTION, + &paths::RESULT, + &paths::BTREESET, + &paths::BTREEMAP, + &paths::VEC_DEQUE, + &paths::LINKED_LIST, + &paths::BINARY_HEAP, + &paths::HASHSET, + &paths::HASHMAP, + &paths::PATH_BUF, + &paths::PATH, + &paths::RECEIVER, + ]; + + let ty_to_check = match probably_ref_ty.sty { + ty::Ref(_, ty_to_check, _) => ty_to_check, + _ => probably_ref_ty, + }; + + let def_id = match ty_to_check.sty { + ty::Array(..) => return Some("array"), + ty::Slice(..) => return Some("slice"), + ty::Adt(adt, _) => adt.did, + _ => return None, + }; + + for path in &INTO_ITER_COLLECTIONS { + if match_def_path(cx.tcx, def_id, path) { + return Some(path.last().unwrap()); + } + } + None +} + #[cfg(test)] mod test { use super::{trim_multiline, without_block_comments}; diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 4108732653f..9ee25aaea5e 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -62,6 +62,7 @@ pub const ORD: [&str; 3] = ["core", "cmp", "Ord"]; pub const OS_STRING: [&str; 4] = ["std", "ffi", "os_str", "OsString"]; pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; pub const PARTIAL_ORD: [&str; 3] = ["core", "cmp", "PartialOrd"]; +pub const PATH: [&str; 3] = ["std", "path", "Path"]; pub const PATH_BUF: [&str; 3] = ["std", "path", "PathBuf"]; pub const PATH_TO_PATH_BUF: [&str; 4] = ["std", "path", "Path", "to_path_buf"]; pub const PTR_NULL: [&str; 2] = ["ptr", "null"]; @@ -80,6 +81,7 @@ pub const RANGE_TO_INCLUSIVE: [&str; 3] = ["core", "ops", "RangeToInclusive"]; pub const RANGE_TO_INCLUSIVE_STD: [&str; 3] = ["std", "ops", "RangeToInclusive"]; pub const RANGE_TO_STD: [&str; 3] = ["std", "ops", "RangeTo"]; pub const RC: [&str; 3] = ["alloc", "rc", "Rc"]; +pub const RECEIVER: [&str; 4] = ["std", "sync", "mpsc", "Receiver"]; pub const REGEX: [&str; 3] = ["regex", "re_unicode", "Regex"]; pub const REGEX_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "unicode", "RegexBuilder", "new"]; pub const REGEX_BYTES_BUILDER_NEW: [&str; 5] = ["regex", "re_builder", "bytes", "RegexBuilder", "new"]; diff --git a/tests/ui/needless_range_loop.rs b/tests/ui/needless_range_loop.rs index 5f22e2645d1..70ad6eac65f 100644 --- a/tests/ui/needless_range_loop.rs +++ b/tests/ui/needless_range_loop.rs @@ -85,4 +85,23 @@ fn main() { for i in 0..vec.len() { vec[i] = Some(1).unwrap_or_else(|| panic!("error on {}", i)); } + + // #3788 + let test = Test { + inner: vec![1, 2, 3, 4], + }; + for i in 0..2 { + println!("{}", test[i]); + } +} + +struct Test { + inner: Vec, +} + +impl std::ops::Index for Test { + type Output = usize; + fn index(&self, index: usize) -> &Self::Output { + &self.inner[index] + } }