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

Show more context for ShellCheck defects and fixes in console output 💾 #300

Merged
merged 4 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ ARG rpm_shellcheck="ShellCheck-${version_shellcheck}.fc${fedora}.${arch}.rpm"
# --- Install dependencies --- #

RUN dnf -y upgrade
RUN dnf -y install git koji \
RUN dnf -y install git koji jq \
&& dnf clean all

# Download rpms from koji
Expand Down
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

* Added defect statistics based on severity levels. They are available in the console output and in the job Summary page.
* New option `scan-directory`. Allows to specify directories that will be scanned. By default Differential ShellCheck scans the whole repository.
* Show more context for ShellCheck defects and fixes in console output. The defect is now shown in the context of the surrounding code.
* Fix autodetection of shell scripts in DEBUG mode
* Fix detection of changed files that might cause failure on paths with special characters.
* Fix count of scanned files in job Summary when running on push event.
Expand Down
5 changes: 2 additions & 3 deletions src/functions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -277,15 +277,14 @@ execute_shellcheck () {
is_true "${INPUT_EXTERNAL_SOURCES}" && local external_sources=--external-sources

local shellcheck_args=(
--format=gcc
--format=json1
"${external_sources:-}"
--severity="${INPUT_SEVERITY}"
"${@}"
)

# The sed part ensures that cstools will recognize the output as being produced by ShellCheck and not GCC.
local output
output=$(shellcheck "${shellcheck_args[@]}" 2> /dev/null | sed -e 's|$| <--[shellcheck]|')
output=$(shellcheck "${shellcheck_args[@]}" 2> /dev/null)

echo "${output}"
}
Expand Down
5 changes: 4 additions & 1 deletion src/index.sh
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ if ! is_strict_check_on_push_demanded; then
execute_shellcheck "${only_changed_scripts[@]}" > ../head-shellcheck.err

# Checkout the base branch/commit
git checkout --force -q -b ci_br_dest "${BASE}"
git checkout --force --quiet -b ci_br_dest "${BASE}"

execute_shellcheck "${only_changed_scripts[@]}" > ../base-shellcheck.err

Expand All @@ -81,6 +81,9 @@ fi

echo

# Checkout the head branch/commit, it's required in order to correctly display defects in console
git checkout --force --quiet -

evaluate_and_print_defects
exit_status=$?

Expand Down
8 changes: 4 additions & 4 deletions src/summary.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Scanned/Changed scripts: \`${#list_of_changed_scripts[@]}\`
get_number_of () {
[[ $# -le 0 ]] && return 1

grep -Eo "[0-9]*" < <(csgrep --mode=stat ../"${1}".log)
jq '.defects | length' "../${1}.log"
}

# Create full Markdown style link to results
Expand Down Expand Up @@ -123,9 +123,9 @@ summary_defect_statistics () {
echo -e "\
#### New defects statistics

| | 👕 Style | 🗒️ Note | ⚠️ Warning | 🛑 Error |
|:--------:|:------------------------:|:-----------------------:|:--------------------------:|:------------------------:|
| 🔢 Count | **${stat_style:-"N/A"}** | **${stat_note:-"N/A"}** | **${stat_warning:-"N/A"}** | **${stat_error:-"N/A"}** |"
| | 👕 Style / 🗒️ Note | ⚠️ Warning | 🛑 Error |
|:--------:|:-----------------------:|:--------------------------:|:------------------------:|
| 🔢 Count | **${stat_info:-"N/A"}** | **${stat_warning:-"N/A"}** | **${stat_error:-"N/A"}** |"
}

# Print useful information at the end of summary report
Expand Down
27 changes: 15 additions & 12 deletions src/validation.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# shellcheck shell=bash
# SPDX-License-Identifier: GPL-3.0-or-later

# shellcheck source=summary.sh
. "${SCRIPT_DIR=}summary.sh"

Check warning

Code scanning / shellcheck

Not following: summary.sh: openBinaryFile: does not exist (No such file or directory)

Not following: summary.sh: openBinaryFile: does not exist (No such file or directory)

# Get file containing fixes based on two input files
# $1 - <string> absolute path to a file containing results from BASE scan
# $2 - <string> absolute path to a file containing results from HEAD scan
Expand All @@ -18,7 +21,7 @@ get_fixes () {
evaluate_and_print_fixes () {
if [[ -s ../fixes.log ]]; then
echo -e "✅ ${GREEN}Fixed defects${NOCOLOR}"
csgrep ../fixes.log
csgrep --embed-context 2 ../fixes.log
else
echo -e "ℹ️ ${YELLOW}No Fixes!${NOCOLOR}"
fi
Expand All @@ -41,11 +44,12 @@ get_defects () {
evaluate_and_print_defects () {
gather_statistics "../defects.log"

if [[ -s ../defects.log ]]; then
num_of_defects=$(get_number_of defects)
if [[ -s ../defects.log ]] && [[ "${num_of_defects}" -gt 0 ]] ; then
print_statistics

echo -e "✋ ${YELLOW}Defects, NEEDS INSPECTION${NOCOLOR}"
csgrep ../defects.log
csgrep --embed-context 4 ../defects.log
return 1
fi

Expand All @@ -58,37 +62,36 @@ print_statistics () {
echo -e "::group::📊 ${WHITE}Statistics of defects${NOCOLOR}"
[[ -n ${stat_error} ]] && echo -e "Error: ${stat_error}"
[[ -n ${stat_warning} ]] && echo -e "Warning: ${stat_warning}"
[[ -n ${stat_note} ]] && echo -e "Note: ${stat_note}"
[[ -n ${stat_style} ]] && echo -e "Style: ${stat_style}"
[[ -n ${stat_info} ]] && echo -e "Style or Note: ${stat_info}"
echo "::endgroup::"
echo
}

# Function to filter out defects by their severity level
# It sets global variables stat_error, stat_warning, stat_note, stat_style depending on INPUT_SEVERITY
# $1 - <string> absolute path to a file containing defects detected by scan in gcc format
# It sets global variables stat_error, stat_warning, stat_info depending on INPUT_SEVERITY
# $1 - <string> absolute path to a file containing defects detected by scan
gather_statistics () {
[[ $# -le 0 ]] && return 1
local logs="$1"

[[ ${INPUT_SEVERITY-} == "style" ]] && stat_style=$(get_number_of_defects_by_severity "style" "${logs}")
[[ ${INPUT_SEVERITY} =~ style|note ]] && stat_note=$(get_number_of_defects_by_severity "note" "${logs}")
[[ ${INPUT_SEVERITY-} =~ style|note ]] && stat_info=$(get_number_of_defects_by_severity "info" "${logs}")
[[ ${INPUT_SEVERITY} =~ style|note|warning ]] && stat_warning=$(get_number_of_defects_by_severity "warning" "${logs}")
[[ ${INPUT_SEVERITY} =~ style|note|warning|error ]] && stat_error=$(get_number_of_defects_by_severity "error" "${logs}")

export stat_style stat_note stat_warning stat_error
export stat_info stat_warning stat_error
}

# Function to get number of defects by severity level
# $1 - <string> severity level
# $2 - <string> absolute path to a file containing defects detected by scan in gcc format
# $2 - <string> absolute path to a file containing defects detected by scan
get_number_of_defects_by_severity () {
[[ $# -le 1 ]] && return 1
local severity="$1"
local logs="$2"
local defects=0

[[ -f "${logs}" ]] || return 1
defects=$(grep --count --extended-regexp "^[^:]+:[0-9]+:[0-9]+: ${severity}\[SC[0-9]+\].*$" "${logs}")
# the optional group is workaround for csdiff issue: https://github.com/csutils/csdiff/issues/138
defects=$(grep --count --extended-regexp "^\s*\"event\": \"${severity}\[SC[0-9]+\]\",$" "${logs}")
echo "${defects}"
}
2 changes: 1 addition & 1 deletion test/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ ARG rpm_shellcheck="ShellCheck-${version_shellcheck}.fc${fedora}.${arch}.rpm"
# --- Install dependencies --- #

RUN dnf -y upgrade
RUN dnf -y install git koji kcov bats diffutils \
RUN dnf -y install git koji kcov bats diffutils jq \
&& dnf clean all

# Download rpms from koji
Expand Down
51 changes: 39 additions & 12 deletions test/diff_scan_summary.bats
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,47 @@ setup () {
INPUT_TRIGGERING_EVENT=""

echo -e \
"Error: SHELLCHECK_WARNING:
src/index.sh:53:10: warning[SC2154]: MAIN_HEADING is referenced but not assigned.

Error: SHELLCHECK_WARNING:
src/index.sh:56:14: warning[SC2154]: WHITE is referenced but not assigned.

Error: SHELLCHECK_WARNING:
src/index.sh:56:56: warning[SC2154]: NOCOLOR is referenced but not assigned.
" > ../defects.log
'{
"defects": [
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 7,
"event": "warning[SC2034]",
"message": "UNUSED_VAR2 appears unused. Verify use (or export if used externally).",
"verbosity_level": 0
}
]
},
{}, {}
]
}' > ../defects.log

echo -e \
"Error: SHELLCHECK_WARNING:
src/index.sh:7:3: note[SC1091]: Not following: functions.sh: openBinaryFile: does not exist (No such file or directory)
" > ../fixes.log
'{
"defects": [
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 6,
"event": "warning[SC2034]",
"message": "UNUSED_VAR appears unused. Verify use (or export if used externally).",
"verbosity_level": 0
}
]
}
]
}' > ../fixes.log

run diff_scan_summary
assert_success
Expand Down
26 changes: 11 additions & 15 deletions test/evaluate_and_print_defects.bats
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,24 @@ setup () {

run evaluate_and_print_defects
assert_failure 1
assert_output "\
assert_output --partial "\
::group::📊 Statistics of defects
Error: 0
Warning: 3
Note: 0
Style: 0
::endgroup::

✋ Defects, NEEDS INSPECTION
Error: SHELLCHECK_WARNING:
src/index.sh:53:10: warning[SC2154]: MAIN_HEADING is referenced but not assigned.

Error: SHELLCHECK_WARNING:
src/index.sh:56:14: warning[SC2154]: WHITE is referenced but not assigned.

Error: SHELLCHECK_WARNING:
src/index.sh:56:56: warning[SC2154]: NOCOLOR is referenced but not assigned."
Warning: 2
Style or Note: 0
::endgroup::"
assert_line --partial "innocent-script.sh:7: warning[SC2034]: UNUSED_VAR2 appears unused. Verify use (or export if used externally)."
assert_line --partial 'innocent-script.sh:11: warning[SC2115]: Use "${var:?}" to ensure this never expands to / .'
}

@test "evaluate_and_print_defects() - no defects" {
source "${PROJECT_ROOT}/src/validation.sh"

echo -e \
'{
"defects": []
}' > ../defects.log

run evaluate_and_print_defects
assert_success
assert_output "🥳 No defects added. Yay!"
Expand Down
5 changes: 2 additions & 3 deletions test/evaluate_and_print_fixes.bats
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ setup () {

run evaluate_and_print_fixes
assert_success
assert_output "\
✅ Fixed defects
assert_output --partial "✅ Fixed defects
Error: SHELLCHECK_WARNING:
src/index.sh:7:3: note[SC1091]: Not following: functions.sh: openBinaryFile: does not exist (No such file or directory)"
innocent-script.sh:6: warning[SC2034]: UNUSED_VAR appears unused. Verify use (or export if used externally)."
}

@test "evaluate_and_print_fixes() - no fixes" {
Expand Down
42 changes: 34 additions & 8 deletions test/fixtures/evaluate_and_print_defects/defects.log
Original file line number Diff line number Diff line change
@@ -1,8 +1,34 @@
Error: SHELLCHECK_WARNING:
src/index.sh:53:10: warning[SC2154]: MAIN_HEADING is referenced but not assigned.

Error: SHELLCHECK_WARNING:
src/index.sh:56:14: warning[SC2154]: WHITE is referenced but not assigned.

Error: SHELLCHECK_WARNING:
src/index.sh:56:56: warning[SC2154]: NOCOLOR is referenced but not assigned.
{
"defects": [
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 7,
"event": "warning[SC2034]",
"message": "UNUSED_VAR2 appears unused. Verify use (or export if used externally).",
"verbosity_level": 0
}
]
},
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 11,
"event": "warning[SC2115]",
"message": "Use \"${var:?}\" to ensure this never expands to / .",
"verbosity_level": 0
}
]
}
]
}
21 changes: 19 additions & 2 deletions test/fixtures/evaluate_and_print_fixes/fixes.log
Original file line number Diff line number Diff line change
@@ -1,2 +1,19 @@
Error: SHELLCHECK_WARNING:
src/index.sh:7:3: note[SC1091]: Not following: functions.sh: openBinaryFile: does not exist (No such file or directory)
{
"defects": [
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 6,
"event": "warning[SC2034]",
"message": "UNUSED_VAR appears unused. Verify use (or export if used externally).",
"verbosity_level": 0
}
]
}
]
}
42 changes: 34 additions & 8 deletions test/fixtures/gather_statistics/defects.log
Original file line number Diff line number Diff line change
@@ -1,8 +1,34 @@
Error: SHELLCHECK_WARNING:
src/index.sh:53:10: warning[SC2154]: MAIN_HEADING is referenced but not assigned.

Error: SHELLCHECK_WARNING:
src/index.sh:56:14: warning[SC2154]: WHITE is referenced but not assigned.

Error: SHELLCHECK_WARNING:
src/index.sh:56:56: warning[SC2154]: NOCOLOR is referenced but not assigned.
{
"defects": [
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 7,
"event": "warning[SC2034]",
"message": "UNUSED_VAR2 appears unused. Verify use (or export if used externally).",
"verbosity_level": 0
}
]
},
{
"checker": "SHELLCHECK_WARNING",
"language": "shell",
"tool": "shellcheck",
"key_event_idx": 0,
"events": [
{
"file_name": "innocent-script.sh",
"line": 11,
"event": "warning[SC2115]",
"message": "Use \"${var:?}\" to ensure this never expands to / .",
"verbosity_level": 0
}
]
}
]
}
Loading