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

Add to reserved keywords #29

Merged
merged 3 commits into from
Feb 10, 2020

Conversation

yucombinator
Copy link
Contributor

@yucombinator yucombinator commented Feb 7, 2020

It is no longer possible to declare types that overrides existing builtins since HHVM 4.28, so we need to add to the list of reserved keywords to avoid possible collisions.

https://hhvm.com/blog/2019/10/23/hhvm-4.28.html

Declaring a type with the same name as a built-in type (e.g. int, Vector, Awaitable) will no longer override the built-in type. Such types can now only be referenced using their namespaced name (e.g. MyNamespace\Vector, or namespace\Vector if in the same namespace).

https://hhvm.com/blog/2019/11/06/hhvm-4.30.html

it is no longer possible to override built-in types using use statements (use MyVector as Vector, use MyType as string, etc.)

This PR adds a list of reserved keywords (possibly might be missing some?). Also ran gofmt over plugin.go

A list of reserved words can be found at https://github.com/facebook/hhvm/blob/master/hphp/hack/src/naming/naming_special_names.rs

@@ -29,7 +30,17 @@ var (
version = "undefined" // go build -ldflags "-X main.version=1"
fversion = flag.Bool("version", false, "print version and exit")
reservedKeywords = []string{"eval", "isset", "unset", "empty", "const", "new", "and", "or",
"xor", "as", "print", "throw", "array", "instanceof", "trait", "class", "interface", "static", "self"}
"xor", "as", "print", "throw", "array", "instanceof", "trait", "class", "interface", "static", "self",
Copy link

Choose a reason for hiding this comment

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

I think either the words in this list should be lowercased or they should be lowercased before comparison here:

for _, keyword := range reservedKeywords {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, I've updated the PR but lowercasing keywords on line 246

@y3llowcake
Copy link
Owner

Can you describe the motivation for this change in a bit more detail? Did you run into a specific name-collision, if so, which one?

A number of the elements you are adding are not "language keywords" they are names from [namespaced] library code. For example, if I create a protobuf with a package named "foobar" a message named "Iterable", does anything actually break?

@y3llowcake y3llowcake merged commit 9426911 into y3llowcake:master Feb 10, 2020
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.

3 participants