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

#[expect(...)] doen't catch lints from different check_* function #97660

Closed
Tracked by #85549
Jarcho opened this issue Jun 2, 2022 · 5 comments
Closed
Tracked by #85549

#[expect(...)] doen't catch lints from different check_* function #97660

Jarcho opened this issue Jun 2, 2022 · 5 comments
Assignees
@xFrednet
Labels
A-clippy Area: Clippy C-bug Category: This is a bug. F-lint_reasons `#![feature(lint_reasons)]`

Comments

@Jarcho
Copy link
Contributor

Jarcho commented Jun 2, 2022 •
edited by rustbot
Loading

Some lints are emitted from a from a different from the area they are linted. e.g. clippy::ptr_arg is linting from check_body, but it can be allowed from the argument (fn foo(#[allow(clippy::ptr_arg)] x: &Vec<u32>) {}).

The following lints in clippy (and probably more) lint from different check_* functions:

  • needless_borrow
  • ref_binding_to_ref
  • duplicate_mod
  • multiple_inherent_impl
  • macro_use_imports
  • manual_non_exhaustive
  • same_name_method
  • async_yields_async
  • await_holding_lock
  • await_holding_refcell
  • await_holding_invalid_type
  • default_numeric_fallback
  • ptr_arg
  • redundant_clone
  • logic_bug
  • nonminimal_bool
  • boxed_local
  • implicit_return
  • needless_return
  • unnecessary_unwrap
  • panicking_unwrap

Some of these don't actually work with allow at a narrower scope either, but some of them check to see if the lint has been allowed on the target item before emitting the lint.

cc #54503
@Jarcho Jarcho added the C-bug Category: This is a bug. label Jun 2, 2022
@Jarcho Jarcho changed the title #[expect(...)] doen't catch lints from wrong check_* function #[expect(...)] doen't catch lints from different check_* function Jun 2, 2022
@xFrednet xFrednet added the F-lint_reasons `#![feature(lint_reasons)]` label Jun 3, 2022
@xFrednet xFrednet mentioned this issue Jun 3, 2022
Closed
9 tasks
@xFrednet
Copy link
Member

xFrednet commented Jun 6, 2022 •
edited
Loading

@rustbot claim. I'll go through these lints one by one. The first two were caused by wrong emission functions on Clippy's side.

Todo List:

  • needless_borrow
  • ref_binding_to_ref
  • same_name_method
  • async_yields_async
  • default_numeric_fallback
  • ptr_arg
  • logic_bug
  • nonminimal_bool
  • boxed_local
  • implicit_return
  • needless_return
  • unnecessary_unwrap
  • panicking_unwrap
  • macro_use_imports
  • manual_non_exhaustive (is_lint_allowed)

See: https://github.com/xFrednet/rust-clippy/tree/rust-97660-catch-emissions-with-expect for the current progress

  • Requites new interface in rustc, to fulfill the expectation manually:

    • duplicate_mod (Early lint pass...)
    • multiple_inherent_impl (is_lint_allowed)

  • Won't fix now:

    The expect attribute works on the body, but not on the guard expression. This is a result of the lint implementation and should therefore be fixed there. Other lint attributes are also ignored on the expr level

    • await_holding_lock
    • await_holding_refcell
    • await_holding_invalid_type

    The fix of these lints, will be tracked in Fix lint attributes/emission location for await_holding_* lints rust-clippy#9047

  • Can't reproduce (But Test added):

    • redundant_clone

This was referenced Jun 9, 2022
Closed
Merged
bors added a commit to rust-lang/rust-clippy that referenced this issue Jun 9, 2022
@bors
Auto merge of #8976 - xFrednet:rust-97660-catch-emissions-with-expect…
b3c94c0 
…, r=Jarcho

Fix some `#[expect]` lint interaction

Fixing the first few lints that aren't caught by `#[expect]`. The root cause of these examples was, that the lint was emitted at the wrong location.

---

changelog: none

r? `@Jarcho`

cc: rust-lang/rust#97660
@xFrednet
Copy link
Member

The next step for this issue is to adapt the linting interface of rustc to allow the manual fulfillment of expectations. Then I have to take a look at the last few lints, but they should all be covered by these changes.

@Jarcho, have you also looked at rustc lints or just at Clippy's lints? 🙃

@Jarcho
Copy link
Contributor Author

Jarcho commented Jun 22, 2022

It doesn't look like any of the rustc lints need to be changed. At least the behaviour of expect shouldn't be different than allow is.

This was referenced Jun 25, 2022
Merged
Open
@xFrednet xFrednet added the A-clippy Area: Clippy label Jun 25, 2022
@xFrednet xFrednet mentioned this issue Jun 25, 2022
Merged
bors added a commit to rust-lang/rust-clippy that referenced this issue Jun 28, 2022
@bors
Auto merge of #9046 - xFrednet:rust-97660-expection-something-somethi…
4995b4e 
…ng, r=Jarcho

Fix `#[expect]` for most clippy lints

This PR fixes most `#[expect]` - lint interactions listed in rust-lang/rust#97660. [My comment in the issue](rust-lang/rust#97660 (comment)) shows the current progress (Once this is merged). I plan to work on `duplicate_mod` and `multiple_inherent_impl` and leave the rest for later. I feel like stabilizing the feature is more important than fixing the last few nits, which currently also don't work with `#[allow]`.

---

changelog: none

r? `@Jarcho`

cc: rust-lang/rust#97660
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 7, 2022
@Dylan-DPC
Rollup merge of rust-lang#98507 - xFrednet:rfc-2383-manual-expectatio…
7260f7e 
…n-magic, r=wesleywiser

Finishing touches for `#[expect]` (RFC 2383)

This PR adds documentation and some functionality to rustc's lint passes, to manually fulfill expectations. This is needed for some lints in Clippy. Hopefully, it should be one of the last things before we can move forward with stabilizing this feature.

As part of this PR, I've also updated `clippy::duplicate_mod` to showcase how this new functionality can be used and to ensure that it works correctly.

---

changelog: [`duplicate_mod`]: Fixed lint attribute interaction

r? `@wesleywiser`

cc: rust-lang#97660, rust-lang#85549

And I guess that's it. Here have a magical unicorn 🦄
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 7, 2022
@Dylan-DPC
Rollup merge of rust-lang#98507 - xFrednet:rfc-2383-manual-expectatio…
93ce6fd 
…n-magic, r=wesleywiser

Finishing touches for `#[expect]` (RFC 2383)

This PR adds documentation and some functionality to rustc's lint passes, to manually fulfill expectations. This is needed for some lints in Clippy. Hopefully, it should be one of the last things before we can move forward with stabilizing this feature.

As part of this PR, I've also updated `clippy::duplicate_mod` to showcase how this new functionality can be used and to ensure that it works correctly.

---

changelog: [`duplicate_mod`]: Fixed lint attribute interaction

r? ``@wesleywiser``

cc: rust-lang#97660, rust-lang#85549

And I guess that's it. Here have a magical unicorn 🦄
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 7, 2022
@Dylan-DPC
Rollup merge of rust-lang#98507 - xFrednet:rfc-2383-manual-expectatio…
469ca7e 
…n-magic, r=wesleywiser

Finishing touches for `#[expect]` (RFC 2383)

This PR adds documentation and some functionality to rustc's lint passes, to manually fulfill expectations. This is needed for some lints in Clippy. Hopefully, it should be one of the last things before we can move forward with stabilizing this feature.

As part of this PR, I've also updated `clippy::duplicate_mod` to showcase how this new functionality can be used and to ensure that it works correctly.

---

changelog: [`duplicate_mod`]: Fixed lint attribute interaction

r? ```@wesleywiser```

cc: rust-lang#97660, rust-lang#85549

And I guess that's it. Here have a magical unicorn 🦄
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Jul 7, 2022
@Dylan-DPC
Rollup merge of rust-lang#98507 - xFrednet:rfc-2383-manual-expectatio…
c815fef 
…n-magic, r=wesleywiser

Finishing touches for `#[expect]` (RFC 2383)

This PR adds documentation and some functionality to rustc's lint passes, to manually fulfill expectations. This is needed for some lints in Clippy. Hopefully, it should be one of the last things before we can move forward with stabilizing this feature.

As part of this PR, I've also updated `clippy::duplicate_mod` to showcase how this new functionality can be used and to ensure that it works correctly.

---

changelog: [`duplicate_mod`]: Fixed lint attribute interaction

r? `@wesleywiser`

cc: rust-lang#97660, rust-lang#85549

And I guess that's it. Here have a magical unicorn 🦄
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jul 14, 2022
@Dylan-DPC
Rollup merge of #98507 - xFrednet:rfc-2383-manual-expectation-magic, …
4f9933a 
…r=wesleywiser

Finishing touches for `#[expect]` (RFC 2383)

This PR adds documentation and some functionality to rustc's lint passes, to manually fulfill expectations. This is needed for some lints in Clippy. Hopefully, it should be one of the last things before we can move forward with stabilizing this feature.

As part of this PR, I've also updated `clippy::duplicate_mod` to showcase how this new functionality can be used and to ensure that it works correctly.

---

changelog: [`duplicate_mod`]: Fixed lint attribute interaction

r? `@wesleywiser`

cc: rust-lang/rust#97660, rust-lang/rust#85549

And I guess that's it. Here have a magical unicorn 🦄
@xFrednet
Copy link
Member

I believe this issue can be closed now. All of these have been the result of suboptimal handling in Clippy. Most of these should be fixed or tracked in a new issue in Clippy.

@Jarcho Could you close it, if you agree? 🙃

@xFrednet xFrednet mentioned this issue Jan 16, 2023
Closed
@Dylan-DPC
Copy link
Member

Closing this as rest of lints are tracked here

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

@xFrednet xFrednet

Labels
A-clippy Area: Clippy C-bug Category: This is a bug. F-lint_reasons `#![feature(lint_reasons)]`
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Jarcho @xFrednet @Dylan-DPC