Improve parsing for ambiguous $((foo) ) and ((foo) ).

ShellCheck/Parser.hs

@@ -1078,13 +1078,18 @@ subParse pos parser input = do
     setPosition lastPosition
     return result
 
-inSeparateContext parser = do
+-- Parse something, but forget all parseProblems
+inSeparateContext = parseForgettingContext True
+-- Parse something, but forget all parseProblems on failure
+forgetOnFailure = parseForgettingContext False
+
+parseForgettingContext alsoOnSuccess parser = do
     context <- Ms.get
     success context <|> failure context
   where
     success c = do
         res <- try parser
-        Ms.put c
+        when alsoOnSuccess $ Ms.put c
         return res
     failure c = do
         Ms.put c
@@ -1321,7 +1326,12 @@ readDoubleQuotedDollar = readDollarExpression <|> readDollarLonely
 
 prop_readDollarExpression1 = isOk readDollarExpression "$(((1) && 3))"
 prop_readDollarExpression2 = isWarning readDollarExpression "$(((1)) && 3)"
-readDollarExpression = readTripleParenthesis "$" readDollarArithmetic readDollarExpansion <|> readDollarArithmetic <|> readDollarBracket <|> readDollarBraceCommandExpansion <|> readDollarBraced <|> readDollarExpansion <|> readDollarVariable
+prop_readDollarExpression3 = isWarning readDollarExpression "$((\"$@\" &); foo;)"
+readDollarExpression :: Monad m => SCParser m Token
+readDollarExpression = arithmetic <|> readDollarExpansion <|> readDollarBracket <|> readDollarBraceCommandExpansion <|> readDollarBraced <|> readDollarVariable
+  where
+    arithmetic = readAmbiguous "$((" readDollarArithmetic readDollarExpansion (\pos ->
+        parseNoteAt pos WarningC 1102 "Shells disambiguate $(( differently or not at all. If the first $( should start command substitution, add a space after it.")
 
 prop_readDollarSingleQuote = isOk readDollarSingleQuote "$'foo\\\'lol'"
 readDollarSingleQuote = called "$'..' expression" $ do
@@ -1367,25 +1377,20 @@ readArithmeticExpression = called "((..)) command" $ do
     string "))"
     return (T_Arithmetic id c)
 
--- Check if maybe ((( was intended as ( (( rather than (( (
+-- If the next characters match prefix, try two different parsers and warn if the alternate parser had to be used
-readTripleParenthesis prefix expected alternative = do
+readAmbiguous :: Monad m => String -> SCParser m p -> SCParser m p -> (SourcePos -> SCParser m ()) -> SCParser m p
-    pos <- try . lookAhead $ do
+readAmbiguous prefix expected alternative warner = do
-        string prefix
+    pos <- getPosition
-        p <- getPosition
+    try . lookAhead $ string prefix
-        string "(((" -- should optimally be "((" but it's noisy and rarely helpful
-        return p
-
     -- If the expected parser fails, try the alt.
     -- If the alt fails, run the expected one again for the errors.
-    try expected <|> tryAlt pos <|> expected
+    try (forgetOnFailure expected) <|> try (withAlt pos) <|> expected
   where
-    tryAlt pos = do
+    withAlt pos = do
-        t <- try alternative
+        t <- forgetOnFailure alternative
-        parseNoteAt pos WarningC 1102 $
+        warner pos
-            "Shells differ in parsing ambiguous " ++ prefix ++ "(((. Use spaces: " ++ prefix ++ "( (( ."
         return t
 
-
 prop_readDollarBraceCommandExpansion1 = isOk readDollarBraceCommandExpansion "${ ls; }"
 prop_readDollarBraceCommandExpansion2 = isOk readDollarBraceCommandExpansion "${\nls\n}"
 readDollarBraceCommandExpansion = called "ksh ${ ..; } command expansion" $ do
@@ -2194,8 +2199,9 @@ readCompoundCommand = do
     id <- getNextId
     cmd <- choice [
         readBraceGroup,
-        readTripleParenthesis "" readArithmeticExpression readSubshell,
+        readAmbiguous "((" readArithmeticExpression readSubshell (\pos ->
-        readArithmeticExpression,
+            parseNoteAt pos WarningC 1105 "Shells disambiguate (( differently or not at all. If the first ( should start a subshell, add a space after it."),
+        --readArithmeticExpression,
         readSubshell,
         readCondition,
         readWhileClause,
@@ -2269,9 +2275,7 @@ readEvalSuffix = many1 (readIoRedirect <|> readCmdWord <|> evalFallback)
 
 -- Get whatever a parser would parse as a string
 readStringForParser parser = do
-    state <- Ms.get
+    pos <- inSeparateContext $ lookAhead (parser >> getPosition)
-    pos <- lookAhead (parser >> getPosition)
-    Ms.put state
     readUntil pos
   where
     readUntil endPos = anyChar `reluctantlyTill` (getPosition >>= guard . (== endPos))