Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix right precedence of Hack pipes #13668

Merged
merged 5 commits into from Sep 8, 2021
Merged

Fix right precedence of Hack pipes #13668

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ad3fc0a
Hack pipes are right-associative
nicolo-ribaudo Aug 11, 2021
083e4b6
[hack pipes] Loosen right precedence of `|>`
nicolo-ribaudo Aug 12, 2021
9310eec
Update operator precedence
nicolo-ribaudo Sep 4, 2021
8f26930
Flow
nicolo-ribaudo Sep 4, 2021
b984f7e
Review
nicolo-ribaudo Sep 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/babel-parser/src/parser/error-message.js
View file
Open in desktop
Expand Up @@ -134,6 +134,7 @@ export const ErrorMessages = makeErrorTemplates(
ParamDupe: "Argument name clash.",
PatternHasAccessor: "Object pattern can't contain getter or setter.",
PatternHasMethod: "Object pattern can't contain methods.",
// This error is only used by the smart-mix proposal
PipeBodyIsTighter:
"Unexpected %0 after pipeline body; any %0 expression acting as Hack-style pipe body must be parenthesized due to its loose operator precedence.",
PipeTopicRequiresHackPipes:
Expand All @@ -144,6 +145,8 @@ export const ErrorMessages = makeErrorTemplates(
'Invalid topic token %0. In order to use %0 as a topic reference, the pipelineOperator plugin must be configured with { "proposal": "hack", "topicToken": "%0" }.',
PipeTopicUnused:
"Hack-style pipe body does not contain a topic reference; Hack-style pipes must use topic at least once.",
PipeUnparenthesizedBody:
"Hack-style pipe body cannot be an unparenthesized %0 expression; please wrap it in parentheses.",

// Messages whose codes start with “Pipeline” or “PrimaryTopic”
// are retained for backwards compatibility
Expand Down
110 changes: 36 additions & 74 deletions packages/babel-parser/src/parser/expression.js
View file
Open in desktop
Expand Up @@ -61,6 +61,9 @@ import { cloneIdentifier } from "./node";
import type { SourceType } from "../options";
*/

// $FlowIgnore
const hasOwn = Function.call.bind(Object.prototype.hasOwnProperty);
nicolo-ribaudo marked this conversation as resolved.
Show resolved Hide resolved

export default class ExpressionParser extends LValParser {
// Forward-declaration: defined in statement.js
/*::
Expand Down Expand Up @@ -285,28 +288,6 @@ export default class ExpressionParser extends LValParser {
const operator = this.state.value;
node.operator = operator;

const leftIsHackPipeExpression =
left.type === "BinaryExpression" &&
left.operator === "|>" &&
this.getPluginOption("pipelineOperator", "proposal") === "hack";

if (leftIsHackPipeExpression) {
// If the pipelinePlugin is configured to use Hack pipes,
// and if an assignment expression’s LHS invalidly contains `|>`,
// then the user likely meant to parenthesize the assignment expression.
// Throw a human-friendly error
// instead of something like 'Invalid left-hand side'.
// For example, `x = x |> y = #` (assuming `#` is the topic reference)
// groups into `x = (x |> y) = #`,
// and `(x |> y)` is an invalid assignment LHS.
// This is because Hack-style `|>` has tighter precedence than `=>`.
// (Unparenthesized `yield` expressions are handled
// in `parseHackPipeBody`,
// and unparenthesized `=>` expressions are handled
// in `checkHackPipeBodyEarlyErrors`.)
throw this.raise(this.state.start, Errors.PipeBodyIsTighter, operator);
}

if (this.match(tt.eq)) {
node.left = this.toAssignable(left, /* isLHS */ true);
refExpressionErrors.doubleProto = -1; // reset because double __proto__ is valid in assignment expression
Expand Down Expand Up @@ -497,16 +478,20 @@ export default class ExpressionParser extends LValParser {
switch (this.getPluginOption("pipelineOperator", "proposal")) {
case "hack":
return this.withTopicBindingContext(() => {
const bodyExpr = this.parseHackPipeBody(op, prec);
this.checkHackPipeBodyEarlyErrors(startPos);
return bodyExpr;
return this.parseHackPipeBody();
});

case "smart":
return this.withTopicBindingContext(() => {
const childExpr = this.parseHackPipeBody(op, prec);
if (this.prodParam.hasYield && this.isContextual("yield")) {
throw this.raise(
this.state.start,
Errors.PipeBodyIsTighter,
this.state.value,
);
}
return this.parseSmartPipelineBodyInStyle(
childExpr,
this.parseExprOpBaseRightExpr(op, prec),
startPos,
startLoc,
);
Expand Down Expand Up @@ -539,37 +524,31 @@ export default class ExpressionParser extends LValParser {
);
}

// Helper function for `parseExprOpRightExpr` for the Hack-pipe operator
// (and the Hack-style smart-mix pipe operator).

parseHackPipeBody(op: TokenType, prec: number): N.Expression {
// If the following expression is invalidly a `yield` expression,
// then throw a human-friendly error.
// A `yield` expression in a generator context (i.e., a [Yield] production)
// starts a YieldExpression.
// Outside of a generator context, any `yield` as a pipe body
// is considered simply an identifier.
// This error is checked here, before actually parsing the body expression,
// because `yield`’s “not allowed as identifier in generator” error
// would otherwise have immediately
// occur before the pipe body is fully parsed.
// (Unparenthesized assignment expressions are handled
// in `parseMaybeAssign`,
// and unparenthesized `=>` expressions are handled
// in `checkHackPipeBodyEarlyErrors`.)
const bodyIsInGeneratorContext = this.prodParam.hasYield;
const bodyIsYieldExpression =
bodyIsInGeneratorContext && this.isContextual("yield");

if (bodyIsYieldExpression) {
throw this.raise(
this.state.start,
Errors.PipeBodyIsTighter,
this.state.value,
parseHackPipeBody(): N.Expression {
const { start } = this.state;

const body = this.parseMaybeAssign();

const invalidBodies = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider hoist this object to top level so it doesn't have to be re-created in parseHackPipeBody().

ArrowFunctionExpression: "arrow function",
AssignmentExpression: "assignment",
ConditionalExpression: "conditional",
YieldExpression: "yield",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could leave a todo item here that invalidBodies currently does not handle TS extensions such as TSAsExpression, they will be supported after TypeScript supports hack pipes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a todo, but I don't think that it will be a problem because:

  • a = b as c is an AssignmentExpression
  • a => b as c is an ArrowFunctionExpression
  • a ? b : c as d is a ConditionalExpression
  • yield as a is a syntax error

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I was meant to the plain as expression a as c. The precedence of as and |> are unclear before TS supports pipes.

};

if (hasOwn(invalidBodies, body.type) && !body.extra?.parenthesized) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can transform invalidBodies to a map so it comes with .has(), and it's also faster than objects.

this.raise(
start,
Errors.PipeUnparenthesizedBody,
invalidBodies[body.type],
);
} else {
return this.parseExprOpBaseRightExpr(op, prec);
}
if (!this.topicReferenceWasUsedInCurrentContext()) {
// A Hack pipe body must use the topic reference at least once.
this.raise(start, Errors.PipeTopicUnused);
}

return body;
}

checkExponentialAfterUnary(node: N.AwaitExpression | N.UnaryExpression) {
Expand Down Expand Up @@ -2738,24 +2717,7 @@ export default class ExpressionParser extends LValParser {
// The `startPos` is the starting position of the pipe body.

checkHackPipeBodyEarlyErrors(startPos: number): void {
// If the following token is invalidly `=>`,
// then throw a human-friendly error
// instead of something like 'Unexpected token, expected ";"'.
// For example, `x => x |> y => #` (assuming `#` is the topic reference)
// groups into `x => (x |> y) => #`,
// and `(x |> y) => #` is an invalid arrow function.
// This is because Hack-style `|>` has tighter precedence than `=>`.
// (Unparenthesized `yield` expressions are handled
// in `parseHackPipeBody`,
// and unparenthesized assignment expressions are handled
// in `parseMaybeAssign`.)
if (this.match(tt.arrow)) {
throw this.raise(
this.state.start,
Errors.PipeBodyIsTighter,
tt.arrow.label,
);
} else if (!this.topicReferenceWasUsedInCurrentContext()) {
if (!this.topicReferenceWasUsedInCurrentContext()) {
// A Hack pipe body must use the topic reference at least once.
this.raise(startPos, Errors.PipeTopicUnused);
}
Expand Down
3 changes: 1 addition & 2 deletions ...rimental/pipeline-operator/hack-hash-proposal-arrow-function-unparenthesized/options.json
View file
Open in desktop
Expand Up @@ -7,6 +7,5 @@
"topicToken": "#"
}
]
],
"throws": "Unexpected => after pipeline body; any => expression acting as Hack-style pipe body must be parenthesized due to its loose operator precedence. (1:8)"
]
}
62 changes: 62 additions & 0 deletions ...erimental/pipeline-operator/hack-hash-proposal-arrow-function-unparenthesized/output.json
View file
Open in desktop
@@ -0,0 +1,62 @@
{
"type": "File",
"start":0,"end":17,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":17}},
"errors": [
"SyntaxError: Hack-style pipe body cannot be an unparenthesized arrow function expression; please wrap it in parentheses. (1:6)"
],
"program": {
"type": "Program",
"start":0,"end":17,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":17}},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start":0,"end":17,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":17}},
"expression": {
"type": "BinaryExpression",
"start":0,"end":16,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":16}},
"left": {
"type": "NumericLiteral",
"start":0,"end":2,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":2}},
"extra": {
"rawValue": 10,
"raw": "10"
},
"value": 10
},
"operator": "|>",
"right": {
"type": "ArrowFunctionExpression",
"start":6,"end":16,"loc":{"start":{"line":1,"column":6},"end":{"line":1,"column":16}},
"id": null,
"generator": false,
"async": false,
"params": [
{
"type": "Identifier",
"start":6,"end":7,"loc":{"start":{"line":1,"column":6},"end":{"line":1,"column":7},"identifierName":"x"},
"name": "x"
}
],
"body": {
"type": "BinaryExpression",
"start":11,"end":16,"loc":{"start":{"line":1,"column":11},"end":{"line":1,"column":16}},
"left": {
"type": "Identifier",
"start":11,"end":12,"loc":{"start":{"line":1,"column":11},"end":{"line":1,"column":12},"identifierName":"x"},
"name": "x"
},
"operator": "+",
"right": {
"type": "TopicReference",
"start":15,"end":16,"loc":{"start":{"line":1,"column":15},"end":{"line":1,"column":16}}
}
}
}
}
}
],
"directives": []
}
}
5 changes: 2 additions & 3 deletions ...res/experimental/pipeline-operator/hack-hash-proposal-assignment-logical-and/options.json
View file
Open in desktop
Expand Up @@ -7,6 +7,5 @@
"topicToken": "#"
}
]
],
"throws": "Unexpected &&= after pipeline body; any &&= expression acting as Hack-style pipe body must be parenthesized due to its loose operator precedence. (1:11)"
}
]
}
44 changes: 44 additions & 0 deletions ...ures/experimental/pipeline-operator/hack-hash-proposal-assignment-logical-and/output.json
View file
Open in desktop
@@ -0,0 +1,44 @@
{
"type": "File",
"start":0,"end":16,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":16}},
"errors": [
"SyntaxError: Hack-style pipe body cannot be an unparenthesized assignment expression; please wrap it in parentheses. (1:9)"
],
"program": {
"type": "Program",
"start":0,"end":16,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":16}},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start":0,"end":16,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":16}},
"expression": {
"type": "BinaryExpression",
"start":0,"end":16,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":16}},
"left": {
"type": "Identifier",
"start":0,"end":5,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":5},"identifierName":"value"},
"name": "value"
},
"operator": "|>",
"right": {
"type": "AssignmentExpression",
"start":9,"end":16,"loc":{"start":{"line":1,"column":9},"end":{"line":1,"column":16}},
"operator": "&&=",
"left": {
"type": "Identifier",
"start":9,"end":10,"loc":{"start":{"line":1,"column":9},"end":{"line":1,"column":10},"identifierName":"x"},
"name": "x"
},
"right": {
"type": "TopicReference",
"start":15,"end":16,"loc":{"start":{"line":1,"column":15},"end":{"line":1,"column":16}}
}
}
}
}
],
"directives": []
}
}
5 changes: 2 additions & 3 deletions ...fixtures/experimental/pipeline-operator/hack-hash-proposal-assignment-normal/options.json
View file
Open in desktop
Expand Up @@ -7,6 +7,5 @@
"topicToken": "#"
}
]
],
"throws": "Unexpected = after pipeline body; any = expression acting as Hack-style pipe body must be parenthesized due to its loose operator precedence. (1:16)"
}
]
}
55 changes: 55 additions & 0 deletions .../fixtures/experimental/pipeline-operator/hack-hash-proposal-assignment-normal/output.json
View file
Open in desktop
@@ -0,0 +1,55 @@
{
"type": "File",
"start":0,"end":19,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":19}},
"errors": [
"SyntaxError: Hack-style pipe body cannot be an unparenthesized assignment expression; please wrap it in parentheses. (1:9)"
],
"program": {
"type": "Program",
"start":0,"end":19,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":19}},
"sourceType": "script",
"interpreter": null,
"body": [
{
"type": "ExpressionStatement",
"start":0,"end":19,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":19}},
"expression": {
"type": "BinaryExpression",
"start":0,"end":19,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":19}},
"left": {
"type": "Identifier",
"start":0,"end":5,"loc":{"start":{"line":1,"column":0},"end":{"line":1,"column":5},"identifierName":"value"},
"name": "value"
},
"operator": "|>",
"right": {
"type": "AssignmentExpression",
"start":9,"end":19,"loc":{"start":{"line":1,"column":9},"end":{"line":1,"column":19}},
"operator": "=",
"left": {
"type": "ArrayPattern",
"start":9,"end":15,"loc":{"start":{"line":1,"column":9},"end":{"line":1,"column":15}},
"elements": [
{
"type": "Identifier",
"start":10,"end":11,"loc":{"start":{"line":1,"column":10},"end":{"line":1,"column":11},"identifierName":"x"},
"name": "x"
},
{
"type": "Identifier",
"start":13,"end":14,"loc":{"start":{"line":1,"column":13},"end":{"line":1,"column":14},"identifierName":"y"},
"name": "y"
}
]
},
"right": {
"type": "TopicReference",
"start":18,"end":19,"loc":{"start":{"line":1,"column":18},"end":{"line":1,"column":19}}
}
}
}
}
],
"directives": []
}
}
5 changes: 2 additions & 3 deletions ...t/fixtures/experimental/pipeline-operator/hack-hash-proposal-assignment-plus/options.json
View file
Open in desktop
Expand Up @@ -7,6 +7,5 @@
"topicToken": "#"
}
]
],
"throws": "Unexpected += after pipeline body; any += expression acting as Hack-style pipe body must be parenthesized due to its loose operator precedence. (1:11)"
}
]
}