Skip to content

Commit

Permalink
Experimental ternaries (#953)
Browse files Browse the repository at this point in the history
* introduction of experimentalTernaries

* rearranging Conditionals to experiment with the formatting

* cleanup and closer the format suggested by prettier

* tuning up the breaking rules

* breaking lines and indenting non experimental ternaries.

* adding and removing parentheses in conditional if condition breaks

* fixing build script

* removing unneeded group

* Fixing indentation within a FunctionCall

* cleanup and documentation

* Adding README information on Experimental Ternaries

* Extra comment

* more readable bracket selection

* condition can be a Conditional if it's wrapped in a parentheses

* not using hardline inside an ifBreak to avoid unexpected group break propagation

* moving check for parent function outside the breaking group, fixing an edge case and the linter error

* clean up code and adding group around falseExpression

* removing unnecessary conditions and hardlines and cleaning up the documentation

* Adding extra examples

* undoing unnecessary change

* group reorganisation that allows us to get rid of ifBreaks and hardlineWithoutBreakParent cleaning the resulting code

* unneeded check for new line on falseExpression

* cleaner output

* cleaning up documentation

* Adding spaces after `:` to align properly with the trueExpression

* adding indentation if we fill a tab before falseExpression

* undoing refactor for performance issues
  • Loading branch information
Janther committed Dec 21, 2023
1 parent e1ca377 commit 6ed961c
Show file tree
Hide file tree
Showing 12 changed files with 2,492 additions and 39 deletions.
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,19 @@ You might have a multi-version project, where different files are compiled with
| ------- | --------------------- | ---------------------- |
| None | `--compiler <string>` | `compiler: "<string>"` |

### Experimental Ternaries

Mimicking prettier's [new ternary formatting](https://prettier.io/blog/2023/11/13/curious-ternaries) for the community to try.

Valid options:

- `true` - Use curious ternaries, with the question mark after the condition.
- `false` - Retain the default behavior of ternaries; keep question marks on the same line as the consequent.

| Default | CLI Override | API Override |
| ------- | -------------------------- | ------------------------------- |
| false | `--experimental-ternaries` | `experimentalTernaries: <bool>` |

## Integrations

### Vim
Expand Down
12 changes: 9 additions & 3 deletions src/binary-operator-printers/logical.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,19 @@ const { group, line, indent } = doc.builders;
const groupIfNecessaryBuilder = (path) => (document) =>
path.getParentNode().type === 'BinaryOperation' ? document : group(document);

const indentIfNecessaryBuilder = (path) => (document) => {
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 (
options.experimentalTernaries &&
parentNode.type === 'Conditional' &&
parentNode.condition === node
)
return document;
if (parentNode.type !== 'BinaryOperation') return indent(document);
if (node === parentNode.right) return document;
node = parentNode;
Expand All @@ -20,9 +26,9 @@ const indentIfNecessaryBuilder = (path) => (document) => {

export const logical = {
match: (op) => ['&&', '||'].includes(op),
print: (node, path, print) => {
print: (node, path, print, options) => {
const groupIfNecessary = groupIfNecessaryBuilder(path);
const indentIfNecessary = indentIfNecessaryBuilder(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
Expand Down
96 changes: 83 additions & 13 deletions src/nodes/Conditional.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,88 @@
import { doc } from 'prettier';
import { printSeparatedItem } from '../common/printer-helpers.js';

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

export const Conditional = {
print: ({ path, print }) =>
group([
path.call(print, 'condition'),
indent([
line,
'? ',
path.call(print, 'trueExpression'),
line,
': ',
path.call(print, 'falseExpression')
])
let groupIndex = 0;
const experimentalTernaries = (node, path, print, options) => {
const parent = path.getParentNode();
const isNested = parent.type === 'Conditional';
const isNestedAsTrueExpression = isNested && parent.trueExpression === node;
const falseExpressionIsNested = node.falseExpression.type === 'Conditional';

// If the `condition` breaks into multiple lines, we add parentheses,
// unless it already is a `TupleExpression`.
const condition = path.call(print, 'condition');
const conditionDoc = group([
node.condition.type === 'TupleExpression'
? condition
: ifBreak(['(', printSeparatedItem(condition), ')'], condition),
' ?'
]);

// To switch between "case-style" and "curious" ternaries we force a new line
// before a nested `trueExpression` if the current `Conditional` is also a
// `trueExpression`.
const trueExpressionDoc = indent([
isNestedAsTrueExpression ? hardline : line,
path.call(print, 'trueExpression')
]);

const conditionAndTrueExpressionGroup = group(
[conditionDoc, trueExpressionDoc],
{ id: `Conditional.trueExpressionDoc-${groupIndex}` }
);

groupIndex += 1;

// For the odd case of `tabWidth` of 1 or 0 we initiate `fillTab` as a single
// space.
let fillTab = ' ';
if (
!falseExpressionIsNested && // avoid processing if it's not needed
(options.tabWidth > 2 || options.useTabs)
) {
fillTab = options.useTabs ? '\t' : ' '.repeat(options.tabWidth - 1);
}

// A nested `falseExpression` is always printed in a new line.
const falseExpression = path.call(print, 'falseExpression');
const falseExpressionDoc = [
isNested ? hardline : line,
':',
falseExpressionIsNested
? [' ', falseExpression]
: ifBreak([fillTab, indent(falseExpression)], [' ', falseExpression], {
// We only add `fillTab` if we are sure the trueExpression is indented
groupId: conditionAndTrueExpressionGroup.id
})
];

const document = group([conditionAndTrueExpressionGroup, falseExpressionDoc]);

return parent.type === 'VariableDeclarationStatement'
? indent([softline, document])
: document;
};

const traditionalTernaries = (path, print) =>
group([
path.call(print, 'condition'),
indent([
// Nested trueExpression and falseExpression are always printed in a new
// line
path.getParentNode().type === 'Conditional' ? hardline : line,
'? ',
path.call(print, 'trueExpression'),
line,
': ',
path.call(print, 'falseExpression')
])
]);

export const Conditional = {
print: ({ node, path, print, options }) =>
options.experimentalTernaries
? experimentalTernaries(node, path, print, options)
: traditionalTernaries(path, print)
};
9 changes: 5 additions & 4 deletions src/nodes/ReturnStatement.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,20 @@ import { doc } from 'prettier';

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

const expression = (node, path, print) => {
const expression = (node, path, print, options) => {
if (node.expression) {
return node.expression.type === 'TupleExpression'
return node.expression.type === 'TupleExpression' ||
(options.experimentalTernaries && node.expression.type === 'Conditional')
? [' ', path.call(print, 'expression')]
: group(indent([line, path.call(print, 'expression')]));
}
return '';
};

export const ReturnStatement = {
print: ({ node, path, print }) => [
print: ({ node, path, print, options }) => [
'return',
expression(node, path, print),
expression(node, path, print, options),
';'
]
};
4 changes: 2 additions & 2 deletions src/nodes/TupleExpression.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ const { group } = doc.builders;
const contents = (node, path, print) =>
node.components?.length === 1 && node.components[0].type === 'BinaryOperation'
? path.map(print, 'components')
: [printSeparatedList(path.map(print, 'components'))];
: printSeparatedList(path.map(print, 'components'));

export const TupleExpression = {
print: ({ node, path, print }) =>
group([
node.isArray ? '[' : '(',
...contents(node, path, print),
contents(node, path, print),
node.isArray ? ']' : ')'
])
};
10 changes: 10 additions & 0 deletions src/options.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const CATEGORY_GLOBAL = 'Global';
const CATEGORY_COMMON = 'Common';
const CATEGORY_JAVASCRIPT = 'JavaScript';
const CATEGORY_SOLIDITY = 'Solidity';

const options = {
Expand Down Expand Up @@ -40,6 +41,15 @@ const options = {
default: false,
description: 'Use single quotes instead of double quotes.'
},
experimentalTernaries: {
category: CATEGORY_JAVASCRIPT,
type: 'boolean',
default: false,
description:
'Use curious ternaries, with the question mark after the condition.',
oppositeDescription:
'Default behavior of ternaries; keep question marks on the same line as the consequent.'
},
compiler: {
category: CATEGORY_SOLIDITY,
type: 'string',
Expand Down
44 changes: 41 additions & 3 deletions src/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,32 @@ const tryHug = (node, operators) => {
return node;
};

// The parser wrongly groups nested Conditionals in the falseExpression
// in the following way:
//
// (a ? b : c) ? d : e;
//
// By reorganizing the group we have more flexibility when printing:
//
// a ? b : (c ? d : e);
//
// this is closer to the executed code and prints the same output.
const rearrangeConditional = (ctx) => {
while (ctx.condition.type === 'Conditional') {
const falseExpression = {
type: 'Conditional',
condition: ctx.condition.falseExpression,
trueExpression: ctx.trueExpression,
falseExpression: ctx.falseExpression
};
rearrangeConditional(falseExpression);

ctx.falseExpression = falseExpression;
ctx.trueExpression = ctx.condition.trueExpression;
ctx.condition = ctx.condition.condition;
}
};

function parse(text, _parsers, options = _parsers) {
const compiler = coerce(options.compiler);
const parsed = parser.parse(text, { loc: true, range: true });
Expand Down Expand Up @@ -57,9 +83,21 @@ function parse(text, _parsers, options = _parsers) {
ctx.loopExpression.omitSemicolon = true;
},
HexLiteral(ctx) {
ctx.value = options.singleQuote
? `hex'${ctx.value.slice(4, -1)}'`
: `hex"${ctx.value.slice(4, -1)}"`;
const value = ctx.value.slice(4, -1);
ctx.value = options.singleQuote ? `hex'${value}'` : `hex"${value}"`;
},
Conditional(ctx) {
rearrangeConditional(ctx);
// We can remove parentheses only because we are sure that the
// `condition` must be a single `bool` value.
while (
ctx.condition.type === 'TupleExpression' &&
!ctx.condition.isArray &&
ctx.condition.components.length === 1 &&
ctx.condition.components[0].type !== 'Conditional'
) {
[ctx.condition] = ctx.condition.components;
}
},
BinaryOperation(ctx) {
switch (ctx.operator) {
Expand Down
11 changes: 4 additions & 7 deletions test.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,10 @@ export default {
filename: 'test.cjs',
path: path.resolve(__dirname, 'dist'),
globalObject: `
typeof globalThis !== "undefined"
? globalThis
: typeof global !== "undefined"
? global
: typeof self !== "undefined"
? self
: this || {}
typeof globalThis !== "undefined" ? globalThis
: typeof global !== "undefined" ? global
: typeof self !== "undefined" ? self
: this || {}
`,
library: {
export: 'default',
Expand Down
Loading

0 comments on commit 6ed961c

Please sign in to comment.