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

mixed_attributes_style should allow mixing outer cfgs and inner doccomments #12435

Closed
apoelstra opened this issue Mar 8, 2024 · 4 comments · Fixed by #12486
Closed

mixed_attributes_style should allow mixing outer cfgs and inner doccomments #12435

apoelstra opened this issue Mar 8, 2024 · 4 comments · Fixed by #12486
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@apoelstra
Copy link

apoelstra commented Mar 8, 2024

Summary

The mixed_attributes_style treats doccomments as attributes, which I suppose they technically are, but this is not how users perceive them. It is common with modules to use inner doccomments (and inner style lints, clippy allows, etc., for that matter, but ok, I can accept that the lint doesn't like that) while having outer cfg gates for features.

Reproducer

I tried this code:

#[cfg(test)]
mod tests {
    //! Doccomment for my tests module
}

This triggers the lint:

warning: item has both inner and outer attributes
 --> src/main.rs:2:1
  |
2 | / #[cfg(test)]
3 | | mod tests {
4 | |     //! Doccomment for my tests module
  | |______________________________________^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mixed_attributes_style
  = note: `#[warn(clippy::mixed_attributes_style)]` on by default

warning: `clippy-demo` (bin "clippy-demo") generated 1 warning
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 6.30s

But I think either fix would be more confusing to read (either add a #![cfg(test)] which I have literally never seen in Rust code, or move the doccomment out, which makes it much harder for readers of the code to see that this is a module-level "header comment").

Version

rustc 1.73.0 (cc66ad468 2023-10-03)
binary: rustc
commit-hash: cc66ad468955717ab92600c770da8c1601a4ff33
commit-date: 2023-10-03
host: x86_64-unknown-linux-gnu
release: 1.73.0
LLVM version: 17.0.2

Additional Labels

No response

@apoelstra apoelstra added the C-bug Category: Clippy is not doing the correct thing label Mar 8, 2024
@J-ZhengLi
Copy link
Member

This might be not as intended @rustbot claim

@jdygert-spok
Copy link

IMO this is a more general problem than doc comments. Many attributes have a "preferred" location. A common case is #[cfg(test)] or #[test] outside, and #![allow(...)] inside.

A more general solution would be to only trigger the lint when the same attribute is both outer and inner on an item.

@workingjubilee
Copy link
Member

workingjubilee commented Mar 21, 2024

Yeah, I kind of hate this lint as it doesn't seem consistent with how I use attributes at all. I most definitely have that "this attribute goes here, this attribute goes here" pattern.

@nyurik
Copy link
Contributor

nyurik commented Mar 21, 2024

I also noticed this lint when running cargo +nightly clippy -p martin-tile-utils on https://github.com/maplibre/martin (observed in b92a407):

warning: item has both inner and outer attributes
   --> martin-tile-utils/src/lib.rs:281:1
    |
281 | / #[cfg(test)]
282 | | mod tests {
283 | |     #![allow(clippy::unreadable_literal)]
    | |_________________________________________^

J-ZhengLi added a commit to J-ZhengLi/rust-clippy that referenced this issue Mar 23, 2024
don't lint [`mixed_attributes_style`] when mixing docs and other attrs

add test files for issue rust-lang#12436

move [`mixed_attributes_style`] to `LateLintPass` to enable global `allow`

stop [`mixed_attributes_style`] from linting on different attributes

add `@compile-flags` to [`mixed_attributes_style`]'s test;

turns out not linting in test mod is not a FN.

Apply suggestions from code review

Co-authored-by: Timo <[email protected]>

move [`mixed_attributes_style`] to late pass and stop it from linting on different kind of attributes
@bors bors closed this as completed in db41621 Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants