Find nearest node when formatting range (#1659)

* Move range extension code into helper functions

* Add findNodeByOffset() helper

This was adapted from cbc1929c64

* Test extending formatted range to entire node

* Fix extending formatted range to entire node

* Fix style errors

* Add run_file test function

This makes it possible to use different options on a per-file basis,
which is useful for things like range formatting tests.

* Test extending the format range to nearest parseable node

This means you can select the range of a `catch` clause, attempt to
format it, and have the `try` formatted as well, rather than throwing an
error.

* Fix extending the format range to nearest parseable node

This means you can select the range of a `catch` clause, attempt to
format it, and have the `try` formatted as well, rather than throwing an
error.

* Test that external indentation is left alone when formatting a range

* Preserve external indentation when formatting a range

* Dedupe range formatting traversal callbacks

* Simplify range formatting traversal using ast-types

See https://github.com/prettier/prettier/pull/1659#issuecomment-302974798

* Make range formatting traversal more efficient

There's less unnecessary parsing now.

* Fix style errors

* Add test where range expanding fails

* Fix test where range expanding fails

This makes sure that the range contains the entirety of the nodes
containing each of the range's endpoints.

* Add test for expanding range to beginning of line

* Pass test for expanding range to beginning of line

This makes it so that indentation before the range is added to the
formatted range.

* Don't parse/stringify AST to detect pre-range indentation

See https://github.com/prettier/prettier/pull/1659#discussion_r117790671

* When formatting a range, find closest statement rather than parsing

The `isStatement` implementation came from `docs/prettier.min.js`.

See https://github.com/prettier/prettier/pull/1659#issuecomment-303154770

* Add test for range-formatting a FunctionDeclaration's argument object

* Include FunctionDeclaration when searching for nearest node to range-format

From the spec, a Program is a series of SourceElements, each of which is
either a Statement or a FunctionDeclaration. See
https://www.ecma-international.org/ecma-262/5.1/#sec-A.5

* Remove unnecessary try-catch

See https://github.com/prettier/prettier/pull/1659#discussion_r117810096

* Add tests with multiple statements

See https://github.com/prettier/prettier/pull/1659#discussion_r117810753

* Remove unnecessary arguments from findNodeByOffset()

* Contract format range to ensure it starts/ends on nodes

* Specify test ranges in the fixtures

See https://github.com/prettier/prettier/pull/1659#discussion_r117811186

* Remove unnecessary comments from range fixtures

* Remove run_file test function

It's no longer used. This essentially reverts
8241216e68f2e0da997a4f558b03658d642c89a2

* Update range formatting docs

Clarify that the range expands to the nearest statement, and not to the
end of the line.

* Don't overwrite test options when detecting range

Now that multiple files share the same object again, we shouldn't be
re-assigning to it.

* Reuse already-read fixtures for AST_COMPARE=1 tests

* Remove `run_file` global from test eslintrc

* Undo package.json churn

`yarn` reformatted it before, but the whitespace visually sets off the
comment, so let's put it back how it was before.

See https://github.com/prettier/prettier/pull/1659#discussion_r117864655

* Remove misleading comments from isSourceElement

See https://github.com/prettier/prettier/pull/1659#discussion_r117865196

* Loop backwards through string instead of reversing it

See https://github.com/prettier/prettier/pull/1659#discussion_r117865759

* Don't recompute indent string when formatting range

See https://github.com/prettier/prettier/pull/1659#discussion_r117867268

* Rename findNodeByOffset to findNodeAtOffset

"Find x by y" is the common usage for finding an `x` by a key `y`.
However, since "by" has positional meaning, let's use "at" instead.

See https://github.com/prettier/prettier/pull/1659#discussion_r117865121

* Always trimRight() in formatRange and explain why

See https://github.com/prettier/prettier/pull/1659#discussion_r117864635

* Test formatting a range that crosses AST levels

See https://github.com/prettier/prettier/pull/1659#issuecomment-303243688

* Fix formatting a range that crosses AST levels

See https://github.com/prettier/prettier/pull/1659#issuecomment-303243688

* Remove unnecessary try-catch

See e52db5e9f9 (r117878763)

* Add test demonstrating range formatting indent detection

* Detect alignment from text on line before range, but don't reformat it

This avoids reformatting non-indentation that happens to precede the
range on the same line, while still correctly indenting the range based
on it.

See https://github.com/prettier/prettier/pull/1659#discussion_r117881430
master
Joseph Frazier 2017-05-23 10:43:58 -04:00 committed by Christopher Chedeau
parent 316c76172b
commit 330601c77d
17 changed files with 403 additions and 34 deletions

View File

@ -267,8 +267,8 @@ Prettier ships with a handful of customizable format options, usable in both the
| **Trailing Commas** - Print trailing commas wherever possible.<br /><br />Valid options: <br /> - `"none"` - no trailing commas <br /> - `"es5"` - trailing commas where valid in ES5 (objects, arrays, etc) <br /> - `"all"` - trailing commas wherever possible (function arguments). This requires node 8 or a [transform](https://babeljs.io/docs/plugins/syntax-trailing-function-commas/). | `"none"` | <code>--trailing-comma <none&#124;es5&#124;all></code> | <code>trailingComma: "<none&#124;es5&#124;all>"</code> |
| **Bracket Spacing** - Print spaces between brackets in object literals.<br /><br />Valid options: <br /> - `true` - Example: `{ foo: bar }` <br /> - `false` - Example: `{foo: bar}` | `true` | `--no-bracket-spacing` | `bracketSpacing: <bool>` |
| **JSX Brackets on Same Line** - Put the `>` of a multi-line JSX element at the end of the last line instead of being alone on the next line | `false` | `--jsx-bracket-same-line` | `jsxBracketSameLine: <bool>` |
| **Range Start** - Format code starting at a given character offset. The range will extend backwards to the start of the line. | `0` | `--range-start <int>` | `rangeStart: <int>` |
| **Range End** - Format code ending at a given character offset (exclusive). The range will extend forwards to the end of the line. | `Infinity` | `--range-end <int>` | `rangeEnd: <int>` |
| **Range Start** - Format code starting at a given character offset. The range will extend backwards to the start of the first line containing the selected statement. | `0` | `--range-start <int>` | `rangeStart: <int>` |
| **Range End** - Format code ending at a given character offset (exclusive). The range will extend forwards to the end of the selected statement. | `Infinity` | `--range-end <int>` | `rangeEnd: <int>` |
| **Parser** - Specify which parser to use. Both parsers support the same set of JavaScript features (including Flow). You shouldn't have to change this setting. | `babylon` | <code>--parser <flow&#124;babylon></code> | <code>parser: "<flow&#124;babylon>"</code> |
### Excluding code from formatting

View File

@ -232,8 +232,12 @@ if (argv["help"] || (!filepatterns.length && !stdin)) {
" --trailing-comma <none|es5|all>\n" +
" Print trailing commas wherever possible. Defaults to none.\n" +
" --parser <flow|babylon> Specify which parse to use. Defaults to babylon.\n" +
" --range-start <int> Format code starting at a given character offset. The range will extend backwards to the start of the line. Defaults to 0.\n" +
" --range-end <int> Format code ending at a given character offset (exclusive). The range will extend forwards to the end of the line. Defaults to Infinity.\n" +
" --range-start <int> Format code starting at a given character offset.\n" +
" The range will extend backwards to the start of the first line containing the selected statement.\n" +
" Defaults to 0.\n" +
" --range-end <int> Format code ending at a given character offset (exclusive).\n" +
" The range will extend forwards to the end of the selected statement.\n" +
" Defaults to Infinity.\n" +
" --no-color Do not colorize error messages.\n" +
" --version or -v Print Prettier version.\n" +
"\n"

164
index.js
View File

@ -3,7 +3,7 @@
const comments = require("./src/comments");
const version = require("./package.json").version;
const printAstToDoc = require("./src/printer").printAstToDoc;
const getAlignmentSize = require("./src/util").getAlignmentSize;
const util = require("./src/util");
const printDocToString = require("./src/doc-printer").printDocToString;
const normalizeOptions = require("./src/options").normalize;
const parser = require("./src/parser");
@ -52,41 +52,159 @@ function ensureAllCommentsPrinted(astComments) {
function format(text, opts, addAlignmentSize) {
addAlignmentSize = addAlignmentSize || 0;
const formattedRangeOnly = formatRange(text, opts);
const ast = parser.parse(text, opts);
const formattedRangeOnly = formatRange(text, opts, ast);
if (formattedRangeOnly) {
return formattedRangeOnly;
}
const ast = parser.parse(text, opts);
const astComments = attachComments(text, ast, opts);
const doc = printAstToDoc(ast, opts, addAlignmentSize);
opts.newLine = guessLineEnding(text);
const str = printDocToString(doc, opts);
ensureAllCommentsPrinted(astComments);
// Remove extra leading newline as well as the added indentation after last newline
// Remove extra leading indentation as well as the added indentation after last newline
if (addAlignmentSize > 0) {
return str.slice(opts.newLine.length).trimRight() + opts.newLine;
return str.trim() + opts.newLine;
}
return str;
}
function formatRange(text, opts) {
// Use `Math.min` since `lastIndexOf` returns 0 when `rangeStart` is 0
const rangeStart = Math.min(
opts.rangeStart,
text.lastIndexOf("\n", opts.rangeStart) + 1
);
// Use `text.length - 1` as the maximum since `indexOf` returns -1 if `fromIndex >= text.length`
const fromIndex = Math.min(opts.rangeEnd, text.length - 1);
const nextNewLineIndex = text.indexOf("\n", fromIndex);
const rangeEnd = (nextNewLineIndex < 0 ? fromIndex : nextNewLineIndex) + 1; // Add one to make rangeEnd exclusive
function findSiblingAncestors(startNodeAndParents, endNodeAndParents) {
let resultStartNode = startNodeAndParents.node;
let resultEndNode = endNodeAndParents.node;
if (0 < rangeStart || rangeEnd < text.length) {
const rangeString = text.substring(rangeStart, rangeEnd);
const alignmentSize = getAlignmentSize(
rangeString.slice(0, rangeString.search(/[^ \t]/)),
opts.tabWidth
for (const endParent of endNodeAndParents.parentNodes) {
if (util.locStart(endParent) >= util.locStart(startNodeAndParents.node)) {
resultEndNode = endParent;
} else {
break;
}
}
for (const startParent of startNodeAndParents.parentNodes) {
if (util.locEnd(startParent) <= util.locEnd(endNodeAndParents.node)) {
resultStartNode = startParent;
} else {
break;
}
}
return {
startNode: resultStartNode,
endNode: resultEndNode
};
}
function findNodeAtOffset(node, offset, parentNodes) {
parentNodes = parentNodes || [];
const start = util.locStart(node);
const end = util.locEnd(node);
if (start <= offset && offset <= end) {
for (const childNode of comments.getSortedChildNodes(node)) {
const childResult = findNodeAtOffset(
childNode,
offset,
[node].concat(parentNodes)
);
if (childResult) {
return childResult;
}
}
if (isSourceElement(node)) {
return {
node: node,
parentNodes: parentNodes
};
}
}
}
// See https://www.ecma-international.org/ecma-262/5.1/#sec-A.5
function isSourceElement(node) {
if (node == null) {
return false;
}
switch (node.type) {
case "FunctionDeclaration":
case "BlockStatement":
case "BreakStatement":
case "ContinueStatement":
case "DebuggerStatement":
case "DoWhileStatement":
case "EmptyStatement":
case "ExpressionStatement":
case "ForInStatement":
case "ForStatement":
case "IfStatement":
case "LabeledStatement":
case "ReturnStatement":
case "SwitchStatement":
case "ThrowStatement":
case "TryStatement":
case "VariableDeclaration":
case "WhileStatement":
case "WithStatement":
return true;
}
return false;
}
function calculateRange(text, opts, ast) {
// Contract the range so that it has non-whitespace characters at its endpoints.
// This ensures we can format a range that doesn't end on a node.
const rangeStringOrig = text.slice(opts.rangeStart, opts.rangeEnd);
const startNonWhitespace = Math.max(
opts.rangeStart + rangeStringOrig.search(/\S/),
opts.rangeStart
);
let endNonWhitespace;
for (
endNonWhitespace = opts.rangeEnd;
endNonWhitespace > opts.rangeStart;
--endNonWhitespace
) {
if (text[endNonWhitespace - 1].match(/\S/)) {
break;
}
}
const startNodeAndParents = findNodeAtOffset(ast, startNonWhitespace);
const endNodeAndParents = findNodeAtOffset(ast, endNonWhitespace);
const siblingAncestors = findSiblingAncestors(
startNodeAndParents,
endNodeAndParents
);
const startNode = siblingAncestors.startNode;
const endNode = siblingAncestors.endNode;
const rangeStart = Math.min(util.locStart(startNode), util.locStart(endNode));
const rangeEnd = Math.max(util.locEnd(startNode), util.locEnd(endNode));
return {
rangeStart: rangeStart,
rangeEnd: rangeEnd
};
}
function formatRange(text, opts, ast) {
if (0 < opts.rangeStart || opts.rangeEnd < text.length) {
const range = calculateRange(text, opts, ast);
const rangeStart = range.rangeStart;
const rangeEnd = range.rangeEnd;
const rangeString = text.slice(rangeStart, rangeEnd);
// Try to extend the range backwards to the beginning of the line.
// This is so we can detect indentation correctly and restore it.
// Use `Math.min` since `lastIndexOf` returns 0 when `rangeStart` is 0
const rangeStart2 = Math.min(
rangeStart,
text.lastIndexOf("\n", rangeStart) + 1
);
const indentString = text.slice(rangeStart2, rangeStart);
const alignmentSize = util.getAlignmentSize(indentString, opts.tabWidth);
const rangeFormatted = format(
rangeString,
@ -98,7 +216,11 @@ function formatRange(text, opts) {
alignmentSize
);
return text.slice(0, rangeStart) + rangeFormatted + text.slice(rangeEnd);
// Since the range contracts to avoid trailing whitespace,
// we need to remove the newline that was inserted by the `format` call.
const rangeTrimmed = rangeFormatted.trimRight();
return text.slice(0, rangeStart) + rangeTrimmed + text.slice(rangeEnd);
}
}

View File

@ -939,4 +939,9 @@ function printComments(path, print, options, needsSemi) {
return concat(leadingParts.concat(trailingParts));
}
module.exports = { attach, printComments, printDanglingComments };
module.exports = {
attach,
printComments,
printDanglingComments,
getSortedChildNodes
};

View File

@ -1,12 +1,166 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`different-levels.js 1`] = `
call(1,2,3)
call(1,2,3)
function f() {
call(1,2,3)
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
call(1,2,3)
call(1, 2, 3);
function f() {
call(1, 2, 3);
}
`;
exports[`function-declaration.js 1`] = `
function ugly ( {a=1, b = 2 } ) {}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
function ugly({ a = 1, b = 2 }) {}
`;
exports[`ignore-indentation.js 1`] = `
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
\`multiline template string
with too much indentation\`
}
}
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
\`multiline template string
with too much indentation\`;
}
}
}
`;
exports[`multiple-statements.js 1`] = `
call(
1, 2,3
);
call(
1, 2,3
);
call(
1, 2,3
);
call(
1, 2,3
);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
call(
1, 2,3
);
call(1, 2, 3);
call(1, 2, 3);
call(
1, 2,3
);
`;
exports[`multiple-statements2.js 1`] = `
call(
1, 2,3
);
call(
1, 2,3
);
call(
1, 2,3
);
call(
1, 2,3
);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
call(
1, 2,3
);
call(1, 2, 3);
call(1, 2, 3);
call(
1, 2,3
);
`;
exports[`nested.js 1`] = `
try {
if (condition) {
body
}
}
catch (err) {}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
try {
if (condition) {
body;
}
} catch (err) {}
`;
exports[`nested2.js 1`] = `
try {
if (condition) {
body
}
}
catch (err) {}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
try {
if (condition) {
body;
}
}
catch (err) {}
`;
exports[`nested3.js 1`] = `
try {
1;if (condition) {
body
}
}
catch (err) {}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
try {
1;if (condition) {
body;
}
}
catch (err) {}
`;
exports[`range.js 1`] = `
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
\`multiline template string
with too much indentation\`
// The [165, 246) range selects the above two lines, including the second newline
}
}
}
@ -16,9 +170,17 @@ function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
\`multiline template string
with too much indentation\`;
// The [165, 246) range selects the above two lines, including the second newline
}
}
}
`;
exports[`try-catch.js 1`] = `
try {}
catch (err) {}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
try {
} catch (err) {}
`;

View File

@ -0,0 +1,5 @@
call(1,2,3)
call(1,<<<PRETTIER_RANGE_START>>>2,3)
function f() {
call(1,<<<PRETTIER_RANGE_END>>>2,3)
}

View File

@ -0,0 +1 @@
function ugly ( <<<PRETTIER_RANGE_START>>>{a=1, b = 2 }<<<PRETTIER_RANGE_END>>> ) {}

View File

@ -0,0 +1,8 @@
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
`multiline template string
with <<<PRETTIER_RANGE_START>>>too<<<PRETTIER_RANGE_END>>> much indentation`
}
}
}

View File

@ -1 +1 @@
run_spec(__dirname, { rangeStart: 180, rangeEnd: 220 });
run_spec(__dirname);

View File

@ -0,0 +1,15 @@
call(
1, 2,3
);
call(
1, <<<PRETTIER_RANGE_START>>>2,3
);
call(
1, <<<PRETTIER_RANGE_END>>>2,3
);
call(
1, 2,3
);

View File

@ -0,0 +1,15 @@
call(
1, 2,3
);
<<<PRETTIER_RANGE_START>>>
call(
1, 2,3
);
call(
1, 2,3
);
<<<PRETTIER_RANGE_END>>>
call(
1, 2,3
);

6
tests/range/nested.js Normal file
View File

@ -0,0 +1,6 @@
<<<PRETTIER_RANGE_START>>>try {
if (condition) {
body
}<<<PRETTIER_RANGE_END>>>
}
catch (err) {}

6
tests/range/nested2.js Normal file
View File

@ -0,0 +1,6 @@
try {
i<<<PRETTIER_RANGE_START>>>f (condition) {
body
}<<<PRETTIER_RANGE_END>>>
}
catch (err) {}

6
tests/range/nested3.js Normal file
View File

@ -0,0 +1,6 @@
try {
1;i<<<PRETTIER_RANGE_START>>>f (condition) {
body
}<<<PRETTIER_RANGE_END>>>
}
catch (err) {}

View File

@ -2,8 +2,7 @@ function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
function ugly ( {a=1, b = 2 } ) {
`multiline template string
with too much indentation`
// The [165, 246) range selects the above two lines, including the second newline
with <<<PRETTIER_RANGE_START>>>too<<<PRETTIER_RANGE_END>>> much indentation`
}
}
}

2
tests/range/try-catch.js Normal file
View File

@ -0,0 +1,2 @@
try {}
<<<PRETTIER_RANGE_START>>>c<<<PRETTIER_RANGE_END>>>atch (err) {}

View File

@ -17,9 +17,23 @@ function run_spec(dirname, options, additionalParsers) {
const extension = extname(filename);
if (/^\.[jt]sx?$/.test(extension) && filename !== "jsfmt.spec.js") {
const path = dirname + "/" + filename;
const mergedOptions = mergeDefaultOptions(options || {});
let rangeStart = 0;
let rangeEnd = Infinity;
const source = read(path)
.replace(/\r\n/g, "\n")
.replace("<<<PRETTIER_RANGE_START>>>", (match, offset) => {
rangeStart = offset;
return "";
})
.replace("<<<PRETTIER_RANGE_END>>>", (match, offset) => {
rangeEnd = offset;
return "";
});
const source = read(path).replace(/\r\n/g, "\n");
const mergedOptions = Object.assign(mergeDefaultOptions(options || {}), {
rangeStart: rangeStart,
rangeEnd: rangeEnd
});
const output = prettyprint(source, path, mergedOptions);
test(`${mergedOptions.parser} - ${parser.parser}-verify`, () => {
expect(raw(source + "~".repeat(80) + "\n" + output)).toMatchSnapshot(
@ -41,7 +55,6 @@ function run_spec(dirname, options, additionalParsers) {
});
if (AST_COMPARE) {
const source = read(dirname + "/" + filename);
const ast = parse(source, mergedOptions);
const astMassaged = massageAST(ast);
let ppastMassaged;