Fix breakages caused by Crystal 1.13 #231
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This causes `require "clear"` to fail using crystal 1.13.0
I've not been able to determine whether this was an intentional change of not, but Crystal 1.13.0 changed the way nil is represented in the case that a nilable type was defined with the `?` shorthand:
macro print_types(name)
{{ p name.type.types }}
end
print_types foo : String?
# => [String, ::Nil]
print_types foo : String | Nil
# => [String, Nil]
This broke the nilable computation in the `has_one` and `belongs_to` macros, resulting in having to define associations thus as as a workaround:
belongs_to foo : Foo | Nil
instead of
belongs_to foo : Foo?
Note that since this approach requires that the associated type is resolvable when the association macro is called, it may necessitate adding some additional `require`s, and is probably therefore not a change to be included in a patch release.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
There were 2 issues:
The first, 653c74c, was causing compilation to fail. I didn't investigate further because this module was clearly incorrectly (it was defining
Clear::Migration::Clear::Migration::Helper).The second issues only affected
has_manyandbelongs_toassociations which were nilable (and defined using the?shorthand). The reason for the breakage and a minimal fix is included in 0aa9beb. A more robust fix is then added in 8cc27f7, but with the caveat that some code may need to be updated with additionalrequires (so that the union type can be resolved in the macro). I would be happy to remove 8cc27f7 from this PR in order to get the minimal fix merged and then to suggest the refactoring later.Motivation and Context
Clear can't currently be used on Crystal 1.13.0 or above. Closes #230.
How Has This Been Tested?
Tests have been run, and the code has been tested against a production codebase.
Types of changes
Checklist:
bin/amebaran without alert.