Make sure entity declaration is not used in the Wasp file
#2152
Conversation
infomiho
changed the title
entity declaration is not used in the Wasp file
| @@ -27,15 +27,15 @@ data TypeError' | |||
| -- We use "unify" in the TypeChecker when trying to infer the common type for | |||
| -- typed expressions that we know should be of the same type (e.g. for | |||
| -- elements in the list). | |||
| = UnificationError TypeCoercionError | |||
| = UnificationError TypeCoercionError | |||
There was a problem hiding this comment.
All seems well, but let me just make sure I understand what's going on.
Can you explain why entities can't be a ParseError (as one would expect) and rather require their own phase in the compilation pipeline:
- Is it to provide a better error message?
- Is it because we hooked into a part of the pipeline in a way that prevents us from changing the parser?
I've read this in the issue description:
We didn't remove the entity declaration to make it easier to switch to using the models from the Prisma file without touching the compiler too much.
But would like some more context if it's easy to explain.
| @@ -138,6 +139,7 @@ import qualified Wasp.Psl.Ast.Schema as Psl.Schema | |||
| analyze :: Psl.Schema.Schema -> String -> Either [AnalyzeError] [Decl] | |||
| analyze prismaSchemaAst = | |||
| (left (map ParseError) . parseStatements) | |||
| >=> (left ((: []) . ValidationError) . validateAst) | |||
There was a problem hiding this comment.
Imagine opening up this PR after reviewing and writing TypeScript all day. And the first thing you see is this line here.
Oh well, time to gaslight myself into thinking Haskell is readable again 😄
|
|
||
| isEntityDecl :: P.WithCtx P.Stmt -> Bool | ||
| isEntityDecl = \case | ||
| P.WithCtx _ (P.Decl "entity" _ _) -> True |
There was a problem hiding this comment.
How come we need to hardcode "entity" here? I haven't looked at this code for a long time so it might be necessary, but it seems weird.
There was a problem hiding this comment.
Well spotted! We should make sure that the entity declaration type name ("entity") is the same in the:
- definition of the entity declaration type
- here.
I'll make it happen 👍
| Nothing -> Right ast | ||
| where | ||
| findEntityDecl :: [P.WithCtx P.Stmt] -> Maybe (P.WithCtx P.Stmt) | ||
| findEntityDecl = find isEntityDecl |
There was a problem hiding this comment.
You can pattern-match the maybe here and return a boolean, no need to propagate it.
You can then call the function includesEntities :: [P.WithCtx P.Stmt] -> Bool
There was a problem hiding this comment.
The issue here is that I need the ctx that contains the SourceRange to produce the error, so I do need the result, not just the Bool answer.
There was a problem hiding this comment.
I've adjusted the code a bit 🤷
|
@sodic I've added an explanation on why we don't just throw a |
Signed-off-by: Mihovil Ilakovac <[email protected]>
Closes #2148
Introduces AST validation:
entitydeclaration is used, throw an errorHow it looks like
LS in VS Code:
Screen.Recording.2024-07-04.at.11.55.23.mov