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.

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

[gbc] gbc should reject methods with same name #388

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ different versioning scheme, following the Haskell community's
reader/writer protocols without having to explicitly compute the full
cross product.
* Add gbc flags to pick which C# files to generate (structs, gRPC, and comm).
* gbc ensures that method names are unique [Issue #381](https://github.com/Microsoft/bond/issues/381)
Copy link
Member

Choose a reason for hiding this comment

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

Minor: "...name are unique with a service."?

Copy link
Member Author

Choose a reason for hiding this comment

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

within?


### C++ ###

Expand Down
15 changes: 11 additions & 4 deletions compiler/src/Language/Bond/Parser.hs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ parseBond ::
-> String -- ^ content of a schema file to parse
-> FilePath -- ^ path of the file being parsed, used to resolve relative import paths
-> ImportResolver -- ^ function to resolve and load imported files
-> IO (Either ParseError Bond) -- ^ function returns 'Bond' which represents the parsed abstract syntax tree
-> IO (Either ParseError Bond) -- ^ function returns 'Bond' which represents the parsed abstract syntax tree
-- or 'ParserError' if parsing failed
parseBond s c f r = runReaderT (runParserT bond (Symbols [] []) s c) (Environment [] [] f r)

Expand Down Expand Up @@ -240,8 +240,9 @@ struct = do
[] -> return fields'
Field {..}:_ -> fail $ "Duplicate definition of the field with ordinal " ++ show fieldOrdinal ++
" and name " ++ show fieldName
where
findDuplicatesBy accessor xs = deleteFirstsBy ((==) `on` accessor) xs (nubBy ((==) `on` accessor) xs)

findDuplicatesBy :: (Eq b) => (a -> b) -> [a] -> [a]
Copy link
Member

Choose a reason for hiding this comment

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

Minor: this feels like an odd place in the file for such a helper method. Move to near beginning/end?

findDuplicatesBy accessor xs = deleteFirstsBy ((==) `on` accessor) xs (nubBy ((==) `on` accessor) xs)

manySortedBy :: (a -> a -> Ordering) -> ParsecT s u m a -> ParsecT s u m [a]
manySortedBy = manyAccum . insertBy
Expand Down Expand Up @@ -410,7 +411,13 @@ service = do
local (with params) $ Service namespaces attr name params <$> methods <* optional semi
where
with params e = e { currentParams = params }
methods = braces $ semiEnd (try event <|> try function)
methods = unique $ braces $ semiEnd (try event <|> try function)
unique p = do
methods' <- p
case findDuplicatesBy methodName methods' of
[] -> return methods'
Function {..}:_ -> fail $ "Duplicate definition of the function with name " ++ show methodName
Event {..}:_ -> fail $ "Duplicate definition of the event with name " ++ show methodName

function :: Parser Method
function = Function <$> attributes <*> payload <*> identifier <*> input
Expand Down
1 change: 1 addition & 0 deletions compiler/tests/Main.hs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ tests = testGroup "Compiler tests"
, testCase "Enum no default value" $ failBadSyntax "Should fail when an enum field has no default value" "enum_no_default"
, testCase "Alias default value" $ failBadSyntax "Should fail when underlying default value is of the wrong type" "aliases_default"
, testCase "Out of range" $ failBadSyntax "Should fail, out of range for int16" "int_out_of_range"
, testCase "Duplicate method definition in service" $ failBadSyntax "Should fail, method name should be unique" "duplicate_method_service"
]
, testGroup "Codegen"
[ utilTestGroup,
Expand Down
12 changes: 12 additions & 0 deletions compiler/tests/schema/error/duplicate_method_service.bond
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace tests
Copy link
Member

Choose a reason for hiding this comment

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

Minor: consider naming this file duplicate_service_method.bond


struct dummy {
0: int32 count;
}

// Invalid service definition with two methods with the same name
service Foo
{
void bar(dummy);
nothing bar();
}