diff --git a/Cargo.toml b/Cargo.toml index b7e66e35..d53b1174 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,7 @@ [workspace] members = [ "wdl", + "wdl-analysis", "wdl-ast", "wdl-gauntlet", "wdl-grammar", @@ -40,3 +41,10 @@ git2 = "0.18.3" temp-dir = "0.1.13" url = "2.5.2" urlencoding = "2.1.3" +parking_lot = "0.12.3" +reqwest = "0.12.5" +petgraph = "0.6.5" +futures = "0.3.30" +glob = "0.3.1" +path-clean = "1.0.1" +indicatif = "0.17.8" diff --git a/Gauntlet.toml b/Gauntlet.toml index c0ebf46d..3f20e066 100644 --- a/Gauntlet.toml +++ b/Gauntlet.toml @@ -376,201 +376,11 @@ document = "biowdl/tasks:/umi.wdl" message = 'umi.wdl:39:60: error: unknown escape sequence `\.`' permalink = "https://github.com/biowdl/tasks/blob/2bf875300d90a3c9c8d670b3d99026452d5dbae2/umi.wdl/#L39" -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/ApplyBQSR.wdl" -message = "ApplyBQSR.wdl:104:6: error: conflicting task name `ApplyBQSR`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/ApplyBQSR.wdl/#L104" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/ApplyBQSRAllArgs.wdl" -message = "ApplyBQSRAllArgs.wdl:291:6: error: conflicting task name `ApplyBQSR`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/ApplyBQSRAllArgs.wdl/#L291" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/BaseRecalibrator.wdl" -message = "BaseRecalibrator.wdl:111:6: error: conflicting task name `BaseRecalibrator`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/BaseRecalibrator.wdl/#L111" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/BaseRecalibratorAllArgs.wdl" -message = "BaseRecalibratorAllArgs.wdl:318:6: error: conflicting task name `BaseRecalibrator`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/BaseRecalibratorAllArgs.wdl/#L318" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/ClipReads.wdl" -message = "ClipReads.wdl:100:6: error: conflicting task name `ClipReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/ClipReads.wdl/#L100" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/ClipReadsAllArgs.wdl" -message = "ClipReadsAllArgs.wdl:304:6: error: conflicting task name `ClipReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/ClipReadsAllArgs.wdl/#L304" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/CollectReadCounts.wdl" -message = "CollectReadCounts.wdl:99:6: error: conflicting task name `CollectReadCounts`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/CollectReadCounts.wdl/#L99" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/CollectReadCountsAllArgs.wdl" -message = "CollectReadCountsAllArgs.wdl:266:6: error: conflicting task name `CollectReadCounts`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/CollectReadCountsAllArgs.wdl/#L266" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/CombineGVCFs.wdl" -message = "CombineGVCFs.wdl:112:6: error: conflicting task name `CombineGVCFs`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/CombineGVCFs.wdl/#L112" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/CombineGVCFsAllArgs.wdl" -message = "CombineGVCFsAllArgs.wdl:323:6: error: conflicting task name `CombineGVCFs`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/CombineGVCFsAllArgs.wdl/#L323" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/CountBases.wdl" -message = "CountBases.wdl:91:6: error: conflicting task name `CountBases`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/CountBases.wdl/#L91" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/CountBasesAllArgs.wdl" -message = "CountBasesAllArgs.wdl:262:6: error: conflicting task name `CountBases`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/CountBasesAllArgs.wdl/#L262" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/CountReads.wdl" -message = "CountReads.wdl:91:6: error: conflicting task name `CountReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/CountReads.wdl/#L91" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/CountReadsAllArgs.wdl" -message = "CountReadsAllArgs.wdl:262:6: error: conflicting task name `CountReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/CountReadsAllArgs.wdl/#L262" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/FixMisencodedBaseQualityReads.wdl" -message = "FixMisencodedBaseQualityReads.wdl:100:6: error: conflicting task name `FixMisencodedBaseQualityReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/FixMisencodedBaseQualityReads.wdl/#L100" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/FixMisencodedBaseQualityReadsAllArgs.wdl" -message = "FixMisencodedBaseQualityReadsAllArgs.wdl:267:6: error: conflicting task name `FixMisencodedBaseQualityReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/FixMisencodedBaseQualityReadsAllArgs.wdl/#L267" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/FlagStat.wdl" -message = "FlagStat.wdl:91:6: error: conflicting task name `FlagStat`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/FlagStat.wdl/#L91" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/FlagStatAllArgs.wdl" -message = "FlagStatAllArgs.wdl:262:6: error: conflicting task name `FlagStat`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/FlagStatAllArgs.wdl/#L262" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/GatherTranches.wdl" -message = "GatherTranches.wdl:95:6: error: conflicting task name `GatherTranches`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/GatherTranches.wdl/#L95" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/GatherTranchesAllArgs.wdl" -message = "GatherTranchesAllArgs.wdl:146:6: error: conflicting task name `GatherTranches`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/GatherTranchesAllArgs.wdl/#L146" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/LeftAlignIndels.wdl" -message = "LeftAlignIndels.wdl:112:6: error: conflicting task name `LeftAlignIndels`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/LeftAlignIndels.wdl/#L112" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/LeftAlignIndelsAllArgs.wdl" -message = "LeftAlignIndelsAllArgs.wdl:267:6: error: conflicting task name `LeftAlignIndels`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/LeftAlignIndelsAllArgs.wdl/#L267" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/PrintReads.wdl" -message = "PrintReads.wdl:100:6: error: conflicting task name `PrintReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/PrintReads.wdl/#L100" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/PrintReadsAllArgs.wdl" -message = "PrintReadsAllArgs.wdl:267:6: error: conflicting task name `PrintReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/PrintReadsAllArgs.wdl/#L267" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/ReadAnonymizer.wdl" -message = "ReadAnonymizer.wdl:112:6: error: conflicting task name `ReadAnonymizer`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/ReadAnonymizer.wdl/#L112" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/ReadAnonymizerAllArgs.wdl" -message = "ReadAnonymizerAllArgs.wdl:275:6: error: conflicting task name `ReadAnonymizer`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/ReadAnonymizerAllArgs.wdl/#L275" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/RevertBaseQualityScores.wdl" -message = "RevertBaseQualityScores.wdl:100:6: error: conflicting task name `RevertBaseQualityScores`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/RevertBaseQualityScores.wdl/#L100" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/RevertBaseQualityScoresAllArgs.wdl" -message = "RevertBaseQualityScoresAllArgs.wdl:267:6: error: conflicting task name `RevertBaseQualityScores`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/RevertBaseQualityScoresAllArgs.wdl/#L267" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/SplitNCigarReads.wdl" -message = "SplitNCigarReads.wdl:112:6: error: conflicting task name `SplitNCigarReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/SplitNCigarReads.wdl/#L112" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/SplitNCigarReadsAllArgs.wdl" -message = "SplitNCigarReadsAllArgs.wdl:291:6: error: conflicting task name `SplitNCigarReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/SplitNCigarReadsAllArgs.wdl/#L291" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/SplitReads.wdl" -message = "SplitReads.wdl:95:6: error: conflicting task name `SplitReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/SplitReads.wdl/#L95" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/SplitReadsAllArgs.wdl" -message = "SplitReadsAllArgs.wdl:274:6: error: conflicting task name `SplitReads`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/SplitReadsAllArgs.wdl/#L274" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/UnmarkDuplicates.wdl" -message = "UnmarkDuplicates.wdl:100:6: error: conflicting task name `UnmarkDuplicates`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/UnmarkDuplicates.wdl/#L100" - -[[diagnostics]] -document = "broadinstitute/gatk-tool-wdls:/wdls/UnmarkDuplicatesAllArgs.wdl" -message = "UnmarkDuplicatesAllArgs.wdl:267:6: error: conflicting task name `UnmarkDuplicates`" -permalink = "https://github.com/broadinstitute/gatk-tool-wdls/blob/4536dd2c6719b86f4c50348f0ce354818260cadb/wdls/UnmarkDuplicatesAllArgs.wdl/#L267" - -[[diagnostics]] -document = "broadinstitute/palantir-workflows:/BenchmarkSVs/WittyerTasks.wdl" -message = "WittyerTasks.wdl:74:14: error: conflicting declaration name `wittyer_config`" -permalink = "https://github.com/broadinstitute/palantir-workflows/blob/1e32078d3b57dcb2291534d0caa30f600d40967e/BenchmarkSVs/WittyerTasks.wdl/#L74" - -[[diagnostics]] -document = "broadinstitute/palantir-workflows:/BenchmarkVCFs/BenchmarkAndCompareVCFs.wdl" -message = "BenchmarkAndCompareVCFs.wdl:43:14: error: conflicting declaration name `preemptible`" -permalink = "https://github.com/broadinstitute/palantir-workflows/blob/1e32078d3b57dcb2291534d0caa30f600d40967e/BenchmarkVCFs/BenchmarkAndCompareVCFs.wdl/#L43" - [[diagnostics]] document = "broadinstitute/palantir-workflows:/HaplotypeMap/BuildHaplotypeMap.wdl" message = "BuildHaplotypeMap.wdl:17:1: error: a WDL document must start with a version statement" permalink = "https://github.com/broadinstitute/palantir-workflows/blob/1e32078d3b57dcb2291534d0caa30f600d40967e/HaplotypeMap/BuildHaplotypeMap.wdl/#L17" -[[diagnostics]] -document = "broadinstitute/palantir-workflows:/LongReadRNABenchmark/IsoformDiscoveryBenchmark.wdl" -message = "IsoformDiscoveryBenchmark.wdl:147:13: error: conflicting scatter variable name `gtf`" -permalink = "https://github.com/broadinstitute/palantir-workflows/blob/1e32078d3b57dcb2291534d0caa30f600d40967e/LongReadRNABenchmark/IsoformDiscoveryBenchmark.wdl/#L147" - -[[diagnostics]] -document = "broadinstitute/warp:/pipelines/broad/dna_seq/somatic/single_sample/wgs/gdc_genome/GDCWholeGenomeSomaticSingleSample.wdl" -message = "GDCWholeGenomeSomaticSingleSample.wdl:672:14: error: conflicting scatter variable name `ubam`" -permalink = "https://github.com/broadinstitute/warp/blob/e49ffbcd72b138ad4dc6ec62c847943908e4abf9/pipelines/broad/dna_seq/somatic/single_sample/wgs/gdc_genome/GDCWholeGenomeSomaticSingleSample.wdl/#L672" - [[diagnostics]] document = "broadinstitute/warp:/pipelines/skylab/scATAC/scATAC.wdl" message = "scATAC.wdl:203:9: error: duplicate key `cpu` in runtime section" diff --git a/README.md b/README.md index b9c07ea9..93c69e2a 100644 --- a/README.md +++ b/README.md @@ -90,21 +90,27 @@ the `wdl` family of crates. The `wdl` CLI tool can be run with the following command: ```bash -cargo run --bin wdl --features binaries -- $ARGS +cargo run --bin wdl --features cli -- $ARGS ``` Where `$ARGS` are the command line arguments to the `wdl` CLI tool. The `wdl` CLI tool currently supports three subcommands: -* `parse` - Parses a WDL source file and prints both the parse diagnostics and - the resulting Concrete Syntax Tree (CST). -* `check` - Parses and validates a WDL source file. Exits with a status code of - `0` if the file is syntactically valid; otherwise, prints the validation - diagnostics and exits with a status code of `1`. -* `lint` - Parses, validates, and runs the linting rules on a WDL - source file. Exits with a status code of `0` if the file passes all lints; - otherwise, prints the linting diagnostics and exits with a status code of `1`. +* `parse` - Parses a WDL document and prints both the parse diagnostics and the + resulting Concrete Syntax Tree (CST). +* `check` - Parses, validates, and analyzes a WDL document or a directory + containing WDL documents. Exits with a status code of `0` if the documents + are valid; otherwise, prints the validation diagnostics and exits with a + status code of `1`. +* `lint` - Parses, validates, and runs the linting rules on a WDL document. + Exits with a status code of `0` if the file passes all lints; otherwise, + prints the linting diagnostics and exits with a status code of `1`. +* `analyze` - Parses, validates, and analyzes a single WDL document or a + directory containing WDL documents. Prints a debug representation of the + document scopes and exits with a status code of `0` if the documents are + valid; otherwise, prints the validation diagnostics and exits with a status + code of `1`. Each of the subcommands supports passing `-` as the file path to denote reading from STDIN instead of a file on disk. diff --git a/wdl-analysis/CHANGELOG.md b/wdl-analysis/CHANGELOG.md new file mode 100644 index 00000000..ddcf6716 --- /dev/null +++ b/wdl-analysis/CHANGELOG.md @@ -0,0 +1,12 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## Unreleased + +### Added + +* Added the `wdl-analysis` crate for analyzing WDL documents ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). diff --git a/wdl-analysis/Cargo.toml b/wdl-analysis/Cargo.toml new file mode 100644 index 00000000..4a4106dd --- /dev/null +++ b/wdl-analysis/Cargo.toml @@ -0,0 +1,41 @@ +[package] +name = "wdl-analysis" +version = "0.1.0" +license.workspace = true +edition.workspace = true +authors.workspace = true +homepage.workspace = true +repository.workspace = true +description = "Analysis of Workflow Description Language (WDL) documents." +documentation = "https://docs.rs/wdl-analysis" + +[dependencies] +wdl-ast = { path = "../wdl-ast", version = "0.4.0" } +wdl-lint = { path = "../wdl-lint", version = "0.3.0", optional = true } +anyhow = { workspace = true } +rowan = { workspace = true } +url = { workspace = true } +tokio = { workspace = true } +parking_lot = { workspace = true } +log = { workspace = true } +rayon = { workspace = true } +reqwest = { workspace = true } +petgraph = { workspace = true } +futures = { workspace = true } +glob = { workspace = true } +path-clean = { workspace = true } +indexmap = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } +colored = { workspace = true } +codespan-reporting = { workspace = true } + +[features] +default = [] +codespan = ["wdl-ast/codespan"] + +[[test]] +name = "analysis" +required-features = ["codespan"] +harness = false diff --git a/wdl-analysis/src/engine.rs b/wdl-analysis/src/engine.rs new file mode 100644 index 00000000..7bbe1827 --- /dev/null +++ b/wdl-analysis/src/engine.rs @@ -0,0 +1,789 @@ +//! Implementation of the analysis engine. + +use std::cell::RefCell; +use std::collections::HashSet; +use std::fmt; +use std::fs; +use std::path::Path; +use std::sync::Arc; +use std::time::Duration; +use std::time::Instant; + +use anyhow::anyhow; +use anyhow::bail; +use anyhow::Context; +use anyhow::Result; +use futures::stream::FuturesUnordered; +use futures::Future; +use futures::StreamExt; +use parking_lot::RwLock; +use petgraph::algo::has_path_connecting; +use petgraph::algo::DfsSpace; +use petgraph::graph::NodeIndex; +use petgraph::stable_graph::StableDiGraph; +use petgraph::visit::Visitable; +use petgraph::Direction; +use reqwest::Client; +use rowan::GreenNode; +use tokio::runtime::Handle; +use tokio::sync::mpsc::unbounded_channel; +use tokio::sync::mpsc::UnboundedReceiver; +use tokio::sync::mpsc::UnboundedSender; +use tokio::sync::oneshot; +use tokio::task::JoinHandle; +use url::Url; +use wdl_ast::Ast; +use wdl_ast::AstNode; +use wdl_ast::AstToken; +use wdl_ast::Diagnostic; +use wdl_ast::SyntaxNode; +use wdl_ast::Validator; + +use crate::rayon::RayonHandle; +use crate::Document; +use crate::DocumentGraph; +use crate::DocumentId; +use crate::DocumentScope; + +/// The minimum number of milliseconds between analysis progress reports. +const MINIMUM_PROGRESS_MILLIS: u128 = 50; + +/// Represents the kind of analysis progress being reported. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ProgressKind { + /// The progress is for parsing documents. + Parsing, + /// The progress is for analyzing documents. + Analyzing, +} + +impl fmt::Display for ProgressKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Parsing => write!(f, "parsing"), + Self::Analyzing => write!(f, "analyzing"), + } + } +} + +/// Represents analysis state. +#[derive(Debug, Default)] + +pub(crate) struct State { + /// The document graph being built. + pub(crate) graph: DocumentGraph, + /// Represents dependency edges that, if they were added to the document + /// graph, would form a cycle. + /// + /// The first in the pair is the importing node and the second is the + /// imported node. + /// + /// This is used to break import cycles; when analyzing the document, if the + /// import exists in this set, a diagnostic will be added and the import + /// otherwise ignored. + pub(crate) cycles: HashSet<(NodeIndex, NodeIndex)>, + /// Space for DFS operations on the document graph. + space: DfsSpace as Visitable>::Map>, +} + +/// Represents the type for progress callbacks. +type ProgressCallback = dyn Fn(ProgressKind, usize, usize) + Send + Sync; + +/// Represents a request to perform analysis. +/// +/// This request is sent to the analysis queue for processing. +struct AnalysisRequest { + /// The identifiers of the documents to analyze. + documents: Vec>, + /// The progress callback to use for the request. + progress: Option>, + /// The sender for completing the analysis request. + completed: oneshot::Sender>, +} + +/// Represents the result of an analysis. +/// +/// Analysis results are cheap to clone. +#[derive(Debug, Clone)] +pub struct AnalysisResult { + /// The id of the analyzed document. + id: Arc, + /// The root node of the document. + /// + /// This is `None` if the document failed to be read. + root: Option, + /// The error encountered when trying to read the document. + /// + /// This is `None` if the document was read. + error: Option>, + /// The diagnostics for the document. + diagnostics: Arc<[Diagnostic]>, + /// The scope of the analyzed document. + scope: Arc, +} + +impl AnalysisResult { + /// Constructs a new analysis result for the given document. + pub(crate) fn new(document: &Document) -> Self { + let state = document.state.completed(); + Self { + id: document.id.clone(), + root: document.root.clone(), + error: document.error.clone(), + diagnostics: state.diagnostics.clone(), + scope: state.scope.clone(), + } + } + + /// Gets the identifier of the document that was analyzed. + pub fn id(&self) -> &DocumentId { + &self.id + } + + /// Gets the root node of the document that was analyzed. + /// + /// Returns `None` if the document could not be read. + pub fn root(&self) -> Option<&GreenNode> { + self.root.as_ref() + } + + /// Gets the error if the document could not be read. + /// + /// Returns `None` if the document was read. + pub fn error(&self) -> Option<&anyhow::Error> { + self.error.as_deref() + } + + /// Gets the diagnostics associated with the document. + pub fn diagnostics(&self) -> &[Diagnostic] { + &self.diagnostics + } + + /// Gets the scope of the analyzed document. + pub fn scope(&self) -> &DocumentScope { + &self.scope + } +} + +/// Represents a Workflow Description Language (WDL) analysis engine. +/// +/// By default, analysis parses documents, performs validation checks, resolves +/// imports, and performs type checking. +/// +/// Each analysis operation is processed in order of request; however, the +/// individual parsing, resolution, and analysis of documents is performed +/// across a thread pool. +#[derive(Debug)] +pub struct AnalysisEngine { + /// The document graph. + graph: Arc>, + /// The sender for sending analysis requests. + sender: UnboundedSender, + /// The join handle of the queue task. + queue: JoinHandle<()>, +} + +impl AnalysisEngine { + /// Creates a new analysis engine using a default validator. + /// + /// The engine must be constructed from the context of a Tokio runtime. + pub fn new() -> Result { + let graph: Arc> = Default::default(); + let (sender, queue) = Self::spawn_analysis_queue_task(graph.clone(), None); + Ok(Self { + graph, + sender, + queue, + }) + } + + /// Creates a new analysis engine with the given function that produces a + /// validator to use. + /// + /// The provided function will be called once per worker thread to + /// initialize a thread-local validator. + /// + /// The engine must be constructed from the context of a Tokio runtime. + pub fn new_with_validator(validator: V) -> Result + where + V: Fn() -> Validator + Send + Sync + 'static, + { + let graph: Arc> = Default::default(); + let (sender, queue) = + Self::spawn_analysis_queue_task(graph.clone(), Some(Arc::new(validator))); + Ok(Self { + graph, + sender, + queue, + }) + } + + /// Analyzes the given file system path. + /// + /// If the path is a directory, the directory will be recursively searched + /// for files with a `.wdl` extension to analyze. + /// + /// Otherwise, a single file is analyzed. + pub async fn analyze(&self, path: &Path) -> Vec { + let documents = Self::find_documents(path).await; + if documents.is_empty() { + log::info!( + "no WDL documents were found for path `{path}`", + path = path.display() + ); + return Vec::new(); + } + + let (tx, rx) = oneshot::channel(); + self.sender + .send(AnalysisRequest { + documents, + progress: None, + completed: tx, + }) + .expect("failed to send analysis request"); + + rx.await.expect("failed to receive analysis results") + } + + /// Analyzes the given file system path and reports progress to the given + /// callback. + /// + /// If the path is a directory, the directory will be recursively searched + /// for files with a `.wdl` extension to analyze. + /// + /// Otherwise, a single file is analyzed. + /// + /// Progress is reported to the provided callback function with a minimum + /// 50ms interval. + pub async fn analyze_with_progress(&self, path: &Path, progress: F) -> Vec + where + F: Fn(ProgressKind, usize, usize) + Send + Sync + 'static, + { + let documents = Self::find_documents(path).await; + if documents.is_empty() { + log::info!( + "no WDL documents were found for path `{path}`", + path = path.display() + ); + return Vec::new(); + } + + let (tx, rx) = oneshot::channel(); + self.sender + .send(AnalysisRequest { + documents, + progress: Some(Box::new(progress)), + completed: tx, + }) + .expect("failed to send analysis request"); + + rx.await.expect("failed to receive analysis results") + } + + /// Gets a previous analysis result for a file. + /// + /// Returns `None` if the file has not been analyzed yet. + pub fn result(&self, path: &Path) -> Option { + let id = DocumentId::try_from(path).ok()?; + let graph = self.graph.read(); + let index = graph.indexes.get(&id)?; + Some(AnalysisResult::new(&graph.inner[*index])) + } + + /// Shuts down the engine and waits for outstanding requests to complete. + pub async fn shutdown(self) { + drop(self.sender); + self.queue.await.expect("expected the queue to shut down"); + } + + /// Spawns the analysis queue task. + fn spawn_analysis_queue_task( + graph: Arc>, + validator: Option Validator + Send + Sync>>, + ) -> (UnboundedSender, JoinHandle<()>) { + let (tx, rx) = unbounded_channel::(); + let handle = tokio::spawn(Self::process_analysis_queue(graph, validator, rx)); + (tx, handle) + } + + /// Processes the analysis queue. + /// + /// The queue task processes analysis requests in the order of insertion + /// into the queue. + /// + /// It is also the only writer to the shared document graph. + async fn process_analysis_queue( + graph: Arc>, + validator: Option Validator + Send + Sync>>, + mut receiver: UnboundedReceiver, + ) { + log::info!("analysis queue has started"); + + let client = Client::default(); + while let Some(request) = receiver.recv().await { + log::info!( + "received request to analyze {count} document(s)", + count = request.documents.len() + ); + + // We start by populating the parse set with the request documents + // After each parse set completes, we search for imports to add to the parse set + // and continue until the parse set is empty; once the graph is built, we spawn + // analysis tasks to process every node in the graph. + let start = Instant::now(); + let mut state = State::default(); + let mut parse_set = request.documents.into_iter().collect::>(); + let mut requested = true; + let handle = Handle::current(); + while !parse_set.is_empty() { + let tasks = parse_set + .iter() + .map(|id| { + Self::spawn_parse_task(&handle, &client, &validator, id.clone(), requested) + }) + .collect::>(); + + // The remaining files to parse were not part of the request + requested = false; + + let parsed = Self::await_with_progress::<_, _, Vec<_>>( + ProgressKind::Parsing, + tasks, + &request.progress, + ) + .await; + + parse_set.clear(); + (state, parse_set) = Self::add_import_dependencies(state, parsed, parse_set).await; + } + + let total = state.graph.inner.node_count(); + let state = Self::spawn_analysis_tasks(state, &request.progress).await; + + // Spawn a task for merging the graph as this takes a lock + let graph = graph.clone(); + let results = RayonHandle::spawn(move || { + log::info!("merging document graphs"); + let mut graph = graph.write(); + graph.merge(state.graph) + }) + .await; + + log::info!( + "analysis request completed with {total} document(s) analyzed in {elapsed:?}", + elapsed = start.elapsed() + ); + + request + .completed + .send(results) + .expect("failed to send analysis results"); + } + + log::info!("analysis queue has shut down"); + } + + /// Finds documents for the given path. + /// + /// If the path is a directory, it is searched for `.wdl` files. + /// + /// Otherwise, returns a single identifier for the given path. + async fn find_documents(path: &Path) -> Vec> { + if path.is_dir() { + let pattern = format!("{path}/**/*.wdl", path = path.display()); + return RayonHandle::spawn(move || { + let options = glob::MatchOptions { + case_sensitive: true, + require_literal_separator: false, + require_literal_leading_dot: true, + }; + + match glob::glob_with(&pattern, options) { + Ok(paths) => paths + .filter_map(|p| match p { + Ok(path) => Some(Arc::new(DocumentId::try_from(path.as_path()).ok()?)), + Err(e) => { + log::error!("error while searching for WDL documents: {e}"); + None + } + }) + .collect(), + Err(e) => { + log::error!("error while searching for WDL documents: {e}"); + Vec::new() + } + } + }) + .await; + } + + DocumentId::try_from(path) + .map(|id| vec![Arc::new(id)]) + .unwrap_or_default() + } + + /// Awaits the given set of futures while providing progress to the given + /// callback. + async fn await_with_progress( + kind: ProgressKind, + tasks: FuturesUnordered, + progress: &Option>, + ) -> C + where + T: Future, + C: Extend + Default, + { + if tasks.is_empty() { + return Default::default(); + } + + let total = tasks.len(); + if let Some(progress) = &progress { + progress(kind, 0, total); + } + + let mut completed = 0; + let mut last_progress = Instant::now(); + let collection = tasks + .map(|r| { + completed += 1; + + if let Some(progress) = progress { + let now = Instant::now(); + if completed < total + && (now - last_progress).as_millis() > MINIMUM_PROGRESS_MILLIS + { + log::info!("{completed} out of {total} {kind} task(s) have completed"); + last_progress = now; + progress(kind, completed, total); + } + } + + r + }) + .collect() + .await; + + log::info!("{total} {kind} task(s) have completed"); + if let Some(progress) = &progress { + progress(kind, total, total); + } + + collection + } + + /// Spawns a parse task on a rayon thread. + fn spawn_parse_task( + handle: &Handle, + client: &Client, + validator: &Option Validator + Send + Sync>>, + id: Arc, + requested: bool, + ) -> RayonHandle { + thread_local! { + static VALIDATOR: RefCell> = const { RefCell::new(None) }; + } + + let handle = handle.clone(); + let client = client.clone(); + let validator = validator.clone(); + RayonHandle::spawn(move || { + VALIDATOR.with_borrow_mut(|v| { + let validator = v.get_or_insert_with(|| validator.map(|v| v()).unwrap_or_default()); + match Self::parse(&handle, &client, Some(validator), &id) { + Ok((root, diagnostics)) => { + Document::from_parse(id, root, diagnostics, requested) + } + Err(e) => { + log::warn!("{e:#}"); + Document::from_error(id, e, requested) + } + } + }) + }) + } + + /// Parses the given document by URI. + /// + /// If the URI is `http` or `https` scheme, it fetches the source from the + /// network. + /// + /// If the URI is `file` scheme, it reads the file from the local file + /// system. + /// + /// Returns the root node and diagnostics upon success or a single document + /// if there was a problem with accessing the document's source. + fn parse( + tokio: &Handle, + client: &Client, + validator: Option<&mut Validator>, + id: &DocumentId, + ) -> Result<(GreenNode, Vec)> { + let source = match id { + DocumentId::Path(path) => fs::read_to_string(path)?, + DocumentId::Uri(uri) => match uri.scheme() { + "https" | "http" => Self::download_source(tokio, client, uri)?, + "file" => { + let path = uri + .to_file_path() + .map_err(|_| anyhow!("invalid file URI `{uri}`"))?; + log::info!("reading document `{path}`", path = path.display()); + fs::read_to_string(&path)? + } + scheme => { + bail!("unsupported URI scheme `{scheme}`"); + } + }, + }; + + let (node, diagnostics) = Self::parse_source(id, &source, validator); + Ok((node, diagnostics)) + } + + /// Parses the given source and validates the result with the given + /// validator. + fn parse_source( + id: &DocumentId, + source: &str, + validator: Option<&mut Validator>, + ) -> (GreenNode, Vec) { + let start = Instant::now(); + let (document, mut diagnostics) = wdl_ast::Document::parse(source); + if let Some(validator) = validator { + diagnostics.extend(validator.validate(&document).err().unwrap_or_default()); + } + log::info!("parsing of `{id}` completed in {:?}", start.elapsed()); + (document.syntax().green().into(), diagnostics) + } + + /// Downloads the source of a `http` or `https` scheme URI. + /// + /// This makes a request on the provided tokio runtime to download the + /// source. + fn download_source(tokio: &Handle, client: &Client, uri: &Url) -> Result { + /// The timeout for downloading the source, in seconds. + const TIMEOUT_IN_SECS: u64 = 30; + + log::info!("downloading source from `{uri}`"); + + // TODO: we should be caching these responses on disk somewhere + tokio.block_on(async { + let resp = client + .get(uri.as_str()) + .timeout(Duration::from_secs(TIMEOUT_IN_SECS)) + .send() + .await?; + + let code = resp.status(); + if !code.is_success() { + bail!("server returned HTTP status {code}"); + } + + resp.text().await.context("failed to read response body") + }) + } + + /// Adds import dependencies of parsed documents to the state. + /// + /// This will add empty nodes to the graph for any missing imports and + /// populate the parse set with documents that need to be parsed. + async fn add_import_dependencies( + mut state: State, + parsed: Vec, + mut parse_set: HashSet>, + ) -> (State, HashSet>) { + RayonHandle::spawn(move || { + for document in parsed { + // Add the newly parsed document to the graph; if the document was previously + // added as an import dependency, it is replaced with the newly parsed document + let id = document.id.clone(); + state.graph.add_document(document); + + let (doc_index, document) = state + .graph + .document(&id) + .expect("document was just added to the state"); + let root = match &document.root { + Some(root) => root, + None => continue, + }; + + match wdl_ast::Document::cast(SyntaxNode::new_root(root.clone())) + .expect("root should cast") + .ast() + { + Ast::Unsupported => {} + Ast::V1(ast) => { + for import in ast.imports() { + let text = match import.uri().text() { + Some(text) => text, + None => continue, + }; + + let import_id = match DocumentId::relative_to(&id, text.as_str()) { + Ok(id) => Arc::new(id), + Err(_) => continue, + }; + + match state.graph.document(&import_id) { + Some((dep_index, _)) => { + // The dependency is already in the graph, so add a dependency + // edge; however, we must detect a cycle before doing so + if has_path_connecting( + &state.graph.inner, + doc_index, + dep_index, + Some(&mut state.space), + ) { + // Adding the edge would cause a cycle, so record the cycle + // instead + log::info!( + "an import cycle was detected between `{id}` and \ + `{import_id}`" + ); + state.cycles.insert((doc_index, dep_index)); + } else { + // The edge won't cause a cycle, so add it + log::info!( + "updating dependency edge from `{id}` to `{import_id}`" + ); + state.graph.inner.update_edge(dep_index, doc_index, ()); + } + } + None => { + // The dependency isn't in the graph; add a new node and + // dependency edge + log::info!( + "updating dependency edge from `{id}` to `{import_id}` \ + (added to parse queue)" + ); + let dep_index = state + .graph + .add_document(Document::new(import_id.clone(), false)); + state.graph.inner.update_edge(dep_index, doc_index, ()); + parse_set.insert(import_id); + } + } + } + } + } + } + + (state, parse_set) + }) + .await + } + + /// Spawns analysis tasks. + /// + /// Analysis tasks are spawned in topological order. + async fn spawn_analysis_tasks(state: State, progress: &Option>) -> State { + // As we're going to be analyzing on multiple threads, wrap the state with a + // `RwLock`. + let mut state = Arc::new(RwLock::new(state)); + let mut remaining: Option, ()>> = None; + let mut set = Vec::new(); + while remaining + .as_ref() + .map(|g| g.node_count() > 0) + .unwrap_or(true) + { + (state, remaining, set) = RayonHandle::spawn(move || { + // Insert a copy of the graph where we just map the nodes to the document + // identifiers; we need a copy as we are going to be removing nodes from the + // graph as we process them in topological order + let g = remaining.get_or_insert_with(|| { + state.read().graph.inner.map(|_, n| n.id.clone(), |_, _| ()) + }); + + // Build a set of nodes with no incoming edges + set.clear(); + for node in g.node_indices() { + if g.edges_directed(node, Direction::Incoming).next().is_none() { + set.push(node); + } + } + + // Remove the nodes we're about to analyze from the "remaining" graph + // This also removes the outgoing edges from those nodes + for index in &set { + g.remove_node(*index); + } + + (state, remaining, set) + }) + .await; + + let tasks = set + .iter() + .map(|index| { + let index = *index; + let state = state.clone(); + RayonHandle::spawn(move || Self::analyze_node(state, index)) + }) + .collect::>(); + + Self::await_with_progress::<_, _, Vec<_>>(ProgressKind::Analyzing, tasks, progress) + .await; + } + + // We're finished with the tasks; there should be no outstanding references to + // the state + Arc::into_inner(state) + .expect("only one reference should remain") + .into_inner() + } + + /// Analyzes a node in the document graph. + /// + /// This completes the analysis state of the node. + fn analyze_node(state: Arc>, index: NodeIndex) { + let (id, root) = { + // scope for read lock + let state = state.read(); + let node = &state.graph.inner[index]; + (node.id.clone(), node.root.clone()) + }; + + log::info!("analyzing `{id}`"); + let start = Instant::now(); + let (scope, diagnostics) = if let Some(root) = root { + let document = + wdl_ast::Document::cast(SyntaxNode::new_root(root)).expect("root should cast"); + let state = state.read(); + DocumentScope::new(&state, &id, &document) + } else { + (Default::default(), Default::default()) + }; + + { + // Scope for write lock + // Write the result of the analysis to the document + let mut state = state.write(); + let doc = &mut state.graph.inner[index]; + let state = doc.state.in_progress(); + + state.scope = scope; + if !diagnostics.is_empty() { + state.diagnostics.extend(diagnostics); + } + + // Complete the analysis of the document + doc.complete(); + } + + log::info!( + "analysis of `{id}` completed in {elapsed:?}", + elapsed = start.elapsed() + ) + } +} + +/// Constant that asserts `AnalysisEngine` is `Send + Sync`; if not, it fails to +/// compile. +const _: () = { + /// Helper that will fail to compile if T is not `Send + Sync`. + const fn _assert() {} + _assert::(); +}; diff --git a/wdl-analysis/src/graph.rs b/wdl-analysis/src/graph.rs new file mode 100644 index 00000000..2beaff9d --- /dev/null +++ b/wdl-analysis/src/graph.rs @@ -0,0 +1,401 @@ +//! Representation of the analysis document graph. + +use std::borrow::Cow; +use std::cmp::Ordering; +use std::collections::HashMap; +use std::fmt; +use std::mem; +use std::path::absolute; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; + +use anyhow::Context; +use anyhow::Result; +use path_clean::clean; +use petgraph::graph::NodeIndex; +use petgraph::stable_graph::StableDiGraph; +use petgraph::visit::EdgeRef; +use petgraph::Direction; +use rowan::GreenNode; +use url::Url; +use wdl_ast::Diagnostic; + +use crate::AnalysisResult; +use crate::DocumentScope; + +/// Represents the identifier of an analyzed document. +#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] +pub enum DocumentId { + /// The identifier is by absolute file path. + Path(PathBuf), + /// The identifier is by URI. + Uri(Url), +} + +impl DocumentId { + /// Makes a document identifier relative to another. + pub(crate) fn relative_to(base: &DocumentId, id: &str) -> Result { + if let Ok(uri) = id.parse() { + return Ok(Self::Uri(uri)); + } + + match base { + Self::Path(base) => Ok(Self::Path(clean( + base.parent().expect("expected a parent").join(id), + ))), + Self::Uri(base) => Ok(Self::Uri(base.join(id)?)), + } + } + + /// Gets the path of the document. + /// + /// Returns `None` if the document does not have a local path. + pub fn path(&self) -> Option> { + match self { + Self::Path(path) => Some(path.into()), + Self::Uri(uri) => uri.to_file_path().ok().map(Into::into), + } + } + + /// Gets the document identifier as a string. + pub fn to_str(&self) -> Cow<'_, str> { + match self { + DocumentId::Path(p) => p.to_string_lossy(), + DocumentId::Uri(u) => u.as_str().into(), + } + } +} + +impl fmt::Display for DocumentId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + DocumentId::Path(path) => write!(f, "{}", path.display()), + DocumentId::Uri(uri) => write!(f, "{}", uri), + } + } +} + +impl TryFrom<&Path> for DocumentId { + type Error = anyhow::Error; + + fn try_from(value: &Path) -> Result { + Ok(Self::Path(clean(absolute(value).with_context(|| { + format!( + "failed to determine the absolute path of `{path}`", + path = value.display() + ) + })?))) + } +} + +impl TryFrom<&str> for DocumentId { + type Error = anyhow::Error; + + fn try_from(value: &str) -> Result { + match Url::parse(value) { + Ok(uri) => Ok(Self::Uri(uri)), + Err(_) => Self::try_from(Path::new(value)), + } + } +} + +impl From for DocumentId { + fn from(value: Url) -> Self { + Self::Uri(value) + } +} + +/// Represents the in-progress analysis state for a document. +#[derive(Debug, Default)] +pub(crate) struct InProgressAnalysisState { + /// The diagnostics of the document. + pub diagnostics: Vec, + /// The document's scope. + pub scope: DocumentScope, +} + +/// Represents the completed analysis state of a document. +#[derive(Debug)] +pub(crate) struct CompletedAnalysisState { + /// The diagnostics of the document. + pub diagnostics: Arc<[Diagnostic]>, + /// The document's scope. + pub scope: Arc, +} + +impl From for CompletedAnalysisState { + fn from(value: InProgressAnalysisState) -> Self { + let mut diagnostics = value.diagnostics; + diagnostics.sort_by(|a, b| match (a.labels().next(), b.labels().next()) { + (None, None) => Ordering::Equal, + (None, Some(_)) => Ordering::Less, + (Some(_), None) => Ordering::Greater, + (Some(a), Some(b)) => a.span().start().cmp(&b.span().start()), + }); + Self { + diagnostics: diagnostics.into(), + scope: value.scope.into(), + } + } +} + +/// Represents the state of a document's analysis. +#[derive(Debug)] +pub(crate) enum AnalysisState { + /// The analysis is in-progress and the data is mutable. + InProgress(InProgressAnalysisState), + /// The analysis has completed and the data is immutable. + Completed(CompletedAnalysisState), +} + +impl AnalysisState { + /// Gets the mutable in-progress analysis state. + /// + /// # Panics + /// + /// Panics if the analysis has completed. + pub(crate) fn in_progress(&mut self) -> &mut InProgressAnalysisState { + match self { + Self::InProgress(state) => state, + Self::Completed(_) => panic!("analysis has completed"), + } + } + + /// Gets the immutable completed analysis state. + /// + /// # Panics + /// + /// Panics if the analysis has not completed. + pub(crate) fn completed(&self) -> &CompletedAnalysisState { + match self { + Self::InProgress(_) => panic!("analysis has not completed"), + Self::Completed(state) => state, + } + } + + /// Completes the analysis state. + /// + /// # Panics + /// + /// Panics if the analysis has already completed. + fn complete(&mut self) { + match self { + Self::InProgress(state) => { + *self = Self::Completed(mem::take(state).into()); + } + Self::Completed(_) => panic!("analysis has completed"), + } + } +} + +impl Default for AnalysisState { + fn default() -> Self { + Self::InProgress(Default::default()) + } +} + +/// Represents an analyzed document. +#[derive(Debug)] +pub(crate) struct Document { + /// The identifier of the analyzed document. + pub id: Arc, + /// The root node of the document. + /// + /// If `None`, it means we failed to read the document's source. + pub root: Option, + /// The error when attempting to read the document's source. + /// + /// This is `Some` if we failed to read the document's source. + pub error: Option>, + /// The analysis state of the document. + pub state: AnalysisState, + /// Whether or not this document is a GC root in the document graph. + /// + /// A GC root won't be removed from the document graph even if there are no + /// outgoing edges. + pub gc_root: bool, +} + +impl Document { + /// Creates a new empty document. + pub fn new(id: Arc, gc_root: bool) -> Self { + Self { + id, + root: None, + error: None, + state: Default::default(), + gc_root, + } + } + + /// Creates a new document from the result of a parse. + pub fn from_parse( + id: Arc, + root: GreenNode, + diagnostics: Vec, + gc_root: bool, + ) -> Self { + Self { + id, + root: Some(root), + error: None, + state: AnalysisState::InProgress(InProgressAnalysisState { + diagnostics, + ..Default::default() + }), + gc_root, + } + } + + /// Creates a new document from an error attempting to read the document. + pub fn from_error(id: Arc, error: anyhow::Error, gc_root: bool) -> Self { + Self { + id, + root: None, + error: Some(Arc::new(error)), + state: Default::default(), + gc_root, + } + } + + /// Called to complete the analysis on the document. + pub fn complete(&mut self) { + self.state.complete(); + } +} + +/// Represents a document graph. +#[derive(Debug, Default)] +pub(crate) struct DocumentGraph { + /// The inner graph. + /// + /// Each node in the graph represents an analyzed file and edges denote + /// import dependency relationships. + pub inner: StableDiGraph, + /// Map from document identifier to graph node index. + pub indexes: HashMap, NodeIndex>, +} + +impl DocumentGraph { + /// Gets a document from the graph. + pub fn document(&self, id: &DocumentId) -> Option<(NodeIndex, &Document)> { + self.indexes + .get(id) + .map(|index| (*index, &self.inner[*index])) + } + + /// Adds a document to the graph. + /// + /// If the document with the same identifier exists in the graph, it is + /// replaced. + pub fn add_document(&mut self, document: Document) -> NodeIndex { + if let Some(index) = self.indexes.get(&document.id) { + self.inner[*index] = document; + return *index; + } + + let id = document.id.clone(); + let index = self.inner.add_node(document); + let prev = self.indexes.insert(id, index); + assert!(prev.is_none()); + index + } + + /// Merges this document graph with the provided one. + /// + /// Returns the result of the analysis. + /// + /// This also performs a GC on the graph to remove non-rooted nodes that + /// have no outgoing edges. + pub fn merge(&mut self, mut other: Self) -> Vec { + let mut remapped = HashMap::new(); + let mut results = Vec::new(); + for (id, other_index) in other.indexes { + let Document { + id: _, + root, + error, + state, + gc_root, + } = &mut other.inner[other_index]; + match self.indexes.get(&id) { + Some(index) => { + remapped.insert(other_index, *index); + + // Existing node, so replace the document contents + let existing = &mut self.inner[*index]; + *existing = Document { + id, + root: mem::take(root), + error: mem::take(error), + state: mem::take(state), + gc_root: existing.gc_root | *gc_root, + }; + + // Add a result for root documents or non-root documents that parsed + if *gc_root || existing.root.is_some() { + results.push(AnalysisResult::new(existing)); + } + + // Remove all edges to this node in self; we'll add the latest edges below. + for edge in self.inner.edges(*index).map(|e| e.id()).collect::>() { + self.inner.remove_edge(edge); + } + } + None => { + let document = Document { + id: id.clone(), + root: mem::take(root), + error: mem::take(error), + state: mem::take(state), + gc_root: *gc_root, + }; + + // Add a result for root documents or non-root documents that parsed + if *gc_root || document.root.is_some() { + results.push(AnalysisResult::new(&document)); + } + + // New node, insert it into the graph + let index = self.inner.add_node(document); + + remapped.insert(other_index, index); + self.indexes.insert(id, index); + } + } + } + + // Now add the edges for the remapped nodes + for edge in other.inner.edge_indices() { + let (from, to) = other.inner.edge_endpoints(edge).expect("edge should exist"); + let from = remapped[&from]; + let to = remapped[&to]; + self.inner.add_edge(from, to, ()); + } + + // Finally, GC any non-gc-root nodes that have no outgoing edges + let mut gc = Vec::new(); + for node in self.inner.node_indices() { + if self.inner[node].gc_root { + continue; + } + + if self + .inner + .edges_directed(node, Direction::Outgoing) + .next() + .is_none() + { + gc.push(node); + } + } + + for node in gc { + self.inner.remove_node(node); + } + + results.sort_by(|a, b| a.id().cmp(b.id())); + results + } +} diff --git a/wdl-analysis/src/lib.rs b/wdl-analysis/src/lib.rs new file mode 100644 index 00000000..b4b689aa --- /dev/null +++ b/wdl-analysis/src/lib.rs @@ -0,0 +1,19 @@ +//! Analysis of Workflow Description Language (WDL) documents. +//! +//! An analyzer can be used to implement the [Language Server Protocol (LSP)](https://microsoft.github.io/language-server-protocol/). + +#![warn(missing_docs)] +#![warn(rust_2018_idioms)] +#![warn(rust_2021_compatibility)] +#![warn(missing_debug_implementations)] +#![warn(clippy::missing_docs_in_private_items)] +#![warn(rustdoc::broken_intra_doc_links)] + +mod engine; +mod graph; +mod rayon; +mod scope; + +pub use engine::*; +pub use graph::*; +pub use scope::*; diff --git a/wdl-analysis/src/rayon.rs b/wdl-analysis/src/rayon.rs new file mode 100644 index 00000000..10778582 --- /dev/null +++ b/wdl-analysis/src/rayon.rs @@ -0,0 +1,48 @@ +//! Helper module for integration of rayon tasks with Tokio. + +use std::future::Future; +use std::pin::Pin; +use std::task::Context; +use std::task::Poll; + +use tokio::sync::oneshot; +use tokio::sync::oneshot::Receiver; + +/// Represents a handle from spawning a task on the rayon thread pool. +/// +/// Awaiting the handle returns the result of the spawned task. +#[must_use] +#[derive(Debug)] +pub struct RayonHandle { + /// The receiver that is notified when the rayon task completes. + rx: Receiver, +} + +impl RayonHandle +where + T: Send + 'static, +{ + /// Spawns a task on the rayon thread pool. + /// + /// The provided function will run on a rayon thread. + /// + /// The return handle must be awaited. + pub fn spawn T + Send + 'static>(func: F) -> Self { + let (tx, rx) = oneshot::channel(); + rayon::spawn(move || { + tx.send(func()).ok(); + }); + + Self { rx } + } +} + +impl Future for RayonHandle { + type Output = T; + + fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { + let rx = Pin::new(&mut self.rx); + rx.poll(cx) + .map(|result| result.expect("failed to receive from oneshot channel")) + } +} diff --git a/wdl-analysis/src/scope.rs b/wdl-analysis/src/scope.rs new file mode 100644 index 00000000..3d2aad87 --- /dev/null +++ b/wdl-analysis/src/scope.rs @@ -0,0 +1,1281 @@ +//! Implementation of scopes for WDL documents. + +use std::fmt; +use std::sync::Arc; + +use indexmap::IndexMap; +use rowan::GreenNode; +use wdl_ast::support::token; +use wdl_ast::v1; +use wdl_ast::v1::ImportStatement; +use wdl_ast::v1::StringPart; +use wdl_ast::v1::WorkflowStatement; +use wdl_ast::Ast; +use wdl_ast::AstNode; +use wdl_ast::AstToken; +use wdl_ast::Diagnostic; +use wdl_ast::Ident; +use wdl_ast::Span; +use wdl_ast::SyntaxElement; +use wdl_ast::SyntaxKind; +use wdl_ast::SyntaxNode; +use wdl_ast::ToSpan; +use wdl_ast::Version; + +use crate::DocumentId; +use crate::State; + +/// Represents the context of a name for diagnostic reporting. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NameContext { + /// The name is a workflow name. + Workflow(Span), + /// The name is a task name. + Task(Span), + /// The name is a struct name. + Struct(Span), + /// The name is a struct member name. + StructMember(Span), + /// A name local to a scope. + Scoped(ScopedNameContext), +} + +impl NameContext { + /// Gets the span of the name. + fn span(&self) -> Span { + match self { + Self::Workflow(s) => *s, + Self::Task(s) => *s, + Self::Struct(s) => *s, + Self::StructMember(s) => *s, + Self::Scoped(n) => n.span(), + } + } +} + +impl fmt::Display for NameContext { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Workflow(_) => write!(f, "workflow"), + Self::Task(_) => write!(f, "task"), + Self::Struct(_) => write!(f, "struct"), + Self::StructMember(_) => write!(f, "struct member"), + Self::Scoped(n) => n.fmt(f), + } + } +} + +/// Creates a "placeholder in import" diagnostic +fn placeholder_in_import(span: Span) -> Diagnostic { + Diagnostic::error("import URI cannot contain placeholders") + .with_label("remove this placeholder", span) +} + +/// Creates a "name conflict" diagnostic +fn name_conflict(name: &str, conflicting: NameContext, first: NameContext) -> Diagnostic { + Diagnostic::error(format!("conflicting {conflicting} name `{name}`")) + .with_label( + format!("this conflicts with a {first} of the same name"), + conflicting.span(), + ) + .with_label( + format!("the {first} with the conflicting name is here"), + first.span(), + ) +} + +/// Creates a "namespace conflict" diagnostic +fn namespace_conflict(name: &str, conflicting: Span, first: Span, suggest_fix: bool) -> Diagnostic { + let diagnostic = Diagnostic::error(format!("conflicting import namespace `{name}`")) + .with_label("this conflicts with another import namespace", conflicting) + .with_label( + "the conflicting import namespace was introduced here", + first, + ); + + if suggest_fix { + diagnostic.with_fix("add an `as` clause to the import to specify a namespace") + } else { + diagnostic + } +} + +/// Creates an "invalid import namespace" diagnostic +fn invalid_import_namespace(span: Span) -> Diagnostic { + Diagnostic::error("import namespace is not a valid WDL identifier") + .with_label("a namespace cannot be derived from this import path", span) + .with_fix("add an `as` clause to the import to specify a namespace") +} + +/// Creates an "import cycle" diagnostic +fn import_cycle(span: Span) -> Diagnostic { + Diagnostic::error("import introduces a dependency cycle") + .with_label("this import has been skipped to break the cycle", span) +} + +/// Creates an "import failure" diagnostic +fn import_failure(uri: &str, error: &anyhow::Error, span: Span) -> Diagnostic { + Diagnostic::error(format!("failed to import `{uri}`: {error:?}")).with_highlight(span) +} + +/// Creates an "incompatible import" diagnostic +fn incompatible_import( + import_version: &str, + import_span: Span, + importer_version: &Version, +) -> Diagnostic { + Diagnostic::error("imported document has incompatible version") + .with_label( + format!("the imported document is version `{import_version}`"), + import_span, + ) + .with_label( + format!( + "the importing document is version `{version}`", + version = importer_version.as_str() + ), + importer_version.span(), + ) +} + +/// Creates an "import missing version" diagnostic +fn import_missing_version(span: Span) -> Diagnostic { + Diagnostic::error("imported document is missing a version statement").with_highlight(span) +} + +/// Creates an "invalid relative import" diagnostic +fn invalid_relative_import(error: &anyhow::Error, span: Span) -> Diagnostic { + Diagnostic::error(format!("{error:?}")).with_highlight(span) +} + +/// Creates a "struct not in scope" diagnostic +fn struct_not_in_scope(name: &Ident) -> Diagnostic { + Diagnostic::error(format!( + "a struct named `{name}` does not exist in the imported document", + name = name.as_str() + )) + .with_label("this struct does not exist", name.span()) +} + +/// Creates an "imported struct conflict" diagnostic +fn imported_struct_conflict( + name: &str, + conflicting: Span, + first: Span, + suggest_fix: bool, +) -> Diagnostic { + let diagnostic = Diagnostic::error(format!("conflicting struct name `{name}`")) + .with_label( + "this import introduces a conflicting definition", + conflicting, + ) + .with_label("the first definition was introduced by this import", first); + + if suggest_fix { + diagnostic.with_fix("add an `alias` clause to the import to specify a different name") + } else { + diagnostic + } +} + +/// Creates a "struct conflicts with import" diagnostic +fn struct_conflicts_with_import(name: &str, conflicting: Span, import: Span) -> Diagnostic { + Diagnostic::error(format!("conflicting struct name `{name}`")) + .with_label("this name conflicts with an imported struct", conflicting) + .with_label("the import that introduced the struct is here", import) + .with_fix( + "either rename the struct or use an `alias` clause on the import with a different name", + ) +} + +/// Creates a "duplicate workflow" diagnostic +fn duplicate_workflow(name: &Ident, first: Span) -> Diagnostic { + Diagnostic::error(format!( + "cannot define workflow `{name}` as only one workflow is allowed per source file", + name = name.as_str(), + )) + .with_label("consider moving this workflow to a new file", name.span()) + .with_label("first workflow is defined here", first) +} + +/// Creates a "call conflict" diagnostic +fn call_conflict(name: &Ident, first: NameContext, suggest_fix: bool) -> Diagnostic { + let diagnostic = Diagnostic::error(format!( + "conflicting call name `{name}`", + name = name.as_str() + )) + .with_label( + format!("this conflicts with a {first} of the same name"), + name.span(), + ) + .with_label( + format!("the {first} with the conflicting name is here"), + first.span(), + ); + + if suggest_fix { + diagnostic.with_fix("add an `as` clause to the call to specify a different name") + } else { + diagnostic + } +} + +/// Represents the context of a name in a workflow or task scope. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScopedNameContext { + /// The name was introduced by an task or workflow input. + Input(Span), + /// The name was introduced by an task or workflow output. + Output(Span), + /// The name was introduced by a private declaration. + Decl(Span), + /// The name was introduced by a workflow call statement. + Call(Span), + /// The name was introduced by a variable in workflow scatter statement. + ScatterVariable(Span), +} + +impl ScopedNameContext { + /// Gets the span of the name. + pub fn span(&self) -> Span { + match self { + Self::Input(s) => *s, + Self::Output(s) => *s, + Self::Decl(s) => *s, + Self::Call(s) => *s, + Self::ScatterVariable(s) => *s, + } + } +} + +impl fmt::Display for ScopedNameContext { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Input(_) => write!(f, "input"), + Self::Output(_) => write!(f, "output"), + Self::Decl(_) => write!(f, "declaration"), + Self::Call(_) => write!(f, "call"), + Self::ScatterVariable(_) => write!(f, "scatter variable"), + } + } +} + +impl From for NameContext { + fn from(context: ScopedNameContext) -> Self { + Self::Scoped(context) + } +} + +/// Represents a name in a task or workflow scope. +#[derive(Debug, Clone)] +pub struct ScopedName { + /// The context of the name. + context: ScopedNameContext, + /// The CST node that introduced the name. + node: GreenNode, + /// Whether or not the name was implicitly introduced. + /// + /// This is true for names introduced in outer scopes from workflow scatter + /// and conditional statements. + implicit: bool, +} + +impl ScopedName { + /// Gets the context of the scoped name. + pub fn context(&self) -> ScopedNameContext { + self.context + } + + /// Gets the node of the scoped name. + /// + /// This may be a bound declaration, an unbound declaration, a workflow call + /// statement, or a workflow scatter statement. + pub fn node(&self) -> &GreenNode { + &self.node + } + + /// Whether or not the name was introduced implicitly into the scope. + /// + /// This is true for names introduced in outer scopes from workflow scatter + /// and conditional statements. + pub fn implicit(&self) -> bool { + self.implicit + } + + /// Determines if the name was introduced for a scatter variable. + fn is_scatter_variable(&self) -> bool { + if !self.implicit { + return matches!(self.context, ScopedNameContext::ScatterVariable(_)); + } + + false + } +} + +/// Represents a namespace introduced by an import. +#[derive(Debug)] +pub struct Namespace { + /// The span of the import that introduced the namespace. + span: Span, + /// The CST node of the import that introduced the namespace. + node: GreenNode, + /// The identifier of the imported document that introduced the namespace. + source: Arc, + /// The namespace's document scope. + scope: Arc, +} + +impl Namespace { + /// Gets the CST node that introduced the namespace. + /// + /// The node is an import statement. + pub fn node(&self) -> &GreenNode { + &self.node + } + + /// Gets the identifier of the imported document that introduced the + /// namespace. + pub fn source(&self) -> &DocumentId { + &self.source + } + + /// Gets the scope of the imported document. + pub fn scope(&self) -> &DocumentScope { + &self.scope + } +} + +/// Represents a struct in a document. +#[derive(Debug)] +pub struct Struct { + /// The span that introduced the struct. + /// + /// This is either the name of a struct definition (local) or an import's + /// URI or alias (imported). + span: Span, + /// The source document that defines the struct. + /// + /// This is `Some` only for imported structs. + source: Option>, + /// The CST node of the struct definition. + node: GreenNode, + /// The members of the struct. + members: Arc>, +} + +impl Struct { + /// Gets the CST node of the struct definition. + pub fn node(&self) -> &GreenNode { + &self.node + } + + /// Gets the source document that defines this struct. + /// + /// Returns `None` for structs defined in the containing scope or `Some` for + /// a struct introduced by an import. + pub fn source(&self) -> Option<&DocumentId> { + self.source.as_deref() + } + + /// Gets the members of the struct. + pub fn members(&self) -> impl Iterator { + self.members.iter().map(|(name, (_, node))| (name, node)) + } + + /// Gets a member of the struct by name. + pub fn get_member(&self, name: &str) -> Option<&GreenNode> { + self.members.get(name).map(|(_, n)| n) + } + + /// Compares two structs for structural equality. + fn is_equal(&self, other: &Self) -> bool { + for ((a_name, a_node), (b_name, b_node)) in self.members().zip(other.members()) { + if a_name != b_name { + return false; + } + + let adecl = v1::UnboundDecl::cast(SyntaxNode::new_root(a_node.clone())) + .expect("node should cast"); + let bdecl = v1::UnboundDecl::cast(SyntaxNode::new_root(b_node.clone())) + .expect("node should cast"); + if adecl.ty() != bdecl.ty() { + return false; + } + } + + true + } +} + +/// Represents a scope in a WDL document. +#[derive(Debug)] +pub struct Scope { + /// The span in the document where the names of the scope are visible. + span: Span, + /// The CST node that introduced the scope. + /// + /// This may be a struct, task, workflow, conditional statement, or scatter + /// statement. + node: GreenNode, + /// The names in the task scope. + names: IndexMap, + /// The child scopes of this scope. + /// + /// Child scopes are from workflow conditional and scatter statements. + children: Vec, +} + +impl Scope { + /// Gets the span where the names of the scope are visible. + pub fn span(&self) -> Span { + self.span + } + + /// Gets the CST node that introduced the scope. + /// + /// This may be a struct, task, workflow, conditional statement, or scatter + /// statement. + pub fn node(&self) -> &GreenNode { + &self.node + } + + /// Gets the names in the scope. + pub fn names(&self) -> impl Iterator { + self.names.iter() + } + + /// Gets a name within the scope. + pub fn get_name(&self, name: &str) -> Option<&ScopedName> { + self.names.get(name) + } + + /// Gets the child scopes of this scope. + /// + /// Child scopes may exist in workflows when conditional or scatter + /// statements are present. + pub fn children(&self) -> impl Iterator { + self.children.iter() + } + + /// Finds the deepest child scope by position within the document. + pub fn find_child_scope(&self, position: usize) -> Option<&Scope> { + let scope = match self + .children + .binary_search_by_key(&position, |s| s.span.start()) + { + Ok(index) => &self.children[index], + Err(insertion) => { + // This indicates that we couldn't find a match and the match would go _before_ + // the first child scope, so there is no corresponding scope. + if insertion == 0 { + return None; + } + + // Check to see if the span before the insertion point actually contains the + // position. + let child = &self.children[insertion - 1]; + if position - child.span.start() < child.span.len() { + return None; + } + + child + } + }; + + Some(scope.find_child_scope(position).unwrap_or(scope)) + } +} + +/// Represents context about a scope in a document. +#[derive(Debug, Clone, Copy)] +enum ScopeContext { + /// The scope is a task. + /// + /// The value is an index into the document's `tasks` collection. + Task(usize), + /// The scope is a workflow. + Workflow, +} + +/// Represents a task scope. +#[derive(Debug)] +struct TaskScope { + /// The span of the task name. + name_span: Span, + /// The scope of the task. + scope: Scope, +} + +/// Represents a workflow scope. +#[derive(Debug)] +struct WorkflowScope { + /// The span of the workflow name. + name_span: Span, + /// The name of the workflow. + name: String, + /// The scope of the task. + scope: Scope, +} + +/// Represents the scope of a document. +#[derive(Debug, Default)] +pub struct DocumentScope { + /// The namespaces in the document. + namespaces: IndexMap, + /// The tasks in the document. + tasks: IndexMap, + /// The singular workflow in the document. + workflow: Option, + /// The structs in the document. + structs: IndexMap, + /// A sorted list of scopes within the document. + /// + /// This can be used to quickly search for a scope by span. + scopes: Vec<(Span, ScopeContext)>, +} + +impl DocumentScope { + /// Creates a new document scope for a given document. + pub(crate) fn new( + state: &State, + id: &DocumentId, + document: &wdl_ast::Document, + ) -> (Self, Vec) { + let mut scope = Self::default(); + let mut diagnostics = Vec::new(); + let version = match document.version_statement() { + Some(stmt) => stmt.version(), + None => { + // Don't process a document with a missing version + return (scope, diagnostics); + } + }; + + match document.ast() { + Ast::Unsupported => {} + Ast::V1(ast) => { + for item in ast.items() { + match item { + v1::DocumentItem::Import(import) => { + scope.add_namespace(state, &import, id, &version, &mut diagnostics); + } + v1::DocumentItem::Struct(s) => { + scope.add_struct(&s, &mut diagnostics); + } + v1::DocumentItem::Task(task) => { + scope.add_task_scope(&task, &mut diagnostics); + } + v1::DocumentItem::Workflow(workflow) => { + scope.add_workflow_scope(&workflow, &mut diagnostics); + } + } + } + } + } + + (scope, diagnostics) + } + + /// Gets the namespaces in the document scope. + pub fn namespaces(&self) -> impl Iterator { + self.namespaces.iter() + } + + /// Gets a namespace in the document scope by name. + pub fn get_namespace(&self, name: &str) -> Option<&Namespace> { + self.namespaces.get(name) + } + + /// Gets the task scopes in the document scope. + pub fn task_scopes(&self) -> impl Iterator { + self.tasks.iter().map(|(n, s)| (n, &s.scope)) + } + + /// Gets a task scope in the document scope by name. + pub fn get_task_scope(&self, name: &str) -> Option<&Scope> { + self.tasks.get(name).map(|s| &s.scope) + } + + /// Gets the workflow scope in the document scope. + pub fn get_workflow_scope(&self) -> Option<&Scope> { + self.workflow.as_ref().map(|s| &s.scope) + } + + /// Gets the structs in the document scope. + pub fn structs(&self) -> impl Iterator { + self.structs.iter() + } + + /// Gets a struct in the document scope by name. + pub fn get_struct(&self, name: &str) -> Option<&Struct> { + self.structs.get(name) + } + + /// Finds the deepest scope based on a position within the document. + pub fn find_scope_by_position(&self, position: usize) -> Option<&Scope> { + let context = match self + .scopes + .binary_search_by_key(&position, |(s, _)| s.start()) + { + Ok(index) => self.scopes[index].1, + Err(insertion) => { + // This indicates that we couldn't find a match and the match would go _before_ + // the first scope, so there is no corresponding scope. + if insertion == 0 { + return None; + } + + // Check to see if the span before the insertion point actually contains the + // position. + let (span, context) = &self.scopes[insertion - 1]; + if position - span.start() < span.len() { + return None; + } + + *context + } + }; + + let scope = match context { + ScopeContext::Task(index) => &self.tasks[index].scope, + ScopeContext::Workflow => { + &self + .workflow + .as_ref() + .expect("expected a workflow scope") + .scope + } + }; + + Some(scope.find_child_scope(position).unwrap_or(scope)) + } + + /// Adds a namespace to the document scope. + fn add_namespace( + &mut self, + state: &State, + import: &ImportStatement, + importer_id: &DocumentId, + importer_version: &Version, + diagnostics: &mut Vec, + ) { + // Start by resolving the import to its document scope + let (id, scope) = match Self::resolve_import(state, import, importer_id, importer_version) { + Ok(scope) => scope, + Err(diagnostic) => { + diagnostics.push(diagnostic); + return; + } + }; + + // Check for conflicting namespaces + let span = import.uri().syntax().text_range().to_span(); + match import.namespace() { + Some((ns, span)) => { + if let Some(prev) = self.namespaces.get(&ns) { + diagnostics.push(namespace_conflict( + &ns, + span, + prev.span, + import.explicit_namespace().is_none(), + )); + return; + } else { + self.namespaces.insert( + ns, + Namespace { + span, + node: import.syntax().green().into(), + source: id.clone(), + scope: scope.clone(), + }, + ); + } + } + None => { + diagnostics.push(invalid_import_namespace(span)); + return; + } + } + + // Get the alias map for the structs in the document + let aliases = import + .aliases() + .filter_map(|a| { + let (from, to) = a.names(); + if !scope.structs.contains_key(from.as_str()) { + diagnostics.push(struct_not_in_scope(&from)); + return None; + } + + Some((from.as_str().to_string(), to)) + }) + .collect::>(); + + // Insert the scope's struct definitions + for (name, scope) in &scope.structs { + let (aliased_name, span, aliased) = aliases + .get(name) + .map(|a| (a.as_str(), a.span(), true)) + .unwrap_or_else(|| (name, span, false)); + match self.structs.get(aliased_name) { + Some(prev) => { + // Import conflicts with a struct defined in this document + if prev.source.is_none() { + diagnostics.push(struct_conflicts_with_import( + aliased_name, + prev.span, + span, + )); + continue; + } + + if !prev.is_equal(scope) { + diagnostics.push(imported_struct_conflict( + aliased_name, + span, + prev.span, + !aliased, + )); + continue; + } + } + None => { + self.structs.insert( + aliased_name.to_string(), + Struct { + span, + source: Some(scope.source.clone().unwrap_or(id.clone())), + node: scope.node.clone(), + members: scope.members.clone(), + }, + ); + } + } + } + } + + /// Adds a struct to the document scope. + fn add_struct(&mut self, definition: &v1::StructDefinition, diagnostics: &mut Vec) { + let name = definition.name(); + if let Some(prev) = self.structs.get(name.as_str()) { + if prev.source.is_some() { + diagnostics.push(struct_conflicts_with_import( + name.as_str(), + name.span(), + prev.span, + )) + } else { + diagnostics.push(name_conflict( + name.as_str(), + NameContext::Struct(name.span()), + NameContext::Struct(prev.span), + )); + } + } else { + let mut members = IndexMap::new(); + for decl in definition.members() { + let name = decl.name(); + if let Some((prev_span, _)) = members.get(name.as_str()) { + diagnostics.push(name_conflict( + name.as_str(), + NameContext::StructMember(name.span()), + NameContext::StructMember(*prev_span), + )); + } else { + members.insert( + name.as_str().to_string(), + (name.span(), decl.syntax().green().into()), + ); + } + } + + self.structs.insert( + name.as_str().to_string(), + Struct { + span: name.span(), + source: None, + node: definition.syntax().green().into(), + members: Arc::new(members), + }, + ); + } + } + + /// Adds inputs to a names collection. + fn add_inputs( + names: &mut IndexMap, + section: &v1::InputSection, + diagnostics: &mut Vec, + ) { + for decl in section.declarations() { + let name = decl.name(); + let context = ScopedNameContext::Input(name.span()); + if let Some(prev) = names.get(name.as_str()) { + diagnostics.push(name_conflict( + name.as_str(), + context.into(), + prev.context().into(), + )); + continue; + } + + names.insert( + name.as_str().to_string(), + ScopedName { + context, + node: decl.syntax().green().into(), + implicit: false, + }, + ); + } + } + + /// Adds outputs to a names collection. + fn add_outputs( + names: &mut IndexMap, + section: &v1::OutputSection, + diagnostics: &mut Vec, + ) { + for decl in section.declarations() { + let name = decl.name(); + let context = ScopedNameContext::Output(name.span()); + if let Some(prev) = names.get(name.as_str()) { + diagnostics.push(name_conflict( + name.as_str(), + context.into(), + prev.context().into(), + )); + continue; + } + + names.insert( + name.as_str().to_string(), + ScopedName { + context, + node: decl.syntax().green().into(), + implicit: false, + }, + ); + } + } + + /// Adds a task scope to the document's scope. + fn add_task_scope(&mut self, task: &v1::TaskDefinition, diagnostics: &mut Vec) { + // Check for a conflict with another task or workflow + let name = task.name(); + if let Some(s) = self.tasks.get(name.as_str()) { + diagnostics.push(name_conflict( + name.as_str(), + NameContext::Task(name.span()), + NameContext::Task(s.name_span), + )); + return; + } else if let Some(s) = &self.workflow { + if s.name == name.as_str() { + diagnostics.push(name_conflict( + name.as_str(), + NameContext::Task(name.span()), + NameContext::Workflow(s.name_span), + )); + return; + } + } + + // Populate the scope's names + let mut names: IndexMap<_, ScopedName> = IndexMap::new(); + let mut saw_input = false; + let mut saw_output = false; + for item in task.items() { + match item { + v1::TaskItem::Input(section) if !saw_input => { + saw_input = true; + Self::add_inputs(&mut names, §ion, diagnostics); + } + v1::TaskItem::Output(section) if !saw_output => { + saw_output = true; + Self::add_outputs(&mut names, §ion, diagnostics); + } + v1::TaskItem::Declaration(decl) => { + let name = decl.name(); + let context = ScopedNameContext::Decl(name.span()); + if let Some(prev) = names.get(name.as_str()) { + diagnostics.push(name_conflict( + name.as_str(), + context.into(), + prev.context().into(), + )); + continue; + } + + names.insert( + name.as_str().to_string(), + ScopedName { + context, + node: decl.syntax().green().into(), + implicit: false, + }, + ); + } + v1::TaskItem::Input(_) + | v1::TaskItem::Output(_) + | v1::TaskItem::Command(_) + | v1::TaskItem::Runtime(_) + | v1::TaskItem::Metadata(_) + | v1::TaskItem::ParameterMetadata(_) => continue, + } + } + + let span = Self::scope_span(task.syntax()); + let (index, _) = self.tasks.insert_full( + name.as_str().to_string(), + TaskScope { + name_span: name.span(), + scope: Scope { + span, + node: task.syntax().green().into(), + names, + children: Default::default(), + }, + }, + ); + + self.scopes.push((span, ScopeContext::Task(index))); + } + + /// Adds a workflow scope to the document scope. + fn add_workflow_scope( + &mut self, + workflow: &v1::WorkflowDefinition, + diagnostics: &mut Vec, + ) { + // Check for conflicts with task names or an existing workspace + let name = workflow.name(); + if let Some(s) = self.tasks.get(name.as_str()) { + diagnostics.push(name_conflict( + name.as_str(), + NameContext::Workflow(name.span()), + NameContext::Task(s.name_span), + )); + return; + } else if let Some(s) = &self.workflow { + diagnostics.push(duplicate_workflow(&name, s.name_span)); + return; + } + + // First populate the "root" scope + let mut scopes = vec![Scope { + span: Self::scope_span(workflow.syntax()), + node: workflow.syntax().green().into(), + names: Default::default(), + children: Default::default(), + }]; + + let mut saw_input = false; + let mut saw_output = false; + for item in workflow.items() { + match item { + v1::WorkflowItem::Input(section) if !saw_input => { + saw_input = true; + let scope = scopes.last_mut().unwrap(); + Self::add_inputs(&mut scope.names, §ion, diagnostics); + } + v1::WorkflowItem::Output(section) if !saw_output => { + saw_output = true; + let scope = scopes.last_mut().unwrap(); + Self::add_outputs(&mut scope.names, §ion, diagnostics); + } + v1::WorkflowItem::Declaration(decl) => { + Self::add_workflow_statement_decls( + &WorkflowStatement::Declaration(decl), + &mut scopes, + diagnostics, + ); + } + v1::WorkflowItem::Conditional(stmt) => { + Self::add_workflow_statement_decls( + &WorkflowStatement::Conditional(stmt), + &mut scopes, + diagnostics, + ); + } + v1::WorkflowItem::Scatter(stmt) => { + Self::add_workflow_statement_decls( + &WorkflowStatement::Scatter(stmt), + &mut scopes, + diagnostics, + ); + } + v1::WorkflowItem::Call(stmt) => { + Self::add_workflow_statement_decls( + &WorkflowStatement::Call(stmt), + &mut scopes, + diagnostics, + ); + } + v1::WorkflowItem::Input(_) + | v1::WorkflowItem::Output(_) + | v1::WorkflowItem::Metadata(_) + | v1::WorkflowItem::ParameterMetadata(_) => continue, + } + } + + let scope = scopes.pop().unwrap(); + let span = scope.span; + self.workflow = Some(WorkflowScope { + name_span: name.span(), + name: name.as_str().to_string(), + scope, + }); + self.scopes.push((span, ScopeContext::Workflow)); + } + + /// Adds declarations from workflow statements. + fn add_workflow_statement_decls( + stmt: &v1::WorkflowStatement, + scopes: &mut Vec, + diagnostics: &mut Vec, + ) { + /// Finds a name by walking up the scope stack + fn find_name<'a>(name: &str, scopes: &'a [Scope]) -> Option<&'a ScopedName> { + for scope in scopes.iter().rev() { + if let Some(name) = scope.names.get(name) { + return Some(name); + } + } + + None + } + + match stmt { + WorkflowStatement::Conditional(stmt) => { + scopes.push(Scope { + span: Self::scope_span(stmt.syntax()), + node: stmt.syntax().green().into(), + names: Default::default(), + children: Default::default(), + }); + + for stmt in stmt.statements() { + Self::add_workflow_statement_decls(&stmt, scopes, diagnostics); + } + + let scope = scopes.pop().unwrap(); + let parent = scopes.last_mut().unwrap(); + for (name, descendant) in &scope.names { + parent.names.insert( + name.clone(), + ScopedName { + context: descendant.context, + node: descendant.node.clone(), + implicit: true, + }, + ); + } + + parent.children.push(scope); + } + WorkflowStatement::Scatter(stmt) => { + let variable = stmt.variable(); + let context = ScopedNameContext::ScatterVariable(variable.span()); + let mut names = IndexMap::new(); + if let Some(prev) = find_name(variable.as_str(), scopes) { + diagnostics.push(name_conflict( + variable.as_str(), + context.into(), + prev.context().into(), + )); + } else { + names.insert( + variable.as_str().to_string(), + ScopedName { + context, + node: stmt.syntax().green().into(), + implicit: false, + }, + ); + } + + scopes.push(Scope { + span: Self::scope_span(stmt.syntax()), + node: stmt.syntax().green().into(), + names, + children: Default::default(), + }); + + for stmt in stmt.statements() { + Self::add_workflow_statement_decls(&stmt, scopes, diagnostics); + } + + let scope = scopes.pop().unwrap(); + let parent = scopes.last_mut().unwrap(); + for (name, descendant) in &scope.names { + // Don't add an implicit name to the parent for the scatter variable + if descendant.is_scatter_variable() { + continue; + } + + parent.names.insert( + name.clone(), + ScopedName { + context: descendant.context, + node: descendant.node.clone(), + implicit: true, + }, + ); + } + + parent.children.push(scope); + } + WorkflowStatement::Call(stmt) => { + let name = stmt + .alias() + .map(|a| a.name()) + .unwrap_or_else(|| stmt.target().name().1); + if let Some(prev) = find_name(name.as_str(), scopes) { + diagnostics.push(call_conflict( + &name, + prev.context().into(), + stmt.alias().is_none(), + )); + + // Define the name in this scope if it conflicted with a scatter variable + if !prev.is_scatter_variable() { + return; + } + } + + scopes.last_mut().unwrap().names.insert( + name.as_str().to_string(), + ScopedName { + context: ScopedNameContext::Call(name.span()), + node: stmt.syntax().green().into(), + implicit: false, + }, + ); + } + WorkflowStatement::Declaration(decl) => { + let name = decl.name(); + let context = ScopedNameContext::Decl(name.span()); + if let Some(prev) = find_name(name.as_str(), scopes) { + diagnostics.push(name_conflict( + name.as_str(), + context.into(), + prev.context().into(), + )); + + // Define the name in this scope if it conflicted with a scatter variable + if !prev.is_scatter_variable() { + return; + } + } + + scopes.last_mut().unwrap().names.insert( + name.as_str().to_string(), + ScopedName { + context, + node: decl.syntax().green().into(), + implicit: false, + }, + ); + } + } + } + + /// Resolves an import to its document scope. + fn resolve_import( + state: &State, + stmt: &v1::ImportStatement, + importer_id: &DocumentId, + importer_version: &Version, + ) -> Result<(Arc, Arc), Diagnostic> { + let uri = stmt.uri(); + let span = uri.syntax().text_range().to_span(); + let text = match uri.text() { + Some(text) => text, + None => { + let span = uri + .parts() + .find_map(|p| match p { + StringPart::Text(_) => None, + StringPart::Placeholder(p) => Some(p), + }) + .expect("should contain a placeholder") + .syntax() + .text_range() + .to_span(); + return Err(placeholder_in_import(span)); + } + }; + + let id = match DocumentId::relative_to(importer_id, text.as_str()) { + Ok(id) => Arc::new(id), + Err(e) => return Err(invalid_relative_import(&e, span)), + }; + + let (index, document) = state + .graph + .document(&id) + .expect("missing import node in graph"); + + // Check for an import cycle to report + if state + .cycles + .contains(&(state.graph.indexes[importer_id], index)) + { + // There was a cycle for this import + return Err(import_cycle(span)); + } + + // Check for a failure to load the import + if let Some(e) = &document.error { + return Err(import_failure(text.as_str(), e, span)); + } + + // Ensure the import has a matching WDL version + let root = wdl_ast::Document::cast(SyntaxNode::new_root( + document.root.clone().expect("document should have a root"), + )) + .expect("root should cast"); + + // Check for compatible imports + match root.version_statement() { + Some(stmt) => { + let our_version = stmt.version(); + if matches!((our_version.as_str().split('.').next(), importer_version.as_str().split('.').next()), (Some(our_major), Some(their_major)) if our_major != their_major) + { + return Err(incompatible_import( + our_version.as_str(), + span, + importer_version, + )); + } + } + None => { + return Err(import_missing_version(span)); + } + } + + Ok((id, state.graph.inner[index].state.completed().scope.clone())) + } + + /// Calculates the span of a scope given a node which uses braces to + /// delineate the scope. + fn scope_span(parent: &SyntaxNode) -> Span { + let open = token(parent, SyntaxKind::OpenBrace).expect("task must have an opening brace"); + let close = parent + .last_child_or_token() + .and_then(SyntaxElement::into_token) + .expect("task must have a last token"); + assert_eq!( + close.kind(), + SyntaxKind::CloseBrace, + "the last token of a task should be a close brace" + ); + let open = open.text_range().to_span(); + let close = close.text_range().to_span(); + + // The span starts after the opening brace and before the closing brace + Span::new(open.end(), close.start() - open.end()) + } +} diff --git a/wdl-analysis/tests/analysis.rs b/wdl-analysis/tests/analysis.rs new file mode 100644 index 00000000..31a38408 --- /dev/null +++ b/wdl-analysis/tests/analysis.rs @@ -0,0 +1,211 @@ +//! The WDL analysis tests. +//! +//! This test looks for directories in `tests/analysis`. +//! +//! Each directory is expected to contain: +//! +//! * `source.wdl` - the test input source to analyze. +//! * `source.diagnostics` - the expected set of diagnostics across all analyzed +//! files. +//! +//! The `source.diagnostics` file may be automatically generated or updated by +//! setting the `BLESS` environment variable when running this test. + +use std::borrow::Cow; +use std::collections::HashSet; +use std::env; +use std::ffi::OsStr; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::process::exit; +use std::sync::Arc; + +use anyhow::bail; +use anyhow::Context; +use anyhow::Result; +use codespan_reporting::files::SimpleFile; +use codespan_reporting::term; +use codespan_reporting::term::termcolor::Buffer; +use codespan_reporting::term::Config; +use colored::Colorize; +use futures::stream::FuturesOrdered; +use futures::StreamExt; +use futures::TryStreamExt; +use pretty_assertions::StrComparison; +use wdl_analysis::AnalysisEngine; +use wdl_analysis::AnalysisResult; +use wdl_ast::Diagnostic; +use wdl_ast::SyntaxNode; + +fn find_tests() -> Vec { + // Check for filter arguments consisting of test names + let mut filter = HashSet::new(); + for arg in std::env::args().skip_while(|a| a != "--").skip(1) { + if !arg.starts_with('-') { + filter.insert(arg); + } + } + + let mut tests: Vec = Vec::new(); + for entry in Path::new("tests/analysis").read_dir().unwrap() { + let entry = entry.expect("failed to read directory"); + let path = entry.path(); + if !path.is_dir() + || (!filter.is_empty() + && !filter.contains(entry.file_name().to_str().expect("name should be UTF-8"))) + { + continue; + } + + tests.push(path); + } + + tests.sort(); + tests +} + +fn normalize(s: &str, is_error: bool) -> String { + if is_error { + // Normalize paths in any error messages + return s.replace('\\', "/").replace("\r\n", "\n"); + } + + // Otherwise, just normalize line endings + s.replace("\r\n", "\n") +} + +fn compare_result(path: &Path, result: &str, is_error: bool) -> Result<()> { + let result = normalize(result, is_error); + if env::var_os("BLESS").is_some() { + fs::write(path, &result).with_context(|| { + format!( + "failed to write result file `{path}`", + path = path.display() + ) + })?; + return Ok(()); + } + + let expected = fs::read_to_string(path) + .with_context(|| format!("failed to read result file `{path}`", path = path.display()))? + .replace("\r\n", "\n"); + + if expected != result { + bail!( + "result is not as expected:\n{}", + StrComparison::new(&expected, &result), + ); + } + + Ok(()) +} + +fn compare_results(test: &Path, results: Vec) -> Result<()> { + let mut buffer = Buffer::no_color(); + let cwd = std::env::current_dir().expect("must have a CWD"); + for result in results { + let path = result.id().path(); + + // Attempt to strip the CWD from the result path + let path: Cow<'_, str> = match &path { + // Strip the CWD from the path + Some(path) => path.strip_prefix(&cwd).unwrap_or(&path).to_string_lossy(), + // Use the id itself if there is no path + None => result.id().to_str(), + }; + + let diagnostics: Cow<'_, [Diagnostic]> = match 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 + .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.as_ref() { + term::emit( + &mut buffer, + &Config::default(), + &file, + &diagnostic.to_codespan(), + ) + .expect("should emit"); + } + } + } + + let output = test.join("source.diagnostics"); + compare_result( + &output, + &String::from_utf8(buffer.into_inner()).expect("should be UTF-8"), + true, + ) +} + +#[tokio::main] +async fn main() { + let tests = find_tests(); + println!("\nrunning {} tests\n", tests.len()); + + let engine = Arc::new(AnalysisEngine::new().expect("failed to create analysis engine")); + + let tests_engine = engine.clone(); + let mut tasks = tests + .iter() + .map(move |test| { + let engine = tests_engine.clone(); + let source = test.join("source.wdl"); + tokio::spawn(async move { engine.analyze(&source).await }) + }) + .collect::>() + .into_stream() + .enumerate(); + + let mut errors = Vec::new(); + while let Some((index, result)) = tasks.next().await { + let test_name = tests[index].file_stem().and_then(OsStr::to_str).unwrap(); + match result { + Ok(results) => match compare_results(&tests[index], results) { + Ok(_) => { + println!("test {test_name} ... {ok}", ok = "ok".green()); + } + Err(e) => { + println!("test {test_name} ... {failed}", failed = "failed".red()); + errors.push((test_name, e.to_string())); + } + }, + Err(e) => { + println!( + "test {test_name} ... {panicked}", + panicked = "panicked".red() + ); + errors.push((test_name, e.to_string())); + } + } + } + + Arc::into_inner(engine) + .expect("should be last engine reference") + .shutdown() + .await; + + if !errors.is_empty() { + eprintln!( + "\n{count} test(s) {failed}:", + count = errors.len(), + failed = "failed".red() + ); + + for (name, msg) in errors.iter() { + eprintln!("{name}: {msg}", msg = msg.red()); + } + + exit(1); + } + + println!("\ntest result: ok. {count} passed\n", count = tests.len()); +} diff --git a/wdl-ast/tests/validation/conflicting-call-names/source.errors b/wdl-analysis/tests/analysis/conflicting-call-names/source.diagnostics similarity index 80% rename from wdl-ast/tests/validation/conflicting-call-names/source.errors rename to wdl-analysis/tests/analysis/conflicting-call-names/source.diagnostics index 286500d5..355e1a79 100644 --- a/wdl-ast/tests/validation/conflicting-call-names/source.errors +++ b/wdl-analysis/tests/analysis/conflicting-call-names/source.diagnostics @@ -1,5 +1,5 @@ error: conflicting call name `my_int` - ┌─ tests/validation/conflicting-call-names/source.wdl:7:10 + ┌─ tests/analysis/conflicting-call-names/source.wdl:7:10 │ 6 │ Int my_int = 0 # FIRST │ ------ the declaration with the conflicting name is here @@ -9,7 +9,7 @@ error: conflicting call name `my_int` = fix: add an `as` clause to the call to specify a different name error: conflicting call name `foo` - ┌─ tests/validation/conflicting-call-names/source.wdl:10:10 + ┌─ tests/analysis/conflicting-call-names/source.wdl:10:10 │ 9 │ call foo # FIRST │ --- the call with the conflicting name is here @@ -19,7 +19,7 @@ error: conflicting call name `foo` = fix: add an `as` clause to the call to specify a different name error: conflicting call name `bar` - ┌─ tests/validation/conflicting-call-names/source.wdl:13:17 + ┌─ tests/analysis/conflicting-call-names/source.wdl:13:17 │ 12 │ call foo as bar # FIRST │ --- the call with the conflicting name is here @@ -27,7 +27,7 @@ error: conflicting call name `bar` │ ^^^ this conflicts with a call of the same name error: conflicting call name `bar` - ┌─ tests/validation/conflicting-call-names/source.wdl:15:10 + ┌─ tests/analysis/conflicting-call-names/source.wdl:15:10 │ 12 │ call foo as bar # FIRST │ --- the call with the conflicting name is here @@ -38,7 +38,7 @@ error: conflicting call name `bar` = fix: add an `as` clause to the call to specify a different name error: conflicting call name `bar` - ┌─ tests/validation/conflicting-call-names/source.wdl:17:14 + ┌─ tests/analysis/conflicting-call-names/source.wdl:17:14 │ 12 │ call foo as bar # FIRST │ --- the call with the conflicting name is here @@ -49,7 +49,7 @@ error: conflicting call name `bar` = fix: add an `as` clause to the call to specify a different name error: conflicting call name `baz` - ┌─ tests/validation/conflicting-call-names/source.wdl:20:17 + ┌─ tests/analysis/conflicting-call-names/source.wdl:20:17 │ 18 │ call foo.baz # FIRST │ --- the call with the conflicting name is here @@ -58,7 +58,7 @@ error: conflicting call name `baz` │ ^^^ this conflicts with a call of the same name error: conflicting call name `foo` - ┌─ tests/validation/conflicting-call-names/source.wdl:23:14 + ┌─ tests/analysis/conflicting-call-names/source.wdl:23:14 │ 9 │ call foo # FIRST │ --- the call with the conflicting name is here @@ -69,7 +69,7 @@ error: conflicting call name `foo` = fix: add an `as` clause to the call to specify a different name error: conflicting call name `x` - ┌─ tests/validation/conflicting-call-names/source.wdl:24:14 + ┌─ tests/analysis/conflicting-call-names/source.wdl:24:14 │ 22 │ scatter (x in []) { │ - the scatter variable with the conflicting name is here @@ -80,23 +80,23 @@ error: conflicting call name `x` = fix: add an `as` clause to the call to specify a different name error: conflicting call name `x` - ┌─ tests/validation/conflicting-call-names/source.wdl:29:10 + ┌─ tests/analysis/conflicting-call-names/source.wdl:28:10 │ 24 │ call x # NOT OK │ - the call with the conflicting name is here · -29 │ call x # NOT OK +28 │ call x # NOT OK │ ^ this conflicts with a call of the same name │ = fix: add an `as` clause to the call to specify a different name error: conflicting call name `ok` - ┌─ tests/validation/conflicting-call-names/source.wdl:30:10 + ┌─ tests/analysis/conflicting-call-names/source.wdl:29:10 │ 25 │ call ok # OK │ -- the call with the conflicting name is here · -30 │ call ok # NOT OK +29 │ call ok # NOT OK │ ^^ this conflicts with a call of the same name │ = fix: add an `as` clause to the call to specify a different name diff --git a/wdl-ast/tests/validation/conflicting-call-names/source.wdl b/wdl-analysis/tests/analysis/conflicting-call-names/source.wdl similarity index 99% rename from wdl-ast/tests/validation/conflicting-call-names/source.wdl rename to wdl-analysis/tests/analysis/conflicting-call-names/source.wdl index 88dba357..4ce92dc1 100644 --- a/wdl-ast/tests/validation/conflicting-call-names/source.wdl +++ b/wdl-analysis/tests/analysis/conflicting-call-names/source.wdl @@ -23,7 +23,6 @@ workflow test { call foo # NOT OK call x # NOT OK call ok # OK - } call x # NOT OK diff --git a/wdl-analysis/tests/analysis/conflicting-decl-names/source.diagnostics b/wdl-analysis/tests/analysis/conflicting-decl-names/source.diagnostics new file mode 100644 index 00000000..1180f81f --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-decl-names/source.diagnostics @@ -0,0 +1,108 @@ +error: conflicting output name `x` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:13:16 + │ + 7 │ Int x + │ - the input with the conflicting name is here + · +13 │ String x = "x" + │ ^ this conflicts with a input of the same name + +error: conflicting output name `y` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:14:16 + │ + 8 │ Int y = 0 + │ - the input with the conflicting name is here + · +14 │ String y = "y" + │ ^ this conflicts with a input of the same name + +error: conflicting declaration name `x` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:19:9 + │ + 7 │ Int x + │ - the input with the conflicting name is here + · +19 │ Int x = y + │ ^ this conflicts with a input of the same name + +error: conflicting output name `x` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:32:16 + │ +26 │ Int x + │ - the input with the conflicting name is here + · +32 │ String x = "x" + │ ^ this conflicts with a input of the same name + +error: conflicting output name `y` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:33:16 + │ +27 │ Int y = 0 + │ - the input with the conflicting name is here + · +33 │ String y = "y" + │ ^ this conflicts with a input of the same name + +error: conflicting declaration name `x` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:38:9 + │ +26 │ Int x + │ - the input with the conflicting name is here + · +38 │ Int x = y + │ ^ this conflicts with a input of the same name + +error: conflicting declaration name `b` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:45:17 + │ +28 │ String b + │ - the input with the conflicting name is here + · +45 │ Int b = 0 + │ ^ this conflicts with a input of the same name + +error: conflicting declaration name `x2` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:46:17 + │ +41 │ Int x2 = 0 + │ -- the declaration with the conflicting name is here + · +46 │ Int x2 = 0 + │ ^^ this conflicts with a declaration of the same name + +error: conflicting scatter variable name `x` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:51:14 + │ +26 │ Int x + │ - the input with the conflicting name is here + · +51 │ scatter (x in [1, 2, 3]) { + │ ^ this conflicts with a input of the same name + +error: conflicting declaration name `z` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:52:13 + │ +37 │ Int z = x + │ - the declaration with the conflicting name is here + · +52 │ Int z = x + │ ^ this conflicts with a declaration of the same name + +error: conflicting declaration name `nested` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:57:21 + │ +55 │ scatter (nested in [1, 2, 3]) { + │ ------ the scatter variable with the conflicting name is here +56 │ scatter (baz in [1, 2, 3]) { +57 │ Int nested = 0 + │ ^^^^^^ this conflicts with a scatter variable of the same name + +error: conflicting declaration name `nested` + ┌─ tests/analysis/conflicting-decl-names/source.wdl:64:13 + │ +57 │ Int nested = 0 + │ ------ the declaration with the conflicting name is here + · +64 │ Int nested = 0 + │ ^^^^^^ this conflicts with a declaration of the same name + diff --git a/wdl-ast/tests/validation/conflicting-decl-names/source.wdl b/wdl-analysis/tests/analysis/conflicting-decl-names/source.wdl similarity index 100% rename from wdl-ast/tests/validation/conflicting-decl-names/source.wdl rename to wdl-analysis/tests/analysis/conflicting-decl-names/source.wdl diff --git a/wdl-analysis/tests/analysis/conflicting-imports/Baz b/wdl-analysis/tests/analysis/conflicting-imports/Baz new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/Baz @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/conflicting-imports/Baz.wdl b/wdl-analysis/tests/analysis/conflicting-imports/Baz.wdl new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/Baz.wdl @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/conflicting-imports/bad-file-name.wdl b/wdl-analysis/tests/analysis/conflicting-imports/bad-file-name.wdl new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/bad-file-name.wdl @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/conflicting-imports/foo b/wdl-analysis/tests/analysis/conflicting-imports/foo new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/foo @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/conflicting-imports/foo.wdl b/wdl-analysis/tests/analysis/conflicting-imports/foo.wdl new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/foo.wdl @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/conflicting-imports/md5sum.wdl b/wdl-analysis/tests/analysis/conflicting-imports/md5sum.wdl new file mode 100644 index 00000000..469d9577 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/md5sum.wdl @@ -0,0 +1,5 @@ +version 1.1 + +workflow test { + +} diff --git a/wdl-analysis/tests/analysis/conflicting-imports/qux/baz.wdl b/wdl-analysis/tests/analysis/conflicting-imports/qux/baz.wdl new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/qux/baz.wdl @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/conflicting-imports/source.diagnostics b/wdl-analysis/tests/analysis/conflicting-imports/source.diagnostics new file mode 100644 index 00000000..026ba4c4 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/source.diagnostics @@ -0,0 +1,79 @@ +error: conflicting import namespace `foo` + ┌─ tests/analysis/conflicting-imports/source.wdl:6:8 + │ +5 │ import "foo.wdl" # First + │ --------- the conflicting import namespace was introduced here +6 │ import "foo" # Conflicts + │ ^^^^^ this conflicts with another import namespace + │ + = fix: add an `as` clause to the import to specify a namespace + +error: import namespace is not a valid WDL identifier + ┌─ tests/analysis/conflicting-imports/source.wdl:7:8 + │ +7 │ import "bad-file-name.wdl" # Bad name + │ ^^^^^^^^^^^^^^^^^^^ a namespace cannot be derived from this import path + │ + = fix: add an `as` clause to the import to specify a namespace + +error: conflicting import namespace `baz` + ┌─ tests/analysis/conflicting-imports/source.wdl:11:21 + │ +10 │ import "qux/baz.wdl" # First + │ ------------- the conflicting import namespace was introduced here +11 │ import "Baz.wdl" as baz # Conflicts + │ ^^^ this conflicts with another import namespace + +error: conflicting import namespace `baz` + ┌─ tests/analysis/conflicting-imports/source.wdl:12:8 + │ +10 │ import "qux/baz.wdl" # First + │ ------------- the conflicting import namespace was introduced here +11 │ import "Baz.wdl" as baz # Conflicts +12 │ import "../conflicting-imports/qux/baz.wdl" # Conflicts + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this conflicts with another import namespace + │ + = fix: add an `as` clause to the import to specify a namespace + +error: conflicting import namespace `md5sum` + ┌─ tests/analysis/conflicting-imports/source.wdl:14:8 + │ +13 │ import "md5sum.wdl" # First + │ ------------ the conflicting import namespace was introduced here +14 │ import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/md5sum.wdl" # Conflicts + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this conflicts with another import namespace + │ + = fix: add an `as` clause to the import to specify a namespace + +error: conflicting import namespace `md5sum` + ┌─ tests/analysis/conflicting-imports/source.wdl:15:8 + │ +13 │ import "md5sum.wdl" # First + │ ------------ the conflicting import namespace was introduced here +14 │ import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/md5sum.wdl" # Conflicts +15 │ import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/md5sum.wdl#something" # Conflicts + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this conflicts with another import namespace + │ + = fix: add an `as` clause to the import to specify a namespace + +error: conflicting import namespace `star` + ┌─ tests/analysis/conflicting-imports/source.wdl:17:8 + │ +16 │ import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/star.wdl?query=foo" # First + │ --------------------------------------------------------------------------------------------------------------------------- the conflicting import namespace was introduced here +17 │ import "star.wdl" # Conflicts + │ ^^^^^^^^^^ this conflicts with another import namespace + │ + = fix: add an `as` clause to the import to specify a namespace + +error: conflicting import namespace `star` + ┌─ tests/analysis/conflicting-imports/source.wdl:18:8 + │ +16 │ import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/star.wdl?query=foo" # First + │ --------------------------------------------------------------------------------------------------------------------------- the conflicting import namespace was introduced here +17 │ import "star.wdl" # Conflicts +18 │ import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/%73tar.wdl" # Conflicts + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this conflicts with another import namespace + │ + = fix: add an `as` clause to the import to specify a namespace + diff --git a/wdl-analysis/tests/analysis/conflicting-imports/source.wdl b/wdl-analysis/tests/analysis/conflicting-imports/source.wdl new file mode 100644 index 00000000..c4c63a2b --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/source.wdl @@ -0,0 +1,20 @@ +## This is a test of conflicting imports. + +version 1.1 + +import "foo.wdl" # First +import "foo" # Conflicts +import "bad-file-name.wdl" # Bad name +import "foo" as bar # First +import "Baz" # First +import "qux/baz.wdl" # First +import "Baz.wdl" as baz # Conflicts +import "../conflicting-imports/qux/baz.wdl" # Conflicts +import "md5sum.wdl" # First +import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/md5sum.wdl" # Conflicts +import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/md5sum.wdl#something" # Conflicts +import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/star.wdl?query=foo" # First +import "star.wdl" # Conflicts +import "https://raw.githubusercontent.com/stjudecloud/workflows/efdca837bc35fe5647de6aa95989652a5a9648dc/tools/%73tar.wdl" # Conflicts + +workflow test {} diff --git a/wdl-analysis/tests/analysis/conflicting-imports/star.wdl b/wdl-analysis/tests/analysis/conflicting-imports/star.wdl new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-imports/star.wdl @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-ast/tests/validation/conflicting-member-names/source.errors b/wdl-analysis/tests/analysis/conflicting-member-names/source.diagnostics similarity index 78% rename from wdl-ast/tests/validation/conflicting-member-names/source.errors rename to wdl-analysis/tests/analysis/conflicting-member-names/source.diagnostics index e596d25c..94fd8d9c 100644 --- a/wdl-ast/tests/validation/conflicting-member-names/source.errors +++ b/wdl-analysis/tests/analysis/conflicting-member-names/source.diagnostics @@ -1,5 +1,5 @@ error: conflicting struct member name `x` - ┌─ tests/validation/conflicting-member-names/source.wdl:8:9 + ┌─ tests/analysis/conflicting-member-names/source.wdl:8:9 │ 6 │ Int x │ - the struct member with the conflicting name is here @@ -8,7 +8,7 @@ error: conflicting struct member name `x` │ ^ this conflicts with a struct member of the same name error: conflicting struct member name `y` - ┌─ tests/validation/conflicting-member-names/source.wdl:9:9 + ┌─ tests/analysis/conflicting-member-names/source.wdl:9:9 │ 7 │ Int y │ - the struct member with the conflicting name is here diff --git a/wdl-ast/tests/validation/conflicting-member-names/source.wdl b/wdl-analysis/tests/analysis/conflicting-member-names/source.wdl similarity index 100% rename from wdl-ast/tests/validation/conflicting-member-names/source.wdl rename to wdl-analysis/tests/analysis/conflicting-member-names/source.wdl diff --git a/wdl-analysis/tests/analysis/conflicting-struct-names/bar.wdl b/wdl-analysis/tests/analysis/conflicting-struct-names/bar.wdl new file mode 100644 index 00000000..96081835 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-struct-names/bar.wdl @@ -0,0 +1,5 @@ +version 1.1 + +struct Baz { + Int x +} diff --git a/wdl-analysis/tests/analysis/conflicting-struct-names/baz.wdl b/wdl-analysis/tests/analysis/conflicting-struct-names/baz.wdl new file mode 100644 index 00000000..30bacf26 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-struct-names/baz.wdl @@ -0,0 +1,9 @@ +version 1.1 + +struct A { + Int x +} + +struct B { + Int x +} diff --git a/wdl-analysis/tests/analysis/conflicting-struct-names/foo.wdl b/wdl-analysis/tests/analysis/conflicting-struct-names/foo.wdl new file mode 100644 index 00000000..04475e80 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-struct-names/foo.wdl @@ -0,0 +1,5 @@ +version 1.1 + +struct Foo { + Int x +} diff --git a/wdl-analysis/tests/analysis/conflicting-struct-names/source.diagnostics b/wdl-analysis/tests/analysis/conflicting-struct-names/source.diagnostics new file mode 100644 index 00000000..bd19fa87 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-struct-names/source.diagnostics @@ -0,0 +1,40 @@ +error: conflicting struct name `Foo` + ┌─ tests/analysis/conflicting-struct-names/source.wdl:7:8 + │ + 7 │ struct Foo { + │ ^^^ this name conflicts with an imported struct + · +27 │ import "bar.wdl" alias Baz as Foo + │ --- the import that introduced the struct is here + │ + = fix: either rename the struct or use an `alias` clause on the import with a different name + +error: conflicting struct name `Foo` + ┌─ tests/analysis/conflicting-struct-names/source.wdl:15:8 + │ + 7 │ struct Foo { + │ --- the struct with the conflicting name is here + · +15 │ struct Foo { + │ ^^^ this conflicts with a struct of the same name + +error: conflicting struct name `Bar` + ┌─ tests/analysis/conflicting-struct-names/source.wdl:19:8 + │ +11 │ struct Bar { + │ --- the struct with the conflicting name is here + · +19 │ struct Bar { + │ ^^^ this conflicts with a struct of the same name + +error: conflicting struct name `Baz` + ┌─ tests/analysis/conflicting-struct-names/source.wdl:23:8 + │ + 5 │ import "foo.wdl" alias Foo as Baz + │ --- the import that introduced the struct is here + · +23 │ struct Baz { + │ ^^^ this name conflicts with an imported struct + │ + = fix: either rename the struct or use an `alias` clause on the import with a different name + diff --git a/wdl-ast/tests/validation/conflicting-struct-names/source.wdl b/wdl-analysis/tests/analysis/conflicting-struct-names/source.wdl similarity index 63% rename from wdl-ast/tests/validation/conflicting-struct-names/source.wdl rename to wdl-analysis/tests/analysis/conflicting-struct-names/source.wdl index 4a32ad0e..80f90f31 100644 --- a/wdl-ast/tests/validation/conflicting-struct-names/source.wdl +++ b/wdl-analysis/tests/analysis/conflicting-struct-names/source.wdl @@ -2,7 +2,7 @@ version 1.1 -import "foo" alias Foo as Baz +import "foo.wdl" alias Foo as Baz struct Foo { Int x @@ -24,6 +24,6 @@ struct Baz { Int x } -import "Bar" alias Baz as Foo +import "bar.wdl" alias Baz as Foo -import "qux" alias A as Qux alias B as Qux +import "baz.wdl" alias A as Qux alias B as Qux diff --git a/wdl-ast/tests/validation/conflicting-task-names/source.errors b/wdl-analysis/tests/analysis/conflicting-task-names/source.diagnostics similarity index 76% rename from wdl-ast/tests/validation/conflicting-task-names/source.errors rename to wdl-analysis/tests/analysis/conflicting-task-names/source.diagnostics index f4ca8fd9..f7117b45 100644 --- a/wdl-ast/tests/validation/conflicting-task-names/source.errors +++ b/wdl-analysis/tests/analysis/conflicting-task-names/source.diagnostics @@ -1,5 +1,5 @@ error: conflicting task name `foo` - ┌─ tests/validation/conflicting-task-names/source.wdl:9:6 + ┌─ tests/analysis/conflicting-task-names/source.wdl:9:6 │ 5 │ workflow foo { │ --- the workflow with the conflicting name is here @@ -8,7 +8,7 @@ error: conflicting task name `foo` │ ^^^ this conflicts with a workflow of the same name error: conflicting task name `bar` - ┌─ tests/validation/conflicting-task-names/source.wdl:17:6 + ┌─ tests/analysis/conflicting-task-names/source.wdl:17:6 │ 13 │ task bar { │ --- the task with the conflicting name is here diff --git a/wdl-ast/tests/validation/conflicting-task-names/source.wdl b/wdl-analysis/tests/analysis/conflicting-task-names/source.wdl similarity index 100% rename from wdl-ast/tests/validation/conflicting-task-names/source.wdl rename to wdl-analysis/tests/analysis/conflicting-task-names/source.wdl diff --git a/wdl-analysis/tests/analysis/conflicting-workflow-names/source.diagnostics b/wdl-analysis/tests/analysis/conflicting-workflow-names/source.diagnostics new file mode 100644 index 00000000..e7f1a605 --- /dev/null +++ b/wdl-analysis/tests/analysis/conflicting-workflow-names/source.diagnostics @@ -0,0 +1,17 @@ +error: conflicting workflow name `foo` + ┌─ tests/analysis/conflicting-workflow-names/source.wdl:9:10 + │ +5 │ task foo { + │ --- the task with the conflicting name is here + · +9 │ workflow foo {} + │ ^^^ this conflicts with a task of the same name + +error: cannot define workflow `bar` as only one workflow is allowed per source file + ┌─ tests/analysis/conflicting-workflow-names/source.wdl:11:10 + │ +10 │ workflow bar {} + │ --- first workflow is defined here +11 │ workflow bar {} + │ ^^^ consider moving this workflow to a new file + diff --git a/wdl-ast/tests/validation/conflicting-workflow-names/source.wdl b/wdl-analysis/tests/analysis/conflicting-workflow-names/source.wdl similarity index 100% rename from wdl-ast/tests/validation/conflicting-workflow-names/source.wdl rename to wdl-analysis/tests/analysis/conflicting-workflow-names/source.wdl diff --git a/wdl-analysis/tests/analysis/import-dependency-cycle/bar.wdl b/wdl-analysis/tests/analysis/import-dependency-cycle/bar.wdl new file mode 100644 index 00000000..a6c86062 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-dependency-cycle/bar.wdl @@ -0,0 +1,7 @@ +version 1.1 + +import "source.wdl" + +struct bar { + Int x +} diff --git a/wdl-analysis/tests/analysis/import-dependency-cycle/foo.wdl b/wdl-analysis/tests/analysis/import-dependency-cycle/foo.wdl new file mode 100644 index 00000000..abd7bd99 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-dependency-cycle/foo.wdl @@ -0,0 +1,7 @@ +version 1.1 + +import "bar.wdl" + +struct foo { + Int x +} diff --git a/wdl-analysis/tests/analysis/import-dependency-cycle/source.diagnostics b/wdl-analysis/tests/analysis/import-dependency-cycle/source.diagnostics new file mode 100644 index 00000000..9612083d --- /dev/null +++ b/wdl-analysis/tests/analysis/import-dependency-cycle/source.diagnostics @@ -0,0 +1,6 @@ +error: import introduces a dependency cycle + ┌─ tests/analysis/import-dependency-cycle/bar.wdl:3:8 + │ +3 │ import "source.wdl" + │ ^^^^^^^^^^^^ this import has been skipped to break the cycle + diff --git a/wdl-analysis/tests/analysis/import-dependency-cycle/source.wdl b/wdl-analysis/tests/analysis/import-dependency-cycle/source.wdl new file mode 100644 index 00000000..92df20f8 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-dependency-cycle/source.wdl @@ -0,0 +1,8 @@ +## This is a test of detecting an import dependency cycle. + +version 1.1 + +import "foo.wdl" + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-failed-http/source.diagnostics b/wdl-analysis/tests/analysis/import-failed-http/source.diagnostics new file mode 100644 index 00000000..95ce8ef1 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-failed-http/source.diagnostics @@ -0,0 +1,6 @@ +error: failed to import `https://www.google.com/404`: server returned HTTP status 404 Not Found + ┌─ tests/analysis/import-failed-http/source.wdl:5:8 + │ +5 │ import "https://www.google.com/404" as foo + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + diff --git a/wdl-analysis/tests/analysis/import-failed-http/source.wdl b/wdl-analysis/tests/analysis/import-failed-http/source.wdl new file mode 100644 index 00000000..951fe21a --- /dev/null +++ b/wdl-analysis/tests/analysis/import-failed-http/source.wdl @@ -0,0 +1,8 @@ +## This is a test of a failed response for a HTTP import + +version 1.1 + +import "https://www.google.com/404" as foo + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-incompatible-versions/foo.wdl b/wdl-analysis/tests/analysis/import-incompatible-versions/foo.wdl new file mode 100644 index 00000000..88354e24 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-incompatible-versions/foo.wdl @@ -0,0 +1,4 @@ +version 2.0 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-incompatible-versions/source.diagnostics b/wdl-analysis/tests/analysis/import-incompatible-versions/source.diagnostics new file mode 100644 index 00000000..3185cb4a --- /dev/null +++ b/wdl-analysis/tests/analysis/import-incompatible-versions/source.diagnostics @@ -0,0 +1,15 @@ +error: unsupported WDL version `2.0` + ┌─ tests/analysis/import-incompatible-versions/foo.wdl:1:9 + │ +1 │ version 2.0 + │ ^^^ this version of WDL is not supported + +error: imported document has incompatible version + ┌─ tests/analysis/import-incompatible-versions/source.wdl:5:8 + │ +3 │ version 1.0 + │ --- the importing document is version `1.0` +4 │ +5 │ import "foo.wdl" + │ ^^^^^^^^^ the imported document is version `2.0` + diff --git a/wdl-analysis/tests/analysis/import-incompatible-versions/source.wdl b/wdl-analysis/tests/analysis/import-incompatible-versions/source.wdl new file mode 100644 index 00000000..21443ed1 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-incompatible-versions/source.wdl @@ -0,0 +1,8 @@ +## This is a test of an import with an incompatible version + +version 1.0 + +import "foo.wdl" + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-missing-version/foo.wdl b/wdl-analysis/tests/analysis/import-missing-version/foo.wdl new file mode 100644 index 00000000..8d519e2c --- /dev/null +++ b/wdl-analysis/tests/analysis/import-missing-version/foo.wdl @@ -0,0 +1,2 @@ +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-missing-version/source.diagnostics b/wdl-analysis/tests/analysis/import-missing-version/source.diagnostics new file mode 100644 index 00000000..39b7e821 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-missing-version/source.diagnostics @@ -0,0 +1,12 @@ +error: a WDL document must start with a version statement + ┌─ tests/analysis/import-missing-version/foo.wdl:1:1 + │ +1 │ workflow test { + │ ^ a version statement must come before this + +error: imported document is missing a version statement + ┌─ tests/analysis/import-missing-version/source.wdl:5:8 + │ +5 │ import "foo.wdl" + │ ^^^^^^^^^ + diff --git a/wdl-analysis/tests/analysis/import-missing-version/source.wdl b/wdl-analysis/tests/analysis/import-missing-version/source.wdl new file mode 100644 index 00000000..c4db28f2 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-missing-version/source.wdl @@ -0,0 +1,8 @@ +## This is a test of importing a file with a missing version. + +version 1.1 + +import "foo.wdl" + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-missing/source.diagnostics b/wdl-analysis/tests/analysis/import-missing/source.diagnostics new file mode 100644 index 00000000..17c35153 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-missing/source.diagnostics @@ -0,0 +1,6 @@ +error: failed to import `foo.wdl`: No such file or directory (os error 2) + ┌─ tests/analysis/import-missing/source.wdl:4:8 + │ +4 │ import "foo.wdl" + │ ^^^^^^^^^ + diff --git a/wdl-analysis/tests/analysis/import-missing/source.wdl b/wdl-analysis/tests/analysis/import-missing/source.wdl new file mode 100644 index 00000000..d9016703 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-missing/source.wdl @@ -0,0 +1,7 @@ +## This is a test of importing a missing import. +version 1.1 + +import "foo.wdl" + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-namespace-invalid/invalid-namespace.wdl b/wdl-analysis/tests/analysis/import-namespace-invalid/invalid-namespace.wdl new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-namespace-invalid/invalid-namespace.wdl @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-namespace-invalid/source.diagnostics b/wdl-analysis/tests/analysis/import-namespace-invalid/source.diagnostics new file mode 100644 index 00000000..be769ffe --- /dev/null +++ b/wdl-analysis/tests/analysis/import-namespace-invalid/source.diagnostics @@ -0,0 +1,8 @@ +error: import namespace is not a valid WDL identifier + ┌─ tests/analysis/import-namespace-invalid/source.wdl:5:8 + │ +5 │ import "invalid-namespace.wdl" + │ ^^^^^^^^^^^^^^^^^^^^^^^ a namespace cannot be derived from this import path + │ + = fix: add an `as` clause to the import to specify a namespace + diff --git a/wdl-analysis/tests/analysis/import-namespace-invalid/source.wdl b/wdl-analysis/tests/analysis/import-namespace-invalid/source.wdl new file mode 100644 index 00000000..f300e411 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-namespace-invalid/source.wdl @@ -0,0 +1,8 @@ +## This is a test of an import statement with an invalid namespace + +version 1.1 + +import "invalid-namespace.wdl" + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-relative/a/file.wdl b/wdl-analysis/tests/analysis/import-relative/a/file.wdl new file mode 100644 index 00000000..415bc2be --- /dev/null +++ b/wdl-analysis/tests/analysis/import-relative/a/file.wdl @@ -0,0 +1,6 @@ +version 1.1 + +import "../b/file.wdl" + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-relative/b/file.wdl b/wdl-analysis/tests/analysis/import-relative/b/file.wdl new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-relative/b/file.wdl @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-relative/source.diagnostics b/wdl-analysis/tests/analysis/import-relative/source.diagnostics new file mode 100644 index 00000000..e69de29b diff --git a/wdl-analysis/tests/analysis/import-relative/source.wdl b/wdl-analysis/tests/analysis/import-relative/source.wdl new file mode 100644 index 00000000..58e99133 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-relative/source.wdl @@ -0,0 +1,9 @@ +## This a test of importing by relative paths. +## There should be no diagnostics generated + +version 1.1 + +import "a/file.wdl" + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/import-unsupported-scheme/source.diagnostics b/wdl-analysis/tests/analysis/import-unsupported-scheme/source.diagnostics new file mode 100644 index 00000000..6e732f1b --- /dev/null +++ b/wdl-analysis/tests/analysis/import-unsupported-scheme/source.diagnostics @@ -0,0 +1,6 @@ +error: failed to import `foo://bar`: unsupported URI scheme `foo` + ┌─ tests/analysis/import-unsupported-scheme/source.wdl:5:8 + │ +5 │ import "foo://bar" as foo + │ ^^^^^^^^^^^ + diff --git a/wdl-analysis/tests/analysis/import-unsupported-scheme/source.wdl b/wdl-analysis/tests/analysis/import-unsupported-scheme/source.wdl new file mode 100644 index 00000000..b5aae8c3 --- /dev/null +++ b/wdl-analysis/tests/analysis/import-unsupported-scheme/source.wdl @@ -0,0 +1,8 @@ +## This is a test of an unsupported URI scheme in an import + +version 1.1 + +import "foo://bar" as foo + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/missing-aliased-struct/bar.wdl b/wdl-analysis/tests/analysis/missing-aliased-struct/bar.wdl new file mode 100644 index 00000000..fa07a1c4 --- /dev/null +++ b/wdl-analysis/tests/analysis/missing-aliased-struct/bar.wdl @@ -0,0 +1,5 @@ +version 1.1 + +struct Bar { + Int x +} diff --git a/wdl-analysis/tests/analysis/missing-aliased-struct/foo.wdl b/wdl-analysis/tests/analysis/missing-aliased-struct/foo.wdl new file mode 100644 index 00000000..05c96938 --- /dev/null +++ b/wdl-analysis/tests/analysis/missing-aliased-struct/foo.wdl @@ -0,0 +1,7 @@ +version 1.1 + +import "bar.wdl" + +struct Qux { + Int x +} diff --git a/wdl-analysis/tests/analysis/missing-aliased-struct/source.diagnostics b/wdl-analysis/tests/analysis/missing-aliased-struct/source.diagnostics new file mode 100644 index 00000000..cc051184 --- /dev/null +++ b/wdl-analysis/tests/analysis/missing-aliased-struct/source.diagnostics @@ -0,0 +1,6 @@ +error: a struct named `Foo` does not exist in the imported document + ┌─ tests/analysis/missing-aliased-struct/source.wdl:5:24 + │ +5 │ import "foo.wdl" alias Foo as Bar alias Bar as Baz alias Qux as Qux2 + │ ^^^ this struct does not exist + diff --git a/wdl-analysis/tests/analysis/missing-aliased-struct/source.wdl b/wdl-analysis/tests/analysis/missing-aliased-struct/source.wdl new file mode 100644 index 00000000..438e4551 --- /dev/null +++ b/wdl-analysis/tests/analysis/missing-aliased-struct/source.wdl @@ -0,0 +1,9 @@ +## This is a test of aliasing a struct that does not exist in an import. + +version 1.1 + +import "foo.wdl" alias Foo as Bar alias Bar as Baz alias Qux as Qux2 + +struct Qux { + Int x +} diff --git a/wdl-analysis/tests/analysis/placeholder-in-import/ok.wdl b/wdl-analysis/tests/analysis/placeholder-in-import/ok.wdl new file mode 100644 index 00000000..9892c584 --- /dev/null +++ b/wdl-analysis/tests/analysis/placeholder-in-import/ok.wdl @@ -0,0 +1,4 @@ +version 1.1 + +workflow test { +} diff --git a/wdl-analysis/tests/analysis/placeholder-in-import/source.diagnostics b/wdl-analysis/tests/analysis/placeholder-in-import/source.diagnostics new file mode 100644 index 00000000..d0f6c5b3 --- /dev/null +++ b/wdl-analysis/tests/analysis/placeholder-in-import/source.diagnostics @@ -0,0 +1,12 @@ +error: import URI cannot contain placeholders + ┌─ tests/analysis/placeholder-in-import/source.wdl:5:23 + │ +5 │ import "this contains ~{"a placeholder"}" as foo + │ ^^^^^^^^^^^^^^^^^^ remove this placeholder + +error: import URI cannot contain placeholders + ┌─ tests/analysis/placeholder-in-import/source.wdl:6:28 + │ +6 │ import "this also contains ${"a placeholder"}" as bar + │ ^^^^^^^^^^^^^^^^^^ remove this placeholder + diff --git a/wdl-analysis/tests/analysis/placeholder-in-import/source.wdl b/wdl-analysis/tests/analysis/placeholder-in-import/source.wdl new file mode 100644 index 00000000..776a2689 --- /dev/null +++ b/wdl-analysis/tests/analysis/placeholder-in-import/source.wdl @@ -0,0 +1,11 @@ +## This is a test of having a placeholder in an import + +version 1.1 + +import "this contains ~{"a placeholder"}" as foo +import "this also contains ${"a placeholder"}" as bar +import "ok.wdl" as baz + +workflow test { + +} diff --git a/wdl-ast/tests/validation/too-many-workflows/source.errors b/wdl-analysis/tests/analysis/too-many-workflows/source.diagnostics similarity index 80% rename from wdl-ast/tests/validation/too-many-workflows/source.errors rename to wdl-analysis/tests/analysis/too-many-workflows/source.diagnostics index e46867d7..279e07a7 100644 --- a/wdl-ast/tests/validation/too-many-workflows/source.errors +++ b/wdl-analysis/tests/analysis/too-many-workflows/source.diagnostics @@ -1,5 +1,5 @@ error: cannot define workflow `bar` as only one workflow is allowed per source file - ┌─ tests/validation/too-many-workflows/source.wdl:9:10 + ┌─ tests/analysis/too-many-workflows/source.wdl:9:10 │ 5 │ workflow foo { │ --- first workflow is defined here @@ -8,7 +8,7 @@ error: cannot define workflow `bar` as only one workflow is allowed per source f │ ^^^ consider moving this workflow to a new file error: cannot define workflow `baz` as only one workflow is allowed per source file - ┌─ tests/validation/too-many-workflows/source.wdl:13:10 + ┌─ tests/analysis/too-many-workflows/source.wdl:13:10 │ 5 │ workflow foo { │ --- first workflow is defined here diff --git a/wdl-ast/tests/validation/too-many-workflows/source.wdl b/wdl-analysis/tests/analysis/too-many-workflows/source.wdl similarity index 100% rename from wdl-ast/tests/validation/too-many-workflows/source.wdl rename to wdl-analysis/tests/analysis/too-many-workflows/source.wdl diff --git a/wdl-ast/CHANGELOG.md b/wdl-ast/CHANGELOG.md index 8499f8c4..d04a8bc6 100644 --- a/wdl-ast/CHANGELOG.md +++ b/wdl-ast/CHANGELOG.md @@ -11,6 +11,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Add support for the exponentiation operator in WDL 1.2 ([#111](https://github.com/stjude-rust-labs/wdl/pull/111)). +### Changed + +* Changed the API for parsing documents; `Document::parse` now returns + `(Document, Vec)` rather than a `Parse` type ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). +* The `Type` enumeration, and friends, in `wdl-ast` no longer implement + `PartialOrd` and `Ord`; those implementations have moved to the sort lint + rule ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). +* The `PartialEq` implementation of the `Type` enumeration, and friends, is now + implemented in terms of WDL type equivalence and not by CST node equivalence + ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). + ## 0.4.0 - 06-28-2024 ### Added diff --git a/wdl-ast/src/lib.rs b/wdl-ast/src/lib.rs index 877bbe20..468d9e78 100644 --- a/wdl-ast/src/lib.rs +++ b/wdl-ast/src/lib.rs @@ -18,21 +18,14 @@ //! use wdl_ast::Document; //! use wdl_ast::Validator; //! -//! match Document::parse(source).into_result() { -//! Ok(document) => { -//! let validator = Validator::default(); -//! match validator.validate(&document) { -//! Ok(_) => { -//! // The document was valid WDL -//! } -//! Err(diagnostics) => { -//! // Handle the failure to validate -//! } -//! } -//! } -//! Err(diagnostics) => { -//! // Handle the failure to parse -//! } +//! let (document, diagnostics) = Document::parse(source); +//! if !diagnostics.is_empty() { +//! // Handle the failure to parse +//! } +//! +//! let mut validator = Validator::default(); +//! if let Err(diagnostics) = validator.validate(&document) { +//! // Handle the failure to validate //! } //! ``` @@ -44,7 +37,6 @@ #![warn(rustdoc::broken_intra_doc_links)] use std::fmt; -use std::sync::Arc; pub use rowan::ast::support; pub use rowan::ast::AstChildren; @@ -161,56 +153,6 @@ impl Ast { } } -/// Represents the result of a parse: a [Document] and a list of diagnostics. -/// -/// A parse always produces a [Document], even for documents that contain -/// syntax errors. -#[derive(Clone, Debug)] -pub struct Parse { - /// The document that was parsed. - document: Document, - /// The parse diagnostics that were encountered. - diagnostics: Option>, -} - -impl Parse { - /// Constructs a new parse result from the given document and list of - /// parser diagnostics. - fn new(document: Document, diagnostics: Vec) -> Parse { - Self { - document, - diagnostics: if diagnostics.is_empty() { - None - } else { - Some(diagnostics.into()) - }, - } - } - - /// Gets the root syntax node from the parse. - pub fn root(&self) -> &SyntaxNode { - &self.document.0 - } - - /// Gets the diagnostics from the parse. - pub fn diagnostics(&self) -> &[Diagnostic] { - self.diagnostics.as_deref().unwrap_or_default() - } - - /// Gets the document resulting from the parse. - pub fn document(&self) -> &Document { - &self.document - } - - /// Converts the parse into a result. - pub fn into_result(self) -> Result> { - match self.diagnostics { - Some(diagnostics) => Err(diagnostics), - None => Ok(self.document), - } - } -} - /// Represents a single WDL document. /// /// See [Document::ast] for getting a version-specific Abstract @@ -227,10 +169,9 @@ impl Document { /// /// ```rust /// # use wdl_ast::{Document, AstToken, Ast}; - /// let parse = Document::parse("version 1.1"); - /// assert!(parse.diagnostics().is_empty()); + /// let (document, diagnostics) = Document::parse("version 1.1"); + /// assert!(diagnostics.is_empty()); /// - /// let document = parse.document(); /// assert_eq!( /// document /// .version_statement() @@ -247,9 +188,9 @@ impl Document { /// Ast::Unsupported => panic!("should be a V1 AST"), /// } /// ``` - pub fn parse(source: &str) -> Parse { + pub fn parse(source: &str) -> (Self, Vec) { let (tree, diagnostics) = SyntaxTree::parse(source); - Parse::new( + ( Document::cast(tree.into_syntax()).expect("document should cast"), diagnostics, ) diff --git a/wdl-ast/src/v1/decls.rs b/wdl-ast/src/v1/decls.rs index 135516ed..af4c6391 100644 --- a/wdl-ast/src/v1/decls.rs +++ b/wdl-ast/src/v1/decls.rs @@ -14,7 +14,7 @@ use crate::SyntaxNode; use crate::WorkflowDescriptionLanguage; /// Represents a `Map` type. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Eq)] pub struct MapType(SyntaxNode); impl MapType { @@ -38,6 +38,12 @@ impl MapType { } } +impl PartialEq for MapType { + fn eq(&self, other: &Self) -> bool { + self.is_optional() == other.is_optional() && self.types() == other.types() + } +} + impl AstNode for MapType { type Language = WorkflowDescriptionLanguage; @@ -74,20 +80,8 @@ impl fmt::Display for MapType { } } -impl PartialOrd for MapType { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for MapType { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.types().cmp(&other.types()) - } -} - /// Represents an `Array` type. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Eq)] pub struct ArrayType(SyntaxNode); impl ArrayType { @@ -110,6 +104,14 @@ impl ArrayType { } } +impl PartialEq for ArrayType { + fn eq(&self, other: &Self) -> bool { + self.is_optional() == other.is_optional() + && self.is_non_empty() == other.is_non_empty() + && self.element_type() == other.element_type() + } +} + impl AstNode for ArrayType { type Language = WorkflowDescriptionLanguage; @@ -147,26 +149,8 @@ impl fmt::Display for ArrayType { } } -impl PartialOrd for ArrayType { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for ArrayType { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - if self.is_non_empty() && !other.is_non_empty() { - std::cmp::Ordering::Less - } else if !self.is_non_empty() && other.is_non_empty() { - std::cmp::Ordering::Greater - } else { - self.element_type().cmp(&other.element_type()) - } - } -} - /// Represents a `Pair` type. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Eq)] pub struct PairType(SyntaxNode); impl PairType { @@ -187,6 +171,12 @@ impl PairType { } } +impl PartialEq for PairType { + fn eq(&self, other: &Self) -> bool { + self.is_optional() == other.is_optional() && self.types() == other.types() + } +} + impl AstNode for PairType { type Language = WorkflowDescriptionLanguage; @@ -223,20 +213,8 @@ impl fmt::Display for PairType { } } -impl PartialOrd for PairType { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for PairType { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.types().cmp(&other.types()) - } -} - /// Represents a `Object` type. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Eq)] pub struct ObjectType(SyntaxNode); impl ObjectType { @@ -249,6 +227,12 @@ impl ObjectType { } } +impl PartialEq for ObjectType { + fn eq(&self, other: &Self) -> bool { + self.is_optional() == other.is_optional() + } +} + impl AstNode for ObjectType { type Language = WorkflowDescriptionLanguage; @@ -284,20 +268,8 @@ impl fmt::Display for ObjectType { } } -impl PartialOrd for ObjectType { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for ObjectType { - fn cmp(&self, _: &Self) -> std::cmp::Ordering { - std::cmp::Ordering::Equal - } -} - /// Represents a reference to a type. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Eq)] pub struct TypeRef(SyntaxNode); impl TypeRef { @@ -315,6 +287,12 @@ impl TypeRef { } } +impl PartialEq for TypeRef { + fn eq(&self, other: &Self) -> bool { + self.is_optional() == other.is_optional() && self.name().as_str() == other.name().as_str() + } +} + impl AstNode for TypeRef { type Language = WorkflowDescriptionLanguage; @@ -351,18 +329,6 @@ impl fmt::Display for TypeRef { } } -impl PartialOrd for TypeRef { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for TypeRef { - fn cmp(&self, _: &Self) -> std::cmp::Ordering { - std::cmp::Ordering::Equal - } -} - /// Represents a kind of primitive type. #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum PrimitiveTypeKind { @@ -379,7 +345,7 @@ pub enum PrimitiveTypeKind { } /// Represents a primitive type. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, Eq)] pub struct PrimitiveType(SyntaxNode); impl PrimitiveType { @@ -405,16 +371,11 @@ impl PrimitiveType { Some(SyntaxKind::QuestionMark) ) } +} - /// Defines an ordering for PrimitiveTypes - fn primitive_type_index(&self) -> usize { - match self.kind() { - PrimitiveTypeKind::Boolean => 2, - PrimitiveTypeKind::Integer => 4, - PrimitiveTypeKind::Float => 3, - PrimitiveTypeKind::String => 1, - PrimitiveTypeKind::File => 0, - } +impl PartialEq for PrimitiveType { + fn eq(&self, other: &Self) -> bool { + self.kind() == other.kind() } } @@ -461,19 +422,6 @@ impl fmt::Display for PrimitiveType { } } -impl PartialOrd for PrimitiveType { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for PrimitiveType { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.primitive_type_index() - .cmp(&other.primitive_type_index()) - } -} - /// Represents a type. #[derive(Clone, Debug, PartialEq, Eq)] pub enum Type { @@ -575,27 +523,6 @@ impl Type { _ => panic!("not a primitive type"), } } - - /// Defines an ordering for types. - fn type_index(&self) -> usize { - match self { - Type::Map(_) => 5, - Type::Array(a) => match a.is_non_empty() { - true => 1, - false => 2, - }, - Type::Pair(_) => 6, - Type::Object(_) => 4, - Type::Ref(_) => 3, - Type::Primitive(p) => match p.kind() { - PrimitiveTypeKind::Boolean => 8, - PrimitiveTypeKind::Integer => 10, - PrimitiveTypeKind::Float => 9, - PrimitiveTypeKind::String => 7, - PrimitiveTypeKind::File => 0, - }, - } - } } impl AstNode for Type { @@ -656,31 +583,6 @@ impl fmt::Display for Type { } } -impl PartialOrd for Type { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for Type { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - compare_types(self, other) - } -} - -/// Compare all variants of Type. -fn compare_types(a: &Type, b: &Type) -> std::cmp::Ordering { - // Check Array, Map, and Pair for sub-types - match (a, b) { - (Type::Map(a), Type::Map(b)) => a.cmp(b), - (Type::Array(a), Type::Array(b)) => a.cmp(b), - (Type::Pair(a), Type::Pair(b)) => a.cmp(b), - (Type::Ref(a), Type::Ref(b)) => a.cmp(b), - (Type::Object(a), Type::Object(b)) => a.cmp(b), - _ => a.type_index().cmp(&b.type_index()), - } -} - /// Represents an unbound declaration. #[derive(Clone, Debug, PartialEq, Eq)] pub struct UnboundDecl(pub(crate) SyntaxNode); @@ -827,20 +729,6 @@ impl Decl { _ => panic!("not an unbound declaration"), } } - - /// Define an ordering for declarations. - fn decl_index(&self) -> usize { - match self { - Self::Bound(b) => match b.ty().is_optional() { - true => 2, - false => 3, - }, - Self::Unbound(u) => match u.ty().is_optional() { - true => 1, - false => 0, - }, - } - } } impl AstNode for Decl { @@ -872,33 +760,6 @@ impl AstNode for Decl { } } -impl PartialOrd for Decl { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for Decl { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - compare_decl(self, other) - } -} - -/// Compare all variants of Decl. -fn compare_decl(a: &Decl, b: &Decl) -> std::cmp::Ordering { - if (matches!(a, Decl::Bound(_)) - && matches!(b, Decl::Bound(_)) - && a.ty().is_optional() == b.ty().is_optional()) - || (matches!(a, Decl::Unbound(_)) - && matches!(b, Decl::Unbound(_)) - && a.ty().is_optional() == b.ty().is_optional()) - { - a.ty().cmp(&b.ty()) - } else { - a.decl_index().cmp(&b.decl_index()) - } -} - #[cfg(test)] mod test { use super::*; @@ -908,7 +769,7 @@ mod test { #[test] fn decls() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -929,7 +790,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -1032,6 +893,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn bound_decl(&mut self, _: &mut Self::State, reason: VisitReason, _: &BoundDecl) { if reason == VisitReason::Enter { self.bound += 1; diff --git a/wdl-ast/src/v1/expr.rs b/wdl-ast/src/v1/expr.rs index 4e73c0f1..d1435c50 100644 --- a/wdl-ast/src/v1/expr.rs +++ b/wdl-ast/src/v1/expr.rs @@ -2082,7 +2082,7 @@ mod test { #[test] fn literal_booleans() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -2093,7 +2093,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -2120,6 +2120,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -2138,7 +2140,7 @@ task test { #[test] fn literal_integer() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -2155,7 +2157,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -2274,6 +2276,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -2304,7 +2308,7 @@ task test { #[test] fn literal_float() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -2321,7 +2325,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -2441,6 +2445,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -2464,7 +2470,7 @@ task test { #[test] fn literal_string() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -2477,7 +2483,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -2543,6 +2549,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -2564,7 +2572,7 @@ task test { #[test] fn literal_array() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -2576,7 +2584,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -2774,6 +2782,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -2822,7 +2832,7 @@ task test { #[test] fn literal_pair() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -2834,7 +2844,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -2970,6 +2980,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -3019,7 +3031,7 @@ task test { #[test] fn literal_map() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3030,7 +3042,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3097,6 +3109,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -3139,7 +3153,7 @@ task test { #[test] fn literal_object() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3150,7 +3164,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3226,6 +3240,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -3285,18 +3301,18 @@ task test { #[test] fn literal_struct() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 -task test { +task test { Foo a = Foo { foo: "bar" } Bar b = Bar { bar: 1, baz: [1, 2, 3] } } "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3374,6 +3390,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -3433,7 +3451,7 @@ task test { #[test] fn literal_none() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3444,7 +3462,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3473,6 +3491,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Literal(LiteralExpr::None(_)) = expr { @@ -3489,7 +3509,7 @@ task test { #[test] fn name_ref() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3500,7 +3520,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3535,6 +3555,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; @@ -3553,7 +3575,7 @@ task test { #[test] fn parenthesized() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3564,7 +3586,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3614,6 +3636,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Parenthesized(_) = expr { @@ -3630,7 +3654,7 @@ task test { #[test] fn if_expr() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3641,7 +3665,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3682,6 +3706,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::If(_) = expr { @@ -3698,7 +3724,7 @@ task test { #[test] fn logical_not() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3709,7 +3735,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3757,6 +3783,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::LogicalNot(_) = expr { @@ -3773,7 +3801,7 @@ task test { #[test] fn negation() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3784,7 +3812,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3834,6 +3862,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Negation(_) = expr { @@ -3850,7 +3880,7 @@ task test { #[test] fn logical_or() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3862,7 +3892,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3896,6 +3926,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::LogicalOr(_) = expr { @@ -3912,7 +3944,7 @@ task test { #[test] fn logical_and() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3924,7 +3956,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -3958,6 +3990,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::LogicalAnd(_) = expr { @@ -3974,7 +4008,7 @@ task test { #[test] fn equality() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -3986,7 +4020,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4020,6 +4054,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Equality(_) = expr { @@ -4036,7 +4072,7 @@ task test { #[test] fn inequality() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4048,7 +4084,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4082,6 +4118,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Inequality(_) = expr { @@ -4098,7 +4136,7 @@ task test { #[test] fn less() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4110,7 +4148,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4160,6 +4198,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Less(_) = expr { @@ -4176,7 +4216,7 @@ task test { #[test] fn less_equal() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4188,7 +4228,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4238,6 +4278,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::LessEqual(_) = expr { @@ -4254,7 +4296,7 @@ task test { #[test] fn greater() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4266,7 +4308,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4316,6 +4358,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Greater(_) = expr { @@ -4332,7 +4376,7 @@ task test { #[test] fn greater_equal() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4344,7 +4388,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4394,6 +4438,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::GreaterEqual(_) = expr { @@ -4410,7 +4456,7 @@ task test { #[test] fn addition() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4422,7 +4468,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4472,6 +4518,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Addition(_) = expr { @@ -4488,7 +4536,7 @@ task test { #[test] fn subtraction() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4500,7 +4548,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4550,6 +4598,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Subtraction(_) = expr { @@ -4566,7 +4616,7 @@ task test { #[test] fn multiplication() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4578,7 +4628,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4628,6 +4678,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Multiplication(_) = expr { @@ -4644,7 +4696,7 @@ task test { #[test] fn division() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4656,7 +4708,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4706,6 +4758,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Division(_) = expr { @@ -4722,7 +4776,7 @@ task test { #[test] fn modulo() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4734,7 +4788,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4784,6 +4838,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Modulo(_) = expr { @@ -4800,7 +4856,7 @@ task test { #[test] fn exponentiation() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.2 @@ -4812,7 +4868,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4880,7 +4936,7 @@ task test { #[test] fn call() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4891,7 +4947,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -4965,6 +5021,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Call(_) = expr { @@ -4981,7 +5039,7 @@ task test { #[test] fn index() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -4992,7 +5050,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -5054,6 +5112,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Index(_) = expr { @@ -5070,7 +5130,7 @@ task test { #[test] fn access() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -5081,7 +5141,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -5127,6 +5187,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn expr(&mut self, _: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Enter { if let Expr::Access(_) = expr { diff --git a/wdl-ast/src/v1/import.rs b/wdl-ast/src/v1/import.rs index c4e625ac..4c6199b0 100644 --- a/wdl-ast/src/v1/import.rs +++ b/wdl-ast/src/v1/import.rs @@ -168,7 +168,7 @@ mod test { #[test] fn import_statements() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -178,7 +178,7 @@ import "baz.wdl" alias A as B alias C as D import "qux.wdl" as x alias A as B alias C as D "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); match document.ast() { Ast::V1(ast) => { let assert_aliases = |mut aliases: AstChildren| { @@ -232,6 +232,8 @@ import "qux.wdl" as x alias A as B alias C as D impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn import_statement( &mut self, _: &mut Self::State, diff --git a/wdl-ast/src/v1/struct.rs b/wdl-ast/src/v1/struct.rs index abebd04a..82247461 100644 --- a/wdl-ast/src/v1/struct.rs +++ b/wdl-ast/src/v1/struct.rs @@ -63,7 +63,7 @@ mod test { #[test] fn struct_definitions() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -96,7 +96,7 @@ struct ComplexTypes { } "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let structs: Vec<_> = ast.structs().collect(); @@ -225,6 +225,8 @@ struct ComplexTypes { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn struct_definition( &mut self, _: &mut Self::State, diff --git a/wdl-ast/src/v1/task.rs b/wdl-ast/src/v1/task.rs index 4f5f26a3..36535193 100644 --- a/wdl-ast/src/v1/task.rs +++ b/wdl-ast/src/v1/task.rs @@ -925,7 +925,7 @@ mod test { #[test] fn tasks() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -962,7 +962,7 @@ task test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let tasks: Vec<_> = ast.tasks().collect(); @@ -1122,6 +1122,8 @@ task test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn task_definition( &mut self, _: &mut Self::State, diff --git a/wdl-ast/src/v1/workflow.rs b/wdl-ast/src/v1/workflow.rs index ab78ffd1..d52681e5 100644 --- a/wdl-ast/src/v1/workflow.rs +++ b/wdl-ast/src/v1/workflow.rs @@ -578,7 +578,7 @@ mod test { #[test] fn workflows() { - let parse = Document::parse( + let (document, diagnostics) = Document::parse( r#" version 1.1 @@ -622,7 +622,7 @@ workflow test { "#, ); - let document = parse.into_result().expect("there should be no errors"); + assert!(diagnostics.is_empty()); let ast = document.ast(); let ast = ast.as_v1().expect("should be a V1 AST"); let workflows: Vec<_> = ast.workflows().collect(); @@ -914,6 +914,8 @@ workflow test { impl Visitor for MyVisitor { type State = (); + fn document(&mut self, _: &mut Self::State, _: VisitReason, _: &Document) {} + fn workflow_definition( &mut self, _: &mut Self::State, diff --git a/wdl-ast/src/validation.rs b/wdl-ast/src/validation.rs index c0e2a6c4..96264034 100644 --- a/wdl-ast/src/validation.rs +++ b/wdl-ast/src/validation.rs @@ -1,7 +1,6 @@ //! Validator for WDL documents. use std::cmp::Ordering; -use std::sync::Arc; use super::v1; use super::Comment; @@ -14,7 +13,6 @@ use crate::Visitor; mod counts; mod keys; -mod names; mod numbers; mod strings; mod version; @@ -45,10 +43,9 @@ impl Diagnostics { /// /// ```rust /// # use wdl_ast::{Document, Validator}; -/// let document = Document::parse("version 1.1\nworkflow example {}") -/// .into_result() -/// .expect("should parse without errors"); -/// let validator = Validator::default(); +/// let (document, diagnostics) = Document::parse("version 1.1\nworkflow example {}"); +/// assert!(diagnostics.is_empty()); +/// let mut validator = Validator::default(); /// assert!(validator.validate(&document).is_ok()); /// ``` #[allow(missing_debug_implementations)] @@ -59,9 +56,9 @@ pub struct Validator { impl Validator { /// Creates a validator with an empty visitors set. - pub fn empty() -> Self { + pub const fn empty() -> Self { Self { - visitors: Default::default(), + visitors: Vec::new(), } } @@ -80,9 +77,9 @@ impl Validator { /// Validates the given document and returns the validation errors upon /// failure. - pub fn validate(mut self, document: &Document) -> Result<(), Arc<[Diagnostic]>> { + pub fn validate(&mut self, document: &Document) -> Result<(), Vec> { let mut diagnostics = Diagnostics::default(); - document.visit(&mut diagnostics, &mut self); + document.visit(&mut diagnostics, self); if diagnostics.0.is_empty() { Ok(()) @@ -96,7 +93,7 @@ impl Validator { (Some(_), None) => Ordering::Greater, (Some(a), Some(b)) => a.span().start().cmp(&b.span().start()), }); - Err(diagnostics.0.into()) + Err(diagnostics.0) } } } @@ -110,7 +107,6 @@ impl Default for Validator { Box::::default(), Box::::default(), Box::::default(), - Box::::default(), Box::::default(), ], } diff --git a/wdl-ast/src/validation/counts.rs b/wdl-ast/src/validation/counts.rs index adf4343a..1abc24bd 100644 --- a/wdl-ast/src/validation/counts.rs +++ b/wdl-ast/src/validation/counts.rs @@ -1,4 +1,4 @@ -//! Validation of various counts in a V1 AST. +//! Validation of various counts in an AST. use std::fmt; @@ -13,6 +13,7 @@ use crate::v1::StructDefinition; use crate::v1::TaskDefinition; use crate::v1::TaskOrWorkflow; use crate::v1::WorkflowDefinition; +use crate::Ast; use crate::AstNode; use crate::AstToken; use crate::Diagnostic; @@ -59,16 +60,8 @@ impl fmt::Display for Section { /// Creates a "at least one definition" diagnostic fn at_least_one_definition() -> Diagnostic { Diagnostic::error("there must be at least one task, workflow, or struct definition in the file") -} - -/// Creates a "duplicate workflow" diagnostic -fn duplicate_workflow(name: Ident, first: Span) -> Diagnostic { - Diagnostic::error(format!( - "cannot define workflow `{name}` as only one workflow is allowed per source file", - name = name.as_str() - )) - .with_label("consider moving this workflow to a new file", name.span()) - .with_label("first workflow is defined here", first) + // This highlight will show the last position in the file + .with_highlight(Span::new(usize::MAX - 1, 1)) } /// Creates a "missing command section" diagnostic @@ -138,8 +131,8 @@ fn empty_struct(name: Ident) -> Diagnostic { /// * Contains non-empty structs #[derive(Default, Debug)] pub struct CountingVisitor { - /// The span of the first workflow in the file. - workflow: Option, + /// Whether or not the document has at least one workflow. + has_workflow: bool, /// Whether or not the document has at least one task. has_task: bool, /// Whether or not the document has at least one struct. @@ -174,34 +167,35 @@ impl CountingVisitor { impl Visitor for CountingVisitor { type State = Diagnostics; - fn document(&mut self, state: &mut Self::State, reason: VisitReason, _: &Document) { + fn document(&mut self, state: &mut Self::State, reason: VisitReason, doc: &Document) { if reason == VisitReason::Enter { + // Upon entry of of a document, reset the visitor entirely. + *self = Default::default(); + return; + } + + // Ignore documents that are not supported + if matches!(doc.ast(), Ast::Unsupported) { return; } - if self.workflow.is_none() && !self.has_task && !self.has_struct { + if !self.has_workflow && !self.has_task && !self.has_struct { state.add(at_least_one_definition()); } } fn workflow_definition( &mut self, - state: &mut Self::State, + _: &mut Self::State, reason: VisitReason, - workflow: &WorkflowDefinition, + _: &WorkflowDefinition, ) { if reason == VisitReason::Exit { self.reset(); return; } - if let Some(first) = self.workflow { - state.add(duplicate_workflow(workflow.name(), first)); - return; - } - - let name = workflow.name(); - self.workflow = Some(name.span()); + self.has_workflow = true; } fn task_definition( diff --git a/wdl-ast/src/validation/keys.rs b/wdl-ast/src/validation/keys.rs index b960d226..6221a3b2 100644 --- a/wdl-ast/src/validation/keys.rs +++ b/wdl-ast/src/validation/keys.rs @@ -1,4 +1,4 @@ -//! Validation of unique keys in a V1 AST. +//! Validation of unique keys in an AST. use std::collections::HashMap; use std::fmt; @@ -12,6 +12,7 @@ use crate::v1::RuntimeSection; use crate::AstToken; use crate::Diagnostic; use crate::Diagnostics; +use crate::Document; use crate::Ident; use crate::Span; use crate::VisitReason; @@ -91,6 +92,15 @@ pub struct UniqueKeysVisitor(HashMap); impl Visitor for UniqueKeysVisitor { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn runtime_section( &mut self, state: &mut Self::State, diff --git a/wdl-ast/src/validation/names.rs b/wdl-ast/src/validation/names.rs deleted file mode 100644 index bcdfd37c..00000000 --- a/wdl-ast/src/validation/names.rs +++ /dev/null @@ -1,378 +0,0 @@ -//! Validation of unique names in a V1 AST. - -use std::collections::HashMap; -use std::fmt; - -use rowan::ast::AstNode; -use wdl_grammar::Diagnostic; -use wdl_grammar::Span; -use wdl_grammar::ToSpan; - -use crate::v1::BoundDecl; -use crate::v1::CallStatement; -use crate::v1::ImportStatement; -use crate::v1::ScatterStatement; -use crate::v1::StructDefinition; -use crate::v1::TaskDefinition; -use crate::v1::UnboundDecl; -use crate::v1::WorkflowDefinition; -use crate::AstToken; -use crate::Diagnostics; -use crate::Ident; -use crate::VisitReason; -use crate::Visitor; - -/// Represents the context of a name. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -enum NameContext { - /// The name is a workflow name. - Workflow(Span), - /// The name is a task name. - Task(Span), - /// The name is a struct name. - Struct(Span), - /// The name is a struct member name. - StructMember(Span), - /// The name is a declaration name. - Declaration(Span), - /// The name is from a call statement. - Call(Span), - /// The name is a scatter variable. - ScatterVariable(Span), -} - -impl NameContext { - /// Gets the span of the name. - fn span(&self) -> Span { - match self { - Self::Workflow(s) => *s, - Self::Task(s) => *s, - Self::Struct(s) => *s, - Self::StructMember(s) => *s, - Self::Declaration(s) => *s, - Self::Call(s) => *s, - Self::ScatterVariable(s) => *s, - } - } -} - -impl fmt::Display for NameContext { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Workflow(_) => write!(f, "workflow"), - Self::Task(_) => write!(f, "task"), - Self::Struct(_) => write!(f, "struct"), - Self::StructMember(_) => write!(f, "struct member"), - Self::Declaration(_) => write!(f, "declaration"), - Self::Call(_) => write!(f, "call"), - Self::ScatterVariable(_) => write!(f, "scatter variable"), - } - } -} - -/// Creates a "name conflict" diagnostic -fn name_conflict(name: &str, conflicting: NameContext, first: NameContext) -> Diagnostic { - Diagnostic::error(format!("conflicting {conflicting} name `{name}`")) - .with_label( - format!("this conflicts with a {first} of the same name"), - conflicting.span(), - ) - .with_label( - format!("the {first} with the conflicting name is here"), - first.span(), - ) -} - -/// Creates a "namespace conflict" diagnostic -fn namespace_conflict(name: &str, conflicting: Span, first: Span, suggest_fix: bool) -> Diagnostic { - let diagnostic = Diagnostic::error(format!("conflicting import namespace `{name}`")) - .with_label("this conflicts with another import namespace", conflicting) - .with_label( - "the conflicting import namespace was introduced here", - first, - ); - - if suggest_fix { - diagnostic.with_fix("add an `as` clause to the import to specify a namespace") - } else { - diagnostic - } -} - -/// Creates a "call conflict" diagnostic -fn call_conflict(name: Ident, first: NameContext, suggest_fix: bool) -> Diagnostic { - let diagnostic = Diagnostic::error(format!( - "conflicting call name `{name}`", - name = name.as_str() - )) - .with_label( - format!("this conflicts with a {first} of the same name"), - name.span(), - ) - .with_label( - format!("the {first} with the conflicting name is here"), - first.span(), - ); - - if suggest_fix { - diagnostic.with_fix("add an `as` clause to the call to specify a different name") - } else { - diagnostic - } -} - -/// Creates an "invalid import namespace" diagnostic -fn invalid_import_namespace(span: Span) -> Diagnostic { - Diagnostic::error("import namespace is not a valid WDL identifier") - .with_label( - "a namespace name cannot be derived from this import path", - span, - ) - .with_fix("add an `as` clause to the import to specify a namespace") -} - -/// A visitor of unique names within an AST. -/// -/// Ensures that the following names are unique: -/// -/// * Workflow names. -/// * Task names. -/// * Struct names from struct declarations and import aliases. -/// * Struct member names. -/// * Declarations and scatter variable names. -#[derive(Debug, Default)] -pub struct UniqueNamesVisitor { - /// A map of namespace names to the span that introduced the name. - namespaces: HashMap, - /// A map of task and workflow names to the span of the first name. - tasks_and_workflows: HashMap, - /// A map of struct names to the span of the first name. - structs: HashMap, - /// A map of decl names to the context of what introduced the name. - /// - /// This map is cleared upon entry to a workflow, task, or struct. - decls: HashMap, - /// Whether or not we're inside a struct definition. - inside_struct: bool, -} - -impl Visitor for UniqueNamesVisitor { - type State = Diagnostics; - - fn import_statement( - &mut self, - state: &mut Self::State, - reason: VisitReason, - stmt: &ImportStatement, - ) { - if reason == VisitReason::Exit { - return; - } - - // Check for unique namespace name - match stmt.namespace() { - Some((ns, span)) => { - if let Some(first) = self.namespaces.get(&ns) { - state.add(namespace_conflict( - &ns, - span, - *first, - stmt.explicit_namespace().is_none(), - )); - } else { - self.namespaces.insert(ns, span); - } - } - None => { - state.add(invalid_import_namespace( - stmt.uri().syntax().text_range().to_span(), - )); - } - } - - // Check for unique struct aliases - for alias in stmt.aliases() { - let (_, name) = alias.names(); - if let Some(first) = self.structs.get(name.as_str()) { - state.add(name_conflict( - name.as_str(), - NameContext::Struct(name.span()), - NameContext::Struct(*first), - )); - } else { - self.structs.insert(name.as_str().to_string(), name.span()); - } - } - } - - fn workflow_definition( - &mut self, - state: &mut Self::State, - reason: VisitReason, - workflow: &WorkflowDefinition, - ) { - if reason == VisitReason::Exit { - return; - } - - self.decls.clear(); - - let name = workflow.name(); - let context = NameContext::Workflow(name.span()); - if let Some(first) = self.tasks_and_workflows.get(name.as_str()) { - state.add(name_conflict(name.as_str(), context, *first)); - } else { - self.tasks_and_workflows - .insert(name.as_str().to_string(), context); - } - } - - fn task_definition( - &mut self, - state: &mut Self::State, - reason: VisitReason, - task: &TaskDefinition, - ) { - if reason == VisitReason::Exit { - return; - } - - self.decls.clear(); - - let name = task.name(); - let context = NameContext::Task(name.span()); - if let Some(first) = self.tasks_and_workflows.get(name.as_str()) { - state.add(name_conflict(name.as_str(), context, *first)); - } else { - self.tasks_and_workflows - .insert(name.as_str().to_string(), context); - } - } - - fn struct_definition( - &mut self, - state: &mut Self::State, - reason: VisitReason, - def: &StructDefinition, - ) { - if reason == VisitReason::Exit { - self.inside_struct = false; - return; - } - - self.inside_struct = true; - self.decls.clear(); - - let name = def.name(); - if let Some(first) = self.structs.get(name.as_str()) { - state.add(name_conflict( - name.as_str(), - NameContext::Struct(name.span()), - NameContext::Struct(*first), - )); - } else { - self.structs.insert(name.as_str().to_string(), name.span()); - } - } - - fn bound_decl(&mut self, state: &mut Self::State, reason: VisitReason, decl: &BoundDecl) { - if reason == VisitReason::Exit { - return; - } - - let name = decl.name(); - let context = NameContext::Declaration(name.span()); - if let Some(first) = self.decls.get_mut(name.as_str()) { - state.add(name_conflict(name.as_str(), context, *first)); - - // If the name came from a scatter variable, "promote" this declaration as the - // source of any additional conflicts. - if let NameContext::ScatterVariable(_) = first { - *first = context; - } - } else { - self.decls.insert(name.as_str().to_string(), context); - } - } - - fn unbound_decl(&mut self, state: &mut Self::State, reason: VisitReason, decl: &UnboundDecl) { - if reason == VisitReason::Exit { - return; - } - - let name = decl.name(); - let context = if self.inside_struct { - NameContext::StructMember(name.span()) - } else { - NameContext::Declaration(name.span()) - }; - - if let Some(first) = self.decls.get_mut(name.as_str()) { - state.add(name_conflict(name.as_str(), context, *first)); - - // If the name came from a scatter variable, "promote" this declaration as the - // source of any additional conflicts. - if let NameContext::ScatterVariable(_) = first { - *first = context; - } - } else { - self.decls.insert(name.as_str().to_string(), context); - } - } - - fn scatter_statement( - &mut self, - state: &mut Self::State, - reason: VisitReason, - stmt: &ScatterStatement, - ) { - let name = stmt.variable(); - if reason == VisitReason::Exit { - // Check to see if this scatter statement introduced the name - // If so, remove it from the set - if let NameContext::ScatterVariable(span) = &self.decls[name.as_str()] { - if name.span() == *span { - self.decls.remove(name.as_str()); - } - } - - return; - } - - let context = NameContext::ScatterVariable(name.span()); - if let Some(first) = self.decls.get(name.as_str()) { - state.add(name_conflict(name.as_str(), context, *first)); - } else { - self.decls.insert(name.as_str().to_string(), context); - } - } - - fn call_statement( - &mut self, - state: &mut Self::State, - reason: VisitReason, - stmt: &CallStatement, - ) { - if reason == VisitReason::Exit { - return; - } - - // Call statements introduce a declaration from the result - let (name, aliased) = stmt - .alias() - .map(|a| (a.name(), true)) - .unwrap_or_else(|| (stmt.target().name().1, false)); - let context = NameContext::Call(name.span()); - if let Some(first) = self.decls.get_mut(name.as_str()) { - state.add(call_conflict(name, *first, !aliased)); - - // If the name came from a scatter variable, "promote" this declaration as the - // source of any additional conflicts. - if let NameContext::ScatterVariable(_) = first { - *first = context; - } - } else { - self.decls.insert(name.as_str().to_string(), context); - } - } -} diff --git a/wdl-ast/src/validation/numbers.rs b/wdl-ast/src/validation/numbers.rs index 6bfb19a2..b0ddc0cd 100644 --- a/wdl-ast/src/validation/numbers.rs +++ b/wdl-ast/src/validation/numbers.rs @@ -1,4 +1,4 @@ -//! Validation of number literals in a V1 AST. +//! Validation of number literals in an AST. use crate::support; use crate::v1::Expr; @@ -7,6 +7,7 @@ use crate::AstNode; use crate::AstToken; use crate::Diagnostic; use crate::Diagnostics; +use crate::Document; use crate::Span; use crate::SyntaxKind; use crate::ToSpan; @@ -45,6 +46,15 @@ pub struct NumberVisitor { impl Visitor for NumberVisitor { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn expr(&mut self, state: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { self.negation_start = None; diff --git a/wdl-ast/src/validation/strings.rs b/wdl-ast/src/validation/strings.rs index fa2d96c2..69fbdbef 100644 --- a/wdl-ast/src/validation/strings.rs +++ b/wdl-ast/src/validation/strings.rs @@ -1,4 +1,4 @@ -//! Validation of string literals in a V1 AST. +//! Validation of string literals in an AST. use wdl_grammar::lexer::v1::EscapeToken; use wdl_grammar::lexer::v1::Logos; @@ -7,7 +7,9 @@ use crate::v1::StringText; use crate::AstToken; use crate::Diagnostic; use crate::Diagnostics; +use crate::Document; use crate::Span; +use crate::VisitReason; use crate::Visitor; /// Creates an "unknown escape sequence" diagnostic @@ -115,12 +117,21 @@ fn check_text(diagnostics: &mut Diagnostics, start: usize, text: &str) { /// /// * Does not contain characters that must be escaped. /// * Does not contain invalid escape sequences. -#[derive(Debug)] +#[derive(Default, Debug)] pub struct LiteralTextVisitor; impl Visitor for LiteralTextVisitor { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn string_text(&mut self, state: &mut Self::State, text: &StringText) { check_text( state, diff --git a/wdl-ast/src/visitor.rs b/wdl-ast/src/visitor.rs index c4cc577a..5a7e52da 100644 --- a/wdl-ast/src/visitor.rs +++ b/wdl-ast/src/visitor.rs @@ -62,7 +62,11 @@ pub trait Visitor: Send + Sync { type State; /// Visits the root document node. - fn document(&mut self, state: &mut Self::State, reason: VisitReason, doc: &Document) {} + /// + /// A visitor must implement this method and response to + /// `VisitReason::Enter` with resetting any internal state so that a visitor + /// may be reused between documents. + fn document(&mut self, state: &mut Self::State, reason: VisitReason, doc: &Document); /// Visits a whitespace token. fn whitespace(&mut self, state: &mut Self::State, whitespace: &Whitespace) {} diff --git a/wdl-ast/tests/validation.rs b/wdl-ast/tests/validation.rs index 26f3d82b..fc2cff86 100644 --- a/wdl-ast/tests/validation.rs +++ b/wdl-ast/tests/validation.rs @@ -1,4 +1,4 @@ -//! The experimental parser validation tests. +//! The parser validation tests. //! //! This test looks for directories in `tests/validation`. //! @@ -9,8 +9,6 @@ //! //! The `source.errors` file may be automatically generated or updated by //! setting the `BLESS` environment variable when running this test. -//! -//! This test currently requires the `experimental` feature to run. use std::collections::HashSet; use std::env; @@ -127,23 +125,23 @@ fn run_test(test: &Path, ntests: &AtomicUsize) -> Result<(), String> { ) })? .replace("\r\n", "\n"); - match Document::parse(&source).into_result() { - Ok(document) => { - let validator = Validator::default(); - let errors = match validator.validate(&document) { - Ok(()) => String::new(), - Err(diagnostics) => format_diagnostics(&diagnostics, &path, &source), - }; - compare_result(&path.with_extension("errors"), &errors, true)?; - } - Err(diagnostics) => { - compare_result( - &path.with_extension("errors"), - &format_diagnostics(&diagnostics, &path, &source), - true, - )?; - } + + let (document, diagnostics) = Document::parse(&source); + if !diagnostics.is_empty() { + compare_result( + &path.with_extension("errors"), + &format_diagnostics(&diagnostics, &path, &source), + true, + )?; + } else { + let mut validator = Validator::default(); + let errors = match validator.validate(&document) { + Ok(()) => String::new(), + Err(diagnostics) => format_diagnostics(&diagnostics, &path, &source), + }; + compare_result(&path.with_extension("errors"), &errors, true)?; } + ntests.fetch_add(1, Ordering::SeqCst); Ok(()) } diff --git a/wdl-ast/tests/validation/conflicting-decl-names/source.errors b/wdl-ast/tests/validation/conflicting-decl-names/source.errors deleted file mode 100644 index 40434cb6..00000000 --- a/wdl-ast/tests/validation/conflicting-decl-names/source.errors +++ /dev/null @@ -1,108 +0,0 @@ -error: conflicting declaration name `x` - ┌─ tests/validation/conflicting-decl-names/source.wdl:13:16 - │ - 7 │ Int x - │ - the declaration with the conflicting name is here - · -13 │ String x = "x" - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `y` - ┌─ tests/validation/conflicting-decl-names/source.wdl:14:16 - │ - 8 │ Int y = 0 - │ - the declaration with the conflicting name is here - · -14 │ String y = "y" - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `x` - ┌─ tests/validation/conflicting-decl-names/source.wdl:19:9 - │ - 7 │ Int x - │ - the declaration with the conflicting name is here - · -19 │ Int x = y - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `x` - ┌─ tests/validation/conflicting-decl-names/source.wdl:32:16 - │ -26 │ Int x - │ - the declaration with the conflicting name is here - · -32 │ String x = "x" - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `y` - ┌─ tests/validation/conflicting-decl-names/source.wdl:33:16 - │ -27 │ Int y = 0 - │ - the declaration with the conflicting name is here - · -33 │ String y = "y" - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `x` - ┌─ tests/validation/conflicting-decl-names/source.wdl:38:9 - │ -26 │ Int x - │ - the declaration with the conflicting name is here - · -38 │ Int x = y - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `b` - ┌─ tests/validation/conflicting-decl-names/source.wdl:45:17 - │ -28 │ String b - │ - the declaration with the conflicting name is here - · -45 │ Int b = 0 - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `x2` - ┌─ tests/validation/conflicting-decl-names/source.wdl:46:17 - │ -41 │ Int x2 = 0 - │ -- the declaration with the conflicting name is here - · -46 │ Int x2 = 0 - │ ^^ this conflicts with a declaration of the same name - -error: conflicting scatter variable name `x` - ┌─ tests/validation/conflicting-decl-names/source.wdl:51:14 - │ -26 │ Int x - │ - the declaration with the conflicting name is here - · -51 │ scatter (x in [1, 2, 3]) { - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `z` - ┌─ tests/validation/conflicting-decl-names/source.wdl:52:13 - │ -37 │ Int z = x - │ - the declaration with the conflicting name is here - · -52 │ Int z = x - │ ^ this conflicts with a declaration of the same name - -error: conflicting declaration name `nested` - ┌─ tests/validation/conflicting-decl-names/source.wdl:57:21 - │ -55 │ scatter (nested in [1, 2, 3]) { - │ ------ the scatter variable with the conflicting name is here -56 │ scatter (baz in [1, 2, 3]) { -57 │ Int nested = 0 - │ ^^^^^^ this conflicts with a scatter variable of the same name - -error: conflicting declaration name `nested` - ┌─ tests/validation/conflicting-decl-names/source.wdl:64:13 - │ -57 │ Int nested = 0 - │ ------ the declaration with the conflicting name is here - · -64 │ Int nested = 0 - │ ^^^^^^ this conflicts with a declaration of the same name - diff --git a/wdl-ast/tests/validation/conflicting-imports/source.errors b/wdl-ast/tests/validation/conflicting-imports/source.errors deleted file mode 100644 index 0c84bd2b..00000000 --- a/wdl-ast/tests/validation/conflicting-imports/source.errors +++ /dev/null @@ -1,77 +0,0 @@ -error: conflicting import namespace `foo` - ┌─ tests/validation/conflicting-imports/source.wdl:6:8 - │ -5 │ import "foo.wdl" # First - │ --------- the conflicting import namespace was introduced here -6 │ import "foo" # Conflicts - │ ^^^^^ this conflicts with another import namespace - │ - = fix: add an `as` clause to the import to specify a namespace - -error: import namespace is not a valid WDL identifier - ┌─ tests/validation/conflicting-imports/source.wdl:7:8 - │ -7 │ import "bad-file-name.wdl" # Bad name - │ ^^^^^^^^^^^^^^^^^^^ a namespace name cannot be derived from this import path - │ - = fix: add an `as` clause to the import to specify a namespace - -error: conflicting import namespace `baz` - ┌─ tests/validation/conflicting-imports/source.wdl:12:22 - │ -11 │ import "/baz.wdl" # First - │ ---------- the conflicting import namespace was introduced here -12 │ import "/Baz.wdl" as baz # Conflicts - │ ^^^ this conflicts with another import namespace - -error: conflicting import namespace `baz` - ┌─ tests/validation/conflicting-imports/source.wdl:13:8 - │ -11 │ import "/baz.wdl" # First - │ ---------- the conflicting import namespace was introduced here -12 │ import "/Baz.wdl" as baz # Conflicts -13 │ import "../foo/bar/baz.wdl" # Conflicts - │ ^^^^^^^^^^^^^^^^^^^^ this conflicts with another import namespace - │ - = fix: add an `as` clause to the import to specify a namespace - -error: conflicting import namespace `foo` - ┌─ tests/validation/conflicting-imports/source.wdl:14:8 - │ - 5 │ import "foo.wdl" # First - │ --------- the conflicting import namespace was introduced here - · -14 │ import "https://example.com/foo.wdl#something" # Conflicts - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this conflicts with another import namespace - │ - = fix: add an `as` clause to the import to specify a namespace - -error: conflicting import namespace `qux` - ┌─ tests/validation/conflicting-imports/source.wdl:16:8 - │ -15 │ import "https://example.com/qux.wdl?query=nope" # First - │ ---------------------------------------- the conflicting import namespace was introduced here -16 │ import "qux.wdl" # Conflicts - │ ^^^^^^^^^ this conflicts with another import namespace - │ - = fix: add an `as` clause to the import to specify a namespace - -error: conflicting import namespace `foo` - ┌─ tests/validation/conflicting-imports/source.wdl:17:8 - │ - 5 │ import "foo.wdl" # First - │ --------- the conflicting import namespace was introduced here - · -17 │ import "https://example.com/%66%6F%6F.wdl" # Conflicts - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this conflicts with another import namespace - │ - = fix: add an `as` clause to the import to specify a namespace - -error: import namespace is not a valid WDL identifier - ┌─ tests/validation/conflicting-imports/source.wdl:18:8 - │ -18 │ import "https://example.com?query=foo" # Bad name - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ a namespace name cannot be derived from this import path - │ - = fix: add an `as` clause to the import to specify a namespace - diff --git a/wdl-ast/tests/validation/conflicting-imports/source.wdl b/wdl-ast/tests/validation/conflicting-imports/source.wdl deleted file mode 100644 index fef1a09d..00000000 --- a/wdl-ast/tests/validation/conflicting-imports/source.wdl +++ /dev/null @@ -1,20 +0,0 @@ -## This is a test of conflicting imports. - -version 1.1 - -import "foo.wdl" # First -import "foo" # Conflicts -import "bad-file-name.wdl" # Bad name -import "foo" as bar # First -import "Baz" # First -import "BAZ" # First -import "/baz.wdl" # First -import "/Baz.wdl" as baz # Conflicts -import "../foo/bar/baz.wdl" # Conflicts -import "https://example.com/foo.wdl#something" # Conflicts -import "https://example.com/qux.wdl?query=nope" # First -import "qux.wdl" # Conflicts -import "https://example.com/%66%6F%6F.wdl" # Conflicts -import "https://example.com?query=foo" # Bad name - -workflow test {} diff --git a/wdl-ast/tests/validation/conflicting-struct-names/source.errors b/wdl-ast/tests/validation/conflicting-struct-names/source.errors deleted file mode 100644 index ca6a9348..00000000 --- a/wdl-ast/tests/validation/conflicting-struct-names/source.errors +++ /dev/null @@ -1,44 +0,0 @@ -error: conflicting struct name `Foo` - ┌─ tests/validation/conflicting-struct-names/source.wdl:15:8 - │ - 7 │ struct Foo { - │ --- the struct with the conflicting name is here - · -15 │ struct Foo { - │ ^^^ this conflicts with a struct of the same name - -error: conflicting struct name `Bar` - ┌─ tests/validation/conflicting-struct-names/source.wdl:19:8 - │ -11 │ struct Bar { - │ --- the struct with the conflicting name is here - · -19 │ struct Bar { - │ ^^^ this conflicts with a struct of the same name - -error: conflicting struct name `Baz` - ┌─ tests/validation/conflicting-struct-names/source.wdl:23:8 - │ - 5 │ import "foo" alias Foo as Baz - │ --- the struct with the conflicting name is here - · -23 │ struct Baz { - │ ^^^ this conflicts with a struct of the same name - -error: conflicting struct name `Foo` - ┌─ tests/validation/conflicting-struct-names/source.wdl:27:27 - │ - 7 │ struct Foo { - │ --- the struct with the conflicting name is here - · -27 │ import "Bar" alias Baz as Foo - │ ^^^ this conflicts with a struct of the same name - -error: conflicting struct name `Qux` - ┌─ tests/validation/conflicting-struct-names/source.wdl:29:40 - │ -29 │ import "qux" alias A as Qux alias B as Qux - │ --- ^^^ this conflicts with a struct of the same name - │ │ - │ the struct with the conflicting name is here - diff --git a/wdl-ast/tests/validation/conflicting-workflow-names/source.errors b/wdl-ast/tests/validation/conflicting-workflow-names/source.errors deleted file mode 100644 index 46f2e48b..00000000 --- a/wdl-ast/tests/validation/conflicting-workflow-names/source.errors +++ /dev/null @@ -1,34 +0,0 @@ -error: conflicting workflow name `foo` - ┌─ tests/validation/conflicting-workflow-names/source.wdl:9:10 - │ -5 │ task foo { - │ --- the task with the conflicting name is here - · -9 │ workflow foo {} - │ ^^^ this conflicts with a task of the same name - -error: cannot define workflow `bar` as only one workflow is allowed per source file - ┌─ tests/validation/conflicting-workflow-names/source.wdl:10:10 - │ - 9 │ workflow foo {} - │ --- first workflow is defined here -10 │ workflow bar {} - │ ^^^ consider moving this workflow to a new file - -error: cannot define workflow `bar` as only one workflow is allowed per source file - ┌─ tests/validation/conflicting-workflow-names/source.wdl:11:10 - │ - 9 │ workflow foo {} - │ --- first workflow is defined here -10 │ workflow bar {} -11 │ workflow bar {} - │ ^^^ consider moving this workflow to a new file - -error: conflicting workflow name `bar` - ┌─ tests/validation/conflicting-workflow-names/source.wdl:11:10 - │ -10 │ workflow bar {} - │ --- the workflow with the conflicting name is here -11 │ workflow bar {} - │ ^^^ this conflicts with a workflow of the same name - diff --git a/wdl-ast/tests/validation/empty/source.errors b/wdl-ast/tests/validation/empty/source.errors index 776fee5c..282c9743 100644 --- a/wdl-ast/tests/validation/empty/source.errors +++ b/wdl-ast/tests/validation/empty/source.errors @@ -1,2 +1,6 @@ error: there must be at least one task, workflow, or struct definition in the file + ┌─ tests/validation/empty/source.wdl:4:1 + │ +4 │ + │ diff --git a/wdl-gauntlet/src/lib.rs b/wdl-gauntlet/src/lib.rs index 2c85e34c..7a462e6a 100644 --- a/wdl-gauntlet/src/lib.rs +++ b/wdl-gauntlet/src/lib.rs @@ -180,27 +180,25 @@ pub async fn gauntlet(args: Args) -> Result<()> { // Parse and validate the document // If "arena mode" is activated, also validate with the lint rules enabled let before = Instant::now(); - let diagnostics = match Document::parse(&source).into_result() { - Ok(document) => { - let mut validator = Validator::default(); - if args.arena { - validator.add_visitor(LintVisitor::default()); - } - - match validator.validate(&document) { - Ok(()) => None, - Err(diagnostics) => Some(diagnostics), - } - } - Err(diagnostics) => { + let (document, diagnostics) = Document::parse(&source); + let diagnostics = if !diagnostics.is_empty() { + if args.arena { // If we're in arena mode, skip over the files that failed to fully parse // We log those diagnostics as part of the normal gauntlet run. - if args.arena { - trace!("skipping {:?} as it has parse errors", abs_path); - continue; - } + trace!("skipping {:?} as it has parse errors", abs_path); + continue; + } + + diagnostics + } else { + let mut validator = Validator::default(); + if args.arena { + validator.add_visitor(LintVisitor::default()); + } - Some(diagnostics) + match validator.validate(&document) { + Ok(()) => diagnostics, + Err(diagnostics) => diagnostics, } }; @@ -209,7 +207,7 @@ pub async fn gauntlet(args: Args) -> Result<()> { // Convert the diagnostics to a set of short-form messages let mut actual = IndexSet::new(); - if let Some(diagnostics) = diagnostics { + if !diagnostics.is_empty() { let file: SimpleFile<_, _> = SimpleFile::new( Path::new(document_identifier.path()) .file_name() diff --git a/wdl-grammar/CHANGELOG.md b/wdl-grammar/CHANGELOG.md index 0d54831e..17fa1b3f 100644 --- a/wdl-grammar/CHANGELOG.md +++ b/wdl-grammar/CHANGELOG.md @@ -11,6 +11,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Add support for the exponentiation operator in WDL 1.2 ([#111](https://github.com/stjude-rust-labs/wdl/pull/111)). +### Fixed + +* The diagnostic for missing a version statement in an empty file now points to + the last position in the file so that the file that caused the error is + attached to the diagnostic ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). + ## 0.5.0 - 06-28-2024 ### Fixed diff --git a/wdl-grammar/src/grammar.rs b/wdl-grammar/src/grammar.rs index 85112d91..2eb48e1c 100644 --- a/wdl-grammar/src/grammar.rs +++ b/wdl-grammar/src/grammar.rs @@ -108,6 +108,9 @@ pub fn document(source: &str, mut parser: PreambleParser<'_>) -> (Vec, Ve if let Some((_, span)) = found { diagnostic = diagnostic.with_label("a version statement must come before this", span); + } else { + // This highlight will show the last position in the file + diagnostic = diagnostic.with_highlight(Span::new(usize::MAX - 1, 1)); } (parser, diagnostic) diff --git a/wdl-grammar/tests/parsing.rs b/wdl-grammar/tests/parsing.rs index 338d1c94..9a3bf219 100644 --- a/wdl-grammar/tests/parsing.rs +++ b/wdl-grammar/tests/parsing.rs @@ -1,4 +1,4 @@ -//! The experimental parser file tests. +//! The parser file tests. //! //! This test looks for directories in `tests/parsing`. //! @@ -11,8 +11,6 @@ //! //! Both `source.tree` and `source.errors` may be automatically generated or //! updated by setting the `BLESS` environment variable when running this test. -//! -//! This test currently requires the `experimental` feature to run. use std::collections::HashSet; use std::env; diff --git a/wdl-grammar/tests/parsing/empty-document/source.errors b/wdl-grammar/tests/parsing/empty-document/source.errors index 3c2e8b89..ad797a99 100644 --- a/wdl-grammar/tests/parsing/empty-document/source.errors +++ b/wdl-grammar/tests/parsing/empty-document/source.errors @@ -1,2 +1,6 @@ error: a WDL document must start with a version statement + ┌─ tests/parsing/empty-document/source.wdl:1:1 + │ +1 │ + │ diff --git a/wdl-lint/CHANGELOG.md b/wdl-lint/CHANGELOG.md index 3b0e5c14..5c206802 100644 --- a/wdl-lint/CHANGELOG.md +++ b/wdl-lint/CHANGELOG.md @@ -12,6 +12,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Added the `SectionOrdering` lint rule ([#109](https://github.com/stjude-rust-labs/wdl/pull/109)). * Added the `DeprecatedObject` lint rule ([#112](https://github.com/stjude-rust-labs/wdl/pull/112)). +### Fixed + +* Fixed a bug in the `PreambleWhitespace` rule that would cause it to fire if + there is only a single blank line after the version statement remaining in + the document ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). + +### Changed + +* All lint rule visitations now reset their states upon document entry, + allowing a validator to be reused between documents ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). +* Moved the `PartialOrd` implementation for types into the `InputSorting` rule. + ## 0.3.0 - 06-28-2024 ### Added diff --git a/wdl-lint/src/lib.rs b/wdl-lint/src/lib.rs index e1aa2c98..beafa324 100644 --- a/wdl-lint/src/lib.rs +++ b/wdl-lint/src/lib.rs @@ -10,22 +10,15 @@ //! use wdl_lint::ast::Validator; //! use wdl_lint::LintVisitor; //! -//! match Document::parse(source).into_result() { -//! Ok(document) => { -//! let mut validator = Validator::default(); -//! validator.add_visitor(LintVisitor::default()); -//! match validator.validate(&document) { -//! Ok(_) => { -//! // The document was valid WDL and passed all lints -//! } -//! Err(diagnostics) => { -//! // Handle the failure to validate -//! } -//! } -//! } -//! Err(diagnostics) => { -//! // Handle the failure to parse -//! } +//! let (document, diagnostics) = Document::parse(source); +//! if !diagnostics.is_empty() { +//! // Handle the failure to parse +//! } +//! +//! let mut validator = Validator::default(); +//! validator.add_visitor(LintVisitor::default()); +//! if let Err(diagnostics) = validator.validate(&document) { +//! // Handle the failure to validate //! } //! ``` diff --git a/wdl-lint/src/rules/call_input_spacing.rs b/wdl-lint/src/rules/call_input_spacing.rs index 1f1e177e..3883b2ae 100644 --- a/wdl-lint/src/rules/call_input_spacing.rs +++ b/wdl-lint/src/rules/call_input_spacing.rs @@ -4,6 +4,7 @@ use wdl_ast::v1::CallStatement; use wdl_ast::AstNode; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::SyntaxKind; use wdl_ast::ToSpan; @@ -50,7 +51,7 @@ fn call_input_assignment(span: Span) -> Diagnostic { } /// Detects unsorted input declarations. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct CallInputSpacingRule; impl Rule for CallInputSpacingRule { @@ -80,6 +81,15 @@ impl Rule for CallInputSpacingRule { impl Visitor for CallInputSpacingRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn call_statement( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/command_mixed_indentation.rs b/wdl-lint/src/rules/command_mixed_indentation.rs index a564a693..d93be6ed 100644 --- a/wdl-lint/src/rules/command_mixed_indentation.rs +++ b/wdl-lint/src/rules/command_mixed_indentation.rs @@ -9,6 +9,7 @@ use wdl_ast::AstNode; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::SyntaxKind; use wdl_ast::ToSpan; @@ -73,7 +74,7 @@ fn mixed_indentation(command: Span, span: Span, kind: IndentationKind) -> Diagno } /// Detects mixed indentation in a command section. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct CommandSectionMixedIndentationRule; impl Rule for CommandSectionMixedIndentationRule { @@ -99,6 +100,15 @@ impl Rule for CommandSectionMixedIndentationRule { impl Visitor for CommandSectionMixedIndentationRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn command_section( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/deprecated_object.rs b/wdl-lint/src/rules/deprecated_object.rs index be32e7c2..4b9b3516 100644 --- a/wdl-lint/src/rules/deprecated_object.rs +++ b/wdl-lint/src/rules/deprecated_object.rs @@ -4,6 +4,7 @@ use wdl_ast::span_of; use wdl_ast::v1::Type; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -24,7 +25,7 @@ fn deprecated_object_use(span: Span) -> Diagnostic { } /// Detects the use of the deprecated `Object` types. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct DeprecatedObjectRule; impl Rule for DeprecatedObjectRule { @@ -55,6 +56,15 @@ impl Rule for DeprecatedObjectRule { impl Visitor for DeprecatedObjectRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn bound_decl( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/double_quotes.rs b/wdl-lint/src/rules/double_quotes.rs index baa5eee5..dc29946e 100644 --- a/wdl-lint/src/rules/double_quotes.rs +++ b/wdl-lint/src/rules/double_quotes.rs @@ -5,6 +5,7 @@ use wdl_ast::v1::Expr; use wdl_ast::v1::LiteralExpr; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -25,7 +26,7 @@ fn use_double_quotes(span: Span) -> Diagnostic { } /// Detects strings that are not defined with double quotes. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct DoubleQuotesRule; impl Rule for DoubleQuotesRule { @@ -51,6 +52,15 @@ impl Rule for DoubleQuotesRule { impl Visitor for DoubleQuotesRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn expr(&mut self, state: &mut Self::State, reason: VisitReason, expr: &Expr) { if reason == VisitReason::Exit { return; diff --git a/wdl-lint/src/rules/ending_newline.rs b/wdl-lint/src/rules/ending_newline.rs index 5dfca5e2..a23ada11 100644 --- a/wdl-lint/src/rules/ending_newline.rs +++ b/wdl-lint/src/rules/ending_newline.rs @@ -1,5 +1,6 @@ //! A lint rule for newlines at the end of the document. +use wdl_ast::Ast; use wdl_ast::AstNode; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; @@ -41,7 +42,7 @@ fn multiple_ending_newline(span: Span, count: usize) -> Diagnostic { } /// Detects missing newline at the end of the document. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct EndingNewlineRule; impl Rule for EndingNewlineRule { @@ -68,6 +69,13 @@ impl Visitor for EndingNewlineRule { fn document(&mut self, state: &mut Self::State, reason: VisitReason, doc: &Document) { if reason == VisitReason::Enter { // We only process on exit so that it's one of the last diagnostics emitted + // Reset the visitor upon document entry + *self = Default::default(); + return; + } + + // Don't run on a document without a supported version + if matches!(doc.ast(), Ast::Unsupported) { return; } diff --git a/wdl-lint/src/rules/import_placement.rs b/wdl-lint/src/rules/import_placement.rs index 0fd492b2..15fc4a0f 100644 --- a/wdl-lint/src/rules/import_placement.rs +++ b/wdl-lint/src/rules/import_placement.rs @@ -7,6 +7,7 @@ use wdl_ast::v1::WorkflowDefinition; use wdl_ast::AstNode; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::ToSpan; use wdl_ast::VisitReason; @@ -59,6 +60,15 @@ impl Rule for ImportPlacementRule { impl Visitor for ImportPlacementRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn import_statement( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/import_sort.rs b/wdl-lint/src/rules/import_sort.rs index daf8e10d..85d63dcb 100644 --- a/wdl-lint/src/rules/import_sort.rs +++ b/wdl-lint/src/rules/import_sort.rs @@ -4,6 +4,7 @@ use wdl_ast::v1::ImportStatement; use wdl_ast::AstNode; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::SyntaxKind; use wdl_ast::SyntaxNode; @@ -35,7 +36,7 @@ fn improper_comment(span: Span) -> Diagnostic { } /// Detects imports that are not sorted lexicographically. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct ImportSortRule; impl Rule for ImportSortRule { @@ -62,32 +63,13 @@ impl Rule for ImportSortRule { impl Visitor for ImportSortRule { type State = Diagnostics; - fn import_statement( - &mut self, - state: &mut Self::State, - reason: VisitReason, - stmt: &ImportStatement, - ) { + fn document(&mut self, state: &mut Self::State, reason: VisitReason, doc: &Document) { if reason == VisitReason::Exit { return; } - // Check for comments inside this import statement. - let internal_comments = stmt - .syntax() - .children_with_tokens() - .filter(|c| c.kind() == SyntaxKind::Comment) - .map(|c| c.into_token().unwrap()); - - for comment in internal_comments { - state.add(improper_comment(comment.text_range().to_span())); - } - } - - fn document(&mut self, state: &mut Self::State, reason: VisitReason, doc: &wdl_ast::Document) { - if reason == VisitReason::Exit { - return; - } + // Reset the visitor upon document entry + *self = Default::default(); let imports = doc .syntax() @@ -106,4 +88,26 @@ impl Visitor for ImportSortRule { prev_import = Some(import); } } + + fn import_statement( + &mut self, + state: &mut Self::State, + reason: VisitReason, + stmt: &ImportStatement, + ) { + if reason == VisitReason::Exit { + return; + } + + // Check for comments inside this import statement. + let internal_comments = stmt + .syntax() + .children_with_tokens() + .filter(|c| c.kind() == SyntaxKind::Comment) + .map(|c| c.into_token().unwrap()); + + for comment in internal_comments { + state.add(improper_comment(comment.text_range().to_span())); + } + } } diff --git a/wdl-lint/src/rules/import_whitespace.rs b/wdl-lint/src/rules/import_whitespace.rs index f6c59aa6..cde3e200 100644 --- a/wdl-lint/src/rules/import_whitespace.rs +++ b/wdl-lint/src/rules/import_whitespace.rs @@ -4,6 +4,7 @@ use wdl_ast::v1::ImportStatement; use wdl_ast::AstNode; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::SyntaxElement; use wdl_ast::SyntaxKind; @@ -78,6 +79,15 @@ impl Rule for ImportWhitespaceRule { impl Visitor for ImportWhitespaceRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn import_statement( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/inconsistent_newlines.rs b/wdl-lint/src/rules/inconsistent_newlines.rs index 9c76876d..98f27dbd 100644 --- a/wdl-lint/src/rules/inconsistent_newlines.rs +++ b/wdl-lint/src/rules/inconsistent_newlines.rs @@ -57,7 +57,14 @@ impl Visitor for InconsistentNewlinesRule { type State = Diagnostics; fn document(&mut self, state: &mut Self::State, reason: VisitReason, _doc: &wdl_ast::Document) { - if reason == VisitReason::Exit && self.newline > 0 && self.carriage_return > 0 { + if reason == VisitReason::Enter { + // We only process on exit so that it's one of the last diagnostics emitted + // Reset the visitor upon document entry + *self = Default::default(); + return; + } + + if self.newline > 0 && self.carriage_return > 0 { state.add(inconsistent_newlines(self.first_inconsistent.unwrap())); } } diff --git a/wdl-lint/src/rules/input_not_sorted.rs b/wdl-lint/src/rules/input_not_sorted.rs index c10b133f..192d3f9e 100644 --- a/wdl-lint/src/rules/input_not_sorted.rs +++ b/wdl-lint/src/rules/input_not_sorted.rs @@ -1,10 +1,15 @@ //! A lint rule for sorting of inputs. +use std::cmp::Ordering; + use wdl_ast::span_of; -use wdl_ast::v1::InputSection; +use wdl_ast::v1; +use wdl_ast::v1::PrimitiveType; use wdl_ast::AstNode; +use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -24,8 +29,160 @@ fn input_not_sorted(span: Span, sorted_inputs: String) -> Diagnostic { .with_fix(format!("sort input statements as: \n{}", sorted_inputs)) } +/// Define an ordering for declarations. +fn decl_index(decl: &v1::Decl) -> usize { + match decl { + v1::Decl::Bound(b) => { + if b.ty().is_optional() { + 2 + } else { + 3 + } + } + v1::Decl::Unbound(u) => { + if u.ty().is_optional() { + 1 + } else { + 0 + } + } + } +} + +/// Defines an ordering for types. +fn type_index(ty: &v1::Type) -> usize { + match ty { + v1::Type::Map(_) => 5, + v1::Type::Array(a) => { + if a.is_non_empty() { + 1 + } else { + 2 + } + } + v1::Type::Pair(_) => 6, + v1::Type::Object(_) => 4, + v1::Type::Ref(_) => 3, + v1::Type::Primitive(p) => match p.kind() { + v1::PrimitiveTypeKind::Boolean => 8, + v1::PrimitiveTypeKind::Integer => 10, + v1::PrimitiveTypeKind::Float => 9, + v1::PrimitiveTypeKind::String => 7, + v1::PrimitiveTypeKind::File => 0, + }, + } +} + +/// Defines an ordering for PrimitiveTypes +fn primitive_type_index(ty: &PrimitiveType) -> usize { + match ty.kind() { + v1::PrimitiveTypeKind::Boolean => 2, + v1::PrimitiveTypeKind::Integer => 4, + v1::PrimitiveTypeKind::Float => 3, + v1::PrimitiveTypeKind::String => 1, + v1::PrimitiveTypeKind::File => 0, + } +} + +/// Compares the ordering of two map types. +fn compare_map_types(a: &v1::MapType, b: &v1::MapType) -> Ordering { + let (akey, aty) = a.types(); + let (bkey, bty) = b.types(); + + let cmp = primitive_type_index(&akey).cmp(&primitive_type_index(&bkey)); + if cmp != Ordering::Equal { + return cmp; + } + + let cmp = compare_types(&aty, &bty); + if cmp != Ordering::Equal { + return cmp; + } + + // Optional check is inverted + b.is_optional().cmp(&a.is_optional()) +} + +/// Compares the ordering of two array types. +fn compare_array_types(a: &v1::ArrayType, b: &v1::ArrayType) -> Ordering { + let cmp = compare_types(&a.element_type(), &b.element_type()); + if cmp != Ordering::Equal { + return cmp; + } + + // Non-empty is inverted + let cmp = b.is_non_empty().cmp(&a.is_non_empty()); + if cmp != Ordering::Equal { + return cmp; + } + + // Optional check is inverted + b.is_optional().cmp(&a.is_optional()) +} + +/// Compares the ordering of two pair types. +fn compare_pair_types(a: &v1::PairType, b: &v1::PairType) -> Ordering { + let (afirst, asecond) = a.types(); + let (bfirst, bsecond) = b.types(); + + let cmp = compare_types(&afirst, &bfirst); + if cmp != Ordering::Equal { + return cmp; + } + + let cmp = compare_types(&asecond, &bsecond); + if cmp != Ordering::Equal { + return cmp; + } + + // Optional check is inverted + b.is_optional().cmp(&a.is_optional()) +} + +/// Compares the ordering of two type references. +fn compare_type_refs(a: &v1::TypeRef, b: &v1::TypeRef) -> Ordering { + let cmp = a.name().as_str().cmp(b.name().as_str()); + if cmp != Ordering::Equal { + return cmp; + } + + // Optional check is inverted + b.is_optional().cmp(&a.is_optional()) +} + +/// Compares the ordering of two types. +fn compare_types(a: &v1::Type, b: &v1::Type) -> Ordering { + // Check Array, Map, and Pair for sub-types + match (a, b) { + (v1::Type::Map(a), v1::Type::Map(b)) => compare_map_types(a, b), + (v1::Type::Array(a), v1::Type::Array(b)) => compare_array_types(a, b), + (v1::Type::Pair(a), v1::Type::Pair(b)) => compare_pair_types(a, b), + (v1::Type::Ref(a), v1::Type::Ref(b)) => compare_type_refs(a, b), + (v1::Type::Object(a), v1::Type::Object(b)) => { + // Optional check is inverted + b.is_optional().cmp(&a.is_optional()) + } + _ => type_index(a).cmp(&type_index(b)), + } +} + +/// Compares two declarations for sorting. +fn compare_decl(a: &v1::Decl, b: &v1::Decl) -> Ordering { + if (matches!(a, v1::Decl::Bound(_)) + && matches!(b, v1::Decl::Bound(_)) + && a.ty().is_optional() == b.ty().is_optional()) + || (matches!(a, v1::Decl::Unbound(_)) + && matches!(b, v1::Decl::Unbound(_)) + && a.ty().is_optional() == b.ty().is_optional()) + { + compare_types(&a.ty(), &b.ty()) + } else { + decl_index(a).cmp(&decl_index(b)) + } +} + /// Detects unsorted input declarations. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct InputNotSortedRule; impl Rule for InputNotSortedRule { @@ -57,11 +214,20 @@ impl Rule for InputNotSortedRule { impl Visitor for InputNotSortedRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn input_section( &mut self, state: &mut Self::State, reason: VisitReason, - input: &InputSection, + input: &v1::InputSection, ) { if reason == VisitReason::Exit { return; @@ -70,7 +236,7 @@ impl Visitor for InputNotSortedRule { // Get input section declarations let decls: Vec<_> = input.declarations().collect(); let mut sorted_decls = decls.clone(); - sorted_decls.sort(); + sorted_decls.sort_by(compare_decl); let input_string: String = sorted_decls .clone() diff --git a/wdl-lint/src/rules/line_width.rs b/wdl-lint/src/rules/line_width.rs index fd84ae1a..f1623983 100644 --- a/wdl-lint/src/rules/line_width.rs +++ b/wdl-lint/src/rules/line_width.rs @@ -1,10 +1,14 @@ //! Ensures that lines do not exceed a certain width. +use wdl_ast::v1; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; +use wdl_ast::VisitReason; use wdl_ast::Visitor; +use wdl_ast::Whitespace; use crate::Rule; use crate::Tag; @@ -33,6 +37,14 @@ pub struct LineWidthRule { } impl LineWidthRule { + /// Constructs a new line width rule with the given maximum line width. + pub fn new(max_width: usize) -> Self { + Self { + max_width, + ..Default::default() + } + } + /// Detects lines that exceed a certain width. fn detect_line_too_long(&mut self, state: &mut Diagnostics, text: &str, start: usize) { let mut cur_newline_offset = start; @@ -94,43 +106,41 @@ impl Rule for LineWidthRule { impl Visitor for LineWidthRule { type State = Diagnostics; - fn whitespace(&mut self, state: &mut Self::State, whitespace: &wdl_ast::Whitespace) { + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Self { + max_width: self.max_width, + ..Default::default() + }; + } + + fn whitespace(&mut self, state: &mut Self::State, whitespace: &Whitespace) { self.detect_line_too_long(state, whitespace.as_str(), whitespace.span().start()); } - fn command_text(&mut self, state: &mut Self::State, text: &wdl_ast::v1::CommandText) { + fn command_text(&mut self, state: &mut Self::State, text: &v1::CommandText) { self.detect_line_too_long(state, text.as_str(), text.span().start()) } fn metadata_section( &mut self, _: &mut Self::State, - reason: wdl_ast::VisitReason, - _: &wdl_ast::v1::MetadataSection, + reason: VisitReason, + _: &v1::MetadataSection, ) { - match reason { - wdl_ast::VisitReason::Enter => { - self.should_ignore = true; - } - wdl_ast::VisitReason::Exit => { - self.should_ignore = false; - } - } + self.should_ignore = matches!(reason, VisitReason::Enter); } fn parameter_metadata_section( &mut self, _: &mut Self::State, - reason: wdl_ast::VisitReason, - _: &wdl_ast::v1::ParameterMetadataSection, + reason: VisitReason, + _: &v1::ParameterMetadataSection, ) { - match reason { - wdl_ast::VisitReason::Enter => { - self.should_ignore = true; - } - wdl_ast::VisitReason::Exit => { - self.should_ignore = false; - } - } + self.should_ignore = matches!(reason, VisitReason::Enter); } } diff --git a/wdl-lint/src/rules/matching_parameter_meta.rs b/wdl-lint/src/rules/matching_parameter_meta.rs index 2ba996fe..b13cedda 100644 --- a/wdl-lint/src/rules/matching_parameter_meta.rs +++ b/wdl-lint/src/rules/matching_parameter_meta.rs @@ -10,6 +10,7 @@ use wdl_ast::v1::WorkflowDefinition; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -63,7 +64,7 @@ fn extra_param_meta(parent: &TaskOrWorkflow, extra: &str, span: Span) -> Diagnos } /// Detects missing or extraneous entries in a `parameter_meta` section. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct MatchingParameterMetaRule; impl Rule for MatchingParameterMetaRule { @@ -127,6 +128,15 @@ fn check_parameter_meta( impl Visitor for MatchingParameterMetaRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn task_definition( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/missing_metas.rs b/wdl-lint/src/rules/missing_metas.rs index 80665f35..90d0defe 100644 --- a/wdl-lint/src/rules/missing_metas.rs +++ b/wdl-lint/src/rules/missing_metas.rs @@ -7,6 +7,7 @@ use wdl_ast::v1::WorkflowDefinition; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -81,7 +82,7 @@ fn missing_sections(name: &str, context: Context, span: Span) -> Diagnostic { } /// A lint rule for missing meta and parameter_meta sections. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct MissingMetasRule; impl Rule for MissingMetasRule { @@ -107,6 +108,15 @@ impl Rule for MissingMetasRule { impl Visitor for MissingMetasRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn task_definition( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/missing_output.rs b/wdl-lint/src/rules/missing_output.rs index eab6f42f..00d41030 100644 --- a/wdl-lint/src/rules/missing_output.rs +++ b/wdl-lint/src/rules/missing_output.rs @@ -7,6 +7,7 @@ use wdl_ast::v1::WorkflowDefinition; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -44,7 +45,7 @@ fn missing_output_section(name: &str, context: Context, span: Span) -> Diagnosti } /// Detects missing `output` section for tasks and workflows. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct MissingOutputRule; impl Rule for MissingOutputRule { @@ -70,6 +71,15 @@ impl Rule for MissingOutputRule { impl Visitor for MissingOutputRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn task_definition( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/missing_runtime.rs b/wdl-lint/src/rules/missing_runtime.rs index 497bbf8e..ad780674 100644 --- a/wdl-lint/src/rules/missing_runtime.rs +++ b/wdl-lint/src/rules/missing_runtime.rs @@ -4,6 +4,7 @@ use wdl_ast::v1::TaskDefinition; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -24,7 +25,7 @@ fn missing_runtime_section(task: &str, span: Span) -> Diagnostic { } /// Detects missing `runtime` section for tasks. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct MissingRuntimeRule; impl Rule for MissingRuntimeRule { @@ -48,6 +49,15 @@ impl Rule for MissingRuntimeRule { impl Visitor for MissingRuntimeRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn task_definition( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/no_curly_commands.rs b/wdl-lint/src/rules/no_curly_commands.rs index 98efbeba..cd27b326 100644 --- a/wdl-lint/src/rules/no_curly_commands.rs +++ b/wdl-lint/src/rules/no_curly_commands.rs @@ -6,6 +6,7 @@ use wdl_ast::AstNode; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::SyntaxKind; use wdl_ast::ToSpan; @@ -30,7 +31,7 @@ fn curly_commands(task: &str, span: Span) -> Diagnostic { } /// Detects curly command section for tasks. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct NoCurlyCommandsRule; impl Rule for NoCurlyCommandsRule { @@ -56,6 +57,15 @@ impl Rule for NoCurlyCommandsRule { impl Visitor for NoCurlyCommandsRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn command_section( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/pascal_case.rs b/wdl-lint/src/rules/pascal_case.rs index a3475537..3a04a647 100644 --- a/wdl-lint/src/rules/pascal_case.rs +++ b/wdl-lint/src/rules/pascal_case.rs @@ -7,6 +7,7 @@ use wdl_ast::v1::StructDefinition; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -27,7 +28,7 @@ fn use_pascal_case(name: &str, properly_cased_name: &str, span: Span) -> Diagnos } /// Detects structs defined without a pascal case name. -#[derive(Debug, Clone, Copy)] +#[derive(Default, Debug, Clone, Copy)] pub struct PascalCaseRule; impl Rule for PascalCaseRule { @@ -64,6 +65,15 @@ fn check_name(name: &str, span: Span, diagnostics: &mut Diagnostics) { impl Visitor for PascalCaseRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn struct_definition( &mut self, state: &mut Self::State, diff --git a/wdl-lint/src/rules/preamble_comments.rs b/wdl-lint/src/rules/preamble_comments.rs index 6f644674..5a8814de 100644 --- a/wdl-lint/src/rules/preamble_comments.rs +++ b/wdl-lint/src/rules/preamble_comments.rs @@ -4,6 +4,7 @@ use wdl_ast::AstToken; use wdl_ast::Comment; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::SyntaxKind; use wdl_ast::VersionStatement; @@ -74,6 +75,15 @@ impl Rule for PreambleCommentsRule { impl Visitor for PreambleCommentsRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn version_statement( &mut self, _: &mut Self::State, diff --git a/wdl-lint/src/rules/preamble_whitespace.rs b/wdl-lint/src/rules/preamble_whitespace.rs index 0e5d8e4d..aae7a9e1 100644 --- a/wdl-lint/src/rules/preamble_whitespace.rs +++ b/wdl-lint/src/rules/preamble_whitespace.rs @@ -4,6 +4,7 @@ use wdl_ast::AstNode; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::SyntaxElement; use wdl_ast::SyntaxKind; @@ -82,6 +83,15 @@ impl Rule for PreambleWhitespaceRule { impl Visitor for PreambleWhitespaceRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn version_statement( &mut self, state: &mut Self::State, @@ -190,8 +200,12 @@ impl Visitor for PreambleWhitespaceRule { } } - // We expected two lines - if count < 2 { + // We expected two lines or one if the whitespace is the last in the file + if count == 0 + || (count == 1 + && (whitespace.syntax().parent().unwrap().kind() != SyntaxKind::RootNode + || whitespace.syntax().next_sibling_or_token().is_some())) + { state.add(expected_blank_line_after(Span::new( span.start() + span.len(), 1, diff --git a/wdl-lint/src/rules/snake_case.rs b/wdl-lint/src/rules/snake_case.rs index ddacaa53..ef517937 100644 --- a/wdl-lint/src/rules/snake_case.rs +++ b/wdl-lint/src/rules/snake_case.rs @@ -16,6 +16,7 @@ use wdl_ast::v1::WorkflowDefinition; use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::VisitReason; use wdl_ast::Visitor; @@ -126,6 +127,15 @@ impl Rule for SnakeCaseRule { impl Visitor for SnakeCaseRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn struct_definition( &mut self, _state: &mut Self::State, diff --git a/wdl-lint/src/rules/whitespace.rs b/wdl-lint/src/rules/whitespace.rs index 607d35ec..d5a0a374 100644 --- a/wdl-lint/src/rules/whitespace.rs +++ b/wdl-lint/src/rules/whitespace.rs @@ -3,6 +3,7 @@ use wdl_ast::AstToken; use wdl_ast::Diagnostic; use wdl_ast::Diagnostics; +use wdl_ast::Document; use wdl_ast::Span; use wdl_ast::SyntaxKind; use wdl_ast::VersionStatement; @@ -74,6 +75,15 @@ impl Rule for WhitespaceRule { impl Visitor for WhitespaceRule { type State = Diagnostics; + fn document(&mut self, _: &mut Self::State, reason: VisitReason, _: &Document) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + fn version_statement( &mut self, _: &mut Self::State, diff --git a/wdl-lint/src/visitor.rs b/wdl-lint/src/visitor.rs index 5b506bda..cabdd609 100644 --- a/wdl-lint/src/visitor.rs +++ b/wdl-lint/src/visitor.rs @@ -165,6 +165,10 @@ impl Visitor for LintVisitor { fn document(&mut self, state: &mut Self::State, reason: VisitReason, doc: &Document) { if reason == VisitReason::Enter { + // Reset state for a new document + self.global.clear(); + self.exceptions.clear(); + // Set the global exceptions if let Some(stmt) = doc.version_statement() { self.global = self.exceptions_for(state, stmt.syntax()); diff --git a/wdl-lint/tests/lints.rs b/wdl-lint/tests/lints.rs index ff3e3b25..61dd62d4 100644 --- a/wdl-lint/tests/lints.rs +++ b/wdl-lint/tests/lints.rs @@ -126,24 +126,23 @@ fn run_test(test: &Path, ntests: &AtomicUsize) -> Result<(), String> { ) })?; - match Document::parse(&source).into_result() { - Ok(document) => { - let mut validator = Validator::default(); - validator.add_visitor(LintVisitor::default()); - let errors = match validator.validate(&document) { - Ok(()) => String::new(), - Err(diagnostics) => format_diagnostics(&diagnostics, &path, &source), - }; - compare_result(&path.with_extension("errors"), &errors, true)?; - } - Err(diagnostics) => { - compare_result( - &path.with_extension("errors"), - &format_diagnostics(&diagnostics, &path, &source), - true, - )?; - } + let (document, diagnostics) = Document::parse(&source); + if !diagnostics.is_empty() { + compare_result( + &path.with_extension("errors"), + &format_diagnostics(&diagnostics, &path, &source), + true, + )?; + } else { + let mut validator = Validator::default(); + validator.add_visitor(LintVisitor::default()); + let errors = match validator.validate(&document) { + Ok(()) => String::new(), + Err(diagnostics) => format_diagnostics(&diagnostics, &path, &source), + }; + compare_result(&path.with_extension("errors"), &errors, true)?; } + ntests.fetch_add(1, Ordering::SeqCst); Ok(()) } diff --git a/wdl-lint/tests/lints/one-line-after-version/source.errors b/wdl-lint/tests/lints/one-line-after-version/source.errors new file mode 100644 index 00000000..c3558bd3 --- /dev/null +++ b/wdl-lint/tests/lints/one-line-after-version/source.errors @@ -0,0 +1,6 @@ +error: there must be at least one task, workflow, or struct definition in the file + ┌─ tests/lints/one-line-after-version/source.wdl:6:1 + │ +6 │ + │ + diff --git a/wdl-lint/tests/lints/one-line-after-version/source.wdl b/wdl-lint/tests/lints/one-line-after-version/source.wdl new file mode 100644 index 00000000..0969eb51 --- /dev/null +++ b/wdl-lint/tests/lints/one-line-after-version/source.wdl @@ -0,0 +1,5 @@ +## This is a test of having just a single line after a version +## There should only be a single diagnostic in the output about +## needing a task/workflow/struct. + +version 1.1 diff --git a/wdl/CHANGELOG.md b/wdl/CHANGELOG.md index 13f3d507..2c04a846 100644 --- a/wdl/CHANGELOG.md +++ b/wdl/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Changed + +* Changed the `check` command to perform full analysis of the given path ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). + +### Added + +* Added an `analysis` command to perform full analysis and to also print the + result of the analysis; currently it just outputs a debug representation of + the analysis results, but that will change in the future ([#110](https://github.com/stjude-rust-labs/wdl/pull/110)). + ## 0.5.0 - 06-28-2024 ### Changed diff --git a/wdl/Cargo.toml b/wdl/Cargo.toml index 8f0478f5..2c1207b5 100644 --- a/wdl/Cargo.toml +++ b/wdl/Cargo.toml @@ -14,10 +14,14 @@ readme = "../README.md" wdl-grammar = { path = "../wdl-grammar", version = "0.5.0", optional = true } wdl-ast = { path = "../wdl-ast", version = "0.4.0", optional = true } wdl-lint = { path = "../wdl-lint", version = "0.3.0", optional = true } +wdl-analysis = { path = "../wdl-analysis", version = "0.1.0", optional = true } clap = { workspace = true, optional = true } anyhow = { workspace = true, optional = true } colored = { workspace = true, optional = true } codespan-reporting = { workspace = true, optional = true } +env_logger = { workspace = true, optional = true } +indicatif = { workspace = true, optional = true } +tokio = { workspace = true, optional = true } [dev-dependencies] clap = { workspace = true } @@ -30,7 +34,7 @@ ast = ["dep:wdl-ast"] grammar = ["dep:wdl-grammar"] lint = ["dep:wdl-lint"] codespan = ["ast", "wdl-ast/codespan", "dep:codespan-reporting"] -binaries = ["codespan", "lint", "dep:clap", "dep:anyhow", "dep:colored"] +cli = ["codespan", "lint", "dep:wdl-analysis", "dep:clap", "dep:anyhow", "dep:colored", "dep:env_logger", "dep:indicatif", "dep:tokio"] [[example]] name = "explore" @@ -42,7 +46,7 @@ required-features = ["codespan"] [[bin]] name = "wdl" -required-features = ["binaries"] +required-features = ["cli"] [package.metadata.docs.rs] all-features = true diff --git a/wdl/examples/explore.rs b/wdl/examples/explore.rs index 36137e52..d2e53946 100644 --- a/wdl/examples/explore.rs +++ b/wdl/examples/explore.rs @@ -63,47 +63,45 @@ pub fn main() -> Result<()> { ) })?; - match Document::parse(&source).into_result() { - Ok(document) => { - let validator = Validator::default(); - if let Err(diagnostics) = validator.validate(&document) { - emit_diagnostics(&args.path, &source, &diagnostics)?; - } + let (document, diagnostics) = Document::parse(&source); + if !diagnostics.is_empty() { + emit_diagnostics(&args.path, &source, &diagnostics)?; + } - match document.ast() { - Ast::V1(ast) => { - let mut tasks = false; - for (i, task) in ast.tasks().enumerate() { - tasks = true; + let mut validator = Validator::default(); + if let Err(diagnostics) = validator.validate(&document) { + emit_diagnostics(&args.path, &source, &diagnostics)?; + } - if i == 0 { - println!("# Tasks\n"); - } + match document.ast() { + Ast::V1(ast) => { + let mut tasks = false; + for (i, task) in ast.tasks().enumerate() { + tasks = true; - explore_task(&task); - } + if i == 0 { + println!("# Tasks\n"); + } - if tasks { - println!(); - } + explore_task(&task); + } - for (i, workflow) in ast.workflows().enumerate() { - if i == 0 { - println!("# Workflows\n"); - } + if tasks { + println!(); + } - explore_workflow(&workflow); - } + for (i, workflow) in ast.workflows().enumerate() { + if i == 0 { + println!("# Workflows\n"); } - Ast::Unsupported => bail!( - "document `{path}` has an unsupported WDL version", - path = args.path.display() - ), + + explore_workflow(&workflow); } } - Err(diagnostics) => { - emit_diagnostics(&args.path, &source, &diagnostics)?; - } + Ast::Unsupported => bail!( + "document `{path}` has an unsupported WDL version", + path = args.path.display() + ), } Ok(()) diff --git a/wdl/examples/parse.rs b/wdl/examples/parse.rs index b08e9c68..02d3b532 100644 --- a/wdl/examples/parse.rs +++ b/wdl/examples/parse.rs @@ -49,18 +49,17 @@ pub fn main() -> Result<()> { ) })?; - match Document::parse(&source).into_result() { - Ok(document) => { - let validator = Validator::default(); - if let Err(diagnostics) = validator.validate(&document) { - emit_diagnostics(&args.path, &source, &diagnostics)?; - } else { - println!("{document:#?}"); - } - } - Err(diagnostics) => { - emit_diagnostics(&args.path, &source, &diagnostics)?; - } + let (document, diagnostics) = Document::parse(&source); + if !diagnostics.is_empty() { + emit_diagnostics(&args.path, &source, &diagnostics)?; + return Ok(()); + } + + let mut validator = Validator::default(); + if let Err(diagnostics) = validator.validate(&document) { + emit_diagnostics(&args.path, &source, &diagnostics)?; + } else { + println!("{document:#?}"); } Ok(()) diff --git a/wdl/src/bin/wdl.rs b/wdl/src/bin/wdl.rs index e3321ddd..dc5b9c26 100644 --- a/wdl/src/bin/wdl.rs +++ b/wdl/src/bin/wdl.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::fs; use std::io::IsTerminal; use std::io::Read; @@ -15,18 +16,23 @@ use codespan_reporting::term::termcolor::ColorChoice; use codespan_reporting::term::termcolor::StandardStream; use codespan_reporting::term::Config; use colored::Colorize; +use indicatif::ProgressBar; +use indicatif::ProgressStyle; use wdl::ast::Diagnostic; use wdl::ast::Document; +use wdl::ast::SyntaxNode; use wdl::ast::Validator; use wdl::lint::LintVisitor; +use wdl_analysis::AnalysisEngine; +use wdl_analysis::AnalysisResult; /// Emits the given diagnostics to the output stream. /// /// The use of color is determined by the presence of a terminal. /// /// In the future, we might want the color choice to be a CLI argument. -fn emit_diagnostics(path: &Path, source: &str, diagnostics: &[Diagnostic]) -> Result<()> { - let file = SimpleFile::new(path.to_str().context("path should be UTF-8")?, source); +fn emit_diagnostics(path: &str, source: &str, diagnostics: &[Diagnostic]) -> Result<()> { + let file = SimpleFile::new(path, source); let mut stream = StandardStream::stdout(ColorChoice::Auto); for diagnostic in diagnostics.iter() { emit( @@ -41,6 +47,76 @@ fn emit_diagnostics(path: &Path, source: &str, diagnostics: &[Diagnostic]) -> Re Ok(()) } +async fn analyze(path: &Path, lint: bool) -> Result> { + let bar = ProgressBar::new(0); + bar.set_style( + ProgressStyle::with_template("[{elapsed_precise}] {bar:40.cyan/blue} {msg} {pos}/{len}") + .unwrap(), + ); + + let engine = AnalysisEngine::new_with_validator(move || { + let mut validator = Validator::default(); + if lint { + validator.add_visitor(LintVisitor::default()); + } + validator + })?; + + let results = engine + .analyze_with_progress(path, move |kind, completed, total| { + if completed == 0 { + bar.set_length(total.try_into().unwrap()); + bar.set_message(format!("{kind}")); + } + bar.set_position(completed.try_into().unwrap()); + }) + .await; + + let mut count = 0; + let cwd = std::env::current_dir().ok(); + for result in &results { + let path = result.id().path(); + + // Attempt to strip the CWD from the result path + let path = match (&cwd, &path) { + // Use the id itself if there is no path + (_, None) => result.id().to_str(), + // 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.error() { + Some(e) => vec![Diagnostic::error(format!("failed to read `{path}`: {e:#}"))].into(), + None => result.diagnostics().into(), + }; + + if !diagnostics.is_empty() { + emit_diagnostics( + &path, + &result + .root() + .map(|n| SyntaxNode::new_root(n.clone()).text().to_string()) + .unwrap_or(String::new()), + &diagnostics, + )?; + count += diagnostics.len(); + } + } + + engine.shutdown().await; + + if count > 0 { + bail!( + "aborting due to previous {count} diagnostic{s}", + s = if count == 1 { "" } else { "s" } + ); + } + + Ok(results) +} + /// Reads source from the given path. /// /// If the path is simply `-`, the source is read from STDIN. @@ -68,14 +144,14 @@ pub struct ParseCommand { } impl ParseCommand { - fn exec(self) -> Result<()> { + async fn exec(self) -> Result<()> { let source = read_source(&self.path)?; - let parse = Document::parse(&source); - if !parse.diagnostics().is_empty() { - emit_diagnostics(&self.path, &source, parse.diagnostics())?; + let (document, diagnostics) = Document::parse(&source); + if !diagnostics.is_empty() { + emit_diagnostics(&self.path.to_string_lossy(), &source, &diagnostics)?; } - println!("{document:#?}", document = parse.document()); + println!("{document:#?}"); Ok(()) } } @@ -90,32 +166,8 @@ pub struct CheckCommand { } impl CheckCommand { - fn exec(self) -> Result<()> { - let source = read_source(&self.path)?; - match Document::parse(&source).into_result() { - Ok(document) => { - let validator = Validator::default(); - if let Err(diagnostics) = validator.validate(&document) { - emit_diagnostics(&self.path, &source, &diagnostics)?; - - bail!( - "aborting due to previous {count} diagnostic{s}", - count = diagnostics.len(), - s = if diagnostics.len() == 1 { "" } else { "s" } - ); - } - } - Err(diagnostics) => { - emit_diagnostics(&self.path, &source, &diagnostics)?; - - bail!( - "aborting due to previous {count} diagnostic{s}", - count = diagnostics.len(), - s = if diagnostics.len() == 1 { "" } else { "s" } - ); - } - } - + async fn exec(self) -> Result<()> { + analyze(&self.path, false).await?; Ok(()) } } @@ -130,37 +182,56 @@ pub struct LintCommand { } impl LintCommand { - fn exec(self) -> Result<()> { + async fn exec(self) -> Result<()> { let source = read_source(&self.path)?; - match Document::parse(&source).into_result() { - Ok(document) => { - let mut validator = Validator::default(); - validator.add_visitor(LintVisitor::default()); - if let Err(diagnostics) = validator.validate(&document) { - emit_diagnostics(&self.path, &source, &diagnostics)?; - - bail!( - "aborting due to previous {count} diagnostic{s}", - count = diagnostics.len(), - s = if diagnostics.len() == 1 { "" } else { "s" } - ); - } - } - Err(diagnostics) => { - emit_diagnostics(&self.path, &source, &diagnostics)?; - - bail!( - "aborting due to previous {count} diagnostic{s}", - count = diagnostics.len(), - s = if diagnostics.len() == 1 { "" } else { "s" } - ); - } + let (document, diagnostics) = Document::parse(&source); + if !diagnostics.is_empty() { + emit_diagnostics(&self.path.to_string_lossy(), &source, &diagnostics)?; + + bail!( + "aborting due to previous {count} diagnostic{s}", + count = diagnostics.len(), + s = if diagnostics.len() == 1 { "" } else { "s" } + ); + } + + let mut validator = Validator::default(); + validator.add_visitor(LintVisitor::default()); + if let Err(diagnostics) = validator.validate(&document) { + emit_diagnostics(&self.path.to_string_lossy(), &source, &diagnostics)?; + + bail!( + "aborting due to previous {count} diagnostic{s}", + count = diagnostics.len(), + s = if diagnostics.len() == 1 { "" } else { "s" } + ); } Ok(()) } } +/// Analyzes a WDL source file. +#[derive(Args)] +#[clap(disable_version_flag = true)] +pub struct AnalyzeCommand { + /// The path to the source WDL file. + #[clap(value_name = "PATH")] + pub path: PathBuf, + + /// Whether or not to run lints as part of analysis. + #[clap(long)] + pub lint: bool, +} + +impl AnalyzeCommand { + async fn exec(self) -> Result<()> { + let results = analyze(&self.path, self.lint).await?; + println!("{:#?}", results); + Ok(()) + } +} + /// A tool for parsing, validating, and linting WDL source code. #[derive(Parser)] #[clap( @@ -173,13 +244,21 @@ enum App { Parse(ParseCommand), Check(CheckCommand), Lint(LintCommand), + Analyze(AnalyzeCommand), } -fn main() -> Result<()> { +#[tokio::main] +async fn main() -> Result<()> { + env_logger::builder() + .format_module_path(false) + .format_target(false) + .init(); + if let Err(e) = match App::parse() { - App::Parse(cmd) => cmd.exec(), - App::Check(cmd) => cmd.exec(), - App::Lint(cmd) => cmd.exec(), + App::Parse(cmd) => cmd.exec().await, + App::Check(cmd) => cmd.exec().await, + App::Lint(cmd) => cmd.exec().await, + App::Analyze(cmd) => cmd.exec().await, } { eprintln!( "{error}: {e:?}", diff --git a/wdl/src/lib.rs b/wdl/src/lib.rs index 69f72de6..2327284e 100644 --- a/wdl/src/lib.rs +++ b/wdl/src/lib.rs @@ -39,21 +39,14 @@ //! use wdl::ast::Document; //! use wdl::ast::Validator; //! -//! match Document::parse(source).into_result() { -//! Ok(document) => { -//! let validator = Validator::default(); -//! match validator.validate(&document) { -//! Ok(_) => { -//! // The document was valid WDL -//! } -//! Err(diagnostics) => { -//! // Handle the failure to validate -//! } -//! } -//! } -//! Err(diagnostics) => { -//! // Handle the failure to parse -//! } +//! let (document, diagnostics) = Document::parse(source); +//! if !diagnostics.is_empty() { +//! // Handle the failure to parse +//! } +//! +//! let mut validator = Validator::default(); +//! if let Err(diagnostics) = validator.validate(&document) { +//! // Handle the failure to validate //! } //! ``` //! @@ -65,22 +58,15 @@ //! use wdl::ast::Validator; //! use wdl::lint::LintVisitor; //! -//! match Document::parse(source).into_result() { -//! Ok(document) => { -//! let mut validator = Validator::default(); -//! validator.add_visitor(LintVisitor::default()); -//! match validator.validate(&document) { -//! Ok(_) => { -//! // The document was valid WDL and passed all lints -//! } -//! Err(diagnostics) => { -//! // Handle the failure to validate -//! } -//! } -//! } -//! Err(diagnostics) => { -//! // Handle the failure to parse -//! } +//! let (document, diagnostics) = Document::parse(source); +//! if !diagnostics.is_empty() { +//! // Handle the failure to parse +//! } +//! +//! let mut validator = Validator::default(); +//! validator.add_visitor(LintVisitor::default()); +//! if let Err(diagnostics) = validator.validate(&document) { +//! // Handle the failure to validate //! } //! ```