koalaman / shellcheck

ShellCheck, a static analysis tool for shell scripts
https://www.shellcheck.net
GNU General Public License v3.0
36.23k stars 1.77k forks source link

Enhancement: Warn about irrelevant directives #1719

Open matthewpersico opened 5 years ago

matthewpersico commented 5 years ago

For bugs

For new checks and feature suggestions

Here's a snippet or screenshot that shows the problem:

#!/usr/bin/env bash

# This one properly complains about SC2012
ls -la "$@"| zenity --text-info --auto-scroll --width=1320 --height=800 --title "ls -la $*" &

# If this used to be the ls command and we corrected it, SC2012 no longer applies.
# shellcheck disable=SC2012 #https://github.com/koalaman/shellcheck/wiki/SC2012
find "$1" -maxdepth 1 -name "$2" -ls | zenity --text-info --auto-scroll --width=1320 --height=800 --title "find $1 -maxdepth 1 -name $2 -ls" &

Here's what shellcheck currently says:

Line 4  SC2012: Use find instead of ls to better handle non-alphanumeric filenames.

Here's what I wanted or expected to see:

Line 4  SC2012: Use find instead of ls to better handle non-alphanumeric filenames.
Line 8  SC2012: Directive does not apply. Perhaps you corrected the code and forgot to remove the directive?
silverwind commented 1 month ago

This is much needed. Ineffective shellcheck disable statements do mask bugs.

There should be a separate rule that searches for ineffective shellcheck disable statements, e.g. not the same rule number like OP suggested, so that it can be disabled, if so desired.