Fix return type of Mix.Task.alias?/1 #6310
Conversation
This function is currently returning either `nil` or the alias itself from the mix file, so it doesn't match with the `boolean` return type indicated by the documentation.
| @@ -193,7 +193,10 @@ defmodule Mix.Task do | |||
| end | |||
|
|
|||
| def alias?(task) when is_atom(task) do | |||
| Mix.Project.config[:aliases][task] | |||
There was a problem hiding this comment.
What about !!Mix.Project.config[:aliases][task]?
Can you also please change the tests to check for == true and == false accordingly?
Thank you!
There was a problem hiding this comment.
I pushed up some changes. Thanks!
There was a problem hiding this comment.
Maybe use != nil? It feels more descriptive to me. 
There was a problem hiding this comment.
Hm maybe, but wouldn't that be leaving the tests open to passing on truthy values?
There was a problem hiding this comment.
@npbee x != nil only differs from !!x when x is false, because false != nil #=> true while !!false #=> false, but since [] here returns either a non-boolean term or nil, they are equivalent here. Since :aliases is a keyword list, can't we do Keyword.has_key?(Mix.Project.config()[:aliases], task)?
There was a problem hiding this comment.
Ah, right thanks! I think I was confused thinking that we were referring to using != nil on the assertions.
I do like either != nil or Keyword.has_key? for being more descriptive. Is there an advantage to using Keyword.has_key?
There was a problem hiding this comment.
Descriptiveness :) If we can, let's go with that.
There was a problem hiding this comment.
Ok sounds good to me! Pushed up the change.
Update the tests to assert explict `true` or `false` values instead of truthy/falsy values.
lexmag
changed the title
Mix.Task.alias?
|
❤️ 💚 💙 💛 💜 |
This function is currently returning either
nilor the alias itself from themix file, so it doesn't match with the
booleanreturn type indicated by thedocumentation.
Thanks!