Skip to content

Commit

Permalink
Use try/finally in AstPath (#13216)
Browse files Browse the repository at this point in the history
  • Loading branch information
fisker committed Aug 5, 2022
1 parent f7cdd32 commit e096ea7
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 49 deletions.
3 changes: 2 additions & 1 deletion .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,10 @@ module.exports = {
},
{
files: [
"tests/format/**/jsfmt.spec.js",
"tests/config/**/*.js",
"tests/format/**/jsfmt.spec.js",
"tests/integration/**/*.js",
"tests/unit/**/*.js",
"scripts/release/__tests__/**/*.spec.js",
],
env: {
Expand Down
6 changes: 5 additions & 1 deletion jest.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ const config = {
escapeString: false,
printBasicPrototype: false,
},
testRegex: "jsfmt\\.spec\\.js$|__tests__/.*\\.js$",
testMatch: [
"<rootDir>/tests/format/**/jsfmt.spec.js",
"<rootDir>/tests/integration/__tests__/**/*.js",
"<rootDir>/tests/unit/**/*.js",
],
testPathIgnorePatterns,
collectCoverage: ENABLE_CODE_COVERAGE,
collectCoverageFrom: ["<rootDir>/src/**/*.js", "<rootDir>/bin/**/*.js"],
Expand Down
45 changes: 18 additions & 27 deletions src/common/ast-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,21 @@ class AstPath {
value = value[name];
stack.push(name, value);
}
const result = callback(this);
stack.length = length;
return result;
try {
return callback(this);
} finally {
stack.length = length;
}
}

callParent(callback, count = 0) {
const stackIndex = getNodeStackIndexHelper(this.stack, count + 1);
const parentValues = this.stack.splice(stackIndex + 1);
const result = callback(this);
this.stack.push(...parentValues);
return result;
try {
return callback(this);
} finally {
this.stack.push(...parentValues);
}
}

// Similar to AstPath.prototype.call, except that the value obtained by
Expand All @@ -89,13 +93,15 @@ class AstPath {
stack.push(name, value);
}

for (let i = 0; i < value.length; ++i) {
stack.push(i, value[i]);
callback(this, i, value);
stack.length -= 2;
try {
for (let i = 0; i < value.length; ++i) {
stack.push(i, value[i]);
callback(this, i, value);
stack.length -= 2;
}
} finally {
stack.length = length;
}

stack.length = length;
}

// Similar to AstPath.prototype.each, except that the results of the
Expand All @@ -109,21 +115,6 @@ class AstPath {
return result;
}

/**
* @param {() => void} callback
* @internal Unstable API. Don't use in plugins for now.
*/
try(callback) {
const { stack } = this;
const stackBackup = [...stack];
try {
return callback();
} finally {
stack.length = 0;
stack.push(...stackBackup);
}
}

/**
* @param {...(
* | ((node: any, name: string | null, number: number | null) => boolean)
Expand Down
38 changes: 18 additions & 20 deletions src/language-js/print/call-arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,26 +110,24 @@ function printCallArguments(path, options, print) {
let printedExpanded = [];

try {
path.try(() => {
iterateCallArgumentsPath(path, (argPath, i) => {
if (shouldGroupFirst && i === 0) {
printedExpanded = [
[
print([], { expandFirstArg: true }),
printedArguments.length > 1 ? "," : "",
hasEmptyLineFollowingFirstArg ? hardline : line,
hasEmptyLineFollowingFirstArg ? hardline : "",
],
...printedArguments.slice(1),
];
}
if (shouldGroupLast && i === lastArgIndex) {
printedExpanded = [
...printedArguments.slice(0, -1),
print([], { expandLastArg: true }),
];
}
});
iterateCallArgumentsPath(path, (argPath, i) => {
if (shouldGroupFirst && i === 0) {
printedExpanded = [
[
print([], { expandFirstArg: true }),
printedArguments.length > 1 ? "," : "",
hasEmptyLineFollowingFirstArg ? hardline : line,
hasEmptyLineFollowingFirstArg ? hardline : "",
],
...printedArguments.slice(1),
];
}
if (shouldGroupLast && i === lastArgIndex) {
printedExpanded = [
...printedArguments.slice(0, -1),
print([], { expandLastArg: true }),
];
}
});
} catch (caught) {
if (caught instanceof ArgExpansionBailout) {
Expand Down
76 changes: 76 additions & 0 deletions tests/unit/ast-path.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import AstPath from "../../src/common/ast-path.js";

const error = new Error("A dummy error");
const throwError = () => {
throw error;
};

describe("AstPath", () => {
const ast = {
property: {
deep: { name: "deep" },
},
children: [{ index: 0 }, { index: 1 }],
};

test("AstPath#call()", () => {
const path = new AstPath(ast);

expect(path.call(() => path.getValue(), "property")).toBe(ast.property);
expect(() => path.call(throwError, "property")).toThrow(error);
expect(path.stack.length).toBe(1);
});

test("AstPath#callParent()", () => {
const path = new AstPath(ast);
path.stack.push("property", ast.property, "deep", ast.property.deep);

expect(path.callParent(() => path.getValue())).toBe(ast.property);
expect(() => path.callParent(throwError)).toThrow(error);
expect(path.stack.length).toBe(5);
});

test("AstPath#each()", () => {
const path = new AstPath(ast);

{
const called = [];
path.each(() => called.push(path.getValue()), "children");
expect(called).toEqual(ast.children);
}

{
const called = [];
expect(() => {
path.each((_, index) => {
if (index === 1) {
throwError();
}
called.push(path.getValue());
}, "children");
}).toThrow(error);
expect(called.length).toBe(1);
expect(path.stack.length).toBe(1);
}
});

test("AstPath#map()", () => {
const path = new AstPath(ast);

expect(path.map(() => path.getValue(), "children")).toEqual(ast.children);

{
let result;
expect(() => {
result = path.map((_, index) => {
if (index === 1) {
throwError();
}
return path.getValue();
}, "children");
}).toThrow(error);
expect(result).toBeUndefined();
expect(path.stack.length).toBe(1);
}
});
});

0 comments on commit e096ea7

Please sign in to comment.