Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mis-parsing GraphQL identifiers when it starts with keywords (fix #20) #21

Merged
merged 12 commits into from
Feb 24, 2020
1 change: 1 addition & 0 deletions package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ benchmarks:
tests:
graphql-parser-test:
main: Spec.hs
other-modules: PrimitiveColumns
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the hspec format, I don't think we have to include them like this. Do you think this is necessary?

source-dirs: test
ghc-options:
- -threaded
Expand Down
41 changes: 29 additions & 12 deletions src/Language/GraphQL/Draft/Parser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module Language.GraphQL.Draft.Parser

, value
, parseValueConst
, nameParser

, graphQLType
, parseGraphQLType
Expand All @@ -24,6 +25,7 @@ import Protolude hiding (option)

import Control.Applicative (many, optional, (<|>))
import Control.Monad.Fail (fail)
import Prelude hiding (fail)
import Data.Aeson.Parser (jstring)
import qualified Data.Attoparsec.ByteString as A
import Data.Attoparsec.Text (Parser, anyChar, char, many1,
Expand Down Expand Up @@ -174,7 +176,7 @@ parseValueConst = runParser valueConst
valueConst :: Parser AST.ValueConst
valueConst = tok (
(fmap (either AST.VCFloat AST.VCInt) number <?> "number")
<|> AST.VCNull <$ tok "null"
<|> AST.VCNull <$ literal "null"
<|> AST.VCBoolean <$> (booleanValue <?> "booleanValue")
<|> AST.VCString <$> (stringValue <?> "stringValue")
-- `true` and `false` have been tried before
Expand All @@ -200,7 +202,7 @@ value :: Parser AST.Value
value = tok (
AST.VVariable <$> (variable <?> "variable")
<|> (fmap (either AST.VFloat AST.VInt) number <?> "number")
<|> AST.VNull <$ tok "null"
<|> AST.VNull <$ literal "null"
<|> AST.VBoolean <$> (booleanValue <?> "booleanValue")
<|> AST.VString <$> (stringValue <?> "stringValue")
-- `true` and `false` have been tried before
Expand All @@ -211,8 +213,9 @@ value = tok (
)

booleanValue :: Parser Bool
booleanValue = True <$ tok "true"
<|> False <$ tok "false"
booleanValue
= True <$ literal "true"
<|> False <$ literal "false"

stringValue :: Parser AST.StringValue
stringValue = do
Expand Down Expand Up @@ -411,6 +414,20 @@ tok :: AT.Parser a -> AT.Parser a
tok p = p <* whiteSpace
{-# INLINE tok #-}

literal :: AT.Parser a -> AT.Parser a
sordina marked this conversation as resolved.
Show resolved Hide resolved
literal p = p <* ends <* whiteSpace
{-# INLINE literal #-}

ends :: AT.Parser ()
ends = do
mc <- AT.peekChar
case mc of
Nothing -> pure ()
Just c ->
if isNonFirstChar c
then mzero
else pure ()

comment :: Parser ()
comment =
AT.char '#' *>
Expand Down Expand Up @@ -440,16 +457,16 @@ whiteSpace = do
nameParser :: AT.Parser AST.Name
nameParser =
AST.Name <$> tok ((<>) <$> AT.takeWhile1 isFirstChar
<*> AT.takeWhile isNonFirstChar)
where

isFirstChar x = isAsciiLower x || isAsciiUpper x || x == '_'
{-# INLINE isFirstChar #-}
<*> AT.takeWhile isNonFirstChar)
{-# INLINE nameParser #-}

isNonFirstChar x = isFirstChar x || isDigit x
{-# INLINE isNonFirstChar #-}
isFirstChar :: Char -> Bool
isFirstChar x = isAsciiLower x || isAsciiUpper x || x == '_'
{-# INLINE isFirstChar #-}

{-# INLINE nameParser #-}
isNonFirstChar :: Char -> Bool
isNonFirstChar x = isFirstChar x || isDigit x
{-# INLINE isNonFirstChar #-}

parens :: Parser a -> Parser a
parens = between "(" ")"
Expand Down
8 changes: 7 additions & 1 deletion test/Spec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,16 @@ import Language.GraphQL.Draft.Generator.Document
import Language.GraphQL.Draft.Parser (parseExecutableDoc)
import Language.GraphQL.Draft.Syntax

import PrimitiveColumns

import qualified Language.GraphQL.Draft.Printer.ByteString as PP.BB
import qualified Language.GraphQL.Draft.Printer.LazyText as PP.TLB
import qualified Language.GraphQL.Draft.Printer.Pretty as PP
import qualified Language.GraphQL.Draft.Printer.Text as PP.TB

-- import Text.Groom

import Prelude hiding (print)

data TestMode = TMDev | TMQuick | TMRelease
deriving (Show)
Expand All @@ -42,12 +47,13 @@ runTest = void . tests

tests :: TestLimit -> IO Bool
tests nTests =
checkParallel $ Group "Test.printer.parser"
checkParallel $ Group "Test.printer.parser" $
[ ("property [ parse (prettyPrint ast) == ast ]", propParserPrettyPrinter nTests)
, ("property [ parse (textBuilderPrint ast) == ast ]", propParserTextPrinter nTests)
, ("property [ parse (lazyTextBuilderPrint ast) == ast ]", propParserLazyTextPrinter nTests)
, ("property [ parse (bytestringBuilderPrint ast) == ast ]", propParserBSPrinter nTests)
]
++ PrimitiveColumns.primitiveTests

propParserPrettyPrinter :: TestLimit -> Property
propParserPrettyPrinter = mkPropParserPrinter PP.renderExecutableDoc
Expand Down
45 changes: 45 additions & 0 deletions test/primitiveColumns.hs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
{-# LANGUAGE OverloadedStrings #-}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name of the file should be PrimitiveColumns.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think we should call it something else. It is not really related to columns, but primitive values of GraphQL.


-- | Regression tests for issue #20 https://github.com/hasura/graphql-parser-hs/issues/20

module PrimitiveColumns (primitiveTests) where

import Hedgehog
import Protolude
import GHC.Base (fail)

import Language.GraphQL.Draft.Syntax
import Language.GraphQL.Draft.Parser

import qualified Language.GraphQL.Draft.Printer.Text as PP.TB
import qualified Language.GraphQL.Draft.Printer as P

primitiveTests :: IsString s => [(s, Property)]
primitiveTests =
[ ("property [ parse (print nameValue) == nameValue ]", propNullNameValue)
, ("property [ parse (print nameBool) == nameBool ]", propBoolNameValue)
, ("property [ parse (print nameName) == nameName ]", propNullNameName)
]

propNullNameValue :: Property
propNullNameValue = property $ either (fail . Protolude.show) (ast ===) astRoundTrip
where
astRoundTrip = (runParser value) printed
printed = PP.TB.render P.value ast
ast = VList (ListValueG { unListValue = [
VEnum (EnumValue{unEnumValue = Name{unName = "nullColumn"}})]})

propBoolNameValue :: Property
propBoolNameValue = property $ either (fail . Protolude.show) (ast ===) astRoundTrip
where
astRoundTrip = (runParser value) printed
printed = PP.TB.render P.value ast
ast = VList (ListValueG { unListValue = [
VEnum (EnumValue{unEnumValue = Name{unName = "trueColumn"}})]})

propNullNameName :: Property
propNullNameName = property $ either (fail . Protolude.show) (ast ===) astRoundTrip
where
astRoundTrip = (runParser nameParser) printed
printed = PP.TB.render P.nameP ast
ast = Name "nullColumntwo"