fix: fixes the line_width rule

stjude-rust-labs · Jul 18, 2024 · 26a6448 · 26a6448
1 parent 2da59bc
commit 26a6448
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 905 deletions.
diff --git a/Arena.toml b/Arena.toml
diff --git a/Gauntlet.toml b/Gauntlet.toml
@@ -24,7 +24,7 @@ commit_hash = "4e7fdfa7992dd1bf4c4721e77c81e4d7b8ecf4fe"
 
 [repositories."broadinstitute/warp"]
 identifier = "broadinstitute/warp"
-commit_hash = "9942eed7d6c8aba87ddf096fc77cddc6f5052e54"
+commit_hash = "25a98392991857f4a8be429075c0c7496834aa88"
 
 [repositories."chanzuckerberg/czid-workflows"]
 identifier = "chanzuckerberg/czid-workflows"
@@ -44,7 +44,7 @@ commit_hash = "c2c13e85efda8ac7aca7f8765fbaa7c2cacb08b2"
 
 [repositories."stjudecloud/workflows"]
 identifier = "stjudecloud/workflows"
-commit_hash = "833b97fb582fe7d4f6df9d29a645d0289f1fc92e"
+commit_hash = "ec7cb3e21ef900d71db040943d7e14e94fa3b567"
 filters = ["/template/task-templates.wdl"]
 
 [repositories."theiagen/public_health_bioinformatics"]
@@ -384,32 +384,32 @@ permalink = "https://github.com/broadinstitute/palantir-workflows/blob/4e7fdfa79
 [[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/9942eed7d6c8aba87ddf096fc77cddc6f5052e54/pipelines/skylab/scATAC/scATAC.wdl/#L203"
+permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/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/9942eed7d6c8aba87ddf096fc77cddc6f5052e54/tasks/broad/GermlineVariantDiscovery.wdl/#L140"
+permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/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/9942eed7d6c8aba87ddf096fc77cddc6f5052e54/tasks/broad/GermlineVariantDiscovery.wdl/#L67"
+permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/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/9942eed7d6c8aba87ddf096fc77cddc6f5052e54/tasks/broad/UltimaGenomicsWholeGenomeGermlineTasks.wdl/#L814"
+permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/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/9942eed7d6c8aba87ddf096fc77cddc6f5052e54/tasks/broad/UltimaGenomicsWholeGenomeGermlineTasks.wdl/#L866"
+permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/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/9942eed7d6c8aba87ddf096fc77cddc6f5052e54/tests/cemba/pr/CheckCembaOutputs.wdl/#L1"
+permalink = "https://github.com/broadinstitute/warp/blob/25a98392991857f4a8be429075c0c7496834aa88/tests/cemba/pr/CheckCembaOutputs.wdl/#L1"
 
 [[diagnostics]]
 document = "chanzuckerberg/czid-workflows:/workflows/index-generation/index-generation.wdl"

diff --git a/wdl-lint/src/rules/line_width.rs b/wdl-lint/src/rules/line_width.rs
@@ -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 {
@@ -48,27 +48,23 @@ 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();
+
+            // let width = UnicodeWidthStr::width(&)
+
+            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);
+        }
     }
 }
 
@@ -77,8 +73,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,
         }
     }
 }
@@ -89,8 +85,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."
     }
 
     fn explanation(&self) -> &'static str {
@@ -139,7 +134,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(
@@ -148,6 +143,6 @@ impl Visitor for LineWidthRule {
         reason: VisitReason,
         _: &v1::ParameterMetadataSection,
     ) {
-        self.should_ignore = matches!(reason, VisitReason::Enter);
+        self.ignored_section = matches!(reason, VisitReason::Enter);
     }
 }
diff --git a/wdl-lint/src/visitor.rs b/wdl-lint/src/visitor.rs
@@ -85,6 +85,7 @@ impl LintVisitor {
         if reason == VisitReason::Enter {
             let exceptions = self.exceptions_for(state, node);
             if !exceptions.is_empty() {
+                println!("Pushing {exceptions:?} for {node}");
                 self.exceptions.push((start, exceptions));
             }
         }

diff --git a/wdl-lint/tests/lints/line-width/source.errors b/wdl-lint/tests/lints/line-width/source.errors
@@ -0,0 +1,16 @@
+note[LineWidth]: line exceeds maximum width of 90
+   ┌─ tests/lints/line-width/source.wdl:12:2
+   │
+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:2
+   │
+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
+
diff --git a/wdl-lint/tests/lints/line-width/source.wdl b/wdl-lint/tests/lints/line-width/source.wdl
@@ -0,0 +1,53 @@
+#@ 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 {}
+
+    #@ 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.