From cf39338b30bed614213c903ddb73e14b2bbf55e6 Mon Sep 17 00:00:00 2001 From: Nilstrieb <48135649+Nilstrieb@users.noreply.github.com> Date: Sun, 31 Dec 2023 17:01:16 +0100 Subject: [PATCH] add in memory AST --- src/formatting.rs | 7 +- src/processor/checker.rs | 2 +- src/processor/files.rs | 138 +++++++++++++++++++++++++++------------ src/processor/mod.rs | 29 ++++---- src/processor/reaper.rs | 22 ++++--- 5 files changed, 128 insertions(+), 70 deletions(-) diff --git a/src/formatting.rs b/src/formatting.rs index 5cc313a..a250cd4 100644 --- a/src/formatting.rs +++ b/src/formatting.rs @@ -1,7 +1,12 @@ use std::collections::HashMap; +use anyhow::Context; use genemichaels::FormatConfig; pub fn format(file: syn::File) -> anyhow::Result { - Ok(genemichaels::format_ast(file, &FormatConfig::default(), HashMap::new())?.rendered) + Ok( + genemichaels::format_ast(file, &FormatConfig::default(), HashMap::new()) + .context("formatting source file")? + .rendered, + ) } diff --git a/src/processor/checker.rs b/src/processor/checker.rs index 0be922f..748fda2 100644 --- a/src/processor/checker.rs +++ b/src/processor/checker.rs @@ -166,7 +166,7 @@ impl PassController { self.state = PassControllerState::Success; } PassControllerState::Bisecting { current, .. } => { - unreachable!("No change while bisecting, current was empty somehow: {current:?}"); + unreachable!("Pass said it didn't change anything in the bisection phase, nils forgot what this means: {current:?}"); } PassControllerState::Success { .. } => {} } diff --git a/src/processor/files.rs b/src/processor/files.rs index 1fe0e8b..f906982 100644 --- a/src/processor/files.rs +++ b/src/processor/files.rs @@ -1,13 +1,90 @@ -use anyhow::{Context, Result}; -use std::{ - fmt::Debug, - fs, - path::{Path, PathBuf}, -}; - -#[derive(PartialEq, Eq, Clone, Hash)] -pub(crate) struct SourceFile { - pub(crate) path: PathBuf, +use anyhow::Result; +use std::{fs, path::Path}; + +pub(crate) use self::file::SourceFile; + +mod file { + use anyhow::{Context, Result}; + use std::{ + cell::RefCell, + path::{Path, PathBuf}, + }; + + use super::{Changes, FileChange}; + + /// The representation of a source file, with the cached AST. + /// IMPORTANT INVARIANT: All file system operations MUST go through this type. + /// This also shouldn't be `Clone`, so the cache is always representative of the file system state. + /// It is inteded for the "cache" to be the source of truth. + pub(crate) struct SourceFile { + path: PathBuf, + content_str: RefCell, + content: RefCell, + } + + impl SourceFile { + pub(crate) fn open(path: PathBuf) -> Result { + let string = std::fs::read_to_string(&path) + .with_context(|| format!("reading file {}", path.display()))?; + let content = syn::parse_file(&string) + .with_context(|| format!("parsing file {}", path.display()))?; + Ok(SourceFile { + path, + content_str: RefCell::new(string), + content: RefCell::new(content), + }) + } + + pub(crate) fn write(&self, new: syn::File) -> Result<()> { + let string = crate::formatting::format(new.clone())?; + std::fs::write(&self.path, &string) + .with_context(|| format!("writing file {}", self.path.display()))?; + *self.content_str.borrow_mut() = string; + *self.content.borrow_mut() = new; + Ok(()) + } + + pub(crate) fn path_no_fs_interact(&self) -> &Path { + &self.path + } + } + + impl PartialEq for SourceFile { + fn eq(&self, other: &Self) -> bool { + self.path == other.path + } + } + + impl std::hash::Hash for SourceFile { + fn hash(&self, state: &mut H) { + self.path.hash(state); + } + } + + impl Eq for SourceFile {} + + impl std::fmt::Debug for SourceFile { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.path.display()) + } + } + + impl SourceFile { + pub(crate) fn try_change<'file, 'change>( + &'file self, + changes: &'change mut Changes, + ) -> Result> { + let path = &self.path; + Ok(FileChange { + path, + source_file: self, + changes, + has_written_change: false, + before_content_str: self.content_str.borrow().clone(), + before_content: self.content.borrow().clone(), + }) + } + } } #[derive(Default)] @@ -17,26 +94,29 @@ pub(crate) struct Changes { pub(crate) struct FileChange<'a, 'b> { pub(crate) path: &'a Path, - content: String, + source_file: &'a SourceFile, + before_content_str: String, + before_content: syn::File, changes: &'b mut Changes, has_written_change: bool, } impl FileChange<'_, '_> { - pub(crate) fn before_content(&self) -> &str { - &self.content + pub(crate) fn before_content(&self) -> (&str, &syn::File) { + (&self.before_content_str, &self.before_content) } - pub(crate) fn write(&mut self, new: &str) -> Result<()> { + pub(crate) fn write(&mut self, new: syn::File) -> Result<()> { self.has_written_change = true; - fs::write(self.path, new).with_context(|| format!("writing file {}", self.path.display())) + self.source_file.write(new)?; + Ok(()) } pub(crate) fn rollback(mut self) -> Result<()> { assert!(self.has_written_change); self.has_written_change = false; - fs::write(self.path, &self.content) - .with_context(|| format!("writing file {}", self.path.display())) + self.source_file.write(self.before_content.clone())?; + Ok(()) } pub(crate) fn commit(mut self) { @@ -49,7 +129,7 @@ impl FileChange<'_, '_> { impl Drop for FileChange<'_, '_> { fn drop(&mut self) { if self.has_written_change { - fs::write(self.path, self.before_content()).ok(); + fs::write(self.path, self.before_content().0).ok(); if !std::thread::panicking() { panic!("File contains unsaved changes!"); } @@ -57,30 +137,8 @@ impl Drop for FileChange<'_, '_> { } } -impl SourceFile { - pub(crate) fn try_change<'file, 'change>( - &'file self, - changes: &'change mut Changes, - ) -> Result> { - let path = &self.path; - Ok(FileChange { - path, - changes, - has_written_change: false, - content: fs::read_to_string(path) - .with_context(|| format!("opening file {}", path.display()))?, - }) - } -} - impl Changes { pub(crate) fn had_changes(&self) -> bool { self.any_change } } - -impl Debug for SourceFile { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - ::fmt(&self.path, f) - } -} diff --git a/src/processor/mod.rs b/src/processor/mod.rs index 28776c6..2843829 100644 --- a/src/processor/mod.rs +++ b/src/processor/mod.rs @@ -98,13 +98,13 @@ impl Minimizer { true } }) - .map(|entry| SourceFile { - path: entry.into_path(), - }) + .map(|entry| SourceFile::open(entry.into_path())) .inspect(|file| { - info!("Collecting file: {}", file.path.display()); + if let Ok(file) = file { + info!("Collecting file: {file:?}"); + } }) - .collect::>(); + .collect::>>()?; if files.is_empty() { bail!("Did not find any files for path {}", path.display()); @@ -188,20 +188,17 @@ impl Minimizer { let mut checker = PassController::new(self.options.clone()); loop { - let file_display = file.path.display(); let mut change = file.try_change(changes)?; - let mut krate = syn::parse_file(change.before_content()) - .with_context(|| format!("parsing file {file_display}"))?; + let (_, krate) = change.before_content(); + let mut krate = krate.clone(); let has_made_change = pass.process_file(&mut krate, file, &mut checker); match has_made_change { ProcessState::Changed | ProcessState::FileInvalidated => { - let result = crate::formatting::format(krate)?; - - change.write(&result)?; + change.write(krate)?; let after = self.build.build()?; - info!("{file_display}: After {}: {after}", pass.name()); + info!("{file:?}: After {}: {after}", pass.name()); if after.reproduces_issue() { change.commit(); @@ -217,13 +214,9 @@ impl Minimizer { } ProcessState::NoChange => { if self.options.no_color { - info!("{file_display}: After {}: no changes", pass.name()); + info!("{file:?}: After {}: no changes", pass.name()); } else { - info!( - "{file_display}: After {}: {}", - pass.name(), - "no changes".yellow() - ); + info!("{file:?}: After {}: {}", pass.name(), "no changes".yellow()); } checker.no_change(); } diff --git a/src/processor/reaper.rs b/src/processor/reaper.rs index a21b486..5361d15 100644 --- a/src/processor/reaper.rs +++ b/src/processor/reaper.rs @@ -10,7 +10,7 @@ use rustfix::{diagnostics::Diagnostic, Suggestion}; use std::{ collections::{HashMap, HashSet}, ops::Range, - path::Path, + path::{Path, PathBuf}, }; use syn::{visit_mut::VisitMut, ImplItem, Item}; @@ -63,7 +63,8 @@ impl Minimizer { ) -> Result<()> { for (sugg_file, suggestions) in suggestions { let Some(file) = self.files.iter().find(|source| { - source.path.ends_with(sugg_file) || sugg_file.ends_with(&source.path) + source.path_no_fs_interact().ends_with(sugg_file) + || sugg_file.ends_with(source.path_no_fs_interact()) }) else { continue; }; @@ -79,13 +80,14 @@ impl Minimizer { .cloned() .collect::>(); - let result = rustfix::apply_suggestions(change.before_content(), &desired_suggestions)?; - - change.write(&result)?; + let result = + rustfix::apply_suggestions(change.before_content().0, &desired_suggestions)?; + let result = syn::parse_file(&result).context("parsing file after rustfix")?; + change.write(result)?; let after = self.build.build()?; - info!("{}: After reaper: {after}", file.path.display()); + info!("{file:?}: After reaper: {after}"); if after.reproduces_issue() { change.commit(); @@ -101,7 +103,7 @@ impl Minimizer { struct DeleteUnusedFunctions { diags: Vec, build: Build, - invalid: HashSet, + invalid: HashSet, } impl DeleteUnusedFunctions { @@ -129,7 +131,7 @@ impl Pass for DeleteUnusedFunctions { checker: &mut super::PassController, ) -> ProcessState { assert!( - !self.invalid.contains(file), + !self.invalid.contains(file.path_no_fs_interact()), "processing with invalid state" ); @@ -137,7 +139,7 @@ impl Pass for DeleteUnusedFunctions { visitor.visit_file_mut(krate); if visitor.process_state == ProcessState::FileInvalidated { - self.invalid.insert(file.clone()); + self.invalid.insert(file.path_no_fs_interact().to_owned()); } visitor.process_state @@ -203,7 +205,7 @@ impl<'a> FindUnusedFunction<'a> { ); // When the project directory is remapped, the path may be absolute or generally have some prefix. - if !file.path.ends_with(&span.file_name) { + if !file.path_no_fs_interact().ends_with(&span.file_name) { return None; }