Provide getStart method for RuleWalker

[jmlopez-rod]

This is not a bug, this is a suggestion that may give us some performance).

After encountering a bug in typescript and doing some digging around I realized that in most of the rules (if not all of them) we use a form of node.getStart(). This triggers the following calls:

        NodeObject.prototype.getStart = function (sourceFile, includeJsDocComment) {
            return ts.getTokenPosOfNode(this, sourceFile, includeJsDocComment);
        };

Note that we never provide the source file so we are then calling ts.getTokenPosOfNode(this) which has this implementation:

function getTokenPosOfNode(node, sourceFile, includeJsDocComment) {
        // With nodes that have no width (i.e. 'Missing' nodes), we actually *don't*
        // want to skip trivia because this will launch us forward to the next token.
        if (nodeIsMissing(node)) {
            return node.pos;
        }
        if (isJSDocNode(node)) {
            return ts.skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.pos, /*stopAfterLineBreak*/ false, /*stopAtComments*/ true);
        }
        if (includeJsDocComment && node.jsDocComments && node.jsDocComments.length > 0) {
            return getTokenPosOfNode(node.jsDocComments[0]);
        }
        // For a syntax list, it is possible that one of its children has JSDocComment nodes, while
        // the syntax list itself considers them as normal trivia. Therefore if we simply skip
        // trivia for the list, we may have skipped the JSDocComment as well. So we should process its
        // first child to determine the actual position of its first token.
        if (node.kind === 286 /* SyntaxList */ && node._children.length > 0) {
            return getTokenPosOfNode(node._children[0], sourceFile, includeJsDocComment);
        }
        return ts.skipTrivia((sourceFile || getSourceFileOfNode(node)).text, node.pos);
    }

The key here is the last return statement: (sourceFile || getSourceFileOfNode(node)).text, since we are not providing the sourceFile this means that we call this bad boy all the time:

    function getSourceFileOfNode(node) {
        while (node && node.kind !== 256 /* SourceFile */) {
            node = node.parent;
        }
        return node;
    }

Which means that if we have a node and we call node.getStart(), we do not get the start of the node for free, in fact, this will be very costly for nodes that are very deeply nested.

Please see the following comment in the issue I reported in typescript:

microsoft/TypeScript#12053 (comment)

In particular, the problem about not storing the source file in the node:

can be easily mitigated since usually source file can be passed as an argument

If you look at the typescript source code and search for .getStart( you will find lines like the
following:

                : ts.createTextSpanFromBounds(node.getStart(curSourceFile), node.getEnd());
                var start = allowPositionInLeadingTrivia ? child.getFullStart() : child.getStart(sourceFile, includeJsDocComment);
                    var start = child.getStart(sourceFile);

In all of those calls we avoid having to traverse the tree up to the root node because we are providing it. There are a few places where they do not provide the source file but it seems that most typescript developers are aware that they need to pass it.

Due to this, I ask that a wrapper be added to the RuleWalker and advise tslint rule developers to use it for performance:

export class RuleWalker extends SyntaxWalker {
    private limit: number;
    private position: number;
    private options: any[];
    private failures: RuleFailure[];
    private disabledIntervals: IDisabledInterval[];
    private ruleName: string;

    constructor(private sourceFile: ts.SourceFile, options: IOptions) {
        super();

        this.position = 0;
        this.failures = [];
        this.options = options.ruleArguments;
        this.limit = this.sourceFile.getFullWidth();
        this.disabledIntervals = options.disabledIntervals;
        this.ruleName = options.ruleName;
    }

    public getSourceFile(): ts.SourceFile {
        return this.sourceFile;
    }

    // new method which provides the source file so that typescript
    // can stop calling getSourceFileOfNode
    public getStart(node: ts.Node) {
        return node.getStart(this.sourceFile);
    }

This additional method would be welcome especially for rules that use getStart a lot.
In the same spirit perhaps we should consider doing something about other methods that take in the source file:

   getSourceFile(): SourceFile; // not free, traverses the tree
   getChildCount(sourceFile?: SourceFile): number; 
   getChildAt(index: number, sourceFile?: SourceFile): Node;
   getChildren(sourceFile?: SourceFile): Node[];
   getStart(sourceFile?: SourceFile, includeJsDocComment?: boolean): number;
   getFullStart(): number;
   getEnd(): number;
   getWidth(sourceFile?: SourceFile): number;
   getFullWidth(): number;
   getLeadingTriviaWidth(sourceFile?: SourceFile): number;
   getFullText(sourceFile?: SourceFile): string;
   getText(sourceFile?: SourceFile): string;
   getFirstToken(sourceFile?: SourceFile): Node;
   getLastToken(sourceFile?: SourceFile): Node;

[nchen63]

As #1977 indicates, profiling does not actually show a benefit to passing in source file