Auto merge of #10975 - hehaoqian:fix_self_named_module_files, r=Centri3,xFrednet

Fix false positive of [self_named_module_files] and [mod_module_files]

changelog: [self_named_module_files] [mod_module_files]: No longer lints dependencies located in subdirectory of workspace

fixes #8887

---

First time contributor here, just read contribution guide today.

I have several questions:

1. ~Is it the correct way to use environment variable `CARGO_HOME` to get the location of cargo home directory?~
(Edit: Code no longer uses CARGO_HOME)
2. How to setup test for this PR? This involves multiple files and `CARGO_HOME` setup. ~Not sure how to do this.~
~Edit: Working on tests right now~ A workspace_test has been added

Author: bors

clippy_lints/src/module_style.rs

@@ -2,6 +2,7 @@ use rustc_ast::ast;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_lint::{EarlyContext, EarlyLintPass, Level, LintContext};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::def_id::LOCAL_CRATE;
 use rustc_span::{FileName, SourceFile, Span, SyntaxContext};
 use std::ffi::OsStr;
 use std::path::{Component, Path};
@@ -90,7 +91,14 @@ impl EarlyLintPass for ModStyle {
         // `{ foo => path/to/foo.rs, .. }
         let mut file_map = FxHashMap::default();
         for file in files.iter() {
-            if let FileName::Real(name) = &file.name && let Some(lp) = name.local_path() {
+            if let FileName::Real(name) = &file.name
+                && let Some(lp) = name.local_path()
+                && file.cnum == LOCAL_CRATE
+            {
+                // [#8887](https://github.com/rust-lang/rust-clippy/issues/8887)
+                // Only check files in the current crate.
+                // Fix false positive that crate dependency in workspace sub directory
+                // is checked unintentionally.
                 let path = if lp.is_relative() {
                     lp
                 } else if let Ok(relative) = lp.strip_prefix(trim_to_src) {

tests/workspace.rs

@@ -6,6 +6,46 @@ use test_utils::{CARGO_CLIPPY_PATH, IS_RUSTC_TEST_SUITE};
 
 mod test_utils;
 
+#[test]
+fn test_module_style_with_dep_in_subdir() {
+    if IS_RUSTC_TEST_SUITE {
+        return;
+    }
+    let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    let target_dir = root.join("target").join("workspace_test");
+    let cwd = root.join("tests/workspace_test");
+
+    // Make sure we start with a clean state
+    Command::new("cargo")
+        .current_dir(&cwd)
+        .env("CARGO_TARGET_DIR", &target_dir)
+        .arg("clean")
+        .args(["-p", "pass-no-mod-with-dep-in-subdir"])
+        .args(["-p", "pass-mod-with-dep-in-subdir"])
+        .output()
+        .unwrap();
+
+    // [#8887](https://github.com/rust-lang/rust-clippy/issues/8887)
+    // `mod.rs` checks should not be applied to crate dependencies
+    // located in the subdirectory of workspace
+    let output = Command::new(&*CARGO_CLIPPY_PATH)
+        .current_dir(&cwd)
+        .env("CARGO_INCREMENTAL", "0")
+        .env("CARGO_TARGET_DIR", &target_dir)
+        .arg("clippy")
+        .args(["-p", "pass-no-mod-with-dep-in-subdir"])
+        .args(["-p", "pass-mod-with-dep-in-subdir"])
+        .arg("--")
+        .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
+        .output()
+        .unwrap();
+
+    println!("status: {}", output.status);
+    println!("stdout: {}", String::from_utf8_lossy(&output.stdout));
+    println!("stderr: {}", String::from_utf8_lossy(&output.stderr));
+    assert!(output.status.success());
+}
+
 #[test]
 fn test_no_deps_ignores_path_deps_in_workspaces() {
     if IS_RUSTC_TEST_SUITE {

tests/workspace_test/Cargo.toml

@@ -4,4 +4,4 @@ version = "0.1.0"
 edition = "2018"
 
 [workspace]
-members = ["subcrate"]
+members = ["subcrate", "module_style/pass_no_mod_with_dep_in_subdir", "module_style/pass_mod_with_dep_in_subdir"]

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/Cargo.toml

@@ -0,0 +1,10 @@
+[package]
+name = "pass-mod-with-dep-in-subdir"
+version = "0.1.0"
+edition = "2018"
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+dep-no-mod = { path = "dep_no_mod"}

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/dep_no_mod/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "dep-no-mod"
+version = "0.1.0"
+edition = "2018"
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/dep_no_mod/src/foo.rs

@@ -0,0 +1,2 @@
+pub mod hello;
+pub struct Thing;

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/dep_no_mod/src/foo/hello.rs

@@ -0,0 +1 @@
+pub struct Hello;

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/dep_no_mod/src/lib.rs

@@ -0,0 +1,5 @@
+pub mod foo;
+
+pub fn foo() {
+    let _ = foo::Thing;
+}

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/src/bad/mod.rs

@@ -0,0 +1 @@
+pub struct Thing;

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/src/main.rs

@@ -0,0 +1,13 @@
+#![deny(clippy::self_named_module_files)]
+
+mod bad;
+mod more;
+extern crate dep_no_mod;
+
+fn main() {
+    let _ = bad::Thing;
+    let _ = more::foo::Foo;
+    let _ = more::inner::Inner;
+    let _ = dep_no_mod::foo::Thing;
+    let _ = dep_no_mod::foo::hello::Hello;
+}

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/src/more/foo.rs

@@ -0,0 +1 @@
+pub struct Foo;

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/src/more/inner/mod.rs

@@ -0,0 +1 @@
+pub struct Inner;

tests/workspace_test/module_style/pass_mod_with_dep_in_subdir/src/more/mod.rs

@@ -0,0 +1,2 @@
+pub mod foo;
+pub mod inner;

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/Cargo.toml

@@ -0,0 +1,10 @@
+[package]
+name = "pass-no-mod-with-dep-in-subdir"
+version = "0.1.0"
+edition = "2018"
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+dep-with-mod = { path = "dep_with_mod"}

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/dep_with_mod/Cargo.toml

@@ -0,0 +1,9 @@
+[package]
+name = "dep-with-mod"
+version = "0.1.0"
+edition = "2018"
+publish = false
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/dep_with_mod/src/lib.rs

@@ -0,0 +1,7 @@
+pub mod with_mod;
+
+pub fn foo() {
+    let _ = with_mod::Thing;
+    let _ = with_mod::inner::stuff::Inner;
+    let _ = with_mod::inner::stuff::most::Snarks;
+}

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/dep_with_mod/src/with_mod/inner.rs

@@ -0,0 +1 @@
+pub mod stuff;

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/dep_with_mod/src/with_mod/inner/stuff.rs

@@ -0,0 +1,3 @@
+pub mod most;
+
+pub struct Inner;

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/dep_with_mod/src/with_mod/inner/stuff/most.rs

@@ -0,0 +1 @@
+pub struct Snarks;

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/dep_with_mod/src/with_mod/mod.rs

@@ -0,0 +1,3 @@
+pub mod inner;
+
+pub struct Thing;

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/src/good.rs

@@ -0,0 +1 @@
+pub struct Thing;

tests/workspace_test/module_style/pass_no_mod_with_dep_in_subdir/src/main.rs

@@ -0,0 +1,9 @@
+#![deny(clippy::mod_module_files)]
+
+mod good;
+pub use dep_with_mod::with_mod::Thing;
+
+fn main() {
+    let _ = good::Thing;
+    let _ = dep_with_mod::with_mod::Thing;
+}