Issue with Crystal Nightly

[mamantoha]

How to reproduce:

crystal spec

Showing last frame. Use --error-trace for full trace.

There was a problem expanding macro 'macro_140716797307408'

Code in /var/lib/snapd/snap/crystal/1627/share/crystal/src/yaml/serialization.cr:188:7

 188 | {% begin %}
       ^
Called macro defined in /var/lib/snapd/snap/crystal/1627/share/crystal/src/yaml/serialization.cr:188:7

 188 | {% begin %}

Which expanded to:

 > 69 |                   
 > 70 | 
 > 71 |                   __temp_744 =
                          ^
Error: type must be Ameba::Severity, not (Ameba::Severity | Nil)

Probably related to recent changes in Crystal - https://github.com/crystal-lang/crystal/pull/13433

[Sija]

Most likely a duplicate of https://github.com/crystal-ameba/ameba/pull/366

@mamantoha Could you please check if the version from master works for you?

[straight-shoota]

Yes, this is literally the same error message as #371. It should not only appear with Crystal nightly but since 1.8.0.

[Sija]

@straight-shoota There's no error for Crystal 1.8.2.

[Sija]

@straight-shoota Also, the #366 fixed the type in the specs, and not in the main application code.

[mamantoha]

@Sija @straight-shoota I checked this on master.

No issue on Crystal 1.8.1

Crystal 1.8.1 [a59a3dbd7] (2023-04-20)

LLVM: 15.0.7
Default target: x86_64-unknown-linux-gnu

Additionally, I updated the Crystal snap from the edge channel and the error no longer appears.

crystal -v
Crystal 1.8.2 [7aa5cdd86] (2023-05-09)

LLVM: 15.0.7
Default target: x86_64-unknown-linux-gnu

[Sija]

Interesting... perhaps an edge-case regression?

[mamantoha]

Perhaps. If the issue reappears, I will reopen the ticket.

[Sija]

@straight-shoota The issue still persists with Crystal 1.9.0-dev, so sth seems broken there...

[mamantoha]

I have confirmed the issue on the Crystal master branch by testing it with CRYSTAL_PATH. Setting the CRYSTAL_PATH to /path/to/crystal/src:lib and running crystal spec resulted in the same error as described in the ticket.

[straight-shoota]

The base issue is the same as #366 though: SeverityYamlConverter.from_yaml returns Severity? and that value is assigned to a variable of type Severity. This worked before only because the Nil type was silently ignored (an actual nil value would've caused a runtime exception).

This error now appears since https://github.com/crystal-lang/crystal/issues/13433 regardless of the fix in #366 because it affects a different location. For some reason this code path was previously not touched by the changes to JSON::Serializable in 1.8.x.

To resolve this, the return type of SeverityYamlConverter.from_yaml needs to be assignable to the instance variable. It's probably best to drop the Nil type and raise directly in the converter.

[Sija]

@mamantoha Could you try ameba's master branch?

[zw963]

Test on 1.5.0, it works, thank you!