feat: Extend unused_function to other R package folders (#374)

Author: etiennebacher

crates/jarl-core/src/lints/base/duplicated_function_definition/duplicated_function_definition.rs

@@ -2,7 +2,7 @@ use biome_rowan::{TextRange, TextSize};
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 
-use crate::package::SharedFileData;
+use crate::package::{FileScope, SharedFileData};
 
 /// ## What it does
 ///
@@ -126,9 +126,9 @@ pub fn scan_top_level_assignments(content: &str) -> Vec<(String, TextRange, u32,
 pub(crate) fn compute_duplicates_from_shared(
     shared_data: &[SharedFileData],
 ) -> HashMap<PathBuf, Vec<(String, TextRange, String)>> {
-    // Group by package root
+    // Group by package root (only R/ files contribute to duplicate checking)
     let mut packages: HashMap<&str, Vec<&SharedFileData>> = HashMap::new();
-    for fd in shared_data {
+    for fd in shared_data.iter().filter(|fd| fd.scope == FileScope::R) {
         packages.entry(&fd.root_key).or_default().push(fd);
     }
 

crates/jarl-core/src/lints/base/unused_function/mod.rs

@@ -661,6 +661,171 @@ mod tests {
         );
     }
 
+    // ── unused functions in tests/ and inst/tinytest/ ───────────────
+
+    #[test]
+    fn test_unused_function_in_tests_flagged() {
+        let dir = TempDir::new().unwrap();
+        let r_dir = dir.path().join("R");
+        fs::create_dir(&r_dir).unwrap();
+        let tests_dir = dir.path().join("tests").join("testthat");
+        fs::create_dir_all(&tests_dir).unwrap();
+        fs::write(dir.path().join("DESCRIPTION"), "Package: test").unwrap();
+        fs::write(dir.path().join("NAMESPACE"), "export(public_fn)\n").unwrap();
+
+        let file_a = r_dir.join("public.R");
+        fs::write(&file_a, "public_fn <- function() 1\n").unwrap();
+
+        // Helper defined in tests/ but never used
+        let test_helper = tests_dir.join("helper.R");
+        fs::write(&test_helper, "unused_test_helper <- function() 42\n").unwrap();
+
+        let mut shared = scan_r_package_paths(std::slice::from_ref(&file_a), true);
+        shared.extend(scan_extra_package_paths(&[test_helper], dir.path()));
+        let result =
+            compute_unused_from_shared(&shared, &default_options(), &read_namespace(dir.path()));
+
+        let has_unused = result
+            .values()
+            .any(|v| v.iter().any(|(n, _, _)| n == "unused_test_helper"));
+        assert!(
+            has_unused,
+            "unused_test_helper is defined in tests/ but never called, should be flagged"
+        );
+    }
+
+    #[test]
+    fn test_used_function_in_tests_not_flagged() {
+        let dir = TempDir::new().unwrap();
+        let r_dir = dir.path().join("R");
+        fs::create_dir(&r_dir).unwrap();
+        let tests_dir = dir.path().join("tests").join("testthat");
+        fs::create_dir_all(&tests_dir).unwrap();
+        fs::write(dir.path().join("DESCRIPTION"), "Package: test").unwrap();
+        fs::write(dir.path().join("NAMESPACE"), "export(public_fn)\n").unwrap();
+
+        let file_a = r_dir.join("public.R");
+        fs::write(&file_a, "public_fn <- function() 1\n").unwrap();
+
+        // Helper defined in tests/ and used in another test file
+        let test_helper = tests_dir.join("helper.R");
+        fs::write(&test_helper, "test_helper <- function() 42\n").unwrap();
+
+        let test_file = tests_dir.join("test-foo.R");
+        fs::write(&test_file, "test_that('works', { test_helper() })\n").unwrap();
+
+        let mut shared = scan_r_package_paths(std::slice::from_ref(&file_a), true);
+        shared.extend(scan_extra_package_paths(
+            &[test_helper, test_file],
+            dir.path(),
+        ));
+        let result =
+            compute_unused_from_shared(&shared, &default_options(), &read_namespace(dir.path()));
+
+        let has_helper = result
+            .values()
+            .any(|v| v.iter().any(|(n, _, _)| n == "test_helper"));
+        assert!(
+            !has_helper,
+            "test_helper is used in another test file, should not be flagged"
+        );
+    }
+
+    #[test]
+    fn test_unused_function_in_inst_tinytest_flagged() {
+        let dir = TempDir::new().unwrap();
+        let r_dir = dir.path().join("R");
+        fs::create_dir(&r_dir).unwrap();
+        let inst_dir = dir.path().join("inst").join("tinytest");
+        fs::create_dir_all(&inst_dir).unwrap();
+        fs::write(dir.path().join("DESCRIPTION"), "Package: test").unwrap();
+        fs::write(dir.path().join("NAMESPACE"), "export(public_fn)\n").unwrap();
+
+        let file_a = r_dir.join("public.R");
+        fs::write(&file_a, "public_fn <- function() 1\n").unwrap();
+
+        // Helper defined in inst/tinytest/ but never used
+        let inst_helper = inst_dir.join("helper.R");
+        fs::write(&inst_helper, "unused_inst_helper <- function() 42\n").unwrap();
+
+        let mut shared = scan_r_package_paths(std::slice::from_ref(&file_a), true);
+        shared.extend(scan_extra_package_paths(&[inst_helper], dir.path()));
+        let result =
+            compute_unused_from_shared(&shared, &default_options(), &read_namespace(dir.path()));
+
+        let has_unused = result
+            .values()
+            .any(|v| v.iter().any(|(n, _, _)| n == "unused_inst_helper"));
+        assert!(
+            has_unused,
+            "unused_inst_helper is defined in inst/tinytest/ but never called, should be flagged"
+        );
+    }
+
+    #[test]
+    fn test_unused_function_in_inst_tests_flagged() {
+        let dir = TempDir::new().unwrap();
+        let r_dir = dir.path().join("R");
+        fs::create_dir(&r_dir).unwrap();
+        let inst_dir = dir.path().join("inst").join("tests");
+        fs::create_dir_all(&inst_dir).unwrap();
+        fs::write(dir.path().join("DESCRIPTION"), "Package: test").unwrap();
+        fs::write(dir.path().join("NAMESPACE"), "export(public_fn)\n").unwrap();
+
+        let file_a = r_dir.join("public.R");
+        fs::write(&file_a, "public_fn <- function() 1\n").unwrap();
+
+        // Helper defined in inst/tests/ but never used
+        let inst_helper = inst_dir.join("helper.R");
+        fs::write(&inst_helper, "unused_inst_helper <- function() 42\n").unwrap();
+
+        let mut shared = scan_r_package_paths(std::slice::from_ref(&file_a), true);
+        shared.extend(scan_extra_package_paths(&[inst_helper], dir.path()));
+        let result =
+            compute_unused_from_shared(&shared, &default_options(), &read_namespace(dir.path()));
+
+        let has_unused = result
+            .values()
+            .any(|v| v.iter().any(|(n, _, _)| n == "unused_inst_helper"));
+        assert!(
+            has_unused,
+            "unused_inst_helper is defined in inst/tests/ but never called, should be flagged"
+        );
+    }
+
+    #[test]
+    fn test_cross_scope_independence() {
+        let dir = TempDir::new().unwrap();
+        let r_dir = dir.path().join("R");
+        fs::create_dir(&r_dir).unwrap();
+        let tests_dir = dir.path().join("tests").join("testthat");
+        fs::create_dir_all(&tests_dir).unwrap();
+        fs::write(dir.path().join("DESCRIPTION"), "Package: test").unwrap();
+        fs::write(dir.path().join("NAMESPACE"), "export(public_fn)\n").unwrap();
+
+        // R/ file references `test_only_helper` but it shouldn't save the
+        // test-scoped definition from being flagged unused within tests/.
+        let file_a = r_dir.join("public.R");
+        fs::write(&file_a, "public_fn <- function() test_only_helper()\n").unwrap();
+
+        let test_helper = tests_dir.join("helper.R");
+        fs::write(&test_helper, "test_only_helper <- function() 42\n").unwrap();
+
+        let mut shared = scan_r_package_paths(std::slice::from_ref(&file_a), true);
+        shared.extend(scan_extra_package_paths(&[test_helper], dir.path()));
+        let result =
+            compute_unused_from_shared(&shared, &default_options(), &read_namespace(dir.path()));
+
+        let has_test_helper = result
+            .values()
+            .any(|v| v.iter().any(|(n, _, _)| n == "test_only_helper"));
+        assert!(
+            has_test_helper,
+            "test_only_helper is only referenced in R/ but defined in tests/, \
+             should be flagged as unused within tests/ scope"
+        );
+    }
+
     #[test]
     fn test_threshold_not_exceeded_shows_diagnostics() {
         let dir = TempDir::new().unwrap();

crates/jarl-core/src/lints/base/unused_function/unused_function.rs

@@ -3,14 +3,18 @@ use regex::Regex;
 use std::collections::{HashMap, HashSet};
 use std::path::{Path, PathBuf};
 
-use crate::package::SharedFileData;
+use crate::package::{FileScope, SharedFileData};
 
 /// ## What it does
 ///
-/// Checks for unused functions, currently limited to R packages. It looks for
-/// functions defined in the `R` folder that are not exported and not used
-/// anywhere in the package (including the `R`, `inst/tinytest`, `inst/tests`,
-/// `src`, and `tests` folders).
+/// Checks for unused functions in R packages. It looks for:
+///
+/// - Functions defined in `R/` that are not exported and not used anywhere in
+///   the package (including `R/`, `inst/tinytest/`, `inst/tests/`, `src/`, and
+///   `tests/`).
+/// - Functions defined in `tests/` that are not used anywhere in `tests/`.
+/// - Functions defined in `inst/tinytest/` or `inst/tests/` that are not used
+///   anywhere within that directory.
 ///
 /// ## Why is this bad?
 ///
@@ -245,6 +249,27 @@ pub(crate) fn has_cpp_extension(path: &Path) -> bool {
     )
 }
 
+/// Extract a human-readable scope directory from a file path, e.g.
+/// `"tests/"` or `"inst/tests/"`. Used in help messages.
+fn scope_dir_from_path(rel_path: &Path) -> String {
+    let components: Vec<_> = rel_path
+        .components()
+        .map(|c| c.as_os_str().to_string_lossy().to_string())
+        .collect();
+    for (i, comp) in components.iter().enumerate() {
+        if comp == "tests" {
+            return "tests/".to_string();
+        }
+        if comp == "inst"
+            && let Some(next) = components.get(i + 1)
+            && (next == "tinytest" || next == "tests")
+        {
+            return format!("inst/{next}/");
+        }
+    }
+    "tests/".to_string()
+}
+
 /// Compute unused functions from pre-scanned shared file data.
 ///
 /// This is the inner logic extracted from `compute_package_unused_functions`,
@@ -267,12 +292,27 @@ pub(crate) fn compute_unused_from_shared(
     let mut result: HashMap<PathBuf, Vec<(String, TextRange, String)>> = HashMap::new();
 
     for (_root_key, file_data) in packages {
-        // Separate R/ files from extra (test/src/tinytest) files.
-        let r_files: Vec<&&SharedFileData> = file_data.iter().filter(|f| f.is_r_dir_file).collect();
-        let extra_files: Vec<&&SharedFileData> =
-            file_data.iter().filter(|f| !f.is_r_dir_file).collect();
+        // Separate files by scope.
+        let r_files: Vec<&&SharedFileData> = file_data
+            .iter()
+            .filter(|f| f.scope == FileScope::R)
+            .collect();
+        let extra_files: Vec<&&SharedFileData> = file_data
+            .iter()
+            .filter(|f| f.scope != FileScope::R)
+            .collect();
+        let tests_files: Vec<&&SharedFileData> = file_data
+            .iter()
+            .filter(|f| f.scope == FileScope::Tests)
+            .collect();
+        let inst_files: Vec<&&SharedFileData> = file_data
+            .iter()
+            .filter(|f| f.scope == FileScope::Inst)
+            .collect();
 
-        // Collect ALL defined function names across the package (for exportPattern matching)
+        // ── R/ scope (existing behavior) ────────────────────────────────
+
+        // Collect ALL defined function names across R/ (for exportPattern matching)
         let all_defined_names: Vec<String> = r_files
             .iter()
             .flat_map(|f| f.assignments.iter().map(|(name, _, _, _)| name.clone()))
@@ -381,6 +421,63 @@ pub(crate) fn compute_unused_from_shared(
                 result.insert(file.rel_path.clone(), unused);
             }
         }
+
+        // ── Tests and Inst scopes ───────────────────────────────
+        // A function defined in one of these directories is unused if it
+        // doesn't appear in any other file within that same scope. No
+        // NAMESPACE export check is needed.
+
+        for scope_files in [&tests_files, &inst_files] {
+            if scope_files.is_empty() {
+                continue;
+            }
+
+            // Total symbol occurrences within this scope.
+            let mut scope_occurrences: HashMap<&str, usize> = HashMap::new();
+            for file in scope_files {
+                for (name, count) in &file.symbol_counts {
+                    *scope_occurrences.entry(name.as_str()).or_insert(0) += count;
+                }
+            }
+
+            // Total definitions within this scope.
+            let mut scope_definitions: HashMap<&str, usize> = HashMap::new();
+            for file in scope_files {
+                for (name, _, _, _) in &file.assignments {
+                    *scope_definitions.entry(name.as_str()).or_insert(0) += 1;
+                }
+            }
+
+            for file in scope_files {
+                let mut unused: Vec<(String, TextRange, String)> = Vec::new();
+
+                for (name, range, line, col) in &file.assignments {
+                    // Skip functions matching user-configured patterns
+                    if options.is_skipped(name) {
+                        continue;
+                    }
+
+                    let occurrences = scope_occurrences.get(name.as_str()).copied().unwrap_or(0);
+                    let definitions = scope_definitions.get(name.as_str()).copied().unwrap_or(0);
+
+                    if occurrences <= definitions {
+                        let scope_dir = scope_dir_from_path(&file.rel_path);
+                        let help = format!(
+                            "Defined at {path}:{line}:{col} but never called in {scope_dir}",
+                            path = file.rel_path.display()
+                        );
+                        unused.push((name.clone(), *range, help));
+                    }
+                }
+
+                if !unused.is_empty() {
+                    result
+                        .entry(file.rel_path.clone())
+                        .or_default()
+                        .extend(unused);
+                }
+            }
+        }
     }
 
     result