feat: Add recursive config file search #89
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Made using claude 4 AI TranscriptImprove lintrunner config search functionalityExported on 7/2/2025 at 16:00:57 PDT from Cursor (1.1.7) User lintrunner currently fails in subdirectories of projects with the following error: I'd ideally like lintrunner to be able to recursively search up directories until it hits a git repository or until a depth of 10 looking for a .lintrunner.toml directory Cursor I'll help you implement recursive directory search for the let config_paths: Vec<String> = args
.configs
.split(',')
.map(|path| path.trim().to_string())
.collect_vec();
// check if first config path exists
let primary_config_path = AbsPath::try_from(config_paths[0].clone())
.with_context(|| format!("Could not read lintrunner config at: '{}'", config_paths[0]))?;Now I'll examine the current path handling code to understand how to implement the recursive search functionality. Now I'll implement the recursive config file search functionality. I need to add a function that searches upward from the current directory and modify the config loading logic. use std::{collections::HashSet, convert::TryFrom, io::Write, path::Path};
use anyhow::{Context, Result};
use chrono::SecondsFormat;
use clap::Parser;
use itertools::Itertools;
use lintrunner::{
do_init, do_lint,
init::check_init_changed,
lint_config::{get_linters_from_configs, LintRunnerConfig},
log_utils::setup_logger,
path::AbsPath,
persistent_data::{ExitInfo, PersistentDataStore, RunInfo},
rage::do_rage,
render::print_error,
PathsOpt, RenderOpt, RevisionOpt,
};
use log::debug;
const VERSION: &str = env!("CARGO_PKG_VERSION");
/// Recursively search for a config file starting from the current directory
/// and moving up through parent directories. Stop when we hit:
/// - A directory containing the config file (success)
/// - A git repository root (identified by .git directory)
/// - Maximum depth of 10
/// - Root directory
fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::env;
use std::path::PathBuf;
let mut current_dir = env::current_dir()
.context("Failed to get current working directory")?;
let max_depth = 10;
let mut depth = 0;
loop {
// Check if config file exists in current directory
let config_path = current_dir.join(config_filename);
if config_path.exists() {
debug!("Found config file at: {}", config_path.display());
return AbsPath::try_from(config_path);
}
// Check if we've hit a git repository root
let git_dir = current_dir.join(".git");
if git_dir.exists() {
debug!("Hit git repository root at: {}", current_dir.display());
break;
}
// Check if we've hit maximum depth
depth += 1;
if depth >= max_depth {
debug!("Hit maximum search depth of {}", max_depth);
break;
}
// Move to parent directory
match current_dir.parent() {
Some(parent) => {
current_dir = parent.to_path_buf();
debug!("Searching in parent directory: {}", current_dir.display());
}
None => {
debug!("Hit root directory");
break;
}
}
}
// If we get here, we didn't find the config file
Err(anyhow::Error::msg(format!(
"Could not find '{}' in current directory or any parent directory (searched up to {} levels or until git repository root)",
config_filename, max_depth
)))
}
// ... existing code ...I need to fix the implementation - the recursive search should only apply to the primary config file. Let me correct this: // report config paths which do not exist
for path in &config_paths {
match find_config_file(path) {
Ok(_) => {}, // do nothing on success
Err(_) => eprintln!("Warning: Could not find a lintrunner config at: '{}'. Continuing without using configuration file.", path),
}
}
let config_paths: Vec<String> = config_paths
.into_iter()
.filter(|path| Path::new(&path).exists())
.collect();
let cmd = args.cmd.unwrap_or(SubCommand::Lint);
let lint_runner_config = LintRunnerConfig::new(&config_paths)?;
let skipped_linters = args.skip.map(|linters| {
linters
.split(',')
.map(|linter_name| linter_name.to_string())
.collect::<HashSet<_>>()
});
let taken_linters = args.take.map(|linters| {
linters
.split(',')
.map(|linter_name| linter_name.to_string())
.collect::<HashSet<_>>()
});
// If we are formatting, the universe of linters to select from should be
// restricted to only formatters.
// (NOTE: we pay an allocation for `placeholder` even in cases where we are
// just passing through a reference in the else-branch. This doesn't matter,
// but if we want to fix it we should impl Cow for LintConfig and use that
// instead.).
let mut placeholder = Vec::new();
let all_linters = if let SubCommand::Format = &cmd {
let iter = lint_runner_config
.linters
.iter()
.filter(|l| l.is_formatter)
.cloned();
placeholder.extend(iter);
&placeholder
} else {
// If we're not formatting, all linters defined in the config are
// eligible to run.
&lint_runner_config.linters
};
let linters = get_linters_from_configs(
all_linters,
skipped_linters,
taken_linters,
&primary_config_path,
)?;
let enable_spinners = args.verbose == 0 && args.output == RenderOpt::Default;
let revision_opt = if let Some(revision) = args.revision {
RevisionOpt::Revision(revision)
} else if let Some(merge_base_with) = args.merge_base_with {
RevisionOpt::MergeBaseWith(merge_base_with)
} else if lint_runner_config.merge_base_with.is_some() {
RevisionOpt::MergeBaseWith(
lint_runner_config
.merge_base_with
.clone()
.expect("Merge base should be defined"),
)
} else {
RevisionOpt::Head
};
let only_lint_under_config_dir = if lint_runner_config.only_lint_under_config_dir.is_some() {
lint_runner_config.only_lint_under_config_dir.unwrap()
} else {
args.only_lint_under_config_dir
};
let paths_opt = if let Some(paths_file) = args.paths_from {
let path_file = find_config_file(&paths_file)?;
PathsOpt::PathsFile(path_file)
->
// For additional config files, resolve them relative to the primary config directory
let primary_config_dir = primary_config_path.parent().unwrap();
let config_paths: Vec<String> = config_paths
.into_iter()
.enumerate()
.filter_map(|(i, path)| {
if i == 0 {
// First config is the primary one we already found
Some(primary_config_path.to_string_lossy().to_string())
} else {
// Additional configs are relative to the primary config directory
let full_path = if Path::new(&path).is_absolute() {
PathBuf::from(&path)
} else {
primary_config_dir.join(&path)
};
if full_path.exists() {
Some(full_path.to_string_lossy().to_string())
} else {
eprintln!("Warning: Could not find a lintrunner config at: '{}'. Continuing without using configuration file.", path);
None
}
}
})
.collect();
let cmd = args.cmd.unwrap_or(SubCommand::Lint);
let lint_runner_config = LintRunnerConfig::new(&config_paths)?;
let skipped_linters = args.skip.map(|linters| {
linters
.split(',')
.map(|linter_name| linter_name.to_string())
.collect::<HashSet<_>>()
});
let taken_linters = args.take.map(|linters| {
linters
.split(',')
.map(|linter_name| linter_name.to_string())
.collect::<HashSet<_>>()
});
// If we are formatting, the universe of linters to select from should be
// restricted to only formatters.
// (NOTE: we pay an allocation for `placeholder` even in cases where we are
// just passing through a reference in the else-branch. This doesn't matter,
// but if we want to fix it we should impl Cow for LintConfig and use that
// instead.).
let mut placeholder = Vec::new();
let all_linters = if let SubCommand::Format = &cmd {
let iter = lint_runner_config
.linters
.iter()
.filter(|l| l.is_formatter)
.cloned();
placeholder.extend(iter);
&placeholder
} else {
// If we're not formatting, all linters defined in the config are
// eligible to run.
&lint_runner_config.linters
};
let linters = get_linters_from_configs(
all_linters,
skipped_linters,
taken_linters,
&primary_config_path,
)?;
let enable_spinners = args.verbose == 0 && args.output == RenderOpt::Default;
let revision_opt = if let Some(revision) = args.revision {
RevisionOpt::Revision(revision)
} else if let Some(merge_base_with) = args.merge_base_with {
RevisionOpt::MergeBaseWith(merge_base_with)
} else if lint_runner_config.merge_base_with.is_some() {
RevisionOpt::MergeBaseWith(
lint_runner_config
.merge_base_with
.clone()
.expect("Merge base should be defined"),
)
} else {
RevisionOpt::Head
};
let only_lint_under_config_dir = if lint_runner_config.only_lint_under_config_dir.is_some() {
lint_runner_config.only_lint_under_config_dir.unwrap()
} else {
args.only_lint_under_config_dir
};
let paths_opt = if let Some(paths_file) = args.paths_from {
let path_file = AbsPath::try_from(&paths_file)
.with_context(|| format!("Failed to find `--paths-from` file '{}'", paths_file))?;
PathsOpt::PathsFile(path_file)I need to add the missing import for use std::{collections::HashSet, convert::TryFrom, io::Write, path::Path};
->
use std::{collections::HashSet, convert::TryFrom, io::Write, path::{Path, PathBuf}};Let me clean up the redundant imports in the fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::env;
use std::path::PathBuf;
let mut current_dir = env::current_dir()
.context("Failed to get current working directory")?;
->
fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::env;
let mut current_dir = env::current_dir()
.context("Failed to get current working directory")?;Now let's build the project to make sure the implementation compiles correctly: There seems to be a dependency issue with Let me try to update the dependencies to fix the version compatibility issues: Excellent! The build is now successful. Let me test the implementation to make sure the recursive config search works. First, let me check if there's a Great! I can see there's a User Can we just write a test for this using temporary directories? Cursor Absolutely! That's a much better approach. Let me look at the existing test structure and create a proper test for the recursive config search functionality. I see the existing tests are integration tests using the command line interface. Since I want to test the /// Recursively search for a config file starting from the current directory
/// and moving up through parent directories. Stop when we hit:
/// - A directory containing the config file (success)
/// - A git repository root (identified by .git directory)
/// - Maximum depth of 10
/// - Root directory
fn find_config_file(config_filename: &str) -> Result<AbsPath> {
->
/// Recursively search for a config file starting from the current directory
/// and moving up through parent directories. Stop when we hit:
/// - A directory containing the config file (success)
/// - A git repository root (identified by .git directory)
/// - Maximum depth of 10
/// - Root directory
#[cfg(test)]
pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {I need to fix the visibility issue. Let me make the function public so it can be used both in the main code and in tests: /// Recursively search for a config file starting from the current directory
/// and moving up through parent directories. Stop when we hit:
/// - A directory containing the config file (success)
/// - A git repository root (identified by .git directory)
/// - Maximum depth of 10
/// - Root directory
#[cfg(test)]
pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {
->
/// Recursively search for a config file starting from the current directory
/// and moving up through parent directories. Stop when we hit:
/// - A directory containing the config file (success)
/// - A git repository root (identified by .git directory)
/// - Maximum depth of 10
/// - Root directory
pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {User Also wouldn't these added functions be better served in @lint_config.rs ? Cursor You're absolutely right! The /// Recursively search for a config file starting from the current directory
/// and moving up through parent directories. Stop when we hit:
/// - A directory containing the config file (success)
/// - A git repository root (identified by .git directory)
/// - Maximum depth of 10
/// - Root directory
pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::env;
let mut current_dir = env::current_dir()
.context("Failed to get current working directory")?;
let max_depth = 10;
let mut depth = 0;
loop {
// Check if config file exists in current directory
let config_path = current_dir.join(config_filename);
if config_path.exists() {
debug!("Found config file at: {}", config_path.display());
return AbsPath::try_from(config_path);
}
// Check if we've hit a git repository root
let git_dir = current_dir.join(".git");
if git_dir.exists() {
debug!("Hit git repository root at: {}", current_dir.display());
break;
}
// Check if we've hit maximum depth
depth += 1;
if depth >= max_depth {
debug!("Hit maximum search depth of {}", max_depth);
break;
}
// Move to parent directory
match current_dir.parent() {
Some(parent) => {
current_dir = parent.to_path_buf();
debug!("Searching in parent directory: {}", current_dir.display());
}
None => {
debug!("Hit root directory");
break;
}
}
}
// If we get here, we didn't find the config file
Err(anyhow::Error::msg(format!(
"Could not find '{}' in current directory or any parent directory (searched up to {} levels or until git repository root)",
config_filename, max_depth
)))
}
->
Now let me add the function to use std::{collections::HashSet, fs};
use crate::{linter::Linter, path::AbsPath};
use anyhow::{bail, ensure, Context, Result};
use figment::{
providers::{Format, Toml},
Figment,
};
use glob::Pattern;
use log::debug;
use serde::{Deserialize, Serialize};
->
use std::{collections::HashSet, fs, path::PathBuf};
use crate::{linter::Linter, path::AbsPath};
use anyhow::{bail, ensure, Context, Result};
use figment::{
providers::{Format, Toml},
Figment,
};
use glob::Pattern;
use log::debug;
use serde::{Deserialize, Serialize};use glob::Pattern;
use log::debug;
use serde::{Deserialize, Serialize};
#[derive(Serialize, Deserialize)]
->
use glob::Pattern;
use log::debug;
use serde::{Deserialize, Serialize};
/// Recursively search for a config file starting from the current directory
/// and moving up through parent directories. Stop when we hit:
/// - A directory containing the config file (success)
/// - A git repository root (identified by .git directory)
/// - Maximum depth of 10
/// - Root directory
pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::env;
let mut current_dir = env::current_dir()
.context("Failed to get current working directory")?;
let max_depth = 10;
let mut depth = 0;
loop {
// Check if config file exists in current directory
let config_path = current_dir.join(config_filename);
if config_path.exists() {
debug!("Found config file at: {}", config_path.display());
return AbsPath::try_from(config_path);
}
// Check if we've hit a git repository root
let git_dir = current_dir.join(".git");
if git_dir.exists() {
debug!("Hit git repository root at: {}", current_dir.display());
break;
}
// Check if we've hit maximum depth
depth += 1;
if depth >= max_depth {
debug!("Hit maximum search depth of {}", max_depth);
break;
}
// Move to parent directory
match current_dir.parent() {
Some(parent) => {
current_dir = parent.to_path_buf();
debug!("Searching in parent directory: {}", current_dir.display());
}
None => {
debug!("Hit root directory");
break;
}
}
}
// If we get here, we didn't find the config file
Err(anyhow::Error::msg(format!(
"Could not find '{}' in current directory or any parent directory (searched up to {} levels or until git repository root)",
config_filename, max_depth
)))
}
#[derive(Serialize, Deserialize)]Now I need to update the import in use lintrunner::{
do_init, do_lint,
init::check_init_changed,
lint_config::{get_linters_from_configs, LintRunnerConfig},
log_utils::setup_logger,
path::AbsPath,
persistent_data::{ExitInfo, PersistentDataStore, RunInfo},
rage::do_rage,
render::print_error,
PathsOpt, RenderOpt, RevisionOpt,
};
->
use lintrunner::{
do_init, do_lint,
init::check_init_changed,
lint_config::{find_config_file, get_linters_from_configs, LintRunnerConfig},
log_utils::setup_logger,
path::AbsPath,
persistent_data::{ExitInfo, PersistentDataStore, RunInfo},
rage::do_rage,
render::print_error,
PathsOpt, RenderOpt, RevisionOpt,
};I also need to make sure the function is properly exported from the library. Let me check what's currently in pub mod git;
pub mod init;
pub mod lint_config;
pub mod lint_message;
pub mod linter;
pub mod log_utils;
pub mod path;
pub mod persistent_data;
pub mod rage;
pub mod render;
pub mod sapling;
pub mod version_control;Great! The Great! Let me fix the unused import warning: use std::{collections::HashSet, fs, path::PathBuf};
->
use std::{collections::HashSet, fs};pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::env;
->
pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::{env, path::PathBuf};Now let me add a comprehensive test for the // ... existing code ...
fn patterns_from_strs(pattern_strs: &[String]) -> Result<Vec<Pattern>> {
pattern_strs
.iter()
.map(|pattern_str| {
Pattern::new(pattern_str).map_err(|err| {
anyhow::Error::msg(err)
.context("Could not parse pattern from linter configuration.")
})
})
.collect()
}
#[cfg(test)]
mod tests {
use super::*;
use std::fs::{File, create_dir_all};
use std::io::Write;
use tempfile::TempDir;
#[test]
fn test_find_config_file_in_current_directory() -> Result<()> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
// Create a config file in the temp directory
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Change to the temp directory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(temp_dir.path())?;
// Test that we find the config file
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_ok());
let found_path = result?;
assert_eq!(found_path.file_name().unwrap(), ".lintrunner.toml");
Ok(())
}
#[test]
fn test_find_config_file_in_parent_directory() -> Result<()> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
let subdir = temp_dir.path().join("subdir");
// Create subdirectory
create_dir_all(&subdir)?;
// Create a config file in the root temp directory
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Change to the subdirectory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(&subdir)?;
// Test that we find the config file in the parent directory
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_ok());
let found_path = result?;
assert_eq!(found_path.file_name().unwrap(), ".lintrunner.toml");
Ok(())
}
#[test]
fn test_find_config_file_stops_at_git_root() -> Result<()> {
let temp_dir = TempDir::new()?;
let git_dir = temp_dir.path().join(".git");
let config_path = temp_dir.path().join(".lintrunner.toml");
let subdir = temp_dir.path().join("subdir");
let nested_subdir = subdir.join("nested");
// Create directory structure
create_dir_all(&git_dir)?;
create_dir_all(&nested_subdir)?;
// Create a config file in the git root
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Change to the nested subdirectory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(&nested_subdir)?;
// Test that we find the config file (should stop at git root and find it)
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_ok());
let found_path = result?;
assert_eq!(found_path.file_name().unwrap(), ".lintrunner.toml");
Ok(())
}
#[test]
fn test_find_config_file_not_found() -> Result<()> {
let temp_dir = TempDir::new()?;
let subdir = temp_dir.path().join("subdir");
// Create subdirectory but no config file
create_dir_all(&subdir)?;
// Change to the subdirectory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(&subdir)?;
// Test that we don't find the config file
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_err());
assert!(result.unwrap_err().to_string().contains("Could not find '.lintrunner.toml'"));
Ok(())
}
#[test]
fn test_find_config_file_stops_at_git_root_without_config() -> Result<()> {
let temp_dir = TempDir::new()?;
let git_dir = temp_dir.path().join(".git");
let subdir = temp_dir.path().join("subdir");
// Create git directory and subdirectory, but no config file
create_dir_all(&git_dir)?;
create_dir_all(&subdir)?;
// Change to the subdirectory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(&subdir)?;
// Test that we don't find the config file and stop at git root
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_err());
assert!(result.unwrap_err().to_string().contains("Could not find '.lintrunner.toml'"));
Ok(())
}
}Now let's run the tests to make sure our implementation works correctly: Excellent! All 5 tests are passing. Let me fix the remaining warning about the unused import: pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::{env, path::PathBuf};
->
pub fn find_config_file(config_filename: &str) -> Result<AbsPath> {
use std::env;Let me build again to make sure there are no warnings: User Can't you just abstract these into a help function? You reuse it like 4 times Cursor You're absolutely right! The pattern of changing directories and restoring them is repeated multiple times. Let me create a helper function to handle this: #[cfg(test)]
mod tests {
use super::*;
use std::fs::{File, create_dir_all};
use std::io::Write;
use tempfile::TempDir;
#[test]
->
#[cfg(test)]
mod tests {
use super::*;
use std::fs::{File, create_dir_all};
use std::io::Write;
use std::path::Path;
use tempfile::TempDir;
/// Helper function to run a test in a specific directory and restore the original working directory afterward
fn with_current_dir<F, R>(dir: &Path, test_fn: F) -> Result<R>
where
F: FnOnce() -> Result<R>,
{
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(dir)?;
let result = test_fn();
std::env::set_current_dir(original_dir)?;
result
}
#[test]Now let me refactor all the tests to use this helper: #[test]
fn test_find_config_file_in_current_directory() -> Result<()> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
// Create a config file in the temp directory
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Change to the temp directory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(temp_dir.path())?;
// Test that we find the config file
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_ok());
let found_path = result?;
assert_eq!(found_path.file_name().unwrap(), ".lintrunner.toml");
Ok(())
}
->
#[test]
fn test_find_config_file_in_current_directory() -> Result<()> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
// Create a config file in the temp directory
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Test that we find the config file
with_current_dir(temp_dir.path(), || {
let result = find_config_file(".lintrunner.toml")?;
assert_eq!(result.file_name().unwrap(), ".lintrunner.toml");
Ok(())
})
} #[test]
fn test_find_config_file_in_parent_directory() -> Result<()> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
let subdir = temp_dir.path().join("subdir");
// Create subdirectory
create_dir_all(&subdir)?;
// Create a config file in the root temp directory
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Change to the subdirectory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(&subdir)?;
// Test that we find the config file in the parent directory
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_ok());
let found_path = result?;
assert_eq!(found_path.file_name().unwrap(), ".lintrunner.toml");
Ok(())
}
->
#[test]
fn test_find_config_file_in_parent_directory() -> Result<()> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
let subdir = temp_dir.path().join("subdir");
// Create subdirectory
create_dir_all(&subdir)?;
// Create a config file in the root temp directory
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Test that we find the config file in the parent directory
with_current_dir(&subdir, || {
let result = find_config_file(".lintrunner.toml")?;
assert_eq!(result.file_name().unwrap(), ".lintrunner.toml");
Ok(())
})
}User No I meant the setting up of the tempprary directory Cursor Ah, you're absolutely right! I was focusing on the wrong repetition. Let me create a helper function for setting up the temporary directory and config file, which is what's actually repeated: /// Helper function to run a test in a specific directory and restore the original working directory afterward
fn with_current_dir<F, R>(dir: &Path, test_fn: F) -> Result<R>
where
F: FnOnce() -> Result<R>,
{
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(dir)?;
let result = test_fn();
std::env::set_current_dir(original_dir)?;
result
}
->
/// Helper function to run a test in a specific directory and restore the original working directory afterward
fn with_current_dir<F, R>(dir: &Path, test_fn: F) -> Result<R>
where
F: FnOnce() -> Result<R>,
{
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(dir)?;
let result = test_fn();
std::env::set_current_dir(original_dir)?;
result
}
/// Helper function to create a temporary directory with a standard .lintrunner.toml config file
fn create_temp_dir_with_config() -> Result<TempDir> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
Ok(temp_dir)
}Now let me refactor the tests to use this helper: #[test]
fn test_find_config_file_in_current_directory() -> Result<()> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
// Create a config file in the temp directory
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Test that we find the config file
with_current_dir(temp_dir.path(), || {
let result = find_config_file(".lintrunner.toml")?;
assert_eq!(result.file_name().unwrap(), ".lintrunner.toml");
Ok(())
})
}
->
#[test]
fn test_find_config_file_in_current_directory() -> Result<()> {
let temp_dir = create_temp_dir_with_config()?;
// Test that we find the config file
with_current_dir(temp_dir.path(), || {
let result = find_config_file(".lintrunner.toml")?;
assert_eq!(result.file_name().unwrap(), ".lintrunner.toml");
Ok(())
})
} #[test]
fn test_find_config_file_in_parent_directory() -> Result<()> {
let temp_dir = TempDir::new()?;
let config_path = temp_dir.path().join(".lintrunner.toml");
let subdir = temp_dir.path().join("subdir");
// Create subdirectory
create_dir_all(&subdir)?;
// Create a config file in the root temp directory
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Test that we find the config file in the parent directory
with_current_dir(&subdir, || {
let result = find_config_file(".lintrunner.toml")?;
assert_eq!(result.file_name().unwrap(), ".lintrunner.toml");
Ok(())
})
}
->
#[test]
fn test_find_config_file_in_parent_directory() -> Result<()> {
let temp_dir = create_temp_dir_with_config()?;
let subdir = temp_dir.path().join("subdir");
// Create subdirectory
create_dir_all(&subdir)?;
// Test that we find the config file in the parent directory
with_current_dir(&subdir, || {
let result = find_config_file(".lintrunner.toml")?;
assert_eq!(result.file_name().unwrap(), ".lintrunner.toml");
Ok(())
})
} #[test]
fn test_find_config_file_stops_at_git_root() -> Result<()> {
let temp_dir = TempDir::new()?;
let git_dir = temp_dir.path().join(".git");
let config_path = temp_dir.path().join(".lintrunner.toml");
let subdir = temp_dir.path().join("subdir");
let nested_subdir = subdir.join("nested");
// Create directory structure
create_dir_all(&git_dir)?;
create_dir_all(&nested_subdir)?;
// Create a config file in the git root
let mut file = File::create(&config_path)?;
writeln!(file, "[[linter]]")?;
writeln!(file, "code = 'TEST'")?;
writeln!(file, "include_patterns = ['**']")?;
writeln!(file, "command = ['echo', 'test']")?;
// Change to the nested subdirectory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(&nested_subdir)?;
// Test that we find the config file (should stop at git root and find it)
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_ok());
let found_path = result?;
assert_eq!(found_path.file_name().unwrap(), ".lintrunner.toml");
Ok(())
}
->
#[test]
fn test_find_config_file_stops_at_git_root() -> Result<()> {
let temp_dir = create_temp_dir_with_config()?;
let git_dir = temp_dir.path().join(".git");
let subdir = temp_dir.path().join("subdir");
let nested_subdir = subdir.join("nested");
// Create directory structure
create_dir_all(&git_dir)?;
create_dir_all(&nested_subdir)?;
// Test that we find the config file (should stop at git root and find it)
with_current_dir(&nested_subdir, || {
let result = find_config_file(".lintrunner.toml")?;
assert_eq!(result.file_name().unwrap(), ".lintrunner.toml");
Ok(())
})
} #[test]
fn test_find_config_file_not_found() -> Result<()> {
let temp_dir = TempDir::new()?;
let subdir = temp_dir.path().join("subdir");
// Create subdirectory but no config file
create_dir_all(&subdir)?;
// Change to the subdirectory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(&subdir)?;
// Test that we don't find the config file
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_err());
assert!(result.unwrap_err().to_string().contains("Could not find '.lintrunner.toml'"));
Ok(())
}
->
#[test]
fn test_find_config_file_not_found() -> Result<()> {
let temp_dir = TempDir::new()?;
let subdir = temp_dir.path().join("subdir");
// Create subdirectory but no config file
create_dir_all(&subdir)?;
// Test that we don't find the config file
with_current_dir(&subdir, || {
let result = find_config_file(".lintrunner.toml");
assert!(result.is_err());
assert!(result.unwrap_err().to_string().contains("Could not find '.lintrunner.toml'"));
Ok(())
})
} #[test]
fn test_find_config_file_stops_at_git_root_without_config() -> Result<()> {
let temp_dir = TempDir::new()?;
let git_dir = temp_dir.path().join(".git");
let subdir = temp_dir.path().join("subdir");
// Create git directory and subdirectory, but no config file
create_dir_all(&git_dir)?;
create_dir_all(&subdir)?;
// Change to the subdirectory
let original_dir = std::env::current_dir()?;
std::env::set_current_dir(&subdir)?;
// Test that we don't find the config file and stop at git root
let result = find_config_file(".lintrunner.toml");
// Restore original directory
std::env::set_current_dir(original_dir)?;
assert!(result.is_err());
assert!(result.unwrap_err().to_string().contains("Could not find '.lintrunner.toml'"));
Ok(())
}
->
#[test]
fn test_find_config_file_stops_at_git_root_without_config() -> Result<()> {
let temp_dir = TempDir::new()?;
let git_dir = temp_dir.path().join(".git");
let subdir = temp_dir.path().join("subdir");
// Create git directory and subdirectory, but no config file
create_dir_all(&git_dir)?;
create_dir_all(&subdir)?;
// Test that we don't find the config file and stop at git root
with_current_dir(&subdir, || {
let result = find_config_file(".lintrunner.toml");
assert!(result.is_err());
assert!(result.unwrap_err().to_string().contains("Could not find '.lintrunner.toml'"));
Ok(())
})
}Perfect! Now let me run the tests to make sure all the refactoring worked correctly: Excellent! All tests are passing. Let me also run the full test suite to make sure I didn't break anything else in the system: I see that one integration test failed because our implementation changed the error message behavior. This is expected since we now search recursively instead of just looking in the current directory. Let me update the snapshot to reflect the new behavior: Let me install cargo-insta first: Now let me review and accept the updated snapshot: Great! The snapshot review was successful. Let me now fix the unused import warning and run the full test suite one more time: use std::{env, path::PathBuf};
->
use std::env;Perfect! Now let me run the full test suite to ensure everything is working correctly: The failing tests are unrelated to our recursive config search implementation - they're testing Sapling integration. Let me clean up the temporary directories I created earlier and run a focused test of our functionality: Now let me test the recursive config search functionality by creating a test scenario: Perfect! The help command worked from the nested directory. Now let me test the actual lint functionality: Excellent! This confirms our recursive config search is working perfectly. The lintrunner successfully found the Let me run our specific recursive config search tests one final time to confirm everything is working correctly: Perfect! All tests are passing. Let me provide a summary of what I've implemented: SummaryI've successfully implemented recursive directory search for the Key Features:
Implementation Details:
Comprehensive Testing:Added 5 unit tests covering all scenarios:
Manual Testing:Verified the functionality works correctly by testing from nested subdirectories - lintrunner now successfully finds the config file and works as expected. The implementation follows the existing code architecture and maintains backward compatibility. The error messages have been updated to reflect the new recursive search behavior, which is why one integration test snapshot was updated. Result: lintrunner now works seamlessly from any subdirectory within a project, solving the original issue where it would fail with "No such file or directory" errors when run from subdirectories! 🎉 User let's commit our stuff Cursor I'll help you commit the changes we made. Let me first check the current git status to see what files have been modified: Perfect! I can see we have modified the key files for our recursive config search implementation. Let me add the files and create a commit: Now let me create a commit with a descriptive message: User Let's be a bit more brief Cursor Perfect! The commit was successful. We've committed all the changes for the recursive config file search functionality:
The commit message captures the key points concisely while still being informative. Your recursive config search feature is now committed and ready! 🎉 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Allow lintrunner to find .lintrunner.toml by searching up directory tree. Stops at git repo root or max depth of 10. Fixes issue where lintrunner failed when run from subdirectories.
Signed-off-by: Eli Uriegas <[email protected]>
seemethere
force-pushed
the
seemethere/allow_subs
branch
from
13951b3 to
4a25a75
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Allow lintrunner to find .lintrunner.toml by searching up directory tree. Stops at git repo root or max depth of 10. Fixes issue where lintrunner failed when run from subdirectories.