Skip to content

fix: validate set and shopt options when reading a Taskfile#2911

Open
posidoni wants to merge 1 commit into
go-task:mainfrom
posidoni:fix-validate-set-shopt-options
Open

fix: validate set and shopt options when reading a Taskfile#2911
posidoni wants to merge 1 commit into
go-task:mainfrom
posidoni:fix-validate-set-shopt-options

Conversation

@posidoni

Copy link
Copy Markdown

Follow-up to #2429. The panic from that issue was fixed upstream in mvdan/sh, but an invalid set:/shopt: option is still only reported when a command actually runs — once per command, with no hint about the actual mistake:

task: [default] echo "hello"
shopt: invalid option name "pipefail"
task: Failed to run task "default": exit status 1

This PR validates set and shopt lists when the Taskfile is read (root, task and command level), following the approach of #2892:

task: invalid shopt option "pipefail" (did you mean to put it in "set"?)
file: Taskfile.yml:1:1
  • Valid values mirror the interpreter's unexported posixOptsTable/bashOptsTable (mvdan.cc/sh/v3/interp), i.e. exactly what works at runtime.
  • shopt: ["-o", "pipefail"] keeps working: a -o entry switches validation of the remaining entries to set options, matching shopt -s -o semantics (covered by a regression test).
  • When an option is placed in the wrong list, the error suggests the right one (pipefailset, globstarshopt).
  • schema.json: the shopt enum was missing three supported options (dotglob, extglob, nocaseglob) — extended to all six; reference docs updated accordingly.

Note on behavior: an invalid option now fails at parse time even if the task declaring it never runs — consistent with other Taskfile validation such as #2892.

Tested with four new tests (an invalid option at each of the three levels, plus the -o regression) and a full go test ./... run.

Disclosure per the AI usage policy: this PR (code and description) was prepared with AI assistance. I reviewed and tested all changes and can explain every line.

@trulede trulede left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IMHO I think it would be better if errors with options were raised by mvdan (which I think does this already, perhaps?) than to have to keep such a list up to date.

End effect would be (is) the same, an error.

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.

2 participants