-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Disallow duplicate declaration names #2015
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder how hard would it be to implement this check in the Analyzer, not in Valid.hs step. I think it could be done, there is a place where we bind the variables, including declarations, and we could check for this tehre.
But, that sounds harder, and then again it is tempting to do it here again. Also, if we do it here, it will work also for TypeScript SDK. So all good, just wanted to mention it.
@@ -149,3 +149,14 @@ spec_checksum :: Spec | |||
spec_checksum = do | |||
it "Correctly calculates checksum of string" $ do | |||
checksumFromString "test" `shouldBe` hexFromString "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" | |||
|
|||
spec_findDuplicateElems :: Spec | |||
spec_findDuplicateElems = do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Signed-off-by: Mihovil Ilakovac <mihovil@ilakovac.com>
9d027d2
to
32c4263
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is it, approved! Left one tiny comment, do as you wish with it.
@@ -41,6 +41,10 @@ makeDeclType ''App | |||
-- | Collection of domain types that are standard for Wasp, that define what the Wasp language looks like. | |||
-- These are injected this way instead of hardcoding them into the Analyzer in order to make it | |||
-- easier to modify and maintain the Wasp compiler/language. | |||
|
|||
-- *** MAKE SURE TO UPDATE: The `validateUniqueDeclarationNames` function in the `Wasp.AppSpec.Valid` module |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, but I normally would put NOTE, because NOTE is so popular that editors will paint it in different color (same like TODO). Up to you though, just a suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted for this comment to stand out more, that's why the custom prefix made sense to me :)
Closes #1234
Compiler checks if there are duplicate declarations within the same declaration type e.g. duplicate queries names are disallowed but queries and actions can still reuse names.