Skip to content

Improve ShellCheck diagnostic span for WDL interpolations#733

Open
kris70lesgo wants to merge 4 commits intostjude-rust-labs:mainfrom
kris70lesgo:fix/shellcheck-span-605
Open

Improve ShellCheck diagnostic span for WDL interpolations#733
kris70lesgo wants to merge 4 commits intostjude-rust-labs:mainfrom
kris70lesgo:fix/shellcheck-span-605

Conversation

@kris70lesgo
Copy link
Contributor

@kris70lesgo kris70lesgo commented Mar 16, 2026

Describe the problem or feature in addition to a link to the issues.

ShellCheck diagnostics for WDL command placeholders currently highlight the entire ~{...} placeholder even when only a specific expression inside the interpolation is problematic.
This pr improves diagnostic precision by tracking the span of the interpolated expression and mapping ShellCheck warnings back to that span instead of the full placeholder.
Changes:

  • Refactor to_bash_var() to return the span of the interpolated expression.
  • Track placeholder -> expression span mapping during command sanitization.
  • Update shellcheck_lint() to use the expression span when emitting diagnostics.
    Example

Before:

~{if threads > 1 then "--threads " + (threads - 1) else ""}
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

After:

"--threads " + (threads - 1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Before submitting this PR, please make sure:

For external contributors:

  • You have read the contributing guide in its entirety.
  • You have not used AI on any parts of this pull request.
  • You have added a few sentences describing the PR here.
  • Your code builds clean without any errors or warnings.

For all contributors:

  • You have added tests (when appropriate).
  • You have added an entry in the CHANGELOG (when appropriate).
  • You have updated the README or other documentation to account for these changes (when appropriate).
  • You have made a PR to the next branch in the sprocket.bio repository (when appropriate).

For PRs containing lint rule changes:

  • You have updated any and all effected entries within RULES.md.
  • You have added a test case in crates/wdl-lint/tests/lints that covers every
    possible diagnostic emitted for the rule within the file where the rule
    is implemented.

@kris70lesgo kris70lesgo requested a review from a team as a code owner March 16, 2026 18:03
@kris70lesgo kris70lesgo requested a review from adthrasher March 16, 2026 18:03
@kris70lesgo
Copy link
Contributor Author

@a-frantz can u review the pr ?

@adthrasher adthrasher added the S-awaiting-pass-CI PR is awaiting CI to pass. label Mar 16, 2026
Copy link
Member

@a-frantz a-frantz left a comment

Choose a reason for hiding this comment

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

this looks like a great start! Can you fix all the issues reported by CI?

@github-actions github-actions bot removed the S-awaiting-pass-CI PR is awaiting CI to pass. label Mar 17, 2026
@kris70lesgo
Copy link
Contributor Author

@a-frantz Thanks for the review , all checks are passing now. If u get time can u review it again ?
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No clear syntax to avoid shellcheck SC2086 for expression substitutions

3 participants