diff --git a/CHANGES.md b/CHANGES.md --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,8 @@ +0.8.0.2 (2016-08-31) +==================== + +- Disallow reserved keywords from being used as identifier names. + 0.8.0.1 (2016-05-24) ==================== diff --git a/language-thrift.cabal b/language-thrift.cabal --- a/language-thrift.cabal +++ b/language-thrift.cabal @@ -1,9 +1,9 @@ --- This file has been generated from package.yaml by hpack version 0.13.0. +-- This file has been generated from package.yaml by hpack version 0.14.1. -- -- see: https://github.com/sol/hpack name: language-thrift -version: 0.8.0.1 +version: 0.8.0.2 cabal-version: >= 1.10 build-type: Simple license: BSD3 @@ -32,21 +32,23 @@ library hs-source-dirs: src + ghc-options: -Wall exposed-modules: Language.Thrift.Parser Language.Thrift.Pretty Language.Thrift.Types + other-modules: + Language.Thrift.Internal.Reserved + Language.Thrift.Internal.Types + Paths_language_thrift + default-language: Haskell2010 build-depends: base >= 4.7 && < 5, ansi-wl-pprint >= 0.6 && < 0.7, + containers >= 0.5 && < 0.6, megaparsec >= 4.0 && < 5.0, text >= 1.2, transformers - default-language: Haskell2010 - other-modules: - Language.Thrift.Internal.Types - Paths_language_thrift - ghc-options: -Wall test-suite spec type: exitcode-stdio-1.0 @@ -54,19 +56,27 @@ build-depends: base >= 4.7 && < 5, ansi-wl-pprint >= 0.6 && < 0.7, + containers >= 0.5 && < 0.6, megaparsec >= 4.0 && < 5.0, text >= 1.2, + transformers, hspec >= 2.0, hspec-discover >= 2.1, language-thrift, QuickCheck >= 2.5 - default-language: Haskell2010 - hs-source-dirs: - test other-modules: + Language.Thrift.Internal.Reserved + Language.Thrift.Internal.Types + Language.Thrift.Parser + Language.Thrift.Pretty + Language.Thrift.Types Language.Thrift.Arbitrary Language.Thrift.ParserSpec Language.Thrift.TypesSpec Spec TestUtils + default-language: Haskell2010 + hs-source-dirs: + src, + test ghc-options: -Wall diff --git a/src/Language/Thrift/Internal/Reserved.hs b/src/Language/Thrift/Internal/Reserved.hs new file mode 100644 --- /dev/null +++ b/src/Language/Thrift/Internal/Reserved.hs @@ -0,0 +1,59 @@ +-- | +-- Module : Language.Thrift.Internal.Reserved +-- Copyright : (c) Abhinav Gupta 2016 +-- License : BSD3 +-- +-- Maintainer : Abhinav Gupta +-- Stability : experimental +-- +-- This module provides information about reserved Thrift identifiers. +module Language.Thrift.Internal.Reserved + ( isReserved + ) where + +import Data.Set (Set) +import qualified Data.Set as Set + +isReserved :: String -> Bool +isReserved = (`Set.member` reservedKeywords) + +reservedKeywords :: Set String +reservedKeywords = Set.fromList + [ "include" + , "namespace" + , "cpp_namespace" + , "php_namespace" + , "py_module" + , "perl_package" + , "ruby_namespace" + , "java_package" + , "cocoa_package" + , "csharp_namespace" + , "typedef" + , "enum" + , "struct" + , "union" + , "exception" + , "required" + , "optional" + , "senum" + , "const" + , "string" + , "binary" + , "slist" + , "bool" + , "byte" + , "i8" + , "i16" + , "i32" + , "i64" + , "double" + , "map" + , "set" + , "list" + , "service" + , "extends" + , "oneway" + , "void" + , "throws" + ] diff --git a/src/Language/Thrift/Parser.hs b/src/Language/Thrift/Parser.hs --- a/src/Language/Thrift/Parser.hs +++ b/src/Language/Thrift/Parser.hs @@ -71,6 +71,8 @@ import qualified Text.Megaparsec as P import qualified Text.Megaparsec.Lexer as PL +import Language.Thrift.Internal.Reserved (isReserved) + import qualified Language.Thrift.Types as T -- | Keeps track of the last docstring seen by the system so that we can @@ -239,13 +241,21 @@ colon = symbolic ':' equals = symbolic '=' +-- | errorUnlessReserved ensures that the given identifier is in the +-- reservedKeywords list. If it's not, we have a bug and we should crash. +errorUnlessReserved :: Monad m => String -> m () +errorUnlessReserved name = + unless (isReserved name) $ + error ("reserved called with unreserved identifier " ++ show name) + -- | Parses a reserved identifier and adds it to the collection of known -- reserved keywords. reserved :: P.Stream s Char => String -> Parser s () -reserved name = P.label name $ token $ P.try $ do - void (P.string name) - P.notFollowedBy (P.alphaNumChar <|> P.oneOf "_.") - +reserved name = + errorUnlessReserved name >> + P.label name $ token $ P.try $ do + void (P.string name) + P.notFollowedBy (P.alphaNumChar <|> P.oneOf "_.") -- | A string literal. @"hello"@ literal :: P.Stream s Char => Parser s Text @@ -267,6 +277,8 @@ name <- (:) <$> (P.letterChar <|> P.char '_') <*> many (P.alphaNumChar <|> P.oneOf "_.") + when (isReserved name) $ + P.unexpected name return (Text.pack name) diff --git a/test/Language/Thrift/Arbitrary.hs b/test/Language/Thrift/Arbitrary.hs --- a/test/Language/Thrift/Arbitrary.hs +++ b/test/Language/Thrift/Arbitrary.hs @@ -17,6 +17,8 @@ import qualified Data.Text as Text +import Language.Thrift.Internal.Reserved (isReserved) + import qualified Language.Thrift.Types as T #ifdef MIN_VERSION_QuickCheck @@ -64,6 +66,15 @@ shrink (Docstring t) = Docstring <$> shrink t +newtype Identifier = Identifier { getIdentifier :: Text } + deriving (Show, Typeable, Generic) + +instance Arbitrary Identifier where + arbitrary = Identifier <$> arbitrary `suchThat` (not . isReserved . Text.unpack) + + shrink (Identifier t) = + [Identifier t' | t' <- shrink t, not (isReserved (Text.unpack t'))] + ------------------------------------------------------------------------------ instance Arbitrary (T.Program ()) where @@ -83,7 +94,11 @@ instance Arbitrary (T.Namespace ()) where shrink = genericShrink - arbitrary = T.Namespace <$> elements scopes <*> arbitrary <*> pure () + arbitrary = + T.Namespace + <$> elements scopes + <*> (getIdentifier <$> arbitrary) + <*> pure () where scopes = ["*", "py", "rb", "java", "hs", "cpp"] @@ -101,7 +116,7 @@ arbitrary = T.Const <$> arbitrary - <*> arbitrary + <*> (getIdentifier <$> arbitrary) <*> arbitrary <*> (getDocstring <$> arbitrary) <*> pure () @@ -110,8 +125,11 @@ shrink = genericShrink arbitrary = T.Service - <$> arbitrary - <*> arbitrary + <$> (getIdentifier <$> arbitrary) + <*> frequency + [ (1, return Nothing) + , (3, Just . getIdentifier <$> arbitrary) + ] <*> arbitrary <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) @@ -123,7 +141,7 @@ T.Function <$> arbitrary <*> halfSize arbitrary - <*> arbitrary + <*> (getIdentifier <$> arbitrary) <*> halfSize arbitrary <*> halfSize arbitrary <*> halfSize arbitrary @@ -145,7 +163,7 @@ shrink = genericShrink arbitrary = T.Typedef <$> arbitrary - <*> arbitrary + <*> (getIdentifier <$> arbitrary) <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) <*> pure () @@ -153,7 +171,7 @@ instance Arbitrary (T.Enum ()) where shrink = genericShrink arbitrary = T.Enum - <$> arbitrary + <$> (getIdentifier <$> arbitrary) <*> arbitrary <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) @@ -163,7 +181,7 @@ shrink = genericShrink arbitrary = T.EnumDef - <$> arbitrary + <$> (getIdentifier <$> arbitrary) <*> arbitrary <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) @@ -172,7 +190,7 @@ instance Arbitrary (T.Struct ()) where shrink = genericShrink arbitrary = T.Struct - <$> arbitrary + <$> (getIdentifier <$> arbitrary) <*> arbitrary <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) @@ -181,7 +199,7 @@ instance Arbitrary (T.Union ()) where shrink = genericShrink arbitrary = T.Union - <$> arbitrary + <$> (getIdentifier <$> arbitrary) <*> arbitrary <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) @@ -190,7 +208,7 @@ instance Arbitrary (T.Exception ()) where shrink = genericShrink arbitrary = T.Exception - <$> arbitrary + <$> (getIdentifier <$> arbitrary) <*> arbitrary <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) @@ -199,7 +217,7 @@ instance Arbitrary (T.Senum ()) where shrink = genericShrink arbitrary = T.Senum - <$> arbitrary + <$> (getIdentifier <$> arbitrary) <*> arbitrary <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) @@ -212,7 +230,7 @@ <$> (fmap getPositive <$> arbitrary) <*> arbitrary <*> halfSize arbitrary - <*> arbitrary + <*> (getIdentifier <$> arbitrary) <*> halfSize arbitrary <*> halfSize arbitrary <*> (getDocstring <$> arbitrary) @@ -221,7 +239,8 @@ instance Arbitrary (T.TypeReference ()) where shrink = genericShrink arbitrary = oneof - [ T.DefinedType <$> arbitrary <*> pure () + [ T.DefinedType <$> (getIdentifier <$> arbitrary) + <*> pure () , halfSize $ T.StringType <$> arbitrary <*> pure () , halfSize $ T.BinaryType <$> arbitrary <*> pure () , halfSize $ T.SListType <$> arbitrary <*> pure () @@ -242,7 +261,7 @@ instance Arbitrary T.TypeAnnotation where shrink = genericShrink - arbitrary = T.TypeAnnotation <$> arbitrary <*> arbitrary + arbitrary = T.TypeAnnotation <$> (getIdentifier <$> arbitrary) <*> arbitrary newtype BasicConstValue = BasicConstValue { @@ -256,7 +275,8 @@ [ T.ConstFloat <$> choose (0.0, 10000.0) <*> pure () , T.ConstInt <$> arbitrary <*> pure () , T.ConstLiteral <$> arbitrary <*> pure () - , T.ConstIdentifier <$> arbitrary <*> pure () + , T.ConstIdentifier <$> (getIdentifier <$> arbitrary) + <*> pure () ] -- | newtype wrapper around const values so that we're not generating lists diff --git a/test/Language/Thrift/ParserSpec.hs b/test/Language/Thrift/ParserSpec.hs --- a/test/Language/Thrift/ParserSpec.hs +++ b/test/Language/Thrift/ParserSpec.hs @@ -80,6 +80,13 @@ , "something_else" ] + it "cannot parse reserved keywords in names" $ + parseFailureCases P.program + [ "enum struct {}" + , "strut Foo { 1: required string service }" + ] + + parseFailureCases :: Show a => Parser a -> [String] -> Expectation parseFailureCases p = mapM_ (p `shouldNotParse`)