Auto merge of #8611 - Alexendoo:module-files-relative-paths, r=llogiq

Handle relative paths in module_files lints

The problem being that when clippy is run in the project's directory `lp` would be a relative path, this wasn't caught by the tests as there `lp` is an absolute path. Being a relative path it did not start with `trim_src_path` and so was ignored

Also allowed the removal of some `.to_os_string`/`.to_owned`s

changelog: Fixes [`self_named_module_files`] and [`mod_module_files`] not linting

Fixes #8123, cc `@DevinR528`

Author: bors

clippy_lints/src/module_style.rs

@@ -1,13 +1,10 @@
-use std::{
-    ffi::OsString,
-    path::{Component, Path},
-};
-
 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::{FileName, RealFileName, SourceFile, Span, SyntaxContext};
+use std::ffi::OsStr;
+use std::path::{Component, Path};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -82,11 +79,7 @@ impl EarlyLintPass for ModStyle {
 
         let files = cx.sess().source_map().files();
 
-        let trim_to_src = if let RealFileName::LocalPath(p) = &cx.sess().opts.working_dir {
-            p.to_string_lossy()
-        } else {
-            return;
-        };
+        let RealFileName::LocalPath(trim_to_src) = &cx.sess().opts.working_dir else { return };
 
         // `folder_segments` is all unique folder path segments `path/to/foo.rs` gives
         // `[path, to]` but not foo
@@ -97,26 +90,27 @@ impl EarlyLintPass for ModStyle {
         // `{ foo => path/to/foo.rs, .. }
         let mut file_map = FxHashMap::default();
         for file in files.iter() {
-            match &file.name {
-                FileName::Real(RealFileName::LocalPath(lp))
-                    if lp.to_string_lossy().starts_with(trim_to_src.as_ref()) =>
-                {
-                    let p = lp.to_string_lossy();
-                    let path = Path::new(p.trim_start_matches(trim_to_src.as_ref()));
-                    if let Some(stem) = path.file_stem() {
-                        file_map.insert(stem.to_os_string(), (file, path.to_owned()));
-                    }
-                    process_paths_for_mod_files(path, &mut folder_segments, &mut mod_folders);
-                    check_self_named_mod_exists(cx, path, file);
-                },
-                _ => {},
+            if let FileName::Real(RealFileName::LocalPath(lp)) = &file.name {
+                let path = if lp.is_relative() {
+                    lp
+                } else if let Ok(relative) = lp.strip_prefix(trim_to_src) {
+                    relative
+                } else {
+                    continue;
+                };
+
+                if let Some(stem) = path.file_stem() {
+                    file_map.insert(stem, (file, path));
+                }
+                process_paths_for_mod_files(path, &mut folder_segments, &mut mod_folders);
+                check_self_named_mod_exists(cx, path, file);
             }
         }
 
         for folder in &folder_segments {
             if !mod_folders.contains(folder) {
                 if let Some((file, path)) = file_map.get(folder) {
-                    let mut correct = path.clone();
+                    let mut correct = path.to_path_buf();
                     correct.pop();
                     correct.push(folder);
                     correct.push("mod.rs");
@@ -138,25 +132,17 @@ impl EarlyLintPass for ModStyle {
 
 /// For each `path` we add each folder component to `folder_segments` and if the file name
 /// is `mod.rs` we add it's parent folder to `mod_folders`.
-fn process_paths_for_mod_files(
-    path: &Path,
-    folder_segments: &mut FxHashSet<OsString>,
-    mod_folders: &mut FxHashSet<OsString>,
+fn process_paths_for_mod_files<'a>(
+    path: &'a Path,
+    folder_segments: &mut FxHashSet<&'a OsStr>,
+    mod_folders: &mut FxHashSet<&'a OsStr>,
 ) {
     let mut comp = path.components().rev().peekable();
     let _ = comp.next();
     if path.ends_with("mod.rs") {
-        mod_folders.insert(comp.peek().map(|c| c.as_os_str().to_owned()).unwrap_or_default());
+        mod_folders.insert(comp.peek().map(|c| c.as_os_str()).unwrap_or_default());
     }
-    let folders = comp
-        .filter_map(|c| {
-            if let Component::Normal(s) = c {
-                Some(s.to_os_string())
-            } else {
-                None
-            }
-        })
-        .collect::<Vec<_>>();
+    let folders = comp.filter_map(|c| if let Component::Normal(s) = c { Some(s) } else { None });
     folder_segments.extend(folders);
 }
 

tests/ui-cargo/module_style/fail_mod/src/main.stderr

@@ -1,19 +1,19 @@
-error: `mod.rs` files are required, found `/bad/inner.rs`
+error: `mod.rs` files are required, found `bad/inner.rs`
   --> $DIR/bad/inner.rs:1:1
    |
 LL | pub mod stuff;
    | ^
    |
    = note: `-D clippy::self-named-module-files` implied by `-D warnings`
-   = help: move `/bad/inner.rs` to `/bad/inner/mod.rs`
+   = help: move `bad/inner.rs` to `bad/inner/mod.rs`
 
-error: `mod.rs` files are required, found `/bad/inner/stuff.rs`
+error: `mod.rs` files are required, found `bad/inner/stuff.rs`
   --> $DIR/bad/inner/stuff.rs:1:1
    |
 LL | pub mod most;
    | ^
    |
-   = help: move `/bad/inner/stuff.rs` to `/bad/inner/stuff/mod.rs`
+   = help: move `bad/inner/stuff.rs` to `bad/inner/stuff/mod.rs`
 
 error: aborting due to 2 previous errors
 

tests/ui-cargo/module_style/fail_no_mod/src/main.stderr

@@ -1,11 +1,11 @@
-error: `mod.rs` files are not allowed, found `/bad/mod.rs`
+error: `mod.rs` files are not allowed, found `bad/mod.rs`
   --> $DIR/bad/mod.rs:1:1
    |
 LL | pub struct Thing;
    | ^
    |
    = note: `-D clippy::mod-module-files` implied by `-D warnings`
-   = help: move `/bad/mod.rs` to `/bad.rs`
+   = help: move `bad/mod.rs` to `bad.rs`
 
 error: aborting due to previous error