feat: improve console output by using csgrep -U

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 · Aug 23, 2023 · 070cb3b · 070cb3b
1 parent f994631
commit 070cb3b
Show file tree

Hide file tree

Showing 26 changed files with 845 additions and 229 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -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

diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -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.

diff --git a/src/index.sh b/src/index.sh
@@ -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
 
@@ -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=$?
 

diff --git a/src/summary.sh b/src/summary.sh
@@ -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
@@ -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

diff --git a/src/validation.sh b/src/validation.sh
@@ -1,6 +1,9 @@
 # shellcheck shell=bash
 # SPDX-License-Identifier: GPL-3.0-or-later
 
+# shellcheck source=summary.sh
+. "${SCRIPT_DIR=}summary.sh"
+
 # 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
@@ -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
@@ -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
 
@@ -58,25 +62,23 @@ 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
+# 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
@@ -90,6 +92,6 @@ get_number_of_defects_by_severity () {
 
   [[ -f "${logs}" ]] || return 1
   # the optional group is workaround for csdiff issue: https://github.com/csutils/csdiff/issues/138
-  defects=$(grep --count --extended-regexp "^[^:]+:[0-9]+:([0-9]+:)? ${severity}\[SC[0-9]+\].*$" "${logs}")
+  defects=$(grep --count --extended-regexp "^\s*\"event\": \"${severity}\[SC[0-9]+\]\",$" "${logs}")
   echo "${defects}"
 }
diff --git a/test/Dockerfile b/test/Dockerfile
@@ -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

diff --git a/test/diff_scan_summary.bats b/test/diff_scan_summary.bats
@@ -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

diff --git a/test/evaluate_and_print_defects.bats b/test/evaluate_and_print_defects.bats
@@ -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!"

diff --git a/test/evaluate_and_print_fixes.bats b/test/evaluate_and_print_fixes.bats
@@ -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" {

diff --git a/test/fixtures/evaluate_and_print_defects/defects.log b/test/fixtures/evaluate_and_print_defects/defects.log
@@ -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
+                }
+            ]
+        }
+    ]
+}
diff --git a/test/fixtures/evaluate_and_print_fixes/fixes.log b/test/fixtures/evaluate_and_print_fixes/fixes.log
@@ -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
+                }
+            ]
+        }
+    ]
+}
diff --git a/test/fixtures/gather_statistics/defects.log b/test/fixtures/gather_statistics/defects.log
@@ -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
+                }
+            ]
+        }
+    ]
+}
diff --git a/test/fixtures/get_defects/base.err b/test/fixtures/get_defects/base.err
@@ -1,4 +1,15 @@
-src/index.sh:7:3: note: Not following: functions.sh: openBinaryFile: does not exist (No such file or directory) [SC1091]
-src/index.sh:39:42: warning: BASE is referenced but not assigned. [SC2154]
-src/index.sh:39:53: warning: HEAD is referenced but not assigned. [SC2154]
-src/index.sh:50:10: warning: VERSIONS_HEADING is referenced but not assigned. [SC2154]
+{
+  "comments": [
+    {
+      "file": "innocent-script.sh",
+      "line": 6,
+      "endLine": 6,
+      "column": 1,
+      "endColumn": 11,
+      "level": "warning",
+      "code": 2034,
+      "message": "UNUSED_VAR appears unused. Verify use (or export if used externally).",
+      "fix": null
+    }
+  ]
+}