Skip to content

Commit

Permalink
feat: LineWidth rule (#99)
Browse files Browse the repository at this point in the history
  • Loading branch information
a-frantz authored Jun 25, 2024
1 parent 044cddb commit 6b209f0
Show file tree
Hide file tree
Showing 11 changed files with 714 additions and 23 deletions.
525 changes: 525 additions & 0 deletions Arena.toml

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Gauntlet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ filters = ["/template/task-templates.wdl"]

[repositories."theiagen/public_health_bioinformatics"]
identifier = "theiagen/public_health_bioinformatics"
commit_hash = "252b98cea3c1fbdd509c6bc3497a6e8060b9bf2a"
commit_hash = "d0377e139855252e15b57d7699aa6f5abb510996"

[[diagnostics]]
document = "aws-samples/amazon-omics-tutorials:/example-workflows/gatk-best-practices/workflows/somatic-snps-and-indels/mutec2.wdl"
Expand Down
1 change: 1 addition & 0 deletions wdl-lint/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

* Added the `LineWidth` lint rule (#99).
* Added the `ImportWhitespace` and `ImportSort` lint rules (#98).
* Added the `MissingMetas` and `MissingOutput` lint rules (#96).
* Added the `PascalCase` lint rule ([#90](https://github.com/stjude-rust-labs/wdl/pull/90)).
Expand Down
2 changes: 2 additions & 0 deletions wdl-lint/RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ be out of sync with released packages.
| `PreambleWhitespace` | Spacing, Style | Ensures that documents have correct whitespace in the preamble. |
| `SnakeCase` | Clarity, Naming, Style | Ensures that tasks, workflows, and variables are defined with snake_case names. |
| `Whitespace` | Spacing, Style | Ensures that a document does not contain undesired whitespace. |
| `LineWidth` | Clarity, Spacing, Style | Ensures that lines do not exceed a certain width. |

1 change: 1 addition & 0 deletions wdl-lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub fn rules() -> Vec<Box<dyn Rule>> {
Box::new(rules::MissingMetasRule),
Box::new(rules::MissingOutputRule),
Box::new(rules::ImportSortRule),
Box::new(rules::LineWidthRule::default()),
];

// Ensure all the rule ids are unique and pascal case
Expand Down
2 changes: 2 additions & 0 deletions wdl-lint/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ mod ending_newline;
mod import_placement;
mod import_sort;
mod import_whitespace;
mod line_width;
mod matching_parameter_meta;
mod missing_metas;
mod missing_output;
Expand All @@ -23,6 +24,7 @@ pub use ending_newline::*;
pub use import_placement::*;
pub use import_sort::*;
pub use import_whitespace::*;
pub use line_width::*;
pub use matching_parameter_meta::*;
pub use missing_metas::*;
pub use missing_output::*;
Expand Down
149 changes: 149 additions & 0 deletions wdl-lint/src/rules/line_width.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
//! Ensures that lines do not exceed a certain width.
use wdl_ast::AstToken;
use wdl_ast::Diagnostic;
use wdl_ast::Diagnostics;
use wdl_ast::Span;
use wdl_ast::Visitor;

use crate::Rule;
use crate::Tag;
use crate::TagSet;

/// The identifier for the line width rule.
const ID: &str = "LineWidth";

/// Creates a diagnostic for when a line exceeds the maximum width.
fn line_too_long(span: Span, max_width: usize) -> Diagnostic {
Diagnostic::note(format!("line exceeds maximum width of {}", max_width))
.with_rule(ID)
.with_highlight(span)
.with_fix("split the line into multiple lines")
}

/// Detects lines that exceed a certain width.
#[derive(Debug, Clone, Copy)]
pub struct LineWidthRule(usize);

/// Implements the default line width rule.
impl Default for LineWidthRule {
fn default() -> Self {
Self(90)
}
}

impl Rule for LineWidthRule {
fn id(&self) -> &'static str {
ID
}

fn description(&self) -> &'static str {
"Ensures that lines do not exceed a certain width. That width is currently set to 90 \
characters."
}

fn explanation(&self) -> &'static str {
"Lines should not exceed a certain width to make it easier to read and understand the \
code. Code within the either the meta or parameter meta sections is not checked. Comments \
are included in the line width check. The current maximum width is 90 characters."
}

fn tags(&self) -> TagSet {
TagSet::new(&[Tag::Style, Tag::Clarity, Tag::Spacing])
}

fn visitor(&self) -> Box<dyn Visitor<State = Diagnostics>> {
Box::new(LineWidthVisitor::new(self.0))
}
}

/// A visitor that detects lines that exceed a certain width.
#[derive(Debug, Clone, Copy)]
struct LineWidthVisitor {
/// The maximum width of a line.
max_width: usize,
/// The offset of the previous newline.
prev_newline_offset: usize,
/// Whether we are in a section that should be ignored.
should_ignore: bool,
}

impl LineWidthVisitor {
/// Creates a new line width visitor.
fn new(max_width: usize) -> Self {
Self {
max_width,
prev_newline_offset: 0,
should_ignore: false,
}
}

/// 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;
text.char_indices().for_each(|(i, c)| {
if c == '\n' {
cur_newline_offset = start + i;
if self.should_ignore {
self.prev_newline_offset = cur_newline_offset + 1;
return;
}

if cur_newline_offset - self.prev_newline_offset > self.max_width {
state.add(line_too_long(
Span::new(
self.prev_newline_offset,
cur_newline_offset - self.prev_newline_offset,
),
self.max_width,
));
}
self.prev_newline_offset = cur_newline_offset + 1;
}
});
}
}

impl Visitor for LineWidthVisitor {
type State = Diagnostics;

fn whitespace(&mut self, state: &mut Self::State, whitespace: &wdl_ast::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) {
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,
) {
match reason {
wdl_ast::VisitReason::Enter => {
self.should_ignore = true;
}
wdl_ast::VisitReason::Exit => {
self.should_ignore = false;
}
}
}

fn parameter_metadata_section(
&mut self,
_: &mut Self::State,
reason: wdl_ast::VisitReason,
_: &wdl_ast::v1::ParameterMetadataSection,
) {
match reason {
wdl_ast::VisitReason::Enter => {
self.should_ignore = true;
}
wdl_ast::VisitReason::Exit => {
self.should_ignore = false;
}
}
}
}
20 changes: 12 additions & 8 deletions wdl-lint/tests/lints/double-quotes/source.errors
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,22 @@ warning[DoubleQuotes]: string defined with single quotes
= fix: change the single quotes to double quotes

warning[DoubleQuotes]: string defined with single quotes
┌─ tests/lints/double-quotes/source.wdl:11:30
11 │ "this string is ok ~{'but this is not and ~{"while this one is okay ~{'this one is not'}"}'}!"
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
┌─ tests/lints/double-quotes/source.wdl:12:13
12 │ ╭ 'but this is not and ~{
13 │ │ "while this one is okay ~{
14 │ │ 'this one is not'
15 │ │ }"
16 │ │ }'
│ ╰──────────────^
= fix: change the single quotes to double quotes

warning[DoubleQuotes]: string defined with single quotes
┌─ tests/lints/double-quotes/source.wdl:11:79
┌─ tests/lints/double-quotes/source.wdl:14:21
11"this string is ok ~{'but this is not and ~{"while this one is okay ~{'this one is not'}"}'}!"
^^^^^^^^^^^^^^^^^
14 'this one is not'
│ ^^^^^^^^^^^^^^^^^
= fix: change the single quotes to double quotes

8 changes: 7 additions & 1 deletion wdl-lint/tests/lints/double-quotes/source.wdl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ workflow test {
String good = "this string is okay"
String bad = 'this string is not okay'
String interpolated = # a comment!
"this string is ok ~{'but this is not and ~{"while this one is okay ~{'this one is not'}"}'}!"
"this string is ok ~{
'but this is not and ~{
"while this one is okay ~{
'this one is not'
}"
}'
}!"
output {}
}
24 changes: 12 additions & 12 deletions wdl-lint/tests/lints/matching-param-meta/source.errors
Original file line number Diff line number Diff line change
@@ -1,31 +1,31 @@
warning[MatchingParameterMeta]: task `t` is missing a parameter metadata key for input `does_not_exist`
┌─ tests/lints/matching-param-meta/source.wdl:9:16
9 │ String does_not_exist
│ ^^^^^^^^^^^^^^ this input does not have an entry in the parameter metadata section
= fix: add a `does_not_exist` key to the `parameter_meta` section with a detailed description of the input.
┌─ tests/lints/matching-param-meta/source.wdl:10:16
10 │ String does_not_exist
│ ^^^^^^^^^^^^^^ this input does not have an entry in the parameter metadata section
= fix: add a `does_not_exist` key to the `parameter_meta` section with a detailed description of the input.

note[MatchingParameterMeta]: task `t` has an extraneous parameter metadata key named `extra`
┌─ tests/lints/matching-param-meta/source.wdl:21:9
┌─ tests/lints/matching-param-meta/source.wdl:22:9
21 │ extra: "this should not be here"
22 │ extra: "this should not be here"
│ ^^^^^ this key does not correspond to any input declaration
= fix: remove the extraneous parameter metadata entry

warning[MatchingParameterMeta]: workflow `w` is missing a parameter metadata key for input `does_not_exist`
┌─ tests/lints/matching-param-meta/source.wdl:34:16
┌─ tests/lints/matching-param-meta/source.wdl:35:16
34 │ String does_not_exist
35 │ String does_not_exist
│ ^^^^^^^^^^^^^^ this input does not have an entry in the parameter metadata section
= fix: add a `does_not_exist` key to the `parameter_meta` section with a detailed description of the input.

note[MatchingParameterMeta]: workflow `w` has an extraneous parameter metadata key named `extra`
┌─ tests/lints/matching-param-meta/source.wdl:46:9
┌─ tests/lints/matching-param-meta/source.wdl:47:9
46 │ extra: "this should not be here"
47 │ extra: "this should not be here"
│ ^^^^^ this key does not correspond to any input declaration
= fix: remove the extraneous parameter metadata entry
Expand Down
3 changes: 2 additions & 1 deletion wdl-lint/tests/lints/matching-param-meta/source.wdl
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
## This is a test for checking for missing and extraneous entries in a `parameter_meta` section.
## This is a test for checking for missing and extraneous entries
## in a `parameter_meta` section.
version 1.1

Expand Down

0 comments on commit 6b209f0

Please sign in to comment.