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

Sumtype enums #151

Merged
merged 15 commits into from
Nov 1, 2017
4 changes: 2 additions & 2 deletions proto-lens-protoc/package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ library:
- Data.Text as Data.ProtoLens.Reexport.Data.Text,
- Lens.Family2 as Data.ProtoLens.Reexport.Lens.Family2,
- Lens.Family2.Unchecked as Data.ProtoLens.Reexport.Lens.Family2.Unchecked,
- Lens.Labels as Data.ProtoLens.Reexport.Lens.Labels

- Lens.Labels as Data.ProtoLens.Reexport.Lens.Labels,
- Text.Read as Data.ProtoLens.Reexport.Text.Read

executables:
proto-lens-protoc:
Expand Down
3 changes: 3 additions & 0 deletions proto-lens-protoc/src/Data/ProtoLens/Compiler/Combinators.hs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,9 @@ ihApp = foldl (Syntax.IHApp ())
tyParen :: Type -> Type
tyParen = Syntax.TyParen ()

paren :: Exp -> Exp
paren = Syntax.Paren ()

type Match = Syntax.Match ()

-- | A simple clause of a function binding.
Expand Down
4 changes: 4 additions & 0 deletions proto-lens-protoc/src/Data/ProtoLens/Compiler/Definitions.hs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ promoteSymbol (Symbol s) = tyPromotedString s
-- | All the information needed to define or use a proto enum type.
data EnumInfo n = EnumInfo
{ enumName :: n
, enumUnrecognizedName :: n
, enumUnrecognizedValueName :: n
, enumDescriptor :: EnumDescriptorProto
, enumValues :: [EnumValueInfo n]
} deriving Functor
Expand Down Expand Up @@ -365,6 +367,8 @@ enumDef protoPrefix hsPrefix d = let
in (mkText (d ^. name)
, Enum EnumInfo
{ enumName = mkHsName (d ^. name)
, enumUnrecognizedName = mkHsName (d ^. name <> "'Unrecognized")
, enumUnrecognizedValueName = mkHsName (d ^. name <> "'UnrecognizedValue")
, enumDescriptor = d
, enumValues = collectEnumValues mkHsName $ d ^. value
})
Expand Down
217 changes: 165 additions & 52 deletions proto-lens-protoc/src/Data/ProtoLens/Compiler/Generate.hs
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ generateModule modName imports syntaxType modifyImport definitions importedEnv
[ "Prelude", "Data.Int", "Data.Word"
, "Data.ProtoLens", "Data.ProtoLens.Message.Enum"
, "Lens.Family2", "Lens.Family2.Unchecked", "Data.Default.Class"
, "Data.Text", "Data.Map" , "Data.ByteString"
, "Lens.Labels"
, "Data.Text", "Data.Map", "Data.ByteString"
, "Lens.Labels", "Text.Read"
]
++ map importSimple imports
env = Map.union (unqualifyEnv definitions) importedEnv
Expand Down Expand Up @@ -259,80 +259,193 @@ generateEnumExports syntaxType e = [exportAll n, exportWith n aliases]

generateEnumDecls :: SyntaxType -> EnumInfo Name -> [Decl]
generateEnumDecls Proto3 info =
-- newtype Foo = Foo Int32
[ newtypeDecl dataName "Data.Int.Int32"
$ deriving' ["Prelude.Eq", "Prelude.Ord", "Prelude.Enum", "Prelude.Bounded"]

-- instance Show Foo where
-- showsPrec _ Value0 = "Value0" -- the Haskell name
-- showsPrec p (Foo k) = showParen (p > 10)
-- $ showString "toEnum " . shows k
, instDecl [] ("Prelude.Show" `ihApp` [dataType])
[ [ match "showsPrec" [pWildCard, pApp (unQual n) []]
$ "Prelude.showString" @@ stringExp (prettyPrint n)
| n <- map enumValueName $ enumValues info
-- data FooEnum
-- = Enum1
-- | Enum2
-- | FooEnum'Unrecognized !FooEnum'UnrecognizedValue
-- deriving (Prelude.Show, Prelude.Eq, Prelude.Ord, Prelude.Read)
[ dataDecl dataName
( (flip conDecl [] <$> constructorNames)
++ [conDecl unrecognizedName [tyCon $ unQual unrecognizedValueName]]
)
$ deriving' ["Prelude.Show", "Prelude.Eq", "Prelude.Ord, Prelude.Read"]

-- newtype FooEnum'UnrecognizedValue = FooEnum'UnrecognizedValue Data.Int.Int32
-- deriving (Prelude.Eq, Prelude.Ord, Prelude.Show, Prelude.Read)
, newtypeDecl unrecognizedValueName
"Data.Int.Int32"
$ deriving' ["Prelude.Eq", "Prelude.Ord", "Prelude.Show, Prelude.Read"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we remove the Read instance, to make sure it's not being used accidentally?

(Also because we'd like toEnum/maybeToEnum to be the only way to construct these values)

Up to you.


-- instance Data.ProtoLens.MessageEnum FooEnum where
-- maybeToEnum 0 = Prelude.Just Enum1
-- maybeToEnum 3 = Prelude.Just Enum2
-- maybeToEnum k
-- = Prelude.Just
-- (FooEnum'Unrecognized
-- (FooEnum'UnrecognizedValue (Prelude.fromIntegral k)))
-- showEnum (FooEnum'Unrecognized (FooEnum'UnrecognizedValue k))
-- = Prelude.show k
-- showEnum k = Prelude.show k
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this comment needs to be updated since you're not calling Prelude.show anymore.

-- readEnum "Enum2a" = Prelude.Just Enum2a -- alias
-- readEnum "Enum2" = Prelude.Just Enum2
-- readEnum "Enum1" = Prelude.Just Enum1
-- readEnum k = Text.Read.readMaybe k >>= maybeToEnum
, instDecl [] ("Data.ProtoLens.MessageEnum" `ihApp` [dataType])
[ [ match "maybeToEnum" [pLitInt k] $ "Prelude.Just" @@ con (unQual c)
| (c, k) <- constructorNumbers
]
++
[ match "showsPrec" ["p", pApp (unQual dataName) ["k"]]
$ "Prelude.showParen" @@ ("Prelude.>" @@ "p" @@ litInt 10)
@@ ("Prelude.." @@ ("Prelude.showString"
@@ stringExp "toEnum ")
@@ ("Prelude.shows" @@ "k"))
[match "maybeToEnum" ["k"]
$ "Prelude.Just" @@
(con (unQual unrecognizedName)
@@ (con (unQual unrecognizedValueName)
@@ ("Prelude.fromIntegral" @@ "k")
)
)
]
]

-- instance MessageEnum Foo where
-- maybeToEnum k = Just $ toEnum k
-- showEnum (Foo 0) = "Value0" -- the proto name
-- showEnum (Foo k) = show k
-- readEnum "Value0" = Just (Foo 0)
-- readEnum _ = Nothing
, instDecl [] ("Data.ProtoLens.MessageEnum" `ihApp` [dataType])
[ [match "maybeToEnum" ["k"]
$ "Prelude.Just" @@ ("Prelude.toEnum" @@ "k")]
, [ match "showEnum" [pVar n] $ stringExp pn
| v <- enumValues info
, isNothing (enumAliasOf v)
, [ match "showEnum" [pApp (unQual n) []]
$ stringExp pn
| v <- filter (null . enumAliasOf) $ enumValues info
, let n = enumValueName v
, let pn = T.unpack $ enumValueDescriptor v ^. name
] ++
[match "showEnum" [pApp (unQual unrecognizedName)
[pApp (unQual unrecognizedValueName) [pVar "k"]]
]
$ "Prelude.show" @@ "k"
]
, [ match "showEnum" [pApp (unQual dataName) ["k"]]
$ "Prelude.show" @@ "k"
]

, [ match "readEnum" [stringPat pn]
$ "Prelude.Just" @@ con (unQual n)
| v <- enumValues info
, let n = enumValueName v
, let pn = T.unpack $ enumValueDescriptor v ^. name
] ++
[match "readEnum" [pVar "k"] $ ("Prelude.>>=" @@ paren ("Text.Read.readMaybe" @@ "k")) @@ ("Data.ProtoLens.maybeToEnum")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Break this line which has gotten pretty long.

]

-- instance Bounded Foo where
-- minBound = Foo1
-- maxBound = FooN
, instDecl [] ("Prelude.Bounded" `ihApp` [dataType])
[[ match "minBound" [] $ con $ unQual minBoundName
, match "maxBound" [] $ con $ unQual maxBoundName
]]

-- instance Enum Foo where
-- toEnum k = maybe (error ("Foo.toEnum: unknown argument for enum Foo: "
-- ++ show k))
-- id (maybeToEnum k)
-- fromEnum Foo1 = 1
-- fromEnum Foo2 = 2
-- ..
-- succ FooN = error "Foo.succ: bad argument FooN."
-- succ Foo1 = Foo2
-- succ Foo2 = Foo3
-- ..
-- pred Foo1 = error "Foo.succ: bad argument Foo1."
-- pred Foo2 = Foo1
-- pred Foo3 = Foo2
-- ..
-- enumFrom = messageEnumFrom
-- enumFromTo = messageEnumFromTo
-- enumFromThen = messageEnumFromThen
-- enumFromThenTo = messageEnumFromThenTo
, instDecl [] ("Prelude.Enum" `ihApp` [dataType])
[[match "toEnum" ["k__"]
$ "Prelude.maybe" @@ errorMessageExpr @@ "Prelude.id"
@@ ("Data.ProtoLens.maybeToEnum" @@ "k__")]
, [ match "fromEnum" [pApp (unQual c) []] $ litInt k
| (c, k) <- constructorNumbers
]
++
[ match "readEnum" [pWildCard] "Prelude.Nothing"
[match "fromEnum" [pApp (unQual unrecognizedName)
[pApp (unQual unrecognizedValueName) [pVar "k"]]
]
$ "Prelude.fromIntegral" @@ "k"
]
, succDecl "succ" maxBoundName succPairs
, succDecl "pred" minBoundName $ map swap succPairs
, alias "enumFrom" "Data.ProtoLens.Message.Enum.messageEnumFrom"
, alias "enumFromTo" "Data.ProtoLens.Message.Enum.messageEnumFromTo"
, alias "enumFromThen" "Data.ProtoLens.Message.Enum.messageEnumFromThen"
, alias "enumFromThenTo"
"Data.ProtoLens.Message.Enum.messageEnumFromThenTo"
]

-- proto3 enums always default to zero.
-- instance Data.Default.Class.Default Foo where
-- def = FirstEnumValue
, instDecl [] ("Data.Default.Class.Default" `ihApp` [dataType])
[[match "def" [] $ "Prelude.toEnum" @@ litInt 0]]
[[match "def" [] defaultCon]]
-- instance Data.ProtoLens.FieldDefault Foo where
-- fieldDefault = FirstEnumValue
, instDecl [] ("Data.ProtoLens.FieldDefault" `ihApp` [dataType])
[[match "fieldDefault" [] $ "Prelude.toEnum" @@ litInt 0]]
]
++
-- pattern Value0 :: Foo
-- pattern Value0 = Foo 0
[[match "fieldDefault" [] defaultCon]]
] ++
-- pattern Enum2a :: FooEnum
-- pattern Enum2a = Enum2
concat
[ [ patSynSig n dataType
, patSyn (pVar n)
$ pApp (unQual dataName) [pLitInt k]
[ [ patSynSig aliasName dataType
, patSyn (pVar aliasName) (pVar originalName)
]
| v <- enumValues info
, let n = enumValueName v
, let k = fromIntegral $ enumValueDescriptor v ^. number
| EnumValueInfo
{ enumValueName = aliasName
, enumAliasOf = Just originalName
} <- enumValues info
]

where
dataName = enumName info
EnumInfo { enumName = dataName
, enumUnrecognizedName = unrecognizedName
, enumUnrecognizedValueName = unrecognizedValueName
, enumDescriptor = ed
} = info
errorMessage = "toEnum: unknown value for enum " ++ unpack (ed ^. name)
++ ": "

errorMessageExpr = "Prelude.error"
@@ ("Prelude.++" @@ stringExp errorMessage
@@ ("Prelude.show" @@ "k__"))
alias funName implName = [match funName [] implName]

dataType = tyCon $ unQual dataName



constructors :: [(Name, EnumValueDescriptorProto)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's a shame this now duplicates logic from the proto2 code...not sure if there's a good way to re-combine the two. (They were split apart more in #137.)

constructors = List.sortBy (comparing ((^. number) . snd))
[(n, d) | EnumValueInfo
{ enumValueName = n
, enumValueDescriptor = d
, enumAliasOf = Nothing
} <- enumValues info
]
constructorNames = map fst constructors

defaultCon = con $ unQual $ head constructorNames

minBoundName = head constructorNames
maxBoundName = last constructorNames

constructorNumbers = map (second (fromIntegral . (^. number))) constructors

succPairs = zip constructorNames $ tail constructorNames
succDecl funName boundName thePairs =
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think succ should be equivalent to toEnum . incr . fromEnum. The implications are kind of subtle though compared to the proto2 semantics listed here: https://github.com/google/proto-lens/blob/master/proto-lens/src/Data/ProtoLens/Message/Enum.hs#L2
If we do make that change, we should note in Data.ProtoLens.Message.Enum that it only applies to proto2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Is not erroring on succ maxEnum and instead returning SomeEnum'UnrecognizedValue... (fromEnum maxEnum + 1) the desired behavior here?

Do you also want me to redefine pred as toEnum . (-1) . fromEnum? Or should pred minEnum still result in an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this would break things, eg for FooEnum as defined in proto3.proto:

  enum FooEnum {
    option allow_alias = true;
    Enum1 = 0;
    Enum2 = 3;
    Enum2a = 3;
  }

it seems like we want succ Enum1 == Enum2, not succ Enum1 == Foo'FooEnum'Unrecognized (Foo'FooEnum'UnrecognizedValue 2)

match funName [pApp (unQual boundName) []]
("Prelude.error" @@ stringExp (concat
[ prettyPrint dataName, ".", prettyPrint funName, ": bad argument "
, prettyPrint boundName, ". This value would be out of bounds."
]))
:
[ match funName [pApp (unQual from) []] $ con $ unQual to
| (from, to) <- thePairs
]
++
[match funName [pWildCard]
("Prelude.error" @@ stringExp (concat
[ prettyPrint dataName, ".", prettyPrint funName, ": bad argument: unrecognized value"
]))
]

generateEnumDecls Proto2 info =
[ dataDecl dataName
[conDecl n [] | n <- constructorNames]
Expand Down
1 change: 1 addition & 0 deletions proto-lens-tests/src/Data/ProtoLens/TestUtil.hs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import Text.PrettyPrint
testMain :: [Test] -> IO ()
testMain = defaultMain

-- todo use this for enum msg on Foo
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove comment

serializeTo :: (Show a, Eq a, Message a)
=> String -> a -> Doc -> Builder.Builder -> Test
serializeTo name x text bs = testCase name $ do
Expand Down
10 changes: 7 additions & 3 deletions proto-lens-tests/tests/proto3_test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,20 @@ main = testMain
, testCase "string" $ (def :: Strings) @=? (def & string .~ "")
, testCase "enum" $ (def :: Foo) @=? (def & enum .~ Foo'Enum1)
]
-- Enums are all pattern aliases
-- Enums are sum types, except for aliases
, testGroup "enum"
[ testCase "aliases are exported" $ Foo'Enum2 @=? Foo'Enum2a
, serializeTo "serializeTo enum"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a copy of this test which uses an unrecognized value?

Also: replace l.107 with more explicit generation:

tagged 6 $ VarInt 3

(where 6 is the proto tag of that field, and 3 is the value corresponding to Enum2)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ping on the first line of the above comment (unless I'm misreading the diff).

More specifically, please add this test which checks that unrecognized values round-trip through serialization as expected.

serializeTo "serializeTo enum unknown"
    (def & enum .~ toEnum 5 :: Foo)
    "enum: 5"
    $ tagged 6 $ VarInt 5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops, sorry about that. Fixed.

(def & enum .~ Foo'Enum2 :: Foo)
"enum: Enum2"
"0\ETX"
, testCase "enum values" $ do
map toEnum [0, 3, 3] @=? [Foo'Enum1, Foo'Enum2, Foo'Enum2a]
["Foo'Enum1", "Foo'Enum2", "Foo'Enum2", "toEnum 5"]
["Foo'Enum1", "Foo'Enum2", "Foo'Enum2", "Foo'FooEnum'Unrecognized (Foo'FooEnum'UnrecognizedValue 5)"]
@=? map show [Foo'Enum1, Foo'Enum2, Foo'Enum2a, toEnum 5]
["Enum1", "Enum2", "Enum2", "6"]
@=? map showEnum [Foo'Enum1, Foo'Enum2, Foo'Enum2a, toEnum 6]
[Just Foo'Enum1, Just Foo'Enum2, Just Foo'Enum2, Nothing, Nothing]
[Just Foo'Enum1, Just Foo'Enum2, Just Foo'Enum2, maybeToEnum 4, maybeToEnum 5]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Test nit (working around the fact that the constructor isn't provided): Add a new test that checks fromEnum <$> maybeToEnum 4 == Just 4

@=? map readEnum ["Enum1", "Enum2", "Enum2a", "4", "5"]
, testCase "enum patterns" $ do
assertBool "enum value" $ case toEnum 3 of
Expand Down