Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Use isSameLine helper #2599

Merged
merged 1 commit into from
May 17, 2017
Merged

Use isSameLine helper #2599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions src/rules/arrowReturnShorthandRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ interface Options {

function walk(ctx: Lint.WalkContext<Options>): void {
const { sourceFile, options: { multiline } } = ctx;
ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
return ts.forEachChild(sourceFile, function cb(node: ts.Node): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have any evidence that adding return makes it faster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have any evidence and am reluctant to write a micro benchmark for this. I doubt the change here will be measurable at all.
It's just my understanding of what's helping the optimizing compiler.

Copy link
Contributor

Choose a reason for hiding this comment

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

Time measurement isn't necessary if you think it means the difference between tail call and not tail call. If return made it into a tail call, then this code would not get a stack overflow:

function f(x) {
    if (x === 0)
        return console.log("done");
    else
        return f(x - 1);
}
f(30000);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andy-hanson You're confusing tail call optimization (something the compiler can do) and tail call elimination (a language feature that the compiler must do).

The latter can be found in the ES Spec as proper tail calls. This feature will enable (almost) infinite recursion and would make your above example run without error. Unfortunately this is very unlikely to land in V8 because of https://github.com/tc39/proposal-ptc-syntax

Tail call opitimization on the other hand can optimize tail calls whether ptc is implemented or not. Unfortunately this does not enable deeper recursion.

if (utils.isArrowFunction(node) && utils.isBlock(node.body)) {
const expr = getSimpleReturnExpression(node.body);
if (expr !== undefined && (multiline || !node.body.getText(sourceFile).includes("\n"))) {
if (expr !== undefined && (multiline || utils.isSameLine(sourceFile, node.body.getStart(sourceFile), node.body.end))) {
const isObjectLiteral = expr.kind === ts.SyntaxKind.ObjectLiteralExpression;
ctx.addFailureAtNode(node.body, Rule.FAILURE_STRING(isObjectLiteral), createFix(node, node.body, expr, sourceFile.text));
}
Expand Down
6 changes: 2 additions & 4 deletions src/rules/semicolonRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ class SemicolonWalker extends Lint.AbstractWalker<Options> {
// check if this is a multi-line arrow function
if (node.initializer !== undefined &&
node.initializer.kind === ts.SyntaxKind.ArrowFunction &&
ts.getLineAndCharacterOfPosition(this.sourceFile, node.getStart(this.sourceFile)).line
!== ts.getLineAndCharacterOfPosition(this.sourceFile, node.getEnd()).line) {
!utils.isSameLine(this.sourceFile, node.getStart(this.sourceFile), node.end)) {
if (this.options.boundClassMethods) {
if (this.sourceFile.text[node.end - 1] === ";" &&
this.isFollowedByLineBreak(node.end)) {
Expand Down Expand Up @@ -234,7 +233,6 @@ class SemicolonWalker extends Lint.AbstractWalker<Options> {
if (nextStatement === undefined) {
return false;
}
return ts.getLineAndCharacterOfPosition(this.sourceFile, node.end).line
=== ts.getLineAndCharacterOfPosition(this.sourceFile, nextStatement.getStart(this.sourceFile)).line;
return utils.isSameLine(this.sourceFile, node.end, nextStatement.getStart(this.sourceFile));
}
}
18 changes: 7 additions & 11 deletions src/rules/trailingCommaRule.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the License.
*/

import { getChildOfKind } from "tsutils";
import { getChildOfKind, isSameLine } from "tsutils";
import * as ts from "typescript";

import * as Lint from "../index";
Expand Down Expand Up @@ -216,18 +216,14 @@ class TrailingCommaWalker extends Lint.AbstractWalker<Options> {

/* Expects `list.length !== 0` */
private checkComma(hasTrailingComma: boolean, list: ts.NodeArray<ts.Node>, closeTokenPos: number, optionKey: OptionName) {
const lastElementLine = ts.getLineAndCharacterOfPosition(this.sourceFile, list[list.length - 1].end).line;
const closeTokenLine = ts.getLineAndCharacterOfPosition(this.sourceFile, closeTokenPos).line;
const option = lastElementLine === closeTokenLine ? this.options.singleline : this.options.multiline;
const shouldHandle = option[optionKey];
const options = isSameLine(this.sourceFile, list[list.length - 1].end, closeTokenPos)
? this.options.singleline
: this.options.multiline;
const option = options[optionKey];

if (shouldHandle === "ignore") {
return;
}

if (shouldHandle === "always" && !hasTrailingComma) {
if (option === "always" && !hasTrailingComma) {
this.addFailureAt(list.end, 0, Rule.FAILURE_STRING_ALWAYS, Lint.Replacement.appendText(list.end, ","));
} else if (shouldHandle === "never" && hasTrailingComma) {
} else if (option === "never" && hasTrailingComma) {
this.addFailureAt(list.end - 1, 1, Rule.FAILURE_STRING_NEVER, Lint.Replacement.deleteText(list.end - 1, 1));
}
}
Expand Down