Skip to content

Commit

Permalink
feat: type checking in tasks.
Browse files Browse the repository at this point in the history
This commit implements full type checking in tasks.

* Detection of cycles in declarations.
* Type calculations of expressions.
* Type checking of string interpolations.

Workflow type checking is forthcoming.

The following have not been implemented yet, which is required for full 1.2
support:

* support `task` name (with "task" hidden type) in commands and output
  declarations.
* support `input`, `output`, and `hints` hidden types in literal expression
  evaluation.

Additionally, the following changes are included:

* Refactored `wdl-analysis` to make certain modules public rather than
  reexporting everything at the root (cleans up the docs a bit).
* Removed DFS space from `DocumentGraph` as it doesn't need to persist after
  analysis completes.
* Simplified `DocumentScope` to contain only whats needed to express names and
  their types.
* Removed the restriction on `Map` type representation to allow `Union` as the
  key type, which allows type checking to work on a literal `{}`, which has
  calculated type `Map[Union, Union]`.
* Fixed `basename` function to accept a `String` argument.
* Fixed `size` function to accept a `String` argument.
* Removed the `span_of` function in favor of an `AstNode` extension trait.
* Changed the `call_expr` grammar rule to only accept an identifier for the
  target and not an expression as functions cannot be values in WDL.
* Fixed the CLI tools to not print colors when output is not to a terminal.
* Fixed incorrect parsing of some expressions which required the interior to be
  non-empty (e.g. `foo[]` is not a valid index expression).
  • Loading branch information
peterhuene committed Aug 28, 2024
1 parent e32d509 commit 9a345dd
Show file tree
Hide file tree
Showing 109 changed files with 4,328 additions and 1,191 deletions.
12 changes: 12 additions & 0 deletions wdl-analysis/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

### Added

* Full type checking support in task definitions ([#163](https://github.com/stjude-rust-labs/wdl/pull/163)).

### Changed

* Refactored crate layout ([#163](https://github.com/stjude-rust-labs/wdl/pull/163)).

### Fixed

* Fixed definition of `basename` and `size` functions to accept `String` ([#163](https://github.com/stjude-rust-labs/wdl/pull/163)).

## 0.2.0 - 08-22-2024

### Added
Expand Down
2 changes: 1 addition & 1 deletion wdl-analysis/src/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use crate::queue::NotifyIncrementalChangeRequest;
use crate::queue::RemoveRequest;
use crate::queue::Request;
use crate::rayon::RayonHandle;
use crate::DocumentScope;
use crate::scope::DocumentScope;

/// Represents the kind of analysis progress being reported.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down
13 changes: 7 additions & 6 deletions wdl-analysis/src/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use indexmap::IndexMap;
use indexmap::IndexSet;
use line_index::LineIndex;
use petgraph::algo::has_path_connecting;
use petgraph::algo::DfsSpace;
use petgraph::graph::NodeIndex;
use petgraph::stable_graph::StableDiGraph;
use petgraph::visit::Bfs;
Expand All @@ -31,9 +30,13 @@ use wdl_ast::Diagnostic;
use wdl_ast::SyntaxNode;
use wdl_ast::Validator;

use crate::DocumentScope;
use crate::scope::DocumentScope;
use crate::IncrementalChange;

/// Represents space for a DFS search of a document graph.
pub type DfsSpace =
petgraph::algo::DfsSpace<NodeIndex, <StableDiGraph<DocumentGraphNode, ()> as Visitable>::Map>;

/// Represents diagnostics for a document node.
#[derive(Debug, Clone)]
pub enum Diagnostics {
Expand Down Expand Up @@ -451,8 +454,6 @@ pub struct DocumentGraph {
/// import relationship exists in this set, a diagnostic will be added and
/// the import otherwise ignored.
cycles: HashSet<(NodeIndex, NodeIndex)>,
/// Space for DFS traversals.
space: DfsSpace<NodeIndex, <StableDiGraph<DocumentGraphNode, ()> as Visitable>::Map>,
}

impl DocumentGraph {
Expand Down Expand Up @@ -594,10 +595,10 @@ impl DocumentGraph {
/// Adds a dependency edge from one document to another.
///
/// If a dependency edge already exists, this is a no-op.
pub fn add_dependency_edge(&mut self, from: NodeIndex, to: NodeIndex) {
pub fn add_dependency_edge(&mut self, from: NodeIndex, to: NodeIndex, space: &mut DfsSpace) {
// Check to see if there is already a path between the nodes; if so, there's a
// cycle
if has_path_connecting(&self.inner, from, to, Some(&mut self.space)) {
if has_path_connecting(&self.inner, from, to, Some(space)) {
// Adding the edge would cause a cycle, so record the cycle instead
log::debug!(
"an import cycle was detected between `{from}` and `{to}`",
Expand Down
11 changes: 5 additions & 6 deletions wdl-analysis/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@
#![warn(rustdoc::broken_intra_doc_links)]

mod analyzer;
// pub mod evaluation;
mod graph;
mod queue;
mod rayon;
mod scope;
mod stdlib;
mod types;
pub mod scope;
pub mod stdlib;
mod task;
pub mod types;

pub use analyzer::*;
pub use scope::*;
pub use stdlib::*;
pub use types::*;
11 changes: 8 additions & 3 deletions wdl-analysis/src/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ use wdl_ast::Ast;
use wdl_ast::AstToken;

use crate::graph::Analysis;
use crate::graph::DfsSpace;
use crate::graph::DocumentGraph;
use crate::graph::ParseState;
use crate::rayon::RayonHandle;
use crate::scope::DocumentScope;
use crate::AnalysisResult;
use crate::DocumentScope;
use crate::IncrementalChange;
use crate::ProgressKind;

Expand Down Expand Up @@ -275,6 +276,7 @@ where

// The current starting offset into the subgraph slice to process
let mut offset = 0;
let mut space = Default::default();

loop {
if completed.is_closed() {
Expand Down Expand Up @@ -316,7 +318,9 @@ where

// Update the graph, potentially adding more nodes to the subgraph
let len = slice.len();
if let Err(e) = self.update_graphs(parsed, &mut subgraph, offset..offset + len) {
if let Err(e) =
self.update_graphs(parsed, &mut subgraph, offset..offset + len, &mut space)
{
return Cancelable::Completed(Err(e));
}

Expand Down Expand Up @@ -509,6 +513,7 @@ where
parsed: Vec<(NodeIndex, Result<ParseState>)>,
subgraph: &mut IndexSet<NodeIndex>,
range: Range<usize>,
space: &mut DfsSpace,
) -> Result<()> {
let mut graph = self.graph.write();

Expand Down Expand Up @@ -541,7 +546,7 @@ where
let import_index = graph
.get_index(&import_uri)
.unwrap_or_else(|| graph.add_node(import_uri, false));
graph.add_dependency_edge(index, import_index);
graph.add_dependency_edge(index, import_index, space);

// Add the import to the subgraph
subgraph.insert(import_index);
Expand Down
Loading

0 comments on commit 9a345dd

Please sign in to comment.