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.

Sign up for GitHub

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

Jump to bottom

Divide (closed) Primitives and (opened) Builtins #119

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

Divide (closed) Primitives and (opened) Builtins #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
25058e2
Decouple Primitives and Builtins
turboMaCk Sep 23, 2021
a5812ea
better organization
turboMaCk Sep 23, 2021
36ca36d
cosmetics
turboMaCk Sep 24, 2021
3175db8
remove trailing whitespaces
turboMaCk Sep 24, 2021
ef64980
format
turboMaCk Sep 24, 2021
96228c8
spike open ElmBuiltin implementation
turboMaCk Sep 24, 2021
cbbe521
Update frontend/tests/Tests/Golden.elm
turboMaCk Sep 24, 2021
659bdb6
Update src/Elm/Ast.hs
turboMaCk Sep 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions frontend/src/Core/Decoder.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ decodePrims = D.succeed T.Prims
|> required "int" D.int
|> required "float" D.float
|> required "text" D.string
|> required "time" Iso.decoder
|> required "value" D.value
|> required "time" Iso.decoder
|> required "value" D.value
Comment on lines -19 to +20
Copy link
Member Author

Choose a reason for hiding this comment

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

related to todo of making implementation less naive

|> required "maybe" (nullable D.int)
|> required "result" (elmStreetDecodeEither D.int D.string)
|> required "pair" (elmStreetDecodePair elmStreetDecodeChar D.bool)
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/Core/Encoder.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ encodePrims x = E.object
, ("int", E.int x.int)
, ("float", E.float x.float)
, ("text", E.string x.text)
, ("time", Iso.encode x.time)
, ("value", Basics.identity x.value)
, ("time", Iso.encode x.time)
, ("value", Basics.identity x.value)
Comment on lines -19 to +20
Copy link
Member Author

Choose a reason for hiding this comment

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

related to todo of making implementation less naive

, ("maybe", (elmStreetEncodeMaybe E.int) x.maybe)
, ("result", (elmStreetEncodeEither E.int E.string) x.result)
, ("pair", (elmStreetEncodePair (E.string << String.fromChar) E.bool) x.pair)
Expand Down
10 changes: 6 additions & 4 deletions frontend/src/Core/Types.elm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ module Core.Types exposing (..)
import Time exposing (Posix)
import Json.Decode exposing (Value)

type alias ElmStreetNonEmptyList a = (a, List a)

Comment on lines +6 to +7
Copy link
Member Author

@turboMaCk turboMaCk Sep 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

this is now required as we can't produce type annotation (a, List a) - but we can produce annotations using this alias.


type alias Prims =
{ unit : ()
Expand All @@ -11,14 +13,14 @@ type alias Prims =
, int : Int
, float : Float
, text : String
, time : Posix
, value : Value
, time : Posix
, value : Value
Comment on lines +16 to +17
Copy link
Member Author

Choose a reason for hiding this comment

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

related to todo of making implementation less naive

, maybe : Maybe Int
, result : Result Int String
, pair : (Char, Bool)
, triple : (Char, Bool, List Int)
, list : List Int
, nonEmpty : (Int, List Int)
, nonEmpty : ElmStreetNonEmptyList Int
}

type MyUnit
Expand Down Expand Up @@ -116,5 +118,5 @@ type alias OneType =
, user : User
, guests : List Guest
, userRequest : UserRequest
, nonEmpty : (MyUnit, List MyUnit)
, nonEmpty : ElmStreetNonEmptyList MyUnit
}
36 changes: 24 additions & 12 deletions src/Elm/Ast.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ converted to this AST which later is going to be pretty-printed.
module Elm.Ast
( ElmDefinition (..)

, ElmPrim (..)
, ElmRecord (..)
, ElmType (..)
, ElmPrim (..)
, ElmBuiltin (..)

, ElmRecordField (..)
, ElmConstructor (..)
Expand All @@ -25,9 +26,10 @@ import Data.Text (Text)

-- | Elm data type definition.
data ElmDefinition
= DefRecord !ElmRecord
= DefPrim !ElmPrim
| DefRecord !ElmRecord
| DefBuiltin !ElmBuiltin
| DefType !ElmType
| DefPrim !ElmPrim
deriving (Show)

-- | AST for @record type alias@ in Elm.
Expand Down Expand Up @@ -70,7 +72,7 @@ isEnum ElmType{..} = null elmTypeVars && null (foldMap elmConstructorFields elmT
getConstructorNames :: ElmType -> [Text]
getConstructorNames ElmType{..} = map elmConstructorName $ toList elmTypeConstructors

-- | Primitive elm types; hardcoded by the language.
-- | Primitive elm types which are parts of a language
data ElmPrim
= ElmUnit -- ^ @()@ type in elm
| ElmNever -- ^ @Never@ type in elm, analogous to Void in Haskell
Expand All @@ -79,25 +81,35 @@ data ElmPrim
| ElmInt -- ^ @Int@
| ElmFloat -- ^ @Float@
| ElmString -- ^ @String@
| ElmTime -- ^ @Posix@ in elm, @UTCTime@ in Haskell
| ElmValue -- ^ @Json.Encode.Value@ in elm, @Data.Aeson.Value@ in Haskell
| ElmMaybe !TypeRef -- ^ @Maybe T@
| ElmResult !TypeRef !TypeRef -- ^ @Result A B@ in elm
| ElmPair !TypeRef !TypeRef -- ^ @(A, B)@ in elm
| ElmTriple !TypeRef !TypeRef !TypeRef -- ^ @(A, B, C)@ in elm
| ElmList !TypeRef -- ^ @List A@ in elm
| ElmNonEmptyPair !TypeRef -- ^ @NonEmpty A@ represented by @(A, List A)@ in elm
deriving (Show)

-- | Builtin types defined by core or 3rd party libraries
-- Included definitions:
-- * @Maybe a@
-- * @Result a b@
-- * @List a@
-- * @Time.Posix@
-- * @Json.Encode.Value@
data ElmBuiltin = ElmBuiltin
{ builtinImplType :: !Text
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change the field prefixes to elmBuiltin? The word Impl doesn't look very informative..

Copy link
Contributor

@jhrcek jhrcek Sep 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Also I find the word "Builtin" a bit misleading.
It's feels to me to have the same meaning as "Elm primitive" as in "it's provided by Elm itself".
How about something like ElmLibrary / ElmLibType ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Impl is leftover from development. I did the change incrementally by porting support from simpler to more complicated types to new structure it will need to be removed.

It's feels to me to have the same meaning as "Elm primitive" as in "it's provided by Elm itself".

agree though even this is already improvement over including these things into primitives. The one way in which these things are built in even if they come from 3rd party library (say time which depends on elm-iso8601-date-strings package) is that their decoders and definition is not generated by elm-street. Including Lib into the name is also option. Though this can be even defined in modules other than the ones generated by elm street as well. This might need some more thinking perhaps.

, builtinImplEncoder :: !Text
Copy link
Contributor

Choose a reason for hiding this comment

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

As a user of this API It's not clear from the type what this Text is supposed to be.
Is it name of the function defined somewhere? (Where?)
Can I add my custom definitions in that place?
Is it supposed to be fully qualified?
Although it's clear from the usage examples, it might not be so clear to library users ..

Copy link
Member Author

Choose a reason for hiding this comment

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

this should be documented, hover before user will be able to define own things free-rely we need to make it possible to define custom imports for generated modules. So it doesn't make much sense documenting this before that part redesign so whole process could be documented.

, builtinImplDecoder :: !Text
, builtinImplParams :: ![TypeRef]
} deriving (Show)

-- | Reference to another existing type.
data TypeRef
= RefPrim !ElmPrim
| RefCustom !TypeName
| RefBuiltin !ElmBuiltin
deriving (Show)

-- | Extracts reference to the existing data type type from some other type elm defintion.
definitionToRef :: ElmDefinition -> TypeRef
definitionToRef = \case
DefRecord ElmRecord{..} -> RefCustom $ TypeName elmRecordName
DefType ElmType{..} -> RefCustom $ TypeName elmTypeName
DefPrim elmPrim -> RefPrim elmPrim
DefType ElmType{..} -> RefCustom $ TypeName elmTypeName
DefPrim elmPrim -> RefPrim elmPrim
DefBuiltin elmBuiltIn -> RefBuiltin elmBuiltIn
2 changes: 2 additions & 0 deletions src/Elm/Generate.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ generateElm Settings{..} = do
, ""
, "import Time exposing (Posix)"
, "import Json.Decode exposing (Value)"
, ""
, "type alias ElmStreetNonEmptyList a = (a, List a)"
]

encoderHeader :: Text
Expand Down
65 changes: 51 additions & 14 deletions src/Elm/Generic.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,9 @@ import GHC.Generics (C1, Constructor (..), D1, Datatype (..), Generic (..), M1 (
import GHC.TypeLits (ErrorMessage (..), Nat, TypeError)
import GHC.TypeNats (type (+), type (<=?))

import Elm.Ast (ElmConstructor (..), ElmDefinition (..), ElmPrim (..), ElmRecord (..),
ElmRecordField (..), ElmType (..), TypeName (..), TypeRef (..), definitionToRef)
import Elm.Ast (ElmBuiltin (..), ElmConstructor (..), ElmDefinition (..), ElmPrim (..),
ElmRecord (..), ElmRecordField (..), ElmType (..), TypeName (..), TypeRef (..),
definitionToRef)

import qualified Data.Text as T
import qualified Data.Text.Lazy as LT (Text)
Expand Down Expand Up @@ -118,32 +119,68 @@ instance Elm Double where toElmDefinition _ = DefPrim ElmFloat
instance Elm Text where toElmDefinition _ = DefPrim ElmString
instance Elm LT.Text where toElmDefinition _ = DefPrim ElmString

instance Elm Value where toElmDefinition _ = DefPrim ElmValue
instance (Elm a, Elm b) => Elm (a, b) where
toElmDefinition _ = DefPrim $ ElmPair (elmRef @a) (elmRef @b)

instance (Elm a, Elm b, Elm c) => Elm (a, b, c) where
toElmDefinition _ = DefPrim $ ElmTriple (elmRef @a) (elmRef @b) (elmRef @c)

-- TODO: should it be 'Bytes' from @bytes@ package?
-- https://package.elm-lang.org/packages/elm/bytes/latest/Bytes
-- instance Elm B.ByteString where toElmDefinition _ = DefPrim ElmString
-- instance Elm LB.ByteString where toElmDefinition _ = DefPrim ElmString

instance Elm UTCTime where toElmDefinition _ = DefPrim ElmTime
----------------------------------------------------------------------------
-- Builtin instances
----------------------------------------------------------------------------

instance Elm Value where
toElmDefinition _ = DefBuiltin $ ElmBuiltin
{ builtinImplType = "Value"
, builtinImplEncoder = "Basics.identity"
, builtinImplDecoder = "D.value"
, builtinImplParams = []
}

instance Elm UTCTime where
toElmDefinition _ = DefBuiltin $ ElmBuiltin
{ builtinImplType = "Posix"
, builtinImplEncoder = "Iso.encode"
, builtinImplDecoder = "Iso.decoder"
, builtinImplParams = []
}

instance Elm a => Elm (Maybe a) where
toElmDefinition _ = DefPrim $ ElmMaybe $ elmRef @a
toElmDefinition _ = DefBuiltin $ ElmBuiltin
{ builtinImplType = "Maybe"
, builtinImplEncoder = "elmStreetEncodeMaybe"
, builtinImplDecoder = "nullable"
, builtinImplParams = [elmRef @a]
}

instance (Elm a, Elm b) => Elm (Either a b) where
toElmDefinition _ = DefPrim $ ElmResult (elmRef @a) (elmRef @b)

instance (Elm a, Elm b) => Elm (a, b) where
toElmDefinition _ = DefPrim $ ElmPair (elmRef @a) (elmRef @b)

instance (Elm a, Elm b, Elm c) => Elm (a, b, c) where
toElmDefinition _ = DefPrim $ ElmTriple (elmRef @a) (elmRef @b) (elmRef @c)
toElmDefinition _ = DefBuiltin $ ElmBuiltin
{ builtinImplType = "Result"
, builtinImplEncoder = "elmStreetEncodeEither"
, builtinImplDecoder = "elmStreetDecodeEither"
, builtinImplParams = [elmRef @a, elmRef @b]
}

instance Elm a => Elm [a] where
toElmDefinition _ = DefPrim $ ElmList (elmRef @a)
toElmDefinition _ = DefBuiltin $ ElmBuiltin
{ builtinImplType = "List"
, builtinImplEncoder = "E.list"
Copy link
Contributor

@jhrcek jhrcek Sep 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The E. and D. in this definition seem magical. Where are they coming from?
EDIT: Ah, I see they're coming from

import Json.Decode as D exposing (..)
import Json.Decode.Pipeline as D exposing (required)
...
import Json.Encode as E exposing (..)

Copy link
Member Author

Choose a reason for hiding this comment

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

yes. I agreee this is not nice though. But I would probably rather address that in separate PR

, builtinImplDecoder = "D.list"
, builtinImplParams = [elmRef @a]
}

instance Elm a => Elm (NonEmpty a) where
toElmDefinition _ = DefPrim $ ElmNonEmptyPair (elmRef @a)
toElmDefinition _ = DefBuiltin $ ElmBuiltin
{ builtinImplType = "ElmStreetNonEmptyList"
, builtinImplEncoder = "elmStreetEncodeNonEmpty"
, builtinImplDecoder = "elmStreetDecodeNonEmpty"
, builtinImplParams = [elmRef @a]
}
Comment on lines +137 to +183
Copy link
Member Author

Choose a reason for hiding this comment

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

utilize new ElmBuiltin in instances we provide out of the box.


----------------------------------------------------------------------------
-- Smart constructors
Expand Down
23 changes: 10 additions & 13 deletions src/Elm/Print/Decoder.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ module Elm.Print.Decoder
, decodeNonEmpty
) where

import Data.List (intersperse)
import Data.List.NonEmpty (toList)
import Data.Text (Text)
import Data.Text.Prettyprint.Doc (Doc, colon, concatWith, dquotes, emptyDoc, equals, line, nest,
parens, pretty, surround, vsep, (<+>))
import Prettyprinter.Util (reflow)

import Elm.Ast (ElmConstructor (..), ElmDefinition (..), ElmPrim (..), ElmRecord (..),
ElmRecordField (..), ElmType (..), TypeName (..), TypeRef (..), isEnum)
import Elm.Ast (ElmBuiltin (..), ElmConstructor (..), ElmDefinition (..), ElmPrim (..),
ElmRecord (..), ElmRecordField (..), ElmType (..), TypeName (..), TypeRef (..),
isEnum)
import Elm.Print.Common (arrow, mkQualified, qualifiedTypeWithVarsDoc, showDoc, wrapParens)

import qualified Data.List.NonEmpty as NE
Expand Down Expand Up @@ -73,6 +76,7 @@ prettyShowDecoder def = showDoc $ case def of
DefRecord elmRecord -> recordDecoderDoc elmRecord
DefType elmType -> typeDecoderDoc elmType
DefPrim _ -> emptyDoc
DefBuiltin _ -> emptyDoc

recordDecoderDoc :: ElmRecord -> Doc ann
recordDecoderDoc ElmRecord{..} =
Expand Down Expand Up @@ -106,11 +110,11 @@ recordDecoderDoc ElmRecord{..} =
<+> wrapParens (typeRefDecoder t)

typeDecoderDoc :: ElmType -> Doc ann
typeDecoderDoc t@ElmType{..} =
typeDecoderDoc ElmType{..} =
-- function defenition: @encodeTypeName : TypeName -> Value@.
decoderDef elmTypeName elmTypeVars
<> line
<> if isEnum t
<> if isEnum ElmType{..}
-- if this is Enum just using the read instance we wrote.
then enumDecoder
else if elmTypeIsNewtype
Expand Down Expand Up @@ -190,22 +194,15 @@ typeRefDecoder (RefPrim elmPrim) = case elmPrim of
ElmInt -> "D.int"
ElmFloat -> "D.float"
ElmString -> "D.string"
ElmTime -> "Iso.decoder"
ElmValue -> "D.value"
ElmMaybe t -> "nullable"
<+> wrapParens (typeRefDecoder t)
ElmResult l r -> "elmStreetDecodeEither"
<+> wrapParens (typeRefDecoder l)
<+> wrapParens (typeRefDecoder r)
ElmPair a b -> "elmStreetDecodePair"
<+> wrapParens (typeRefDecoder a)
<+> wrapParens (typeRefDecoder b)
ElmTriple a b c -> "elmStreetDecodeTriple"
<+> wrapParens (typeRefDecoder a)
<+> wrapParens (typeRefDecoder b)
<+> wrapParens (typeRefDecoder c)
ElmList l -> "D.list" <+> wrapParens (typeRefDecoder l)
ElmNonEmptyPair a -> "elmStreetDecodeNonEmpty" <+> wrapParens (typeRefDecoder a)
typeRefDecoder (RefBuiltin ElmBuiltin{..}) =
reflow builtinImplDecoder <+> mconcat (intersperse " " (fmap (wrapParens . typeRefDecoder) builtinImplParams))

-- | The definition of the @decodeTYPENAME@ function.
decoderDef
Expand Down
24 changes: 10 additions & 14 deletions src/Elm/Print/Encoder.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,17 @@ module Elm.Print.Encoder
, encodeNonEmpty
) where

import Data.List (intersperse)
import Data.List.NonEmpty (NonEmpty, toList)
import Data.Text (Text)
import Data.Text.Prettyprint.Doc (Doc, brackets, colon, comma, concatWith, dquotes, emptyDoc,
equals, lbracket, line, nest, parens, pretty, rbracket, surround,
vsep, (<+>))
import Prettyprinter.Util (reflow)

import Elm.Ast (ElmConstructor (..), ElmDefinition (..), ElmPrim (..), ElmRecord (..),
ElmRecordField (..), ElmType (..), TypeName (..), TypeRef (..), isEnum)
import Elm.Ast (ElmBuiltin (..), ElmConstructor (..), ElmDefinition (..), ElmPrim (..),
ElmRecord (..), ElmRecordField (..), ElmType (..), TypeName (..), TypeRef (..),
isEnum)
import Elm.Print.Common (arrow, mkQualified, qualifiedTypeWithVarsDoc, showDoc, wrapParens)

import qualified Data.List.NonEmpty as NE
Expand All @@ -44,14 +47,15 @@ prettyShowEncoder def = showDoc $ case def of
DefRecord elmRecord -> recordEncoderDoc elmRecord
DefType elmType -> typeEncoderDoc elmType
DefPrim _ -> emptyDoc
DefBuiltin _ -> emptyDoc

-- | Encoder for 'ElmType' (which is either enum or the Sum type).
typeEncoderDoc :: ElmType -> Doc ann
typeEncoderDoc t@ElmType{..} =
typeEncoderDoc ElmType{..} =
-- function definition: @encodeTypeName : TypeName -> Value@.
encoderDef elmTypeName elmTypeVars
<> line
<> if isEnum t
<> if isEnum ElmType{..}
-- if this is Enum just using the show instance we wrote.
then enumEncoder
else if elmTypeIsNewtype
Expand Down Expand Up @@ -182,23 +186,15 @@ typeRefEncoder (RefPrim elmPrim) = case elmPrim of
ElmInt -> "E.int"
ElmFloat -> "E.float"
ElmString -> "E.string"
ElmTime -> "Iso.encode"
ElmValue -> "Basics.identity"
ElmMaybe t -> "elmStreetEncodeMaybe"
<+> wrapParens (typeRefEncoder t)
ElmResult l r -> "elmStreetEncodeEither"
<+> wrapParens (typeRefEncoder l)
<+> wrapParens (typeRefEncoder r)
ElmPair a b -> "elmStreetEncodePair"
<+> wrapParens (typeRefEncoder a)
<+> wrapParens (typeRefEncoder b)
ElmTriple a b c -> "elmStreetEncodeTriple"
<+> wrapParens (typeRefEncoder a)
<+> wrapParens (typeRefEncoder b)
<+> wrapParens (typeRefEncoder c)
ElmList l -> "E.list" <+> wrapParens (typeRefEncoder l)
ElmNonEmptyPair a -> "elmStreetEncodeNonEmpty"
<+> wrapParens (typeRefEncoder a)
typeRefEncoder (RefBuiltin ElmBuiltin{..}) =
reflow builtinImplEncoder <+> mconcat (intersperse " " (fmap (wrapParens . typeRefEncoder) builtinImplParams))

-- | @JSON@ encoder Elm help function for 'Maybe's.
encodeMaybe :: Text
Expand Down
Loading