Skip to content

Commit

Permalink
feat: upgrade to new wdl parser (#6)
Browse files Browse the repository at this point in the history 
* [WIP]chore: use latest wdl crates

* Update explain.rs

* Update explain.rs

* Update explain.rs

* Update file.rs

* Update explain.rs

* feat: separate check and lint commands

* chore: apply review feedback

Co-Authored-By: Peter Huene <peter@huene.dev>

---------

Co-authored-by: Peter Huene <peter@huene.dev>
  • Loading branch information
@a-frantz @peterhuene
a-frantz and peterhuene authored Jun 18, 2024
1 parent be447be commit 1b7ee2a
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 287 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ nonempty = "0.9.0"
pest = { version = "2.7.5", features = ["pretty-print"] }
tracing = "0.1.40"
tracing-subscriber = "0.3.18"
wdl = { version = "0.3.0", features = ["ast", "core", "grammar"] }
wdl = { version = "0.4.0", features = ["codespan"] }
walkdir = "2.4.0"
colored = "2.1.0"
2 changes: 1 addition & 1 deletion src/commands.rs
View file
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
pub mod check;
pub mod explain;
pub mod lint;
61 changes: 40 additions & 21 deletions src/commands/lint.rs → src/commands/check.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ use codespan_reporting::term::DisplayStyle;

#[derive(Clone, Debug, Default, ValueEnum)]
pub enum Mode {
/// Prints concerns as multiple lines.
/// Prints diagnostics as multiple lines.
#[default]
Full,

/// Prints concerns as one line.
/// Prints diagnostics as one line.
OneLine,
}

Expand All @@ -26,49 +26,68 @@ impl std::fmt::Display for Mode {
}
}

/// Arguments for the `lint` subcommand.
/// Common arguments for the `check` and `lint` subcommands.
#[derive(Parser, Debug)]
#[command(author, version, about)]
pub struct Args {
/// The files or directories to lint.
pub struct Common {
/// The files or directories to check.
#[arg(required = true)]
paths: Vec<PathBuf>,

/// The extensions to collect when expanding a directory.
#[arg(short, long, default_value = "wdl")]
extensions: Vec<String>,

/// Disables color output.
#[arg(long)]
no_color: bool,

/// The report mode.
#[arg(short = 'm', long, default_value_t, value_name = "MODE")]
report_mode: Mode,
}

/// The specification version.
#[arg(short, long, default_value_t, value_enum, value_name = "VERSION")]
specification_version: wdl::core::Version,
/// Arguments for the `check` subcommand.
#[derive(Parser, Debug)]
#[command(author, version, about)]
pub struct CheckArgs {
#[command(flatten)]
common: Common,

/// Perform lint checks in addition to syntax validation.
#[arg(short, long)]
lint: bool,
}

pub fn lint(args: Args) -> anyhow::Result<()> {
let (config, writer) = get_display_config(&args);
/// Arguments for the `lint` subcommand.
#[derive(Parser, Debug)]
#[command(author, version, about)]
pub struct LintArgs {
#[command(flatten)]
common: Common,
}

match sprocket::file::Repository::try_new(args.paths, args.extensions)?
.report_concerns(config, writer)?
pub fn check(args: CheckArgs) -> 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)?
{
// There are parse errors or validation failures.
// There are syntax errors.
(true, _) => std::process::exit(1),
// There are no parse errors or validation failures, but there are lint warnings.
// There are lint failures.
(false, true) => std::process::exit(2),
// There are no concerns.
_ => {}
// There are no diagnostics.
(false, false) => {}
}

Ok(())
}

fn get_display_config(args: &Args) -> (Config, StandardStream) {
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,
Mode::OneLine => DisplayStyle::Short,
Expand Down
55 changes: 28 additions & 27 deletions src/commands/explain.rs
View file
Original file line number Diff line number Diff line change
@@ -1,49 +1,50 @@
use clap::Parser;
use colored::Colorize;
use wdl::ast::v1::lint as ast_lint;
use wdl::core::concern::lint::Rule;
use wdl::grammar::v1::lint as grammar_lint;
use wdl::lint;

/// Arguments for the `explain` subcommand.
#[derive(Parser, Debug)]
#[command(author, version, about)]
#[command(author, version, about, after_help = list_all_rules())]
pub struct Args {
/// The name or code of the rule to explain.
/// The name of the rule to explain.
#[arg(required = true)]
pub rule_identifier: String,
pub rule_name: String,
}

pub fn pretty_print_rule<E>(rule: &dyn Rule<E>) {
println!("{}", rule.name().bold().underline());
println!("{}", format!("{}::{}", rule.code(), rule.tags(),).yellow());
println!();
println!("{}", rule.body());
pub fn list_all_rules() -> String {
let mut result = "Available rules:".to_owned();
for rule in lint::v1::rules() {
result.push_str(&format!("\n - {}", rule.id()));
}
result
}

pub fn pretty_print_rule(rule: &dyn lint::v1::Rule) -> String {
let mut result = format!("{}", rule.id().bold().underline());
result = format!("{}\n{}", result, rule.description());
result = format!("{}\n{}", result, format!("{}", rule.tags()).yellow());
result = format!("{}\n\n{}", result, rule.explanation());
match rule.url() {
Some(url) => format!("{}\n{}", result, url.underline().blue()),
None => result,
}
}

pub fn explain(args: Args) -> anyhow::Result<()> {
let ident = args.rule_identifier;
let name = args.rule_name;
let lowercase_name = name.to_lowercase();

let rule = grammar_lint::rules()
let rule = lint::v1::rules()
.into_iter()
.find(|rule| rule.name() == ident || rule.code().to_string() == ident);
.find(|rule| rule.id().to_lowercase() == lowercase_name);

match rule {
Some(rule) => {
pretty_print_rule(&*rule);
println!("{}", pretty_print_rule(&*rule));
}
None => {
let rule = ast_lint::rules()
.into_iter()
.find(|rule| rule.name() == ident || rule.code().to_string() == ident);

match rule {
Some(rule) => {
pretty_print_rule(&*rule);
}
None => {
anyhow::bail!("No rule found with the identifier '{}'", ident);
}
}
println!("{}", list_all_rules());
anyhow::bail!("No rule found with the name '{}'", name);
}
}

Expand Down
159 changes: 34 additions & 125 deletions src/file.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,12 @@ use codespan_reporting::files::SimpleFiles;
use codespan_reporting::term::termcolor::StandardStream;
use codespan_reporting::term::Config;
use indexmap::IndexMap;
use wdl::core::Concern;

use crate::report::Reporter;

/// A filesystem error.
#[derive(Debug)]
pub enum Error {
/// A WDL 1.x abstract syntax tree error.
AstV1(wdl::ast::v1::Error),

/// A WDL 1.x grammar error.
GrammarV1(wdl::grammar::v1::Error),

/// An invalid file name was provided.
InvalidFileName(PathBuf),

Expand All @@ -38,8 +31,6 @@ pub enum Error {
impl std::fmt::Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Error::AstV1(err) => write!(f, "ast error: {err}"),
Error::GrammarV1(err) => write!(f, "grammar error: {err}"),
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()),
Expand Down Expand Up @@ -148,129 +139,47 @@ impl Repository {
Ok(())
}

/// Attempts to parse an existing entry into a WDL v1.x abstract syntax
/// tree.
///
/// # Examples
///
/// ```no_run
/// use sprocket::file::Repository;
///
/// let mut repository = Repository::default();
/// repository.load("test.wdl")?;
/// let ast = repository.parse("test.wdl")?;
///
/// assert!(matches!(ast.tree(), Some(_)));
///
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn parse(&self, entry: impl AsRef<str>) -> Result<wdl::ast::v1::Result> {
let entry = entry.as_ref();
let handle = *self
.handles
.get(entry)
.ok_or(Error::MissingEntry(entry.to_owned()))?;

let file = match self.sources.get(handle) {
Ok(result) => result,
// SAFETY: this entry will _always_ exist in the inner
// [`SimpleFiles`], as we just ensured it existed in the mapping
// between entry names and handles.
Err(_) => unreachable!(),
};

let mut all_concerns = wdl::core::concern::concerns::Builder::default();

let (pt, concerns) = wdl::grammar::v1::parse(file.source())
.map_err(Error::GrammarV1)?
.into_parts();

if let Some(concerns) = concerns {
for concern in concerns.into_inner() {
all_concerns = all_concerns.push(concern);
}
}

let pt = match pt {
Some(pt) => pt,
None => {
// SAFETY: because `grammar::v1::parse` returns a
// `grammar::v1::Result`, we know that either the concerns or the
// parse tree must be [`Some`] (else, this would have failed at
// `grammar::v1::Result` creation time). That said, we just checked
// that `pt` is [`None`]. In this case, it must follow that the
// concerns are not empty. As such, this will always unwrap.
return Ok(wdl::ast::v1::Result::try_new(None, all_concerns.build()).unwrap());
}
};

let (ast, concerns) = wdl::ast::v1::parse(pt).map_err(Error::AstV1)?.into_parts();

if let Some(concerns) = concerns {
for concern in concerns.into_inner() {
all_concerns = all_concerns.push(concern);
}
}

match ast {
Some(ast) => {
// SAFETY: the ast is [`Some`], so this will always unwrap.
Ok(wdl::ast::v1::Result::try_new(Some(ast), all_concerns.build()).unwrap())
}
None => {
// SAFETY: because `ast::v1::parse` returns a
// `ast::v1::Result`, we know that either the concerns or the
// parse tree must be [`Some`] (else, this would have failed at
// `ast::v1::Result` creation time). That said, we just checked
// that `ast` is [`None`]. In this case, it must follow that the
// concerns are not empty. As such, this will always unwrap.
Ok(wdl::ast::v1::Result::try_new(None, all_concerns.build()).unwrap())
}
}
}

/// Reports all concerns for all documents in the [`Repository`].
///
/// # Examples
///
/// ```no_run
/// use codespan_reporting::term::termcolor::ColorChoice;
/// use codespan_reporting::term::termcolor::StandardStream;
/// use codespan_reporting::term::Config;
/// use sprocket::file::Repository;
///
/// let mut repository = Repository::default();
/// repository.load("test.wdl")?;
///
/// let config = Config::default();
/// let writer = StandardStream::stderr(ColorChoice::Always);
/// repository.report_concerns(config, writer);
///
/// # Ok::<(), Box<dyn std::error::Error>>(())
/// ```
pub fn report_concerns(&self, config: Config, writer: StandardStream) -> Result<(bool, bool)> {
let mut reporter = Reporter::new(config, writer, &self.sources);
let mut reported_error = false;
let mut reported_warning = false;

for (file_name, handle) in self.handles.iter() {
let document = self.parse(file_name)?;

if let Some(concerns) = document.into_concerns() {
for concern in concerns.into_inner() {
reporter.report_concern(&concern, *handle);
/// Reports all diagnostics for all documents in the [`Repository`].
pub fn report_diagnostics(
&self,
config: Config,
writer: StandardStream,
lint: bool,
) -> 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");
match wdl::ast::Document::parse(file.source()).into_result() {
Ok(document) => {
let validator = wdl::ast::Validator::default();
if let Err(diagnostics) = validator.validate(&document) {
reporter.emit_diagnostics(file, &diagnostics)?;
syntax_failure = true;
continue;
}

match concern {
Concern::ParseError(_) | Concern::ValidationFailure(_) => {
reported_error = true
if lint {
let mut linter = wdl::ast::Validator::empty();
linter.add_v1_visitors(
wdl::lint::v1::rules().into_iter().map(|r| r.visitor()),
);
if let Err(diagnostics) = linter.validate(&document) {
reporter.emit_diagnostics(file, &diagnostics)?;
lint_failure = true;
}
Concern::LintWarning(_) => reported_warning = true,
}
}
Err(diagnostics) => {
reporter.emit_diagnostics(file, &diagnostics)?;
syntax_failure = true;
}
}
}

Ok((reported_error, reported_warning))
Ok((syntax_failure, lint_failure))
}
}

Expand Down
Loading

0 comments on commit 1b7ee2a

Please sign in to comment.