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

✨ Integration with shellcheck #72

Open
franchb opened this issue May 22, 2024 · 7 comments
Open

✨ Integration with shellcheck #72

franchb opened this issue May 22, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@franchb
Copy link

franchb commented May 22, 2024

Usecase

As a DevSecOps specialist, I regularly enforce a check stage for all bash scripts in our project repository utilizing shellcheck, a tool that provides linting for bash scripts to ensure they are written following best practices and avoid common mistakes.

I am interested in experimenting with writing my automation scripts using the Amber scripting language and then compiling them into bash scripts. Ьн CI/CD system currently only permits the execution of bash scripts.

To streamline this workflow, I am seeking a feature in Amber that would allow the automatic generation of bash scripts that are compliant with shellcheck standards right out of the box. This would enable me to leverage the power and convenience of Amber without the need for post-processing or manual corrections on the generated bash scripts.

The ability to do this would ensure that we maintain our code quality and security standards without compromising on efficiency or having to disable shellcheck for these generated scripts.

@Ph0enixKM Ph0enixKM changed the title [Feature request] Integration with shellcheck ✨ [Feature request] Integration with shellcheck May 25, 2024
@Mte90
Copy link
Member

Mte90 commented Jun 12, 2024

Right now there are various optimization that we can do to be more aligned with shellcheck:

cd /tmp > /dev/null 2>&1 https://www.shellcheck.net/wiki/SC2164

wget ${__0_download_url} https://www.shellcheck.net/wiki/SC2086

if [ $(echo '!' $__AF_dir_exist4_v0__57 | bc -l | sed '/\./ s/\.\{0,1\}0\{1,\}$//') != 0 ]; then https://www.shellcheck.net/wiki/SC2046

let index=${index}+1 https://www.shellcheck.net/wiki/SC2219

@Mte90
Copy link
Member

Mte90 commented Jun 12, 2024

So looking at the error below, everytime Amber print a variable it should wrap them. In this way various error are automatically fixed.

For cd is more difficult as requires something to add stuff based on the command parsed. Maybe this can be fixed with #178

I think that it will be cool if the bash generated is ShellCheck "approved".

@Ph0enixKM Ph0enixKM changed the title ✨ [Feature request] Integration with shellcheck ✨ Integration with shellcheck Jun 16, 2024
@Mte90 Mte90 added the enhancement New feature or request label Jun 24, 2024
@Mte90
Copy link
Member

Mte90 commented Jul 4, 2024

Another Amber example:

move_to_bin(get_download_path("artempyanykh/marksman", 1), "marksman")

Generate this bash code:

 move_to_bin__41_v0 "${__AF_get_download_path40_v0__39_13}" "marksman";
 __AF_move_to_bin41_v0__39_1=$__AF_move_to_bin41_v0;
 echo $__AF_move_to_bin41_v0__39_1 > /dev/null 2>&1

For shellcheck the last like should be:

echo "$__AF_move_to_bin41_v0__39_1" > /dev/null 2>&1

@Ph0enixKM
Copy link
Collaborator

Ph0enixKM commented Jul 4, 2024

We should add shellcheck tests for Amber compiled code and stdlib tests @Mte90

@Mte90
Copy link
Member

Mte90 commented Jul 4, 2024

Yes when we will address all of them is something we can do

@Mte90
Copy link
Member

Mte90 commented Jul 9, 2024

So seems that it is possible to use Shellcheck to autofix the errors that find it.

shellcheck -f diff file.sh | git apply

Basically generating a diff file and using git to apply, so in the meantime we can integrate it if the machine has the tool.
That integration should include also some tests that let us spot the issue that we didn't addressed yet in Amber.

@Ph0enixKM
Copy link
Collaborator

@Mte90 I want to make it right. Let's make shellcheck tests on the bash code that gets produced by the Amber validity and stdlib tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants