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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
Jun 24, 2021
Merged

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

merged 1 commit into from
Jun 24, 2021

Conversation

DevinR528
Copy link
Contributor

fixes #7276

changelog: Add [`import_rename`] a lint that enforces import renaming defined in the config file.

@rust-highfive
Copy link

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label May 31, 2021
@DevinR528 DevinR528 mentioned this pull request Jun 1, 2021
Closed
@bors
Copy link
Contributor

bors commented Jun 10, 2021

☔ The latest upstream changes (presumably #7315) made this pull request unmergeable. Please resolve the merge conflicts.

@DevinR528 DevinR528 mentioned this pull request Jun 12, 2021
Merged
camsteffen
camsteffen suggested changes Jun 13, 2021
View reviewed changes
clippy_lints/src/import_rename.rs Outdated Show resolved Hide resolved
clippy_lints/src/lib.rs Outdated Show resolved Hide resolved
tests/ui-toml/toml_import_rename/conf_import_rename.rs Outdated Show resolved Hide resolved
clippy_lints/src/import_rename.rs Outdated Show resolved Hide resolved
bors added a commit that referenced this pull request Jun 14, 2021
@bors
Auto merge of #7345 - DevinR528:disallowed-fix, r=Manishearth
2d1e9ab 
Remove requirement of fully qualified path for disallowed_method/type

changelog: Remove the need for fully qualified paths in disallowed_method and disallowed_type

After fixing this issue in [import_rename](#7300 (comment)) I figured I'd fix it for these two.

If this does in fact fix the **Known problems:** section I was planning to remove them from both lints after confirmation.
@jplatte jplatte mentioned this pull request Jun 15, 2021
Open
@bors
Copy link
Contributor

bors commented Jun 19, 2021

☔ The latest upstream changes (presumably #7299) made this pull request unmergeable. Please resolve the merge conflicts.

@DevinR528 DevinR528 force-pushed the import-rename branch 2 times, most recently from 2ceaffa to 8b44012 Compare June 20, 2021 10:32
@bors
Copy link
Contributor

bors commented Jun 21, 2021

☔ The latest upstream changes (presumably #7385) made this pull request unmergeable. Please resolve the merge conflicts.

@flip1995
Copy link
Member

@DevinR528 This will need a rebase.

r? @camsteffen You pretty much reviewed this PR yourself. Can you please take over the review of this PR?

@rust-highfive rust-highfive assigned camsteffen and unassigned flip1995 Jun 21, 2021
@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jun 21, 2021
@DevinR528
Copy link
Contributor Author

Rebased and ready to go.

Is there a web page like the one with all the clippy lints with the config options? I feel like I've seen/used it or I'm imagining 🤷

@flip1995
Copy link
Member

The config options are on the webpage with the lint documentation. To try it out, you can run cargo dev serve in your checkout. This will open a browser window with the website including your local changes. You can then search for the lint to check if it has the documentation for the config.

camsteffen
camsteffen suggested changes Jun 21, 2021
View reviewed changes
Copy link
Contributor

@camsteffen camsteffen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure about the lint name. deny(import_rename) looks like a total ban of renames. missing_enforced_import_renames is a bit long...

I think the config option could be named enforced_import_renames to be more clear.

CC @rust-lang/clippy thoughts?

clippy_lints/src/import_rename.rs Outdated Show resolved Hide resolved
clippy_lints/src/import_rename.rs Outdated Show resolved Hide resolved
clippy_lints/src/import_rename.rs Outdated Show resolved Hide resolved
clippy_lints/src/utils/conf.rs Outdated Show resolved Hide resolved
clippy_lints/src/import_rename.rs Outdated Show resolved Hide resolved
clippy_lints/src/import_rename.rs Outdated Show resolved Hide resolved
@Manishearth
Copy link
Member

I like the rename proposed by @camsteffen

camsteffen
camsteffen suggested changes Jun 22, 2021
View reviewed changes
clippy_lints/src/missing_enforced_import_rename.rs Outdated Show resolved Hide resolved
clippy_lints/src/missing_enforced_import_rename.rs Outdated Show resolved Hide resolved
clippy_lints/src/missing_enforced_import_rename.rs Outdated Show resolved Hide resolved
camsteffen
camsteffen reviewed Jun 24, 2021
View reviewed changes
Copy link
Contributor

@camsteffen camsteffen left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit. Please squash commits then r=me. Thanks!

clippy_lints/src/missing_enforced_import_rename.rs Outdated Show resolved Hide resolved
@camsteffen
Copy link
Contributor

@bors delegate+

@bors
Copy link
Contributor

bors commented Jun 24, 2021

✌️ @DevinR528 can now approve this pull request

@camsteffen
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 24, 2021

📌 Commit 9492de5 has been approved by camsteffen

@bors
Copy link
Contributor

bors commented Jun 24, 2021

⌛ Testing commit 9492de5 with merge b286b38...

@bors
Copy link
Contributor

bors commented Jun 24, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: camsteffen
Pushing b286b38 to master...

@bors bors merged commit b286b38 into rust-lang:master Jun 24, 2021
@DevinR528 DevinR528 deleted the import-rename branch June 25, 2021 10:27
@DevinR528
Copy link
Contributor Author

Thanks everyone who helped especially @camsteffen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 flip1995 left review comments

@camsteffen camsteffen camsteffen requested changes

@jplatte jplatte jplatte left review comments

@0969397110 0969397110 0969397110 approved these changes

Assignees

@camsteffen camsteffen

Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New lint: enforce import renaming
8 participants
@DevinR528 @rust-highfive @bors @flip1995 @Manishearth @camsteffen @jplatte @0969397110