Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add import_rename lint, this adds a field on the Conf struct #7300

Merged
merged 1 commit into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2657,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
Expand Down
5 changes: 5 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,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;
Expand Down Expand Up @@ -813,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,
Expand Down Expand Up @@ -1017,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),
Expand Down Expand Up @@ -2077,6 +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::<FxHashSet<_>>();
store.register_late_pass(move || box disallowed_type::DisallowedType::new(&disallowed_types));
let import_renames = conf.enforced_import_renames.clone();
store.register_late_pass(move || box missing_enforced_import_rename::ImportRename::new(import_renames.clone()));

}

Expand Down
102 changes: 102 additions & 0 deletions clippy_lints/src/missing_enforced_import_rename.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
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, 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 for imports that do not rename the item as specified
/// in the `enforce-import-renames` config option.
///
/// **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
///
/// **Example:**
///
/// An example clippy.toml configuration:
/// ```toml
/// # clippy.toml
/// enforced-import-renames = [ { path = "serde_json::Value", rename = "JsonValue" }]
/// ```
///
/// ```rust,ignore
/// use serde_json::Value;
/// ```
/// Use instead:
/// ```rust,ignore
/// use serde_json::Value as JsonValue;
/// ```
pub MISSING_ENFORCED_IMPORT_RENAMES,
restriction,
"enforce import renames"
}

pub struct ImportRename {
conf_renames: Vec<Rename>,
renames: FxHashMap<DefId, Symbol>,
}

impl ImportRename {
pub fn new(conf_renames: Vec<Rename>) -> Self {
Self {
conf_renames,
renames: FxHashMap::default(),
}
}
}

impl_lint_pass!(ImportRename => [MISSING_ENFORCED_IMPORT_RENAMES]);

impl LateLintPass<'_> for ImportRename {
fn check_crate(&mut self, cx: &LateContext<'_>, _: &Crate<'_>) {
for Rename { path, rename } in &self.conf_renames {
if let Res::Def(_, id) = clippy_utils::path_to_res(cx, &path.split("::").collect::<Vec<_>>()) {
self.renames.insert(id, Symbol::intern(rename));
}
}
}

fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if_chain! {
if let ItemKind::Use(path, UseKind::Single) = &item.kind;
if let Res::Def(_, id) = path.res;
if let Some(name) = self.renames.get(&id);
// Remove semicolon since it is not present for nested imports
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 {
span_lint_and_sugg(
cx,
MISSING_ENFORCED_IMPORT_RENAMES,
span_without_semi,
"this import should be renamed",
"try",
format!(
"{} as {}",
import,
name,
),
Applicability::MachineApplicable,
);
}
}
}
}
9 changes: 9 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ use std::error::Error;
use std::path::{Path, PathBuf};
use std::{env, fmt, fs, io};

/// Holds information used by `MISSING_ENFORCED_IMPORT_RENAMES` lint.
#[derive(Clone, Debug, Deserialize)]
pub struct Rename {
pub path: String,
pub rename: String,
}

/// Conf with parse errors
#[derive(Default)]
pub struct TryConf {
Expand Down Expand Up @@ -203,6 +210,8 @@ define_Conf! {
(cargo_ignore_publish: bool = false),
/// Lint: NONSTANDARD_MACRO_BRACES. Enforce the named macros always use the braces specified. <br> 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<crate::nonstandard_macro_braces::MacroMatcher> = 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<crate::utils::conf::Rename> = Vec::new()),
}

/// Search for the configuration file.
Expand Down
10 changes: 10 additions & 0 deletions tests/ui-toml/missing_enforced_import_rename/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
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::sync::Mutex", rename = "StdMutie" }
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#![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() {
use std::collections::BTreeMap as OopsWrongRename;
}
Original file line number Diff line number Diff line change
@@ -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

2 changes: 1 addition & 1 deletion tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
@@ -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`, `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