Skip to content

Commit

Permalink
Don't use data-default-class to build protos.
Browse files Browse the repository at this point in the history
Fixes google#194.  `data-default` is problematic since it's (1) lawless and (2)
creeps into places it's not really intended for; e.g., using it for an integer
value instead of an explicit 0.  But protobufs *do* have a useful law, namely,
`encodeMessage def == ""`.

This change makes `Data.ProtoLens.Message` (and thus `Data.ProtoLens`, which reexports it):

- Add a `defaultMessage` method to `Message`.
- Redefine `def = defaultMessage` for convenience.

I'm not convinced this is the best long-term UX; suggestions welcome.
  • Loading branch information
judah committed Aug 23, 2018
1 parent 591a8fa commit 80a2bc7
Show file tree
Hide file tree
Showing 18 changed files with 263 additions and 296 deletions.
2 changes: 1 addition & 1 deletion proto-lens-combinators/package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ library:
exposed-modules:
- Data.ProtoLens.Combinators
dependencies:
- data-default-class >= 0.0 && < 0.2
- proto-lens == 0.4.*
- transformers >= 0.4 && < 0.6

tests:
Expand Down
4 changes: 2 additions & 2 deletions proto-lens-combinators/src/Data/ProtoLens/Combinators.hs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module Data.ProtoLens.Combinators
) where

import Control.Monad.Trans.State (State, execState)
import Data.Default.Class (Default(def))
import Data.ProtoLens (Message, def)
import Data.Maybe (isJust)
import Lens.Family2 (LensLike, Phantom, Setter, to, (.~))

Expand All @@ -33,7 +33,7 @@ clear = (.~ Nothing)

-- | Creates a 'Default' and then applies the provided `State` to it. This is
-- convenient for creating complicated structures.
make :: Default def => State def a -> def
make :: Message msg => State msg a -> msg
make = modifyInState def

-- | Allows one to modify a value in the 'State' monad. Note that this is
Expand Down
6 changes: 5 additions & 1 deletion proto-lens-discrimination/package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ custom-setup:

dependencies:
- base >= 4.9 && < 4.12
- data-default >= 0.5 && < 0.8
- bytestring == 0.10.*
- contravariant >= 1.3 && < 1.5
- containers >= 0.5 && < 0.6
Expand Down Expand Up @@ -54,10 +53,15 @@ tests:
other-modules:
- Proto.Enum
- Proto.Enum_Fields
- Proto.Map
- Proto.Map_Fields

# Manually list autogenerated modules, to avoid hpack
# requiring `cabal-version: >= 2.0`.
verbatim:
- |
autogen-modules:
Proto.Enum
Proto.Enum_Fields
Proto.Map
Proto.Map_Fields
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ module Data.ProtoLens.Discrimination
import Data.Bits ((.|.), shift)
import qualified Data.ByteString as B
import qualified Data.ByteString.Unsafe as B (unsafeIndex)
import Data.Default (Default(def))
import Data.Foldable (foldMap)
import Data.Functor.Contravariant
( Contravariant(contramap)
Expand All @@ -42,6 +41,8 @@ import Data.ProtoLens.Message
( FieldDescriptor(FieldDescriptor)
, FieldTypeDescriptor(..)
, FieldAccessor(..)
, Message
, def
)

-- | Sort values according to a Foldable of field descriptors.
Expand Down Expand Up @@ -126,7 +127,7 @@ discFieldSet discList disc32 disc64 discBS discInt =
-- This should be identical to sorting @Map key value@s using the 'Ord' of
-- @key@ and the 'Message' of value.
discProtoMapAssocs
:: forall f entry key value. (Contravariant f, Default entry)
:: forall f entry key value. (Contravariant f, Message entry)
=> (forall a. f a -> f [a])
-> f entry
-> Lens' entry key
Expand Down
10 changes: 7 additions & 3 deletions proto-lens-discrimination/tests/disc_test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@
module Main where

import Control.Applicative ((<$>))
import Data.Default (Default(def))
import Data.Discrimination (Sort, runSort, sorting, sorting1)
import Data.Int (Int32)
import Data.List (sortBy)
import Lens.Family2 ((&), (^.), (.~))
import Lens.Family2 (Lens', (&), (^.), (.~))
import Lens.Family2.Stock (_1, _2)
import Test.HUnit ((@=?), Assertion)
import Test.Framework (testGroup, defaultMain)
Expand All @@ -24,12 +23,15 @@ import Data.ProtoLens.Message
, Message(fieldsByTextFormatName)
, ScalarField(..)
, MessageOrGroup(..)
, def
)
import Data.ProtoLens.Discrimination (discProtoMapAssocs)
import Data.ProtoLens.Sort

import Proto.Enum
import Proto.Enum_Fields
import Proto.Map
import Proto.Map_Fields


sortCompare :: Sort a -> a -> a -> Ordering
Expand Down Expand Up @@ -96,7 +98,9 @@ protoMapSortTest = testGroup "map"
in compare x' y' == sortCompare c x' y'
]
where
c = discProtoMapAssocs sorting1 sorting _1 _2
c = discProtoMapAssocs sorting1 sortingMessage
(key :: Lens' MapWrapper'IntMapEntry Int32)
value

fieldSortTest :: Test
fieldSortTest = testProperty "compares by field" $
Expand Down
8 changes: 8 additions & 0 deletions proto-lens-discrimination/tests/map.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
syntax = "proto2";

package map;

message MapWrapper {
map<int32, int32> int_map = 1;
}

49 changes: 18 additions & 31 deletions proto-lens-protoc/src/Data/ProtoLens/Compiler/Generate.hs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ generateModule modName imports syntaxType modifyImport definitions importedEnv s
sharedImports = map (modifyImport . importSimple)
[ "Prelude", "Data.Int", "Data.Word"
, "Data.ProtoLens", "Data.ProtoLens.Message.Enum", "Data.ProtoLens.Service.Types"
, "Lens.Family2", "Lens.Family2.Unchecked", "Data.Default.Class"
, "Lens.Family2", "Lens.Family2.Unchecked"
, "Data.Text", "Data.Map", "Data.ByteString", "Data.ByteString.Char8"
, "Lens.Labels", "Text.Read"
]
Expand Down Expand Up @@ -302,26 +302,8 @@ generateMessageDecls fieldModName syntaxType env protoName info =
, let sym = promoteSymbol $ lensSymbol i
]
++
-- instance Data.Default.Class.Default Bar where
[ uncommented $ instDecl [] ("Data.Default.Class.Default" `ihApp` [dataType])
-- def = Bar { _Bar_foo = 0 }
[
[ match "def" []
$ recConstr (unQual dataName) $
[ fieldUpdate (unQual $ haskellRecordFieldName $ plainFieldName f)
(hsFieldDefault syntaxType env (fieldDescriptor f))
| f <- messageFields info
] ++
[ fieldUpdate (unQual $ haskellRecordFieldName $ oneofFieldName o)
"Prelude.Nothing"
| o <- messageOneofFields info
] ++
[ fieldUpdate (unQual $ messageUnknownFields info)
"[]"]
]
]
-- instance Message.Message Bar where
, uncommented $ instDecl [] ("Data.ProtoLens.Message" `ihApp` [dataType])
[ uncommented $ instDecl [] ("Data.ProtoLens.Message" `ihApp` [dataType])
$ messageInstance syntaxType env protoName info
]
where
Expand Down Expand Up @@ -523,10 +505,6 @@ generateEnumDecls Proto3 info =
"Data.ProtoLens.Message.Enum.messageEnumFromThenTo"
]

-- instance Data.Default.Class.Default Foo where
-- def = FirstEnumValue
, instDecl [] ("Data.Default.Class.Default" `ihApp` [dataType])
[[match "def" [] defaultCon]]
-- instance Data.ProtoLens.FieldDefault Foo where
-- fieldDefault = FirstEnumValue
, instDecl [] ("Data.ProtoLens.FieldDefault" `ihApp` [dataType])
Expand Down Expand Up @@ -601,10 +579,6 @@ generateEnumDecls Proto2 info =
[ dataDecl dataName
[conDecl n [] | n <- constructorNames]
$ deriving' ["Prelude.Show", "Prelude.Eq", "Prelude.Ord"]
-- instance Data.Default.Class.Default Foo where
-- def = FirstEnumValue
, instDecl [] ("Data.Default.Class.Default" `ihApp` [dataType])
[[match "def" [] defaultCon]]
-- instance Data.ProtoLens.FieldDefault Foo where
-- fieldDefault = FirstEnumValue
, instDecl [] ("Data.ProtoLens.FieldDefault" `ihApp` [dataType])
Expand Down Expand Up @@ -949,10 +923,10 @@ hsFieldDefault syntaxType env fd

hsFieldValueDefault :: Env QName -> FieldDescriptorProto -> Exp
hsFieldValueDefault env fd = case fd ^. type' of
FieldDescriptorProto'TYPE_MESSAGE -> "Data.Default.Class.def"
FieldDescriptorProto'TYPE_GROUP -> "Data.Default.Class.def"
FieldDescriptorProto'TYPE_MESSAGE -> "Data.ProtoLens.defaultMessage"
FieldDescriptorProto'TYPE_GROUP -> "Data.ProtoLens.defaultMessage"
FieldDescriptorProto'TYPE_ENUM
| T.null def -> "Data.Default.Class.def"
| T.null def -> "Data.ProtoLens.fieldDefault"
| Enum e <- definedFieldType fd env
, Just v <- List.lookup def [ (enumValueDescriptor v ^. name, enumValueName v)
| v <- enumValues e
Expand Down Expand Up @@ -1047,6 +1021,19 @@ messageInstance syntaxType env protoName m =
let' (map (fieldDescriptorVarBind $ messageName m) $ fields)
$ "Data.Map.fromList" @@ list fieldsByTag ]
, [ match "unknownFields" [] $ rawFieldAccessor (unQual $ messageUnknownFields m) ]
, [ match "defaultMessage" []
$ recConstr (unQual $ messageName m) $
[ fieldUpdate (unQual $ haskellRecordFieldName $ plainFieldName f)
(hsFieldDefault syntaxType env (fieldDescriptor f))
| f <- messageFields m
] ++
[ fieldUpdate (unQual $ haskellRecordFieldName $ oneofFieldName o)
"Prelude.Nothing"
| o <- messageOneofFields m
] ++
[ fieldUpdate (unQual $ messageUnknownFields m)
"[]"]
]
]
where
fieldsByTag =
Expand Down
1 change: 0 additions & 1 deletion proto-lens-tests/package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ dependencies:
- QuickCheck
- base
- bytestring
- data-default-class
- lens-family
- pretty
- proto-lens
Expand Down
4 changes: 2 additions & 2 deletions proto-lens-tests/tests/imports_test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
{-# LANGUAGE TypeApplications #-}
module Main where

import Data.Default.Class (Default(def))
import Data.ProtoLens (Message, def)
import Lens.Labels (Lens', view, set)
import Test.Framework.Providers.HUnit (testCase)
import Test.HUnit ((@=?))
Expand All @@ -30,7 +30,7 @@ main = testMain
-- (In this test, we're checking that the sub-field type got imported
-- correctly from another file.)
testField
:: forall a b . (Default a, Default b, Eq b, Show b)
:: forall a b . (Message a, Message b, Eq b, Show b)
=> Lens' a b -> IO ()
testField f = def @=? view f (set f def def)

Expand Down
3 changes: 1 addition & 2 deletions proto-lens-tests/tests/labels_test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import qualified Lens.Family2
import qualified Lens.Family
import Proto.Canonical (Test1, Test3)

import Data.Default.Class (def)
import Data.ProtoLens (build)
import Data.ProtoLens (build, def)
import Data.ProtoLens.TestUtil
import Test.HUnit ((@?=))
import Test.Framework.Providers.HUnit (testCase)
Expand Down
2 changes: 1 addition & 1 deletion proto-lens-tests/tests/required_test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
{-# LANGUAGE OverloadedStrings #-}
module Main where

import Data.Default.Class (def)
import Data.Monoid ((<>))
import Data.ProtoLens (def)
import Lens.Family ((&), (.~))
import Proto.Required (Foo)
import Proto.Required_Fields (a, b)
Expand Down
3 changes: 1 addition & 2 deletions proto-lens-tests/tests/unwrapped_test.hs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ import Lens.Family ((&), (.~), (^.))
import Lens.Labels.Unwrapped ()
import Proto.Canonical (Test3)

import Data.Default.Class (def)
import Data.ProtoLens (build)
import Data.ProtoLens (build, def)
import Data.ProtoLens.TestUtil
import Test.HUnit ((@?=))
import Test.Framework.Providers.HUnit (testCase)
Expand Down
1 change: 0 additions & 1 deletion proto-lens/package.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ library:
- bytestring == 0.10.*
- containers == 0.5.*
- deepseq == 1.4.*
- data-default-class >= 0.0 && < 0.2
- lens-family == 1.2.*
- lens-labels == 0.2.*
- parsec == 3.1.*
Expand Down
26 changes: 20 additions & 6 deletions proto-lens/src/Data/ProtoLens/Message.hs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ module Data.ProtoLens.Message (
FieldDefault(..),
MessageEnum(..),
-- * Building protocol buffers
Default(..),
def,
build,
-- * Proto registries
Registry,
Expand All @@ -51,7 +51,6 @@ module Data.ProtoLens.Message (
) where

import qualified Data.ByteString as B
import Data.Default.Class
import Data.Int
import qualified Data.Map as Map
import Data.Map (Map)
Expand All @@ -71,12 +70,19 @@ import Data.ProtoLens.Encoding.Wire
-- | Every protocol buffer is an instance of 'Message'. This class enables
-- serialization by providing reflection of all of the fields that may be used
-- by this type.
class Default msg => Message msg where
class Message msg where
-- | A unique identifier for this type, of the format
-- @"packagename.messagename"@.
messageName :: Proxy msg -> T.Text

-- | A message with all fields set to their default values.
--
-- Satisfies @encodeMessage defaultMessage == ""@ and @decodeMessage "" == Right defaultMessage@.
defaultMessage :: msg

-- | The fields of the proto, indexed by their (integer) tag.
fieldsByTag :: Map Tag (FieldDescriptor msg)

-- | This map is keyed by the name of the field used for text format protos.
-- This is just the field name for every field except for group fields,
-- which use their Message type name in text protos instead of their
Expand All @@ -85,6 +91,7 @@ class Default msg => Message msg where
fieldsByTextFormatName :: Map String (FieldDescriptor msg)
fieldsByTextFormatName =
Map.fromList [(n, f) | f@(FieldDescriptor n _ _) <- allFields]

-- | Access the unknown fields of a Message.
unknownFields :: Lens' msg FieldSet

Expand Down Expand Up @@ -142,8 +149,9 @@ data WireDefault value where

-- | A proto3 field type with an implicit default value.
--
-- This is distinct from 'Data.Default' to avoid orphan instances, and because
-- 'Bool' doesn't necessarily have a good Default instance for general usage.
-- This is distinct from, say, 'Data.Default' to avoid orphan instances, and
-- because 'Bool' doesn't necessarily have a good Default instance for general
-- usage.
class FieldDefault value where
fieldDefault :: value

Expand Down Expand Up @@ -236,14 +244,20 @@ class (Enum a, Bounded a) => MessageEnum a where
-- no corresponding value was defined in the .proto file.
readEnum :: String -> Maybe a

-- | A message with all fields set to their default values.
--
-- This is an elided version 'defaultMessage'.
def :: Message a => a
def = defaultMessage

-- | Utility function for building a message from a default value.
-- For example:
--
-- > instance Default A where ...
-- > x, y :: Lens' A Int
-- > m :: A
-- > m = build ((x .~ 5) . (y .~ 7))
build :: Default a => (a -> a) -> a
build :: Message a => (a -> a) -> a
build = ($ def)

-- | A helper lens for accessing optional fields.
Expand Down
Loading

0 comments on commit 80a2bc7

Please sign in to comment.