Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improvements to the check command. #18

Merged
merged 4 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 65 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
2 changes: 2 additions & 0 deletions src/commands.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! Implementation of sprocket CLI commands.

pub mod analyzer;
pub mod check;
pub mod explain;
2 changes: 2 additions & 0 deletions src/commands/analyzer.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! Implementation of the analyzer command.

use clap::Parser;
use wdl::lsp::Server;
use wdl::lsp::ServerOptions;
Expand Down
162 changes: 146 additions & 16 deletions src/commands/check.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions src/commands/explain.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//! Implementation of the explain command.

use clap::Parser;
use colored::Colorize;
use wdl::lint;
Expand All @@ -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() {
Expand All @@ -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());
Expand All @@ -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();
Expand Down
Loading
Loading