knrafto / language-bash

Parse and pretty-print Bash shell scripts
BSD 3-Clause "New" or "Revised" License
35 stars 9 forks source link

Double-quoted strings are not parsed by parseTestExpr #12

Closed pbiggar closed 8 years ago

pbiggar commented 8 years ago

Again, wondering if this is a design decision?

When I call parseTestExpr on a list of words (the '[' and ']' have already been stripped off), the words are not parsed and double-quoted strings with command substitutions in them are not processed.

How were you thinking this should be accomplished?

convertTest :: [W.Word] -> Expr
convertTest ws = case condExpr of
    Left  err -> Debug $ "doesn't parse" ++ (show err) ++ (show strs)
    Right e -> convertCommand e
    where condExpr = C.parseTestExpr strs
          strs = (map W.unquote ws)
knrafto commented 8 years ago

That's because unquote strips off all double quotes and command substitutions. Unfortunately parseTestExpr takes a list of plain Strings because the Bash shell only parses the expression after command substitutions etc. are evaluated. You could process the Words another way into strings, or adapt the parser to take Words instead of Strings.

pbiggar commented 8 years ago

Cool. That got me going in the right direction, thanks. There's a few gotchas, so I've included my code and some notes below for anyone who ventures this way again.

-- | Tests (handles test, '[' and '[[')
convertTest :: [W.Word] -> Expr

-- convertTest receives a list of words. Bash evaluates many of those words (expanding arguments
-- and parameters, etc), before parsing them and passing them to `test`. That means that the 
-- run-time values are important, and it's impossible to parse it statically.
-- For example, what does `[ "$x" a ]` do?

-- parseTestExpr is really designed for run-time use, and doesn't produce parsed
-- words. For example [ "x" = "`uname`" ] won't result in a CommandSubst with
-- "uname" in it, because Bash doesn't actually do that, semantically. But
-- that's what we want!

-- Another apporach is to wrap the string in [[. Unfortunately, [[ doesn't actually work the same as [, for example -a doesn't work the same.

-- I think the correct approach is to parse it, then reparse the words again.
convertTest ws = case condExpr of
    Left  err -> Debug $ "doesn't parse" ++ (show err) ++ (show hacked)
    Right e -> (convertCondExpr (convertStrCondExpr2WordCondExpr e))
    where condExpr = C.parseTestExpr strs
          strs = (map W.unquote hacked)
          hacked = hackTestExpr ws

-- | break the bash rules to fix up mistakes
hackTestExpr :: [W.Word] -> [W.Word]
-- [ -a asd ] works, but [ ! -a asd] doesnt because -a is the "and" operator. -e
-- does the same though.
hackTestExpr ws@(n:a:rest)
  | n == W.fromString "!" && a == W.fromString "-a" = (n:(W.fromString "-e"):rest)
  | otherwise = ws
hackTestExpr ws = ws

-- parseTestExpr gives a CondExpr string, not a CondExpr Word
convertStrCondExpr2WordCondExpr :: C.CondExpr String -> C.CondExpr W.Word
convertStrCondExpr2WordCondExpr = csce2wce
csce2wce :: C.CondExpr String -> C.CondExpr W.Word
csce2wce (C.Unary uop a) = C.Unary uop (parseString2Word a)
csce2wce (C.Binary a bop b) = C.Binary (parseString2Word a) bop (parseString2Word b)
csce2wce (C.Not a) = C.Not (csce2wce a)
csce2wce (C.And a b) = C.And (csce2wce a) (csce2wce b)
csce2wce (C.Or a b) = C.Or (csce2wce a) (csce2wce b)

parseString2Word :: String -> W.Word
parseString2Word w = word
    where
      word = case BashParse.parse "source" source of
            Left err ->
              error ("nested parse of " ++ source ++ " failed: " ++ (show err))
            Right (S.List []) -> W.fromString ""
            Right (S.List
                   [S.Statement
                    (S.Last
                     (S.Pipeline {S.commands=[S.Command
                                              (S.SimpleCommand [] [x]) []]}))
                   _]) ->
              x
            Right e ->
              error ("nested parse of " ++ source ++ " was unexpected: " ++ (show e))
      source = BashPretty.prettyText w
knrafto commented 8 years ago

Glad you found a workaround!

It looks like convertStrCondExpr2WordCondExpr is equivalent to fmap parseString2Word, since CondExpr has a Functor instance.

Actually, you can use Language.Bash.Parse.Word.word directly to parse words:

parseString2Word :: String -> Either ParseError Word
parseString2Word s = Text.Parsec.parse Language.Bash.Parse.Word.word s s

then since CondExpr is Traversable,

traverse parseString2Word :: CondExpr String -> Either ParseError (CondExpr Word)
pbiggar commented 8 years ago

Thanks, this really simplified things! Haskell beginner here, and new to Functors and Traversable. Pretty cool!