-
-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
define $.Function and $.UnaryTypeVariable #79
Conversation
This looks great, David! I should have a chance to dig into the code tomorrow evening. |
@@ -219,41 +164,43 @@ | |||
}; | |||
}; | |||
|
|||
switch (typeOf(x)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any particular reason to remove typeOf
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was part of why quest to reduce the number of helper functions. It was only used in two places so I decided it wasn't pulling its weight. toString.call(x)
has the advantage of being more explicit too, once one is aware that toString
is a reference to Object.prototype.toString
.
I'm happy to reinstate typeOf
if you think it improves readability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, not at all I think you're right to make things more explicit.
779fc34
to
ccf8b8e
Compare
var result = t.type.validate(ys[idx2]); | ||
if (result.isLeft) { | ||
var value = result.value.value; | ||
var propPath = [k].concat(result.value.propPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason not to use local mutation here, I seem to remember some issue from a while back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may be thinking of #53. I think it would be okay to use local mutation here, but it would take significant mental effort to convince myself that no one else is holding a reference to the array. I don't see a significant downside to creating a new array here, particularly since we're about to short-circuit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, best leave it as it is.
0ba0bb6
to
41b0b77
Compare
'\n' + | ||
'1) "foo" :: String\n' + | ||
'\n' + | ||
'The value at position 1 is not a member of ‘FiniteNumber’.\n')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a result of sanctuary-js/sanctuary#234 I realized we lacked tests for the two cases above. After writing the tests it was apparent that we also lacked logic to handle these two cases. I've made some changes so these tests now pass.
/cc @jdudek
41b0b77
to
0c69ad6
Compare
0c69ad6
to
bc7bf30
Compare
26652da
to
5f8b5c1
Compare
5f8b5c1
to
5d40326
Compare
TL;DR we can now do this:
This pull request sits atop #78 and should not be merged until #78 has been merged.
sanctuary-js/sanctuary#232 is heavily dependent on this pull request.
This pull request adds support for what I believe to be the two missing pieces of the puzzle:
A wonderful consequence of this change is that type signatures in error messages will now match the type signatures which appear in documentation – no more
map :: Function -> a -> b
!$.Function :: Array Type -> Type
The type formerly known as
$.Function
is now known as$.AnyFunction
.$.Function
is now a constructor for function types.Examples:
$.Function([$.Date, $.String])
represents theDate -> String
type; and$.Function([a, b, a])
represents the(a, b) -> a
type.What will happen when one evaluates
formatDates(d => d.toISOString(), [new Date(0)])
?d => d.toISOString()
is in fact a function (of some sort).[new Date(0)]
is in fact an array.new Date(0)
is in fact a Date object.f
will be decorated along these lines:implementation
will be applied to the above function anddates
.$.Function
takes an array of types just asdef
does.$.Function
can thus be used to represent functions of any arity (including nullary functions). The downside of this approach is that representing curried functions is awkward:a -> b -> a
, for example, is$.Function([a, $.Function([b, a])])
. The solution is to define a binary type constructor:One can then express
a -> b -> a
asFn(a, Fn(b, a))
.$.UnaryTypeVariable :: String -> Type -> Type
This essentially combines
$.TypeVariable
and$.UnaryType
. See the readme for details.Limitations
Only functions at the top level of the types array are wrapped.
The function in
[$.Array($.Function([a, b])), $.Array(a), $.Array(b)]
, for example, will not be wrapped. I hope to address this limitation in the future.Breaking API changes
$.Function
has been repurposed. When upgrading the most straightforward substitution is:%s/\$\.\<Function\>/$.AnyFunction/g
, but in most cases one will wish to faithfully describe the types of one's higher-order functions (using the new$.Function
).._1
and._2
. They are now exposed via.types.$1.extractor
and.types.$2.extractor
..$1
and.$2
. They are now exposed via.types.$1.type
and.types.$2.type
..fields
. The type of thex
field, for example, was previously exposed via.fields.x
. It is now exposed via.types.x.type
. The similarity between RecordType values and values of parameterized types is intentional, and allows code sharing where previously branching logic was required. If one squints one can see a type such asPair a b
as a record type:{ $1 :: a, $2 :: b }
.Style changes
[a]
is now writtenArray a
.Array#map
,Object.keys
, and other useful ES5 functions.Nothing :: Maybe a
rather thanNothing :: -> Maybe a
.Refactoring
It took many, many hours but I was successful in creating a function,
createType
, which makes it clear what is required of a type and removes duplication.$.TypeVariable
, before:$.TypeVariable
, after:$.UnaryType
, before:$.UnaryType
, after:$.Function
and$.UnaryTypeVariable
are also defined viacreateType
.This is a great opportunity to familiarize yourself with this codebase if you're interested but haven't known where to begin. The code should be easier to follow now, though there are still several functions which take six, seven, or even nine (!) arguments without many comments beyond the type annotations. I'm happy to answer questions; please let me know if anything is unclear. Future readers (including future me) will appreciate anything we can do now to clarify the code.