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

mod_module_files does nothing #8123

Closed
phi-fell opened this issue Dec 14, 2021 · 2 comments · Fixed by #8611
Closed

mod_module_files does nothing #8123

phi-fell opened this issue Dec 14, 2021 · 2 comments · Fixed by #8611
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@phi-fell
Copy link

phi-fell commented Dec 14, 2021

Summary

TL;DR: clippy::mod_module_files seems to do absolutely nothing, whether as a crate attribute or as a command line arg.

I'm using clippy via rustup

This occurs both on my laptop (WSL, Ubuntu 18.04) as well as a remote server (Ubuntu 20.04), but most of this was done on my laptop, I just checked the command line option on the remote to rule out this being WSL related.

cargo clippy -V: clippy 0.1.57 (f1edd04 2021-11-29)

EDIT:
In case this is useful: I cloned the clippy repo, and ran the tests (which passed) but if I manually cd into the test directories and run cargo clippy (or nightly) the results did not match up with what the relevant src/main.stderr contained.

i.e.

~/rust-clippy$ TESTNAME="module_style" cargo uitest
    Finished test [unoptimized + debuginfo] target(s) in 0.27s
     Running tests/compile-test.rs (target/debug/deps/compile_test-2c69db81b6b806f2)

running 1 test
test [ui] ui-cargo/module_style/fail_no_mod/src/main.rs ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

etc...

vs

~/rust-clippy/tests/ui-cargo/module_style/fail_mod$ cargo clippy
    Checking fail v0.1.0 (/home/username/rust-clippy/tests/ui-cargo/module_style/fail_mod)
    Finished dev [unoptimized + debuginfo] target(s) in 0.15s

^ would expect an error based on the contents of tests/ui-cargo/module_style/fail_mod/src/main.stderr (unless I'm misunderstanding how these tests work...)

Reproducer

I used cargo new my_project to start a new project.
I created a file my_project/src/my_module/mod.rs with the (presumably unimportant) contents:

pub fn my_fn() {
    println!("Text");
}

my_project/src/main.rs:

// seems to do nothing
#![forbid(clippy::mod_module_files)]
// seems to do nothing
#![forbid(clippy::self_named_module_files)]
// works fine
#![forbid(clippy::expect_used)]

mod my_module;

fn main() {
    my_module::my_fn();
    let option = Some(5);
    let seven = option.expect("see previous line") + 2;
    println!("seven is {}", seven);
}

running cargo clippy correctly gives an error for the use of .expect but no module errors

I expected to see an error due to a mod.rs file (I included self_named as well in case I somehow mixed the two up. - in a larger project that has both mod.rs and self named modules, neither lint seems to do anything.)

I thought this could be related to #6610 so I added a .cargo/config.toml:

[target.'cfg(feature = "cargo-clippy")']
rustflags = [
  "-Dclippy::mod_module_files",
  "-Dclippy::expect_used",
]

( I tried it with just expect, just module_files, both, neither)
and then I tried running
cargo clippy -- -W clippy::mod_module_files to no avail

all three of these methods worked with a variety of other lints I tried, and all three give an error if the named lint does not exist (or if the name is misspelled), I tried them each individually and in concert. I think this rules out e.g. accidentally using the wrong clippy version or a typo on my part (as there would have been an error that the lint does not exist, which did happen when I first tested on another machine, until I ran rustup update).

I also initially ran mod_module_files (and self named) on a larger project (the above was to redo it on a minimal example to make sure it wasn't some weird configuration issue or something on the larger project). I cannot get mod_module_files to give a warning/error on anything at all.

Version

rustc 1.57.0 (f1edd0429 2021-11-29)
binary: rustc
commit-hash: f1edd0429582dd29cccacaf50fd134b05593bd9c
commit-date: 2021-11-29
host: x86_64-unknown-linux-gnu
release: 1.57.0
LLVM version: 13.0.0

Additional Labels

No response

@phi-fell phi-fell added the C-bug Category: Clippy is not doing the correct thing label Dec 14, 2021
@phi-fell
Copy link
Author

phi-fell commented Dec 14, 2021

After further investigation, I'm a bit confused. In an attempt to debug this somewhat I cloned and edited clippy to put in some diagnostics, and followed the steps in basics.md (linked from CONTRIBUTING.md) to install clippy from source.

For some reason running cargo +nightly-2021-12-02 clippy does not run the edited and recompiled clippy (I tested by editing various lint messages), however running clippy-driver +nightly-2021-12-02 does run the edited clippy.

I may (?) have found the source of the issue:
module_style.rs

match &file.name {
                FileName::Real(RealFileName::LocalPath(lp))
                    if lp.to_string_lossy().starts_with(trim_to_src.as_ref()) =>
                {
...etc,

trim_to_src is the absolute path to the working dir, and when running tests lp is the absolute path to the file (e.g. /home/user/rust-clippy/tests/ui-cargo/module_style/fail_mod/src/main.rs), but when clippy-driver is run, lp is e.g. src/main.rs - which results in all files and directories being ignored by the lint (at least when run via clippy-driver)

I don't have a way to verify whether this is also true when clippy is run as cargo clippy, because, as mentioned above, that isn't working for me, and I don't really have a way to debug further without fixing that.

@DevinR528
Copy link
Contributor

DevinR528 commented Mar 14, 2022

The purpose of that match guard is to prevent it from warning on dependencies, basically, it was a way of filtering out paths like /cargo/registry/src/github-.com-123kfgvj1234/... but it seems to be filtering more than intended 🤔 .

I can also confirm on a new lib/bin with a module folder and a mod.rs file in it the lint never triggers, although on a large project with many subcrates I was able to get it to trigger.

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.

2 participants