Skip to content

Commit 0e16843

Browse files
evanczevancz
committed
fix oversights in Parse.Module.checkEffects
1 parent d86b5e6 commit 0e16843

File tree

2 files changed

+135
-21
lines changed

2 files changed

+135
-21
lines changed

‎compiler/src/Parse/Module.hs‎

Lines changed: 58 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,37 @@ import qualified Reporting.Error.Syntax as E
2929
-- FROM BYTE STRING
3030

3131

32-
fromByteString :: Pkg.Name -> BS.ByteString -> Either E.Error Src.Module
33-
fromByteString pkg source =
34-
case P.fromByteString (chompModule pkg) E.ModuleBadEnd source of
35-
Right modul -> checkModule modul
32+
fromByteString :: ProjectType -> BS.ByteString -> Either E.Error Src.Module
33+
fromByteString projectType source =
34+
case P.fromByteString (chompModule projectType) E.ModuleBadEnd source of
35+
Right modul -> checkModule projectType modul
3636
Left err -> Left (E.ParseError err)
3737

3838

3939

40+
-- PROJECT TYPE
41+
42+
43+
data ProjectType
44+
= Package Pkg.Name
45+
| Application
46+
47+
48+
isCore :: ProjectType -> Bool
49+
isCore projectType =
50+
case projectType of
51+
Package pkg -> pkg == Pkg.core
52+
Application -> False
53+
54+
55+
isKernel :: ProjectType -> Bool
56+
isKernel projectType =
57+
case projectType of
58+
Package pkg -> Pkg.isKernel pkg
59+
Application -> False
60+
61+
62+
4063
-- MODULE
4164

4265

@@ -49,11 +72,11 @@ data Module =
4972
}
5073

5174

52-
chompModule :: Pkg.Name -> Parser E.Module Module
53-
chompModule pkg =
75+
chompModule :: ProjectType -> Parser E.Module Module
76+
chompModule projectType =
5477
do header <- chompHeader
55-
imports <- chompImports (if pkg == Pkg.core then [] else Imports.defaults)
56-
infixes <- if Pkg.isKernel pkg then chompInfixes [] else return []
78+
imports <- chompImports (if isCore projectType then [] else Imports.defaults)
79+
infixes <- if isKernel projectType then chompInfixes [] else return []
5780
decls <- specialize E.Declarations $ chompDecls []
5881
return (Module header imports infixes decls)
5982

@@ -62,15 +85,15 @@ chompModule pkg =
6285
-- CHECK MODULE
6386

6487

65-
checkModule :: Module -> Either E.Error Src.Module
66-
checkModule (Module maybeHeader imports infixes decls) =
88+
checkModule :: ProjectType -> Module -> Either E.Error Src.Module
89+
checkModule projectType (Module maybeHeader imports infixes decls) =
6790
let
6891
(values, unions, aliases, ports) = categorizeDecls [] [] [] [] decls
6992
in
7093
case maybeHeader of
7194
Just (Header name effects exports docs) ->
7295
Src.Module (Just name) exports (toDocs docs decls) imports values unions aliases infixes
73-
<$> checkEffects ports effects
96+
<$> checkEffects projectType ports effects
7497

7598
Nothing ->
7699
Right $
@@ -80,23 +103,37 @@ checkModule (Module maybeHeader imports infixes decls) =
80103
_:_ -> Src.Ports ports
81104

82105

83-
checkEffects :: [Src.Port] -> Effects -> Either E.Error Src.Effects
84-
checkEffects ports effects =
106+
checkEffects :: ProjectType -> [Src.Port] -> Effects -> Either E.Error Src.Effects
107+
checkEffects projectType ports effects =
85108
case effects of
86109
NoEffects region ->
87110
case ports of
88-
[] -> Right Src.NoEffects
89-
_:_ -> Left (E.UnexpectedPort region)
111+
[] ->
112+
Right Src.NoEffects
113+
114+
Src.Port name _ : _ ->
115+
case projectType of
116+
Package _ -> Left (E.NoPortsInPackage name)
117+
Application -> Left (E.UnexpectedPort region)
90118

91119
Ports region ->
92-
case ports of
93-
[] -> Left (E.NoPorts region)
94-
_:_ -> Right (Src.Ports ports)
120+
case projectType of
121+
Package _ ->
122+
Left (E.NoPortModulesInPackage region)
123+
124+
Application ->
125+
case ports of
126+
[] -> Left (E.NoPorts region)
127+
_:_ -> Right (Src.Ports ports)
95128

96129
Manager region manager ->
97-
case ports of
98-
[] -> Right (Src.Manager region manager)
99-
_:_ -> Left (E.UnexpectedPort region)
130+
if isKernel projectType then
131+
case ports of
132+
[] -> Right (Src.Manager region manager)
133+
_:_ -> Left (E.UnexpectedPort region)
134+
else
135+
Left (E.NoEffectsOutsideKernel region)
136+
100137

101138

102139
categorizeDecls :: [A.Located Src.Value] -> [A.Located Src.Union] -> [A.Located Src.Alias] -> [Src.Port] -> [Decl.Decl] -> ( [A.Located Src.Value], [A.Located Src.Union], [A.Located Src.Alias], [Src.Port] )

‎compiler/src/Reporting/Error/Syntax.hs‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ data Error
6969
| ModuleNameMismatch ModuleName.Raw (A.Located ModuleName.Raw)
7070
| UnexpectedPort A.Region
7171
| NoPorts A.Region
72+
| NoPortsInPackage (A.Located Name.Name)
73+
| NoPortModulesInPackage A.Region
74+
| NoEffectsOutsideKernel A.Region
7275
| ParseError Module
7376

7477

@@ -561,10 +564,84 @@ toReport source err =
561564
]
562565
)
563566

567+
NoPortsInPackage (A.At region _) ->
568+
Report.Report "PACKAGES CANNOT HAVE PORTS" region [] $
569+
Code.toSnippet source region Nothing
570+
(
571+
D.reflow $
572+
"Packages cannot declare any ports, so I am getting stuck here:"
573+
,
574+
D.stack
575+
[ D.reflow $
576+
"Remove this port declaration."
577+
, noteForPortsInPackage
578+
]
579+
)
580+
581+
NoPortModulesInPackage region ->
582+
Report.Report "PACKAGES CANNOT HAVE PORTS" region [] $
583+
Code.toSnippet source region Nothing
584+
(
585+
D.reflow $
586+
"Packages cannot declare any ports, so I am getting stuck here:"
587+
,
588+
D.stack
589+
[ D.fillSep $
590+
["Remove","the",D.cyan "port","keyword","and","I"
591+
,"should","be","able","to","continue."
592+
]
593+
, noteForPortsInPackage
594+
]
595+
)
596+
597+
NoEffectsOutsideKernel region ->
598+
Report.Report "INVALID EFFECT MODULE" region [] $
599+
Code.toSnippet source region Nothing
600+
(
601+
D.reflow $
602+
"It is not possible to declare an `effect module` outside the @elm organization,\
603+
\ so I am getting stuck here:"
604+
,
605+
D.stack
606+
[ D.reflow $
607+
"Switch to a normal module declaration."
608+
, D.toSimpleNote $
609+
"Effect modules are designed to allow certain core functionality to be\
610+
\ defined separately from the compiler. So the @elm organization has access to\
611+
\ this so that certain changes, extensions, and fixes can be introduced without\
612+
\ needing to release new Elm binaries. For example, we want to make it possible\
613+
\ to test effects, but this may require changes to the design of effect modules.\
614+
\ By only having them defined in the @elm organization, that kind of design work\
615+
\ can proceed much more smoothly."
616+
]
617+
)
618+
564619
ParseError modul ->
565620
toParseErrorReport source modul
566621

567622

623+
noteForPortsInPackage :: D.Doc
624+
noteForPortsInPackage =
625+
D.stack
626+
[ D.toSimpleNote $
627+
"One of the major goals of the package ecosystem is to be completely written\
628+
\ in Elm. This means when you install an Elm package, you can be sure you are safe\
629+
\ from security issues (like code that runs side-effects on install) and that you\
630+
\ are not going to get any runtime exceptions coming from your new dependency.\
631+
\ This design also sets the Elm ecosystem up well to target other platforms (like\
632+
\ mobile phones, WebAssembly, etc.) since no community code explicitly depends on\
633+
\ JavaScript even existing."
634+
, D.reflow $
635+
"Given that overall goal, allowing ports in packages would lead to some pretty\
636+
\ surprising behavior. If ports were allowed in packages, you could install a\
637+
\ package but not realize that it brings in an indirect dependency that defines a\
638+
\ port. Now you have a program that does not work and the fix is to realize that\
639+
\ some JavaScript needs to be added. That would be extremely frustrating! So why\
640+
\ not allow the package author to include the necessary JS code as well? Now we\
641+
\ are back in conflict with our goals for the ecosystem overall."
642+
]
643+
644+
568645
toParseErrorReport :: Code.Source -> Module -> Report.Report
569646
toParseErrorReport source modul =
570647
case modul of

0 commit comments

Comments
 (0)