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

Class name collision #246

Merged
merged 7 commits into from
Nov 19, 2020
Merged

Class name collision #246

merged 7 commits into from
Nov 19, 2020

Conversation

TwitchBronBron
Copy link
Member

@TwitchBronBron TwitchBronBron commented Nov 19, 2020

Catch when local variables and scope functions have the same name as a class.

Fixes #245

@@ -590,6 +590,11 @@ export let DiagnosticMessages = {
message: `There are multiple components with the name '${componentName}'`,
code: 1115,
severity: DiagnosticSeverity.Error
}),
functionCannotHaveSameNameAsClass: (className: string) => ({
message: `Function cannot have the same name as class '${className}'`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe more consistent with LocalVar message?

Copy link
Member Author

Choose a reason for hiding this comment

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

@elsassph which LocalVar message are you referring to?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, this one: localVarSameNameAsClass. Yeah, good call. It's subtle, but they should be consistent.

src/Scope.ts Outdated
if (lowerFuncName) {

//find function declarations with the same name as a stdlib function
if (globalCallableMap.has(func.getName(ParseMode.BrighterScript))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use lowerFuncName?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh yeah, good catch! I fixed it and added this unit test to ensure it doesn't regress.

@TwitchBronBron TwitchBronBron merged commit d76a7d3 into master Nov 19, 2020
@TwitchBronBron TwitchBronBron deleted the class-name-collision branch November 19, 2020 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show diagnostic error when a variable name or function name overwrites/collides with a Class name
2 participants