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

Can't use property name :: Type when name is a keyword #284

Closed
asterite opened this issue Dec 2, 2014 · 5 comments
Closed

Can't use property name :: Type when name is a keyword #284

asterite opened this issue Dec 2, 2014 · 5 comments

Comments

@asterite
Copy link
Member

asterite commented Dec 2, 2014

Right now we support this:

class Person
  property name :: String
end

This makes use of the name :: String syntax, which is used to declare a variable with a type. The AST node representing this expression is what get passed to the property macro.

Buf this doesn't work:

class Event
  property end :: Time
end

The problem is that end :: Time is not a valid expression, because end is a keyword and you can't have a variable named end.

This innocent limitation becomes harder to detect when a macro in turn uses this macro. For example until this commit you couldn't declare a json mapping with keywords as names. That commit's "solution" was to write the contents of the property macro with a bit more thought. But it's (just a bit) annoying and make code reusability less effective.

This happened to me once, and now it happened to someone else. That means we have to do something about it.

Possible solutions:

  1. Have a different macro, typed_property, that would accept two AST nodes: typed_property end, Time. With this option we could remove the property foo :: Bar syntax to avoid these mistakes, or just leave both options so you can choose the one that's more readable to you.
  2. Make property (and similar macros) not accept a list of nodes. So doing property foo, bar, baz would be impossible. But add an overload to property that accepts two arguments: property foo, Int32.
  3. Think of another AST node that we can use that has a name and a Type.
  4. Allow end :: Time to be a valid expression. The "problem" is that this doesn't make sense in real code as you can't later refer to the end variable, so would be a hack in my opinion (but a pragmatic hack :-P).
  5. Leave things as they are. Just be careful when using property with a type restriction in other macros.
  6. Other ideas...?
@vendethiel
Copy link
Contributor

pragmatic

That's a terrible word :).

Can.t you self.end later on?

@sferik
Copy link
Contributor

sferik commented Dec 2, 2014

I just ran into this issue here.

@vendethiel
Copy link
Contributor

I just ran into this issue here.

It's actually in the first post, "happened to someone else" :)

@papamarkou
Copy link
Contributor

I was thinking that the type of input arguments to methods are specified via single colons, so this tempts me to think that using double colons to specify the type of properties is not so desired due to lack of unification. One option would be to use property end : Time, or (in case this causes any unwanted side-effects), use property end, Time (this is option 2 above).

@papamarkou papamarkou mentioned this issue Dec 2, 2014
@asterite asterite removed the kind:bug label Aug 26, 2015
@asterite
Copy link
Member Author

I'll close this. In the case of JSON mapping we do it another way. For other macros that would't like to delegate to this property syntax, the solution is to use write the setters/getters manually (it's just a few lines).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants