Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: implement static analysis for workflows. #199

Merged
merged 11 commits into from
Oct 1, 2024

Conversation

peterhuene
Copy link
Collaborator

@peterhuene peterhuene commented Sep 27, 2024

This PR implements full static analysis for workflows, including type
checking.

To perform analysis on workflows, a WorkflowGraph was implemented
that will return a topologically-sorted set of nodes for evaluating the
contents of a workflow in the correct order.

This also includes several fixes required to remove incorrect diagnostics from
checking the workflows repository:

  • Treat a coercion to T? for a function argument of type T as a preference
    over any other coercion.
  • Fix the signature of select_first such that it is monomorphic.
  • Only consider signatures in overload resolution that have sufficient
    arguments.
  • Allow coercion from File and Directory to String.
  • Allow non-empty array literals to coerce to either empty or non-empty.
  • Fix element type calculations for Array and Map so that [a, b] and { "a": a, "b": b }
    successfully calculates when a is coercible to b (the reverse already works).
  • Fix if expression type calculation such that if (x) then a else b works
    when a is coercible to b (the reverse already works).
  • Ensure that only equality/inequality expressions are supported on File and
    Directory now that there is a coercion to String.
  • Allow index expressions on Map.

Included with these changes is a refactoring to place all of analysis
diagnostics in diagnostics.rs.

Fixes #197.
Fixes #196.
Fixes #167.
Fixes stjude-rust-labs/sprocket#21.

Before submitting this PR, please make sure:

  • You have added a few sentences describing the PR here.
  • You have added yourself or the appropriate individual as the assignee.
  • You have added at least one relevant code reviewer to the PR.
  • Your code builds clean without any errors or warnings.
  • You have added tests (when appropriate).
  • You have updated the README or other documentation to account for these
    changes (when appropriate).
  • You have added an entry to the relevant CHANGELOG.md (see
    "keep a changelog" for more information).
  • Your commit messages follow the conventional commit style.

This commit refactors out the task evaluation graph into its own public type.
This commit merges the type checking for tasks in with the creation of the task
within the document scope.

To do this, two passes are done over the AST's top level items: first imports
and struct definitions then tasks and workflow.

This allows the population of a task or a workflow in the document scope to
calculate type declarations up front and do the type checking when processing
the declaration, as declarations are added to the scope in topological order
now.
This commit implements full static analysis for workflows, including type
checking.

To perform analysis on workflows, a `WorkflowEvaluationGraph` was implemented
that will return a topologically-sorted set of nodes for evaluating the
contents of a workflow in the correct order.

This also includes several fixes required to remove incorrect diagnostics from
checking the `workflows` repository:

* Treat a coercion to `T?` for a function argument of type `T` as a preference
  over any other coercion.
* Fix the signature of `select_any` such that it is monomorphic.
* Only consider signatures in overload resolution that have sufficient
  arguments.
* Allow coercion from `File` and `Directory` to `String`.
* Allow non-empty array literals to coerce to either empty or non-empty.
* Fix element type calculations for `Array` and `Map` so that `[a, b]`
  successfully calculates when `a` is coercible to `b`.
* Fix `if` expression type calculation such that `if (x) then a else b` works
  when `a` is coercible to `b`.
* Ensure that only equality/inequality expressions are supported on `File` and
  `Directory` now that there is a coercion to `String`.
* Allow index expressions on `Map`.

Included with these changes is a refactoring to place all of analysis
diagnostics in `diagnostics.rs`.

Fixes stjude-rust-labs#197.
Fixes stjude-rust-labs#196.
Fixes stjude-rust-labs#167.
Fixes stjude-rust-labs/sprocket#21.
@peterhuene peterhuene changed the title feat: implement static analysis for workflows. Beta feat: implement static analysis for workflows. Sep 27, 2024
Add the fixes to the `wdl-analysis` CHANGELOG and correct the link to the PR.
This commits adds detection of duplicate call inputs to AST validation.

Additionally, it adds a diagnostic for missing required inputs in a call
statement.
@peterhuene peterhuene force-pushed the workflow-type-checking branch from 549375c to a972279 Compare September 28, 2024 04:39
This commit respects `allow_nested_inputs`:

* With `allow_nested_inputs` or `allowNestedInputs` in a workflow `hints`
  section in 1.2.
* With `allowNestedInputs` in a workflow `meta` section in 1.1 and 1.2.
* By default in 1.0.

When set, the check for required inputs will be suppressed.
@peterhuene peterhuene force-pushed the workflow-type-checking branch from a972279 to a6e846e Compare September 28, 2024 07:04
Copy link
Member

@a-frantz a-frantz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty awesome Peter! 🚀

wdl-analysis/src/diagnostics.rs Show resolved Hide resolved
wdl-analysis/src/diagnostics.rs Outdated Show resolved Hide resolved
wdl-analysis/tests/analysis/call-unknown-import/source.wdl Outdated Show resolved Hide resolved
@peterhuene peterhuene requested a review from a-frantz October 1, 2024 00:39
@peterhuene peterhuene merged commit 6349eb6 into stjude-rust-labs:main Oct 1, 2024
8 checks passed
@peterhuene peterhuene deleted the workflow-type-checking branch October 1, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants