feat: improvements to the check command. (#18)

* feat: improvements to the `check` command.

This commit contains the following improvements:

* use `wdl-analysis` for the `check` command that will perform a full
  static-analysis on the specified file or directory.
* add a progress bar to display for the `check` command.
* use `clap-verbosity-flag` for the `quiet` and `verbose` flags; this has the
  advantage of being error level by default and each `-v` specified drops the
  log level down by one; this gives a little more control over the previous
  implementation that supported either info (default), debug (-v), or error
  (-q).
* we now colorize the `error` part of handling errors from command execution,
  displaying the full context of the error, and exiting with status code 1.

* feat: add `--deny-notes` option to the `check` command.

* feat: make the `lint` command also perform an analysis.

* chore: code cleanup and removal of unused code.

This commit also exposes the commands as part of the `sprocket` crate from
which `main.rs` will use.

Author: peterhuene

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.lock

@@ -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"

Cargo.toml

@@ -1,3 +1,5 @@
+//! Implementation of sprocket CLI commands.
+
 pub mod analyzer;
 pub mod check;
 pub mod explain;

src/commands.rs

@@ -1,3 +1,5 @@
+//! Implementation of the analyzer command.
+
 use clap::Parser;
 use wdl::lsp::Server;
 use wdl::lsp::ServerOptions;

src/commands/analyzer.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,

src/commands/check.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();