Conversation
…Vici37/cr-source-typer) and running it with the below incantation: ``` CRYSTAL_PATH="./src" ./typer spec/std_spec.cr \ --error-trace --exclude src/crystal/ \ --stats --progress \ --union-size-threshold 2 \ --ignore-private-defs \ src/spec ``` This is related to crystal-lang#15682 .
| def negative_failure_message(actual_value : Bool?) : String | ||
| "Expected: #{actual_value.pretty_inspect} not to be truthy" |
There was a problem hiding this comment.
If the message is correct and this accept truthy values, then Bool? is too restrictive.
| end | ||
|
|
||
| def failure_message(actual_value) | ||
| def failure_message(actual_value : Bool?) : String |
straight-shoota
left a comment
straight-shoota
left a comment
There was a problem hiding this comment.
I don't think it makes much sense to add type restrictions to undocumented, internal APIs. They mostly create noise and require review effort without much benefit.
Just glancing over it, I notice a few odd things but I don't want to put in the effort to review and fix them. It's just not worth it.
This obviously doesn't mean we should never use type restrictions on internal APIs, but it be for a reason. Not sprinkle some autogenerated type annotations everywhere.
|
Yup, fair enough. I'm fine closing this PR based on that @straight-shoota - the crystal core team would be the main consumers of these internal API type restrictions, and if you don't want them, then they serve no purpose :) |
|
The methods in |
|
I'll create a new PR for just that file then. Current new feature for the typifier is to optionally ignore |
3 participants
This is the output of compiling cr-source-typer and running it with the below incantation:
This is related to #15682 .
This touched more than I expected, but everything looks correct to me. Comments always welcome :)