Skip to content

Binary operation grouping and indentation review #1133

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/binary-operator-printers/addition.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { binaryOperationPrinter } from './printers/binary-operation-printer.js';
import { bit } from './bit.js';
import { shift } from './shift.js';
import { inequality } from './inequality.js';
import { equality } from './equality.js';
import { logical } from './logical.js';

export const addition = {
match: (op) => ['+', '-'].includes(op),
print: binaryOperationPrinter([shift, bit, inequality, equality, logical])
};
51 changes: 0 additions & 51 deletions src/binary-operator-printers/arithmetic.js

This file was deleted.

7 changes: 5 additions & 2 deletions src/binary-operator-printers/bit.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { arithmetic } from './arithmetic.js';
import { binaryOperationPrinter } from './printers/binary-operation-printer.js';
import { inequality } from './inequality.js';
import { equality } from './equality.js';
import { logical } from './logical.js';

export const bit = {
match: (op) => ['&', '|', '^'].includes(op),
print: arithmetic.print
print: binaryOperationPrinter([inequality, equality, logical])
};
36 changes: 0 additions & 36 deletions src/binary-operator-printers/comparison.js

This file was deleted.

7 changes: 7 additions & 0 deletions src/binary-operator-printers/equality.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { binaryOperationPrinter } from './printers/binary-operation-printer.js';
import { logical } from './logical.js';

export const equality = {
match: (op) => ['==', '!='].includes(op),
print: binaryOperationPrinter([logical])
};
35 changes: 22 additions & 13 deletions src/binary-operator-printers/exponentiation.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,28 @@
import { doc } from 'prettier';
import { createBinaryOperationPrinter } from './printers/create-binary-operation-printer.js';
import { createIndentIfNecessaryBuilder } from './printers/create-indent-if-necessary-builder.js';
import { multiplication } from './multiplication.js';
import { addition } from './addition.js';
import { shift } from './shift.js';
import { bit } from './bit.js';
import { inequality } from './inequality.js';
import { equality } from './equality.js';
import { logical } from './logical.js';

const { group, indent, line } = doc.builders;
const { group } = doc.builders;

export const exponentiation = {
match: (op) => op === '**',
print: (node, path, print) => {
const right = [' ', node.operator, line, path.call(print, 'right')];
// If it's a single binary operation, avoid having a small right
// operand like - 1 on its own line
const shouldGroup =
node.left.type !== 'BinaryOperation' &&
path.getParentNode().type !== 'BinaryOperation';
return group([
path.call(print, 'left'),
indent(shouldGroup ? group(right) : right)
]);
}
print: createBinaryOperationPrinter(
() => (document) => group(document), // always group
createIndentIfNecessaryBuilder([
multiplication,
addition,
shift,
bit,
inequality,
equality,
logical
])
)
};
6 changes: 4 additions & 2 deletions src/binary-operator-printers/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
export * from './arithmetic.js';
export * from './addition.js';
export * from './assignment.js';
export * from './bit.js';
export * from './comparison.js';
export * from './equality.js';
export * from './exponentiation.js';
export * from './inequality.js';
export * from './logical.js';
export * from './multiplication.js';
export * from './shift.js';
8 changes: 8 additions & 0 deletions src/binary-operator-printers/inequality.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { binaryOperationPrinter } from './printers/binary-operation-printer.js';
import { logical } from './logical.js';
import { equality } from './equality.js';

export const inequality = {
match: (op) => ['<', '>', '<=', '>='].includes(op),
print: binaryOperationPrinter([logical, equality])
};
35 changes: 11 additions & 24 deletions src/binary-operator-printers/logical.js
Original file line number Diff line number Diff line change
@@ -1,45 +1,32 @@
import { doc } from 'prettier';
import { createBinaryOperationPrinter } from './printers/create-binary-operation-printer.js';
import { createGroupIfNecessaryBuilder } from './printers/create-group-if-necessary-builder.js';
import { notIndentParentTypes } from './printers/create-indent-if-necessary-builder.js';
import { shouldGroupOrIndent } from './utils/should-group-or-indent.js';

const { group, line, indent } = doc.builders;

const groupIfNecessaryBuilder = (path) => (document) =>
path.getParentNode().type === 'BinaryOperation' ? document : group(document);
const { indent } = doc.builders;

const indentIfNecessaryBuilder = (path, options) => (document) => {
let node = path.getNode();
for (let i = 0; ; i += 1) {
const parentNode = path.getParentNode(i);
if (parentNode.type === 'ReturnStatement') return document;
if (parentNode.type === 'IfStatement') return document;
if (parentNode.type === 'WhileStatement') return document;
if (notIndentParentTypes.includes(parentNode.type)) return document;
if (
options.experimentalTernaries &&
parentNode.type === 'Conditional' &&
parentNode.condition === node
)
return document;
if (parentNode.type !== 'BinaryOperation') return indent(document);
if (shouldGroupOrIndent(parentNode, [])) return indent(document);
if (node === parentNode.right) return document;
node = parentNode;
}
};

export const logical = {
match: (op) => ['&&', '||'].includes(op),
print: (node, path, print, options) => {
const groupIfNecessary = groupIfNecessaryBuilder(path);
const indentIfNecessary = indentIfNecessaryBuilder(path, options);

const right = [node.operator, line, path.call(print, 'right')];
// If it's a single binary operation, avoid having a small right
// operand like - 1 on its own line
const shouldGroup =
node.left.type !== 'BinaryOperation' &&
path.getParentNode().type !== 'BinaryOperation';
return groupIfNecessary([
path.call(print, 'left'),
' ',
indentIfNecessary(shouldGroup ? group(right) : right)
]);
}
print: createBinaryOperationPrinter(
createGroupIfNecessaryBuilder([]),
indentIfNecessaryBuilder
)
};
19 changes: 19 additions & 0 deletions src/binary-operator-printers/multiplication.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { binaryOperationPrinter } from './printers/binary-operation-printer.js';
import { addition } from './addition.js';
import { bit } from './bit.js';
import { equality } from './equality.js';
import { inequality } from './inequality.js';
import { shift } from './shift.js';
import { logical } from './logical.js';

export const multiplication = {
match: (op) => ['*', '/', '%'].includes(op),
print: binaryOperationPrinter([
addition,
shift,
bit,
inequality,
equality,
logical
])
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { createBinaryOperationPrinter } from './create-binary-operation-printer.js';
import { createGroupIfNecessaryBuilder } from './create-group-if-necessary-builder.js';
import { createIndentIfNecessaryBuilder } from './create-indent-if-necessary-builder.js';

export const binaryOperationPrinter = (shouldGroupAndIndentMatchers) =>
createBinaryOperationPrinter(
createGroupIfNecessaryBuilder(shouldGroupAndIndentMatchers),
createIndentIfNecessaryBuilder(shouldGroupAndIndentMatchers)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { doc } from 'prettier';
import { assignment } from '../assignment.js';

const { group, line } = doc.builders;

const rightOperandPrinter = (node, path, print) => {
const right = [' ', node.operator, line, path.call(print, 'right')];

// If it's a single binary operation, avoid having a small right
// operand like - 1 on its own line
const parent = path.getParentNode();
return node.left.type !== 'BinaryOperation' &&
parent.type !== 'BinaryOperation'
? group(right)
: right;
};

export const createBinaryOperationPrinter =
(groupIfNecessaryBuilder, indentIfNecessaryBuilder) =>
(node, path, print, options) => {
const groupIfNecessary = groupIfNecessaryBuilder(path);
const indentIfNecessary = indentIfNecessaryBuilder(path, options);

return groupIfNecessary([
path.call(print, 'left'),
indentIfNecessary(rightOperandPrinter(node, path, print))
]);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { doc } from 'prettier';
import { shouldGroupOrIndent } from '../utils/should-group-or-indent.js';

const { group } = doc.builders;

export const createGroupIfNecessaryBuilder =
(shouldIndentMatchers) => (path) => (document) => {
const parentNode = path.getParentNode();
if (shouldGroupOrIndent(parentNode, shouldIndentMatchers))
return group(document);
return document;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { doc } from 'prettier';
import { shouldGroupOrIndent } from '../utils/should-group-or-indent.js';

const { indent } = doc.builders;

export const notIndentParentTypes = [
'ReturnStatement',
'IfStatement',
'ForStatement',
'WhileStatement'
];

export const createIndentIfNecessaryBuilder =
(shouldIndentMatchers) => (path) => (document) => {
let node = path.getNode();
for (let i = 0; ; i += 1) {
const parentNode = path.getParentNode(i);
if (notIndentParentTypes.includes(parentNode.type)) return document;
if (shouldGroupOrIndent(parentNode, shouldIndentMatchers))
return indent(document);
if (node === parentNode.right) return document;
node = parentNode;
}
};
8 changes: 6 additions & 2 deletions src/binary-operator-printers/shift.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { arithmetic } from './arithmetic.js';
import { binaryOperationPrinter } from './printers/binary-operation-printer.js';
import { bit } from './bit.js';
import { inequality } from './inequality.js';
import { equality } from './equality.js';
import { logical } from './logical.js';

export const shift = {
match: (op) => ['<<', '>>'].includes(op),
print: arithmetic.print
print: binaryOperationPrinter([bit, inequality, equality, logical])
};
3 changes: 3 additions & 0 deletions src/binary-operator-printers/utils/should-group-or-indent.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export const shouldGroupOrIndent = ({ type, operator }, matchers) =>
type !== 'BinaryOperation' ||
matchers.some((matcher) => matcher.match(operator));
16 changes: 15 additions & 1 deletion src/slang-nodes/AdditiveExpression.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { NonterminalKind } from '@nomicfoundation/slang/cst';
import { printBinaryOperation } from '../slang-printers/print-binary-operation.js';
import { createHugFunction } from '../slang-utils/create-hug-function.js';
import { createKindCheckFunction } from '../slang-utils/create-kind-check-function.js';
import { getNodeMetadata, updateMetadata } from '../slang-utils/metadata.js';
import { Expression } from './Expression.js';

Expand All @@ -11,6 +12,19 @@ import type { PrintFunction, SlangNode } from '../types.d.ts';

const tryToHug = createHugFunction(['%']);

const printAdditiveExpression = printBinaryOperation(
createKindCheckFunction([
NonterminalKind.ShiftExpression,
NonterminalKind.BitwiseAndExpression,
NonterminalKind.BitwiseOrExpression,
NonterminalKind.BitwiseXorExpression,
NonterminalKind.InequalityExpression,
NonterminalKind.EqualityExpression,
NonterminalKind.AndExpression,
NonterminalKind.OrExpression
])
);

export class AdditiveExpression implements SlangNode {
readonly kind = NonterminalKind.AdditiveExpression;

Expand Down Expand Up @@ -45,6 +59,6 @@ export class AdditiveExpression implements SlangNode {
print: PrintFunction,
options: ParserOptions<AstNode>
): Doc {
return printBinaryOperation(this, path, print, options);
return printAdditiveExpression(this, path, print, options);
}
}
Loading