Skip to content

Commit

Permalink
fix: make --all-files work with sl
Browse files Browse the repository at this point in the history
  • Loading branch information
suo committed Feb 10, 2024
1 parent 0527cf0 commit 2df4572
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 12 deletions.
24 changes: 24 additions & 0 deletions src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,30 @@ impl version_control::System for Repo {
.collect::<Vec<AbsPath>>();
Ok(filtered_files)
}

fn get_all_files(&self, _under: Option<&AbsPath>) -> Result<Vec<AbsPath>> {
let output = Command::new("git")
.arg("grep")
.arg("-Il")
.arg(".")
.current_dir(&self.root)
.output()?;

ensure_output("git grep -Il", &output)?;

let files =
std::str::from_utf8(&output.stdout).context("failed to parse paths_cmd output")?;
let files = files
.lines()
.map(|s| s.to_string())
.collect::<HashSet<String>>();
let mut files = files.into_iter().collect::<Vec<String>>();
files.sort();
files
.into_iter()
.map(AbsPath::try_from)
.collect::<Result<_>>()
}
}

pub fn get_paths_from_cmd(paths_cmd: &str) -> Result<Vec<AbsPath>> {
Expand Down
18 changes: 11 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,12 @@ pub fn do_lint(
return Ok(0);
}

let config_dir = if only_lint_under_config_dir {
Some(AbsPath::try_from(linters[0].get_config_dir())?)
} else {
None
};

let mut files = match paths_opt {
PathsOpt::Auto => {
let relative_to = match revision_opt {
Expand All @@ -186,20 +192,18 @@ pub fn do_lint(
PathsOpt::PathsCmd(paths_cmd) => get_paths_from_cmd(&paths_cmd)?,
PathsOpt::Paths(paths) => get_paths_from_input(paths)?,
PathsOpt::PathsFile(file) => get_paths_from_file(file)?,
PathsOpt::AllFiles => get_paths_from_cmd("git grep -Il .")?,
PathsOpt::AllFiles => repo.get_all_files(config_dir.as_ref())?,
};

// Sort and unique the files so we pass a consistent ordering to linters
files.sort();
files.dedup();

if only_lint_under_config_dir {
let config_dir = linters[0].get_config_dir();
if let Some(config_dir) = config_dir {
files = files
.into_iter()
.filter(|path| path.starts_with(config_dir))
.filter(|path| path.starts_with(&config_dir))
.collect();
}
files.sort();
files.dedup();

let files = Arc::new(files);

Expand Down
83 changes: 79 additions & 4 deletions src/sapling.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use crate::{log_utils, path, version_control};
use crate::{
log_utils,
path::{self, AbsPath},
version_control,
};

use anyhow;

Expand Down Expand Up @@ -41,6 +45,44 @@ impl version_control::System for Repo {
Ok(merge_base.to_string())
}

fn get_all_files(&self, under: Option<&AbsPath>) -> anyhow::Result<Vec<AbsPath>> {
// Output of sl status looks like:
// D src/lib.rs
// M foo/bar.baz
let re = regex::Regex::new(r"^[A-Z?]\s+")?;

let mut cmd = std::process::Command::new("sl");
cmd.arg("status").arg("--all");
if let Some(under) = under {
cmd.arg(under.as_os_str());
}
cmd.current_dir(&self.root);
let output = cmd.output()?;
log_utils::ensure_output(&format!("{:?}", cmd), &output)?;
let commit_files_str = std::str::from_utf8(&output.stdout)?;
let commit_files: std::collections::HashSet<String> = commit_files_str
.split('\n')
.map(|x| x.to_string())
.map(|line| re.replace(&line, "").to_string())
.filter(|line| !line.is_empty())
.filter(|line| !line.starts_with('I'))
.collect();

let filtered_commit_files = commit_files
.into_iter()
.map(|f| format!("{}", self.root.join(f).display()))
.filter_map(|f| match path::AbsPath::try_from(&f) {
Ok(abs_path) => Some(abs_path),
Err(_) => {
eprintln!("Failed to find file while gathering files to lint: {}", f);
None
}
})
.collect::<Vec<path::AbsPath>>();

Ok(filtered_commit_files)
}

fn get_changed_files(&self, relative_to: Option<&str>) -> anyhow::Result<Vec<path::AbsPath>> {
// Output of sl status looks like:
// D src/lib.rs
Expand Down Expand Up @@ -92,7 +134,7 @@ mod tests {
use std::{fs::OpenOptions, io::Write, sync::Mutex}; // 1.4.0

static SL_GLOBAL_MUTEX: Lazy<Mutex<()>> = Lazy::new(Mutex::default);
use crate::testing;
use crate::{testing, version_control::System};

use super::*;
use anyhow::Result;
Expand Down Expand Up @@ -175,7 +217,6 @@ mod tests {
fn changed_files(&self, relative_to: Option<&str>) -> Result<Vec<String>> {
let _shared = SL_GLOBAL_MUTEX.lock().unwrap();
std::env::set_current_dir(&self.root)?;
use version_control::System;
let repo = Repo::new()?;
let files = repo.get_changed_files(relative_to)?;
let files = files
Expand All @@ -188,10 +229,16 @@ mod tests {
fn merge_base_with(&self, merge_base_with: &str) -> Result<String> {
let _shared = SL_GLOBAL_MUTEX.lock().unwrap();
std::env::set_current_dir(&self.root)?;
use version_control::System;
let repo = Repo::new()?;
repo.get_merge_base_with(merge_base_with)
}

fn get_all_files(&self) -> Result<Vec<AbsPath>> {
let _shared = SL_GLOBAL_MUTEX.lock().unwrap();
std::env::set_current_dir(&self.root)?;
let repo = Repo::new()?;
repo.get_all_files(None)
}
}

// Should properly detect changes in the commit (and not check other files)
Expand Down Expand Up @@ -374,6 +421,34 @@ mod tests {
Ok(())
}

#[test]
#[cfg_attr(target_os = "windows", ignore)] // remove when sapling installation is better
#[cfg_attr(target_os = "linux", ignore)] // remove when sapling installation is better
fn get_all_files() -> Result<()> {
let git = testing::GitCheckout::new()?;
git.write_file("test_1.txt", "Initial commit")?;
git.write_file("test_2.txt", "Initial commit")?;
git.write_file("test_3.txt", "Initial commit")?;
git.write_file("test_4.txt", "Initial commit")?;

git.add(".")?;
git.commit("I am main")?;
let sl = SaplingClone::new(&git)?;
let mut all_files = sl.get_all_files()?;
all_files.sort();
assert_eq!(
all_files,
vec!(
AbsPath::try_from("README")?,
AbsPath::try_from("test_1.txt")?,
AbsPath::try_from("test_2.txt")?,
AbsPath::try_from("test_3.txt")?,
AbsPath::try_from("test_4.txt")?
)
);
Ok(())
}

#[test]
#[cfg_attr(target_os = "windows", ignore)] // remove when sapling installation is better
#[cfg_attr(target_os = "linux", ignore)] // remove when sapling installation is better
Expand Down
13 changes: 12 additions & 1 deletion src/version_control.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use crate::{git, path, sapling};
use crate::{
git,
path::{self, AbsPath},
sapling,
};

use anyhow;

Expand Down Expand Up @@ -27,6 +31,9 @@ pub trait System {

// Gets the files that have changed relative to the given commit.
fn get_changed_files(&self, relative_to: Option<&str>) -> anyhow::Result<Vec<path::AbsPath>>;

// Get all files in the repo.
fn get_all_files(&self, under: Option<&AbsPath>) -> anyhow::Result<Vec<AbsPath>>;
}

impl Repo {
Expand Down Expand Up @@ -57,4 +64,8 @@ impl Repo {
RepoImpl::Sapling(sapling) => Box::new(sapling as &dyn System),
}
}

pub fn get_all_files(&self, under: Option<&AbsPath>) -> anyhow::Result<Vec<AbsPath>> {
self.get_system().get_all_files(under)
}
}

0 comments on commit 2df4572

Please sign in to comment.