Skip to content

Commit

Permalink
fix: address stylistic comments from PR (stjude-rust-labs#191)
Browse files Browse the repository at this point in the history
  • Loading branch information
thatRichman committed Dec 18, 2024
1 parent 3ad8425 commit 75e04c4
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 10 deletions.
2 changes: 1 addition & 1 deletion wdl-lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ pub trait Rule: Visitor<State = Diagnostics> {
/// Gets the default rule set.
pub fn rules() -> Vec<Box<dyn Rule>> {
let rules: Vec<Box<dyn Rule>> = vec![
Box::<rules::CommandSectionShellCheck>::default(),
Box::<rules::DoubleQuotesRule>::default(),
Box::<rules::NoCurlyCommandsRule>::default(),
Box::<rules::SnakeCaseRule>::default(),
Expand Down Expand Up @@ -126,6 +125,7 @@ pub fn rules() -> Vec<Box<dyn Rule>> {
Box::<rules::PreambleCommentAfterVersionRule>::default(),
Box::<rules::MalformedLintDirectiveRule>::default(),
Box::<rules::RedundantInputAssignment>::default(),
Box::<rules::ShellCheckRule>::default(),
];

// Ensure all the rule ids are unique and pascal case
Expand Down
4 changes: 2 additions & 2 deletions wdl-lint/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::*;
Expand Down
18 changes: 11 additions & 7 deletions wdl-lint/src/rules/command_shellcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -121,7 +124,7 @@ fn run_shellcheck(command: &str) -> Result<Vec<ShellCheckComment>> {
#[derive(Default, Debug, Clone, Copy)]
pub struct ShellCheckRule;

impl Rule for CommandSectionShellCheck {
impl Rule for ShellCheckRule {
fn id(&self) -> &'static str {
ID
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
);
Expand Down

0 comments on commit 75e04c4

Please sign in to comment.