Swap priority order of bools/strings if strictBooleans is on #6044
Conversation
|
Oh wait, I should update the docs in this PR as well... will have to go back and add that... |
|
OK, updated this with documentation! |
| @@ -10,6 +10,7 @@ import { maxValue, minValue, places } from "./utils"; | |||
| export function isMoreSpecificMultiple( | |||
| types1: Format.Types.OptionallyNamedType[], | |||
| types2: Format.Types.OptionallyNamedType[], | |||
| strictBooleans: boolean, | |||
There was a problem hiding this comment.
I think this should be in an options style argument
There was a problem hiding this comment.
It could be if you really want, but this is sufficiently internal that I'm not really worried about that sort of thing -- there isn't a need to maintain a stable interface here, we can just change it as needed. Adding yet another options here seems more confusing than just adding a flag.
There was a problem hiding this comment.
I really think we should always lean in favor of options-style arguments, but I won't press the issue here
packages/codec/lib/wrap/priority.ts
Outdated
| typeClasses.pop(); | ||
| typeClasses.pop(); |
There was a problem hiding this comment.
This looks brittle; can we do it another way?
There was a problem hiding this comment.
Sure, there's any number of alternative ways to do it, the question is what's good without being too repetitive. I guess we could have everything but the last two, then tack on the last two depending on the flag? Would that be less brittle?
There was a problem hiding this comment.
(I've made this change, let me know if you think a different way would be better.)
Addresses #6028.
The point of this is that, if there's an overloaded function with a bool overload and a string overload, then
truffle call Contract f trueought to hit the bool overload, not the string overload. Of course this only makes sense if we're restricted to string input, which is more or less what thestrictBooleansflag means.I added tests also. You could also test it manually by making a function with a
stringoverload and abooloverload and try calling it withtruffle call.Arguably this is a breaking change (to codec and encoder), but I'd consider it a bug fix. Overload resolution is never guaranteed anyway! :)