refine: improve test and report error better (#1)

src/Lib/AST/Definition.hs

@@ -26,7 +26,9 @@ pModifierDefinition :: Parser ModifierDefinition
 pModifierDefinition = do
   ident <-
     pOneKeyword "modifier"
-      *> pMany1Spaces
+      *> ( pMany1Spaces
+             <|> fail "space is required after keyword 'modifier'"
+         )
       *> pIdentifier
   args <- pManySpaces >> optionMaybe pFnDeclArgsInParentheses
 
@@ -71,7 +73,9 @@ pEventDefinition :: Parser EventDefinition
 pEventDefinition = do
   ident <-
     pOneKeyword "event"
-      *> pMany1Spaces
+      *> ( pMany1Spaces
+             <|> fail "space is required after keyword 'event'"
+         )
       *> pIdentifier
   params <-
     between
@@ -81,7 +85,7 @@ pEventDefinition = do
       )
 
   isAnonymous <- pOneKeyword "anonymous" $> True <|> return False
-  _ <- pOneKeyword semicolon
+  _ <- pManySpaces *> pOneKeyword semicolon
   return
     EventDefinition
       { eventParameters = params,
@@ -93,7 +97,9 @@ pErrorDefinition :: Parser ErrorDefinition
 pErrorDefinition = do
   ident <-
     pOneKeyword "error"
-      *> pMany1Spaces
+      *> ( pMany1Spaces
+             <|> fail "space is required after keyword 'error'"
+         )
       *> pIdentifier
       <* pManySpaces
   args <-
@@ -102,7 +108,7 @@ pErrorDefinition = do
       (pOneKeyword rightParenthesis)
       ( sepBy (pManySpaces *> pErrorParameter <* pManySpaces) (char ',')
       )
-
+  _ <- pManySpaces *> pOneKeyword semicolon
   return $
     ErrorDefinition
       { errParameters = args,
@@ -121,8 +127,8 @@ pErrorParameter = do
 
 pInheritanceSpecifier :: Parser InheritanceSpecifier
 pInheritanceSpecifier = do
-  path <- pIdentifierPath <* pManySpaces
-  args <- pFnCallArgs
+  path <- pIdentifierPath
+  args <- optionMaybe pFnCallArgs
   return $
     InheritanceSpecifier
       { inheritanceCallArgs = args,
@@ -201,26 +207,33 @@ pInterfaceDefinition = do
   name <-
     pManySpaces
       *> pOneKeyword "interface"
-      *> pMany1Spaces
+      *> ( pMany1Spaces
+             <|> fail "space is required after keyword 'interface'"
+         )
       *> pIdentifier
-      <* pMany1Spaces
-      <* pOneKeyword "is"
-      <* pMany1Spaces
+      <* pManySpaces
+
   iSpecicier <-
-    sepBy
-      (pManySpaces *> pInheritanceSpecifier <* pManySpaces)
-      (char ',')
+    optionMaybe $
+      try
+        ( pOneKeyword "is"
+            *> pMany1Spaces
+            *> sepBy
+              (pManySpaces *> pInheritanceSpecifier <* pManySpaces)
+              (char ',')
+        )
+
   body <-
     pManySpaces
       *> between
-        (pOneKeyword leftParenthesis)
-        (pOneKeyword rightParenthesis)
+        (pOneKeyword leftCurlyBrace)
+        (pOneKeyword rightCurlyBrace)
         (pManySpaces *> pContractBody <* pManySpaces)
 
   return $
     InterfaceDefinition
       { interfaceName = name,
-        interfaceInheritanceSpecifiers = iSpecicier,
+        interfaceInheritanceSpecifiers = fromMaybe [] iSpecicier,
         interfaceBody = body
       }
 
@@ -229,14 +242,14 @@ pLibraryDefinition = do
   name <-
     pManySpaces
       *> pOneKeyword "library"
-      *> pMany1Spaces
+      *> (pMany1Spaces <|> fail "space is required after keyword 'library'")
       *> pIdentifier
       <* pManySpaces
   body <-
     pManySpaces
       *> between
-        (pOneKeyword leftParenthesis)
-        (pOneKeyword rightParenthesis)
+        (pOneKeyword leftCurlyBrace)
+        (pOneKeyword rightCurlyBrace)
         (pManySpaces *> pContractBody <* pManySpaces)
   return $
     LibraryDefinition

src/Lib/AST/Function.hs

@@ -83,7 +83,6 @@ pFunctionDefinition = do
           (pOneKeyword rightCurlyBrace)
           (many $ pState <* pManySpaces)
 
-  -- why 'many anyChar' doesn't work?
   return
     ( FunctionDefinition
         { fnDefName = name,
@@ -114,7 +113,7 @@ pReturnsClause =
   do
     pManySpaces
       *> pOneKeyword keywordReturns
-      *> pMany1Spaces
+      *> pManySpaces
       *> pFunctionReturnTypeWithQuote
       <* pManySpaces
 

src/Lib/AST/Model.hs

@@ -551,7 +551,7 @@ data LibraryDefinition = LibraryDefinition
 
 data InheritanceSpecifier = InheritanceSpecifier
   { inheritancePath :: IdentifierPath,
-    inheritanceCallArgs :: FnCallArgs
+    inheritanceCallArgs :: Maybe FnCallArgs
   }
   deriving (Show, Eq)
 

src/Lib/Parser.hs

@@ -81,6 +81,8 @@ pBool =
 isUnderscore :: Char -> Bool
 isUnderscore = (== '_')
 
+-- todo: currently the pIdentifier will stop once the character could not be recognized as a vliad char
+-- for example variable&abc will stop parsing at '&', think about whether we can improve it or not
 pIdentifier :: Parser Text
 pIdentifier = do
   first <- letter <|> char '_'

tests/Lib/AST/DefinitionSpec.hs

@@ -3,7 +3,7 @@
 module Lib.AST.DefinitionSpec (spec) where
 
 import Control.Monad (forM_)
-import Lib.AST.Definition (pContractBody, pEventDefinition, pModifierDefinition)
+import Lib.AST.Definition (pContractBody, pErrorDefinition, pEventDefinition, pInterfaceDefinition, pLibraryDefinition, pModifierDefinition)
 import Lib.AST.Model
 import Lib.TestCommon (exactlyParserVerifier, leftRightJustifier, newParserVerifier, resultIsRight)
 import Test.Hspec (Spec)
@@ -13,6 +13,9 @@ spec = do
   parseModifierDefinitionSpec
   parseEventDefinitionSpec
   parseContractBodySpec
+  parseErrorDefintionSpec
+  parseInterfaceDefintionSpec
+  parseLibraryDefinitionSpec
 
 parseModifierDefinitionSpec :: Spec
 parseModifierDefinitionSpec = do
@@ -116,6 +119,10 @@ parseModifierDefinitionSpec = do
                   }
               ),
             ""
+          ),
+          ( "modifierExampleM(uint256 memory hello, string str);",
+            Left ["\"E\"", "space", "space is required after keyword 'modifier'"],
+            "modifierExampleM(uint256 memory hello, string str);"
           )
         ]
 
@@ -222,6 +229,10 @@ parseEventDefinitionSpec = do
                   }
               ),
             ""
+          ),
+          ( "eventEA(string 0 str, string) anonymous;",
+            Left ["\"E\"", "space", "space is required after keyword 'event'"],
+            "eventEA(string 0 str, string) anonymous;"
           )
         ]
 
@@ -262,6 +273,7 @@ parseContractBodySpec = do
             \using a.b.c for Bitmap;\n\
             \function receive(string name) public pure { count += 1; } \n\
             \function fallback(string name) public pure { count += 1; } \n\
+            \function getResult() external view returns(uint);\n\
             \ ",
             resultIsRight,
             ""
@@ -270,3 +282,158 @@ parseContractBodySpec = do
 
   -- we won't put the
   forM_ testCases $ newParserVerifier leftRightJustifier "contract body" pContractBody
+
+parseErrorDefintionSpec :: Spec
+parseErrorDefintionSpec = do
+  let testCases =
+        [ ( "error Unauthorized();",
+            Right (ErrorDefinition {errName = "Unauthorized", errParameters = []}),
+            ""
+          ),
+          ( "error InsufficientBalance(uint256 available, uint256 required);",
+            Right (ErrorDefinition {errName = "InsufficientBalance", errParameters = [ErrorParameter {errParamType = STypeUint 256, errParamName = Just "available"}, ErrorParameter {errParamType = STypeUint 256, errParamName = Just "required"}]}),
+            ""
+          ),
+          ( "errorUnauthorized();",
+            Left ["\"U\"", "space", "space is required after keyword 'error'"],
+            "errorUnauthorized();"
+          )
+        ]
+
+  forM_ testCases $ exactlyParserVerifier "modifier definition" pErrorDefinition
+
+parseInterfaceDefintionSpec :: Spec
+parseInterfaceDefintionSpec = do
+  let testCases =
+        [ ( "interface Calculator { \n \
+            \ function getResult() external view returns(uint); \
+            \ }",
+            Right
+              ( InterfaceDefinition
+                  { interfaceName = "Calculator",
+                    interfaceInheritanceSpecifiers = [],
+                    interfaceBody =
+                      ContractBody
+                        { ctBodyConstructor = Nothing,
+                          ctBodyFunctions = [FunctionDefinition {fnDefName = FnNormal "getResult", fnState = FnStateView, fnVisibility = FnExternal, fnModifierInvocations = [], fnFnOverrideSpecifier = Nothing, fnIsVirtual = False, fargs = [], fnReturnTyp = Just (STypeUint 256), fnBody = Nothing}],
+                          ctBodyModifiers = [],
+                          ctBodyFallbackFunctions = [],
+                          ctBodyReceiveFunctions = [],
+                          ctBodyStructDefinitions = [],
+                          ctBodyEnumDefinitions = [],
+                          ctBodyUserDefinedValueTypeDefinition = [],
+                          ctBodyStateVariables = [],
+                          ctBodyEventDefinitions = [],
+                          ctBodyErrorDefinitions = [],
+                          ctBodyUsingDirectives = [],
+                          ctBodyAllFields = [CBFSSumFunction (FunctionDefinition {fnDefName = FnNormal "getResult", fnState = FnStateView, fnVisibility = FnExternal, fnModifierInvocations = [], fnFnOverrideSpecifier = Nothing, fnIsVirtual = False, fargs = [], fnReturnTyp = Just (STypeUint 256), fnBody = Nothing})]
+                        }
+                  }
+              ),
+            ""
+          ),
+          ( "interface Calculator is MyInterface { \n \
+            \ function getResult() external view returns(uint); \
+            \ }",
+            Right
+              ( InterfaceDefinition
+                  { interfaceName = "Calculator",
+                    interfaceInheritanceSpecifiers = [InheritanceSpecifier {inheritancePath = ["MyInterface"], inheritanceCallArgs = Nothing}],
+                    interfaceBody =
+                      ContractBody
+                        { ctBodyConstructor = Nothing,
+                          ctBodyFunctions = [FunctionDefinition {fnDefName = FnNormal "getResult", fnState = FnStateView, fnVisibility = FnExternal, fnModifierInvocations = [], fnFnOverrideSpecifier = Nothing, fnIsVirtual = False, fargs = [], fnReturnTyp = Just (STypeUint 256), fnBody = Nothing}],
+                          ctBodyModifiers = [],
+                          ctBodyFallbackFunctions = [],
+                          ctBodyReceiveFunctions = [],
+                          ctBodyStructDefinitions = [],
+                          ctBodyEnumDefinitions = [],
+                          ctBodyUserDefinedValueTypeDefinition = [],
+                          ctBodyStateVariables = [],
+                          ctBodyEventDefinitions = [],
+                          ctBodyErrorDefinitions = [],
+                          ctBodyUsingDirectives = [],
+                          ctBodyAllFields = [CBFSSumFunction (FunctionDefinition {fnDefName = FnNormal "getResult", fnState = FnStateView, fnVisibility = FnExternal, fnModifierInvocations = [], fnFnOverrideSpecifier = Nothing, fnIsVirtual = False, fargs = [], fnReturnTyp = Just (STypeUint 256), fnBody = Nothing})]
+                        }
+                  }
+              ),
+            ""
+          ),
+          ( "interface Calculator is MyInterface(123, \"hello\"), A.MyInterface() { \n \
+            \ function getResult() external view returns(uint); \
+            \ }",
+            Right
+              ( InterfaceDefinition
+                  { interfaceName = "Calculator",
+                    interfaceInheritanceSpecifiers =
+                      [ InheritanceSpecifier {inheritancePath = ["MyInterface"], inheritanceCallArgs = Just (FnCallArgsList [SExprL (LNum 123), SExprL (LString "hello")])},
+                        InheritanceSpecifier
+                          { inheritancePath = ["A", "MyInterface"],
+                            inheritanceCallArgs = Just (FnCallArgsList [])
+                          }
+                      ],
+                    interfaceBody =
+                      ContractBody
+                        { ctBodyConstructor = Nothing,
+                          ctBodyFunctions = [FunctionDefinition {fnDefName = FnNormal "getResult", fnState = FnStateView, fnVisibility = FnExternal, fnModifierInvocations = [], fnFnOverrideSpecifier = Nothing, fnIsVirtual = False, fargs = [], fnReturnTyp = Just (STypeUint 256), fnBody = Nothing}],
+                          ctBodyModifiers = [],
+                          ctBodyFallbackFunctions = [],
+                          ctBodyReceiveFunctions = [],
+                          ctBodyStructDefinitions = [],
+                          ctBodyEnumDefinitions = [],
+                          ctBodyUserDefinedValueTypeDefinition = [],
+                          ctBodyStateVariables = [],
+                          ctBodyEventDefinitions = [],
+                          ctBodyErrorDefinitions = [],
+                          ctBodyUsingDirectives = [],
+                          ctBodyAllFields = [CBFSSumFunction (FunctionDefinition {fnDefName = FnNormal "getResult", fnState = FnStateView, fnVisibility = FnExternal, fnModifierInvocations = [], fnFnOverrideSpecifier = Nothing, fnIsVirtual = False, fargs = [], fnReturnTyp = Just (STypeUint 256), fnBody = Nothing})]
+                        }
+                  }
+              ),
+            ""
+          ),
+          ( "interfaceABC();",
+            Left ["\"A\"", "space", "space is required after keyword 'interface'"],
+            "interfaceABC();"
+          )
+        ]
+
+  forM_ testCases $ exactlyParserVerifier "interface definition" pInterfaceDefinition
+
+parseLibraryDefinitionSpec :: Spec
+parseLibraryDefinitionSpec = do
+  let testCases =
+        [ ( "library Lib{}",
+            Right (LibraryDefinition {libraryName = "Lib", libraryBody = ContractBody {ctBodyConstructor = Nothing, ctBodyFunctions = [], ctBodyModifiers = [], ctBodyFallbackFunctions = [], ctBodyReceiveFunctions = [], ctBodyStructDefinitions = [], ctBodyEnumDefinitions = [], ctBodyUserDefinedValueTypeDefinition = [], ctBodyStateVariables = [], ctBodyEventDefinitions = [], ctBodyErrorDefinitions = [], ctBodyUsingDirectives = [], ctBodyAllFields = []}}),
+            ""
+          ),
+          ( "library Lib{function getResult() external view returns(uint);}",
+            Right
+              ( LibraryDefinition
+                  { libraryName = "Lib",
+                    libraryBody =
+                      ContractBody
+                        { ctBodyConstructor = Nothing,
+                          ctBodyFunctions = [FunctionDefinition {fnDefName = FnNormal "getResult", fnState = FnStateView, fnVisibility = FnExternal, fnModifierInvocations = [], fnFnOverrideSpecifier = Nothing, fnIsVirtual = False, fargs = [], fnReturnTyp = Just (STypeUint 256), fnBody = Nothing}],
+                          ctBodyModifiers = [],
+                          ctBodyFallbackFunctions = [],
+                          ctBodyReceiveFunctions = [],
+                          ctBodyStructDefinitions = [],
+                          ctBodyEnumDefinitions = [],
+                          ctBodyUserDefinedValueTypeDefinition = [],
+                          ctBodyStateVariables = [],
+                          ctBodyEventDefinitions = [],
+                          ctBodyErrorDefinitions = [],
+                          ctBodyUsingDirectives = [],
+                          ctBodyAllFields = [CBFSSumFunction (FunctionDefinition {fnDefName = FnNormal "getResult", fnState = FnStateView, fnVisibility = FnExternal, fnModifierInvocations = [], fnFnOverrideSpecifier = Nothing, fnIsVirtual = False, fargs = [], fnReturnTyp = Just (STypeUint 256), fnBody = Nothing})]
+                        }
+                  }
+              ),
+            ""
+          ),
+          ( "libraryLib{}",
+            Left ["\"L\"", "space", "space is required after keyword 'library'"],
+            "libraryLib{}"
+          )
+        ]
+  forM_ testCases $ exactlyParserVerifier "library definition" pLibraryDefinition

tests/Lib/AST/FunctionSpec.hs

@@ -368,6 +368,21 @@ parseFunctionSignatureSpec = do
                   fnBody = Nothing
                 },
             ""
+          ),
+          ( "function getResult() external view returns(uint);",
+            Right
+              FunctionDefinition
+                { fnDefName = FnNormal "getResult",
+                  fnState = FnStateView,
+                  fnVisibility = FnExternal,
+                  fnModifierInvocations = [],
+                  fnFnOverrideSpecifier = Nothing,
+                  fnIsVirtual = False,
+                  fargs = [],
+                  fnReturnTyp = Just (STypeUint 256),
+                  fnBody = Nothing
+                },
+            ""
           )
         ]
   forM_ testCases $ exactlyParserVerifier "whole function" pFunctionDefinition

tests/Lib/TestCommon.hs

@@ -39,8 +39,9 @@ leftRightJustifier expectedResult result = do
         if isRight result
           then return ()
           else expectationFailure $ "Expected Right but got Left: " ++ show (fromLeft (error "") result)
-      Left _ -> do
+      Left err -> do
         errorPos (fromLeft (error "") result) `shouldBe` initialPos ""
+        err `shouldBe` []
 
 exactlyJustifier :: (Eq a, Show a) => Justifier a a
 exactlyJustifier expectedResult result = do