From 5a7aab3e3cdf4eb17190be207cebcc365d3eb464 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Sun, 8 Dec 2024 14:54:22 -0500 Subject: [PATCH 01/52] feat: implement shellcheck lints in command blocks (#191) --- wdl-lint/Cargo.toml | 3 + wdl-lint/src/lib.rs | 1 + wdl-lint/src/rules.rs | 2 + wdl-lint/src/rules/command_shellcheck.rs | 357 ++++++++++++++++++ wdl-lint/src/util.rs | 22 ++ .../lints/command-shellcheck-ok/source.errors | 49 +++ .../lints/command-shellcheck-ok/source.wdl | 51 +++ .../command-shellcheck-warn/source.errors | 16 + .../lints/command-shellcheck-warn/source.wdl | 38 ++ 9 files changed, 539 insertions(+) create mode 100644 wdl-lint/src/rules/command_shellcheck.rs create mode 100644 wdl-lint/tests/lints/command-shellcheck-ok/source.errors create mode 100644 wdl-lint/tests/lints/command-shellcheck-ok/source.wdl create mode 100644 wdl-lint/tests/lints/command-shellcheck-warn/source.errors create mode 100644 wdl-lint/tests/lints/command-shellcheck-warn/source.wdl diff --git a/wdl-lint/Cargo.toml b/wdl-lint/Cargo.toml index 70bb16ec8..2f2476379 100644 --- a/wdl-lint/Cargo.toml +++ b/wdl-lint/Cargo.toml @@ -16,6 +16,9 @@ wdl-ast = { path = "../wdl-ast", version = "0.9.0" } convert_case = { workspace = true } indexmap = { workspace = true } rowan = { workspace = true } +serde_json = { workspace = true } +anyhow = { workspace = true } +serde = { workspace = true } [dev-dependencies] codespan-reporting = { workspace = true } diff --git a/wdl-lint/src/lib.rs b/wdl-lint/src/lib.rs index 8362dae1f..d45df6554 100644 --- a/wdl-lint/src/lib.rs +++ b/wdl-lint/src/lib.rs @@ -84,6 +84,7 @@ pub trait Rule: Visitor { /// Gets the default rule set. pub fn rules() -> Vec> { let rules: Vec> = vec![ + Box::::default(), Box::::default(), Box::::default(), Box::::default(), diff --git a/wdl-lint/src/rules.rs b/wdl-lint/src/rules.rs index 9d49dfdde..50a925b31 100644 --- a/wdl-lint/src/rules.rs +++ b/wdl-lint/src/rules.rs @@ -3,6 +3,7 @@ mod blank_lines_between_elements; mod call_input_spacing; mod command_mixed_indentation; +mod command_shellcheck; mod comment_whitespace; mod container_value; mod deprecated_object; @@ -45,6 +46,7 @@ mod whitespace; pub use blank_lines_between_elements::*; pub use call_input_spacing::*; pub use command_mixed_indentation::*; +pub use command_shellcheck::*; pub use comment_whitespace::*; pub use container_value::*; pub use deprecated_object::*; diff --git a/wdl-lint/src/rules/command_shellcheck.rs b/wdl-lint/src/rules/command_shellcheck.rs new file mode 100644 index 000000000..109f16a26 --- /dev/null +++ b/wdl-lint/src/rules/command_shellcheck.rs @@ -0,0 +1,357 @@ +//! A lint rule for checking mixed indentation in command text. +use std::collections::HashMap; +use std::collections::HashSet; +use std::io::Write; +use std::process; +use std::process::Stdio; + +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use serde::Deserialize; +use serde_json; + +use wdl_ast::AstNode; +use wdl_ast::AstToken; +use wdl_ast::Diagnostic; +use wdl_ast::Diagnostics; +use wdl_ast::Document; +use wdl_ast::Span; +use wdl_ast::SupportedVersion; +use wdl_ast::SyntaxElement; +use wdl_ast::SyntaxKind; +use wdl_ast::ToSpan; +use wdl_ast::VisitReason; +use wdl_ast::Visitor; +use wdl_ast::support; +use wdl_ast::v1::CommandPart; +use wdl_ast::v1::CommandSection; +use wdl_ast::v1::Placeholder; +use wdl_ast::v1::StrippedCommandPart; +use wdl_ast::v1::TaskDefinition; + +use crate::Rule; +use crate::Tag; +use crate::TagSet; +use crate::util::{count_leading_whitespace, lines_with_offset, program_exists}; + +/// The shellcheck executable +const SHELLCHECK_BIN: &str = "shellcheck"; + +/// shellcheck lints that we want to suppress +const SHELLCHECK_SUPPRESS: &[&str] = &[ + "1009", // the mentioned parser error was in... + "1072", // Unexpected.. + "1083", // this {/} is literal + "2043", // this loop will only ever run once for constant value + "2050", // This expression is constant + "2157", // Argument to -n is always true due to literal strings + "2193", // The arguments to this comparison can never be equal +]; + +/// The identifier for the command section ShellCheck rule. +const ID: &str = "CommandSectionShellCheck"; + +/// A ShellCheck comment +/// +/// The file and fix fields are ommitted as we have no use for them. +#[derive(Clone, Debug, Deserialize)] +struct ShellCheckComment { + /// line number comment starts on + pub line: usize, + /// line number comment ends on + #[serde(rename = "endLine")] + pub end_line: usize, + /// column comment starts on + pub column: usize, + /// column comment ends on + #[serde(rename = "endColumn")] + pub end_column: usize, + /// severity of the comment + pub level: String, + /// shellcheck error code + pub code: usize, + /// message associated with the comment + pub message: String, +} + +/// Run shellcheck +/// +/// writes command text to stdin of shellcheck process +/// and returns parsed ShellCheckComments +fn run_shellcheck(command: &str) -> Result> { + let mut sc_proc = process::Command::new(SHELLCHECK_BIN) + .args([ + "-s", + "bash", + "-f", + "json", + "-e", + &SHELLCHECK_SUPPRESS.join(","), + "-", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .context("error calling shellcheck")?; + { + let mut proc_stdin = sc_proc + .stdin + .take() + .context("error obtaining stdin of shellcheck")?; + proc_stdin.write_all(command.as_bytes())?; + } + + let output = sc_proc + .wait_with_output() + .context("error calling shellcheck")?; + + // shellcheck returns exit code 1 if + // any checked files result in comments + // so cannot check with status.success() + match output.status.code() { + Some(0) | Some(1) => serde_json::from_slice::>(&output.stdout) + .context("failed to parse shellcheck output"), + Some(code) => bail!("error calling shellcheck: exit code {}", code), + None => bail!("error calling check: process interrupted"), + } +} + +/// Runs ShellCheck on a command section and reports violations +#[derive(Default, Debug, Clone, Copy)] +pub struct CommandSectionShellCheck; + +impl Rule for CommandSectionShellCheck { + fn id(&self) -> &'static str { + ID + } + + fn description(&self) -> &'static str { + "Ensures that command blocks are free of ShellCheck violations." + } + + fn explanation(&self) -> &'static str { + "ShellCheck is a static analysis tool and linter for sh / bash. \ + The lints provided by ShellCheck help prevent common errors and \ + pitfalls in your scripts. Following its recommendations will increase \ + the robustness of your command blocks." + } + + fn tags(&self) -> TagSet { + TagSet::new(&[Tag::Correctness, Tag::Style, Tag::Portability]) + } + + fn exceptable_nodes(&self) -> Option<&'static [SyntaxKind]> { + Some(&[ + SyntaxKind::VersionStatementNode, + SyntaxKind::TaskDefinitionNode, + SyntaxKind::CommandSectionNode, + ]) + } +} + +/// Convert a WDL placeholder to a bash variable +fn to_bash_var(placeholder: &Placeholder) -> String { + let placeholder_text = placeholder.syntax().text().to_string(); + let mut bash_var = String::from("_WL"); + bash_var.push_str( + &placeholder_text + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '_' { + c + } else { + '_' + } + }) + .collect::(), + ); + bash_var +} + +/// retrieve all input and private declations for a task +fn task_declarations(task: &TaskDefinition) -> HashSet { + let mut decls = HashSet::new(); + if let Some(input) = task.input() { + for decl in input.declarations() { + decls.insert(decl.name().as_str().to_owned()); + } + } + + for decl in task.declarations() { + decls.insert(decl.name().as_str().to_owned()); + } + decls +} + +/// Creates a "ShellCheck lint" diagnostic from a ShellCheckComment +fn shellcheck_lint(comment: &ShellCheckComment, span: Span) -> Diagnostic { + dbg!( + Diagnostic::note("shellcheck violations within a command") + .with_rule(ID) + .with_label( + format!("SC{}[{}]: {}", comment.code, comment.level, comment.message), + span, + ) + .with_fix("address ShellCheck violations") + ) +} + +impl Visitor for CommandSectionShellCheck { + type State = Diagnostics; + + fn document( + &mut self, + _: &mut Self::State, + reason: VisitReason, + _: &Document, + _: SupportedVersion, + ) { + if reason == VisitReason::Exit { + return; + } + + // Reset the visitor upon document entry + *self = Default::default(); + } + + fn command_section( + &mut self, + state: &mut Self::State, + reason: VisitReason, + section: &CommandSection, + ) { + if reason == VisitReason::Exit { + return; + } + + if !program_exists(SHELLCHECK_BIN) { + return; + } + + // collect declarations so we can ignore placeholder variables + let parent_task = section.parent().into_task().expect("parent is a task"); + let mut decls = task_declarations(&parent_task); + + // replace all placeholders in the command with + // dummy bash variables + let mut sanitized_command = String::new(); + if let Some(cmd_parts) = section.strip_whitespace() { + cmd_parts.iter().for_each(|part| match part { + StrippedCommandPart::Text(text) => { + sanitized_command.push_str(text); + } + StrippedCommandPart::Placeholder(placeholder) => { + let bash_var = to_bash_var(placeholder); + // we need to save the var so we can suppress later + decls.insert(bash_var.clone()); + let mut expansion = String::from("\"$"); + expansion.push_str(&bash_var); + expansion.push('"'); + sanitized_command.push_str(&expansion); + } + }); + } else { + // TODO add a diagnostic explaining why this is being skipped? + return; + } + + // get leading whitespace so we can add it to each span + // TODO there may be a more elegant way to do this + let leading_whitespace = { + if let Some(first_part) = section.parts().next() { + match first_part { + CommandPart::Text(text) => { + let text_str = text + .as_str() + .strip_prefix("\n") + .unwrap_or_else(|| text.as_str()); + count_leading_whitespace(text_str) + } + CommandPart::Placeholder(pholder) => { + let pholder_str = &pholder.syntax().text().to_string(); + count_leading_whitespace( + pholder_str.strip_prefix("\n").unwrap_or(pholder_str), + ) + } + } + } else { + 0 + } + }; + + // Map each actual line of the command to its corresponding + // CommandPart and start position + let mut line_map = HashMap::new(); + let mut on_same_line = false; + let mut line_num = 0usize; + for part in section.parts() { + match part { + CommandPart::Text(ref text) => { + let mut last_line = 0usize; + for (_, start, _) in lines_with_offset(text.as_str()) { + // Check to see if we are still on the previous line + // This happens after we encounter a placeholder + if on_same_line { + on_same_line = false; + last_line = last_line.saturating_sub(1); + } + line_map.insert(line_num + last_line, (part.clone(), start)); + last_line += 1; + } + line_num += last_line; + } + CommandPart::Placeholder(_) => { + on_same_line = true; + } + } + } + + match run_shellcheck(&sanitized_command) { + Ok(comments) => { + for comment in comments { + // skip declarations that shellcheck is unaware of + if comment.code == 2154 + && decls.contains(comment.message.split_whitespace().next().unwrap_or("")) + { + continue; + } + let (rel_part, start) = line_map + .get(&comment.line) + .expect("shellcheck line corresponds to command line"); + let rel_token = match rel_part { + CommandPart::Text(text) => text.syntax(), + CommandPart::Placeholder(phold) => { + &support::token(phold.syntax(), SyntaxKind::PlaceholderNode) + .expect("should have a placeholder node token") + } + }; + let inner_span = { + let outer_span = rel_token.text_range().to_span(); + Span::new( + outer_span.start() + leading_whitespace + start + comment.column - 1, + comment.end_column - comment.column, + ) + }; + state.exceptable_add( + shellcheck_lint(&comment, inner_span), + SyntaxElement::from(rel_token.clone()), + &self.exceptable_nodes(), + ) + } + } + Err(e) => { + let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) + .expect("should have a command keyword token"); + state.exceptable_add( + Diagnostic::error("failed to run shellcheck on command block") + .with_label(e.to_string(), command_keyword.text_range().to_span()) + .with_rule(ID) + .with_fix("ensure shellcheck is installed or disable this lint"), + SyntaxElement::from(section.syntax().clone()), + &self.exceptable_nodes(), + ); + } + } + } +} diff --git a/wdl-lint/src/util.rs b/wdl-lint/src/util.rs index 8220afc05..dea4411cf 100644 --- a/wdl-lint/src/util.rs +++ b/wdl-lint/src/util.rs @@ -1,9 +1,20 @@ //! A module for utility functions for the lint rules. +use std::process::Command; +use std::process::Stdio; + use wdl_ast::AstToken; use wdl_ast::Comment; use wdl_ast::SyntaxKind; +/// Count amount of leading whitespace in a str +pub fn count_leading_whitespace(input: &str) -> usize { + input + .chars() + .take_while(|ch| ch.is_whitespace() && *ch != '\n') + .count() +} + /// Detect if a comment is in-line or not by looking for `\n` in the prior /// whitespace. pub fn is_inline_comment(token: &Comment) -> bool { @@ -60,6 +71,17 @@ pub fn lines_with_offset(s: &str) -> impl Iterator }) } +/// check if a program exists via `which` +pub fn program_exists(exec: &str) -> bool { + Command::new("which") + .arg(exec) + .stdout(Stdio::null()) + .stdin(Stdio::null()) + .stderr(Stdio::null()) + .status() + .is_ok_and(|r| r.success()) +} + /// Strips a single newline from the end of a string. pub fn strip_newline(s: &str) -> Option<&str> { s.strip_suffix("\r\n").or_else(|| s.strip_suffix('\n')) diff --git a/wdl-lint/tests/lints/command-shellcheck-ok/source.errors b/wdl-lint/tests/lints/command-shellcheck-ok/source.errors new file mode 100644 index 000000000..b23463316 --- /dev/null +++ b/wdl-lint/tests/lints/command-shellcheck-ok/source.errors @@ -0,0 +1,49 @@ +note[SectionOrdering]: sections are not in order for task `test1` + ┌─ tests/lints/command-shellcheck-ok/source.wdl:7:6 + │ + 7 │ task test1 { + │ ^^^^^ this task contains sections that are out of order + · +13 │ meta {} + │ ---- this section is out of order + │ + = fix: order as `meta`, `parameter_meta`, `input`, private declarations, `command`, `output`, `requirements`/`runtime` + +note[BlankLinesBetweenElements]: extra blank line(s) found + ┌─ tests/lints/command-shellcheck-ok/source.wdl:7:13 + │ +7 │ task test1 { + │ ╭────────────^ +8 │ │ +9 │ │ input { + │ ╰────^ + │ + = fix: remove extra blank line(s) + +warning[MatchingParameterMeta]: task `test1` is missing a parameter metadata key for input `foo` + ┌─ tests/lints/command-shellcheck-ok/source.wdl:10:14 + │ +10 │ String foo = "hello" + │ ^^^ this input does not have an entry in the parameter metadata section + │ + = fix: add a `foo` key to the `parameter_meta` section with a detailed description of the input. + +note[SectionOrdering]: sections are not in order for task `test2` + ┌─ tests/lints/command-shellcheck-ok/source.wdl:30:6 + │ +30 │ task test2 { + │ ^^^^^ this task contains sections that are out of order + · +35 │ meta {} + │ ---- this section is out of order + │ + = fix: order as `meta`, `parameter_meta`, `input`, private declarations, `command`, `output`, `requirements`/`runtime` + +warning[MatchingParameterMeta]: task `test2` is missing a parameter metadata key for input `foo` + ┌─ tests/lints/command-shellcheck-ok/source.wdl:32:14 + │ +32 │ String foo = "hello" + │ ^^^ this input does not have an entry in the parameter metadata section + │ + = fix: add a `foo` key to the `parameter_meta` section with a detailed description of the input. + diff --git a/wdl-lint/tests/lints/command-shellcheck-ok/source.wdl b/wdl-lint/tests/lints/command-shellcheck-ok/source.wdl new file mode 100644 index 000000000..95dfde8b2 --- /dev/null +++ b/wdl-lint/tests/lints/command-shellcheck-ok/source.wdl @@ -0,0 +1,51 @@ +#@ except: DescriptionMissing, RuntimeSectionKeys + +## This is a test of having no shellcheck errors + +version 1.1 + +task test1 { + + input { + String foo = "hello" + } + + meta {} + + parameter_meta {} + + String bar = "there" + + command <<< + set -eo pipefail + + echo ~{hello} ~{there} + >>> + + output {} + + runtime {} +} + +task test2 { + input { + String foo = "hello" + } + + meta {} + + parameter_meta {} + + String bar = "there" + + #@ except: NoCurlyCommands + command { + set -eo pipefail + + echo ~{hello} ~{there} + } + + output {} + + runtime {} +} diff --git a/wdl-lint/tests/lints/command-shellcheck-warn/source.errors b/wdl-lint/tests/lints/command-shellcheck-warn/source.errors new file mode 100644 index 000000000..119351d35 --- /dev/null +++ b/wdl-lint/tests/lints/command-shellcheck-warn/source.errors @@ -0,0 +1,16 @@ +note[CommandSectionShellCheck]: shellcheck violations within a command + ┌─ tests/lints/command-shellcheck-warn/source.wdl:15:10 + │ +15 │ echo $foo + │ ^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address ShellCheck violations + +note[CommandSectionShellCheck]: shellcheck violations within a command + ┌─ tests/lints/command-shellcheck-warn/source.wdl:32:12 + │ +32 │ echo $foo + │ ^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address ShellCheck violations + diff --git a/wdl-lint/tests/lints/command-shellcheck-warn/source.wdl b/wdl-lint/tests/lints/command-shellcheck-warn/source.wdl new file mode 100644 index 000000000..3508d5419 --- /dev/null +++ b/wdl-lint/tests/lints/command-shellcheck-warn/source.wdl @@ -0,0 +1,38 @@ +#@ except: DescriptionMissing, RuntimeSectionKeys + +## This is a test of having shellcheck warnings + +version 1.1 + +task test1 { + meta {} + + parameter_meta {} + + command <<< + set -eo pipefail + foo="123 456" + echo $foo + >>> + + output {} + + runtime {} +} + +task test2 { + meta {} + + parameter_meta {} + + #@ except: NoCurlyCommands + command { + set -eo pipefail + foo="123 456" + echo $foo + } + + output {} + + runtime {} +} From 3ad84255ccdd892e22641e94ef726eefcb7aa068 Mon Sep 17 00:00:00 2001 From: Spencer Richman Date: Sun, 8 Dec 2024 18:08:25 -0500 Subject: [PATCH 02/52] fix: apply stylistic suggestions from PR (#191) Co-authored-by: Clay McLeod <3411613+claymcleod@users.noreply.github.com> --- wdl-lint/src/rules/command_shellcheck.rs | 36 ++++++++++++------------ wdl-lint/src/util.rs | 4 +-- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/wdl-lint/src/rules/command_shellcheck.rs b/wdl-lint/src/rules/command_shellcheck.rs index 109f16a26..5b63e07b6 100644 --- a/wdl-lint/src/rules/command_shellcheck.rs +++ b/wdl-lint/src/rules/command_shellcheck.rs @@ -52,7 +52,7 @@ const SHELLCHECK_SUPPRESS: &[&str] = &[ /// The identifier for the command section ShellCheck rule. const ID: &str = "CommandSectionShellCheck"; -/// A ShellCheck comment +/// A ShellCheck comment. /// /// The file and fix fields are ommitted as we have no use for them. #[derive(Clone, Debug, Deserialize)] @@ -93,33 +93,33 @@ fn run_shellcheck(command: &str) -> Result> { .stdin(Stdio::piped()) .stdout(Stdio::piped()) .spawn() - .context("error calling shellcheck")?; + .context("spawning the `shellcheck` process")?; { let mut proc_stdin = sc_proc .stdin .take() - .context("error obtaining stdin of shellcheck")?; + .context("obtaining the STDIN handle of the `shellcheck` process")?; proc_stdin.write_all(command.as_bytes())?; } let output = sc_proc .wait_with_output() - .context("error calling shellcheck")?; + .context("waiting for the `shellcheck` process to complete")?; // shellcheck returns exit code 1 if // any checked files result in comments // so cannot check with status.success() match output.status.code() { Some(0) | Some(1) => serde_json::from_slice::>(&output.stdout) - .context("failed to parse shellcheck output"), - Some(code) => bail!("error calling shellcheck: exit code {}", code), - None => bail!("error calling check: process interrupted"), + .context("deserializing STDOUT from `shellcheck` process"), + Some(code) => bail!("unexpected `shellcheck` exit code: {}", code), + None => bail!("the `shellcheck` process appears to have been interrupted"), } } /// Runs ShellCheck on a command section and reports violations #[derive(Default, Debug, Clone, Copy)] -pub struct CommandSectionShellCheck; +pub struct ShellCheckRule; impl Rule for CommandSectionShellCheck { fn id(&self) -> &'static str { @@ -150,7 +150,7 @@ impl Rule for CommandSectionShellCheck { } } -/// Convert a WDL placeholder to a bash variable +/// Convert a WDL placeholder to a bash variable. fn to_bash_var(placeholder: &Placeholder) -> String { let placeholder_text = placeholder.syntax().text().to_string(); let mut bash_var = String::from("_WL"); @@ -169,8 +169,8 @@ fn to_bash_var(placeholder: &Placeholder) -> String { bash_var } -/// retrieve all input and private declations for a task -fn task_declarations(task: &TaskDefinition) -> HashSet { +/// Retrieve all input and private declarations for a task. +fn gather_task_declarations(task: &TaskDefinition) -> HashSet { let mut decls = HashSet::new(); if let Some(input) = task.input() { for decl in input.declarations() { @@ -187,13 +187,13 @@ fn task_declarations(task: &TaskDefinition) -> HashSet { /// Creates a "ShellCheck lint" diagnostic from a ShellCheckComment fn shellcheck_lint(comment: &ShellCheckComment, span: Span) -> Diagnostic { dbg!( - Diagnostic::note("shellcheck violations within a command") + Diagnostic::note("`shellcheck` reported the following diagnostic") .with_rule(ID) .with_label( format!("SC{}[{}]: {}", comment.code, comment.level, comment.message), span, ) - .with_fix("address ShellCheck violations") + .with_fix("address the diagnostics as recommended in the message") ) } @@ -229,11 +229,11 @@ impl Visitor for CommandSectionShellCheck { return; } - // collect declarations so we can ignore placeholder variables + // Collect declarations so we can ignore placeholder variables let parent_task = section.parent().into_task().expect("parent is a task"); let mut decls = task_declarations(&parent_task); - // replace all placeholders in the command with + // Replace all placeholders in the command with // dummy bash variables let mut sanitized_command = String::new(); if let Some(cmd_parts) = section.strip_whitespace() { @@ -281,7 +281,7 @@ impl Visitor for CommandSectionShellCheck { }; // Map each actual line of the command to its corresponding - // CommandPart and start position + // `CommandPart` and start position. let mut line_map = HashMap::new(); let mut on_same_line = false; let mut line_num = 0usize; @@ -310,7 +310,7 @@ impl Visitor for CommandSectionShellCheck { match run_shellcheck(&sanitized_command) { Ok(comments) => { for comment in comments { - // skip declarations that shellcheck is unaware of + // Skip declarations that shellcheck is unaware of. if comment.code == 2154 && decls.contains(comment.message.split_whitespace().next().unwrap_or("")) { @@ -344,7 +344,7 @@ impl Visitor for CommandSectionShellCheck { let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) .expect("should have a command keyword token"); state.exceptable_add( - Diagnostic::error("failed to run shellcheck on command block") + Diagnostic::error("running `shellcheck` on command block") .with_label(e.to_string(), command_keyword.text_range().to_span()) .with_rule(ID) .with_fix("ensure shellcheck is installed or disable this lint"), diff --git a/wdl-lint/src/util.rs b/wdl-lint/src/util.rs index dea4411cf..f5188c0d3 100644 --- a/wdl-lint/src/util.rs +++ b/wdl-lint/src/util.rs @@ -7,7 +7,7 @@ use wdl_ast::AstToken; use wdl_ast::Comment; use wdl_ast::SyntaxKind; -/// Count amount of leading whitespace in a str +/// Counts the amount of leading whitespace in a string slice. pub fn count_leading_whitespace(input: &str) -> usize { input .chars() @@ -71,7 +71,7 @@ pub fn lines_with_offset(s: &str) -> impl Iterator }) } -/// check if a program exists via `which` +/// Check whether or not a program exists via `which`. pub fn program_exists(exec: &str) -> bool { Command::new("which") .arg(exec) From 75e04c4485450a60fbbb401daef4f94aed0d735e Mon Sep 17 00:00:00 2001 From: thatrichman Date: Sun, 8 Dec 2024 18:16:37 -0500 Subject: [PATCH 03/52] fix: address stylistic comments from PR (#191) --- wdl-lint/src/lib.rs | 2 +- wdl-lint/src/rules.rs | 4 ++-- wdl-lint/src/rules/command_shellcheck.rs | 18 +++++++++++------- 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/wdl-lint/src/lib.rs b/wdl-lint/src/lib.rs index d45df6554..87aabf5a3 100644 --- a/wdl-lint/src/lib.rs +++ b/wdl-lint/src/lib.rs @@ -84,7 +84,6 @@ pub trait Rule: Visitor { /// Gets the default rule set. pub fn rules() -> Vec> { let rules: Vec> = vec![ - Box::::default(), Box::::default(), Box::::default(), Box::::default(), @@ -126,6 +125,7 @@ pub fn rules() -> Vec> { Box::::default(), Box::::default(), Box::::default(), + Box::::default(), ]; // Ensure all the rule ids are unique and pascal case diff --git a/wdl-lint/src/rules.rs b/wdl-lint/src/rules.rs index 50a925b31..e7ae6c59a 100644 --- a/wdl-lint/src/rules.rs +++ b/wdl-lint/src/rules.rs @@ -3,7 +3,7 @@ mod blank_lines_between_elements; mod call_input_spacing; mod command_mixed_indentation; -mod command_shellcheck; +mod shellcheck; mod comment_whitespace; mod container_value; mod deprecated_object; @@ -46,7 +46,7 @@ mod whitespace; pub use blank_lines_between_elements::*; pub use call_input_spacing::*; pub use command_mixed_indentation::*; -pub use command_shellcheck::*; +pub use shellcheck::*; pub use comment_whitespace::*; pub use container_value::*; pub use deprecated_object::*; diff --git a/wdl-lint/src/rules/command_shellcheck.rs b/wdl-lint/src/rules/command_shellcheck.rs index 5b63e07b6..a4922e6c7 100644 --- a/wdl-lint/src/rules/command_shellcheck.rs +++ b/wdl-lint/src/rules/command_shellcheck.rs @@ -49,6 +49,9 @@ const SHELLCHECK_SUPPRESS: &[&str] = &[ "2193", // The arguments to this comparison can never be equal ]; +/// ShellCheck: var is referenced by not assigned. +const SHELLCHECK_REFERENCED_UNASSIGNED: usize = 2154; + /// The identifier for the command section ShellCheck rule. const ID: &str = "CommandSectionShellCheck"; @@ -121,7 +124,7 @@ fn run_shellcheck(command: &str) -> Result> { #[derive(Default, Debug, Clone, Copy)] pub struct ShellCheckRule; -impl Rule for CommandSectionShellCheck { +impl Rule for ShellCheckRule { fn id(&self) -> &'static str { ID } @@ -197,7 +200,7 @@ fn shellcheck_lint(comment: &ShellCheckComment, span: Span) -> Diagnostic { ) } -impl Visitor for CommandSectionShellCheck { +impl Visitor for ShellCheckRule { type State = Diagnostics; fn document( @@ -231,7 +234,7 @@ impl Visitor for CommandSectionShellCheck { // Collect declarations so we can ignore placeholder variables let parent_task = section.parent().into_task().expect("parent is a task"); - let mut decls = task_declarations(&parent_task); + let mut decls = gather_task_declarations(&parent_task); // Replace all placeholders in the command with // dummy bash variables @@ -252,12 +255,13 @@ impl Visitor for CommandSectionShellCheck { } }); } else { - // TODO add a diagnostic explaining why this is being skipped? + // This is the case where the command section contains + // mixed indentation. We silently return and allow + // the mixed indentation lint to report this. return; } // get leading whitespace so we can add it to each span - // TODO there may be a more elegant way to do this let leading_whitespace = { if let Some(first_part) = section.parts().next() { match first_part { @@ -311,7 +315,7 @@ impl Visitor for CommandSectionShellCheck { Ok(comments) => { for comment in comments { // Skip declarations that shellcheck is unaware of. - if comment.code == 2154 + if comment.code == SHELLCHECK_REFERENCED_UNASSIGNED && decls.contains(comment.message.split_whitespace().next().unwrap_or("")) { continue; @@ -347,7 +351,7 @@ impl Visitor for CommandSectionShellCheck { Diagnostic::error("running `shellcheck` on command block") .with_label(e.to_string(), command_keyword.text_range().to_span()) .with_rule(ID) - .with_fix("ensure shellcheck is installed or disable this lint"), + .with_fix("address reported error."), SyntaxElement::from(section.syntax().clone()), &self.exceptable_nodes(), ); From a8c4d3480bddae512d4480020a0bc90660e99c68 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Sun, 8 Dec 2024 19:04:19 -0500 Subject: [PATCH 04/52] fix: only test for shellcheck once Tests if shellcheck exists and assigns the result to a OnceLock. If shellcheck is not found, adds a diagnostic and returns. --- .../{command_shellcheck.rs => shellcheck.rs} | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) rename wdl-lint/src/rules/{command_shellcheck.rs => shellcheck.rs} (92%) diff --git a/wdl-lint/src/rules/command_shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs similarity index 92% rename from wdl-lint/src/rules/command_shellcheck.rs rename to wdl-lint/src/rules/shellcheck.rs index a4922e6c7..d82af7bca 100644 --- a/wdl-lint/src/rules/command_shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -4,6 +4,7 @@ use std::collections::HashSet; use std::io::Write; use std::process; use std::process::Stdio; +use std::sync::OnceLock; use anyhow::Context; use anyhow::Result; @@ -52,6 +53,9 @@ const SHELLCHECK_SUPPRESS: &[&str] = &[ /// ShellCheck: var is referenced by not assigned. const SHELLCHECK_REFERENCED_UNASSIGNED: usize = 2154; +/// Whether or not shellcheck exists on the system +static SHELLCHECK_EXISTS: OnceLock = OnceLock::new(); + /// The identifier for the command section ShellCheck rule. const ID: &str = "CommandSectionShellCheck"; @@ -228,7 +232,25 @@ impl Visitor for ShellCheckRule { return; } - if !program_exists(SHELLCHECK_BIN) { + if !SHELLCHECK_EXISTS.get_or_init(|| { + if !program_exists(SHELLCHECK_BIN) { + let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) + .expect("should have a command keyword token"); + state.exceptable_add( + Diagnostic::error("running `shellcheck` on command section") + .with_label( + "could not find `shellcheck` executable.", + command_keyword.text_range().to_span(), + ) + .with_rule(ID) + .with_fix("install shellcheck or disable this lint."), + SyntaxElement::from(section.syntax().clone()), + &self.exceptable_nodes(), + ); + return false; + } + true + }) { return; } From a36ae839fec34d745c9a9781b4ca7fcb691dd142 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Mon, 9 Dec 2024 22:30:27 -0500 Subject: [PATCH 05/52] fix: address stylistic comments from PR (#191) --- Cargo.toml | 1 + wdl-lint/Cargo.toml | 5 +++-- wdl-lint/src/rules/shellcheck.rs | 18 +++++++++--------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4998b185d..c06ac77c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,6 +47,7 @@ parking_lot = "0.12.3" path-clean = "1.0.1" petgraph = "0.6.5" pretty_assertions = "1.4.0" +rand = "0.8.5" rayon = "1.10.0" regex = "1.11.1" reqwest = { version = "0.12.5", default-features = false, features = ["rustls-tls", "http2", "charset"] } diff --git a/wdl-lint/Cargo.toml b/wdl-lint/Cargo.toml index 2f2476379..438af688b 100644 --- a/wdl-lint/Cargo.toml +++ b/wdl-lint/Cargo.toml @@ -13,12 +13,13 @@ readme = "../README.md" [dependencies] wdl-ast = { path = "../wdl-ast", version = "0.9.0" } +anyhow = { workspace = true } convert_case = { workspace = true } indexmap = { workspace = true } +rand = { workspace = true } rowan = { workspace = true } -serde_json = { workspace = true } -anyhow = { workspace = true } serde = { workspace = true } +serde_json = { workspace = true } [dev-dependencies] codespan-reporting = { workspace = true } diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index d82af7bca..43b155a87 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -1,4 +1,4 @@ -//! A lint rule for checking mixed indentation in command text. +//! A lint rule for running shellcheck against command sections. use std::collections::HashMap; use std::collections::HashSet; use std::io::Write; @@ -63,7 +63,7 @@ const ID: &str = "CommandSectionShellCheck"; /// /// The file and fix fields are ommitted as we have no use for them. #[derive(Clone, Debug, Deserialize)] -struct ShellCheckComment { +struct ShellCheckDiagnostic { /// line number comment starts on pub line: usize, /// line number comment ends on @@ -82,11 +82,11 @@ struct ShellCheckComment { pub message: String, } -/// Run shellcheck +/// Run shellcheck on a command. /// /// writes command text to stdin of shellcheck process -/// and returns parsed ShellCheckComments -fn run_shellcheck(command: &str) -> Result> { +/// and returns parsed `ShellCheckDiagnostic`s +fn run_shellcheck(command: &str) -> Result> { let mut sc_proc = process::Command::new(SHELLCHECK_BIN) .args([ "-s", @@ -117,7 +117,7 @@ fn run_shellcheck(command: &str) -> Result> { // any checked files result in comments // so cannot check with status.success() match output.status.code() { - Some(0) | Some(1) => serde_json::from_slice::>(&output.stdout) + Some(0) | Some(1) => serde_json::from_slice::>(&output.stdout) .context("deserializing STDOUT from `shellcheck` process"), Some(code) => bail!("unexpected `shellcheck` exit code: {}", code), None => bail!("the `shellcheck` process appears to have been interrupted"), @@ -138,10 +138,10 @@ impl Rule for ShellCheckRule { } fn explanation(&self) -> &'static str { - "ShellCheck is a static analysis tool and linter for sh / bash. \ + "ShellCheck (https://shellcheck.net) is a static analysis tool and linter for sh / bash. \ The lints provided by ShellCheck help prevent common errors and \ pitfalls in your scripts. Following its recommendations will increase \ - the robustness of your command blocks." + the robustness of your command sections." } fn tags(&self) -> TagSet { @@ -370,7 +370,7 @@ impl Visitor for ShellCheckRule { let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) .expect("should have a command keyword token"); state.exceptable_add( - Diagnostic::error("running `shellcheck` on command block") + Diagnostic::error("running `shellcheck` on command section") .with_label(e.to_string(), command_keyword.text_range().to_span()) .with_rule(ID) .with_fix("address reported error."), From 19a66922f20c4053809119632fa3c4821e677fbf Mon Sep 17 00:00:00 2001 From: thatrichman Date: Mon, 9 Dec 2024 22:31:14 -0500 Subject: [PATCH 06/52] fix: use a random string for bash variables --- wdl-lint/src/rules/shellcheck.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 43b155a87..6bedff8b5 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -9,6 +9,8 @@ use std::sync::OnceLock; use anyhow::Context; use anyhow::Result; use anyhow::bail; +use rand::distributions::Alphanumeric; +use rand::distributions::DistString; use serde::Deserialize; use serde_json; @@ -157,23 +159,13 @@ impl Rule for ShellCheckRule { } } -/// Convert a WDL placeholder to a bash variable. +/// Convert a WDL `Placeholder` to a bash variable declaration. +/// +/// returns a random string three characters shorter than the Placeholder's length +/// to account for '~{}'. fn to_bash_var(placeholder: &Placeholder) -> String { - let placeholder_text = placeholder.syntax().text().to_string(); - let mut bash_var = String::from("_WL"); - bash_var.push_str( - &placeholder_text - .chars() - .map(|c| { - if c.is_ascii_alphanumeric() || c == '_' { - c - } else { - '_' - } - }) - .collect::(), - ); - bash_var + let placeholder_len: usize = placeholder.syntax().text_range().len().into(); + Alphanumeric.sample_string(&mut rand::thread_rng(), placeholder_len - 3) } /// Retrieve all input and private declarations for a task. From 5a8cc11b90498b196c565175bd0351f53c220f4d Mon Sep 17 00:00:00 2001 From: thatrichman Date: Mon, 9 Dec 2024 22:32:34 -0500 Subject: [PATCH 07/52] fix: remove some suppressed lints, enable style lints --- wdl-lint/src/rules/shellcheck.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 6bedff8b5..703b4b79b 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -44,12 +44,8 @@ const SHELLCHECK_BIN: &str = "shellcheck"; /// shellcheck lints that we want to suppress const SHELLCHECK_SUPPRESS: &[&str] = &[ "1009", // the mentioned parser error was in... - "1072", // Unexpected.. + "1072", // Unexpected "1083", // this {/} is literal - "2043", // this loop will only ever run once for constant value - "2050", // This expression is constant - "2157", // Argument to -n is always true due to literal strings - "2193", // The arguments to this comparison can never be equal ]; /// ShellCheck: var is referenced by not assigned. @@ -97,6 +93,8 @@ fn run_shellcheck(command: &str) -> Result> { "json", "-e", &SHELLCHECK_SUPPRESS.join(","), + "-S", + "style", "-", ]) .stdin(Stdio::piped()) From c1375b942d74bc5dfb96bae27f996ef5db4485e3 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Mon, 9 Dec 2024 22:36:05 -0500 Subject: [PATCH 08/52] refactor: split into smaller functions --- wdl-lint/src/rules/shellcheck.rs | 69 +++++++++++++++++++++++++++----- 1 file changed, 58 insertions(+), 11 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 703b4b79b..0285f3247 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -181,17 +181,64 @@ fn gather_task_declarations(task: &TaskDefinition) -> HashSet { decls } -/// Creates a "ShellCheck lint" diagnostic from a ShellCheckComment -fn shellcheck_lint(comment: &ShellCheckComment, span: Span) -> Diagnostic { - dbg!( - Diagnostic::note("`shellcheck` reported the following diagnostic") - .with_rule(ID) - .with_label( - format!("SC{}[{}]: {}", comment.code, comment.level, comment.message), - span, - ) - .with_fix("address the diagnostics as recommended in the message") - ) +/// Creates a "ShellCheck lint" diagnostic from a ShellCheckDiagnostic +fn shellcheck_lint(comment: &ShellCheckDiagnostic, span: Span) -> Diagnostic { + Diagnostic::note("`shellcheck` reported the following diagnostic") + .with_rule(ID) + .with_label( + format!("SC{}[{}]: {}", comment.code, comment.level, comment.message), + span, + ) + .with_fix("address the diagnostics as recommended in the message") +} + +/// Sanitize a `CommandSection`. +/// +/// Removes all trailing whitespace, replaces placeholders +/// with dummy bash variables, and records declarations. +/// +/// If the section contains mixed indentation, returns None +fn sanitize_command(section: &CommandSection) -> Option<(String, HashSet)> { + let mut sanitized_command = String::new(); + let mut decls = HashSet::new(); + if let Some(cmd_parts) = section.strip_whitespace() { + cmd_parts.iter().for_each(|part| match part { + StrippedCommandPart::Text(text) => { + sanitized_command.push_str(text); + } + StrippedCommandPart::Placeholder(placeholder) => { + let bash_var = to_bash_var(placeholder); + // we need to save the var so we can suppress later + decls.insert(bash_var.clone()); + let mut expansion = String::from("\"$"); + expansion.push_str(&bash_var); + expansion.push('"'); + sanitized_command.push_str(&expansion); + } + }); + Some((sanitized_command, decls)) + } else { + None + } +} + +/// Returns the amount of leading whitespace characters in a `CommandSection`. +/// +/// Only checks the first `CommandPart::Text`. +fn count_command_whitespace(section: &CommandSection) -> usize { + if let Some(first_text) = section.parts().find(|p| matches!(p, CommandPart::Text(..))) { + match first_text { + CommandPart::Text(text) => { + let text_str = text + .as_str() + .strip_prefix("\n") + .unwrap_or_else(|| text.as_str()); + return count_leading_whitespace(text_str); + } + CommandPart::Placeholder(_) => unreachable!(), + } + } + 0 } impl Visitor for ShellCheckRule { From 38e999408a4a220437f0a4ba332ed4d0b0de8309 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Mon, 9 Dec 2024 22:37:12 -0500 Subject: [PATCH 09/52] fix: rework WDL -> bash script line mapping --- wdl-lint/src/rules/shellcheck.rs | 100 ++++++++++--------------------- wdl-lint/src/util.rs | 7 +-- 2 files changed, 34 insertions(+), 73 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 0285f3247..266d5631a 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -295,74 +295,40 @@ impl Visitor for ShellCheckRule { let parent_task = section.parent().into_task().expect("parent is a task"); let mut decls = gather_task_declarations(&parent_task); - // Replace all placeholders in the command with - // dummy bash variables - let mut sanitized_command = String::new(); - if let Some(cmd_parts) = section.strip_whitespace() { - cmd_parts.iter().for_each(|part| match part { - StrippedCommandPart::Text(text) => { - sanitized_command.push_str(text); - } - StrippedCommandPart::Placeholder(placeholder) => { - let bash_var = to_bash_var(placeholder); - // we need to save the var so we can suppress later - decls.insert(bash_var.clone()); - let mut expansion = String::from("\"$"); - expansion.push_str(&bash_var); - expansion.push('"'); - sanitized_command.push_str(&expansion); - } - }); - } else { + // Replace all placeholders in the command with dummy bash variables + let Some((sanitized_command, cmd_decls)) = sanitize_command(section) else { // This is the case where the command section contains // mixed indentation. We silently return and allow // the mixed indentation lint to report this. return; - } - - // get leading whitespace so we can add it to each span - let leading_whitespace = { - if let Some(first_part) = section.parts().next() { - match first_part { - CommandPart::Text(text) => { - let text_str = text - .as_str() - .strip_prefix("\n") - .unwrap_or_else(|| text.as_str()); - count_leading_whitespace(text_str) - } - CommandPart::Placeholder(pholder) => { - let pholder_str = &pholder.syntax().text().to_string(); - count_leading_whitespace( - pholder_str.strip_prefix("\n").unwrap_or(pholder_str), - ) - } - } - } else { - 0 - } }; + decls.extend(cmd_decls); + + // Get leading whitespace so we can add it to each span + let leading_whitespace = count_command_whitespace(section); // Map each actual line of the command to its corresponding // `CommandPart` and start position. let mut line_map = HashMap::new(); + let mut line_num = 1; let mut on_same_line = false; - let mut line_num = 0usize; for part in section.parts() { match part { CommandPart::Text(ref text) => { - let mut last_line = 0usize; - for (_, start, _) in lines_with_offset(text.as_str()) { - // Check to see if we are still on the previous line - // This happens after we encounter a placeholder + for (line, start, _) in lines_with_offset(text.as_str()) { + if line_num == 1 && line.trim().is_empty() { + continue; + } if on_same_line { on_same_line = false; - last_line = last_line.saturating_sub(1); + continue; } - line_map.insert(line_num + last_line, (part.clone(), start)); - last_line += 1; + line_map.insert( + line_num, + text.span().start() + start + leading_whitespace - 1, + ); + line_num += 1; } - line_num += last_line; } CommandPart::Placeholder(_) => { on_same_line = true; @@ -371,34 +337,30 @@ impl Visitor for ShellCheckRule { } match run_shellcheck(&sanitized_command) { - Ok(comments) => { - for comment in comments { + Ok(diagnostics) => { + for diagnostic in diagnostics { // Skip declarations that shellcheck is unaware of. - if comment.code == SHELLCHECK_REFERENCED_UNASSIGNED - && decls.contains(comment.message.split_whitespace().next().unwrap_or("")) + // ShellCheck's message always starts with the variable name + // that is unassigned. + let target_variable = + diagnostic.message.split_whitespace().next().unwrap_or(""); + if diagnostic.code == SHELLCHECK_REFERENCED_UNASSIGNED + && decls.contains(target_variable) { continue; } - let (rel_part, start) = line_map - .get(&comment.line) + let start = line_map + .get(&diagnostic.line) .expect("shellcheck line corresponds to command line"); - let rel_token = match rel_part { - CommandPart::Text(text) => text.syntax(), - CommandPart::Placeholder(phold) => { - &support::token(phold.syntax(), SyntaxKind::PlaceholderNode) - .expect("should have a placeholder node token") - } - }; let inner_span = { - let outer_span = rel_token.text_range().to_span(); Span::new( - outer_span.start() + leading_whitespace + start + comment.column - 1, - comment.end_column - comment.column, + start + diagnostic.column, + diagnostic.end_column - diagnostic.column, ) }; state.exceptable_add( - shellcheck_lint(&comment, inner_span), - SyntaxElement::from(rel_token.clone()), + shellcheck_lint(&diagnostic, inner_span), + SyntaxElement::from(section.syntax().clone()), &self.exceptable_nodes(), ) } diff --git a/wdl-lint/src/util.rs b/wdl-lint/src/util.rs index f5188c0d3..2f952dd7a 100644 --- a/wdl-lint/src/util.rs +++ b/wdl-lint/src/util.rs @@ -8,11 +8,10 @@ use wdl_ast::Comment; use wdl_ast::SyntaxKind; /// Counts the amount of leading whitespace in a string slice. +/// +/// Returns when the first non-whitespace character is encountered. pub fn count_leading_whitespace(input: &str) -> usize { - input - .chars() - .take_while(|ch| ch.is_whitespace() && *ch != '\n') - .count() + input.chars().take_while(|ch| ch.is_whitespace()).count() } /// Detect if a comment is in-line or not by looking for `\n` in the prior From c0b9825f846aba3c86285f8ebd1c0b5552927fdf Mon Sep 17 00:00:00 2001 From: thatrichman Date: Mon, 9 Dec 2024 22:37:42 -0500 Subject: [PATCH 10/52] fix: make `program_exists` cross-platform --- wdl-lint/src/util.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/wdl-lint/src/util.rs b/wdl-lint/src/util.rs index 2f952dd7a..746dec42e 100644 --- a/wdl-lint/src/util.rs +++ b/wdl-lint/src/util.rs @@ -70,9 +70,13 @@ pub fn lines_with_offset(s: &str) -> impl Iterator }) } -/// Check whether or not a program exists via `which`. +/// Check whether or not a program exists. +/// +/// On unix-like OSes, uses `which`. +/// On Windows, uses `where.exe`. pub fn program_exists(exec: &str) -> bool { - Command::new("which") + let finder = if cfg!(windows) { "where.exe" } else { "which" }; + Command::new(finder) .arg(exec) .stdout(Stdio::null()) .stdin(Stdio::null()) From 1423a22b78b3678d3e12dcaed0d8c373a7747aaa Mon Sep 17 00:00:00 2001 From: thatrichman Date: Mon, 9 Dec 2024 22:38:10 -0500 Subject: [PATCH 11/52] feat: add tests for shellcheck lint --- wdl-lint/src/util.rs | 23 +++++++++ .../lints/command-shellcheck-ok/source.errors | 49 ------------------ .../lints/command-shellcheck-ok/source.wdl | 51 ------------------- .../command-shellcheck-warn/source.errors | 16 ------ .../lints/command-shellcheck-warn/source.wdl | 38 -------------- .../source.errors | 8 +++ 6 files changed, 31 insertions(+), 154 deletions(-) delete mode 100644 wdl-lint/tests/lints/command-shellcheck-ok/source.errors delete mode 100644 wdl-lint/tests/lints/command-shellcheck-ok/source.wdl delete mode 100644 wdl-lint/tests/lints/command-shellcheck-warn/source.errors delete mode 100644 wdl-lint/tests/lints/command-shellcheck-warn/source.wdl diff --git a/wdl-lint/src/util.rs b/wdl-lint/src/util.rs index 746dec42e..c14da4d4e 100644 --- a/wdl-lint/src/util.rs +++ b/wdl-lint/src/util.rs @@ -170,4 +170,27 @@ task foo { # an in-line comment ("and even a \r that should not be a newline", 53, 95), ]); } + + #[test] + fn test_count_leading_whitespace() { + let s = " this string has four leading spaces"; + assert_eq!(count_leading_whitespace(s), 4); + let s = "\t\t\t\tthis string has four leading tabs"; + assert_eq!(count_leading_whitespace(s), 4); + let s = "\r\r\r\rthis has four leading carriage returns"; + assert_eq!(count_leading_whitespace(s), 4); + let s = "\n starts with a newline"; + assert_eq!(count_leading_whitespace(s), 2); + let s = "I have no leading whitespace"; + assert_eq!(count_leading_whitespace(s), 0); + } + + #[test] + fn test_program_exists() { + if cfg!(windows) { + assert!(program_exists("where.exe")); + } else { + assert!(program_exists("which")); + } + } } diff --git a/wdl-lint/tests/lints/command-shellcheck-ok/source.errors b/wdl-lint/tests/lints/command-shellcheck-ok/source.errors deleted file mode 100644 index b23463316..000000000 --- a/wdl-lint/tests/lints/command-shellcheck-ok/source.errors +++ /dev/null @@ -1,49 +0,0 @@ -note[SectionOrdering]: sections are not in order for task `test1` - ┌─ tests/lints/command-shellcheck-ok/source.wdl:7:6 - │ - 7 │ task test1 { - │ ^^^^^ this task contains sections that are out of order - · -13 │ meta {} - │ ---- this section is out of order - │ - = fix: order as `meta`, `parameter_meta`, `input`, private declarations, `command`, `output`, `requirements`/`runtime` - -note[BlankLinesBetweenElements]: extra blank line(s) found - ┌─ tests/lints/command-shellcheck-ok/source.wdl:7:13 - │ -7 │ task test1 { - │ ╭────────────^ -8 │ │ -9 │ │ input { - │ ╰────^ - │ - = fix: remove extra blank line(s) - -warning[MatchingParameterMeta]: task `test1` is missing a parameter metadata key for input `foo` - ┌─ tests/lints/command-shellcheck-ok/source.wdl:10:14 - │ -10 │ String foo = "hello" - │ ^^^ this input does not have an entry in the parameter metadata section - │ - = fix: add a `foo` key to the `parameter_meta` section with a detailed description of the input. - -note[SectionOrdering]: sections are not in order for task `test2` - ┌─ tests/lints/command-shellcheck-ok/source.wdl:30:6 - │ -30 │ task test2 { - │ ^^^^^ this task contains sections that are out of order - · -35 │ meta {} - │ ---- this section is out of order - │ - = fix: order as `meta`, `parameter_meta`, `input`, private declarations, `command`, `output`, `requirements`/`runtime` - -warning[MatchingParameterMeta]: task `test2` is missing a parameter metadata key for input `foo` - ┌─ tests/lints/command-shellcheck-ok/source.wdl:32:14 - │ -32 │ String foo = "hello" - │ ^^^ this input does not have an entry in the parameter metadata section - │ - = fix: add a `foo` key to the `parameter_meta` section with a detailed description of the input. - diff --git a/wdl-lint/tests/lints/command-shellcheck-ok/source.wdl b/wdl-lint/tests/lints/command-shellcheck-ok/source.wdl deleted file mode 100644 index 95dfde8b2..000000000 --- a/wdl-lint/tests/lints/command-shellcheck-ok/source.wdl +++ /dev/null @@ -1,51 +0,0 @@ -#@ except: DescriptionMissing, RuntimeSectionKeys - -## This is a test of having no shellcheck errors - -version 1.1 - -task test1 { - - input { - String foo = "hello" - } - - meta {} - - parameter_meta {} - - String bar = "there" - - command <<< - set -eo pipefail - - echo ~{hello} ~{there} - >>> - - output {} - - runtime {} -} - -task test2 { - input { - String foo = "hello" - } - - meta {} - - parameter_meta {} - - String bar = "there" - - #@ except: NoCurlyCommands - command { - set -eo pipefail - - echo ~{hello} ~{there} - } - - output {} - - runtime {} -} diff --git a/wdl-lint/tests/lints/command-shellcheck-warn/source.errors b/wdl-lint/tests/lints/command-shellcheck-warn/source.errors deleted file mode 100644 index 119351d35..000000000 --- a/wdl-lint/tests/lints/command-shellcheck-warn/source.errors +++ /dev/null @@ -1,16 +0,0 @@ -note[CommandSectionShellCheck]: shellcheck violations within a command - ┌─ tests/lints/command-shellcheck-warn/source.wdl:15:10 - │ -15 │ echo $foo - │ ^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. - │ - = fix: address ShellCheck violations - -note[CommandSectionShellCheck]: shellcheck violations within a command - ┌─ tests/lints/command-shellcheck-warn/source.wdl:32:12 - │ -32 │ echo $foo - │ ^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. - │ - = fix: address ShellCheck violations - diff --git a/wdl-lint/tests/lints/command-shellcheck-warn/source.wdl b/wdl-lint/tests/lints/command-shellcheck-warn/source.wdl deleted file mode 100644 index 3508d5419..000000000 --- a/wdl-lint/tests/lints/command-shellcheck-warn/source.wdl +++ /dev/null @@ -1,38 +0,0 @@ -#@ except: DescriptionMissing, RuntimeSectionKeys - -## This is a test of having shellcheck warnings - -version 1.1 - -task test1 { - meta {} - - parameter_meta {} - - command <<< - set -eo pipefail - foo="123 456" - echo $foo - >>> - - output {} - - runtime {} -} - -task test2 { - meta {} - - parameter_meta {} - - #@ except: NoCurlyCommands - command { - set -eo pipefail - foo="123 456" - echo $foo - } - - output {} - - runtime {} -} diff --git a/wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors b/wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors index 07521d2dc..78934ec73 100644 --- a/wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors +++ b/wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors @@ -46,3 +46,11 @@ note[DeprecatedPlaceholderOption]: use of the deprecated `default` placeholder o │ = fix: replace the `default` placeholder option with a call to the `select_first()` standard library function +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/deprecated-placeholder-options-v1.1/source.wdl:33:27 + │ +33 │ python script.py ~{sep(" ", numbers)} + │ ^ SC1037[error]: Braces are required for positionals over 9, e.g. ${10}. + │ + = fix: address the diagnostics as recommended in the message + From f7c5431e650fe03259d7a5e4b7d169e8e8356d99 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Mon, 9 Dec 2024 23:05:55 -0500 Subject: [PATCH 12/52] fix: prefix all bash vars with 'WDL' --- wdl-lint/src/rules/shellcheck.rs | 11 ++++++++--- .../deprecated-placeholder-options-v1.1/source.errors | 8 -------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 266d5631a..4a2706fa2 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -159,11 +159,16 @@ impl Rule for ShellCheckRule { /// Convert a WDL `Placeholder` to a bash variable declaration. /// -/// returns a random string three characters shorter than the Placeholder's length -/// to account for '~{}'. +/// returns "WDL" + random alphnumeric characters. +/// 'WDL' + '~{}' = 6. fn to_bash_var(placeholder: &Placeholder) -> String { let placeholder_len: usize = placeholder.syntax().text_range().len().into(); - Alphanumeric.sample_string(&mut rand::thread_rng(), placeholder_len - 3) + // don't start variable with numbers + let mut bash_var = String::from("WDL"); + bash_var.push_str( + &Alphanumeric.sample_string(&mut rand::thread_rng(), placeholder_len.saturating_sub(6)), + ); + bash_var } /// Retrieve all input and private declarations for a task. diff --git a/wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors b/wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors index 78934ec73..07521d2dc 100644 --- a/wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors +++ b/wdl-lint/tests/lints/deprecated-placeholder-options-v1.1/source.errors @@ -46,11 +46,3 @@ note[DeprecatedPlaceholderOption]: use of the deprecated `default` placeholder o │ = fix: replace the `default` placeholder option with a call to the `select_first()` standard library function -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic - ┌─ tests/lints/deprecated-placeholder-options-v1.1/source.wdl:33:27 - │ -33 │ python script.py ~{sep(" ", numbers)} - │ ^ SC1037[error]: Braces are required for positionals over 9, e.g. ${10}. - │ - = fix: address the diagnostics as recommended in the message - From 05a583ba30f4a2d0897c111c64e3456c0e8fb9da Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 10 Dec 2024 08:54:14 -0500 Subject: [PATCH 13/52] feat: add tests for shellcheck lints --- .../lints/shellcheck-error/source.errors | 48 +++++ .../tests/lints/shellcheck-error/source.wdl | 43 +++++ .../tests/lints/shellcheck-ok/source.errors | 0 wdl-lint/tests/lints/shellcheck-ok/source.wdl | 54 ++++++ .../lints/shellcheck-style/source.errors | 32 ++++ .../tests/lints/shellcheck-style/source.wdl | 43 +++++ .../tests/lints/shellcheck-warn/source.errors | 176 ++++++++++++++++++ .../tests/lints/shellcheck-warn/source.wdl | 60 ++++++ 8 files changed, 456 insertions(+) create mode 100644 wdl-lint/tests/lints/shellcheck-error/source.errors create mode 100644 wdl-lint/tests/lints/shellcheck-error/source.wdl create mode 100644 wdl-lint/tests/lints/shellcheck-ok/source.errors create mode 100644 wdl-lint/tests/lints/shellcheck-ok/source.wdl create mode 100644 wdl-lint/tests/lints/shellcheck-style/source.errors create mode 100644 wdl-lint/tests/lints/shellcheck-style/source.wdl create mode 100644 wdl-lint/tests/lints/shellcheck-warn/source.errors create mode 100644 wdl-lint/tests/lints/shellcheck-warn/source.wdl diff --git a/wdl-lint/tests/lints/shellcheck-error/source.errors b/wdl-lint/tests/lints/shellcheck-error/source.errors new file mode 100644 index 000000000..8da8ac0d3 --- /dev/null +++ b/wdl-lint/tests/lints/shellcheck-error/source.errors @@ -0,0 +1,48 @@ +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-error/source.wdl:18:10 + │ +18 │ if [ -f "$broken"] + │ ^ SC1073[error]: Couldn't parse this test expression. Fix to allow more checks. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-error/source.wdl:18:15 + │ +18 │ if [ -f "$broken"] + │ ^ SC1019[error]: Expected this to be an argument to the unary condition. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-error/source.wdl:18:25 + │ +18 │ if [ -f "$broken"] + │ ^ SC1020[error]: You need a space before the ]. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-error/source.wdl:37:10 + │ +37 │ if [ -f "$broken"] + │ ^ SC1073[error]: Couldn't parse this test expression. Fix to allow more checks. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-error/source.wdl:37:15 + │ +37 │ if [ -f "$broken"] + │ ^ SC1019[error]: Expected this to be an argument to the unary condition. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-error/source.wdl:37:25 + │ +37 │ if [ -f "$broken"] + │ ^ SC1020[error]: You need a space before the ]. + │ + = fix: address the diagnostics as recommended in the message + diff --git a/wdl-lint/tests/lints/shellcheck-error/source.wdl b/wdl-lint/tests/lints/shellcheck-error/source.wdl new file mode 100644 index 000000000..63525fff4 --- /dev/null +++ b/wdl-lint/tests/lints/shellcheck-error/source.wdl @@ -0,0 +1,43 @@ +#@ except: DescriptionMissing, RuntimeSectionKeys, MatchingParameterMeta, NoCurlyCommands + +## This is a test of having shellcheck error lints + +version 1.1 + +task test1 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command <<< + somecommand.py [[ -f $broken_test]] + if [ -f "$broken"] + >>> + + output {} + + runtime {} +} + +task test2 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command { + somecommand.py [[ -f $broken_test]] + if [ -f "$broken"] + } + + output {} + + runtime {} +} diff --git a/wdl-lint/tests/lints/shellcheck-ok/source.errors b/wdl-lint/tests/lints/shellcheck-ok/source.errors new file mode 100644 index 000000000..e69de29bb diff --git a/wdl-lint/tests/lints/shellcheck-ok/source.wdl b/wdl-lint/tests/lints/shellcheck-ok/source.wdl new file mode 100644 index 000000000..0f6191b04 --- /dev/null +++ b/wdl-lint/tests/lints/shellcheck-ok/source.wdl @@ -0,0 +1,54 @@ +#@ except: DescriptionMissing, RuntimeSectionKeys, MatchingParameterMeta, NoCurlyCommands + +## This is a test of having no shellcheck lints + +version 1.1 + +task test1 { + meta {} + + parameter_meta {} + + input { + Boolean i_quote_my_shellvars + Int placeholder + } + + command <<< + set -eo pipefail + + echo "$placeholder" + + if [[ "$i_quote_my_shellvars" ]]; then + echo "shellcheck will be happy" + fi + >>> + + output {} + + runtime {} +} + +task test2 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command { + set -eo pipefail + + echo "$placeholder" + + if [[ "$I_quote_my_shellvars" ]]; then + echo "all is well" + fi + } + + output {} + + runtime {} +} diff --git a/wdl-lint/tests/lints/shellcheck-style/source.errors b/wdl-lint/tests/lints/shellcheck-style/source.errors new file mode 100644 index 000000000..87b6b6ac1 --- /dev/null +++ b/wdl-lint/tests/lints/shellcheck-style/source.errors @@ -0,0 +1,32 @@ +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-style/source.wdl:17:7 + │ +17 │ [[ ]] + │ ^^^ SC2212[style]: Use 'false' instead of empty [/[[ conditionals. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-style/source.wdl:18:9 + │ +18 │ [ true ] + │ ^^^^ SC2160[style]: Instead of '[ true ]', just use 'true'. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-style/source.wdl:36:7 + │ +36 │ [[ ]] + │ ^^^ SC2212[style]: Use 'false' instead of empty [/[[ conditionals. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-style/source.wdl:37:9 + │ +37 │ [ true ] + │ ^^^^ SC2160[style]: Instead of '[ true ]', just use 'true'. + │ + = fix: address the diagnostics as recommended in the message + diff --git a/wdl-lint/tests/lints/shellcheck-style/source.wdl b/wdl-lint/tests/lints/shellcheck-style/source.wdl new file mode 100644 index 000000000..4d3795289 --- /dev/null +++ b/wdl-lint/tests/lints/shellcheck-style/source.wdl @@ -0,0 +1,43 @@ +#@ except: DescriptionMissing, RuntimeSectionKeys, MatchingParameterMeta, NoCurlyCommands + +## This is a test of having shellcheck style lints + +version 1.1 + +task test1 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command <<< + [[ ]] + [ true ] + >>> + + output {} + + runtime {} +} + +task test2 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command { + [[ ]] + [ true ] + } + + output {} + + runtime {} +} diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.errors b/wdl-lint/tests/lints/shellcheck-warn/source.errors new file mode 100644 index 000000000..489f94715 --- /dev/null +++ b/wdl-lint/tests/lints/shellcheck-warn/source.errors @@ -0,0 +1,176 @@ +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:17:22 + │ +17 │ somecommand.py $line17 ~{placeholder} + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:17:22 + │ +17 │ somecommand.py $line17 ~{placeholder} + │ ^^^^^^^ SC2154[warning]: line17 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:18:37 + │ +18 │ somecommand.py ~{placeholder} $line18 + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:18:37 + │ +18 │ somecommand.py ~{placeholder} $line18 + │ ^^^^^^^ SC2154[warning]: line18 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:19:36 + │ +19 │ somecommand.py ~{placeholder}$line19 + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:19:36 + │ +19 │ somecommand.py ~{placeholder}$line19 + │ ^^^^^^^ SC2154[warning]: line19 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:30:22 + │ +30 │ somecommand.py $line30~{placeholder} + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:30:22 + │ +30 │ somecommand.py $line30~{placeholder} + │ ^^^^^^^ SC2154[warning]: line30 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:31:27 + │ +31 │ somecommand.py [ -f $line31 ] ~{placeholder} + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:31:27 + │ +31 │ somecommand.py [ -f $line31 ] ~{placeholder} + │ ^^^^^^^ SC2154[warning]: line31 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:49:22 + │ +49 │ somecommand.py $line49 ~{placeholder} + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:49:22 + │ +49 │ somecommand.py $line49 ~{placeholder} + │ ^^^^^^^ SC2154[warning]: line49 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:50:37 + │ +50 │ somecommand.py ~{placeholder} $line50 + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:50:37 + │ +50 │ somecommand.py ~{placeholder} $line50 + │ ^^^^^^^ SC2154[warning]: line50 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:51:36 + │ +51 │ somecommand.py ~{placeholder}$line51 + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:51:36 + │ +51 │ somecommand.py ~{placeholder}$line51 + │ ^^^^^^^ SC2154[warning]: line51 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:52:22 + │ +52 │ somecommand.py $line52~{placeholder} + │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:52:22 + │ +52 │ somecommand.py $line52~{placeholder} + │ ^^^^^^^ SC2154[warning]: line52 is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:53:27 + │ +53 │ somecommand.py [ -f $bad_test ] ~{placeholder} + │ ^^^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:53:27 + │ +53 │ somecommand.py [ -f $bad_test ] ~{placeholder} + │ ^^^^^^^^^ SC2154[warning]: bad_test is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:54:27 + │ +54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} + │ ^^^^^^^^^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ + = fix: address the diagnostics as recommended in the message + +note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:54:27 + │ +54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} + │ ^^^^^^^^^^^^^^^ SC2154[warning]: trailing_space is referenced but not assigned. + │ + = fix: address the diagnostics as recommended in the message + diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.wdl b/wdl-lint/tests/lints/shellcheck-warn/source.wdl new file mode 100644 index 000000000..bc863b26d --- /dev/null +++ b/wdl-lint/tests/lints/shellcheck-warn/source.wdl @@ -0,0 +1,60 @@ +#@ except: DescriptionMissing, RuntimeSectionKeys, MatchingParameterMeta, NoCurlyCommands + +## This is a test of having shellcheck warnings + +version 1.1 + +task test1 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command <<< + somecommand.py $line17 ~{placeholder} + somecommand.py ~{placeholder} $line18 + somecommand.py ~{placeholder}$line19 + + + + + + + + + + + somecommand.py $line30~{placeholder} + somecommand.py [ -f $line31 ] ~{placeholder} + >>> + + output {} + + runtime {} +} + +task test2 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command <<< + somecommand.py $line49 ~{placeholder} + somecommand.py ~{placeholder} $line50 + somecommand.py ~{placeholder}$line51 + somecommand.py $line52~{placeholder} + somecommand.py [ -f $bad_test ] ~{placeholder} + somecommand.py [ -f $trailing_space ] ~{placeholder} + >>> + + output {} + + runtime {} +} From 7cbe4ac76fa1ec6df306da1d225d1f068f40df06 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 18:41:19 -0500 Subject: [PATCH 14/52] fix: use diagnostic terminology consistently --- wdl-lint/src/rules/shellcheck.rs | 34 +++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 4a2706fa2..56eff73e7 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -51,13 +51,16 @@ const SHELLCHECK_SUPPRESS: &[&str] = &[ /// ShellCheck: var is referenced by not assigned. const SHELLCHECK_REFERENCED_UNASSIGNED: usize = 2154; +/// ShellCheck wiki base url. +const SHELLCHECK_WIKI: &str = "https://www.shellcheck.net/wiki"; + /// Whether or not shellcheck exists on the system static SHELLCHECK_EXISTS: OnceLock = OnceLock::new(); /// The identifier for the command section ShellCheck rule. -const ID: &str = "CommandSectionShellCheck"; +const ID: &str = "ShellCheck"; -/// A ShellCheck comment. +/// A ShellCheck diagnostic. /// /// The file and fix fields are ommitted as we have no use for them. #[derive(Clone, Debug, Deserialize)] @@ -87,15 +90,15 @@ struct ShellCheckDiagnostic { fn run_shellcheck(command: &str) -> Result> { let mut sc_proc = process::Command::new(SHELLCHECK_BIN) .args([ - "-s", + "-s", // bash shell "bash", - "-f", + "-f", // output JSON "json", - "-e", + "-e", // errors to suppress &SHELLCHECK_SUPPRESS.join(","), - "-S", + "-S", // set minimum lint level to style "style", - "-", + "-", // input is piped to STDIN ]) .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -145,7 +148,7 @@ impl Rule for ShellCheckRule { } fn tags(&self) -> TagSet { - TagSet::new(&[Tag::Correctness, Tag::Style, Tag::Portability]) + TagSet::new(&[Tag::Correctness, Tag::Portability]) } fn exceptable_nodes(&self) -> Option<&'static [SyntaxKind]> { @@ -186,15 +189,22 @@ fn gather_task_declarations(task: &TaskDefinition) -> HashSet { decls } -/// Creates a "ShellCheck lint" diagnostic from a ShellCheckDiagnostic -fn shellcheck_lint(comment: &ShellCheckDiagnostic, span: Span) -> Diagnostic { +/// Creates a "ShellCheck lint" diagnostic from a `ShellCheckDiagnostic` +fn shellcheck_lint(diagnostic: &ShellCheckDiagnostic, span: Span) -> Diagnostic { Diagnostic::note("`shellcheck` reported the following diagnostic") .with_rule(ID) .with_label( - format!("SC{}[{}]: {}", comment.code, comment.level, comment.message), + format!( + "SC{}[{}]: {}", + diagnostic.code, diagnostic.level, diagnostic.message + ), + span, + ) + .with_label( + format!("more info: {}/SC{}", &SHELLCHECK_WIKI, diagnostic.code), span, ) - .with_fix("address the diagnostics as recommended in the message") + .with_fix("address the diagnostic as recommended in the message") } /// Sanitize a `CommandSection`. From a97fcadf3bcbb94f8363f79847d0f63e3e31717f Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 18:42:12 -0500 Subject: [PATCH 15/52] fix: correct diagnostic placement algorithm --- wdl-lint/src/rules/shellcheck.rs | 92 ++- .../lints/shellcheck-error/source.errors | 54 +- .../lints/shellcheck-style/source.errors | 36 +- .../tests/lints/shellcheck-warn/source.errors | 627 ++++++++++++++++-- .../tests/lints/shellcheck-warn/source.wdl | 80 ++- 5 files changed, 741 insertions(+), 148 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 56eff73e7..a5883f9de 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -237,23 +237,38 @@ fn sanitize_command(section: &CommandSection) -> Option<(String, HashSet } } -/// Returns the amount of leading whitespace characters in a `CommandSection`. -/// -/// Only checks the first `CommandPart::Text`. -fn count_command_whitespace(section: &CommandSection) -> usize { - if let Some(first_text) = section.parts().find(|p| matches!(p, CommandPart::Text(..))) { - match first_text { - CommandPart::Text(text) => { - let text_str = text - .as_str() - .strip_prefix("\n") - .unwrap_or_else(|| text.as_str()); - return count_leading_whitespace(text_str); +/// Maps each line as shellcheck sees it to its corresponding start position in the source. +fn map_shellcheck_lines(section: &CommandSection) -> HashMap { + let mut line_map = HashMap::new(); + let mut line_num = 1; + let mut skip_next_line = false; + for part in section.parts() { + match part { + CommandPart::Text(ref text) => { + for (line, line_start, _) in lines_with_offset(text.as_str()) { + // Add back leading whitespace that is stripped from the sanitized command. + // The first line is removed entirely, UNLESS there is content on it. + let leading_ws = if line_num > 1 || ! line.trim().is_empty() { + count_leading_whitespace(line) + } else { + continue; + }; + // this occurs after encountering a placeholder + if skip_next_line { + skip_next_line = false; + continue; + } + let adjusted_start = text.span().start() + line_start + leading_ws; + line_map.insert(line_num, adjusted_start); + line_num += 1; + } + } + CommandPart::Placeholder(_) => { + skip_next_line = true; } - CommandPart::Placeholder(_) => unreachable!(), } } - 0 + line_map } impl Visitor for ShellCheckRule { @@ -318,38 +333,7 @@ impl Visitor for ShellCheckRule { return; }; decls.extend(cmd_decls); - - // Get leading whitespace so we can add it to each span - let leading_whitespace = count_command_whitespace(section); - - // Map each actual line of the command to its corresponding - // `CommandPart` and start position. - let mut line_map = HashMap::new(); - let mut line_num = 1; - let mut on_same_line = false; - for part in section.parts() { - match part { - CommandPart::Text(ref text) => { - for (line, start, _) in lines_with_offset(text.as_str()) { - if line_num == 1 && line.trim().is_empty() { - continue; - } - if on_same_line { - on_same_line = false; - continue; - } - line_map.insert( - line_num, - text.span().start() + start + leading_whitespace - 1, - ); - line_num += 1; - } - } - CommandPart::Placeholder(_) => { - on_same_line = true; - } - } - } + let line_map = map_shellcheck_lines(section); match run_shellcheck(&sanitized_command) { Ok(diagnostics) => { @@ -367,12 +351,18 @@ impl Visitor for ShellCheckRule { let start = line_map .get(&diagnostic.line) .expect("shellcheck line corresponds to command line"); - let inner_span = { - Span::new( - start + diagnostic.column, - diagnostic.end_column - diagnostic.column, - ) + let len = if diagnostic.end_line > diagnostic.line { + // this is a multiline diagnostic + let end_line_start = line_map + .get(&diagnostic.end_line) + .expect("shellcheck line corresponds to command line"); + end_line_start + diagnostic.end_column + } else { + // single line diagnostic + (diagnostic.end_column).saturating_sub(diagnostic.column) }; + // shellcheck 1-indexes columns, so subtract 1. + let inner_span = { Span::new(start + diagnostic.column - 1, len) }; state.exceptable_add( shellcheck_lint(&diagnostic, inner_span), SyntaxElement::from(section.syntax().clone()), diff --git a/wdl-lint/tests/lints/shellcheck-error/source.errors b/wdl-lint/tests/lints/shellcheck-error/source.errors index 8da8ac0d3..7d8f06f55 100644 --- a/wdl-lint/tests/lints/shellcheck-error/source.errors +++ b/wdl-lint/tests/lints/shellcheck-error/source.errors @@ -1,48 +1,66 @@ -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-error/source.wdl:18:10 │ 18 │ if [ -f "$broken"] - │ ^ SC1073[error]: Couldn't parse this test expression. Fix to allow more checks. + │ ^ + │ │ + │ SC1073[error]: Couldn't parse this test expression. Fix to allow more checks. + │ more info: https://www.shellcheck.net/wiki/SC1073 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-error/source.wdl:18:15 │ 18 │ if [ -f "$broken"] - │ ^ SC1019[error]: Expected this to be an argument to the unary condition. + │ ^ + │ │ + │ SC1019[error]: Expected this to be an argument to the unary condition. + │ more info: https://www.shellcheck.net/wiki/SC1019 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-error/source.wdl:18:25 │ 18 │ if [ -f "$broken"] - │ ^ SC1020[error]: You need a space before the ]. + │ ^ + │ + │ SC1020[error]: You need a space before the ]. + │ more info: https://www.shellcheck.net/wiki/SC1020 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-error/source.wdl:37:10 │ 37 │ if [ -f "$broken"] - │ ^ SC1073[error]: Couldn't parse this test expression. Fix to allow more checks. + │ ^ + │ │ + │ SC1073[error]: Couldn't parse this test expression. Fix to allow more checks. + │ more info: https://www.shellcheck.net/wiki/SC1073 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-error/source.wdl:37:15 │ 37 │ if [ -f "$broken"] - │ ^ SC1019[error]: Expected this to be an argument to the unary condition. + │ ^ + │ │ + │ SC1019[error]: Expected this to be an argument to the unary condition. + │ more info: https://www.shellcheck.net/wiki/SC1019 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-error/source.wdl:37:25 │ 37 │ if [ -f "$broken"] - │ ^ SC1020[error]: You need a space before the ]. + │ ^ + │ + │ SC1020[error]: You need a space before the ]. + │ more info: https://www.shellcheck.net/wiki/SC1020 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message diff --git a/wdl-lint/tests/lints/shellcheck-style/source.errors b/wdl-lint/tests/lints/shellcheck-style/source.errors index 87b6b6ac1..637a0586b 100644 --- a/wdl-lint/tests/lints/shellcheck-style/source.errors +++ b/wdl-lint/tests/lints/shellcheck-style/source.errors @@ -1,32 +1,44 @@ -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-style/source.wdl:17:7 │ 17 │ [[ ]] - │ ^^^ SC2212[style]: Use 'false' instead of empty [/[[ conditionals. + │ ^^^ + │ │ + │ SC2212[style]: Use 'false' instead of empty [/[[ conditionals. + │ more info: https://www.shellcheck.net/wiki/SC2212 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-style/source.wdl:18:9 │ 18 │ [ true ] - │ ^^^^ SC2160[style]: Instead of '[ true ]', just use 'true'. + │ ^^^^ + │ │ + │ SC2160[style]: Instead of '[ true ]', just use 'true'. + │ more info: https://www.shellcheck.net/wiki/SC2160 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-style/source.wdl:36:7 │ 36 │ [[ ]] - │ ^^^ SC2212[style]: Use 'false' instead of empty [/[[ conditionals. + │ ^^^ + │ │ + │ SC2212[style]: Use 'false' instead of empty [/[[ conditionals. + │ more info: https://www.shellcheck.net/wiki/SC2212 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-style/source.wdl:37:9 │ 37 │ [ true ] - │ ^^^^ SC2160[style]: Instead of '[ true ]', just use 'true'. + │ ^^^^ + │ │ + │ SC2160[style]: Instead of '[ true ]', just use 'true'. + │ more info: https://www.shellcheck.net/wiki/SC2160 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.errors b/wdl-lint/tests/lints/shellcheck-warn/source.errors index 489f94715..aa76167ba 100644 --- a/wdl-lint/tests/lints/shellcheck-warn/source.errors +++ b/wdl-lint/tests/lints/shellcheck-warn/source.errors @@ -1,176 +1,671 @@ -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:17:22 │ 17 │ somecommand.py $line17 ~{placeholder} - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:17:22 │ 17 │ somecommand.py $line17 ~{placeholder} - │ ^^^^^^^ SC2154[warning]: line17 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line17 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:18:37 │ 18 │ somecommand.py ~{placeholder} $line18 - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:18:37 │ 18 │ somecommand.py ~{placeholder} $line18 - │ ^^^^^^^ SC2154[warning]: line18 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line18 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:19:36 │ 19 │ somecommand.py ~{placeholder}$line19 - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:19:36 │ 19 │ somecommand.py ~{placeholder}$line19 - │ ^^^^^^^ SC2154[warning]: line19 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line19 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:30:22 │ 30 │ somecommand.py $line30~{placeholder} - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:30:22 │ 30 │ somecommand.py $line30~{placeholder} - │ ^^^^^^^ SC2154[warning]: line30 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line30 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:31:27 │ 31 │ somecommand.py [ -f $line31 ] ~{placeholder} - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:31:27 │ 31 │ somecommand.py [ -f $line31 ] ~{placeholder} - │ ^^^^^^^ SC2154[warning]: line31 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line31 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:49:22 │ 49 │ somecommand.py $line49 ~{placeholder} - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:49:22 │ 49 │ somecommand.py $line49 ~{placeholder} - │ ^^^^^^^ SC2154[warning]: line49 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line49 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:50:37 │ 50 │ somecommand.py ~{placeholder} $line50 - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:50:37 │ 50 │ somecommand.py ~{placeholder} $line50 - │ ^^^^^^^ SC2154[warning]: line50 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line50 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:51:36 │ 51 │ somecommand.py ~{placeholder}$line51 - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:51:36 │ 51 │ somecommand.py ~{placeholder}$line51 - │ ^^^^^^^ SC2154[warning]: line51 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line51 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:52:22 │ 52 │ somecommand.py $line52~{placeholder} - │ ^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:52:22 │ 52 │ somecommand.py $line52~{placeholder} - │ ^^^^^^^ SC2154[warning]: line52 is referenced but not assigned. + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line52 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:53:27 │ 53 │ somecommand.py [ -f $bad_test ] ~{placeholder} - │ ^^^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:53:27 │ 53 │ somecommand.py [ -f $bad_test ] ~{placeholder} - │ ^^^^^^^^^ SC2154[warning]: bad_test is referenced but not assigned. + │ ^^^^^^^^^ + │ │ + │ SC2154[warning]: bad_test is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:54:27 │ 54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} - │ ^^^^^^^^^^^^^^^ SC2086[info]: Double quote to prevent globbing and word splitting. + │ ^^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message -note[CommandSectionShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: `shellcheck` reported the following diagnostic ┌─ tests/lints/shellcheck-warn/source.wdl:54:27 │ 54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} - │ ^^^^^^^^^^^^^^^ SC2154[warning]: trailing_space is referenced but not assigned. + │ ^^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: trailing_space is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ - = fix: address the diagnostics as recommended in the message + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:72:22 + │ +72 │ somecommand.py $line72 ~{placeholder} + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:72:22 + │ +72 │ somecommand.py $line72 ~{placeholder} + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line72 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:73:37 + │ +73 │ somecommand.py ~{placeholder} $line73 + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:73:37 + │ +73 │ somecommand.py ~{placeholder} $line73 + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line73 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:74:36 + │ +74 │ somecommand.py ~{placeholder}$line74 + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:74:36 + │ +74 │ somecommand.py ~{placeholder}$line74 + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line74 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:75:22 + │ +75 │ somecommand.py $line75~{placeholder} + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:75:22 + │ +75 │ somecommand.py $line75~{placeholder} + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line75 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:76:22 + │ +76 │ ~{placeholder} $line76_trailing_pholder ~{placeholder} + │ ^^^^^^^^^^^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:76:22 + │ +76 │ ~{placeholder} $line76_trailing_pholder ~{placeholder} + │ ^^^^^^^^^^^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: line76_trailing_pholder is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:77:37 + │ +77 │ ~{placeholder} somecommand.py $leading_pholder + │ ^^^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:77:37 + │ +77 │ ~{placeholder} somecommand.py $leading_pholder + │ ^^^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: leading_pholder is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:96:22 + │ +96 │ somecommand.py $line96 ~{placeholder} + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:96:22 + │ +96 │ somecommand.py $line96 ~{placeholder} + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line96 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:97:37 + │ +97 │ somecommand.py ~{placeholder} $line97 + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:97:37 + │ +97 │ somecommand.py ~{placeholder} $line97 + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line97 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:98:36 + │ +98 │ somecommand.py ~{placeholder}$line98 + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:98:36 + │ +98 │ somecommand.py ~{placeholder}$line98 + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line98 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:99:22 + │ +99 │ somecommand.py $line99~{placeholder} + │ ^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:99:22 + │ +99 │ somecommand.py $line99~{placeholder} + │ ^^^^^^^ + │ │ + │ SC2154[warning]: line99 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:100:22 + │ +100 │ ~{placeholder} $line100_trailing_pholder ~{placeholder} + │ ^^^^^^^^^^^^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:100:22 + │ +100 │ ~{placeholder} $line100_trailing_pholder ~{placeholder} + │ ^^^^^^^^^^^^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: line100_trailing_pholder is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:101:37 + │ +101 │ ~{placeholder} somecommand.py $leading_pholder + │ ^^^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:101:37 + │ +101 │ ~{placeholder} somecommand.py $leading_pholder + │ ^^^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: leading_pholder is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:118:34 + │ +118 │ command <<< weird stuff $firstlinelint + │ ^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:118:34 + │ +118 │ command <<< weird stuff $firstlinelint + │ ^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: firstlinelint is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:120:22 + │ +120 │ somecommand.py $line120 ~{placeholder} + │ ^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:120:22 + │ +120 │ somecommand.py $line120 ~{placeholder} + │ ^^^^^^^^ + │ │ + │ SC2154[warning]: line120 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:121:37 + │ +121 │ somecommand.py ~{placeholder} $line121 + │ ^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:121:37 + │ +121 │ somecommand.py ~{placeholder} $line121 + │ ^^^^^^^^ + │ │ + │ SC2154[warning]: line121 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:122:36 + │ +122 │ somecommand.py ~{placeholder}$line122 + │ ^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:122:36 + │ +122 │ somecommand.py ~{placeholder}$line122 + │ ^^^^^^^^ + │ │ + │ SC2154[warning]: line122 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:123:22 + │ +123 │ somecommand.py $line123~{placeholder} + │ ^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:123:22 + │ +123 │ somecommand.py $line123~{placeholder} + │ ^^^^^^^^ + │ │ + │ SC2154[warning]: line123 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:124:22 + │ +124 │ ~{placeholder} $line124_trailing_pholder ~{placeholder} + │ ^^^^^^^^^^^^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:124:22 + │ +124 │ ~{placeholder} $line124_trailing_pholder ~{placeholder} + │ ^^^^^^^^^^^^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: line124_trailing_pholder is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:126:37 + │ +126 │ ~{placeholder} somecommand.py $leading_pholder + │ ^^^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:126:37 + │ +126 │ ~{placeholder} somecommand.py $leading_pholder + │ ^^^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: leading_pholder is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:132:7 + │ +132 │ $occurs_after_multiline + │ ^^^^^^^^^^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: occurs_after_multiline is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.wdl b/wdl-lint/tests/lints/shellcheck-warn/source.wdl index bc863b26d..1e20c1856 100644 --- a/wdl-lint/tests/lints/shellcheck-warn/source.wdl +++ b/wdl-lint/tests/lints/shellcheck-warn/source.wdl @@ -45,13 +45,91 @@ task test2 { Int placeholder } - command <<< + command { somecommand.py $line49 ~{placeholder} somecommand.py ~{placeholder} $line50 somecommand.py ~{placeholder}$line51 somecommand.py $line52~{placeholder} somecommand.py [ -f $bad_test ] ~{placeholder} somecommand.py [ -f $trailing_space ] ~{placeholder} + } + + output {} + + runtime {} +} + +task test3 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command <<< # weird whitespace + somecommand.py $line72 ~{placeholder} + somecommand.py ~{placeholder} $line73 + somecommand.py ~{placeholder}$line74 + somecommand.py $line75~{placeholder} + ~{placeholder} $line76_trailing_pholder ~{placeholder} + ~{placeholder} somecommand.py $leading_pholder + >>> + + output {} + + runtime {} +} + +task test4 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command <<< + # other weird whitspace + somecommand.py $line96 ~{placeholder} + somecommand.py ~{placeholder} $line97 + somecommand.py ~{placeholder}$line98 + somecommand.py $line99~{placeholder} + ~{placeholder} $line100_trailing_pholder ~{placeholder} + ~{placeholder} somecommand.py $leading_pholder + >>> + + output {} + + runtime {} +} + +task test5 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command <<< weird stuff $firstlinelint + # other weird whitespace + somecommand.py $line120 ~{placeholder} + somecommand.py ~{placeholder} $line121 + somecommand.py ~{placeholder}$line122 + somecommand.py $line123~{placeholder} + ~{placeholder} $line124_trailing_pholder ~{placeholder} + ~{by + myself} + ~{placeholder} somecommand.py $leading_pholder + + ~{ + multiline + + placeholder + } + $occurs_after_multiline >>> output {} From 405afff522b636900644a688fd82771649654d2a Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 19:10:14 -0500 Subject: [PATCH 16/52] fix: disable missing shellcheck diagnostic --- wdl-lint/src/rules/shellcheck.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index a5883f9de..086e37601 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -301,19 +301,23 @@ impl Visitor for ShellCheckRule { if !SHELLCHECK_EXISTS.get_or_init(|| { if !program_exists(SHELLCHECK_BIN) { - let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) - .expect("should have a command keyword token"); - state.exceptable_add( - Diagnostic::error("running `shellcheck` on command section") - .with_label( - "could not find `shellcheck` executable.", - command_keyword.text_range().to_span(), - ) - .with_rule(ID) - .with_fix("install shellcheck or disable this lint."), - SyntaxElement::from(section.syntax().clone()), - &self.exceptable_nodes(), - ); + // TODO this diagnostic is currently disabled. + // When support for opt-in lints is added, this diagnostic + // should be enabled. + // + // let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) + // .expect("should have a command keyword token"); + // state.exceptable_add( + // Diagnostic::error("running `shellcheck` on command section") + // .with_label( + // "could not find `shellcheck` executable.", + // command_keyword.text_range().to_span(), + // ) + // .with_rule(ID) + // .with_fix("install shellcheck or disable this lint."), + // SyntaxElement::from(section.syntax().clone()), + // &self.exceptable_nodes(), + // ); return false; } true From ed879c915ea000078e34ab43fc70b97f859d0cbd Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 19:32:59 -0500 Subject: [PATCH 17/52] feat: add debug log for shellcheck process id --- wdl-lint/Cargo.toml | 1 + wdl-lint/src/rules/shellcheck.rs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/wdl-lint/Cargo.toml b/wdl-lint/Cargo.toml index 438af688b..cc1d12119 100644 --- a/wdl-lint/Cargo.toml +++ b/wdl-lint/Cargo.toml @@ -20,6 +20,7 @@ rand = { workspace = true } rowan = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +tracing = { workspace = true } [dev-dependencies] codespan-reporting = { workspace = true } diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 086e37601..36f5a6cbe 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -13,6 +13,7 @@ use rand::distributions::Alphanumeric; use rand::distributions::DistString; use serde::Deserialize; use serde_json; +use tracing::debug; use wdl_ast::AstNode; use wdl_ast::AstToken; @@ -104,6 +105,7 @@ fn run_shellcheck(command: &str) -> Result> { .stdout(Stdio::piped()) .spawn() .context("spawning the `shellcheck` process")?; + debug!("`shellcheck` process id: {}", sc_proc.id()); { let mut proc_stdin = sc_proc .stdin From d7382e4f9225840dce1b1728e5439599a44bd16a Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 19:35:19 -0500 Subject: [PATCH 18/52] feat: clarify suppressed diagnostics --- wdl-lint/src/rules/shellcheck.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 36f5a6cbe..4906cb0b1 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -43,10 +43,11 @@ use crate::util::{count_leading_whitespace, lines_with_offset, program_exists}; const SHELLCHECK_BIN: &str = "shellcheck"; /// shellcheck lints that we want to suppress +/// these two lints always co-occur with a more +/// informative message. const SHELLCHECK_SUPPRESS: &[&str] = &[ - "1009", // the mentioned parser error was in... - "1072", // Unexpected - "1083", // this {/} is literal + "1009", // the mentioned parser error was in... (unhelpful commentary) + "1072", // Unexpected eof (unhelpful commentary) ]; /// ShellCheck: var is referenced by not assigned. From ce77d8ae34b3dff4c29c4854ce4b4272e28f94bc Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 21:19:32 -0500 Subject: [PATCH 19/52] fix: remove task definition from shellcheck exceptable nodes --- wdl-lint/src/rules/shellcheck.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 4906cb0b1..86009fadb 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -157,7 +157,6 @@ impl Rule for ShellCheckRule { fn exceptable_nodes(&self) -> Option<&'static [SyntaxKind]> { Some(&[ SyntaxKind::VersionStatementNode, - SyntaxKind::TaskDefinitionNode, SyntaxKind::CommandSectionNode, ]) } From 282a52c8d8e4331e3adaff3bb68bb37397b49600 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 21:22:17 -0500 Subject: [PATCH 20/52] fix: multiline diagnostic placement --- wdl-lint/src/rules/shellcheck.rs | 44 ++++++++++++------- .../tests/lints/shellcheck-warn/source.errors | 33 ++++++++++++++ .../tests/lints/shellcheck-warn/source.wdl | 12 +++++ 3 files changed, 72 insertions(+), 17 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 86009fadb..0cd048de2 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -250,7 +250,7 @@ fn map_shellcheck_lines(section: &CommandSection) -> HashMap { for (line, line_start, _) in lines_with_offset(text.as_str()) { // Add back leading whitespace that is stripped from the sanitized command. // The first line is removed entirely, UNLESS there is content on it. - let leading_ws = if line_num > 1 || ! line.trim().is_empty() { + let leading_ws = if line_num > 1 || !line.trim().is_empty() { count_leading_whitespace(line) } else { continue; @@ -273,6 +273,30 @@ fn map_shellcheck_lines(section: &CommandSection) -> HashMap { line_map } +/// Calculates the correct `Span` for a `ShellCheckDiagnostic` relative to the source. +fn calculate_span(diagnostic: &ShellCheckDiagnostic, line_map: &HashMap) -> Span { + // shellcheck 1-indexes columns, so subtract 1. + let start = line_map + .get(&diagnostic.line) + .expect("shellcheck line corresponds to command line") + + diagnostic.column + - 1; + let len = if diagnostic.end_line > diagnostic.line { + // this is a multiline diagnostic + let end_line_end = line_map + .get(&diagnostic.end_line) + .expect("shellcheck line corresponds to command line") + + diagnostic.end_column + - 1; + // - 2 to discount first and last newlines + end_line_end.saturating_sub(start) - 2 + } else { + // single line diagnostic + (diagnostic.end_column).saturating_sub(diagnostic.column) + }; + Span::new(start, len) +} + impl Visitor for ShellCheckRule { type State = Diagnostics; @@ -354,23 +378,9 @@ impl Visitor for ShellCheckRule { { continue; } - let start = line_map - .get(&diagnostic.line) - .expect("shellcheck line corresponds to command line"); - let len = if diagnostic.end_line > diagnostic.line { - // this is a multiline diagnostic - let end_line_start = line_map - .get(&diagnostic.end_line) - .expect("shellcheck line corresponds to command line"); - end_line_start + diagnostic.end_column - } else { - // single line diagnostic - (diagnostic.end_column).saturating_sub(diagnostic.column) - }; - // shellcheck 1-indexes columns, so subtract 1. - let inner_span = { Span::new(start + diagnostic.column - 1, len) }; + let span = calculate_span(&diagnostic, &line_map); state.exceptable_add( - shellcheck_lint(&diagnostic, inner_span), + shellcheck_lint(&diagnostic, span), SyntaxElement::from(section.syntax().clone()), &self.exceptable_nodes(), ) diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.errors b/wdl-lint/tests/lints/shellcheck-warn/source.errors index aa76167ba..5082a6c66 100644 --- a/wdl-lint/tests/lints/shellcheck-warn/source.errors +++ b/wdl-lint/tests/lints/shellcheck-warn/source.errors @@ -669,3 +669,36 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:134:7 + │ +134 │ ╭ ╭ $(echo This is a +135 │ │ │ very long string that should be quoted) + │ ╰─│───────────────────────────────────────────────^ SC2091[warning]: Remove surrounding $() to avoid executing output (or use eval if intentional). + │ ╰───────────────────────────────────────────────' more info: https://www.shellcheck.net/wiki/SC2091 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:137:7 + │ +137 │ ╭ ╭ $(echo This is an +138 │ │ │ even longer very long string that should really +139 │ │ │ be quoted) + │ ╰─│──────────────────^ SC2091[warning]: Remove surrounding $() to avoid executing output (or use eval if intentional). + │ ╰──────────────────' more info: https://www.shellcheck.net/wiki/SC2091 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: `shellcheck` reported the following diagnostic + ┌─ tests/lints/shellcheck-warn/source.wdl:141:7 + │ +141 │ ╭ ╭ $(echo This is an +142 │ │ │ even longer very long string that should really +143 │ │ │ really really really +144 │ │ │ ought to be quoted) + │ ╰─│───────────────────────────^ SC2091[warning]: Remove surrounding $() to avoid executing output (or use eval if intentional). + │ ╰───────────────────────────' more info: https://www.shellcheck.net/wiki/SC2091 + │ + = fix: address the diagnostic as recommended in the message + diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.wdl b/wdl-lint/tests/lints/shellcheck-warn/source.wdl index 1e20c1856..c20e7790e 100644 --- a/wdl-lint/tests/lints/shellcheck-warn/source.wdl +++ b/wdl-lint/tests/lints/shellcheck-warn/source.wdl @@ -130,6 +130,18 @@ task test5 { placeholder } $occurs_after_multiline + + $(echo This is a + very long string that should be quoted) + + $(echo This is an + even longer very long string that should really + be quoted) + + $(echo This is an + even longer very long string that should really + really really really + ought to be quoted) >>> output {} From 007592c89d1417dba1d8ad330f7d41c7c8ecf807 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 21:22:40 -0500 Subject: [PATCH 21/52] fix: clarify language for disabled diangostic --- wdl-lint/src/rules/shellcheck.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 0cd048de2..1fc345052 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -334,13 +334,13 @@ impl Visitor for ShellCheckRule { // let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) // .expect("should have a command keyword token"); // state.exceptable_add( - // Diagnostic::error("running `shellcheck` on command section") + // Diagnostic::note("running `shellcheck` on command section") // .with_label( // "could not find `shellcheck` executable.", // command_keyword.text_range().to_span(), // ) // .with_rule(ID) - // .with_fix("install shellcheck or disable this lint."), + // .with_fix("install shellcheck (https://www.shellcheck.net) or disable this lint."), // SyntaxElement::from(section.syntax().clone()), // &self.exceptable_nodes(), // ); From ce1e69e09b6a19d1e5382918473a3bd23e78c839 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 11 Dec 2024 21:33:14 -0500 Subject: [PATCH 22/52] feat: update RULES.md and CHANGELOG.md --- wdl-lint/CHANGELOG.md | 1 + wdl-lint/RULES.md | 1 + 2 files changed, 2 insertions(+) diff --git a/wdl-lint/CHANGELOG.md b/wdl-lint/CHANGELOG.md index 84af96752..ad59bf692 100644 --- a/wdl-lint/CHANGELOG.md +++ b/wdl-lint/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Added +* Added a `ShellCheck` rule ([#264](https://github.com/stjude-rust-labs/wdl/pull/264)). * Added a `RedundantInputAssignment` rule ([#244](https://github.com/stjude-rust-labs/wdl/pull/244)). ## Changed diff --git a/wdl-lint/RULES.md b/wdl-lint/RULES.md index adaa862cd..beae8a000 100644 --- a/wdl-lint/RULES.md +++ b/wdl-lint/RULES.md @@ -43,6 +43,7 @@ be out of sync with released packages. | `RuntimeSectionKeys` | Completeness, Deprecated | Ensures that runtime sections have the appropriate keys. | | `RedundantInputAssignment` | Style | Ensures that redundant input assignments are shortened | | `SectionOrdering` | Sorting, Style | Ensures that sections within tasks and workflows are sorted. | +| `ShellCheck` | Correctness, Portability |Ensures that command sections are free of [shellcheck](https://www.shellcheck.net) diagnostics | | `SnakeCase` | Clarity, Naming, Style | Ensures that tasks, workflows, and variables are defined with snake_case names. | | `Todo` | Completeness | Ensures that `TODO` statements are flagged for followup. | | `TrailingComma` | Style | Ensures that lists and objects in meta have a trailing comma. | From c9fe0442e14e585a670686b37615e48c3d001073 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Fri, 13 Dec 2024 09:50:08 -0500 Subject: [PATCH 23/52] fix: apply formatting rules --- wdl-lint/src/rules/shellcheck.rs | 33 +++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 1fc345052..ea56426d2 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -14,7 +14,6 @@ use rand::distributions::DistString; use serde::Deserialize; use serde_json; use tracing::debug; - use wdl_ast::AstNode; use wdl_ast::AstToken; use wdl_ast::Diagnostic; @@ -37,7 +36,10 @@ use wdl_ast::v1::TaskDefinition; use crate::Rule; use crate::Tag; use crate::TagSet; -use crate::util::{count_leading_whitespace, lines_with_offset, program_exists}; +use crate::util::count_leading_whitespace; +use crate::util::is_properly_quoted; +use crate::util::lines_with_offset; +use crate::util::program_exists; /// The shellcheck executable const SHELLCHECK_BIN: &str = "shellcheck"; @@ -145,9 +147,8 @@ impl Rule for ShellCheckRule { fn explanation(&self) -> &'static str { "ShellCheck (https://shellcheck.net) is a static analysis tool and linter for sh / bash. \ - The lints provided by ShellCheck help prevent common errors and \ - pitfalls in your scripts. Following its recommendations will increase \ - the robustness of your command sections." + The lints provided by ShellCheck help prevent common errors and pitfalls in your scripts. \ + Following its recommendations will increase the robustness of your command sections." } fn tags(&self) -> TagSet { @@ -239,7 +240,8 @@ fn sanitize_command(section: &CommandSection) -> Option<(String, HashSet } } -/// Maps each line as shellcheck sees it to its corresponding start position in the source. +/// Maps each line as shellcheck sees it to its corresponding start position in +/// the source. fn map_shellcheck_lines(section: &CommandSection) -> HashMap { let mut line_map = HashMap::new(); let mut line_num = 1; @@ -248,6 +250,11 @@ fn map_shellcheck_lines(section: &CommandSection) -> HashMap { match part { CommandPart::Text(ref text) => { for (line, line_start, _) in lines_with_offset(text.as_str()) { + // this occurs after encountering a placeholder + if skip_next_line { + skip_next_line = false; + continue; + } // Add back leading whitespace that is stripped from the sanitized command. // The first line is removed entirely, UNLESS there is content on it. let leading_ws = if line_num > 1 || !line.trim().is_empty() { @@ -255,11 +262,6 @@ fn map_shellcheck_lines(section: &CommandSection) -> HashMap { } else { continue; }; - // this occurs after encountering a placeholder - if skip_next_line { - skip_next_line = false; - continue; - } let adjusted_start = text.span().start() + line_start + leading_ws; line_map.insert(line_num, adjusted_start); line_num += 1; @@ -273,7 +275,8 @@ fn map_shellcheck_lines(section: &CommandSection) -> HashMap { line_map } -/// Calculates the correct `Span` for a `ShellCheckDiagnostic` relative to the source. +/// Calculates the correct `Span` for a `ShellCheckDiagnostic` relative to the +/// source. fn calculate_span(diagnostic: &ShellCheckDiagnostic, line_map: &HashMap) -> Span { // shellcheck 1-indexes columns, so subtract 1. let start = line_map @@ -331,9 +334,9 @@ impl Visitor for ShellCheckRule { // When support for opt-in lints is added, this diagnostic // should be enabled. // - // let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) - // .expect("should have a command keyword token"); - // state.exceptable_add( + // let command_keyword = support::token(section.syntax(), + // SyntaxKind::CommandKeyword) .expect("should have a + // command keyword token"); state.exceptable_add( // Diagnostic::note("running `shellcheck` on command section") // .with_label( // "could not find `shellcheck` executable.", From d41908561c1bbbf5c3870b40bd8b00bb439e52da Mon Sep 17 00:00:00 2001 From: thatrichman Date: Fri, 13 Dec 2024 09:51:02 -0500 Subject: [PATCH 24/52] fix: do not overquote bash vars --- wdl-lint/src/rules/shellcheck.rs | 17 ++++++++++++++-- wdl-lint/src/util.rs | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index ea56426d2..a2bdc228b 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -219,18 +219,31 @@ fn shellcheck_lint(diagnostic: &ShellCheckDiagnostic, span: Span) -> Diagnostic fn sanitize_command(section: &CommandSection) -> Option<(String, HashSet)> { let mut sanitized_command = String::new(); let mut decls = HashSet::new(); + let mut needs_quotes = true; if let Some(cmd_parts) = section.strip_whitespace() { cmd_parts.iter().for_each(|part| match part { StrippedCommandPart::Text(text) => { sanitized_command.push_str(text); + // if this text section is not properly is not properly quoted + // then the next placeholder does *not* need double quotes + // because it will end up enclosed. + needs_quotes ^= !is_properly_quoted(text, '"'); } StrippedCommandPart::Placeholder(placeholder) => { let bash_var = to_bash_var(placeholder); // we need to save the var so we can suppress later decls.insert(bash_var.clone()); - let mut expansion = String::from("\"$"); + let mut expansion = String::from("$"); expansion.push_str(&bash_var); - expansion.push('"'); + if needs_quotes { + // surround with quotes + expansion.insert(0, '"'); + expansion.push('"'); + } else { + // surround with {} + expansion.insert(1, '{'); + expansion.push('}'); + } sanitized_command.push_str(&expansion); } }); diff --git a/wdl-lint/src/util.rs b/wdl-lint/src/util.rs index c14da4d4e..ee529ba8d 100644 --- a/wdl-lint/src/util.rs +++ b/wdl-lint/src/util.rs @@ -34,6 +34,22 @@ pub fn is_inline_comment(token: &Comment) -> bool { false } +/// Determines if a string containing embedded quotes is properly quoted. +pub fn is_properly_quoted(s: &str, quote_char: char) -> bool { + let mut closed = true; + let mut escaped = false; + s.chars().for_each(|c| { + if c == '\\' { + escaped = true; + } else if !escaped && c == quote_char { + closed = !closed; + } else { + escaped = false; + } + }); + closed +} + /// Iterates over the lines of a string and returns the line, starting offset, /// and next possible starting offset. pub fn lines_with_offset(s: &str) -> impl Iterator { @@ -193,4 +209,22 @@ task foo { # an in-line comment assert!(program_exists("which")); } } + + #[test] + fn test_is_properly_quoted() { + let s = "\"this string is quoted properly.\""; + assert!(is_properly_quoted(s, '"')); + let s = "\"this string has an escaped \\\" quote.\""; + assert!(is_properly_quoted(s, '"')); + let s = "\"this string is missing an end quote"; + assert_eq!(is_properly_quoted(s, '"'), false); + let s = "this string is missing an open quote\""; + assert_eq!(is_properly_quoted(s, '"'), false); + let s = "\"this string has an irrelevant escape \\ \""; + assert!(is_properly_quoted(s, '"')); + let s = "'this string has single quotes'"; + assert!(is_properly_quoted(s, '\'')); + let s = "this string has unclosed single quotes'"; + assert_eq!(is_properly_quoted(s, '\''), false); + } } From 9bcd09df6b1ed247d431ee998b168715eff96968 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Fri, 13 Dec 2024 09:51:46 -0500 Subject: [PATCH 25/52] fix: apply formatting rules --- wdl-lint/src/rules.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wdl-lint/src/rules.rs b/wdl-lint/src/rules.rs index e7ae6c59a..564f4aef4 100644 --- a/wdl-lint/src/rules.rs +++ b/wdl-lint/src/rules.rs @@ -3,7 +3,6 @@ mod blank_lines_between_elements; mod call_input_spacing; mod command_mixed_indentation; -mod shellcheck; mod comment_whitespace; mod container_value; mod deprecated_object; @@ -36,6 +35,7 @@ mod preamble_formatting; mod redundant_input_assignment; mod runtime_section_keys; mod section_order; +mod shellcheck; mod snake_case; mod todo; mod trailing_comma; @@ -46,7 +46,6 @@ mod whitespace; pub use blank_lines_between_elements::*; pub use call_input_spacing::*; pub use command_mixed_indentation::*; -pub use shellcheck::*; pub use comment_whitespace::*; pub use container_value::*; pub use deprecated_object::*; @@ -79,6 +78,7 @@ pub use preamble_formatting::*; pub use redundant_input_assignment::*; pub use runtime_section_keys::*; pub use section_order::*; +pub use shellcheck::*; pub use snake_case::*; pub use todo::*; pub use trailing_comma::*; From 4215c5da6ce4e728b8dcbebe1b78fc6d30e68900 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Fri, 13 Dec 2024 09:55:39 -0500 Subject: [PATCH 26/52] fix: diagnostic comments must be unique --- wdl-lint/src/rules/shellcheck.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index a2bdc228b..ba0ff9556 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -194,15 +194,13 @@ fn gather_task_declarations(task: &TaskDefinition) -> HashSet { /// Creates a "ShellCheck lint" diagnostic from a `ShellCheckDiagnostic` fn shellcheck_lint(diagnostic: &ShellCheckDiagnostic, span: Span) -> Diagnostic { - Diagnostic::note("`shellcheck` reported the following diagnostic") + let label = format!( + "SC{}[{}]: {}", + diagnostic.code, diagnostic.level, diagnostic.message + ); + Diagnostic::note(&diagnostic.message) .with_rule(ID) - .with_label( - format!( - "SC{}[{}]: {}", - diagnostic.code, diagnostic.level, diagnostic.message - ), - span, - ) + .with_label(label, span) .with_label( format!("more info: {}/SC{}", &SHELLCHECK_WIKI, diagnostic.code), span, From 23dfe5abfd6ed8ed54c05362cd82ac03092a2ae2 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Thu, 12 Dec 2024 22:19:36 -0500 Subject: [PATCH 27/52] feat: add install-shellcheck action --- .../actions/install-shellcheck/action.yaml | 21 +++++++++++++++++++ .github/workflows/CI.yml | 6 ++++++ 2 files changed, 27 insertions(+) create mode 100644 .github/actions/install-shellcheck/action.yaml diff --git a/.github/actions/install-shellcheck/action.yaml b/.github/actions/install-shellcheck/action.yaml new file mode 100644 index 000000000..fad644b6b --- /dev/null +++ b/.github/actions/install-shellcheck/action.yaml @@ -0,0 +1,21 @@ +name: Install ShellCheck +description: "Installs ShellCheck. Supports Ubuntu, macOS, and Windows." +runs: + using: "composite" + steps: + - uses: awalsh128/cache-apt-pkgs-action@latest + if: runner.os == 'Linux' + with: + packages: shellcheck + version: 1.0 + - name: install shellcheck -- macOS + shell: bash + if: runner.os == 'macOS' + run: brew install shellcheck + - name: install shellcheck -- Windows + if: runner.os == 'Windows' + shell: powershell + run: | + gh release download -R koalaman/shellcheck -p 'shellcheck-v*.zip' -O shellcheck-latest.zip + 7z x shellcheck-latest.zip + echo $(Get-Location).Path >> "$GITHUB_PATH" diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 1f01e4b23..1474743a2 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -5,6 +5,10 @@ on: branches: - main pull_request: + workflow_dispatch: + +env: + GH_TOKEN: ${{ github.token }} jobs: format: @@ -60,6 +64,7 @@ jobs: os: [windows-latest, macos-latest, ubuntu-latest] steps: - uses: actions/checkout@v4 + - uses: ./.github/actions/install-shellcheck - run: git config --global core.autocrlf false - name: Update Rust run: rustup update stable && rustup default stable @@ -73,6 +78,7 @@ jobs: os: [windows-latest, macos-latest, ubuntu-latest] steps: - uses: actions/checkout@v4 + - uses: ./.github/actions/install-shellcheck - run: git config --global core.autocrlf false - name: Update Rust run: rustup update stable && rustup default stable From e948c81cc5ba8aa548b5d298cc68eb85991ad192 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Fri, 13 Dec 2024 16:33:54 -0500 Subject: [PATCH 28/52] fix: update source.errors with new lint format --- .../lints/shellcheck-error/source.errors | 12 +- .../lints/shellcheck-style/source.errors | 8 +- .../tests/lints/shellcheck-warn/source.errors | 148 +++++++++--------- 3 files changed, 84 insertions(+), 84 deletions(-) diff --git a/wdl-lint/tests/lints/shellcheck-error/source.errors b/wdl-lint/tests/lints/shellcheck-error/source.errors index 7d8f06f55..c5471c0cd 100644 --- a/wdl-lint/tests/lints/shellcheck-error/source.errors +++ b/wdl-lint/tests/lints/shellcheck-error/source.errors @@ -1,4 +1,4 @@ -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Couldn't parse this test expression. Fix to allow more checks. ┌─ tests/lints/shellcheck-error/source.wdl:18:10 │ 18 │ if [ -f "$broken"] @@ -9,7 +9,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Expected this to be an argument to the unary condition. ┌─ tests/lints/shellcheck-error/source.wdl:18:15 │ 18 │ if [ -f "$broken"] @@ -20,7 +20,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: You need a space before the ]. ┌─ tests/lints/shellcheck-error/source.wdl:18:25 │ 18 │ if [ -f "$broken"] @@ -31,7 +31,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Couldn't parse this test expression. Fix to allow more checks. ┌─ tests/lints/shellcheck-error/source.wdl:37:10 │ 37 │ if [ -f "$broken"] @@ -42,7 +42,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Expected this to be an argument to the unary condition. ┌─ tests/lints/shellcheck-error/source.wdl:37:15 │ 37 │ if [ -f "$broken"] @@ -53,7 +53,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: You need a space before the ]. ┌─ tests/lints/shellcheck-error/source.wdl:37:25 │ 37 │ if [ -f "$broken"] diff --git a/wdl-lint/tests/lints/shellcheck-style/source.errors b/wdl-lint/tests/lints/shellcheck-style/source.errors index 637a0586b..bc53fec1d 100644 --- a/wdl-lint/tests/lints/shellcheck-style/source.errors +++ b/wdl-lint/tests/lints/shellcheck-style/source.errors @@ -1,4 +1,4 @@ -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Use 'false' instead of empty [/[[ conditionals. ┌─ tests/lints/shellcheck-style/source.wdl:17:7 │ 17 │ [[ ]] @@ -9,7 +9,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Instead of '[ true ]', just use 'true'. ┌─ tests/lints/shellcheck-style/source.wdl:18:9 │ 18 │ [ true ] @@ -20,7 +20,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Use 'false' instead of empty [/[[ conditionals. ┌─ tests/lints/shellcheck-style/source.wdl:36:7 │ 36 │ [[ ]] @@ -31,7 +31,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Instead of '[ true ]', just use 'true'. ┌─ tests/lints/shellcheck-style/source.wdl:37:9 │ 37 │ [ true ] diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.errors b/wdl-lint/tests/lints/shellcheck-warn/source.errors index 5082a6c66..d38de1852 100644 --- a/wdl-lint/tests/lints/shellcheck-warn/source.errors +++ b/wdl-lint/tests/lints/shellcheck-warn/source.errors @@ -1,4 +1,4 @@ -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:17:22 │ 17 │ somecommand.py $line17 ~{placeholder} @@ -9,7 +9,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line17 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:17:22 │ 17 │ somecommand.py $line17 ~{placeholder} @@ -20,7 +20,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:18:37 │ 18 │ somecommand.py ~{placeholder} $line18 @@ -31,7 +31,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line18 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:18:37 │ 18 │ somecommand.py ~{placeholder} $line18 @@ -42,7 +42,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:19:36 │ 19 │ somecommand.py ~{placeholder}$line19 @@ -53,7 +53,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line19 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:19:36 │ 19 │ somecommand.py ~{placeholder}$line19 @@ -64,7 +64,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:30:22 │ 30 │ somecommand.py $line30~{placeholder} @@ -75,7 +75,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line30 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:30:22 │ 30 │ somecommand.py $line30~{placeholder} @@ -86,7 +86,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:31:27 │ 31 │ somecommand.py [ -f $line31 ] ~{placeholder} @@ -97,7 +97,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line31 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:31:27 │ 31 │ somecommand.py [ -f $line31 ] ~{placeholder} @@ -108,7 +108,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:49:22 │ 49 │ somecommand.py $line49 ~{placeholder} @@ -119,7 +119,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line49 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:49:22 │ 49 │ somecommand.py $line49 ~{placeholder} @@ -130,7 +130,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:50:37 │ 50 │ somecommand.py ~{placeholder} $line50 @@ -141,7 +141,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line50 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:50:37 │ 50 │ somecommand.py ~{placeholder} $line50 @@ -152,7 +152,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:51:36 │ 51 │ somecommand.py ~{placeholder}$line51 @@ -163,7 +163,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line51 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:51:36 │ 51 │ somecommand.py ~{placeholder}$line51 @@ -174,7 +174,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:52:22 │ 52 │ somecommand.py $line52~{placeholder} @@ -185,7 +185,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line52 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:52:22 │ 52 │ somecommand.py $line52~{placeholder} @@ -196,7 +196,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:53:27 │ 53 │ somecommand.py [ -f $bad_test ] ~{placeholder} @@ -207,7 +207,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: bad_test is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:53:27 │ 53 │ somecommand.py [ -f $bad_test ] ~{placeholder} @@ -218,7 +218,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:54:27 │ 54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} @@ -229,7 +229,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: trailing_space is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:54:27 │ 54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} @@ -240,7 +240,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:72:22 │ 72 │ somecommand.py $line72 ~{placeholder} @@ -251,7 +251,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line72 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:72:22 │ 72 │ somecommand.py $line72 ~{placeholder} @@ -262,7 +262,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:73:37 │ 73 │ somecommand.py ~{placeholder} $line73 @@ -273,7 +273,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line73 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:73:37 │ 73 │ somecommand.py ~{placeholder} $line73 @@ -284,7 +284,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:74:36 │ 74 │ somecommand.py ~{placeholder}$line74 @@ -295,7 +295,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line74 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:74:36 │ 74 │ somecommand.py ~{placeholder}$line74 @@ -306,7 +306,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:75:22 │ 75 │ somecommand.py $line75~{placeholder} @@ -317,7 +317,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line75 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:75:22 │ 75 │ somecommand.py $line75~{placeholder} @@ -328,7 +328,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:76:22 │ 76 │ ~{placeholder} $line76_trailing_pholder ~{placeholder} @@ -339,7 +339,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line76_trailing_pholder is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:76:22 │ 76 │ ~{placeholder} $line76_trailing_pholder ~{placeholder} @@ -350,7 +350,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:77:37 │ 77 │ ~{placeholder} somecommand.py $leading_pholder @@ -361,7 +361,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: leading_pholder is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:77:37 │ 77 │ ~{placeholder} somecommand.py $leading_pholder @@ -372,7 +372,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:96:22 │ 96 │ somecommand.py $line96 ~{placeholder} @@ -383,7 +383,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line96 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:96:22 │ 96 │ somecommand.py $line96 ~{placeholder} @@ -394,7 +394,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:97:37 │ 97 │ somecommand.py ~{placeholder} $line97 @@ -405,7 +405,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line97 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:97:37 │ 97 │ somecommand.py ~{placeholder} $line97 @@ -416,7 +416,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:98:36 │ 98 │ somecommand.py ~{placeholder}$line98 @@ -427,7 +427,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line98 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:98:36 │ 98 │ somecommand.py ~{placeholder}$line98 @@ -438,7 +438,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:99:22 │ 99 │ somecommand.py $line99~{placeholder} @@ -449,7 +449,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line99 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:99:22 │ 99 │ somecommand.py $line99~{placeholder} @@ -460,7 +460,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:100:22 │ 100 │ ~{placeholder} $line100_trailing_pholder ~{placeholder} @@ -471,7 +471,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line100_trailing_pholder is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:100:22 │ 100 │ ~{placeholder} $line100_trailing_pholder ~{placeholder} @@ -482,7 +482,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:101:37 │ 101 │ ~{placeholder} somecommand.py $leading_pholder @@ -493,7 +493,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: leading_pholder is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:101:37 │ 101 │ ~{placeholder} somecommand.py $leading_pholder @@ -504,29 +504,29 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic - ┌─ tests/lints/shellcheck-warn/source.wdl:118:34 +note[ShellCheck]: Double quote to prevent globbing and word splitting. + ┌─ tests/lints/shellcheck-warn/source.wdl:118:28 │ 118 │ command <<< weird stuff $firstlinelint - │ ^^^^^^^^^^^^^^ - │ │ - │ SC2086[info]: Double quote to prevent globbing and word splitting. - │ more info: https://www.shellcheck.net/wiki/SC2086 + │ ^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic - ┌─ tests/lints/shellcheck-warn/source.wdl:118:34 +note[ShellCheck]: firstlinelint is referenced but not assigned. + ┌─ tests/lints/shellcheck-warn/source.wdl:118:28 │ 118 │ command <<< weird stuff $firstlinelint - │ ^^^^^^^^^^^^^^ - │ │ - │ SC2154[warning]: firstlinelint is referenced but not assigned. - │ more info: https://www.shellcheck.net/wiki/SC2154 + │ ^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: firstlinelint is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:120:22 │ 120 │ somecommand.py $line120 ~{placeholder} @@ -537,7 +537,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line120 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:120:22 │ 120 │ somecommand.py $line120 ~{placeholder} @@ -548,7 +548,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:121:37 │ 121 │ somecommand.py ~{placeholder} $line121 @@ -559,7 +559,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line121 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:121:37 │ 121 │ somecommand.py ~{placeholder} $line121 @@ -570,7 +570,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:122:36 │ 122 │ somecommand.py ~{placeholder}$line122 @@ -581,7 +581,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line122 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:122:36 │ 122 │ somecommand.py ~{placeholder}$line122 @@ -592,7 +592,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:123:22 │ 123 │ somecommand.py $line123~{placeholder} @@ -603,7 +603,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line123 is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:123:22 │ 123 │ somecommand.py $line123~{placeholder} @@ -614,7 +614,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:124:22 │ 124 │ ~{placeholder} $line124_trailing_pholder ~{placeholder} @@ -625,7 +625,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: line124_trailing_pholder is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:124:22 │ 124 │ ~{placeholder} $line124_trailing_pholder ~{placeholder} @@ -636,7 +636,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:126:37 │ 126 │ ~{placeholder} somecommand.py $leading_pholder @@ -647,7 +647,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: leading_pholder is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:126:37 │ 126 │ ~{placeholder} somecommand.py $leading_pholder @@ -658,7 +658,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: occurs_after_multiline is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:132:7 │ 132 │ $occurs_after_multiline @@ -669,7 +669,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Remove surrounding $() to avoid executing output (or use eval if intentional). ┌─ tests/lints/shellcheck-warn/source.wdl:134:7 │ 134 │ ╭ ╭ $(echo This is a @@ -679,7 +679,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Remove surrounding $() to avoid executing output (or use eval if intentional). ┌─ tests/lints/shellcheck-warn/source.wdl:137:7 │ 137 │ ╭ ╭ $(echo This is an @@ -690,7 +690,7 @@ note[ShellCheck]: `shellcheck` reported the following diagnostic │ = fix: address the diagnostic as recommended in the message -note[ShellCheck]: `shellcheck` reported the following diagnostic +note[ShellCheck]: Remove surrounding $() to avoid executing output (or use eval if intentional). ┌─ tests/lints/shellcheck-warn/source.wdl:141:7 │ 141 │ ╭ ╭ $(echo This is an From b538be306ea545e016145547014cef9807512c54 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Fri, 13 Dec 2024 19:40:02 -0500 Subject: [PATCH 29/52] fix: add shellcheck to windows path correctly --- .github/actions/install-shellcheck/action.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/install-shellcheck/action.yaml b/.github/actions/install-shellcheck/action.yaml index fad644b6b..db8a7a577 100644 --- a/.github/actions/install-shellcheck/action.yaml +++ b/.github/actions/install-shellcheck/action.yaml @@ -18,4 +18,4 @@ runs: run: | gh release download -R koalaman/shellcheck -p 'shellcheck-v*.zip' -O shellcheck-latest.zip 7z x shellcheck-latest.zip - echo $(Get-Location).Path >> "$GITHUB_PATH" + Add-Content $env:GITHUB_PATH "$(Get-Location).Path" From 264fa716388b228876b1d10fee7e78df26ea25e3 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Sat, 14 Dec 2024 18:39:20 -0500 Subject: [PATCH 30/52] fix: reduce false-positives by sometimes using literals --- wdl-lint/src/rules/shellcheck.rs | 33 +++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index ba0ff9556..985583e7e 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -166,7 +166,10 @@ impl Rule for ShellCheckRule { /// Convert a WDL `Placeholder` to a bash variable declaration. /// /// returns "WDL" + random alphnumeric characters. -/// 'WDL' + '~{}' = 6. +/// The returned value is shorter than the placeholder by 3 characters so +/// that the caller may pad with $, {}, or other characters as necessary +/// depending on whether or not the variable needs to be treated as a +/// declaration or literal. fn to_bash_var(placeholder: &Placeholder) -> String { let placeholder_len: usize = placeholder.syntax().text_range().len().into(); // don't start variable with numbers @@ -218,12 +221,16 @@ fn sanitize_command(section: &CommandSection) -> Option<(String, HashSet let mut sanitized_command = String::new(); let mut decls = HashSet::new(); let mut needs_quotes = true; + let mut is_literal = false; if let Some(cmd_parts) = section.strip_whitespace() { cmd_parts.iter().for_each(|part| match part { StrippedCommandPart::Text(text) => { sanitized_command.push_str(text); - // if this text section is not properly is not properly quoted - // then the next placeholder does *not* need double quotes + // if this placeholder is in a single-quoted segment + // don't treat as an expansion but rather a literal. + is_literal ^= !is_properly_quoted(text, '\''); + // if this text section is not properly quoted then the + // next placeholder does *not* need double quotes // because it will end up enclosed. needs_quotes ^= !is_properly_quoted(text, '"'); } @@ -231,18 +238,18 @@ fn sanitize_command(section: &CommandSection) -> Option<(String, HashSet let bash_var = to_bash_var(placeholder); // we need to save the var so we can suppress later decls.insert(bash_var.clone()); - let mut expansion = String::from("$"); - expansion.push_str(&bash_var); - if needs_quotes { - // surround with quotes - expansion.insert(0, '"'); - expansion.push('"'); + + if is_literal { + // pad literal with three underscores to account for ~{} + sanitized_command.push_str(&format!("___{bash_var}")); + } else if needs_quotes { + // surround with quotes for proper form + sanitized_command.push_str(&format!("\"${bash_var}\"")); } else { - // surround with {} - expansion.insert(1, '{'); - expansion.push('}'); + // surround with curly braces because already + // inside of a quoted segment. + sanitized_command.push_str(&format!("${{{bash_var}}}")); } - sanitized_command.push_str(&expansion); } }); Some((sanitized_command, decls)) From 698a638cbd49a09ba0fce0792ea8395b4878fc48 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 18:57:46 -0500 Subject: [PATCH 31/52] feat: enable optional shellcheck lints in cli --- wdl/src/bin/wdl.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/wdl/src/bin/wdl.rs b/wdl/src/bin/wdl.rs index 8de375cf0..4ed3abca4 100644 --- a/wdl/src/bin/wdl.rs +++ b/wdl/src/bin/wdl.rs @@ -9,6 +9,7 @@ use std::fs; use std::io::IsTerminal; use std::io::Read; use std::io::stderr; +use std::iter; use std::path::Path; use std::path::PathBuf; use std::path::absolute; @@ -50,6 +51,7 @@ use wdl_engine::local::LocalTaskExecutionBackend; use wdl_engine::v1::TaskEvaluator; use wdl_format::Formatter; use wdl_format::element::node::AstNodeFormatExt as _; +use wdl_lint::rules::ShellCheckRule; /// Emits the given diagnostics to the output stream. /// @@ -295,6 +297,9 @@ pub struct LintCommand { /// The path to the source WDL file. #[clap(value_name = "PATH")] pub path: PathBuf, + /// Enable shellcheck lints. + #[clap(long, action)] + pub shellcheck: bool, } impl LintCommand { @@ -314,6 +319,9 @@ impl LintCommand { let mut validator = Validator::default(); validator.add_visitor(LintVisitor::default()); + if self.shellcheck { + validator.add_visitor(ShellCheckRule); + } if let Err(diagnostics) = validator.validate(&document) { emit_diagnostics(&self.path.to_string_lossy(), &source, &diagnostics)?; From 886ae2cb923621d5ebab1f379f9dd1af137a447a Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 18:58:12 -0500 Subject: [PATCH 32/52] fix: disable install-shellcheck action --- .github/workflows/CI.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 1474743a2..4a8412b7b 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -64,7 +64,6 @@ jobs: os: [windows-latest, macos-latest, ubuntu-latest] steps: - uses: actions/checkout@v4 - - uses: ./.github/actions/install-shellcheck - run: git config --global core.autocrlf false - name: Update Rust run: rustup update stable && rustup default stable @@ -78,7 +77,6 @@ jobs: os: [windows-latest, macos-latest, ubuntu-latest] steps: - uses: actions/checkout@v4 - - uses: ./.github/actions/install-shellcheck - run: git config --global core.autocrlf false - name: Update Rust run: rustup update stable && rustup default stable From 3d4c32dd5ffd4264918d99a4f92fab725a99a062 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 19:00:54 -0500 Subject: [PATCH 33/52] fix: remove shellcheck from default rules --- wdl-lint/RULES.md | 2 +- wdl-lint/src/lib.rs | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/wdl-lint/RULES.md b/wdl-lint/RULES.md index beae8a000..4ac5098ae 100644 --- a/wdl-lint/RULES.md +++ b/wdl-lint/RULES.md @@ -43,7 +43,7 @@ be out of sync with released packages. | `RuntimeSectionKeys` | Completeness, Deprecated | Ensures that runtime sections have the appropriate keys. | | `RedundantInputAssignment` | Style | Ensures that redundant input assignments are shortened | | `SectionOrdering` | Sorting, Style | Ensures that sections within tasks and workflows are sorted. | -| `ShellCheck` | Correctness, Portability |Ensures that command sections are free of [shellcheck](https://www.shellcheck.net) diagnostics | +| `ShellCheck` | Correctness, Portability | (BETA) Ensures that command sections are free of shellcheck diagnostics. | | `SnakeCase` | Clarity, Naming, Style | Ensures that tasks, workflows, and variables are defined with snake_case names. | | `Todo` | Completeness | Ensures that `TODO` statements are flagged for followup. | | `TrailingComma` | Style | Ensures that lists and objects in meta have a trailing comma. | diff --git a/wdl-lint/src/lib.rs b/wdl-lint/src/lib.rs index 87aabf5a3..8362dae1f 100644 --- a/wdl-lint/src/lib.rs +++ b/wdl-lint/src/lib.rs @@ -125,7 +125,6 @@ pub fn rules() -> Vec> { Box::::default(), Box::::default(), Box::::default(), - Box::::default(), ]; // Ensure all the rule ids are unique and pascal case From 28d10715ba53e5db808944a37578340b31a4d303 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 19:01:01 -0500 Subject: [PATCH 34/52] chore: wording --- wdl-lint/src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wdl-lint/src/util.rs b/wdl-lint/src/util.rs index ee529ba8d..fb85ff4c6 100644 --- a/wdl-lint/src/util.rs +++ b/wdl-lint/src/util.rs @@ -34,7 +34,7 @@ pub fn is_inline_comment(token: &Comment) -> bool { false } -/// Determines if a string containing embedded quotes is properly quoted. +/// Determines whether or not a string containing embedded quotes is properly quoted. pub fn is_properly_quoted(s: &str, quote_char: char) -> bool { let mut closed = true; let mut escaped = false; From 7edf51416760fb81e0126e4a279bc728f97ad3d3 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 19:01:27 -0500 Subject: [PATCH 35/52] chore: fmt --- wdl-lint/src/util.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wdl-lint/src/util.rs b/wdl-lint/src/util.rs index fb85ff4c6..d676838a4 100644 --- a/wdl-lint/src/util.rs +++ b/wdl-lint/src/util.rs @@ -34,7 +34,8 @@ pub fn is_inline_comment(token: &Comment) -> bool { false } -/// Determines whether or not a string containing embedded quotes is properly quoted. +/// Determines whether or not a string containing embedded quotes is properly +/// quoted. pub fn is_properly_quoted(s: &str, quote_char: char) -> bool { let mut closed = true; let mut escaped = false; From fead4cd2fa57786919c7cf8bd7b9ce5e01c9ca62 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 20:23:59 -0500 Subject: [PATCH 36/52] chore: grammar --- wdl-lint/src/rules/shellcheck.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 985583e7e..5b044bf02 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -44,15 +44,15 @@ use crate::util::program_exists; /// The shellcheck executable const SHELLCHECK_BIN: &str = "shellcheck"; -/// shellcheck lints that we want to suppress -/// these two lints always co-occur with a more +/// Shellcheck lints that we want to suppresks. +/// These two lints always co-occur with a more /// informative message. const SHELLCHECK_SUPPRESS: &[&str] = &[ "1009", // the mentioned parser error was in... (unhelpful commentary) "1072", // Unexpected eof (unhelpful commentary) ]; -/// ShellCheck: var is referenced by not assigned. +/// ShellCheck: var is referenced but not assigned. const SHELLCHECK_REFERENCED_UNASSIGNED: usize = 2154; /// ShellCheck wiki base url. @@ -66,7 +66,7 @@ const ID: &str = "ShellCheck"; /// A ShellCheck diagnostic. /// -/// The file and fix fields are ommitted as we have no use for them. +/// The `file` and `fix` fields are ommitted as we have no use for them. #[derive(Clone, Debug, Deserialize)] struct ShellCheckDiagnostic { /// line number comment starts on @@ -132,7 +132,7 @@ fn run_shellcheck(command: &str) -> Result> { } } -/// Runs ShellCheck on a command section and reports violations +/// Runs ShellCheck on a command section and reports diagnostics. #[derive(Default, Debug, Clone, Copy)] pub struct ShellCheckRule; @@ -165,11 +165,11 @@ impl Rule for ShellCheckRule { /// Convert a WDL `Placeholder` to a bash variable declaration. /// -/// returns "WDL" + random alphnumeric characters. +/// Returns "WDL" + random alphnumeric characters. /// The returned value is shorter than the placeholder by 3 characters so -/// that the caller may pad with $, {}, or other characters as necessary +/// that the caller may pad with other characters as necessary /// depending on whether or not the variable needs to be treated as a -/// declaration or literal. +/// declaration, expansion, or literal. fn to_bash_var(placeholder: &Placeholder) -> String { let placeholder_len: usize = placeholder.syntax().text_range().len().into(); // don't start variable with numbers @@ -214,9 +214,9 @@ fn shellcheck_lint(diagnostic: &ShellCheckDiagnostic, span: Span) -> Diagnostic /// Sanitize a `CommandSection`. /// /// Removes all trailing whitespace, replaces placeholders -/// with dummy bash variables, and records declarations. +/// with dummy bash variables or literals, and records declarations. /// -/// If the section contains mixed indentation, returns None +/// If the section contains mixed indentation, returns None. fn sanitize_command(section: &CommandSection) -> Option<(String, HashSet)> { let mut sanitized_command = String::new(); let mut decls = HashSet::new(); From 47e911cdad7f1fa78552006b63e1c29ede1d9fb7 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 20:31:52 -0500 Subject: [PATCH 37/52] fix: add shellcheck lint rule to tests --- wdl-lint/tests/lints.rs | 2 + .../tests/lints/shellcheck-warn/source.errors | 68 +++++++++++++++---- .../tests/lints/shellcheck-warn/source.wdl | 6 +- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/wdl-lint/tests/lints.rs b/wdl-lint/tests/lints.rs index 12b4d761e..6117196c1 100644 --- a/wdl-lint/tests/lints.rs +++ b/wdl-lint/tests/lints.rs @@ -31,6 +31,7 @@ use rayon::prelude::*; use wdl_ast::Diagnostic; use wdl_ast::Document; use wdl_ast::Validator; +use wdl_lint::rules::ShellCheckRule; use wdl_lint::LintVisitor; /// Finds tests for this package. @@ -141,6 +142,7 @@ fn run_test(test: &Path, ntests: &AtomicUsize) -> Result<(), String> { } else { let mut validator = Validator::default(); validator.add_visitor(LintVisitor::default()); + validator.add_visitor(ShellCheckRule::default()); let errors = match validator.validate(&document) { Ok(()) => String::new(), Err(diagnostics) => format_diagnostics(&diagnostics, &path, &source), diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.errors b/wdl-lint/tests/lints/shellcheck-warn/source.errors index d38de1852..1781d2ffc 100644 --- a/wdl-lint/tests/lints/shellcheck-warn/source.errors +++ b/wdl-lint/tests/lints/shellcheck-warn/source.errors @@ -221,7 +221,7 @@ note[ShellCheck]: bad_test is referenced but not assigned. note[ShellCheck]: Double quote to prevent globbing and word splitting. ┌─ tests/lints/shellcheck-warn/source.wdl:54:27 │ -54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} +54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} │ ^^^^^^^^^^^^^^^ │ │ │ SC2086[info]: Double quote to prevent globbing and word splitting. @@ -232,7 +232,7 @@ note[ShellCheck]: Double quote to prevent globbing and word splitting. note[ShellCheck]: trailing_space is referenced but not assigned. ┌─ tests/lints/shellcheck-warn/source.wdl:54:27 │ -54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} +54 │ somecommand.py [ -f $trailing_space ] ~{placeholder} │ ^^^^^^^^^^^^^^^ │ │ │ SC2154[warning]: trailing_space is referenced but not assigned. @@ -505,24 +505,24 @@ note[ShellCheck]: leading_pholder is referenced but not assigned. = fix: address the diagnostic as recommended in the message note[ShellCheck]: Double quote to prevent globbing and word splitting. - ┌─ tests/lints/shellcheck-warn/source.wdl:118:28 + ┌─ tests/lints/shellcheck-warn/source.wdl:118:34 │ 118 │ command <<< weird stuff $firstlinelint - │ ^^^^^^^^^^^^^^ - │ │ - │ SC2086[info]: Double quote to prevent globbing and word splitting. - │ more info: https://www.shellcheck.net/wiki/SC2086 + │ ^^^^^^^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 │ = fix: address the diagnostic as recommended in the message note[ShellCheck]: firstlinelint is referenced but not assigned. - ┌─ tests/lints/shellcheck-warn/source.wdl:118:28 + ┌─ tests/lints/shellcheck-warn/source.wdl:118:34 │ 118 │ command <<< weird stuff $firstlinelint - │ ^^^^^^^^^^^^^^ - │ │ - │ SC2154[warning]: firstlinelint is referenced but not assigned. - │ more info: https://www.shellcheck.net/wiki/SC2154 + │ ^^^^^^^^^^^^^^ + │ │ + │ SC2154[warning]: firstlinelint is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 │ = fix: address the diagnostic as recommended in the message @@ -702,3 +702,47 @@ note[ShellCheck]: Remove surrounding $() to avoid executing output (or use eval │ = fix: address the diagnostic as recommended in the message +note[ShellCheck]: Remove surrounding $() to avoid executing output (or use eval if intentional). + ┌─ tests/lints/shellcheck-warn/source.wdl:146:7 + │ +146 │ ╭ ╭ $(echo this is a $lint146 that occurs in a / +147 │ │ │ multiline command / +148 │ │ │ with line breaks) + │ ╰─│─────────────────────────^ SC2091[warning]: Remove surrounding $() to avoid executing output (or use eval if intentional). + │ ╰─────────────────────────' more info: https://www.shellcheck.net/wiki/SC2091 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. + ┌─ tests/lints/shellcheck-warn/source.wdl:146:7 + │ +146 │ ╭ ╭ $(echo this is a $lint146 that occurs in a / +147 │ │ │ multiline command / +148 │ │ │ with line breaks) + │ ╰─│─────────────────────────^ SC2116[style]: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. + │ ╰─────────────────────────' more info: https://www.shellcheck.net/wiki/SC2116 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: Double quote to prevent globbing and word splitting. + ┌─ tests/lints/shellcheck-warn/source.wdl:146:24 + │ +146 │ $(echo this is a $lint146 that occurs in a / + │ ^^^^^^^^ + │ │ + │ SC2086[info]: Double quote to prevent globbing and word splitting. + │ more info: https://www.shellcheck.net/wiki/SC2086 + │ + = fix: address the diagnostic as recommended in the message + +note[ShellCheck]: lint146 is referenced but not assigned. + ┌─ tests/lints/shellcheck-warn/source.wdl:146:24 + │ +146 │ $(echo this is a $lint146 that occurs in a / + │ ^^^^^^^^ + │ │ + │ SC2154[warning]: lint146 is referenced but not assigned. + │ more info: https://www.shellcheck.net/wiki/SC2154 + │ + = fix: address the diagnostic as recommended in the message + diff --git a/wdl-lint/tests/lints/shellcheck-warn/source.wdl b/wdl-lint/tests/lints/shellcheck-warn/source.wdl index c20e7790e..66760195b 100644 --- a/wdl-lint/tests/lints/shellcheck-warn/source.wdl +++ b/wdl-lint/tests/lints/shellcheck-warn/source.wdl @@ -51,7 +51,7 @@ task test2 { somecommand.py ~{placeholder}$line51 somecommand.py $line52~{placeholder} somecommand.py [ -f $bad_test ] ~{placeholder} - somecommand.py [ -f $trailing_space ] ~{placeholder} + somecommand.py [ -f $trailing_space ] ~{placeholder} } output {} @@ -142,6 +142,10 @@ task test5 { even longer very long string that should really really really really ought to be quoted) + + $(echo this is a $lint146 that occurs in a \ + multiline command \ + with line breaks) >>> output {} From e83d79a50ef8e0a10914cf4b7a960367dae4d52d Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 20:51:25 -0500 Subject: [PATCH 38/52] feat: add shellcheck option to gauntlet --- gauntlet/src/lib.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/gauntlet/src/lib.rs b/gauntlet/src/lib.rs index 4c19b0638..38cc4eb15 100644 --- a/gauntlet/src/lib.rs +++ b/gauntlet/src/lib.rs @@ -44,6 +44,7 @@ use wdl::analysis::rules; use wdl::ast::Diagnostic; use wdl::lint::LintVisitor; use wdl::lint::ast::Validator; +use wdl::lint::rules::ShellCheckRule; use crate::repository::WorkDir; @@ -124,6 +125,10 @@ pub struct Args { /// Additional information is logged in the console. #[arg(short, long)] pub verbose: bool, + + /// Enable shellcheck lints. + #[arg(long, action)] + pub shellcheck: bool, } /// Main function for this subcommand. @@ -185,6 +190,9 @@ pub async fn gauntlet(args: Args) -> Result<()> { if args.arena { validator.add_visitor(LintVisitor::default()); } + if args.shellcheck { + validator.add_visitor(ShellCheckRule); + } validator }, From 76ed9090c9f958fbbc438169d8b7e9e0432bfb64 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 20:51:59 -0500 Subject: [PATCH 39/52] chore: fmt --- wdl-lint/tests/lints.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wdl-lint/tests/lints.rs b/wdl-lint/tests/lints.rs index 6117196c1..9cd0aa85e 100644 --- a/wdl-lint/tests/lints.rs +++ b/wdl-lint/tests/lints.rs @@ -31,8 +31,8 @@ use rayon::prelude::*; use wdl_ast::Diagnostic; use wdl_ast::Document; use wdl_ast::Validator; -use wdl_lint::rules::ShellCheckRule; use wdl_lint::LintVisitor; +use wdl_lint::rules::ShellCheckRule; /// Finds tests for this package. fn find_tests() -> Vec { From 91046fffa1db6e0078605d66602424d534470663 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 21:04:42 -0500 Subject: [PATCH 40/52] fix: undo changes to CI.yml --- .github/workflows/CI.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 4a8412b7b..1f01e4b23 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -5,10 +5,6 @@ on: branches: - main pull_request: - workflow_dispatch: - -env: - GH_TOKEN: ${{ github.token }} jobs: format: From d1bb4ae20625ee3d508f535425c84dbd5fbd73ee Mon Sep 17 00:00:00 2001 From: thatrichman Date: Tue, 17 Dec 2024 21:08:40 -0500 Subject: [PATCH 41/52] fix: always use latest github release of shellcheck --- .github/actions/install-shellcheck/action.yaml | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/actions/install-shellcheck/action.yaml b/.github/actions/install-shellcheck/action.yaml index db8a7a577..83aac585c 100644 --- a/.github/actions/install-shellcheck/action.yaml +++ b/.github/actions/install-shellcheck/action.yaml @@ -5,13 +5,20 @@ runs: steps: - uses: awalsh128/cache-apt-pkgs-action@latest if: runner.os == 'Linux' - with: - packages: shellcheck - version: 1.0 + run: + gh release download -R koalaman/shellcheck -p 'shellcheck-v*.linux.x86_64.tar.xz' -O shellcheck-latest.tar.xz + tar -xvf shellcheck-latest.tar.xz + chmod +x ./shellcheck + echo ${{github.workspace}} >> "$GITHUB_PATH" + export PATH="$PATH":"$(pwd)" - name: install shellcheck -- macOS shell: bash if: runner.os == 'macOS' - run: brew install shellcheck + run: | + gh release download -R koalaman/shellcheck -p 'shellcheck-v*.darwin.aarch64.tar.xz' -O shellcheck-latest.tar.xz + tar -xvf shellcheck-latest.tar.xz + chmod +x ./shellcheck + echo ${{github.workspace}} >> "$GITHUB_PATH" - name: install shellcheck -- Windows if: runner.os == 'Windows' shell: powershell From 58b8d67cffc3d62e7e0bf5d289187d3093c05e22 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 18 Dec 2024 08:55:20 -0500 Subject: [PATCH 42/52] chore: lints --- wdl/src/bin/wdl.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/wdl/src/bin/wdl.rs b/wdl/src/bin/wdl.rs index 4ed3abca4..66aae2102 100644 --- a/wdl/src/bin/wdl.rs +++ b/wdl/src/bin/wdl.rs @@ -9,7 +9,6 @@ use std::fs; use std::io::IsTerminal; use std::io::Read; use std::io::stderr; -use std::iter; use std::path::Path; use std::path::PathBuf; use std::path::absolute; From 5e942c56a222142ec0ab3e381fe782135b664589 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 18 Dec 2024 16:33:28 -0500 Subject: [PATCH 43/52] chore: update wdl and gauntlet changelogs --- gauntlet/CHANGELOG.md | 1 + wdl/CHANGELOG.md | 1 + 2 files changed, 2 insertions(+) diff --git a/gauntlet/CHANGELOG.md b/gauntlet/CHANGELOG.md index e2a17754c..ca5ccb0d5 100644 --- a/gauntlet/CHANGELOG.md +++ b/gauntlet/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +* `--shellcheck` flag to run shellcheck lints in both standard and arena modes ([#264](https://github.com/stjude-rust-labs/wdl/pull/264)) * Full analysis instead of basic validation ([#207](https://github.com/stjude-rust-labs/wdl/pull/172)) * Checkout submodules ([#207](https://github.com/stjude-rust-labs/wdl/pull/172)) diff --git a/wdl/CHANGELOG.md b/wdl/CHANGELOG.md index a02852ce7..d9fa7c583 100644 --- a/wdl/CHANGELOG.md +++ b/wdl/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +* Add `--shellcheck` flag to `wdl lint` subcommand to run shellcheck when linting ([#264](https://github.com/stjude-rust-labs/wdl/pull/264)) * Implemented the `wdl doc` subcommand for generating documentation (**currently in ALPHA testing**) ([#248](https://github.com/stjude-rust-labs/wdl/pull/248)). * Added an `--open` flag to `wdl doc` subcommand ([#269](https://github.com/stjude-rust-labs/wdl/pull/269)). * Added the `engine` module containing the implementation of `wdl-engine` ([#265](https://github.com/stjude-rust-labs/wdl/pull/265)). From d8dfef1207f9f87d7007650bc8359c069b5dfb09 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 18 Dec 2024 16:33:44 -0500 Subject: [PATCH 44/52] fix: re-enable missing shellcheck diagnostic --- wdl-lint/src/rules/shellcheck.rs | 35 ++++++++++++++++---------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/wdl-lint/src/rules/shellcheck.rs b/wdl-lint/src/rules/shellcheck.rs index 5b044bf02..062166d83 100644 --- a/wdl-lint/src/rules/shellcheck.rs +++ b/wdl-lint/src/rules/shellcheck.rs @@ -348,23 +348,24 @@ impl Visitor for ShellCheckRule { if !SHELLCHECK_EXISTS.get_or_init(|| { if !program_exists(SHELLCHECK_BIN) { - // TODO this diagnostic is currently disabled. - // When support for opt-in lints is added, this diagnostic - // should be enabled. - // - // let command_keyword = support::token(section.syntax(), - // SyntaxKind::CommandKeyword) .expect("should have a - // command keyword token"); state.exceptable_add( - // Diagnostic::note("running `shellcheck` on command section") - // .with_label( - // "could not find `shellcheck` executable.", - // command_keyword.text_range().to_span(), - // ) - // .with_rule(ID) - // .with_fix("install shellcheck (https://www.shellcheck.net) or disable this lint."), - // SyntaxElement::from(section.syntax().clone()), - // &self.exceptable_nodes(), - // ); + let command_keyword = support::token(section.syntax(), SyntaxKind::CommandKeyword) + .expect( + "should have a + command keyword token", + ); + state.exceptable_add( + Diagnostic::note("running `shellcheck` on command section") + .with_label( + "could not find `shellcheck` executable.", + command_keyword.text_range().to_span(), + ) + .with_rule(ID) + .with_fix( + "install shellcheck (https://www.shellcheck.net) or disable this lint.", + ), + SyntaxElement::from(section.syntax().clone()), + &self.exceptable_nodes(), + ); return false; } true From 984dd706a765bcd2e99500c5f0dbb7240403564f Mon Sep 17 00:00:00 2001 From: thatrichman Date: Wed, 18 Dec 2024 17:09:07 -0500 Subject: [PATCH 45/52] fix: include optional rules when checking rule validity --- wdl-lint/src/lib.rs | 31 +++++++++++++++++++ .../src/rules/misplaced_lint_directive.rs | 6 ++++ wdl-lint/tests/lints/shellcheck-ok/source.wdl | 25 +++++++++++++++ 3 files changed, 62 insertions(+) diff --git a/wdl-lint/src/lib.rs b/wdl-lint/src/lib.rs index 8362dae1f..22d5c94d2 100644 --- a/wdl-lint/src/lib.rs +++ b/wdl-lint/src/lib.rs @@ -29,6 +29,8 @@ #![warn(clippy::missing_docs_in_private_items)] #![warn(rustdoc::broken_intra_doc_links)] +use std::collections::HashSet; + use wdl_ast::Diagnostics; use wdl_ast::SyntaxKind; use wdl_ast::Visitor; @@ -150,3 +152,32 @@ pub fn rules() -> Vec> { rules } + +/// Gets the optional rule set. +pub fn optional_rules() -> Vec> { + let opt_rules: Vec> = vec![Box::::default()]; + + // Ensure all the rule ids are unique and pascal case + #[cfg(debug_assertions)] + { + use crate::rules; + use convert_case::Case; + use convert_case::Casing; + let mut set: HashSet<&str> = HashSet::from_iter(rules().iter().map(|r| r.id())); + for r in opt_rules.iter() { + if r.id().to_case(Case::Pascal) != r.id() { + panic!("lint rule id `{id}` is not pascal case", id = r.id()); + } + + if !set.insert(r.id()) { + panic!("duplicate rule id `{id}`", id = r.id()); + } + + if RESERVED_RULE_IDS.contains(&r.id()) { + panic!("rule id `{id}` is reserved", id = r.id()); + } + } + } + + opt_rules +} diff --git a/wdl-lint/src/rules/misplaced_lint_directive.rs b/wdl-lint/src/rules/misplaced_lint_directive.rs index 750ce1ffb..af68aa0af 100644 --- a/wdl-lint/src/rules/misplaced_lint_directive.rs +++ b/wdl-lint/src/rules/misplaced_lint_directive.rs @@ -17,9 +17,11 @@ use wdl_ast::ToSpan; use wdl_ast::VisitReason; use wdl_ast::Visitor; +use super::ShellCheckRule; use crate::Rule; use crate::Tag; use crate::TagSet; +use crate::optional_rules; use crate::rules; /// The identifier for the unknown rule rule. @@ -60,6 +62,10 @@ pub static RULE_MAP: LazyLock Date: Wed, 18 Dec 2024 17:10:04 -0500 Subject: [PATCH 46/52] chore: fmt --- wdl-lint/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/wdl-lint/src/lib.rs b/wdl-lint/src/lib.rs index 22d5c94d2..e35d55a20 100644 --- a/wdl-lint/src/lib.rs +++ b/wdl-lint/src/lib.rs @@ -160,9 +160,10 @@ pub fn optional_rules() -> Vec> { // Ensure all the rule ids are unique and pascal case #[cfg(debug_assertions)] { - use crate::rules; use convert_case::Case; use convert_case::Casing; + + use crate::rules; let mut set: HashSet<&str> = HashSet::from_iter(rules().iter().map(|r| r.id())); for r in opt_rules.iter() { if r.id().to_case(Case::Pascal) != r.id() { From 7db42462d713afbab308c948b210e2185470db34 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Thu, 19 Dec 2024 15:23:13 -0500 Subject: [PATCH 47/52] chore: lint --- wdl-lint/src/rules/misplaced_lint_directive.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/wdl-lint/src/rules/misplaced_lint_directive.rs b/wdl-lint/src/rules/misplaced_lint_directive.rs index af68aa0af..2e8591121 100644 --- a/wdl-lint/src/rules/misplaced_lint_directive.rs +++ b/wdl-lint/src/rules/misplaced_lint_directive.rs @@ -17,7 +17,6 @@ use wdl_ast::ToSpan; use wdl_ast::VisitReason; use wdl_ast::Visitor; -use super::ShellCheckRule; use crate::Rule; use crate::Tag; use crate::TagSet; From 7b50b5b59884fc8a27b633c0b2c328efda85a727 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Thu, 19 Dec 2024 15:25:02 -0500 Subject: [PATCH 48/52] fix: only use shellcheck rule in arena mode --- gauntlet/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gauntlet/src/lib.rs b/gauntlet/src/lib.rs index 38cc4eb15..09010b48c 100644 --- a/gauntlet/src/lib.rs +++ b/gauntlet/src/lib.rs @@ -127,7 +127,7 @@ pub struct Args { pub verbose: bool, /// Enable shellcheck lints. - #[arg(long, action)] + #[arg(long, action, requires = "arena")] pub shellcheck: bool, } @@ -189,9 +189,9 @@ pub async fn gauntlet(args: Args) -> Result<()> { }; if args.arena { validator.add_visitor(LintVisitor::default()); - } - if args.shellcheck { - validator.add_visitor(ShellCheckRule); + if args.shellcheck { + validator.add_visitor(ShellCheckRule); + } } validator From 3dfa8fdf5dff95ae78d034c779efa74cfcd419d0 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Thu, 19 Dec 2024 15:28:51 -0500 Subject: [PATCH 49/52] chore: lint --- wdl-lint/src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/wdl-lint/src/lib.rs b/wdl-lint/src/lib.rs index e35d55a20..cffdc5bc7 100644 --- a/wdl-lint/src/lib.rs +++ b/wdl-lint/src/lib.rs @@ -29,8 +29,6 @@ #![warn(clippy::missing_docs_in_private_items)] #![warn(rustdoc::broken_intra_doc_links)] -use std::collections::HashSet; - use wdl_ast::Diagnostics; use wdl_ast::SyntaxKind; use wdl_ast::Visitor; @@ -164,7 +162,8 @@ pub fn optional_rules() -> Vec> { use convert_case::Casing; use crate::rules; - let mut set: HashSet<&str> = HashSet::from_iter(rules().iter().map(|r| r.id())); + let mut set: std::collections::HashSet<&str> = + std::collections::HashSet::from_iter(rules().iter().map(|r| r.id())); for r in opt_rules.iter() { if r.id().to_case(Case::Pascal) != r.id() { panic!("lint rule id `{id}` is not pascal case", id = r.id()); From 3a8e4aeba209174a8195d0c04e0bf63794ab7491 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Thu, 19 Dec 2024 16:50:23 -0500 Subject: [PATCH 50/52] fix: re-enable shellcheck install for CI --- .github/actions/install-shellcheck/action.yaml | 13 ++++++++----- .github/workflows/CI.yml | 4 ++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.github/actions/install-shellcheck/action.yaml b/.github/actions/install-shellcheck/action.yaml index 83aac585c..67427252d 100644 --- a/.github/actions/install-shellcheck/action.yaml +++ b/.github/actions/install-shellcheck/action.yaml @@ -3,11 +3,12 @@ description: "Installs ShellCheck. Supports Ubuntu, macOS, and Windows." runs: using: "composite" steps: - - uses: awalsh128/cache-apt-pkgs-action@latest + - name: install shellcheck -- Ubuntu if: runner.os == 'Linux' - run: + shell: bash + run: | gh release download -R koalaman/shellcheck -p 'shellcheck-v*.linux.x86_64.tar.xz' -O shellcheck-latest.tar.xz - tar -xvf shellcheck-latest.tar.xz + tar -xvf shellcheck-latest.tar.xz --strip-components 1 chmod +x ./shellcheck echo ${{github.workspace}} >> "$GITHUB_PATH" export PATH="$PATH":"$(pwd)" @@ -16,7 +17,7 @@ runs: if: runner.os == 'macOS' run: | gh release download -R koalaman/shellcheck -p 'shellcheck-v*.darwin.aarch64.tar.xz' -O shellcheck-latest.tar.xz - tar -xvf shellcheck-latest.tar.xz + tar -xvf shellcheck-latest.tar.xz --strip-components 1 chmod +x ./shellcheck echo ${{github.workspace}} >> "$GITHUB_PATH" - name: install shellcheck -- Windows @@ -25,4 +26,6 @@ runs: run: | gh release download -R koalaman/shellcheck -p 'shellcheck-v*.zip' -O shellcheck-latest.zip 7z x shellcheck-latest.zip - Add-Content $env:GITHUB_PATH "$(Get-Location).Path" + New-Item -ItemType Directory -Force -Path "$env:USERPROFILE\.local\bin" + Move-Item -Force "shellcheck.exe" "$env:USERPROFILE\.local\bin" + Add-Content $env:GITHUB_PATH "$env:USERPROFILE\.local\bin" diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 1f01e4b23..59986a4c9 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -6,6 +6,9 @@ on: - main pull_request: +env: + GH_TOKEN: ${{ github.token }} + jobs: format: runs-on: ubuntu-latest @@ -39,6 +42,7 @@ jobs: os: [ubuntu-latest, macos-latest, windows-latest] steps: - uses: actions/checkout@v4 + - uses: ./.github/actions/install-shellcheck - name: Update Rust run: rustup update stable && rustup default stable - run: cargo test --all --all-features From 24d111482121cd53c24da25624e2e151fbc3128a Mon Sep 17 00:00:00 2001 From: Spencer Richman Date: Fri, 20 Dec 2024 09:10:46 -0500 Subject: [PATCH 51/52] fix: gauntlet/CHANGELOG.md description Co-authored-by: Andrew Frantz --- gauntlet/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gauntlet/CHANGELOG.md b/gauntlet/CHANGELOG.md index ca5ccb0d5..9192b641b 100644 --- a/gauntlet/CHANGELOG.md +++ b/gauntlet/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -* `--shellcheck` flag to run shellcheck lints in both standard and arena modes ([#264](https://github.com/stjude-rust-labs/wdl/pull/264)) +* `--shellcheck` flag to run shellcheck lints in arena mode ([#264](https://github.com/stjude-rust-labs/wdl/pull/264)) * Full analysis instead of basic validation ([#207](https://github.com/stjude-rust-labs/wdl/pull/172)) * Checkout submodules ([#207](https://github.com/stjude-rust-labs/wdl/pull/172)) From 90404305bbd305dead390dc450f579f9844bb6e2 Mon Sep 17 00:00:00 2001 From: thatrichman Date: Fri, 20 Dec 2024 09:14:03 -0500 Subject: [PATCH 52/52] feat: add test shellcheck disable directives --- wdl-lint/tests/lints/shellcheck-ok/source.wdl | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/wdl-lint/tests/lints/shellcheck-ok/source.wdl b/wdl-lint/tests/lints/shellcheck-ok/source.wdl index 3ee61e202..de3769ca3 100644 --- a/wdl-lint/tests/lints/shellcheck-ok/source.wdl +++ b/wdl-lint/tests/lints/shellcheck-ok/source.wdl @@ -77,3 +77,25 @@ task test3 { runtime {} } + +task test4 { + meta {} + + parameter_meta {} + + input { + Int placeholder + } + + command { + set -eo pipefail + + unquoted_var="foo bar baz" + # shellcheck disable=SC2086 + echo $unquoted_var + } + + output {} + + runtime {} +}