Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement shellcheck lints in command blocks (#191) #264

Merged
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
5a7aab3
feat: implement shellcheck lints in command blocks (#191)
thatRichman Dec 8, 2024
3ad8425
fix: apply stylistic suggestions from PR (#191)
thatRichman Dec 8, 2024
75e04c4
fix: address stylistic comments from PR (#191)
thatRichman Dec 8, 2024
a8c4d34
fix: only test for shellcheck once
thatRichman Dec 9, 2024
a36ae83
fix: address stylistic comments from PR (#191)
thatRichman Dec 10, 2024
19a6692
fix: use a random string for bash variables
thatRichman Dec 10, 2024
5a8cc11
fix: remove some suppressed lints, enable style lints
thatRichman Dec 10, 2024
c1375b9
refactor: split into smaller functions
thatRichman Dec 10, 2024
38e9994
fix: rework WDL -> bash script line mapping
thatRichman Dec 10, 2024
c0b9825
fix: make `program_exists` cross-platform
thatRichman Dec 10, 2024
1423a22
feat: add tests for shellcheck lint
thatRichman Dec 10, 2024
f7c5431
fix: prefix all bash vars with 'WDL'
thatRichman Dec 10, 2024
05a583b
feat: add tests for shellcheck lints
thatRichman Dec 10, 2024
7cbe4ac
fix: use diagnostic terminology consistently
thatRichman Dec 11, 2024
a97fcad
fix: correct diagnostic placement algorithm
thatRichman Dec 11, 2024
405afff
fix: disable missing shellcheck diagnostic
thatRichman Dec 12, 2024
ed879c9
feat: add debug log for shellcheck process id
thatRichman Dec 12, 2024
d7382e4
feat: clarify suppressed diagnostics
thatRichman Dec 12, 2024
ce77d8a
fix: remove task definition from shellcheck exceptable nodes
thatRichman Dec 12, 2024
282a52c
fix: multiline diagnostic placement
thatRichman Dec 12, 2024
007592c
fix: clarify language for disabled diangostic
thatRichman Dec 12, 2024
ce1e69e
feat: update RULES.md and CHANGELOG.md
thatRichman Dec 12, 2024
c9fe044
fix: apply formatting rules
thatRichman Dec 13, 2024
d419085
fix: do not overquote bash vars
thatRichman Dec 13, 2024
9bcd09d
fix: apply formatting rules
thatRichman Dec 13, 2024
4215c5d
fix: diagnostic comments must be unique
thatRichman Dec 13, 2024
23dfe5a
feat: add install-shellcheck action
thatRichman Dec 13, 2024
e948c81
fix: update source.errors with new lint format
thatRichman Dec 13, 2024
b538be3
fix: add shellcheck to windows path correctly
thatRichman Dec 14, 2024
264fa71
fix: reduce false-positives by sometimes using literals
thatRichman Dec 14, 2024
698a638
feat: enable optional shellcheck lints in cli
thatRichman Dec 17, 2024
886ae2c
fix: disable install-shellcheck action
thatRichman Dec 17, 2024
3d4c32d
fix: remove shellcheck from default rules
thatRichman Dec 18, 2024
28d1071
chore: wording
thatRichman Dec 18, 2024
7edf514
chore: fmt
thatRichman Dec 18, 2024
fead4cd
chore: grammar
thatRichman Dec 18, 2024
47e911c
fix: add shellcheck lint rule to tests
thatRichman Dec 18, 2024
e83d79a
feat: add shellcheck option to gauntlet
thatRichman Dec 18, 2024
76ed909
chore: fmt
thatRichman Dec 18, 2024
91046ff
fix: undo changes to CI.yml
thatRichman Dec 18, 2024
d1bb4ae
fix: always use latest github release of shellcheck
thatRichman Dec 18, 2024
58b8d67
chore: lints
thatRichman Dec 18, 2024
5e942c5
chore: update wdl and gauntlet changelogs
thatRichman Dec 18, 2024
d8dfef1
fix: re-enable missing shellcheck diagnostic
thatRichman Dec 18, 2024
984dd70
fix: include optional rules when checking rule validity
thatRichman Dec 18, 2024
9cd70c0
chore: fmt
thatRichman Dec 18, 2024
7db4246
chore: lint
thatRichman Dec 19, 2024
7b50b5b
fix: only use shellcheck rule in arena mode
thatRichman Dec 19, 2024
3dfa8fd
chore: lint
thatRichman Dec 19, 2024
3a8e4ae
fix: re-enable shellcheck install for CI
thatRichman Dec 19, 2024
24d1114
fix: gauntlet/CHANGELOG.md description
thatRichman Dec 20, 2024
9040430
feat: add test shellcheck disable directives
thatRichman Dec 20, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/actions/install-shellcheck/action.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
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'
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: |
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
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"
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
1 change: 1 addition & 0 deletions gauntlet/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

* `--shellcheck` flag to run shellcheck lints in both standard and arena modes ([#264](https://github.com/stjude-rust-labs/wdl/pull/264))
thatRichman marked this conversation as resolved.
Show resolved Hide resolved
* 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))

Expand Down
8 changes: 8 additions & 0 deletions gauntlet/src/lib.rs
thatRichman marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -185,6 +190,9 @@ pub async fn gauntlet(args: Args) -> Result<()> {
if args.arena {
validator.add_visitor(LintVisitor::default());
}
if args.shellcheck {
thatRichman marked this conversation as resolved.
Show resolved Hide resolved
validator.add_visitor(ShellCheckRule);
}

validator
},
Expand Down
1 change: 1 addition & 0 deletions wdl-lint/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Added

* Added 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
Expand Down
5 changes: 5 additions & 0 deletions wdl-lint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,14 @@ 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 = { workspace = true }
serde_json = { workspace = true }
tracing = { workspace = true }

[dev-dependencies]
codespan-reporting = { workspace = true }
Expand Down
1 change: 1 addition & 0 deletions wdl-lint/RULES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | (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. |
Expand Down
32 changes: 32 additions & 0 deletions wdl-lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -150,3 +152,33 @@ pub fn rules() -> Vec<Box<dyn Rule>> {

rules
}

/// Gets the optional rule set.
pub fn optional_rules() -> Vec<Box<dyn Rule>> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is probably overkill since this will (hopefully) be a temporary home for shellcheck.

let opt_rules: Vec<Box<dyn Rule>> = vec![Box::<rules::ShellCheckRule>::default()];

// Ensure all the rule ids are unique and pascal case
#[cfg(debug_assertions)]
{
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() {
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
}
2 changes: 2 additions & 0 deletions wdl-lint/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,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;
Expand Down Expand Up @@ -77,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::*;
Expand Down
6 changes: 6 additions & 0 deletions wdl-lint/src/rules/misplaced_lint_directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -60,6 +62,10 @@ pub static RULE_MAP: LazyLock<HashMap<&'static str, Option<&'static [SyntaxKind]
for rule in rules() {
map.insert(rule.id(), rule.exceptable_nodes());
}
// insert optional rules as well
for rule in optional_rules() {
map.insert(rule.id(), rule.exceptable_nodes());
}
map
});

Expand Down
Loading
Loading