mrtazz / checkmake

experimental linter/analyzer for Makefiles
MIT License
1.02k stars 44 forks source link

Support shellcheck for checking commands inside recipes #83

Open EmilyGraceSeville7cf opened 1 year ago

EmilyGraceSeville7cf commented 1 year ago

Expected behaviour

Shell code is expected to be linted.

Actual behaviour

Now no errors are thrown for incorrect shell commands.

Output of checkmake --version

Output of checkmake --debug <your makefile>

Output of make --version

Sample Makefile to reproduce issue

(some of these things might not apply but the more you can provide the easier it will be to fix this bug. Thanks!)

jenstroeger commented 1 year ago

If the linted Makefile sets the shell then running shellcheck on individual recipes would make a lot of sense.

cooljeanius commented 7 months ago

One thing to note is that there are a few differences between shell commands as embedded in Makefiles, and normal shell commands... like, shellcheck's rule about using $(...) instead of backticks probably wouldn't make as much sense when run from within checkmake, since $(...) has its own separate meaning from within the Makefile context, and there's that rule about doubling up on dollar signs...

jenstroeger commented 7 months ago

One thing to note is that there are a few differences between shell commands as embedded in Makefiles, and normal shell commands... like, shellcheck's rule about using $(...) instead of backticks probably wouldn't make as much sense when run from within checkmake, since $(...) has its own separate meaning from within the Makefile context, and there's that rule about doubling up on dollar signs...

In which case it should be escaped using $$(...) as you can see in this Makefile.

cooljeanius commented 7 months ago

One thing to note is that there are a few differences between shell commands as embedded in Makefiles, and normal shell commands... like, shellcheck's rule about using $(...) instead of backticks probably wouldn't make as much sense when run from within checkmake, since $(...) has its own separate meaning from within the Makefile context, and there's that rule about doubling up on dollar signs...

In which case it should be escaped using $$(...) as you can see in this Makefile.

Is there a way to edit shellcheck's output to get it to say that instead, though?

jenstroeger commented 7 months ago

Is there a way to edit shellcheck's output to get it to say that instead, though?

You mean ask shellcheck to propose $$(...) to replace backticks if it checks Makefile recipes?

cooljeanius commented 7 months ago

Is there a way to edit shellcheck's output to get it to say that instead, though?

You mean ask shellcheck to propose $$(...) to replace backticks if it checks Makefile recipes?

Well, I'm not quite sure: is this the kind of thing that should be done on shellcheck's end, or should checkmake instead handle transforming shellcheck's output itself before passing it along to the user?

jenstroeger commented 7 months ago

Well, I'm not quite sure: is this the kind of thing that should be done on shellcheck's end, or should checkmake instead handle transforming shellcheck's output itself before passing it along to the user?

Can checkmate relay the output? That’d be very useful, esp if it can readjust the line numbers. Regarding the escaping, I think checkmate should warn and suggest these two options — hopefully the user understands the difference and chooses whichever works best for her in context.

cooljeanius commented 7 months ago

Well, I'm not quite sure: is this the kind of thing that should be done on shellcheck's end, or should checkmake instead handle transforming shellcheck's output itself before passing it along to the user?

Can checkmate relay the output?

I don't know; hey @mrtazz can it do so?