Skip to content

Add shellcheck_toolchain rule - #48

Merged
aignas merged 5 commits into
aignas:mainfrom
UebelAndre:aspect
Mar 4, 2026
Merged

aignas merged 5 commits into
aignas:mainfrom
UebelAndre:aspect

Conversation

@UebelAndre

@UebelAndre UebelAndre commented Feb 21, 2026 •

Copy link
Copy Markdown
Contributor

This allows for users to provide custom shellcheck binaries to power the shellcheck rules.

Additionally, this adds support for .shellcheckrc files via --@rules_shellcheck//shellcheck:rc=<label_of_file>

closes #24

@UebelAndre
UebelAndre marked this pull request as ready for review February 21, 2026 16:50
@UebelAndre

Copy link
Copy Markdown
Contributor Author

@aignas this also fixes the release workflow to not trigger on any push to main. That was a misunderstanding on my part from #47

@UebelAndre

Copy link
Copy Markdown
Contributor Author

It'd also be awesome if you could cut a release after this PR 🙏

@UebelAndre

Copy link
Copy Markdown
Contributor Author

@aignas friendly ping here 🙏

@aignas aignas left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for all of the contributions.

Comment thread shellcheck/internal/rules.bzl
Comment thread shellcheck/internal/extensions.bzl Outdated
Comment on lines +124 to +137
alias(
name = "shellcheck",
actual = "shellcheck.exe",
)

alias(
name = "{name}",
actual = "shellcheck.exe",
)

shellcheck_toolchain(
name = "toolchain",
shellcheck = ":shellcheck",
)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nit it would be nice to have a single macro here. That we if we update the macro implementation (e.g. change the alias), then we don't need to refetch everything.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean by "single macro". Can you expand?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Having a macro defined somewhere in the ruleset that takes care of the differences between windows and linux via select statements. That way we would have a single template for both cases. For example we have this whl_library_targets.bzl in rules_python that has the macro that contains all of the logic for how the targets get constructed and the whl_library BUILD.bazel file just loads and calls that macro.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand the direction still. Are you suggesting the rendered BUILD file contain a wrapper macro call instead of shellcheck_toolchain directly? Or something else? I'm happy to make changes around this code, I just fail to understand the direction you'd like me to go 😅

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Yes having a wrapper macro to create all of the targets was what I had in mind.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done!

Comment thread shellcheck/internal/extensions.bzl Outdated
Comment thread .github/workflows/release.yml Outdated
@UebelAndre

Copy link
Copy Markdown
Contributor Author

@aignas friendly ping here 🙏

@UebelAndre

Copy link
Copy Markdown
Contributor Author

@aignas any more changes you'd like to see? I'm eager to have this available in the Bazel Central Registry 😅

@aignas aignas left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you!

@aignas
aignas merged commit 6ec80c8 into aignas:main Mar 4, 2026
3 checks passed
@UebelAndre
UebelAndre deleted the aspect branch March 4, 2026 11:51
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.

Support specifying .shellcheckrc via a flag

2 participants