diff --git a/Cargo.toml b/Cargo.toml index e4469123a..587e70d61 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,4 +1,5 @@ [workspace] +resolver = "1" members = [ # library projects "lsp-positions", diff --git a/stack-graphs/Cargo.toml b/stack-graphs/Cargo.toml index e283e25f6..2e55e1768 100644 --- a/stack-graphs/Cargo.toml +++ b/stack-graphs/Cargo.toml @@ -28,7 +28,7 @@ bincode = { version = "2.0.0-rc.3", optional = true } bitvec = "1.0" controlled-option = "0.4" either = "1.6" -enumset = "1.0" +enumset = "1.1" fxhash = "0.2" itertools = "0.10" libc = "0.2" diff --git a/tree-sitter-stack-graphs/CHANGELOG.md b/tree-sitter-stack-graphs/CHANGELOG.md index d4e1d197f..8d8ef7ca1 100644 --- a/tree-sitter-stack-graphs/CHANGELOG.md +++ b/tree-sitter-stack-graphs/CHANGELOG.md @@ -5,6 +5,14 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Library + +#### Changed + +- A new `Reporter` trait is used to support reporting status from CLI actions such as indexing and testing. The CLI actions have been cleaned up to ensure that they are not writing directly to the console anymore, but only call the reporter for output. The `Reporter` trait replaces the old inaccessible `Logger` trait so that clients can more easily implement their own reporters if necessary. A `ConsoleLogger` is provided for clients who just need console printing. + ## v0.7.1 -- 2023-07-27 Support `stack-graphs` version `0.12`. diff --git a/tree-sitter-stack-graphs/Cargo.toml b/tree-sitter-stack-graphs/Cargo.toml index 7b5abd92b..2d6dabbd1 100644 --- a/tree-sitter-stack-graphs/Cargo.toml +++ b/tree-sitter-stack-graphs/Cargo.toml @@ -26,8 +26,8 @@ cli = [ "base64", "clap", "colored", - "dirs", "dialoguer", + "dirs", "env_logger", "indoc", "pathdiff", @@ -52,12 +52,12 @@ lsp = [ anyhow = "1.0" base64 = { version = "0.21", optional = true } capture-it = { version = "0.3", optional = true } -clap = { version = "4", optional = true, features=["derive"] } +clap = { version = "4", optional = true, features = ["derive"] } colored = { version = "2.0", optional = true } controlled-option = ">=0.4" crossbeam-channel = { version = "0.5", optional = true } dialoguer = { version = "0.10", optional = true } -dirs = { version = "5", optional=true } +dirs = { version = "5", optional = true } env_logger = { version = "0.9", optional = true } indoc = { version = "1.0", optional = true } itertools = "0.10" diff --git a/tree-sitter-stack-graphs/src/cli.rs b/tree-sitter-stack-graphs/src/cli.rs index 859cbb5c2..719ce8f18 100644 --- a/tree-sitter-stack-graphs/src/cli.rs +++ b/tree-sitter-stack-graphs/src/cli.rs @@ -69,7 +69,7 @@ pub mod parse; pub mod query; pub mod status; pub mod test; -mod util; +pub mod util; pub mod visualize; pub mod path_loading { @@ -78,6 +78,7 @@ pub mod path_loading { use clap::Subcommand; use crate::cli::clean::CleanArgs; + use crate::cli::database::DatabaseArgs; use crate::cli::index::IndexArgs; use crate::cli::init::InitArgs; use crate::cli::load::PathLoaderArgs; @@ -90,8 +91,6 @@ pub mod path_loading { use crate::cli::test::TestArgs; use crate::cli::visualize::VisualizeArgs; - use super::database::DatabaseArgs; - #[derive(Subcommand)] pub enum Subcommands { Clean(Clean), @@ -297,6 +296,7 @@ pub mod provided_languages { use clap::Subcommand; use crate::cli::clean::CleanArgs; + use crate::cli::database::DatabaseArgs; use crate::cli::index::IndexArgs; use crate::cli::init::InitArgs; use crate::cli::load::LanguageConfigurationsLoaderArgs; @@ -310,8 +310,6 @@ pub mod provided_languages { use crate::cli::visualize::VisualizeArgs; use crate::loader::LanguageConfiguration; - use super::database::DatabaseArgs; - #[derive(Subcommand)] pub enum Subcommands { Clean(Clean), diff --git a/tree-sitter-stack-graphs/src/cli/index.rs b/tree-sitter-stack-graphs/src/cli/index.rs index 633866379..a90056f4b 100644 --- a/tree-sitter-stack-graphs/src/cli/index.rs +++ b/tree-sitter-stack-graphs/src/cli/index.rs @@ -21,6 +21,16 @@ use std::time::Duration; use thiserror::Error; use tree_sitter_graph::Variables; +use crate::cli::util::duration_from_seconds_str; +use crate::cli::util::iter_files_and_directories; +use crate::cli::util::reporter::ConsoleReporter; +use crate::cli::util::reporter::Level; +use crate::cli::util::reporter::Reporter; +use crate::cli::util::sha1; +use crate::cli::util::wait_for_input; +use crate::cli::util::BuildErrorWithSource; +use crate::cli::util::CLIFileReporter; +use crate::cli::util::ExistingPathBufValueParser; use crate::loader::FileLanguageConfigurations; use crate::loader::FileReader; use crate::loader::Loader; @@ -29,16 +39,6 @@ use crate::CancelAfterDuration; use crate::CancellationFlag; use crate::NoCancellation; -use super::util::duration_from_seconds_str; -use super::util::iter_files_and_directories; -use super::util::sha1; -use super::util::wait_for_input; -use super::util::BuildErrorWithSource; -use super::util::ConsoleLogger; -use super::util::ExistingPathBufValueParser; -use super::util::FileLogger; -use super::util::Logger; - #[derive(Args)] pub struct IndexArgs { /// Source file or directory paths to index. @@ -101,8 +101,8 @@ impl IndexArgs { wait_for_input()?; } let mut db = SQLiteWriter::open(&db_path)?; - let logger = ConsoleLogger::new(self.verbose, !self.hide_error_details); - let mut indexer = Indexer::new(&mut db, &mut loader, &logger); + let reporter = self.get_reporter(); + let mut indexer = Indexer::new(&mut db, &mut loader, &reporter); indexer.force = self.force; indexer.max_file_time = self.max_file_time; @@ -114,12 +114,37 @@ impl IndexArgs { indexer.index_all(source_paths, self.continue_from, &NoCancellation)?; Ok(()) } + + fn get_reporter(&self) -> ConsoleReporter { + return ConsoleReporter { + skipped_level: if self.verbose { + Level::Summary + } else { + Level::None + }, + succeeded_level: if self.verbose { + Level::Summary + } else { + Level::None + }, + failed_level: if self.hide_error_details { + Level::Summary + } else { + Level::Details + }, + canceled_level: if self.hide_error_details { + Level::Summary + } else { + Level::Details + }, + }; + } } pub struct Indexer<'a> { db: &'a mut SQLiteWriter, loader: &'a mut Loader, - logger: &'a dyn Logger, + reporter: &'a dyn Reporter, /// Index files, even if they already exist in the database. pub force: bool, /// Maximum time per file. @@ -127,11 +152,15 @@ pub struct Indexer<'a> { } impl<'a> Indexer<'a> { - pub fn new(db: &'a mut SQLiteWriter, loader: &'a mut Loader, logger: &'a dyn Logger) -> Self { + pub fn new( + db: &'a mut SQLiteWriter, + loader: &'a mut Loader, + reporter: &'a dyn Reporter, + ) -> Self { Self { db, loader, - logger, + reporter, force: false, max_file_time: None, } @@ -149,6 +178,7 @@ impl<'a> Indexer<'a> { Q: AsRef, { for (source_root, source_path, strict) in iter_files_and_directories(source_paths) { + let mut file_status = CLIFileReporter::new(self.reporter, &source_path); cancellation_flag.check("indexing all files")?; self.index_file( &source_root, @@ -156,7 +186,9 @@ impl<'a> Indexer<'a> { strict, &mut continue_from, cancellation_flag, + &mut file_status, )?; + file_status.assert_reported(); } Ok(()) } @@ -167,13 +199,16 @@ impl<'a> Indexer<'a> { source_path: &Path, cancellation_flag: &dyn CancellationFlag, ) -> Result<()> { + let mut file_status = CLIFileReporter::new(self.reporter, source_path); self.index_file( &source_root, &source_path, true, &mut None::<&Path>, cancellation_flag, + &mut file_status, )?; + file_status.assert_reported(); Ok(()) } @@ -185,22 +220,25 @@ impl<'a> Indexer<'a> { missing_is_error: bool, continue_from: &mut Option

, cancellation_flag: &dyn CancellationFlag, + file_status: &mut CLIFileReporter, ) -> Result<()> where P: AsRef, { - let mut file_status = self.logger.file(source_path); match self.index_file_inner( source_root, source_path, missing_is_error, continue_from, cancellation_flag, - file_status.as_mut(), + file_status, ) { - ok @ Ok(_) => ok, + ok @ Ok(_) => { + file_status.assert_reported(); + ok + } err @ Err(_) => { - file_status.default_failure("error", Some(&format!("Error analyzing file {}. To continue analysis from this file later, add: --continue-from {}", source_path.display(), source_path.display()))); + file_status.failure_if_processing("error", Some(&format!("Error analyzing file {}. To continue analysis from this file later, add: --continue-from {}", source_path.display(), source_path.display()))); err } } @@ -213,7 +251,7 @@ impl<'a> Indexer<'a> { missing_is_error: bool, continue_from: &mut Option

, cancellation_flag: &dyn CancellationFlag, - file_status: &mut dyn FileLogger, + file_status: &mut CLIFileReporter<'_>, ) -> Result<()> where P: AsRef, @@ -245,22 +283,28 @@ impl<'a> Indexer<'a> { let source = file_reader.get(source_path)?; let tag = sha1(source); - if !self.force { - match self - .db - .status_for_file(&source_path.to_string_lossy(), Some(&tag))? - { - FileStatus::Missing => {} - FileStatus::Indexed => { + let success_status = match self + .db + .status_for_file(&source_path.to_string_lossy(), Some(&tag))? + { + FileStatus::Missing => "indexed", + FileStatus::Indexed => { + if self.force { + "reindexed" + } else { file_status.skipped("cached index", None); return Ok(()); } - FileStatus::Error(error) => { + } + FileStatus::Error(error) => { + if self.force { + "reindexed" + } else { file_status.skipped(&format!("cached error ({})", error), None); return Ok(()); } } - } + }; let file_cancellation_flag = CancelAfterDuration::from_option(self.max_file_time); let cancellation_flag = cancellation_flag | file_cancellation_flag.as_ref(); @@ -331,7 +375,7 @@ impl<'a> Indexer<'a> { self.db .store_result_for_file(&graph, file, &tag, &mut partials, &paths)?; - file_status.success("success", None); + file_status.success(success_status, None); Ok(()) } diff --git a/tree-sitter-stack-graphs/src/cli/lsp.rs b/tree-sitter-stack-graphs/src/cli/lsp.rs index 3b80ff420..03f1c8060 100644 --- a/tree-sitter-stack-graphs/src/cli/lsp.rs +++ b/tree-sitter-stack-graphs/src/cli/lsp.rs @@ -28,21 +28,19 @@ use tower_lsp::LanguageServer; use tower_lsp::LspService; use tower_lsp::Server; +use crate::cli::index::Indexer; +use crate::cli::query::Querier; +use crate::cli::query::QueryError; +use crate::cli::util::duration_from_milliseconds_str; +use crate::cli::util::duration_from_seconds_str; +use crate::cli::util::reporter::Reporter; +use crate::cli::util::SourcePosition; +use crate::cli::util::SourceSpan; use crate::loader::Loader; use crate::AtomicCancellationFlag; use crate::CancelAfterDuration; use crate::CancellationFlag; -use super::index::Indexer; -use super::query::Querier; -use super::query::QueryError; -use super::util::duration_from_milliseconds_str; -use super::util::duration_from_seconds_str; -use super::util::FileLogger; -use super::util::Logger; -use super::util::SourcePosition; -use super::util::SourceSpan; - #[derive(Args, Clone)] pub struct LspArgs { /// Maximum index runtime per workspace folder in seconds. @@ -208,14 +206,14 @@ impl Backend { } }; - let logger = LspLogger { + let reporter = LspReporter { handle: handle.clone(), logger: self.logger.clone(), }; let folder_cancellation_flag = CancelAfterDuration::from_option(self.args.max_folder_index_time); let cancellation_flag = cancellation_flag | folder_cancellation_flag.as_ref(); - let mut indexer = Indexer::new(&mut db, &mut loader, &logger); + let mut indexer = Indexer::new(&mut db, &mut loader, &reporter); indexer.max_file_time = self.args.max_file_index_time; let result = indexer.index_all(vec![path], None::<&Path>, &cancellation_flag); @@ -283,12 +281,12 @@ impl Backend { }; let handle = Handle::current(); - let logger = LspLogger { + let reporter = LspReporter { handle: handle.clone(), logger: self.logger.clone(), }; let result = { - let mut querier = Querier::new(&mut db, &logger); + let mut querier = Querier::new(&mut db, &reporter); let cancellation_flag = CancelAfterDuration::from_option(self.args.max_query_time); querier.definitions(reference, cancellation_flag.as_ref()) }; @@ -530,16 +528,16 @@ struct BackendLogger { } impl BackendLogger { - async fn info(&self, message: M) { - self.client.log_message(MessageType::INFO, message).await + async fn log(&self, level: MessageType, message: M) { + self.client.log_message(level, message).await } - async fn warning(&self, message: M) { - self.client.log_message(MessageType::WARNING, message).await + async fn info(&self, message: M) { + self.log(MessageType::INFO, message).await } async fn error(&self, message: M) { - self.client.log_message(MessageType::ERROR, message).await + self.log(MessageType::ERROR, message).await } } @@ -596,90 +594,43 @@ impl Job { } } -struct LspLogger { - handle: Handle, - logger: BackendLogger, -} -struct LspFileLogger<'a> { - path: &'a Path, +struct LspReporter { handle: Handle, logger: BackendLogger, } -impl Logger for LspLogger { - fn file<'a>(&self, path: &'a Path) -> Box { - Box::new(LspFileLogger { - path, - handle: self.handle.clone(), - logger: self.logger.clone(), - }) - } -} - -impl FileLogger for LspFileLogger<'_> { - fn default_failure(&mut self, status: &str, _details: Option<&dyn std::fmt::Display>) { +impl LspReporter { + fn report(&self, level: MessageType, path: &Path, status: &str) { let logger = self.logger.clone(); - let path = self.path.to_owned(); + let path = path.to_owned(); let status = status.to_owned(); self.handle.spawn(async move { logger - .error(format!("{}: {}", path.display(), status)) + .log(level, format!("{}: {}", path.display(), status)) .await; }); } +} - fn failure(&mut self, status: &str, _details: Option<&dyn std::fmt::Display>) { - let logger = self.logger.clone(); - let path = self.path.to_owned(); - let status = status.to_owned(); - self.handle.spawn(async move { - logger - .error(format!("{}: {}", path.display(), status)) - .await; - }); +impl Reporter for LspReporter { + fn skipped(&self, path: &Path, summary: &str, _details: Option<&dyn std::fmt::Display>) { + self.report(MessageType::INFO, path, summary) } - fn skipped(&mut self, status: &str, _details: Option<&dyn std::fmt::Display>) { - let logger = self.logger.clone(); - let path = self.path.to_owned(); - let status = status.to_owned(); - self.handle.spawn(async move { - logger - .info(format!("{}: skipped: {}", path.display(), status)) - .await; - }); + fn started(&self, path: &Path) { + self.report(MessageType::INFO, path, "started") } - fn warning(&mut self, status: &str, _details: Option<&dyn std::fmt::Display>) { - let logger = self.logger.clone(); - let path = self.path.to_owned(); - let status = status.to_owned(); - self.handle.spawn(async move { - logger - .warning(format!("{}: {}", path.display(), status)) - .await; - }); + fn succeeded(&self, path: &Path, summary: &str, _details: Option<&dyn std::fmt::Display>) { + self.report(MessageType::INFO, path, summary) } - fn success(&mut self, status: &str, _details: Option<&dyn std::fmt::Display>) { - let logger = self.logger.clone(); - let path = self.path.to_owned(); - let status = status.to_owned(); - self.handle.spawn(async move { - logger - .info(format!("{}: success: {}", path.display(), status)) - .await; - }); + fn failed(&self, path: &Path, summary: &str, _details: Option<&dyn std::fmt::Display>) { + self.report(MessageType::ERROR, path, summary) } - fn processing(&mut self) { - let logger = self.logger.clone(); - let path = self.path.to_owned(); - self.handle.spawn(async move { - logger - .info(format!("{}: processing...", path.display())) - .await; - }); + fn cancelled(&self, path: &Path, summary: &str, _details: Option<&dyn std::fmt::Display>) { + self.report(MessageType::WARNING, path, summary) } } diff --git a/tree-sitter-stack-graphs/src/cli/parse.rs b/tree-sitter-stack-graphs/src/cli/parse.rs index b4cc688f8..aff7681b1 100644 --- a/tree-sitter-stack-graphs/src/cli/parse.rs +++ b/tree-sitter-stack-graphs/src/cli/parse.rs @@ -13,13 +13,12 @@ use std::path::PathBuf; use tree_sitter::Parser; use tree_sitter_graph::parse_error::ParseError; +use crate::cli::util::ExistingPathBufValueParser; use crate::loader::FileReader; use crate::loader::Loader; use crate::util::DisplayParseErrorsPretty; use crate::BuildError; -use super::util::ExistingPathBufValueParser; - #[derive(Args)] pub struct ParseArgs { /// Source file path to parse. diff --git a/tree-sitter-stack-graphs/src/cli/query.rs b/tree-sitter-stack-graphs/src/cli/query.rs index f0256bc67..583aee85b 100644 --- a/tree-sitter-stack-graphs/src/cli/query.rs +++ b/tree-sitter-stack-graphs/src/cli/query.rs @@ -16,17 +16,16 @@ use std::path::PathBuf; use thiserror::Error; use tree_sitter_graph::parse_error::Excerpt; +use crate::cli::util::reporter::ConsoleReporter; +use crate::cli::util::reporter::Reporter; +use crate::cli::util::sha1; +use crate::cli::util::wait_for_input; +use crate::cli::util::SourcePosition; +use crate::cli::util::SourceSpan; use crate::loader::FileReader; use crate::CancellationFlag; use crate::NoCancellation; -use super::util::sha1; -use super::util::wait_for_input; -use super::util::ConsoleLogger; -use super::util::Logger; -use super::util::SourcePosition; -use super::util::SourceSpan; - #[derive(Args)] pub struct QueryArgs { /// Wait for user input before starting analysis. Useful for profiling. @@ -54,8 +53,8 @@ pub enum Target { impl Target { pub fn run(self, db: &mut SQLiteReader) -> anyhow::Result<()> { - let logger = ConsoleLogger::new(true, true); - let mut querier = Querier::new(db, &logger); + let reporter = ConsoleReporter::details(); + let mut querier = Querier::new(db, &reporter); match self { Self::Definition(cmd) => cmd.run(&mut querier), } @@ -135,12 +134,12 @@ impl Definition { pub struct Querier<'a> { db: &'a mut SQLiteReader, - logger: &'a dyn Logger, + reporter: &'a dyn Reporter, } impl<'a> Querier<'a> { - pub fn new(db: &'a mut SQLiteReader, logger: &'a dyn Logger) -> Self { - Self { db, logger } + pub fn new(db: &'a mut SQLiteReader, reporter: &'a dyn Reporter) -> Self { + Self { db, reporter } } pub fn definitions( @@ -149,7 +148,6 @@ impl<'a> Querier<'a> { cancellation_flag: &dyn CancellationFlag, ) -> Result> { let log_path = PathBuf::from(reference.to_string()); - let mut logger = self.logger.file(&log_path); let mut file_reader = FileReader::new(); let tag = file_reader.get(&reference.path).ok().map(sha1); @@ -159,12 +157,13 @@ impl<'a> Querier<'a> { { FileStatus::Indexed => {} _ => { - logger.failure("file not indexed", None); + self.reporter.started(&log_path); + self.reporter.failed(&log_path, "file not indexed", None); return Ok(Vec::default()); } } - logger.processing(); + self.reporter.started(&log_path); self.db .load_graph_for_file(&reference.path.to_string_lossy())?; @@ -172,7 +171,8 @@ impl<'a> Querier<'a> { let starting_nodes = reference.iter_references(graph).collect::>(); if starting_nodes.is_empty() { - logger.warning("no references at location", None); + self.reporter + .cancelled(&log_path, "no references at location", None); return Ok(Vec::default()); } @@ -191,7 +191,7 @@ impl<'a> Querier<'a> { reference_paths.push(p.clone()); }, ) { - logger.failure("query timed out", None); + self.reporter.failed(&log_path, "query timed out", None); return Err(err.into()); } @@ -199,7 +199,7 @@ impl<'a> Querier<'a> { let mut actual_paths = Vec::new(); for reference_path in &reference_paths { if let Err(err) = cancellation_flag.check("shadowing") { - logger.failure("query timed out", None); + self.reporter.failed(&log_path, "query timed out", None); return Err(err.into()); } if reference_paths @@ -232,7 +232,8 @@ impl<'a> Querier<'a> { } let count: usize = result.iter().map(|r| r.targets.len()).sum(); - logger.success( + self.reporter.succeeded( + &log_path, &format!( "found {} definitions for {} references", count, diff --git a/tree-sitter-stack-graphs/src/cli/status.rs b/tree-sitter-stack-graphs/src/cli/status.rs index d877d29bc..847837b80 100644 --- a/tree-sitter-stack-graphs/src/cli/status.rs +++ b/tree-sitter-stack-graphs/src/cli/status.rs @@ -14,8 +14,8 @@ use stack_graphs::storage::SQLiteReader; use std::path::Path; use std::path::PathBuf; -use super::util::ConsoleFileLogger; -use super::util::FileLogger; +use crate::cli::util::reporter::ConsoleReporter; +use crate::cli::util::reporter::Reporter; #[derive(Args)] #[clap(group( @@ -41,38 +41,48 @@ pub struct StatusArgs { impl StatusArgs { pub fn run(self, db_path: &Path) -> anyhow::Result<()> { + let reporter = self.get_reporter(); let mut db = SQLiteReader::open(&db_path)?; if self.all { let mut files = db.list_all()?; let mut entries = files.try_iter()?; - self.status(&mut entries)?; + self.status(&mut entries, &reporter)?; } else { for source_path in &self.source_paths { let source_path = source_path.canonicalize()?; let mut files = db.list_file_or_directory(&source_path)?; let mut entries = files.try_iter()?; - self.status(&mut entries)?; + self.status(&mut entries, &reporter)?; } } Ok(()) } + fn get_reporter(&self) -> ConsoleReporter { + if self.verbose { + ConsoleReporter::details() + } else { + ConsoleReporter::summary() + } + } + fn status( &self, entries: &mut impl Iterator>, + reporter: &dyn Reporter, ) -> anyhow::Result<()> { for entry in entries { let entry = entry?; - let mut logger = ConsoleFileLogger::new(&Path::new(&entry.path), true, self.verbose); + reporter.started(&entry.path); match &entry.status { FileStatus::Missing => { - logger.skipped("missing", None); + reporter.cancelled(&entry.path, "missing", None); } FileStatus::Indexed => { - logger.success("indexed", None); + reporter.succeeded(&entry.path, "indexed", None); } FileStatus::Error(error) => { - logger.failure("failed", Some(error)); + reporter.failed(&entry.path, "failed", Some(error)); } } } diff --git a/tree-sitter-stack-graphs/src/cli/test.rs b/tree-sitter-stack-graphs/src/cli/test.rs index 195f9a532..9566687f1 100644 --- a/tree-sitter-stack-graphs/src/cli/test.rs +++ b/tree-sitter-stack-graphs/src/cli/test.rs @@ -24,9 +24,10 @@ use tree_sitter_graph::Variables; use crate::cli::util::duration_from_seconds_str; use crate::cli::util::iter_files_and_directories; -use crate::cli::util::ConsoleFileLogger; +use crate::cli::util::reporter::ConsoleReporter; +use crate::cli::util::reporter::Level; +use crate::cli::util::CLIFileReporter; use crate::cli::util::ExistingPathBufValueParser; -use crate::cli::util::FileLogger; use crate::cli::util::PathSpec; use crate::loader::ContentProvider; use crate::loader::FileReader; @@ -171,9 +172,13 @@ impl TestArgs { } pub fn run(self, mut loader: Loader) -> anyhow::Result<()> { + let reporter = self.get_reporter(); let mut total_result = TestResult::new(); for (test_root, test_path, _) in iter_files_and_directories(self.test_paths.clone()) { - let test_result = self.run_test(&test_root, &test_path, &mut loader)?; + let mut file_status = CLIFileReporter::new(&reporter, &test_path); + let test_result = + self.run_test(&test_root, &test_path, &mut loader, &mut file_status)?; + file_status.assert_reported(); total_result.absorb(test_result); } if total_result.failure_count() > 0 { @@ -182,19 +187,39 @@ impl TestArgs { Ok(()) } + fn get_reporter(&self) -> ConsoleReporter { + return ConsoleReporter { + skipped_level: if self.show_skipped { + Level::Summary + } else { + Level::None + }, + succeeded_level: if self.quiet { + Level::None + } else { + Level::Summary + }, + failed_level: if self.hide_error_details { + Level::Summary + } else { + Level::Details + }, + canceled_level: Level::Details, // tester doesn't report canceled + }; + } + /// Run test file. Takes care of the output when an error is returned. fn run_test( &self, test_root: &Path, test_path: &Path, loader: &mut Loader, + file_status: &mut CLIFileReporter, ) -> anyhow::Result { - let mut file_status = - ConsoleFileLogger::new(test_path, !self.quiet, !self.hide_error_details); - match self.run_test_inner(test_root, test_path, loader, &mut file_status) { + match self.run_test_inner(test_root, test_path, loader, file_status) { ok @ Ok(_) => ok, err @ Err(_) => { - file_status.default_failure("error", None); + file_status.failure_if_processing("error", None); err } } @@ -205,7 +230,7 @@ impl TestArgs { test_root: &Path, test_path: &Path, loader: &mut Loader, - file_status: &mut ConsoleFileLogger, + file_status: &mut CLIFileReporter, ) -> anyhow::Result { let cancellation_flag = CancelAfterDuration::from_option(self.max_test_time); @@ -230,9 +255,7 @@ impl TestArgs { .map_or(false, |e| e == "skip"), _ => false, }) { - if self.show_skipped { - file_status.warning("skipped", None); - } + file_status.skipped("skipped", None); return Ok(TestResult::new()); } @@ -314,8 +337,8 @@ impl TestArgs { )?; } let result = test.run(&mut partials, &mut db, cancellation_flag.as_ref())?; - let success = self.handle_result(&result, file_status)?; - if self.output_mode.test(!success) { + let success = result.failure_count() == 0; + let outputs = if self.output_mode.test(!success) { let files = test.fragments.iter().map(|f| f.file).collect::>(); self.save_output( test_root, @@ -326,8 +349,30 @@ impl TestArgs { &|_: &StackGraph, h: &Handle| files.contains(h), success, cancellation_flag.as_ref(), - )?; + )? + } else { + Vec::default() + }; + + if success { + let details = outputs.join("\n"); + file_status.success("success", Some(&details)); + } else { + let details = result + .failures_iter() + .map(|f| f.to_string()) + .chain(outputs) + .join("\n"); + file_status.failure( + &format!( + "{}/{} assertions failed", + result.failure_count(), + result.count(), + ), + Some(&details), + ); } + Ok(result) } @@ -342,27 +387,6 @@ impl TestArgs { Ok(()) } - fn handle_result( - &self, - result: &TestResult, - file_status: &mut ConsoleFileLogger, - ) -> anyhow::Result { - let success = result.failure_count() == 0; - if success { - file_status.success("success", None); - } else { - file_status.failure( - &format!( - "{}/{} assertions failed", - result.failure_count(), - result.count(), - ), - Some(&result.failures_iter().join("\n")), - ); - } - Ok(success) - } - fn save_output( &self, test_root: &Path, @@ -373,7 +397,8 @@ impl TestArgs { filter: &dyn Filter, success: bool, cancellation_flag: &dyn CancellationFlag, - ) -> anyhow::Result<()> { + ) -> anyhow::Result> { + let mut outputs = Vec::with_capacity(3); let save_graph = self .save_graph .as_ref() @@ -390,7 +415,11 @@ impl TestArgs { if let Some(path) = save_graph { self.save_graph(&path, &graph, filter)?; if !success || !self.quiet { - println!("{}: graph at {}", test_path.display(), path.display()); + outputs.push(format!( + "{}: graph at {}", + test_path.display(), + path.display() + )); } } @@ -403,21 +432,25 @@ impl TestArgs { if let Some(path) = save_paths { self.save_paths(&path, graph, partials, &mut db, filter)?; if !success || !self.quiet { - println!("{}: paths at {}", test_path.display(), path.display()); + outputs.push(format!( + "{}: paths at {}", + test_path.display(), + path.display() + )); } } if let Some(path) = save_visualization { self.save_visualization(&path, graph, partials, &mut db, filter, &test_path)?; if !success || !self.quiet { - println!( + outputs.push(format!( "{}: visualization at {}", test_path.display(), path.display() - ); + )); } } - Ok(()) + Ok(outputs) } fn save_graph( diff --git a/tree-sitter-stack-graphs/src/cli/util.rs b/tree-sitter-stack-graphs/src/cli/util.rs index e222fe434..66181a1fd 100644 --- a/tree-sitter-stack-graphs/src/cli/util.rs +++ b/tree-sitter-stack-graphs/src/cli/util.rs @@ -12,7 +12,6 @@ use clap::builder::TypedValueParser; use clap::error::ContextKind; use clap::error::ContextValue; use clap::error::ErrorKind; -use colored::Colorize; use lsp_positions::Span; use sha1::Digest; use sha1::Sha1; @@ -26,10 +25,12 @@ use std::ops::Range; use std::path::Path; use std::path::PathBuf; use std::time::Duration; -#[cfg(debug_assertions)] -use std::time::Instant; use walkdir::WalkDir; +use self::reporter::Reporter; + +pub mod reporter; + #[derive(Clone)] pub(crate) struct ExistingPathBufValueParser; @@ -297,13 +298,13 @@ pub struct SourceSpan { } impl SourceSpan { - pub fn first_line(&self) -> usize { + pub(crate) fn first_line(&self) -> usize { self.span.start.line } /// Returns a range for the first line of this span. If multiple lines are spanned, it /// will use usize::MAX for the range's end. - pub fn first_line_column_range(&self) -> Range { + pub(crate) fn first_line_column_range(&self) -> Range { let start = self.span.start.column.grapheme_offset; let end = if self.span.start.line == self.span.end.line { self.span.end.column.grapheme_offset @@ -314,13 +315,13 @@ impl SourceSpan { } } -pub fn duration_from_seconds_str(s: &str) -> Result { +pub(crate) fn duration_from_seconds_str(s: &str) -> Result { let seconds = s.parse::()?; Ok(Duration::new(seconds, 0)) } #[cfg(feature = "lsp")] -pub fn duration_from_milliseconds_str(s: &str) -> Result { +pub(crate) fn duration_from_milliseconds_str(s: &str) -> Result { let milliseconds = s.parse::()?; let seconds = milliseconds / 1000; let nano_seconds = (milliseconds % 1000) as u32 * 1_000_000; @@ -364,182 +365,98 @@ where .flatten() } -pub trait Logger { - fn file<'a>(&self, path: &'a Path) -> Box; -} - -pub trait FileLogger { - fn processing(&mut self) {} - fn failure(&mut self, _status: &str, _details: Option<&dyn std::fmt::Display>) {} - fn skipped(&mut self, _status: &str, _details: Option<&dyn std::fmt::Display>) {} - fn success(&mut self, _status: &str, _details: Option<&dyn std::fmt::Display>) {} - fn warning(&mut self, _status: &str, _details: Option<&dyn std::fmt::Display>) {} - fn default_failure(&mut self, _status: &str, _details: Option<&dyn std::fmt::Display>) {} -} - -pub struct ConsoleLogger { - show_info: bool, - show_details: bool, -} - -impl ConsoleLogger { - pub fn new(show_info: bool, show_details: bool) -> Self { - Self { - show_info, - show_details, - } - } -} - -impl Logger for ConsoleLogger { - fn file<'a>(&self, path: &'a Path) -> Box { - Box::new(ConsoleFileLogger::new( - path, - self.show_info, - self.show_details, - )) - } -} - -pub struct ConsoleFileLogger<'a> { +/// Wraps a reporter and ensures that reporter is called properly without requiring +/// the caller of the wrapper to be overly careful about which methods must be called +/// in which order +pub(super) struct CLIFileReporter<'a> { + reporter: &'a dyn Reporter, path: &'a Path, - show_info: bool, - show_details: bool, path_logged: bool, - #[cfg(debug_assertions)] - processing_started: Option, + status_logged: bool, } -impl<'a> ConsoleFileLogger<'a> { - pub fn new(path: &'a Path, show_info: bool, show_details: bool) -> Self { +impl<'a> CLIFileReporter<'a> { + pub(super) fn new(reporter: &'a dyn Reporter, path: &'a Path) -> Self { Self { + reporter, path, - show_info, - show_details, path_logged: false, - #[cfg(debug_assertions)] - processing_started: None, + status_logged: false, } } - fn print_path(&mut self) { + pub(super) fn processing(&mut self) { if self.path_logged { - return; + panic!("Already started or finished"); } - print!("{}: ", self.path.display()); + self.reporter.started(self.path); self.path_logged = true; } - #[cfg(debug_assertions)] - fn print_processing_time(&mut self) { - if let Some(processing_started) = self.processing_started { - print!(" [{:.2} s]", processing_started.elapsed().as_secs_f64()); - } - } - - fn flush(&mut self) { - std::io::stdout().flush().expect("flush should succeed"); - } -} - -impl FileLogger for ConsoleFileLogger<'_> { - fn processing(&mut self) { - #[cfg(debug_assertions)] - { - self.processing_started = Some(Instant::now()); + fn ensure_started(&mut self) { + if self.status_logged { + panic!("Status already logged"); } - if !self.show_info { - return; + if !self.path_logged { + self.reporter.started(self.path); + self.path_logged = true; } - self.print_path(); - self.flush(); } - fn success(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { - if !self.show_info { - return; - } - self.print_path(); - print!("{}", status.green()); - #[cfg(debug_assertions)] - self.print_processing_time(); - println!(); - self.path_logged = false; - self.flush(); - if !self.show_details { - return; - } - if let Some(details) = details { - println!("{}", details); - } + pub(super) fn success(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { + self.ensure_started(); + self.reporter.succeeded(self.path, status, details); + self.status_logged = true; } - fn skipped(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { - if !self.show_info { - return; - } - self.print_path(); - print!("{}", status.dimmed()); - #[cfg(debug_assertions)] - self.print_processing_time(); - println!(); - self.path_logged = false; - self.flush(); - if !self.show_details { - return; + pub(super) fn skipped(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { + if self.path_logged { + panic!("Skipped after starting"); } - if let Some(details) = details { - println!("{}", details); + if self.status_logged { + panic!("Status already logged"); } + self.reporter.skipped(self.path, status, details); + self.status_logged = true; } - fn warning(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { - self.print_path(); - print!("{}", status.yellow()); - #[cfg(debug_assertions)] - self.print_processing_time(); - println!(); - self.path_logged = false; - self.flush(); - if !self.show_details { - return; - } - if let Some(details) = details { - println!("{}", details); - } + pub(super) fn warning(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { + self.ensure_started(); + self.reporter.cancelled(self.path, status, details); + self.status_logged = true; } - fn failure(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { - self.print_path(); - print!("{}", status.red()); - #[cfg(debug_assertions)] - self.print_processing_time(); - println!(); - self.path_logged = false; - self.flush(); - if !self.show_details { - return; - } - if let Some(details) = details { - println!("{}", details); - } + pub(super) fn failure(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { + self.ensure_started(); + self.reporter.failed(self.path, status, details); + self.status_logged = true; } - fn default_failure(&mut self, status: &str, details: Option<&dyn std::fmt::Display>) { + pub(super) fn failure_if_processing( + &mut self, + status: &str, + details: Option<&dyn std::fmt::Display>, + ) { if !self.path_logged { return; } self.failure(status, details); } + + pub(super) fn assert_reported(&mut self) { + if self.path_logged && !self.status_logged { + panic!("status not reported"); + } + } } -pub fn sha1(value: &str) -> String { +pub(crate) fn sha1(value: &str) -> String { let mut hasher = Sha1::new(); hasher.update(value); base64::prelude::BASE64_STANDARD_NO_PAD.encode(hasher.finalize()) } -pub fn wait_for_input() -> anyhow::Result<()> { +pub(crate) fn wait_for_input() -> anyhow::Result<()> { print!(""); std::io::stdout().flush()?; let mut input = String::new(); @@ -548,7 +465,7 @@ pub fn wait_for_input() -> anyhow::Result<()> { } /// Wraps a build error with the relevant sources -pub struct BuildErrorWithSource<'a> { +pub(crate) struct BuildErrorWithSource<'a> { pub inner: crate::BuildError, pub source_path: PathBuf, pub source_str: &'a str, diff --git a/tree-sitter-stack-graphs/src/cli/util/reporter.rs b/tree-sitter-stack-graphs/src/cli/util/reporter.rs new file mode 100644 index 000000000..0dc249a4a --- /dev/null +++ b/tree-sitter-stack-graphs/src/cli/util/reporter.rs @@ -0,0 +1,178 @@ +// -*- coding: utf-8 -*- +// ------------------------------------------------------------------------------------------------ +// Copyright © 2023, stack-graphs authors. +// Licensed under either of Apache License, Version 2.0, or MIT license, at your option. +// Please see the LICENSE-APACHE or LICENSE-MIT files in this distribution for license details. +// ------------------------------------------------------------------------------------------------ + +use colored::ColoredString; +use colored::Colorize; +use std::io::Write; +use std::path::Path; + +/// Trait that supports reporting file processing status. +/// +/// For each file, either +/// - [`skipped`] is called once, or +/// - [`started`] and then one of [`succeeded`], [`failed`], or [`canceled`] are called. +/// +/// Guidance for severity of these statuses: +/// - Failed files should be reported as errors. +/// - Canceled files can be reported as warnings. +/// - Succeeded and skipped files can be reported as info. +pub trait Reporter { + /// File was skipped. + fn skipped(&self, path: &Path, summary: &str, details: Option<&dyn std::fmt::Display>); + + /// File processing started. + fn started(&self, path: &Path); + + /// File was processed and succeeded. + fn succeeded(&self, path: &Path, summary: &str, details: Option<&dyn std::fmt::Display>); + + /// File was processed and failed. + fn failed(&self, path: &Path, summary: &str, details: Option<&dyn std::fmt::Display>); + + /// File could not be processed and was canceled. + fn cancelled(&self, path: &Path, summary: &str, details: Option<&dyn std::fmt::Display>); +} + +/// An enum describing the level of detail that should be reported. +#[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] +pub enum Level { + None, + Summary, + Details, +} + +/// A console reporter that outputs the path when processing starts, and appends +/// the status once finished. +#[derive(Clone, Copy, Debug)] +pub struct ConsoleReporter { + pub skipped_level: Level, + pub succeeded_level: Level, + pub failed_level: Level, + pub canceled_level: Level, +} + +impl ConsoleReporter { + pub fn none() -> Self { + Self { + skipped_level: Level::None, + succeeded_level: Level::None, + failed_level: Level::None, + canceled_level: Level::None, + } + } + + pub fn summary() -> Self { + Self { + skipped_level: Level::Summary, + succeeded_level: Level::Summary, + failed_level: Level::Summary, + canceled_level: Level::Summary, + } + } + + pub fn details() -> Self { + Self { + skipped_level: Level::Details, + succeeded_level: Level::Details, + failed_level: Level::Details, + canceled_level: Level::Details, + } + } + fn all_results_are_reported(&self) -> bool { + *[self.succeeded_level, self.failed_level, self.canceled_level] + .iter() + .min() + .unwrap() + > Level::None + } + + fn print_path(&self, path: &Path) { + print!("{}: ", path.display()); + self.flush(); + } + + fn print_result( + &self, + print_details: bool, + summary: ColoredString, + details: Option<&dyn std::fmt::Display>, + ) { + println!("{}", summary); + if !print_details { + return; + } + if let Some(details) = details { + println!("{}", details); + } + } + + fn flush(&self) { + std::io::stdout().flush().expect("flush should succeed"); + } +} + +impl Reporter for ConsoleReporter { + fn skipped(&self, path: &Path, summary: &str, details: Option<&dyn std::fmt::Display>) { + if self.skipped_level < Level::Summary { + return; + } + self.print_path(path); + self.print_result( + self.skipped_level >= Level::Details, + summary.dimmed(), + details, + ); + } + + fn started(&self, path: &Path) { + if self.all_results_are_reported() { + // we can already output the path + self.print_path(path); + } + } + + fn succeeded(&self, path: &Path, summary: &str, details: Option<&dyn std::fmt::Display>) { + if self.succeeded_level < Level::Summary { + return; + } + if !self.all_results_are_reported() { + // the path wasn't outputed when started + self.print_path(path); + } + self.print_result( + self.succeeded_level >= Level::Details, + summary.green(), + details, + ) + } + + fn failed(&self, path: &Path, summary: &str, details: Option<&dyn std::fmt::Display>) { + if self.failed_level < Level::Summary { + return; + } + if !self.all_results_are_reported() { + // the path wasn't outputed when started + self.print_path(path); + } + self.print_result(self.failed_level >= Level::Details, summary.red(), details) + } + + fn cancelled(&self, path: &Path, summary: &str, details: Option<&dyn std::fmt::Display>) { + if self.canceled_level < Level::Summary { + return; + } + if !self.all_results_are_reported() { + // the path wasn't outputed when started + self.print_path(path); + } + self.print_result( + self.canceled_level >= Level::Details, + summary.yellow(), + details, + ) + } +}