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

Lint against impl in block expressions when could be outside of block expression #2124

Open
Havvy opened this issue Oct 9, 2017 · 1 comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-style Lint: Belongs in the style lint group L-unnecessary Lint: Warn about unnecessary code T-middle Type: Probably requires verifiying types

Comments

@Havvy
Copy link

Havvy commented Oct 9, 2017

Implementations exist globally no matter where they are defined. It's possible to define items in block expressions, and is done primarily for scoping where the item is accessible.

Since putting the implementation into a block expression does not actually limit scope, it should not be done unless absolutely required. Implementations in block expressions are only absolutely required when one of the item being implemented, the trait being implemented, or a generic bound (including in where clauses) refers to an item defined in the block expression.

One can also argue that if one has to define implementations in the block expression that you're being needlessly complex since you can always move both the implementation and the item it requires out of the block expression and into the containing module (or a submodule), and thus just disallow all implementations in block expressions.

The lint forbidding it because of complexity is easier to implement than the lint forbidding it only in confusing cases.

@oli-obk oli-obk added L-style Lint: Belongs in the style lint group L-unnecessary Lint: Warn about unnecessary code good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints T-middle Type: Probably requires verifiying types labels Oct 10, 2017
@Manishearth Manishearth added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. and removed good-first-issue These issues are a good way to get started with Clippy labels Aug 1, 2018
@Manishearth
Copy link
Member

We should do this, but only for impls not accompanied by their struct/enum.

This is somewhat common for function-local structs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-style Lint: Belongs in the style lint group L-unnecessary Lint: Warn about unnecessary code T-middle Type: Probably requires verifiying types
Projects
None yet
Development

No branches or pull requests

3 participants