From a76ddd42cef75dbd72773302027feb54b4306b1e Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 29 Mar 2021 13:39:48 -0500 Subject: [PATCH] refactor(cli): Pull out policy creation --- src/args.rs | 2 +- src/config.rs | 31 +++++---- src/file.rs | 13 ++-- src/main.rs | 52 +++++++------- src/policy.rs | 188 ++++++++++++++++++++++++++++++++++++++++++++------ 5 files changed, 217 insertions(+), 69 deletions(-) diff --git a/src/args.rs b/src/args.rs index 38d6b1d..16e2db7 100644 --- a/src/args.rs +++ b/src/args.rs @@ -101,7 +101,7 @@ pub(crate) struct Args { pub(crate) verbose: clap_verbosity_flag::Verbosity, } -#[derive(Debug, StructOpt)] +#[derive(Debug, Clone, StructOpt)] #[structopt(rename_all = "kebab-case")] pub(crate) struct FileArgs { #[structopt(long, overrides_with("no-binary"))] diff --git a/src/config.rs b/src/config.rs index fa10e79..6f9ca10 100644 --- a/src/config.rs +++ b/src/config.rs @@ -117,6 +117,17 @@ pub struct Config { } impl Config { + pub fn from_dir(cwd: &std::path::Path) -> Result, anyhow::Error> { + let config = if let Some(path) = + find_project_file(cwd, &["typos.toml", "_typos.toml", ".typos.toml"]) + { + Some(Self::from_file(&path)?) + } else { + None + }; + Ok(config) + } + pub fn from_file(path: &std::path::Path) -> Result { let s = std::fs::read_to_string(path)?; Self::from_toml(&s) @@ -134,14 +145,6 @@ impl Config { } } - pub fn derive(cwd: &std::path::Path) -> Result { - if let Some(path) = find_project_file(cwd, &["typos.toml", "_typos.toml", ".typos.toml"]) { - Self::from_file(&path) - } else { - Ok(Default::default()) - } - } - pub fn update(&mut self, source: &dyn ConfigSource) { if let Some(walk) = source.walk() { self.files.update(walk); @@ -522,13 +525,11 @@ impl DictSource for DictConfig { } fn find_project_file(dir: &std::path::Path, names: &[&str]) -> Option { - for ancestor in dir.ancestors() { - let mut file_path = ancestor.join("placeholder"); - for name in names { - file_path.set_file_name(name); - if file_path.exists() { - return Some(file_path); - } + let mut file_path = dir.join("placeholder"); + for name in names { + file_path.set_file_name(name); + if file_path.exists() { + return Some(file_path); } } None diff --git a/src/file.rs b/src/file.rs index bc2015e..549109c 100644 --- a/src/file.rs +++ b/src/file.rs @@ -552,11 +552,11 @@ fn fix_buffer(mut buffer: Vec, typos: impl Iterator Result<(), ignore::Error> { for entry in walk { - walk_entry(entry, checks, policy, reporter)?; + walk_entry(entry, checks, engine, reporter)?; } Ok(()) } @@ -564,13 +564,13 @@ pub fn walk_path( pub fn walk_path_parallel( walk: ignore::WalkParallel, checks: &dyn FileChecker, - policy: &crate::policy::Policy, + engine: &crate::policy::ConfigEngine, reporter: &dyn report::Report, ) -> Result<(), ignore::Error> { let error: std::sync::Mutex> = std::sync::Mutex::new(Ok(())); walk.run(|| { Box::new(|entry: Result| { - match walk_entry(entry, checks, policy, reporter) { + match walk_entry(entry, checks, engine, reporter) { Ok(()) => ignore::WalkState::Continue, Err(err) => { *error.lock().unwrap() = Err(err); @@ -586,7 +586,7 @@ pub fn walk_path_parallel( fn walk_entry( entry: Result, checks: &dyn FileChecker, - policy: &crate::policy::Policy, + engine: &crate::policy::ConfigEngine, reporter: &dyn report::Report, ) -> Result<(), ignore::Error> { let entry = match entry { @@ -603,7 +603,8 @@ fn walk_entry( } else { entry.path() }; - checks.check_file(path, explicit, policy, reporter)?; + let policy = engine.policy(path); + checks.check_file(path, explicit, &policy, reporter)?; } Ok(()) diff --git a/src/main.rs b/src/main.rs index 3e8a605..183d20d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -57,7 +57,19 @@ fn run_dump_config(args: &args::Args, output_path: &std::path::Path) -> proc_exi path.as_path() }; - let config = load_config(cwd, &args).with_code(proc_exit::Code::CONFIG_ERR)?; + let storage = typos_cli::policy::ConfigStorage::new(); + let mut overrides = config::EngineConfig::default(); + overrides.update(&args.overrides); + let mut engine = typos_cli::policy::ConfigEngine::new(&storage); + engine.set_isolated(args.isolated).set_overrides(overrides); + if let Some(path) = args.custom_config.as_ref() { + let custom = config::Config::from_file(path).with_code(proc_exit::Code::CONFIG_ERR)?; + engine.set_custom_config(custom); + } + let config = engine + .load_config(cwd) + .with_code(proc_exit::Code::CONFIG_ERR)?; + let mut defaulted_config = config::Config::from_defaults(); defaulted_config.update(&config); let output = toml::to_string_pretty(&defaulted_config).with_code(proc_exit::Code::FAILURE)?; @@ -88,12 +100,21 @@ fn run_checks(args: &args::Args) -> proc_exit::ExitResult { } else { path.as_path() }; - let config = load_config(cwd, &args).with_code(proc_exit::Code::CONFIG_ERR)?; let storage = typos_cli::policy::ConfigStorage::new(); - let engine = typos_cli::policy::ConfigEngine::new(config, &storage); - let files = engine.files(); - let policy = engine.policy(); + let mut overrides = config::EngineConfig::default(); + overrides.update(&args.overrides); + let mut engine = typos_cli::policy::ConfigEngine::new(&storage); + engine.set_isolated(args.isolated).set_overrides(overrides); + if let Some(path) = args.custom_config.as_ref() { + let custom = config::Config::from_file(path).with_code(proc_exit::Code::CONFIG_ERR)?; + engine.set_custom_config(custom); + } + + engine + .init_dir(cwd) + .with_code(proc_exit::Code::CONFIG_ERR)?; + let files = engine.files(cwd); let threads = if path.is_file() { 1 } else { args.threads }; let single_threaded = threads == 1; @@ -131,12 +152,12 @@ fn run_checks(args: &args::Args) -> proc_exit::ExitResult { }; if single_threaded { - typos_cli::file::walk_path(walk.build(), selected_checks, &policy, reporter) + typos_cli::file::walk_path(walk.build(), selected_checks, &engine, reporter) } else { typos_cli::file::walk_path_parallel( walk.build_parallel(), selected_checks, - &policy, + &engine, reporter, ) } @@ -189,20 +210,3 @@ fn init_logging(level: Option) { builder.init(); } } - -fn load_config(cwd: &std::path::Path, args: &args::Args) -> Result { - let mut config = config::Config::default(); - - if !args.isolated { - let derived = config::Config::derive(cwd)?; - config.update(&derived); - } - if let Some(path) = args.custom_config.as_ref() { - config.update(&config::Config::from_file(path)?); - } - - config.update(&args.config); - config.default.update(&args.overrides); - - Ok(config) -} diff --git a/src/policy.rs b/src/policy.rs index fb032f9..f5be135 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -1,30 +1,142 @@ pub struct ConfigStorage { - arena: typed_arena::Arena, + arena: std::sync::Mutex>, } impl ConfigStorage { pub fn new() -> Self { Self { - arena: typed_arena::Arena::new(), + arena: std::sync::Mutex::new(typed_arena::Arena::new()), } } fn get<'s>(&'s self, other: &str) -> &'s str { - self.arena.alloc(kstring::KString::from_ref(other)) + // Safe because we the references are stable once created. + // + // Trying to get this handled inside of `typed_arena` directly, see + // https://github.com/SimonSapin/rust-typed-arena/issues/49#issuecomment-809517312 + unsafe { + std::mem::transmute::<&str, &str>( + self.arena + .lock() + .unwrap() + .alloc(kstring::KString::from_ref(other)), + ) + } } } pub struct ConfigEngine<'s> { - files: crate::config::Walk, - check_filenames: bool, - check_files: bool, - binary: bool, - tokenizer: typos::tokens::Tokenizer, - dict: crate::dict::Override<'s, 's, crate::dict::BuiltIn>, + storage: &'s ConfigStorage, + + overrides: Option, + custom: Option, + isolated: bool, + + configs: std::collections::HashMap, + files: Intern, + tokenizer: Intern, + dict: Intern>, } impl<'s> ConfigEngine<'s> { - pub fn new(config: crate::config::Config, storage: &'s ConfigStorage) -> Self { + pub fn new(storage: &'s ConfigStorage) -> Self { + Self { + storage, + overrides: Default::default(), + custom: Default::default(), + configs: Default::default(), + isolated: false, + files: Default::default(), + tokenizer: Default::default(), + dict: Default::default(), + } + } + + pub fn set_overrides(&mut self, overrides: crate::config::EngineConfig) -> &mut Self { + self.overrides = Some(overrides); + self + } + + pub fn set_custom_config(&mut self, custom: crate::config::Config) -> &mut Self { + self.custom = Some(custom); + self + } + + pub fn set_isolated(&mut self, isolated: bool) -> &mut Self { + self.isolated = isolated; + self + } + + pub fn files(&mut self, cwd: &std::path::Path) -> &crate::config::Walk { + let dir = self + .configs + .get(cwd) + .expect("`init_dir` must be called first"); + self.get_files(dir) + } + + pub fn policy(&self, path: &std::path::Path) -> Policy<'_, '_> { + let dir = self + .get_dir(path) + .expect("`files()` should be called first"); + Policy { + check_filenames: dir.check_filenames, + check_files: dir.check_files, + binary: dir.binary, + tokenizer: self.get_tokenizer(dir), + dict: self.get_dict(dir), + } + } + + fn get_files(&self, dir: &DirConfig) -> &crate::config::Walk { + self.files.get(dir.files) + } + + fn get_tokenizer(&self, dir: &DirConfig) -> &typos::tokens::Tokenizer { + self.tokenizer.get(dir.tokenizer) + } + + fn get_dict(&self, dir: &DirConfig) -> &dyn typos::Dictionary { + self.dict.get(dir.dict) + } + + fn get_dir(&self, path: &std::path::Path) -> Option<&DirConfig> { + for path in path.ancestors() { + if let Some(dir) = self.configs.get(path) { + return Some(dir); + } + } + None + } + + pub fn load_config( + &self, + cwd: &std::path::Path, + ) -> Result { + let mut config = crate::config::Config::default(); + + if !self.isolated { + if let Some(derived) = crate::config::Config::from_dir(cwd)? { + config.update(&derived); + } + } + if let Some(custom) = self.custom.as_ref() { + config.update(custom); + } + if let Some(overrides) = self.overrides.as_ref() { + config.default.update(overrides); + } + + Ok(config) + } + + pub fn init_dir(&mut self, cwd: &std::path::Path) -> Result<(), anyhow::Error> { + if self.configs.contains_key(cwd) { + return Ok(()); + } + + let config = self.load_config(cwd)?; + let crate::config::Config { files, default } = config; let binary = default.binary(); let check_filename = default.check_filename(); @@ -49,39 +161,69 @@ impl<'s> ConfigEngine<'s> { dict.identifiers( dict_config .extend_identifiers() - .map(|(k, v)| (storage.get(k), storage.get(v))), + .map(|(k, v)| (self.storage.get(k), self.storage.get(v))), ); dict.words( dict_config .extend_words() - .map(|(k, v)| (storage.get(k), storage.get(v))), + .map(|(k, v)| (self.storage.get(k), self.storage.get(v))), ); - Self { + let dict = self.dict.intern(dict); + let files = self.files.intern(files); + let tokenizer = self.tokenizer.intern(tokenizer); + + let dir = DirConfig { files, check_filenames: check_filename, check_files: check_file, binary: binary, tokenizer, dict, + }; + + self.configs.insert(cwd.to_owned(), dir); + Ok(()) + } +} + +struct Intern { + data: Vec, +} + +impl Intern { + pub fn new() -> Self { + Self { + data: Default::default(), } } - pub fn files(&self) -> &crate::config::Walk { - &self.files + pub fn intern(&mut self, value: T) -> usize { + let symbol = self.data.len(); + self.data.push(value); + symbol } - pub fn policy(&self) -> Policy<'_, '_> { - Policy { - check_filenames: self.check_filenames, - check_files: self.check_files, - binary: self.binary, - tokenizer: &self.tokenizer, - dict: &self.dict, - } + pub fn get(&self, symbol: usize) -> &T { + &self.data[symbol] } } +impl Default for Intern { + fn default() -> Self { + Self::new() + } +} + +struct DirConfig { + files: usize, + tokenizer: usize, + dict: usize, + check_filenames: bool, + check_files: bool, + binary: bool, +} + #[non_exhaustive] #[derive(derive_setters::Setters)] pub struct Policy<'t, 'd> {