From e123abd1a6bc978d1fb2a8a6e3c49c5fb8c718eb Mon Sep 17 00:00:00 2001 From: Devin Ragotzy Date: Mon, 21 Jun 2021 16:39:36 -0400 Subject: [PATCH] Rename lint and fix review issues --- CHANGELOG.md | 2 +- clippy_lints/src/lib.rs | 10 ++--- ...e.rs => missing_enforced_import_rename.rs} | 44 ++++++++++--------- clippy_lints/src/utils/conf.rs | 8 ++-- .../clippy.toml | 5 ++- .../conf_missing_enforced_import_rename.rs} | 4 +- ...conf_missing_enforced_import_rename.stderr | 40 +++++++++++++++++ .../conf_import_rename.stderr | 34 -------------- .../toml_unknown_key/conf_unknown_key.stderr | 2 +- 9 files changed, 81 insertions(+), 68 deletions(-) rename clippy_lints/src/{import_rename.rs => missing_enforced_import_rename.rs} (61%) rename tests/ui-toml/{toml_import_rename => missing_enforced_import_rename}/clippy.toml (70%) rename tests/ui-toml/{toml_import_rename/conf_import_rename.rs => missing_enforced_import_rename/conf_missing_enforced_import_rename.rs} (75%) create mode 100644 tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.stderr delete mode 100644 tests/ui-toml/toml_import_rename/conf_import_rename.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 8472d49011e0..5f62a172035d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2562,7 +2562,6 @@ Released 2018-09-13 [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher [`implicit_return`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_return [`implicit_saturating_sub`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_saturating_sub -[`import_rename`]: https://rust-lang.github.io/rust-clippy/master/index.html#import_rename [`imprecise_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#imprecise_flops [`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping [`inconsistent_struct_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_struct_constructor @@ -2658,6 +2657,7 @@ Released 2018-09-13 [`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op [`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items +[`missing_enforced_import_renames`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_enforced_import_renames [`missing_errors_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_errors_doc [`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items [`missing_panics_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 4487be56b3ef..9cffeeb02249 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -226,7 +226,6 @@ mod if_then_some_else_none; mod implicit_hasher; mod implicit_return; mod implicit_saturating_sub; -mod import_rename; mod inconsistent_struct_constructor; mod indexing_slicing; mod infinite_iter; @@ -268,6 +267,7 @@ mod misc; mod misc_early; mod missing_const_for_fn; mod missing_doc; +mod missing_enforced_import_rename; mod missing_inline; mod modulo_arithmetic; mod multiple_crate_versions; @@ -649,7 +649,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: implicit_hasher::IMPLICIT_HASHER, implicit_return::IMPLICIT_RETURN, implicit_saturating_sub::IMPLICIT_SATURATING_SUB, - import_rename::IMPORT_RENAME, inconsistent_struct_constructor::INCONSISTENT_STRUCT_CONSTRUCTOR, indexing_slicing::INDEXING_SLICING, indexing_slicing::OUT_OF_BOUNDS_INDEXING, @@ -815,6 +814,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: misc_early::ZERO_PREFIXED_LITERAL, missing_const_for_fn::MISSING_CONST_FOR_FN, missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS, + missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES, missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS, modulo_arithmetic::MODULO_ARITHMETIC, multiple_crate_versions::MULTIPLE_CRATE_VERSIONS, @@ -1002,7 +1002,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(float_literal::LOSSY_FLOAT_LITERAL), LintId::of(if_then_some_else_none::IF_THEN_SOME_ELSE_NONE), LintId::of(implicit_return::IMPLICIT_RETURN), - LintId::of(import_rename::IMPORT_RENAME), LintId::of(indexing_slicing::INDEXING_SLICING), LintId::of(inherent_impl::MULTIPLE_INHERENT_IMPL), LintId::of(integer_division::INTEGER_DIVISION), @@ -1020,6 +1019,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(misc::FLOAT_CMP_CONST), LintId::of(misc_early::UNNEEDED_FIELD_PATTERN), LintId::of(missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS), + LintId::of(missing_enforced_import_rename::MISSING_ENFORCED_IMPORT_RENAMES), LintId::of(missing_inline::MISSING_INLINE_IN_PUBLIC_ITEMS), LintId::of(modulo_arithmetic::MODULO_ARITHMETIC), LintId::of(panic_in_result_fn::PANIC_IN_RESULT_FN), @@ -2080,8 +2080,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box unused_async::UnusedAsync); let disallowed_types = conf.disallowed_types.iter().cloned().collect::>(); store.register_late_pass(move || box disallowed_type::DisallowedType::new(&disallowed_types)); - let import_renames = conf.import_renames.clone(); - store.register_late_pass(move || box import_rename::ImportRename::new(import_renames.clone())); + let import_renames = conf.enforced_import_renames.clone(); + store.register_late_pass(move || box missing_enforced_import_rename::ImportRename::new(import_renames.clone())); } diff --git a/clippy_lints/src/import_rename.rs b/clippy_lints/src/missing_enforced_import_rename.rs similarity index 61% rename from clippy_lints/src/import_rename.rs rename to clippy_lints/src/missing_enforced_import_rename.rs index 414c2f65e39a..3fea4fcb5cb0 100644 --- a/clippy_lints/src/import_rename.rs +++ b/clippy_lints/src/missing_enforced_import_rename.rs @@ -3,17 +3,19 @@ use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_opt}; use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; use rustc_hir::{def::Res, def_id::DefId, Crate, Item, ItemKind, UseKind}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::Symbol; use crate::utils::conf::Rename; declare_clippy_lint! { - /// **What it does:** Checks that any import matching `import_renames` is renamed. + /// **What it does:** Checks for imports that do not rename the item as specified + /// in the `enforce-import-renames` config option. /// - /// **Why is this bad?** This is bad for some items because their defined names are - /// too vauge outside of their defining scope. + /// **Why is this bad?** Consistency is important, if a project has defined import + /// renames they should be followed. More practically, some item names are too + /// vague outside of their defining scope this can enforce a more meaningful naming. /// /// **Known problems:** None /// @@ -22,7 +24,7 @@ declare_clippy_lint! { /// An example clippy.toml configuration: /// ```toml /// # clippy.toml - /// import-renames = [ {path = "serde_json::Value", rename = "JsonValue" }] + /// enforced-import-renames = [ {path = "serde_json::Value", rename = "JsonValue" }] /// ``` /// /// ```rust,ignore @@ -32,7 +34,7 @@ declare_clippy_lint! { /// ```rust,ignore /// use serde_json::Value as JsonValue; /// ``` - pub IMPORT_RENAME, + pub MISSING_ENFORCED_IMPORT_RENAMES, restriction, "enforce import renames" } @@ -51,7 +53,7 @@ impl ImportRename { } } -impl_lint_pass!(ImportRename => [IMPORT_RENAME]); +impl_lint_pass!(ImportRename => [MISSING_ENFORCED_IMPORT_RENAMES]); impl LateLintPass<'_> for ImportRename { fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) { @@ -66,29 +68,31 @@ impl LateLintPass<'_> for ImportRename { if_chain! { if let ItemKind::Use(path, UseKind::Single) = &item.kind; if let Res::Def(_, id) = path.res; - if let Some(snip) = snippet_opt(cx, item.span); if let Some(name) = self.renames.get(&id); // Remove semicolon since it is not present for nested imports - if !snip.replace(";", "").ends_with(&*name.as_str()); + let span_without_semi = cx.sess().source_map().span_until_char(item.span, ';'); + if let Some(snip) = snippet_opt(cx, span_without_semi); + if let Some(import) = match snip.split_once(" as ") { + None => Some(snip.as_str()), + Some((import, rename)) => { + if rename.trim() == &*name.as_str() { + None + } else { + Some(import.trim()) + } + }, + }; then { - let import = if snip.ends_with(';') { - snip.split_whitespace().take(2).collect::>().join(" ").replace(";", "") - } else if snip.contains("as") { - snip.split_whitespace().take(1).collect::>().join(" ") - } else { - snip.clone() - }; span_lint_and_sugg( cx, - IMPORT_RENAME, - item.span, + MISSING_ENFORCED_IMPORT_RENAMES, + span_without_semi, "this import should be renamed", "try", format!( - "{} as {}{}", + "{} as {}", import, name, - if snip.ends_with(';') { ";" } else { "" }, ), Applicability::MachineApplicable, ); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index fbe1ce2d7016..6fc4998318cc 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -8,8 +8,8 @@ use std::error::Error; use std::path::{Path, PathBuf}; use std::{env, fmt, fs, io}; -/// Holds information used by `IMPORT_RENAME` lint. -#[derive(Clone, Debug, Deserialize, PartialEq, Eq, Hash)] +/// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint. +#[derive(Clone, Debug, Deserialize)] pub struct Rename { pub path: String, pub rename: String, @@ -210,8 +210,8 @@ define_Conf! { (cargo_ignore_publish: bool = false), /// Lint: NONSTANDARD_MACRO_BRACES. Enforce the named macros always use the braces specified.
A `MacroMatcher` can be added like so `{ name = "macro_name", brace = "(" }`. If the macro is could be used with a full path two `MacroMatcher`s have to be added one with the full path `crate_name::macro_name` and one with just the macro name. (standard_macro_braces: Vec = Vec::new()), - /// Lint: IMPORT_RENAMES. The list of imports to always rename, a fully qualified path followed by the rename. - (import_renames: Vec = Vec::new()), + /// Lint: MISSING_ENFORCED_IMPORT_RENAMES. The list of imports to always rename, a fully qualified path followed by the rename. + (enforced_import_renames: Vec = Vec::new()), } /// Search for the configuration file. diff --git a/tests/ui-toml/toml_import_rename/clippy.toml b/tests/ui-toml/missing_enforced_import_rename/clippy.toml similarity index 70% rename from tests/ui-toml/toml_import_rename/clippy.toml rename to tests/ui-toml/missing_enforced_import_rename/clippy.toml index 4c02788ebc2b..05ba822874d5 100644 --- a/tests/ui-toml/toml_import_rename/clippy.toml +++ b/tests/ui-toml/missing_enforced_import_rename/clippy.toml @@ -1,9 +1,10 @@ -import-renames = [ +enforced-import-renames = [ { path = "std::option::Option", rename = "Maybe" }, { path = "std::process::Child", rename = "Kid" }, { path = "std::process::exit", rename = "goodbye" }, { path = "std::collections::BTreeMap", rename = "Map" }, { path = "std::clone", rename = "foo" }, { path = "std::thread::sleep", rename = "thread_sleep" }, - { path = "std::any::type_name", rename = "ident" } + { path = "std::any::type_name", rename = "ident" }, + { path = "std::sync::Mutex", rename = "StdMutie" } ] diff --git a/tests/ui-toml/toml_import_rename/conf_import_rename.rs b/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.rs similarity index 75% rename from tests/ui-toml/toml_import_rename/conf_import_rename.rs rename to tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.rs index b05a666d09cc..f60058c86288 100644 --- a/tests/ui-toml/toml_import_rename/conf_import_rename.rs +++ b/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.rs @@ -1,12 +1,14 @@ -#![warn(clippy::import_rename)] +#![warn(clippy::missing_enforced_import_renames)] use std::alloc as colla; use std::option::Option as Maybe; use std::process::{exit as wrong_exit, Child as Kid}; use std::thread::sleep; +#[rustfmt::skip] use std::{ any::{type_name, Any}, clone, + sync :: Mutex, }; fn main() { diff --git a/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.stderr b/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.stderr new file mode 100644 index 000000000000..45de8fdffef7 --- /dev/null +++ b/tests/ui-toml/missing_enforced_import_rename/conf_missing_enforced_import_rename.stderr @@ -0,0 +1,40 @@ +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:5:20 + | +LL | use std::process::{exit as wrong_exit, Child as Kid}; + | ^^^^^^^^^^^^^^^^^^ help: try: `exit as goodbye` + | + = note: `-D clippy::missing-enforced-import-renames` implied by `-D warnings` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:6:1 + | +LL | use std::thread::sleep; + | ^^^^^^^^^^^^^^^^^^^^^^ help: try: `use std::thread::sleep as thread_sleep` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:9:11 + | +LL | any::{type_name, Any}, + | ^^^^^^^^^ help: try: `type_name as ident` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:10:5 + | +LL | clone, + | ^^^^^ help: try: `clone as foo` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:11:5 + | +LL | sync :: Mutex, + | ^^^^^^^^^^^^^ help: try: `sync :: Mutex as StdMutie` + +error: this import should be renamed + --> $DIR/conf_missing_enforced_import_rename.rs:15:5 + | +LL | use std::collections::BTreeMap as OopsWrongRename; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `use std::collections::BTreeMap as Map` + +error: aborting due to 6 previous errors + diff --git a/tests/ui-toml/toml_import_rename/conf_import_rename.stderr b/tests/ui-toml/toml_import_rename/conf_import_rename.stderr deleted file mode 100644 index e10474255ca0..000000000000 --- a/tests/ui-toml/toml_import_rename/conf_import_rename.stderr +++ /dev/null @@ -1,34 +0,0 @@ -error: this import should be renamed - --> $DIR/conf_import_rename.rs:5:20 - | -LL | use std::process::{exit as wrong_exit, Child as Kid}; - | ^^^^^^^^^^^^^^^^^^ help: try: `exit as goodbye` - | - = note: `-D clippy::import-rename` implied by `-D warnings` - -error: this import should be renamed - --> $DIR/conf_import_rename.rs:6:1 - | -LL | use std::thread::sleep; - | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `use std::thread::sleep as thread_sleep;` - -error: this import should be renamed - --> $DIR/conf_import_rename.rs:8:11 - | -LL | any::{type_name, Any}, - | ^^^^^^^^^ help: try: `type_name as ident` - -error: this import should be renamed - --> $DIR/conf_import_rename.rs:9:5 - | -LL | clone, - | ^^^^^ help: try: `clone as foo` - -error: this import should be renamed - --> $DIR/conf_import_rename.rs:13:5 - | -LL | use std::collections::BTreeMap as OopsWrongRename; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `use std::collections::BTreeMap as Map;` - -error: aborting due to 5 previous errors - diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index ecf95b2870c6..aa7bfa2cc8cd 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -1,4 +1,4 @@ -error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `import-renames`, `third-party` at line 5 column 1 +error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `third-party` at line 5 column 1 error: aborting due to previous error