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

Use span of impl/trait in len_without_is_empty error message, rather … #1559

Merged
merged 3 commits into from
Feb 21, 2017

Conversation

theotherphil
Copy link
Contributor

…than the span of the len method. (Fixes https://github.com/Manishearth/rust-clippy/issues/1532)

11 | | 1
12 | | }
| |_____^ ...ending here
13 | | }
| |_^ ...ending here
Copy link
Member

Choose a reason for hiding this comment

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

This makes me wonder what happens when there is more than one impl for a type.

Can you also had a test that checks that this actually fixes #1532?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. The current lint has a false positive if len and is_empty are defined in separate impl blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've pushed a change which:

  • adds an extra, irrelevant impl block to PubOne to check that we this doesn't generate any errors
  • adds a PubAllowed struct equivalent to PubOne but with an allow attribution on the relevant impl.

Are you happy to merge this as is, or do you want to wait for a fix to the impls-in-different-blocks issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you happy to merge this as is, or do you want to wait for a fix to the impls-in-different-blocks issue?

That's a different issue. I reported it as #1562

…ences len_without_is_empty. Add extra impl block to PubOne to check that this doesn't get flagged@
@oli-obk oli-obk merged commit 3900223 into rust-lang:master Feb 21, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Feb 21, 2017

thanks!

bors added a commit that referenced this pull request Mar 7, 2021
`len_without_is_empty` will now consider multiple impl blocks

fixes #1562

This also reverts #1559 as the `#[allow]` now works on the `len` method. A note has also been added to point out where the `empty` method is, if it exists.

changelog: `len_without_is_empty` will now consider multiple impl blocks
changelog: `len_without_is_empty` will now consider `#[allow]` on both the `len` method, and the type definition
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants