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
WIP: Transform for F# Pipeline #1
Changes from 3 commits
bbb8a14
573fe40
34c4d70
4811f08
ecbb465
f50bd7a
63b7c04
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
import { types as t } from "@babel/core"; | ||
import maybeOptimizePipelineSequence from "./maybeOptimizePipelineSequence"; | ||
|
||
const fsharpVisitor = { | ||
BinaryExpression(path) { | ||
const { scope } = path; | ||
const { node } = path; | ||
const { operator, left, right } = node; | ||
if (operator !== "|>") return; | ||
|
||
const placeholder = scope.generateUidIdentifierBasedOnNode(left); | ||
scope.push({ id: placeholder }); | ||
|
||
const call = | ||
right.type === "AwaitExpression" | ||
? t.awaitExpression(t.cloneNode(placeholder)) | ||
: t.callExpression(right, [t.cloneNode(placeholder)]); | ||
const sequence = t.sequenceExpression([ | ||
t.assignmentExpression("=", t.cloneNode(placeholder), left), | ||
call, | ||
]); | ||
path.replaceWith(sequence); | ||
maybeOptimizePipelineSequence(path); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of creating a temporary path for the sequence expression and then optimizing it away, would it be possible to do something like this? path.replaceWith(
buildOptimizedSequenceExpression({
assign: t.assignmentExpression("=", t.cloneNode(placeholder), left),
call,
path,
})
); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started with something similar to this but ended up passing the path in because I needed it for the |
||
}, | ||
}; | ||
|
||
export default fsharpVisitor; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
import { types as t } from "@babel/core"; | ||
|
||
// tries to optimize sequence expressions in the format | ||
// (a = b, ((c) => d + e)(a)) | ||
// to | ||
// (a = b, a + e) | ||
const maybeOptimizePipelineSequence = path => { | ||
const [assignNode, callNode] = path.node.expressions; | ||
const { left: placeholderNode, right: pipelineLeft } = assignNode; | ||
const { callee: calledExpression } = callNode; | ||
|
||
let optimizeArrow = | ||
t.isArrowFunctionExpression(calledExpression) && | ||
t.isExpression(calledExpression.body) && | ||
!calledExpression.async && | ||
!calledExpression.generator; | ||
let param; | ||
|
||
if (optimizeArrow) { | ||
const { params } = calledExpression; | ||
if (params.length === 1 && t.isIdentifier(params[0])) { | ||
param = params[0]; | ||
} else if (params.length > 0) { | ||
optimizeArrow = false; | ||
} | ||
} else if (t.isIdentifier(calledExpression, { name: "eval" })) { | ||
const evalSequence = t.sequenceExpression([ | ||
t.numericLiteral(0), | ||
calledExpression, | ||
]); | ||
|
||
path.get("expressions.1.callee").replaceWith(evalSequence); | ||
} | ||
|
||
if (optimizeArrow && !param) { | ||
// Arrow function with 0 arguments | ||
path.replaceWith( | ||
t.sequenceExpression([pipelineLeft, calledExpression.body]), | ||
); | ||
return; | ||
} | ||
|
||
if (param) { | ||
path | ||
.get("expressions.1.callee.body") | ||
.scope.rename(param.name, placeholderNode.name); | ||
path.get("expressions.1").replaceWith(calledExpression.body); | ||
} | ||
}; | ||
|
||
export default maybeOptimizePipelineSequence; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
const y = 2; | ||
|
||
const f = (x) => (x | ||
|> (y) => y + 1) | ||
|> (z) => z * y | ||
|
||
const _f = (x) => x | ||
|> (y) => y + 1 | ||
|> (z) => z * y | ||
|
||
const g = (x) => x | ||
|> (y) => ( | ||
y + 1 | ||
|> (z) => z * y | ||
) | ||
|
||
const _g = (x) => x | ||
|> (y => ( | ||
y + 1 | ||
|> (z) => z * y) | ||
) | ||
|
||
const __g = (x) => x | ||
|> (y => { return ( | ||
y + 1 | ||
|> (z) => z * y); | ||
} | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All this random indentation makes it really hard to review this file 😛 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is hard for me too :-p I'm still getting the hang of how to indent pipelines. I'll see what I can do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see 4811f08 and let me know what you think. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah they look better. As a general rule, I think that if the pipeline body is multiline, return (
x => y
); Let's hope Prettier implements them soon 😛 |
||
|
||
expect( f(1)).toBe(4); | ||
expect( _f(1)).toBe(4); | ||
expect( g(1)).toBe(2); | ||
expect( _g(1)).toBe(2); | ||
expect(__g(1)).toBe(2); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
var result = [5,10] | ||
|> (_ => _.map(x => x * 2)) | ||
|> (_ => _.reduce( (a,b) => a + b )) | ||
|> (sum => sum + 1) | ||
|
||
expect(result).toBe(31); | ||
|
||
|
||
var inc = (x) => x + 1; | ||
var double = (x) => x * 2; | ||
|
||
var result2 = [4, 9].map( x => x |> inc |> double ) | ||
|
||
expect(result2).toEqual([10, 20]); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
var result = [5,10] | ||
|> (_ => _.map(x => x * 2)) | ||
|> (_ => _.reduce( (a,b) => a + b )) | ||
|> (sum => sum + 1) | ||
|
||
expect(result).toBe(31); | ||
thiagoarrais marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
var inc = (x) => x + 1; | ||
var double = (x) => x * 2; | ||
|
||
var result2 = [4, 9].map( x => x |> inc |> double ) | ||
|
||
expect(result2).toEqual([10, 20]); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
var _ref, _ref2, _ref3; | ||
|
||
var result = (_ref = (_ref2 = (_ref3 = [5, 10], _ref3.map(x => x * 2)), _ref2.reduce((a, b) => a + b)), _ref + 1); | ||
expect(result).toBe(31); | ||
|
||
var inc = x => x + 1; | ||
|
||
var double = x => x * 2; | ||
|
||
var result2 = [4, 9].map(x => { | ||
var _ref4, _x; | ||
|
||
return _ref4 = (_x = x, inc(_x)), double(_ref4); | ||
}); | ||
expect(result2).toEqual([10, 20]); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
const triple = (x) => x * 3; | ||
|
||
async function myFunction(n) { | ||
return n | ||
|> Math.abs | ||
|> Promise.resolve | ||
|> await | ||
|> triple; | ||
} | ||
|
||
return myFunction(-7).then(result => { | ||
expect(result).toBe(21); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
async function myFunction(n) { | ||
return n | ||
|> Math.abs | ||
|> Promise.resolve | ||
|> await; | ||
} | ||
thiagoarrais marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
{ | ||
"plugins": [ | ||
["proposal-pipeline-operator", { "proposal": "fsharp" }] | ||
], | ||
"parserOpts": { | ||
"allowReturnOutsideFunction": true | ||
}, | ||
"minNodeVersion": "8.0.0" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
async function myFunction(n) { | ||
var _ref, _ref2, _n; | ||
|
||
return _ref = (_ref2 = (_n = n, Math.abs(_n)), Promise.resolve(_ref2)), await _ref; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
var inc = (x) => x + 1 | ||
|
||
expect(10 |> inc).toBe(11); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
var inc = (x) => x + 1 | ||
|
||
expect(10 |> inc).toBe(11); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
var _; | ||
|
||
var inc = x => x + 1; | ||
|
||
expect((_ = 10, inc(_))).toBe(11); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If at least one of the following conditions is true, we could optimize it to
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. Will do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This also applies to minimal. Right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added this in 63b7c04. I'm marking the PR as WIP, though. I'm uncomfortable with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you prefer, we can work on this optimization in a separate PR, and keep the current one as it was. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
(function() { | ||
'use strict'; | ||
var result = '(function() { return this; })()' | ||
|> eval; | ||
|
||
expect(result).not.toBeUndefined(); | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
(function() { | ||
'use strict'; | ||
var result = '(function() { return this; })()' | ||
|> eval; | ||
|
||
expect(result).not.toBeUndefined(); | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
(function () { | ||
'use strict'; | ||
|
||
var _functionReturn; | ||
|
||
var result = (_functionReturn = '(function() { return this; })()', (0, eval)(_functionReturn)); | ||
expect(result).not.toBeUndefined(); | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
var array = [10,20,30]; | ||
|
||
var last = array |> (a => a[a.length-1]); | ||
|
||
expect(last).toBe(30); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
var _array; | ||
|
||
var array = [10, 20, 30]; | ||
var last = (_array = array, _array[_array.length - 1]); | ||
expect(last).toBe(30); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
var a = 1, | ||
b = 2, | ||
c = 3; | ||
var result = a | ||
|> (() => b) | ||
|> (() => c); | ||
|
||
expect(result).toBe(c); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
var a = 1, | ||
b = 2, | ||
c = 3; | ||
var result = a | ||
|> (() => b) | ||
|> (() => c); | ||
|
||
expect(result).toBe(c); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
var _ref, _a; | ||
|
||
var a = 1, | ||
b = 2, | ||
c = 3; | ||
var result = ((a, b), c); | ||
expect(result).toBe(c); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"plugins": [ | ||
["proposal-pipeline-operator", { "proposal": "fsharp" }] | ||
], | ||
"parserOpts": { | ||
"allowReturnOutsideFunction": true | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
const y = 2; | ||
const f = (x) => x | ||
|> (y => y + 1) | ||
|> (z => z * y) | ||
|
||
const g = (x) => x | ||
|> (y => | ||
y + 1 | ||
|> (z => z * y) | ||
) | ||
|
||
const h = (x) => x | ||
|> (y => ( | ||
y + 1 | ||
|> (z => z * y) | ||
)) | ||
|
||
expect(f(1)).toBe(4); | ||
expect(g(1)).toBe(2); | ||
expect(h(1)).toBe(2); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
const y = 2; | ||
const f = (x) => x | ||
|> (y => y + 1) | ||
|> (z => z * y) | ||
|
||
const g = (x) => x | ||
|> (y => | ||
y + 1 | ||
|> (z => z * y) | ||
) | ||
|
||
const h = (x) => x | ||
|> (y => ( | ||
y + 1 | ||
|> (z => z * y) | ||
)) | ||
|
||
expect(f(1)).toBe(4); | ||
expect(g(1)).toBe(2); | ||
expect(h(1)).toBe(2); |
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.
Nit: these two statements can be merged
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.
Done in ecbb465