Skip to content

Tracking Issue: Prevent duplicate diagnostic emission of lints #12379

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
14 of 20 tasks
flip1995 opened this issue Feb 28, 2024 · 3 comments
Open
14 of 20 tasks

Tracking Issue: Prevent duplicate diagnostic emission of lints #12379

flip1995 opened this issue Feb 28, 2024 · 3 comments
Labels
C-tracking-issue Category: Tracking Issue E-medium Call for participation: Medium difficulty level problem and requires some initial experience.

Comments

@flip1995
Copy link
Member

flip1995 commented Feb 28, 2024

Description

Follow up to #12374

That PR added the -Zdeduplicate-diagnostics=yes flag to the following test files, to suppress the warning that will now be produced should new lints/tests emit duplicated diagnostics. Old lints should still be cleaned up though. Those are the test files that were affected:

  • tests/ui/assign_ops2.rs
  • tests/ui/else_if_without_else.rs
  • tests/ui/identity_op.rs
  • tests/ui/indexing_slicing_index.rs
  • tests/ui/manual_retain.rs
  • tests/ui/many_single_char_names.rs
  • tests/ui/min_rust_version_invalid_attr.rs
  • tests/ui/mut_mut.rs
  • tests/ui/no_effect_replace.rs
  • tests/ui/nonminimal_bool_methods.rs
  • tests/ui/nonminimal_bool.rs
  • tests/ui/option_option.rs
  • tests/ui/ptr_as_ptr.rs
  • tests/ui/renamed_builtin_attr.rs
  • tests/ui/single_match_else.rs
  • tests/ui/single_match.rs
  • tests/ui/std_instead_of_core.rs
  • tests/ui/suspicious_operation_groupings.rs
  • tests/ui/type_complexity.rs
  • tests/ui/unknown_attribute.rs

To fix some of those:

  1. Pick one test and leave a comment about it
  2. Remove the -Zdeduplicate-diagnostics=yes flag from it
  3. Figure out the cause of duplicated diagnostic
  4. Fix it and write a comment here so that it gets marked as resolved.

Version

No response

Additional Labels

No response

@flip1995 flip1995 added good first issue These issues are a good way to get started with Clippy E-medium Call for participation: Medium difficulty level problem and requires some initial experience. C-tracking-issue Category: Tracking Issue labels Feb 28, 2024
bors added a commit that referenced this issue Mar 3, 2024
… r=Alexendoo

Dedup std_instead_of_core by using first segment span for uniqueness

Relates to #12379.

Instead of checking that the paths have an identical span, it checks that the relevant `std` part of the path segment's span is identical. Added a multiline test, because my first implementation was worse and failed that, then I realized that you could grab the span off the first_segment `Ident`.

I did find another bug that isn't addressed by this, and that exists on master as well.

The path:
```Rust
use std::{io::Write, fmt::Display};
```

Will get fixed into:
```Rust
use core::{io::Write, fmt::Display};
```

Which doesn't compile since `io::Write` isn't in `core`, if any of those paths are present in `core` it'll do the replace and cause a miscompilation. Do you think I should file a separate bug for that? Since `rustfmt` default splits those up it isn't that big of a deal.

Rustfmt:
```Rust
// Pre
use std::{io::Write, fmt::Display};
// Post
use std::fmt::Display;
use std::io::Write;
```
---

changelog: [`std_instead_of_core`]: Fix duplicated output on multiple imports
bors added a commit that referenced this issue Mar 4, 2024
…exendoo

[`identity_op`]: Fix duplicate diagnostics

Relates to #12379

In the `identity_op` lint, the following diagnostic was emitted two times

```
  --> tests/ui/identity_op.rs:156:5
   |
LL |     1 * 1;
   |     ^^^^^ help: consider reducing it to: `1`
   |
```

because both of the left operand and the right operand are the identity element of the multiplication.

This PR fixes the issue so that if a diagnostic is created for an operand, the check of the other operand will be skipped. It's fine because the result is always the same in the affected operators.

---

changelog: [`identity_op`]: Fix duplicate diagnostics
bors added a commit that referenced this issue Mar 5, 2024
[`misrefactored_assign_op`]: Fix duplicate diagnostics

Relate to #12379

The following diagnostics appear twice
```
  --> tests/ui/assign_ops2.rs:26:5
   |
LL |     a *= a * a;
   |     ^^^^^^^^^^
   |
help: did you mean `a = a * a` or `a = a * a * a`? Consider replacing it with
```

because `a` (lhs) appears in both left operand and right operand in the right hand side.
This PR fixes the issue so that if a diagnostic is created for an operand, the check of the other operand will be skipped. It's fine because the result is always the same in the affected operators.

changelog: [`misrefactored_assign_op`]: Fix duplicate diagnostics
bors added a commit that referenced this issue Mar 6, 2024
Remove double expr lint

Related to #12379.

Previously the code manually checked nested binop exprs in unary exprs, but those were caught anyway by `check_expr`. Removed that code path, the path is used in the tests.

---

changelog: [`nonminimal_bool`] Remove duplicate output on nested Binops in Unary exprs.
bors added a commit that referenced this issue Mar 9, 2024
…xendoo

[`no_effect_replace`]: Fix duplicate diagnostics

Relates to #12379

Fixes `no_effect_replace` duplicate diagnostics

---

changelog: [`no_effect_replace`]: Fix duplicate diagnostics
bors added a commit that referenced this issue Mar 9, 2024
[`mut_mut`]: Fix duplicate diags

Relates to #12379

The `mut_mut` lint produced two diagnostics for each `mut mut` pattern in `ty` inside  `block`s because `MutVisitor::visit_ty` was called from `MutMut::check_ty` and  `MutMut::check_block` independently. This PR fixes the issue.

---

changelog: [`mut_mut`]: Fix duplicate diagnostics
bors added a commit that referenced this issue Mar 11, 2024
[`single_match`]: Fix duplicate diagnostics

Relates to #12379

edit two test file
`tests/ui/single_match_else.rs`
`tests/ui/single_match.rs`

those two test file point to the same lint

---

changelog: [`single_match`] Fix duplicate diagnostics
bors added a commit that referenced this issue Mar 11, 2024
[`manual_retain`]: Fix duplicate diagnostics

Relates to: #12379

The first lint guard executed in `LateLintPass::check_expr` was testing if the parent was of type `ExprKind::Assign`.  This meant the lint emitted on both sides of the assignment operator when `check_expr` is called on either `Expr`.  The guard in the fix only lints once when the `Expr` is of kind `Assign`.

changelog:  Fix duplicate lint diagnostic emission from [`manual_retain`]
bors added a commit that referenced this issue Mar 16, 2024
[`else_if_without_else`]: Fix duplicate diagnostics

Relates to: #12379

changelog:  Fix duplicate lint diagnostic emission from [`else_if_without_else`]
bors added a commit that referenced this issue Mar 17, 2024
[`option_option`]: Fix duplicate diagnostics

Relates to #12379

This `option_option` lint change skips checks against `ty`s inside `field_def`s defined by external macro to prevent duplicate diagnostics to the same span `ty` by multiple `Struct` definitions.

---

changelog: [`option_option`]: Fix duplicate diagnostics
bors added a commit that referenced this issue Apr 15, 2024
…xyas

[`ptr_as_ptr`]: Fix duplicate diagnostics

Relates to #12379

`ptr_as_ptr::check` is called twice in `clippy_lints/src/casts/mod.rs`

---

changelog: [`ptr_as_ptr`]: Fix duplicate diagnostics
bors added a commit that referenced this issue Apr 16, 2024
…xyas

[`ptr_as_ptr`]: Fix duplicate diagnostics

Relates to #12379

`ptr_as_ptr::check` is called twice in `clippy_lints/src/casts/mod.rs`

---

changelog: [`ptr_as_ptr`]: Fix duplicate diagnostics
bors added a commit that referenced this issue Apr 29, 2024
…rors, r=xFrednet

[`type_complexity`]: Fix duplicate errors

Relates to #12379

Following message was duplicated:
```
LL |     fn def_method(&self, p: Vec<Vec<Box<(u32, u32, u32, u32)>>>) {}
   |                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: very complex type used. Consider factoring parts into `type` definitions
  --> tests/ui/type_complexity.rs:55:15
```

Methods `check_trait_item` and `check_fn` were both checking method named def_method.
Now `check_trait_item` only checks methods without body.

---
changelog: [`type_complexity`]: Fix duplicate diagnostics
@UlazkaMateusz
Copy link
Contributor

Hey, tests/ui/type_complexity.rs is fixed and merged

@flip1995
Copy link
Member Author

Thanks, only 7 to go 🚀 I love how well this works, we have amazing contributors!

bors added a commit that referenced this issue May 28, 2024
[`many_single_char_names`]: Deduplicate diagnostics

Relates to #12379

Fix `many_single_char_names` lint so that it doesn't emit diagnostics when the current level of the scope doesn't contain any single character name.

```rust
let (a, b, c, d): (i32, i32, i32, i32);
match 1 {
  1 => (),
  e => {},
}
```
produced the exact same MANY_SINGLE_CHAR_NAMES diagnostic at each of the Arm `e => {}` and the Block `{}`.

---

changelog: [`many_single_char_names`]: Fix duplicate diagnostics
bors added a commit that referenced this issue Jun 5, 2024
Dedup nonminimal_bool_methods diags

Relates to #12379

Fix `nonminimal_bool` lint so that it doesn't check the same span multiple times.

`NotSimplificationVisitor` was called for each expression from `NonminimalBoolVisitor` whereas `NotSimplificationVisitor` also recursively checked all expressions.

---

changelog: [`nonminimal_bool`]: Fix duplicate diagnostics
@grtn316
Copy link

grtn316 commented Jun 17, 2024

It appears the duplicate diag messages for:tests/ui/indexing_slicing_index.rs may be coming from the compiler instead of this project.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
C-tracking-issue Category: Tracking Issue E-medium Call for participation: Medium difficulty level problem and requires some initial experience.
Projects
None yet
Development

No branches or pull requests

4 participants