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

Remove reduntant “?” from type definitions in bang accessors! #3914

Merged

Conversation

Sija
Copy link
Contributor

@Sija Sija commented Jan 17, 2017

This PR removes redundant nilable ? symbol from getter/setter/accessor! type declarations throughout the code since they're added in macro definition already.

@spalladino spalladino added this to the Next milestone Jan 19, 2017
@spalladino spalladino merged commit a38087e into crystal-lang:master Jan 19, 2017
@spalladino
Copy link
Contributor

Good catch @Sija, thanks!

@RX14
Copy link
Contributor

RX14 commented Jan 19, 2017

Would it be possible or advisable to add this as a special-case formatter rule? Especially as they're macros not syntax.

@asterite
Copy link
Member

@RX14 The formatter has no notion of semantic, and one could redefine property! inside another type

@RX14
Copy link
Contributor

RX14 commented Jan 19, 2017

@asterite that's what I was thinking, I wasn't sure if the formatter would be able to tell if it was the stdlib property! or not.

@bcardiff
Copy link
Member

But the property! macro could raise a compilation error if the type is nilable. 💡

@Sija
Copy link
Contributor Author

Sija commented Jan 19, 2017

another option could be IMHO to make compiler disallow double ? modifier.

@bcardiff
Copy link
Member

@Sija that would be a check in a syntax level that incomplete. Trying to do it at a semantic level would be to disallow unions with more than one Nil, but there are path where you can get unions with two nillables, or build across macros invocation. Hard or imposible to differentiate.

Given that a property! could be called from other macro it might block the user if the generated code plays with Nil and wraps types on the go. I think there is nothing else to do for the time being.

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

Successfully merging this pull request may close these issues.

None yet

5 participants