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 class info to not-nil assertion of getter! #8296

Merged

Conversation

@icy-arctic-fox
Copy link
Contributor

icy-arctic-fox commented Oct 8, 2019

Continuation of #8200 to address some comments about adding class information. Thought it was a good idea.

The error messages now look like:

Class#some_property cannot be nil
Class.some_class_property cannot be nil

Added type annotations to generated getter methods when they are available.

src/object.cr Outdated Show resolved Hide resolved
@icy-arctic-fox icy-arctic-fox force-pushed the icy-arctic-fox:not-nil-error-message branch from df4727f to b7dece0 Oct 8, 2019
src/object.cr Outdated Show resolved Hide resolved
@icy-arctic-fox icy-arctic-fox marked this pull request as ready for review Oct 8, 2019
def {{method_prefix}}\{{name.id}}?
{{var_prefix}}\{{name.id}}
end
def {{method_prefix}}\{{name.var.id}}? : \{{name.type}}?

This comment has been minimized.

Copy link
@Blacksmoke16

Blacksmoke16 Oct 8, 2019

Contributor

To prevent some duplication you could prob do like {% if name.is_a?(TypeDeclaration) %} : \{{name.type}}? {% end %} Or something along those lines. As opposed to duplicating each method with only diff being the type restrictions.

This comment has been minimized.

Copy link
@icy-arctic-fox

icy-arctic-fox Oct 8, 2019

Author Contributor

I had that originally, but then I noticed that all the other macros use this pattern, so I made it consistent. I'd be fine to change it.

Copy link
Member

sdogruyol left a comment

Thank you @icy-arctic-fox 🙏

@straight-shoota

This comment has been minimized.

Copy link
Member

straight-shoota commented Oct 11, 2019

@icy-arctic-fox Could you please squash this branch into two commits, one adding type name in error message, the other adding type annotations?

Adds the type name to the getter! macro.
Type annotations are applied when they are provided to the macro.
@icy-arctic-fox icy-arctic-fox force-pushed the icy-arctic-fox:not-nil-error-message branch from 111c213 to ee01a89 Oct 11, 2019
@icy-arctic-fox

This comment has been minimized.

Copy link
Contributor Author

icy-arctic-fox commented Oct 11, 2019

Done and done.

@straight-shoota straight-shoota merged commit 92b9264 into crystal-lang:master Oct 11, 2019
6 checks passed
6 checks passed
ci/circleci: check_format Your tests passed on CircleCI!
Details
ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
ci/circleci: test_linux32_std Your tests passed on CircleCI!
Details
ci/circleci: test_preview_mt Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.