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

New lint [error_impl_error] #11107

Merged
merged 3 commits into from
Jul 20, 2023
Merged

New lint [error_impl_error] #11107

merged 3 commits into from
Jul 20, 2023

Conversation

Centri3
Copy link
Member

@Centri3 Centri3 commented Jul 4, 2023

Closes #11101

changelog: New lint [error_impl_error]

@rustbot
Copy link
Collaborator

rustbot commented Jul 4, 2023

r? @Jarcho

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 4, 2023
Copy link
Contributor

@Jarcho Jarcho left a comment

Choose a reason for hiding this comment

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

This really should be restricted to types exported from the current module. Module private types have none of the listed downsides.

clippy_lints/src/error_impl_error.rs Outdated Show resolved Hide resolved
@Centri3
Copy link
Member Author

Centri3 commented Jul 5, 2023

Could be a configuration option

@bors
Copy link
Collaborator

bors commented Jul 8, 2023

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

@Centri3 Centri3 force-pushed the error_impl_error branch 3 times, most recently from 715d6bc to d680000 Compare July 14, 2023 00:37
@Jarcho
Copy link
Contributor

Jarcho commented Jul 16, 2023

I was referring to not linting only for module-private types, not crate-private types. I doubt there's any use in having a config for this as they are unusual enough as is, and importing them would be even rarer as only sub-modules could do that.

For types visible outside their own module I would still lint them as normal. If enough people want to ignore them, but still keep the lint otherwise then this can revisited, but I would rather not have a config option unless it's going to be useful.

@Centri3
Copy link
Member Author

Centri3 commented Jul 16, 2023

ah, yeah that would likely be rare enough that it wouldn't be too useful

@bors
Copy link
Collaborator

bors commented Jul 18, 2023

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

@Centri3 Centri3 force-pushed the error_impl_error branch 2 times, most recently from 09a7c64 to 44bb903 Compare July 18, 2023 15:36
CHANGELOG.md Outdated Show resolved Hide resolved
book/src/lint_configuration.md Outdated Show resolved Hide resolved
clippy_lints/src/error_impl_error.rs Outdated Show resolved Hide resolved
@Centri3 Centri3 force-pushed the error_impl_error branch 2 times, most recently from d68bbe6 to ad86796 Compare July 19, 2023 05:26
CHANGELOG.md Outdated Show resolved Hide resolved
Also no longer lints non-exported types now
@Jarcho
Copy link
Contributor

Jarcho commented Jul 20, 2023

Thank you. @bors r+

@bors
Copy link
Collaborator

bors commented Jul 20, 2023

📌 Commit 19b0e84 has been approved by Jarcho

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 20, 2023

⌛ Testing commit 19b0e84 with merge d71fbb9...

@bors
Copy link
Collaborator

bors commented Jul 20, 2023

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

@bors bors merged commit d71fbb9 into rust-lang:master Jul 20, 2023
8 checks passed
@bors bors mentioned this pull request Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New lint suggestion Error impl Error
4 participants