lib/types: Fix coercedTo check #36638
Conversation
Without this change
(coercedTo str toInt int).check "foo"
would evaluate to true, even though
(coercedTo str toInt int).merge {} [{ value = "foo"; }]
will throw an error because "foo" can't be coerced to an int.
|
So this adds both the check that |
|
Ping @rycee and @Profpatsch |
|
Can you add a succeeding/failing test pair to |
|
@Profpatsch I added some tests. One thing I'm not sure about is whether this assert: Line 430 in db5f8fa check already, so it shouldn't even get to that point with an invalid type.
|
|
Now only the two uses of |
|
@Profpatsch Pretty sure that's not possible because the value to check occurs in the function argument to those two attributes, and the attributes can in theory be used separately anyways. A question: I changed the name of types.coercedTo str toInt int # "int or string convertible to it"because it makes more sense instead of |
|
I like the improved documentation. |
There was a problem hiding this comment.
Sorry for the delay.
- I prefer the new wording as it explicit the manipulated type. (maybe preferred?)
- Thanks for adding tests
- I do not think the overhead would be a big deal at the moment, and if it is, then this can probably be separated in multiple options.
|
Nice, thanks! I will merge then. |
Without this change
would evaluate to true, even though
will throw an error because "foo" can't be coerced to an int.
Motivation for this change
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)