feat: improvements to the check command.

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

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"

src/commands/check.rs

@@ -1,12 +1,24 @@
+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;
 
 #[derive(Clone, Debug, Default, ValueEnum)]
 pub enum Mode {
@@ -55,9 +67,13 @@ pub struct CheckArgs {
     #[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,
 }
 
 /// Arguments for the `lint` subcommand.
@@ -68,15 +84,117 @@ pub struct LintArgs {
     common: Common,
 }
 
-pub fn check(args: CheckArgs) -> anyhow::Result<()> {
+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;
+    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 => {}
+                }
+
+                emit(&mut stream, &config, &file, &diagnostic.to_codespan())
+                    .context("failed to emit diagnostic")?;
+            }
+        }
     }
 
+    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" }
+        );
+    }
+
+    Ok(())
+}
+
+pub fn lint(args: LintArgs) -> anyhow::Result<()> {
     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)?
+        .report_diagnostics(config, writer, true, args.common.except)?
     {
         // There are syntax errors.
         (true, _) => std::process::exit(1),
@@ -89,13 +207,6 @@ pub fn check(args: CheckArgs) -> anyhow::Result<()> {
     Ok(())
 }
 
-pub fn lint(args: LintArgs) -> anyhow::Result<()> {
-    check(CheckArgs {
-        common: args.common,
-        lint: true,
-    })
-}
-
 fn get_display_config(args: &Common) -> (Config, StandardStream) {
     let display_style = match args.report_mode {
         Mode::Full => DisplayStyle::Rich,

src/main.rs

@@ -5,8 +5,11 @@ 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;
+use tracing_log::AsTrace;
 
 pub mod commands;
 
@@ -15,7 +18,8 @@ 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,38 +38,24 @@ 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::Check(args) => commands::check::check(args).await,
         Commands::Lint(args) => commands::check::lint(args),
         Commands::Explain(args) => commands::explain::explain(args),
         Commands::Analyzer(args) => commands::analyzer::analyzer(args).await,
@@ -74,7 +64,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);
     }
 }