Skip to content

Commit

Permalink
Merge pull request #255 from dahlia/nicer-error-on-name-dups
Browse files Browse the repository at this point in the history 
Nicer error message on name duplicates
  • Loading branch information
@dahlia
dahlia committed Mar 27, 2018
2 parents 4529e07 + 8fd42c8 commit f7d40c8
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 58 deletions.
9 changes: 9 additions & 0 deletions CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ To be released.
// ^~~~~~~~~~~ ^~~
~~~~~~~~
[[#254], [#255]]
- Enum members and union tags became disallowed to shadow other enum members
and union tags even if they belong to an other type. It's because in some
target language we compile them into objects that share the same namespace,
Expand Down Expand Up @@ -111,6 +113,11 @@ To be released.
union qux = bar (text a) | quux (text b);
~~~~~~~~
[[#254], [#255]]
- Fixed a compiler bug that an error message on name duplicates had referred
to a wrong line/column number. [[#255]]
### Python target
- Generated Python packages became to have two [entry points] (a feature
Expand Down Expand Up @@ -159,6 +166,8 @@ To be released.
[#13]: https://github.com/spoqa/nirum/issues/13
[#220]: https://github.com/spoqa/nirum/issues/220
[#227]: https://github.com/spoqa/nirum/pull/227
[#254]: https://github.com/spoqa/nirum/pull/254
[#255]: https://github.com/spoqa/nirum/pull/255
[entry points]: https://setuptools.readthedocs.io/en/latest/pkg_resources.html#entry-points
[python2-numbers-integral]: https://docs.python.org/2/library/numbers.html#numbers.Integral
[python2-basestring]: https://docs.python.org/2/library/functions.html#basestring
Expand Down
144 changes: 98 additions & 46 deletions src/Nirum/Parser.hs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ module Nirum.Parser ( Parser
, unionTypeDeclaration
) where

import Control.Monad (void)
import Control.Monad (void, when)
import Data.Void
import qualified System.IO as SIO

import qualified Data.List as L
import Data.Map.Strict as Map hiding (foldl)
import Data.Map.Strict as Map hiding (foldl, toList)
import Data.Set hiding (foldl)
import qualified Data.Text as T
import qualified Data.Text.IO as TIO
Expand All @@ -54,16 +54,17 @@ import Text.Megaparsec.Char.Lexer (charLiteral)

import qualified Nirum.Constructs.Annotation as A
import Nirum.Constructs.Declaration (Declaration)
import qualified Nirum.Constructs.Declaration as D
import Nirum.Constructs.Docs (Docs (Docs))
import Nirum.Constructs.DeclarationSet as DeclarationSet
import Nirum.Constructs.DeclarationSet as DeclarationSet hiding (toList)
import Nirum.Constructs.Identifier ( Identifier
, identifierRule
, reservedKeywords
, toString
)
import Nirum.Constructs.Module (Module (Module))
import Nirum.Constructs.ModulePath (ModulePath (ModulePath, ModuleName))
import Nirum.Constructs.Name (Name (Name))
import Nirum.Constructs.Name (Name (Name, facialName))
import Nirum.Constructs.Service ( Method (Method)
, Parameter (Parameter)
, Service (Service)
Expand All @@ -83,6 +84,19 @@ import Nirum.Constructs.TypeExpression ( TypeExpression ( ListModifier
type Parser = Parsec Void T.Text
type ParseError = E.ParseError Char Void

-- | State-tracking 'many'.
many' :: [a] -> ([a] -> Parser a) -> Parser [a]
many' i p = do
r <- optional (p i)
case r of
Nothing -> return i
Just v -> many' (i ++ [v]) p
-- FIXME: i ++ [v] is not efficient

-- | Get the facial name of a declaration.
declFacialName :: Declaration d => d -> Identifier
declFacialName = facialName . D.name

-- CHECK: If a new reserved keyword is introduced, it has to be also
-- added to `reservedKeywords` set in the `Nirum.Constructs.Identifier`
-- module.
Expand Down Expand Up @@ -117,13 +131,35 @@ identifier =

name :: Parser Name
name = do
facialName <- identifier <?> "facial name"
behindName <- option facialName $ try $ do
facialName' <- identifier <?> "facial name"
behindName <- option facialName' $ try $ do
spaces
char '/'
spaces
identifier <?> "behind name"
return $ Name facialName behindName
return $ Name facialName' behindName

uniqueIdentifier :: [Identifier] -> String -> Parser Identifier
uniqueIdentifier forwardNames label' = try $ do
ident <- lookAhead identP
when (ident `elem` forwardNames)
(fail $ "the " ++ label' ++ " `" ++ toString ident ++
"` is duplicated")
identP
where
identP :: Parser Identifier
identP = identifier <?> label'

uniqueName :: [Identifier] -> String -> Parser Name
uniqueName forwardNames label' = try $ do
Name fName _ <- lookAhead nameP
when (fName `elem` forwardNames)
(fail $ "the " ++ label' ++ " `" ++ toString fName ++
"` is duplicated")
nameP
where
nameP :: Parser Name
nameP = name <?> label'

annotationArgumentValue :: Parser T.Text
annotationArgumentValue = do
Expand Down Expand Up @@ -239,12 +275,12 @@ annotationsWithDocs :: Monad m
annotationsWithDocs set' (Just docs') = A.insertDocs docs' set'
annotationsWithDocs set' Nothing = return set'

aliasTypeDeclaration :: Parser TypeDeclaration
aliasTypeDeclaration = do
aliasTypeDeclaration :: [Identifier] -> Parser TypeDeclaration
aliasTypeDeclaration forwardNames = do
annotationSet' <- annotationSet <?> "type alias annotations"
string' "type" <?> "type alias keyword"
spaces
typeName <- identifier <?> "alias type name"
typeName <- uniqueIdentifier forwardNames "alias type name"
let name' = Name typeName typeName
spaces
char '='
Expand All @@ -257,12 +293,12 @@ aliasTypeDeclaration = do
return $ TypeDeclaration name' (Alias canonicalType') annotationSet''


unboxedTypeDeclaration :: Parser TypeDeclaration
unboxedTypeDeclaration = do
unboxedTypeDeclaration :: [Identifier] -> Parser TypeDeclaration
unboxedTypeDeclaration forwardNames = do
annotationSet' <- annotationSet <?> "unboxed type annotations"
string' "unboxed" <?> "unboxed type keyword"
spaces
typeName <- identifier <?> "unboxed type name"
typeName <- uniqueIdentifier forwardNames "unboxed type name"
let name' = Name typeName typeName
spaces
char '('
Expand All @@ -276,11 +312,11 @@ unboxedTypeDeclaration = do
annotationSet'' <- annotationsWithDocs annotationSet' docs'
return $ TypeDeclaration name' (UnboxedType innerType') annotationSet''

enumMember :: Parser EnumMember
enumMember = do
enumMember :: [Identifier] -> Parser EnumMember
enumMember forwardNames = do
annotationSet' <- annotationSet <?> "enum member annotations"
spaces
memberName <- name <?> "enum member name"
memberName <- uniqueName forwardNames "enum member name"
spaces
docs' <- optional $ do
d <- docs <?> "enum member docs"
Expand Down Expand Up @@ -309,12 +345,12 @@ handleNameDuplicationError label' (Left dup) =
BehindNameDuplication (Name _ bname) -> ("behind", bname)
FacialNameDuplication (Name fname _) -> ("facial", fname)

enumTypeDeclaration :: Parser TypeDeclaration
enumTypeDeclaration = do
enumTypeDeclaration :: [Identifier] -> Parser TypeDeclaration
enumTypeDeclaration forwardNames = do
annotationSet' <- annotationSet <?> "enum type annotations"
string "enum" <?> "enum keyword"
spaces
typeName <- name <?> "enum type name"
typeName@(Name typeFName _) <- uniqueName forwardNames "enum type name"
spaces
frontDocs <- optional $ do
d <- docs <?> "enum type docs"
Expand All @@ -329,7 +365,9 @@ enumTypeDeclaration = do
spaces
return d
annotationSet'' <- annotationsWithDocs annotationSet' docs'
members' <- (enumMember `sepBy1` (spaces >> char '|' >> spaces))
members' <- sepBy1
(enumMember (typeFName : forwardNames))
(spaces >> char '|' >> spaces)
<?> "enum members"
case DeclarationSet.fromList members' of
Left (BehindNameDuplication (Name _ bname)) ->
Expand Down Expand Up @@ -387,12 +425,12 @@ fieldSet = do
fields' <- fields <?> "fields"
handleNameDuplication "field" fields' return

recordTypeDeclaration :: Parser TypeDeclaration
recordTypeDeclaration = do
recordTypeDeclaration :: [Identifier] -> Parser TypeDeclaration
recordTypeDeclaration forwardNames = do
annotationSet' <- annotationSet <?> "record type annotations"
string "record" <?> "record keyword"
spaces
typeName <- name <?> "record type name"
typeName <- uniqueName forwardNames "record type name"
spaces
char '('
spaces
Expand All @@ -408,13 +446,13 @@ recordTypeDeclaration = do
annotationSet'' <- annotationsWithDocs annotationSet' docs'
return $ TypeDeclaration typeName (RecordType fields') annotationSet''

tag :: Parser (Tag, Bool)
tag = do
tag :: [Identifier] -> Parser (Tag, Bool)
tag forwardNames = do
annotationSet' <- annotationSet <?> "union tag annotations"
spaces
default' <- optional (string "default" <?> "default tag")
spaces
tagName' <- name <?> "union tag name"
tagName' <- uniqueName forwardNames "union tag name"
spaces
paren <- optional $ char '('
spaces
Expand Down Expand Up @@ -444,21 +482,23 @@ tag = do
Nothing -> False
)

unionTypeDeclaration :: Parser TypeDeclaration
unionTypeDeclaration = do
unionTypeDeclaration :: [Identifier] -> Parser TypeDeclaration
unionTypeDeclaration forwardNames = do
annotationSet' <- annotationSet <?> "union type annotations"
string "union" <?> "union keyword"
spaces
typeName <- name <?> "union type name"
typeName <- uniqueName forwardNames "union type name"
spaces
docs' <- optional $ do
d <- docs <?> "union type docs"
spaces
return d
char '='
spaces
tags' <- (tag `sepBy1` try (spaces >> char '|' >> spaces))
<?> "union tags"
tags' <- sepBy1
(tag forwardNames)
(try (spaces >> char '|' >> spaces))
<?> "union tags"
let tags'' = [t | (t, _) <- tags']
let defaultTag' = do
(t''', _) <- L.find snd tags'
Expand All @@ -473,19 +513,23 @@ unionTypeDeclaration = do
unionType tags'' defaultTag'
return $ TypeDeclaration typeName ut annotationSet''

typeDeclaration :: Parser TypeDeclaration
typeDeclaration = do
typeDeclaration :: [Identifier] -> Parser TypeDeclaration
typeDeclaration forwardNames = do
-- Preconsume the common prefix (annotations) to disambiguate
-- the continued branches of parsers.
spaces
annotationSet' <- annotationSet <?> "type annotations"
spaces
typeDecl <- choice
[ unless' ["union", "record", "enum", "unboxed"] aliasTypeDeclaration
, unless' ["union", "record", "enum"] unboxedTypeDeclaration
, unless' ["union", "record"] enumTypeDeclaration
, unless' ["union"] recordTypeDeclaration
, unionTypeDeclaration
[ unless' ["union", "record", "enum", "unboxed"]
(aliasTypeDeclaration forwardNames)
, unless' ["union", "record", "enum"]
(unboxedTypeDeclaration forwardNames)
, unless' ["union", "record"]
(enumTypeDeclaration forwardNames)
, unless' ["union"]
(recordTypeDeclaration forwardNames)
, unionTypeDeclaration forwardNames
] <?> "type declaration (e.g. enum, record, unboxed, union)"
-- In theory, though it preconsumes annotationSet' before parsing typeDecl
-- so that typeDecl itself has no annotations, to prepare for an
Expand Down Expand Up @@ -547,12 +591,12 @@ methodSet = do
methods' <- methods <?> "service methods"
handleNameDuplication "method" methods' return

serviceDeclaration :: Parser TypeDeclaration
serviceDeclaration = do
serviceDeclaration :: [Identifier] -> Parser TypeDeclaration
serviceDeclaration forwardNames = do
annotationSet' <- annotationSet <?> "service annotation"
string "service" <?> "service keyword"
spaces
serviceName' <- name <?> "service name"
serviceName' <- uniqueName forwardNames "service name"
spaces
char '('
spaces
Expand Down Expand Up @@ -619,16 +663,24 @@ module' = do
importList <- imports
spaces
return importList
types <- many $ do
let imports' = [i | l <- importLists, i <- l]
types <- many' imports' $ \ tds -> do
typeDecl <- do
-- Preconsume the common prefix (annotations) to disambiguate
-- the continued branches of parsers.
spaces
annotationSet' <- annotationSet <?> "annotations"
spaces
decl <- choice [ notFollowedBy (string "service") >> typeDeclaration
, serviceDeclaration <?> "service declaration"
]
let forwardNames =
[ n
| td <- tds
, n <- declFacialName td : toList (D.extraPublicNames td)
]
decl <- choice
[ notFollowedBy (string "service")
>> typeDeclaration forwardNames
, serviceDeclaration forwardNames <?> "service declaration"
]
-- In theory, though it preconsumes annotationSet' before parsing
-- decl so that decl itself has no annotations, to prepare for an
-- unlikely situation (that I bet it'll never happen)
Expand All @@ -642,7 +694,7 @@ module' = do
_ -> decl -- Never happen!
spaces
return typeDecl
handleNameDuplication "type" (types ++ [i | l <- importLists, i <- l]) $
handleNameDuplication "type" types $
\ typeSet -> return $ Module typeSet docs'

file :: Parser Module
Expand Down
Loading

0 comments on commit f7d40c8

Please sign in to comment.