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.

Sign up for GitHub

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

revise: makes various improvements to the line_width rule #130

Merged
merged 1 commit into from
Jul 19, 2024
Merged

revise: makes various improvements to the line_width rule #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions Arena.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ commit_hash = "c2c13e85efda8ac7aca7f8765fbaa7c2cacb08b2"

[repositories."stjudecloud/workflows"]
identifier = "stjudecloud/workflows"
commit_hash = "ec7cb3e21ef900d71db040943d7e14e94fa3b567"
commit_hash = "129060215443bbd1b67c4d3d149d48acdab90ef2"
filters = ["/template/task-templates.wdl"]

[[diagnostics]]
Expand Down Expand Up @@ -1118,79 +1118,79 @@ permalink = "https://github.com/getwilds/ww-vc-trio/blob/c2c13e85efda8ac7aca7f87
[[diagnostics]]
claymcleod marked this conversation as resolved.
Show resolved Hide resolved
document = "stjudecloud/workflows:/data_structures/read_group.wdl"
message = "read_group.wdl:134:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/data_structures/read_group.wdl/#L134"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/data_structures/read_group.wdl/#L134"
Copy link
Member Author

Choose a reason for hiding this comment

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

This is still going to exist until we implement the fix we discussed in Slack. That is probably the another PR in the future.


[[diagnostics]]
document = "stjudecloud/workflows:/tools/arriba.wdl"
message = "arriba.wdl:147:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/arriba.wdl/#L147"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/arriba.wdl/#L147"
Copy link
Member Author

Choose a reason for hiding this comment

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

Same here.


[[diagnostics]]
document = "stjudecloud/workflows:/tools/estimate.wdl"
message = "estimate.wdl:39:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/estimate.wdl/#L39"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/estimate.wdl/#L39"

[[diagnostics]]
document = "stjudecloud/workflows:/tools/gatk4.wdl"
message = "gatk4.wdl:127:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/gatk4.wdl/#L127"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/gatk4.wdl/#L127"

[[diagnostics]]
document = "stjudecloud/workflows:/tools/gatk4.wdl"
message = "gatk4.wdl:194:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/gatk4.wdl/#L194"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/gatk4.wdl/#L194"

[[diagnostics]]
document = "stjudecloud/workflows:/tools/gatk4.wdl"
message = "gatk4.wdl:280:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/gatk4.wdl/#L280"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/gatk4.wdl/#L280"

[[diagnostics]]
document = "stjudecloud/workflows:/tools/htseq.wdl"
message = "htseq.wdl:163:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/htseq.wdl/#L163"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/htseq.wdl/#L163"

[[diagnostics]]
document = "stjudecloud/workflows:/tools/ngsderive.wdl"
message = "ngsderive.wdl:239:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/ngsderive.wdl/#L239"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/ngsderive.wdl/#L239"

[[diagnostics]]
document = "stjudecloud/workflows:/tools/star.wdl"
message = "star.wdl:632:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/star.wdl/#L632"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/star.wdl/#L632"

[[diagnostics]]
document = "stjudecloud/workflows:/tools/util.wdl"
message = "util.wdl:458:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/util.wdl/#L458"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/util.wdl/#L458"

[[diagnostics]]
document = "stjudecloud/workflows:/tools/util.wdl"
message = "util.wdl:713:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/tools/util.wdl/#L713"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/tools/util.wdl/#L713"

[[diagnostics]]
document = "stjudecloud/workflows:/workflows/chipseq/chipseq-standard.wdl"
message = "chipseq-standard.wdl:10:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/workflows/chipseq/chipseq-standard.wdl/#L10"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/workflows/chipseq/chipseq-standard.wdl/#L10"

[[diagnostics]]
document = "stjudecloud/workflows:/workflows/chipseq/chipseq-standard.wdl"
message = "chipseq-standard.wdl:11:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/workflows/chipseq/chipseq-standard.wdl/#L11"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/workflows/chipseq/chipseq-standard.wdl/#L11"

[[diagnostics]]
document = "stjudecloud/workflows:/workflows/chipseq/chipseq-standard.wdl"
message = "chipseq-standard.wdl:12:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/workflows/chipseq/chipseq-standard.wdl/#L12"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/workflows/chipseq/chipseq-standard.wdl/#L12"

[[diagnostics]]
document = "stjudecloud/workflows:/workflows/general/alignment-post.wdl"
message = "alignment-post.wdl:6:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/workflows/general/alignment-post.wdl/#L6"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/workflows/general/alignment-post.wdl/#L6"

[[diagnostics]]
document = "stjudecloud/workflows:/workflows/qc/quality-check-standard.wdl"
message = "quality-check-standard.wdl:111:1: note[LineWidth]: line exceeds maximum width of 90"
permalink = "https://github.com/stjudecloud/workflows/blob/ec7cb3e21ef900d71db040943d7e14e94fa3b567/workflows/qc/quality-check-standard.wdl/#L111"
permalink = "https://github.com/stjudecloud/workflows/blob/129060215443bbd1b67c4d3d149d48acdab90ef2/workflows/qc/quality-check-standard.wdl/#L111"
22 changes: 11 additions & 11 deletions Gauntlet.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ commit_hash = "4536dd2c6719b86f4c50348f0ce354818260cadb"

[repositories."broadinstitute/palantir-workflows"]
identifier = "broadinstitute/palantir-workflows"
commit_hash = "4e7fdfa7992dd1bf4c4721e77c81e4d7b8ecf4fe"
commit_hash = "efb0eadd70fe21fe9484a310f12a6c54b752db43"

[repositories."broadinstitute/warp"]
identifier = "broadinstitute/warp"
commit_hash = "25a98392991857f4a8be429075c0c7496834aa88"
commit_hash = "16d9ae6fc19071acd3541ac2950908d2cc2d03bd"

[repositories."chanzuckerberg/czid-workflows"]
identifier = "chanzuckerberg/czid-workflows"
Expand All @@ -44,12 +44,12 @@ commit_hash = "c2c13e85efda8ac7aca7f8765fbaa7c2cacb08b2"

[repositories."stjudecloud/workflows"]
identifier = "stjudecloud/workflows"
commit_hash = "ec7cb3e21ef900d71db040943d7e14e94fa3b567"
commit_hash = "129060215443bbd1b67c4d3d149d48acdab90ef2"
filters = ["/template/task-templates.wdl"]

[repositories."theiagen/public_health_bioinformatics"]
identifier = "theiagen/public_health_bioinformatics"
commit_hash = "42a06efca2be1935cc8b5936b1fa9164e5646b80"
commit_hash = "6d406b65a1cfa0b7e1042da2e750cbbf91531c92"

[[diagnostics]]
document = "aws-samples/amazon-omics-tutorials:/example-workflows/gatk-best-practices/workflows/somatic-snps-and-indels/mutec2.wdl"
Expand Down Expand Up @@ -379,37 +379,37 @@ permalink = "https://github.com/biowdl/tasks/blob/2bf875300d90a3c9c8d670b3d99026
[[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/4e7fdfa7992dd1bf4c4721e77c81e4d7b8ecf4fe/HaplotypeMap/BuildHaplotypeMap.wdl/#L17"
permalink = "https://github.com/broadinstitute/palantir-workflows/blob/efb0eadd70fe21fe9484a310f12a6c54b752db43/HaplotypeMap/BuildHaplotypeMap.wdl/#L17"

[[diagnostics]]
document = "broadinstitute/warp:/pipelines/skylab/scATAC/scATAC.wdl"
message = "scATAC.wdl:203:9: error: duplicate key `cpu` in runtime section"
permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/pipelines/skylab/scATAC/scATAC.wdl/#L203"
permalink = "https://github.com/broadinstitute/warp/blob/16d9ae6fc19071acd3541ac2950908d2cc2d03bd/pipelines/skylab/scATAC/scATAC.wdl/#L203"

[[diagnostics]]
document = "broadinstitute/warp:/tasks/broad/GermlineVariantDiscovery.wdl"
message = "GermlineVariantDiscovery.wdl:140:32: error: expected string, but found integer"
permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/tasks/broad/GermlineVariantDiscovery.wdl/#L140"
permalink = "https://github.com/broadinstitute/warp/blob/16d9ae6fc19071acd3541ac2950908d2cc2d03bd/tasks/broad/GermlineVariantDiscovery.wdl/#L140"

[[diagnostics]]
document = "broadinstitute/warp:/tasks/broad/GermlineVariantDiscovery.wdl"
message = "GermlineVariantDiscovery.wdl:67:32: error: expected string, but found integer"
permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/tasks/broad/GermlineVariantDiscovery.wdl/#L67"
permalink = "https://github.com/broadinstitute/warp/blob/16d9ae6fc19071acd3541ac2950908d2cc2d03bd/tasks/broad/GermlineVariantDiscovery.wdl/#L67"

[[diagnostics]]
document = "broadinstitute/warp:/tasks/broad/UltimaGenomicsWholeGenomeGermlineTasks.wdl"
message = "UltimaGenomicsWholeGenomeGermlineTasks.wdl:814:27: error: expected string, but found integer"
permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/tasks/broad/UltimaGenomicsWholeGenomeGermlineTasks.wdl/#L814"
permalink = "https://github.com/broadinstitute/warp/blob/16d9ae6fc19071acd3541ac2950908d2cc2d03bd/tasks/broad/UltimaGenomicsWholeGenomeGermlineTasks.wdl/#L814"

[[diagnostics]]
document = "broadinstitute/warp:/tasks/broad/UltimaGenomicsWholeGenomeGermlineTasks.wdl"
message = "UltimaGenomicsWholeGenomeGermlineTasks.wdl:866:27: error: expected string, but found integer"
permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/tasks/broad/UltimaGenomicsWholeGenomeGermlineTasks.wdl/#L866"
permalink = "https://github.com/broadinstitute/warp/blob/16d9ae6fc19071acd3541ac2950908d2cc2d03bd/tasks/broad/UltimaGenomicsWholeGenomeGermlineTasks.wdl/#L866"

[[diagnostics]]
document = "broadinstitute/warp:/tests/cemba/pr/CheckCembaOutputs.wdl"
message = "CheckCembaOutputs.wdl:1:1: error: a WDL document must start with a version statement"
permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/tests/cemba/pr/CheckCembaOutputs.wdl/#L1"
permalink = "https://github.com/broadinstitute/warp/blob/16d9ae6fc19071acd3541ac2950908d2cc2d03bd/tests/cemba/pr/CheckCembaOutputs.wdl/#L1"

[[diagnostics]]
document = "chanzuckerberg/czid-workflows:/workflows/index-generation/index-generation.wdl"
Expand Down
53 changes: 23 additions & 30 deletions wdl-lint/src/rules/line_width.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ fn line_too_long(span: Span, max_width: usize) -> Diagnostic {
}

/// Detects lines that exceed a certain width.
#[derive(Debug, Clone, Copy)]
#[derive(Clone, Copy, Debug)]
pub struct LineWidthRule {
/// The maximum width of a line.
max_width: usize,
/// The offset of the previous newline.
prev_newline_offset: usize,
/// The offset of the last newline character seen (if it exists).
previous_newline_offset: Option<usize>,
/// Whether we are in a section that should be ignored.
should_ignore: bool,
ignored_section: bool,
}

impl LineWidthRule {
Expand All @@ -48,27 +48,21 @@ impl LineWidthRule {

/// 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;
for offset in text
.char_indices()
.filter(|(_, c)| *c == '\n')
.map(|(offset, _)| offset)
{
let current_offset = start + offset;
let previous_offset = self.previous_newline_offset.unwrap_or_default();

if !self.ignored_section && current_offset - previous_offset > self.max_width {
let span = Span::new(previous_offset, current_offset - previous_offset);
state.add(line_too_long(span, self.max_width));
}
});

self.previous_newline_offset = Some(current_offset + 1);
}
}
}

Expand All @@ -77,8 +71,8 @@ impl Default for LineWidthRule {
fn default() -> Self {
Self {
max_width: 90,
prev_newline_offset: 0,
should_ignore: false,
previous_newline_offset: None,
ignored_section: false,
}
}
}
Expand All @@ -89,8 +83,7 @@ impl Rule for LineWidthRule {
}

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

Choose a reason for hiding this comment

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

The default should be described in the explanation (which it is), so it can be removed here.

}

fn explanation(&self) -> &'static str {
Expand Down Expand Up @@ -139,7 +132,7 @@ impl Visitor for LineWidthRule {
reason: VisitReason,
_: &v1::MetadataSection,
) {
self.should_ignore = matches!(reason, VisitReason::Enter);
self.ignored_section = matches!(reason, VisitReason::Enter);
}

fn parameter_metadata_section(
Expand All @@ -148,6 +141,6 @@ impl Visitor for LineWidthRule {
reason: VisitReason,
_: &v1::ParameterMetadataSection,
) {
self.should_ignore = matches!(reason, VisitReason::Enter);
self.ignored_section = matches!(reason, VisitReason::Enter);
}
}
46 changes: 46 additions & 0 deletions wdl-lint/tests/lints/line-width/source.errors
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
note[LineWidth]: line exceeds maximum width of 90
┌─ tests/lints/line-width/source.wdl:12:1
│
12 │ this is going to be a really long line that is almost certainly going to be longer than the maximum line length I would hope /
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
│
= fix: split the line into multiple lines

note[LineWidth]: line exceeds maximum width of 90
┌─ tests/lints/line-width/source.wdl:14:1
│
14 │ this line is also going to be very very very long just to trip up our maximum line lint /
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
│
= fix: split the line into multiple lines

note[LineWidth]: line exceeds maximum width of 90
┌─ tests/lints/line-width/source.wdl:31:1
│
31 │ ╭ command <<<
Copy link
Member Author

Choose a reason for hiding this comment

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

See the comment in the source text for this diagnostic.

32 │ │ bin /
33 │ │ this is going to be a really long line that is not going to trip up our maximum line lint because it is excepted /
34 │ │ some other /
· │
37 │ │ options
38 │ │ >>>
│ ╰───────^
│
= fix: split the line into multiple lines

note[LineWidth]: line exceeds maximum width of 90
┌─ tests/lints/line-width/source.wdl:49:1
│
49 │ command <<< this is a task that has a very very very long command section on the first line. >>>
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
│
= fix: split the line into multiple lines

note[LineWidth]: line exceeds maximum width of 90
┌─ tests/lints/line-width/source.wdl:55:1
│
55 │ # Here is a very very very very very very very very very long comment that should absolutely eclipse 90 characters.
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
│
= fix: split the line into multiple lines

55 changes: 55 additions & 0 deletions wdl-lint/tests/lints/line-width/source.wdl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#@ except: DescriptionMissing, RuntimeSectionKeys
version 1.1

task task_a {
meta {}
parameter_meta {}
input {}

command <<<
bin \
this is going to be a really long line that is almost certainly going to be longer than the maximum line length I would hope \
some other \
this line is also going to be very very very long just to trip up our maximum line lint \
shorter \
options
>>>

output {}
runtime {}
}

task task_b {
meta {}
parameter_meta {}
input {}

# This except currently causes the entire command block to fail.
# That will be fixed in the future when we fix how excepting works.
#@ except: LineWidth
command <<<
bin \
this is going to be a really long line that is not going to trip up our maximum line lint because it is excepted \
some other \
this line is also going to be very very very long but it will also not trip up our maximum line line because it is excepted \
shorter \
options
>>>

output {}
runtime {}
}

task task_c {
meta {}
parameter_meta {}
input {}

command <<< this is a task that has a very very very long command section on the first line. >>>

output {}
runtime {}
}

# Here is a very very very very very very very very very long comment that should absolutely eclipse 90 characters.
Loading