From def78c9acce95f33f56e3f601ccb2e4dc02bee11 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Tue, 24 Dec 2024 12:47:16 +0000 Subject: [PATCH 1/2] Store Clippy repository in a `Rc` To facilitate an experimental auto-correct feature --- dylint/src/package_options/revs.rs | 60 ++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/dylint/src/package_options/revs.rs b/dylint/src/package_options/revs.rs index fbd755bc0..101ae2889 100644 --- a/dylint/src/package_options/revs.rs +++ b/dylint/src/package_options/revs.rs @@ -5,6 +5,7 @@ use dylint_internal::{ git2::{Commit, ObjectType, Oid, Repository}, }; use if_chain::if_chain; +use std::{cell::RefCell, rc::Rc}; use tempfile::{tempdir, TempDir}; const RUST_CLIPPY_URL: &str = "https://github.com/rust-lang/rust-clippy"; @@ -17,30 +18,28 @@ pub struct Rev { } pub struct Revs { - tempdir: TempDir, - repository: Repository, + repository: Rc, } pub struct RevIter<'revs> { - revs: &'revs Revs, + repository: Rc, commit: Commit<'revs>, curr_rev: Option, } impl Revs { pub fn new(quiet: bool) -> Result { - let tempdir = tempdir().with_context(|| "`tempdir` failed")?; - - let repository = clone(RUST_CLIPPY_URL, "master", tempdir.path(), quiet)?; + let repository = clippy_repository(quiet)?; - Ok(Self { - tempdir, - repository, - }) + Ok(Self { repository }) } #[allow(clippy::iter_not_returning_iterator)] pub fn iter(&self) -> Result { + let path = self + .repository + .workdir() + .ok_or_else(|| anyhow!("Could not get working directory"))?; let object = { let head = self.repository.head()?; let oid = head.target().ok_or_else(|| anyhow!("Could not get HEAD"))?; @@ -49,11 +48,11 @@ impl Revs { let commit = object .as_commit() .ok_or_else(|| anyhow!("Object is not a commit"))?; - let version = clippy_utils_package_version(self.tempdir.path())?; - let channel = toolchain_channel(self.tempdir.path())?; + let version = clippy_utils_package_version(path)?; + let channel = toolchain_channel(path)?; let oid = commit.id(); Ok(RevIter { - revs: self, + repository: self.repository.clone(), commit: commit.clone(), curr_rev: Some(Rev { version, @@ -79,6 +78,10 @@ impl Iterator for RevIter<'_> { let curr_rev = if let Some(rev) = self.curr_rev.take() { rev } else { + let path = self + .repository + .workdir() + .ok_or_else(|| anyhow!("Could not get working directory"))?; // smoelius: Note that this is not `git log`'s default behavior. Rather, this // behavior corresponds to: // git log --first-parent @@ -88,20 +91,18 @@ impl Iterator for RevIter<'_> { } else { return Ok(None); }; - self.revs - .repository + self.repository .checkout_tree(commit.as_object(), None) .with_context(|| { format!("`checkout_tree` failed for `{:?}`", commit.as_object()) })?; - self.revs - .repository + self.repository .set_head_detached(commit.id()) .with_context(|| { format!("`set_head_detached` failed for `{}`", commit.id()) })?; - let version = clippy_utils_package_version(self.revs.tempdir.path())?; - let channel = toolchain_channel(self.revs.tempdir.path())?; + let version = clippy_utils_package_version(path)?; + let channel = toolchain_channel(path)?; let oid = commit.id(); Rev { version, @@ -124,6 +125,27 @@ impl Iterator for RevIter<'_> { } } +// smoelius: `thread_local!` because `git2::Repository` cannot be shared between threads safely. +thread_local! { + static TMPDIR_AND_REPOSITORY: RefCell)>> = const { RefCell::new(None) }; +} + +pub fn clippy_repository(quiet: bool) -> Result> { + TMPDIR_AND_REPOSITORY.with_borrow_mut(|cell| { + if let Some((_, repository)) = cell { + return Ok(repository.clone()); + } + + let tempdir = tempdir().with_context(|| "`tempdir` failed")?; + + let repository = clone(RUST_CLIPPY_URL, "master", tempdir.path(), quiet).map(Rc::new)?; + + cell.replace((tempdir, repository.clone())); + + Ok(repository) + }) +} + #[allow(clippy::unwrap_used)] #[cfg(test)] mod test { From 75176ac2f71dfdaa2f25a78585b76bb42a3bef67 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Wed, 25 Dec 2024 13:02:33 +0000 Subject: [PATCH 2/2] Create dylint/src/package_options/common.rs --- dylint/src/package_options/common.rs | 41 ++++++++++++++++++++++++++++ dylint/src/package_options/mod.rs | 17 ++---------- dylint/src/package_options/revs.rs | 28 ++----------------- 3 files changed, 46 insertions(+), 40 deletions(-) create mode 100644 dylint/src/package_options/common.rs diff --git a/dylint/src/package_options/common.rs b/dylint/src/package_options/common.rs new file mode 100644 index 000000000..fb642721d --- /dev/null +++ b/dylint/src/package_options/common.rs @@ -0,0 +1,41 @@ +use anyhow::{Context, Result}; +use dylint_internal::{clone, git2::Repository}; +use std::{cell::RefCell, rc::Rc}; +use tempfile::{tempdir, TempDir}; + +const RUST_CLIPPY_URL: &str = "https://github.com/rust-lang/rust-clippy"; + +// smoelius: `thread_local!` because `git2::Repository` cannot be shared between threads safely. +thread_local! { + static TMPDIR_AND_REPOSITORY: RefCell)>> = const { RefCell::new(None) }; +} + +pub fn clippy_repository(quiet: bool) -> Result> { + TMPDIR_AND_REPOSITORY.with_borrow_mut(|cell| { + if let Some((_, repository)) = cell { + return Ok(repository.clone()); + } + + let tempdir = tempdir().with_context(|| "`tempdir` failed")?; + + let repository = clone(RUST_CLIPPY_URL, "master", tempdir.path(), quiet).map(Rc::new)?; + + cell.replace((tempdir, repository.clone())); + + Ok(repository) + }) +} + +pub fn parse_as_nightly(channel: &str) -> Option<[u32; 3]> { + channel.strip_prefix("nightly-").and_then(parse_date) +} + +fn parse_date(date_str: &str) -> Option<[u32; 3]> { + date_str + .split('-') + .map(str::parse::) + .map(Result::ok) + .collect::>>() + .map(<[u32; 3]>::try_from) + .and_then(Result::ok) +} diff --git a/dylint/src/package_options/mod.rs b/dylint/src/package_options/mod.rs index 9cc4cc6f1..e5d78e666 100644 --- a/dylint/src/package_options/mod.rs +++ b/dylint/src/package_options/mod.rs @@ -18,6 +18,9 @@ use std::{ use tempfile::tempdir; use walkdir::WalkDir; +mod common; +use common::parse_as_nightly; + mod revs; use revs::Revs; @@ -158,17 +161,3 @@ pub fn upgrade_package(opts: &opts::Dylint, upgrade_opts: &opts::Upgrade) -> Res Ok(()) } - -fn parse_as_nightly(channel: &str) -> Option<[u32; 3]> { - channel.strip_prefix("nightly-").and_then(parse_date) -} - -fn parse_date(date_str: &str) -> Option<[u32; 3]> { - date_str - .split('-') - .map(str::parse::) - .map(Result::ok) - .collect::>>() - .map(<[u32; 3]>::try_from) - .and_then(Result::ok) -} diff --git a/dylint/src/package_options/revs.rs b/dylint/src/package_options/revs.rs index 101ae2889..883ce2a85 100644 --- a/dylint/src/package_options/revs.rs +++ b/dylint/src/package_options/revs.rs @@ -1,14 +1,11 @@ +use super::common::clippy_repository; use anyhow::{anyhow, Context, Result}; use dylint_internal::{ clippy_utils::{clippy_utils_package_version, toolchain_channel}, - clone, git2::{Commit, ObjectType, Oid, Repository}, }; use if_chain::if_chain; -use std::{cell::RefCell, rc::Rc}; -use tempfile::{tempdir, TempDir}; - -const RUST_CLIPPY_URL: &str = "https://github.com/rust-lang/rust-clippy"; +use std::rc::Rc; #[derive(Debug, Eq, PartialEq)] pub struct Rev { @@ -125,27 +122,6 @@ impl Iterator for RevIter<'_> { } } -// smoelius: `thread_local!` because `git2::Repository` cannot be shared between threads safely. -thread_local! { - static TMPDIR_AND_REPOSITORY: RefCell)>> = const { RefCell::new(None) }; -} - -pub fn clippy_repository(quiet: bool) -> Result> { - TMPDIR_AND_REPOSITORY.with_borrow_mut(|cell| { - if let Some((_, repository)) = cell { - return Ok(repository.clone()); - } - - let tempdir = tempdir().with_context(|| "`tempdir` failed")?; - - let repository = clone(RUST_CLIPPY_URL, "master", tempdir.path(), quiet).map(Rc::new)?; - - cell.replace((tempdir, repository.clone())); - - Ok(repository) - }) -} - #[allow(clippy::unwrap_used)] #[cfg(test)] mod test {