Add arg_iter lint to recommend IntoIterator over Iterator for argumen… #1581
Conversation
|
|
…d update arg_iter version to 4.1.0
|
@timsburk Sorry for the delay. I have a little bit of a reviewing backlog. But I will try to get to this soon. |
|
@timsburk Sorry I haven't had a chance to review this. I am getting ready to travel, and I will get back early next week. I will make it a priority to review this then. |
There was a problem hiding this comment.
@timsburk Thanks very much for your patience.
Let's please move this lint from general to supplementary.
As noted below, I think the current tests are not being run. I will need to review the lint's logic more carefully, but I would like to see the tests run first.
| @@ -31,6 +32,10 @@ wrong_serialize_struct_arg = { path = "wrong_serialize_struct_arg", features = [ | |||
|
|
|||
| dylint_linting = { path = "../../utils/linting" } | |||
|
|
|||
| [features] | |||
| # Feature to avoid duplicate lint registration for abs_home_path | |||
| avoid_duplicate_abs_home_path = [] | |||
| @@ -37,4 +37,60 @@ For each lint subdirectory: | |||
| rlib = ["dylint_linting/constituent"] | |||
| ``` | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
This file will need to be updated, but it should be possible to do that automatically. (I'll follow up with instructions later during the review.)
I don't think these manual changes should be necessary.
| #[cfg(test)] | ||
| mod tests { | ||
| #[test] | ||
| fn it_works() { | ||
| assert!(true); | ||
| } | ||
| } |
There was a problem hiding this comment.
This looks like a mistake(?).
Also, I don't think the current tests are being run.
Please add something like the following to this file:
#[test]
fn ui() {
dylint_testing::ui_test(env!("CARGO_PKG_NAME"), "ui");
}| #[no_mangle] | ||
| pub fn register_lints(_sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) { | ||
| lint_store.register_lints(&[ARG_ITER]); | ||
| lint_store.register_late_pass(|_| Box::new(ArgIter)); | ||
| } |
There was a problem hiding this comment.
| #[no_mangle] | |
| pub fn register_lints(_sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) { | |
| lint_store.register_lints(&[ARG_ITER]); | |
| lint_store.register_late_pass(|_| Box::new(ArgIter)); | |
| } |
If you rewrite the lint to use declare_late_lint!, this register_lints function should be unnecessary.
| #[no_mangle] | ||
| pub fn register_lints(_sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) { | ||
| lint_store.register_lints(&[ARG_ITER]); | ||
| lint_store.register_late_pass(|_| Box::new(ArgIter)); | ||
| } |
There was a problem hiding this comment.
| #[no_mangle] | |
| pub fn register_lints(_sess: &rustc_session::Session, lint_store: &mut rustc_lint::LintStore) { | |
| lint_store.register_lints(&[ARG_ITER]); | |
| lint_store.register_late_pass(|_| Box::new(ArgIter)); | |
| } |
If you rewrite the lint to use declare_late_lint!, this register_lints function should be unnecessary.
| @@ -0,0 +1,31 @@ | |||
| // This should trigger our lint | |||
There was a problem hiding this comment.
I think this file can be removed.
| @@ -0,0 +1,6 @@ | |||
| [package] | |||
There was a problem hiding this comment.
I think this file can be removed.
| @@ -0,0 +1,31 @@ | |||
| // This should trigger our lint | |||
There was a problem hiding this comment.
I think this file can be removed.
| use rustc_span::symbol::Symbol; | ||
| use rustc_span::def_id::LocalDefId; | ||
|
|
||
| declare_lint! { |
There was a problem hiding this comment.
Please use dylint_linting::declare_late_lint!.
You can find examples in the supplementary directory, e.g., here:
Using dylint_linting::declare_late_lint! will make some of the other declarations in this file unnecessary, e.g., the declaration of the register_lints function.
This PR implements the lint mentioned in issue #598 that suggests using
IntoIteratorinstead ofIteratorfor function parameter types when possible.Implementation details
examples/general/arg_iterthat detects when a function parameter type requires theIteratortraitIntoIteratorprovides more flexibilityTesting
Iteratorbounds that could be changedIteratoris needed for other bounds)This allows functions to accept a wider range of types (like slices, arrays, vectors) without requiring callers to explicitly call
.iter()or.into_iter().