From 3d03b0ab3b818ccf51bc38fa7ca19dc2475c17f6 Mon Sep 17 00:00:00 2001
From: Vidar Holen <spam@vidarholen.net>
Date: Tue, 28 Aug 2018 20:15:54 -0700
Subject: [PATCH] Suggest -z/-n instead of ! -n/-z (fixes #1326).

---
 CHANGELOG.md                |  1 +
 src/ShellCheck/Analytics.hs | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7aeec4c..33f9338 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## ???
 ### Added
 - Command line option --severity/-S for filtering by minimum severity
+- SC2236/SC2237: Suggest -n/-z instead of ! -z/-n
 ### Changed
 - Most warnings now have useful end positions
 - SC1117 about unknown double-quoted escape sequences has been retired
diff --git a/src/ShellCheck/Analytics.hs b/src/ShellCheck/Analytics.hs
index 365dd85..738f033 100644
--- a/src/ShellCheck/Analytics.hs
+++ b/src/ShellCheck/Analytics.hs
@@ -168,6 +168,7 @@ nodeChecks = [
     ,checkPipeToNowhere
     ,checkForLoopGlobVariables
     ,checkSubshelledTests
+    ,checkInvertedStringTest
     ]
 
 
@@ -3004,5 +3005,26 @@ checkSubshelledTests params t =
             T_Annotation {} -> True
             _ -> False
 
+prop_checkInvertedStringTest1 = verify checkInvertedStringTest "[ ! -z $var ]"
+prop_checkInvertedStringTest2 = verify checkInvertedStringTest "! [[ -n $var ]]"
+prop_checkInvertedStringTest3 = verifyNot checkInvertedStringTest "! [ -x $var ]"
+prop_checkInvertedStringTest4 = verifyNot checkInvertedStringTest "[[ ! -w $var ]]"
+prop_checkInvertedStringTest5 = verifyNot checkInvertedStringTest "[ -z $var ]"
+checkInvertedStringTest _ t =
+    case t of
+        TC_Unary _ _ "!" (TC_Unary _ _ op _) ->
+            case op of
+                "-n" -> style (getId t) 2236 "Use -z instead of ! -n."
+                "-z" -> style (getId t) 2236 "Use -n instead of ! -z."
+                _ -> return ()
+        T_Banged _ (T_Pipeline _ _
+          [T_Redirecting _ _ (T_Condition _ _ (TC_Unary _ _ op _))]) ->
+            case op of
+                "-n" -> style (getId t) 2237 "Use [ -z .. ] instead of ! [ -n .. ]."
+                "-z" -> style (getId t) 2237 "Use [ -n .. ] instead of ! [ -z .. ]."
+                _ -> return ()
+        _ -> return ()
+
+
 return []
 runTests =  $( [| $(forAllProperties) (quickCheckWithResult (stdArgs { maxSuccess = 1 }) ) |])