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

Shown alerts should include a possible solution (if available) #88

Open
mrc0mmand opened this issue Aug 16, 2022 · 6 comments · Fixed by #300
Open

Shown alerts should include a possible solution (if available) #88

mrc0mmand opened this issue Aug 16, 2022 · 6 comments · Fixed by #300
Assignees
Labels

Comments

@mrc0mmand
Copy link
Member

Type of issue

Feature Request

Description

ShellCheck provides possible solutions for reported issues in the Did you mean section, e.g.:

In /github/workspace/src/partition/test-repart.sh line 250:
JSON_OUTPUT=$("$repart" --definitions="$D/definitions-overrides" --dry-run=yes --empty=create --size=100M --json=pretty $D/test-drop-in-image)
                                                                                                                        ^-- SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
JSON_OUTPUT=$("$repart" --definitions="$D/definitions-overrides" --dry-run=yes --empty=create --size=100M --json=pretty "$D"/test-drop-in-image)

which the currently shown alerts don't include:

image

Describe the solution you'd like

Having the suggested solution as part of the alert would be great, maybe even in some diff-aware form, so the difference is clearly visible.

@jamacku
Copy link
Member

jamacku commented Aug 17, 2022

We are aware of this issue. @lzaoral is already working on solution in:

Sorry for the inconvenience.

@lzaoral
Copy link
Member

lzaoral commented Aug 17, 2022

csutils/csdiff#68 is a different issue. csdiff relies on ShellCheck's GCC output formatter which, unfortunately, does not include suggested fixes. [1]

[1] https://www.shellcheck.net/wiki/Integration#GCC-compatible-error-messages

@jamacku
Copy link
Member

jamacku commented Aug 10, 2023

I'm currently working on getting sarif-fmt package to Fedora. It could improve the current situation once available.

Example of output of sarif-fmt for ShellCheck:

$ shellcheck --format=json shell.sh | shellcheck-sarif | sarif-fmt
note: Use $(...) notation instead of legacy backticks `...`.
   ┌─ innocent-script.sh:15:15
   │
15 │ combined_file=`cat ${files}`
   │               ^^^^^^^^^^^^^^
   │
   ┌─ innocent-script.sh:15:15
   │
15 │ combined_file=`cat ${files}`
   │               -
   │
   ┌─ innocent-script.sh:15:28
   │
15 │ combined_file=`cat ${files}`
   │                            -
   │
   = SC2006
   = For more information: https://www.shellcheck.net/wiki/SC2006

Note: to make it work, we have to start using ShellCheck JSON format instead of GCC since GCC format is missing important information about fixes and defect position

@jamacku jamacku pinned this issue Aug 11, 2023
@jamacku jamacku added this to the v5.0.0 milestone Aug 14, 2023
jamacku added a commit to jamacku/differential-shellcheck that referenced this issue Aug 23, 2023
Show more context for ShellCheck defects and fixes in console output.
Using the `csgrep --embed-context` option allows us to show a part of
the code in which a defect is reported. This is much more user-friendly
than the current plain GCC output format.

This commit also fixes issues with statistics calculations.

Related to: redhat-plumbers-in-action#88
jamacku added a commit to jamacku/differential-shellcheck that referenced this issue Aug 23, 2023
Show more context for ShellCheck defects and fixes in console output.
Using the `csgrep --embed-context` option allows us to show a part of
the code in which a defect is reported. This is much more user-friendly
than the current plain GCC output format.

This commit also fixes issues with statistics calculations.

Related to: redhat-plumbers-in-action#88
jamacku added a commit that referenced this issue Aug 23, 2023
Show more context for ShellCheck defects and fixes in console output.
Using the `csgrep --embed-context` option allows us to show a part of
the code in which a defect is reported. This is much more user-friendly
than the current plain GCC output format.

This commit also fixes issues with statistics calculations.

Related to: #88
@jamacku jamacku linked a pull request Aug 23, 2023 that will close this issue
4 tasks
@lzaoral lzaoral removed their assignment Sep 13, 2023
@jamacku jamacku removed this from the v5.0.0 milestone Oct 3, 2023
@jamacku
Copy link
Member

jamacku commented Apr 19, 2024

Following PR should make the situation a bit better:

@jamacku jamacku linked a pull request Apr 19, 2024 that will close this issue
@jamacku
Copy link
Member

jamacku commented Apr 29, 2024

Possible solution would be implementation of:

@mpoberezhniy
Copy link
Contributor

There is an action to add suggestions based on a git diff: reviewdog/action-suggester. It already implements filtering changes related to a change set so it can be used with shellcheck directly. It modifies all the lines that are available in PR context (as diff by default shows 3 lines of context and GitHub allows commenting on those lines).

I think having a write option in differential-shellcheck may be beneficial for local development when dealing with large files. It would allow applying the fixes only to the changed lines and allow using differential-shellcheck action with action-suggester.

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