diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a89b91..6671b84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Changed + +* Implemented the `check` command as a full static analysis ([#17](https://github.com/stjude-rust-labs/sprocket/pull/17)). + ## 0.6.0 - 08-22-2024 ### Added diff --git a/Cargo.lock b/Cargo.lock index f23369a..98a2028 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -179,6 +179,16 @@ dependencies = [ "clap_derive", ] +[[package]] +name = "clap-verbosity-flag" +version = "2.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "63d19864d6b68464c59f7162c9914a0b569ddc2926b4a2d71afe62a9738eff53" +dependencies = [ + "clap", + "log", +] + [[package]] name = "clap_builder" version = "4.5.11" @@ -235,6 +245,19 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "console" +version = "0.15.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e1f83fc076bd6dd27517eacdf25fef6c4dfe5f1d7448bafaaf3a26f13b5e4eb" +dependencies = [ + "encode_unicode", + "lazy_static", + "libc", + "unicode-width", + "windows-sys 0.52.0", +] + [[package]] name = "convert_case" version = "0.6.0" @@ -319,6 +342,12 @@ version = "1.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" +[[package]] +name = "encode_unicode" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a357d28ed41a50f9c765dbfe56cbc04a64e53e5fc58ba79fbc34c10ef3df831f" + [[package]] name = "encoding_rs" version = "0.8.34" @@ -691,6 +720,28 @@ dependencies = [ "serde", ] +[[package]] +name = "indicatif" +version = "0.17.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "763a5a8f45087d6bcea4222e7b72c291a054edf80e4ef6efd2a4979878c7bea3" +dependencies = [ + "console", + "instant", + "number_prefix", + "portable-atomic", + "unicode-width", +] + +[[package]] +name = "instant" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e0242819d153cba4b4b05a5a8f2a7e9bbf97b6055b2a002b395c96b5ff3c0222" +dependencies = [ + "cfg-if", +] + [[package]] name = "ipnet" version = "2.9.0" @@ -895,6 +946,12 @@ version = "0.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "51d515d32fb182ee37cda2ccdcb92950d6a3c2893aa280e540671c2cd0f3b1d9" +[[package]] +name = "number_prefix" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "830b246a0e5f20af87141b25c173cd1b609bd7779a4617d6ec582abaf90870f3" + [[package]] name = "object" version = "0.36.2" @@ -1056,6 +1113,12 @@ version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d231b230927b5e4ad203db57bbcbee2802f6bce620b1e4a9024a07d94e2907ec" +[[package]] +name = "portable-atomic" +version = "1.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da544ee218f0d287a911e9c99a39a8c9bc8fcad3cb8db5959940044ecfc67265" + [[package]] name = "powerfmt" version = "0.2.0" @@ -1413,10 +1476,12 @@ version = "0.6.0" dependencies = [ "anyhow", "clap", + "clap-verbosity-flag", "codespan-reporting", "colored", "git-testament", "indexmap", + "indicatif", "nonempty", "pest", "tokio", diff --git a/Cargo.toml b/Cargo.toml index 8111ace..ff627df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,8 +20,10 @@ nonempty = "0.10.0" pest = { version = "2.7.11", features = ["pretty-print"] } tracing = "0.1.40" tracing-subscriber = "0.3.18" -wdl = { version = "0.7.0", features = ["codespan", "lsp"] } +wdl = { version = "0.7.0", features = ["codespan", "lsp", "analysis"] } walkdir = "2.5.0" colored = "2.1.0" tokio = { version = "1.39.1", features = ["full"] } tracing-log = "0.2.0" +indicatif = "0.17.8" +clap-verbosity-flag = "2.2.1" diff --git a/src/commands.rs b/src/commands.rs index 5ec0bc6..0f0c938 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1,3 +1,5 @@ +//! Implementation of sprocket CLI commands. + pub mod analyzer; pub mod check; pub mod explain; diff --git a/src/commands/analyzer.rs b/src/commands/analyzer.rs index 9dc342b..f6cc6d3 100644 --- a/src/commands/analyzer.rs +++ b/src/commands/analyzer.rs @@ -1,3 +1,5 @@ +//! Implementation of the analyzer command. + use clap::Parser; use wdl::lsp::Server; use wdl::lsp::ServerOptions; diff --git a/src/commands/check.rs b/src/commands/check.rs index 99c1827..daabf8b 100644 --- a/src/commands/check.rs +++ b/src/commands/check.rs @@ -1,13 +1,28 @@ +//! Implementation of the check and lint commands. + +use std::borrow::Cow; use std::path::PathBuf; use anyhow::bail; +use anyhow::Context; use clap::Parser; use clap::ValueEnum; +use codespan_reporting::files::SimpleFile; +use codespan_reporting::term::emit; use codespan_reporting::term::termcolor::ColorChoice; use codespan_reporting::term::termcolor::StandardStream; use codespan_reporting::term::Config; use codespan_reporting::term::DisplayStyle; - +use indicatif::ProgressBar; +use indicatif::ProgressStyle; +use wdl::analysis::Analyzer; +use wdl::ast::Diagnostic; +use wdl::ast::Severity; +use wdl::ast::SyntaxNode; +use wdl::ast::Validator; +use wdl::lint::LintVisitor; + +/// The diagnostic mode to use for reporting diagnostics. #[derive(Clone, Debug, Default, ValueEnum)] pub enum Mode { /// Prints diagnostics as multiple lines. @@ -52,50 +67,165 @@ pub struct Common { #[derive(Parser, Debug)] #[command(author, version, about)] pub struct CheckArgs { + /// The common command line arguments. #[command(flatten)] common: Common, - /// Perform lint checks in addition to syntax validation. + /// Perform lint checks in addition to checking for errors. #[arg(short, long)] lint: bool, + + /// Causes the command to fail if warnings were reported. + #[clap(long)] + deny_warnings: bool, + + /// Causes the command to fail if notes were reported. + #[clap(long)] + deny_notes: bool, } /// Arguments for the `lint` subcommand. #[derive(Parser, Debug)] #[command(author, version, about)] pub struct LintArgs { + /// The command command line arguments. #[command(flatten)] common: Common, + + /// Causes the command to fail if warnings were reported. + #[clap(long)] + deny_warnings: bool, + + /// Causes the command to fail if notes were reported. + #[clap(long)] + deny_notes: bool, } -pub fn check(args: CheckArgs) -> anyhow::Result<()> { +/// Checks WDL source files for diagnostics. +pub async fn check(args: CheckArgs) -> anyhow::Result<()> { if !args.lint && !args.common.except.is_empty() { - bail!("cannot specify --except without --lint"); + bail!("cannot specify `--except` without `--lint`"); + } + + let (config, mut stream) = get_display_config(&args.common); + + let lint = args.lint; + let except_rules = args.common.except; + let analyzer = Analyzer::new_with_validator( + move |bar: ProgressBar, kind, completed, total| async move { + if completed == 0 { + bar.set_length(total.try_into().unwrap()); + bar.set_message(format!("{kind}")); + } + bar.set_position(completed.try_into().unwrap()); + }, + move || { + let mut validator = Validator::empty(); + + if lint { + let visitor = LintVisitor::new(wdl::lint::rules().into_iter().filter_map(|rule| { + if except_rules.contains(&rule.id().to_string()) { + None + } else { + Some(rule) + } + })); + validator.add_visitor(visitor); + } + + validator + }, + ); + + let bar = ProgressBar::new(0); + bar.set_style( + ProgressStyle::with_template("[{elapsed_precise}] {bar:40.cyan/blue} {msg} {pos}/{len}") + .unwrap(), + ); + + analyzer.add_documents(args.common.paths).await?; + let results = analyzer + .analyze(bar.clone()) + .await + .context("failed to analyze documents")?; + + // Drop (hide) the progress bar before emitting any diagnostics + drop(bar); + + let cwd = std::env::current_dir().ok(); + let mut error_count = 0; + let mut warning_count = 0; + let mut note_count = 0; + for result in &results { + let path = result.uri().to_file_path().ok(); + + // Attempt to strip the CWD from the result path + let path = match (&cwd, &path) { + // Only display diagnostics for local files. + (_, None) => continue, + // Use just the path if there's no CWD + (None, Some(path)) => path.to_string_lossy(), + // Strip the CWD from the path + (Some(cwd), Some(path)) => path.strip_prefix(cwd).unwrap_or(path).to_string_lossy(), + }; + + let diagnostics: Cow<'_, [Diagnostic]> = match result.parse_result().error() { + Some(e) => vec![Diagnostic::error(format!("failed to read `{path}`: {e:#}"))].into(), + None => result.diagnostics().into(), + }; + + if !diagnostics.is_empty() { + let source = result + .parse_result() + .root() + .map(|n| SyntaxNode::new_root(n.clone()).text().to_string()) + .unwrap_or(String::new()); + let file = SimpleFile::new(path, source); + for diagnostic in diagnostics.iter() { + match diagnostic.severity() { + Severity::Error => error_count += 1, + Severity::Warning => warning_count += 1, + Severity::Note => note_count += 1, + } + + emit(&mut stream, &config, &file, &diagnostic.to_codespan()) + .context("failed to emit diagnostic")?; + } + } } - let (config, writer) = get_display_config(&args.common); - - match sprocket::file::Repository::try_new(args.common.paths, vec!["wdl".to_string()])? - .report_diagnostics(config, writer, args.lint, args.common.except)? - { - // There are syntax errors. - (true, _) => std::process::exit(1), - // There are lint failures. - (false, true) => std::process::exit(2), - // There are no diagnostics. - (false, false) => {} + if error_count > 0 { + bail!( + "aborting due to previous {error_count} error{s}", + s = if error_count == 1 { "" } else { "s" } + ); + } else if args.deny_warnings && warning_count > 0 { + bail!( + "aborting due to previous {warning_count} warning{s} (`--deny-warnings` was specified)", + s = if warning_count == 1 { "" } else { "s" } + ); + } else if args.deny_notes && note_count > 0 { + bail!( + "aborting due to previous {note_count} note{s} (`--deny-notes` was specified)", + s = if note_count == 1 { "" } else { "s" } + ); } Ok(()) } -pub fn lint(args: LintArgs) -> anyhow::Result<()> { +/// Lints WDL source files. +pub async fn lint(args: LintArgs) -> anyhow::Result<()> { check(CheckArgs { common: args.common, lint: true, + deny_warnings: args.deny_warnings, + deny_notes: args.deny_notes, }) + .await } +/// Gets the display config to use for reporting diagnostics. fn get_display_config(args: &Common) -> (Config, StandardStream) { let display_style = match args.report_mode { Mode::Full => DisplayStyle::Rich, diff --git a/src/commands/explain.rs b/src/commands/explain.rs index 2268e2e..cd489ed 100644 --- a/src/commands/explain.rs +++ b/src/commands/explain.rs @@ -1,3 +1,5 @@ +//! Implementation of the explain command. + use clap::Parser; use colored::Colorize; use wdl::lint; @@ -11,6 +13,7 @@ pub struct Args { pub rule_name: String, } +/// Lists all rules as a string for displaying after CLI help. pub fn list_all_rules() -> String { let mut result = "Available rules:".to_owned(); for rule in lint::rules() { @@ -19,6 +22,7 @@ pub fn list_all_rules() -> String { result } +/// Pretty prints a rule to a string. pub fn pretty_print_rule(rule: &dyn lint::Rule) -> String { let mut result = format!("{}", rule.id().bold().underline()); result = format!("{}\n{}", result, rule.description()); @@ -30,6 +34,7 @@ pub fn pretty_print_rule(rule: &dyn lint::Rule) -> String { } } +/// Explains a lint rule. pub fn explain(args: Args) -> anyhow::Result<()> { let name = args.rule_name; let lowercase_name = name.to_lowercase(); diff --git a/src/file.rs b/src/file.rs deleted file mode 100644 index d5202de..0000000 --- a/src/file.rs +++ /dev/null @@ -1,227 +0,0 @@ -//! Filesystems. - -use std::path::PathBuf; - -use codespan_reporting::files::SimpleFiles; -use codespan_reporting::term::termcolor::StandardStream; -use codespan_reporting::term::Config; -use indexmap::IndexMap; -use wdl::ast::Document; -use wdl::ast::Validator; -use wdl::lint::LintVisitor; - -use crate::report::Reporter; - -/// A filesystem error. -#[derive(Debug)] -pub enum Error { - /// An invalid file name was provided. - InvalidFileName(PathBuf), - - /// An input/output error. - Io(std::io::Error), - - /// Attempted to parse an entry that does not exist in the [`Repository`]. - MissingEntry(String), - - /// The item located at a path was missing. - MissingPath(PathBuf), - - /// The item located at a path was not a file. - NonFile(PathBuf), -} - -impl std::fmt::Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Error::InvalidFileName(path) => write!(f, "invalid file name: {}", path.display()), - Error::Io(err) => write!(f, "i/o error: {}", err), - Error::MissingPath(path) => write!(f, "missing path: {}", path.display()), - Error::NonFile(path) => write!(f, "not a file: {}", path.display()), - Error::MissingEntry(entry) => write!(f, "missing entry: {entry}"), - } - } -} - -impl std::error::Error for Error {} - -/// A [`Result`](std::result::Result) with an [`Error`]. -type Result = std::result::Result; - -/// A repository of files and associated source code. -#[derive(Debug)] -pub struct Repository { - /// The mapping of entries in the source code map to file handles. - handles: IndexMap, - - /// The inner source code map. - sources: SimpleFiles, -} - -impl Default for Repository { - fn default() -> Self { - Self { - sources: SimpleFiles::new(), - handles: Default::default(), - } - } -} - -impl Repository { - /// Creates a new [`Repository`]. - /// - /// # Examples - /// - /// ```no_run - /// use std::path::PathBuf; - /// - /// use sprocket::file::Repository; - /// - /// let mut repository = Repository::try_new(vec![PathBuf::from(".")], vec![String::from("wdl")]); - /// ``` - pub fn try_new(paths: Vec, extensions: Vec) -> Result { - let mut repository = Self::default(); - - for path in expand_paths(paths, extensions)? { - repository.load(path)?; - } - - Ok(repository) - } - - /// Inserts a new entry into the [`Repository`]. - /// - /// **Note:** typically, you won't want to do this directly except in - /// special cases. Instead, prefer using the [`load()`](Repository::load()) - /// method. - /// - /// # Examples - /// - /// ``` - /// use sprocket::file::Repository; - /// - /// let mut repository = Repository::default(); - /// repository.insert("foo.txt", "bar"); - /// ``` - pub fn insert(&mut self, path: impl Into, content: impl Into) { - let path = path.into().to_string_lossy().to_string(); - let content = content.into(); - - let handle = self.sources.add(path.clone(), content); - self.handles.insert(path, handle); - } - - /// Attempts to load a new file and its contents into the [`Repository`]. - /// - /// An error is thrown if any issues are encountered when reading the file. - /// - /// # Examples - /// - /// ```no_run - /// use sprocket::file::Repository; - /// - /// let mut repository = Repository::default(); - /// repository.load("test.wdl")?; - /// - /// # Ok::<(), Box>(()) - /// ``` - pub fn load(&mut self, path: impl Into) -> Result<()> { - let path = path.into(); - - if !path.exists() { - return Err(Error::MissingPath(path)); - } - - if !path.is_file() { - return Err(Error::NonFile(path)); - } - - let content = std::fs::read_to_string(&path).map_err(Error::Io)?; - self.insert(path, content); - - Ok(()) - } - - /// Reports all diagnostics for all documents in the [`Repository`]. - pub fn report_diagnostics( - &self, - config: Config, - writer: StandardStream, - lint: bool, - except_rules: Vec, - ) -> anyhow::Result<(bool, bool)> { - let mut reporter = Reporter::new(config, writer); - let mut syntax_failure = false; - let mut lint_failure = false; - - for (_path, handle) in self.handles.iter() { - let file = self.sources.get(*handle).expect("Expected to find file"); - let (document, diagnostics) = Document::parse(file.source()); - if !diagnostics.is_empty() { - reporter.emit_diagnostics(file, &diagnostics)?; - syntax_failure = true; - continue; - } - - let mut validator = Validator::default(); - if let Err(diagnostic) = validator.validate(&document) { - reporter.emit_diagnostics(file, &diagnostic)?; - syntax_failure = true; - continue; - } - - if lint { - let mut linter = Validator::empty(); - let visitor = LintVisitor::new(wdl::lint::rules().into_iter().filter_map(|rule| { - if except_rules.contains(&rule.id().to_string()) { - None - } else { - Some(rule) - } - })); - linter.add_visitor(visitor); - if let Err(diagnostic) = linter.validate(&document) { - reporter.emit_diagnostics(file, &diagnostic)?; - lint_failure = true; - } - } - } - - Ok((syntax_failure, lint_failure)) - } -} - -/// Expands a set of [`PathBuf`]s. -/// -/// This means that, for each [`PathBuf`], -/// -/// * if the path exists and is a file, the file is added to the result. -/// * if the path exists and is a directory, all files underneath that directory -/// (including recursively traversed directories) that have an extension in -/// the `extensions` list are added to the result. -/// * if the path does not exist, an error is thrown. -pub fn expand_paths(paths: Vec, extensions: Vec) -> Result> { - paths.into_iter().try_fold(Vec::new(), |mut acc, path| { - if !path.exists() { - return Err(Error::MissingPath(path)); - } - - if path.is_file() { - acc.push(path); - } else if path.is_dir() { - let dir_files = walkdir::WalkDir::new(path) - .into_iter() - .filter_map(std::result::Result::ok) - .filter(|entry| { - extensions - .iter() - .any(|ext| entry.path().extension() == Some(ext.as_ref())) - }) - .map(|entry| entry.path().to_path_buf()); - acc.extend(dir_files) - } - - acc.sort(); - Ok(acc) - }) -} diff --git a/src/lib.rs b/src/lib.rs index 0a2cb75..bd9e1f6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,5 +7,4 @@ #![warn(clippy::missing_docs_in_private_items)] #![warn(rustdoc::broken_intra_doc_links)] -pub mod file; -pub mod report; +pub mod commands; diff --git a/src/main.rs b/src/main.rs index f88ed58..b5a4e6f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,17 +5,20 @@ use std::io::IsTerminal; use clap::Parser; use clap::Subcommand; +use clap_verbosity_flag::Verbosity; +use colored::Colorize; use git_testament::git_testament; use git_testament::render_testament; - -pub mod commands; +use sprocket::commands; +use tracing_log::AsTrace; git_testament!(TESTAMENT); #[derive(Subcommand)] #[allow(clippy::large_enum_variant)] enum Commands { - /// Checks the syntactic validity of Workflow Description Language files. + /// Checks a WDL file (or a directory containing WDL files) and reports + /// diagnostics. Check(commands::check::CheckArgs), /// Lints Workflow Description Language files. @@ -34,39 +37,25 @@ struct Cli { #[command(subcommand)] pub command: Commands, - /// Only errors are printed to the stderr stream. - #[arg(short, long, global = true)] - pub quiet: bool, - - /// All available information, including debug information, is printed to - /// stderr. - #[arg(short, long, global = true)] - pub verbose: bool, + #[command(flatten)] + verbose: Verbosity, } pub async fn inner() -> anyhow::Result<()> { let cli = Cli::parse(); - let level = if cli.verbose { - tracing::Level::DEBUG - } else if cli.quiet { - tracing::Level::ERROR - } else { - tracing::Level::INFO - }; - tracing_log::LogTracer::init()?; let subscriber = tracing_subscriber::fmt::Subscriber::builder() - .with_max_level(level) + .with_max_level(cli.verbose.log_level_filter().as_trace()) .with_writer(std::io::stderr) .with_ansi(stderr().is_terminal()) .finish(); tracing::subscriber::set_global_default(subscriber)?; match cli.command { - Commands::Check(args) => commands::check::check(args), - Commands::Lint(args) => commands::check::lint(args), + Commands::Check(args) => commands::check::check(args).await, + Commands::Lint(args) => commands::check::lint(args).await, Commands::Explain(args) => commands::explain::explain(args), Commands::Analyzer(args) => commands::analyzer::analyzer(args).await, } @@ -74,7 +63,15 @@ pub async fn inner() -> anyhow::Result<()> { #[tokio::main] pub async fn main() { - if let Err(err) = inner().await { - eprintln!("error: {}", err); + if let Err(e) = inner().await { + eprintln!( + "{error}: {e:?}", + error = if std::io::stderr().is_terminal() { + "error".red().bold() + } else { + "error".normal() + } + ); + std::process::exit(1); } } diff --git a/src/report.rs b/src/report.rs deleted file mode 100644 index ff54d7c..0000000 --- a/src/report.rs +++ /dev/null @@ -1,45 +0,0 @@ -//! Reporting. - -use anyhow::Context; -use anyhow::Result; -use codespan_reporting::files::SimpleFile; -use codespan_reporting::term::emit; -use codespan_reporting::term::termcolor::StandardStream; -use codespan_reporting::term::Config; -use wdl::grammar::Diagnostic; - -/// A reporter for Sprocket. -#[derive(Debug)] -pub(crate) struct Reporter { - /// The configuration. - config: Config, - - /// The stream to write to. - stream: StandardStream, -} - -impl Reporter { - /// Creates a new [`Reporter`]. - pub(crate) fn new(config: Config, stream: StandardStream) -> Self { - Self { config, stream } - } - - /// Reports diagnostics to the terminal. - pub(crate) fn emit_diagnostics( - &mut self, - file: &SimpleFile, - diagnostics: &[Diagnostic], - ) -> Result<()> { - for diagnostic in diagnostics.iter() { - emit( - &mut self.stream, - &self.config, - file, - &diagnostic.to_codespan(), - ) - .context("failed to emit diagnostic")?; - } - - Ok(()) - } -}