Skip to content

Move function typed properties from the SourceFile to a dedicated functions #1700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 3, 2015
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
10 changes: 5 additions & 5 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ module ts {
}

function getLineOfLocalPosition(currentSourceFile: SourceFile, pos: number) {
return currentSourceFile.getLineAndCharacterFromPosition(pos).line;
return getLineAndCharacterOfPosition(currentSourceFile, pos).line;
}

function emitNewLineBeforeLeadingComments(currentSourceFile: SourceFile, writer: EmitTextWriter, node: TextRange, leadingComments: CommentRange[]) {
Expand Down Expand Up @@ -169,15 +169,15 @@ module ts {

function writeCommentRange(currentSourceFile: SourceFile, writer: EmitTextWriter, comment: CommentRange, newLine: string){
if (currentSourceFile.text.charCodeAt(comment.pos + 1) === CharacterCodes.asterisk) {
var firstCommentLineAndCharacter = currentSourceFile.getLineAndCharacterFromPosition(comment.pos);
var firstCommentLineAndCharacter = getLineAndCharacterOfPosition(currentSourceFile, comment.pos);
var firstCommentLineIndent: number;
for (var pos = comment.pos, currentLine = firstCommentLineAndCharacter.line; pos < comment.end; currentLine++) {
var nextLineStart = currentSourceFile.getPositionFromLineAndCharacter(currentLine + 1, /*character*/1);
var nextLineStart = getPositionFromLineAndCharacter(currentSourceFile, currentLine + 1, /*character*/1);

if (pos !== comment.pos) {
// If we are not emitting first line, we need to write the spaces to adjust the alignment
if (firstCommentLineIndent === undefined) {
firstCommentLineIndent = calculateIndent(currentSourceFile.getPositionFromLineAndCharacter(firstCommentLineAndCharacter.line, /*character*/1),
firstCommentLineIndent = calculateIndent(getPositionFromLineAndCharacter(currentSourceFile, firstCommentLineAndCharacter.line, /*character*/1),
comment.pos);
}

Expand Down Expand Up @@ -1644,7 +1644,7 @@ module ts {
}

function recordSourceMapSpan(pos: number) {
var sourceLinePos = currentSourceFile.getLineAndCharacterFromPosition(pos);
var sourceLinePos = getLineAndCharacterOfPosition(currentSourceFile, pos);
var emittedLine = writer.getLine();
var emittedColumn = writer.getColumn();

Expand Down
823 changes: 401 additions & 422 deletions src/compiler/parser.ts

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ module ts {
else {
files.push(file);
}
forEach(file.getSyntacticDiagnostics(), e => {
forEach(getSyntacticDiagnostics(file), e => {
errors.push(e);
});
}
Expand Down
17 changes: 12 additions & 5 deletions src/compiler/scanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,12 +278,20 @@ module ts {
return result;
}

export function getPositionFromLineAndCharacter(lineStarts: number[], line: number, character: number): number {
export function getPositionFromLineAndCharacter(sourceFile: SourceFile, line: number, character: number): number {
return computePositionFromLineAndCharacter(getLineStarts(sourceFile), line, character);
}

export function computePositionFromLineAndCharacter(lineStarts: number[], line: number, character: number): number {
Debug.assert(line > 0);
return lineStarts[line - 1] + character - 1;
}

export function getLineAndCharacterOfPosition(lineStarts: number[], position: number) {
export function getLineStarts(sourceFile: SourceFile): number[] {
return sourceFile.lineMap || (sourceFile.lineMap = computeLineStarts(sourceFile.text));
}

export function computeLineAndCharacterOfPosition(lineStarts: number[], position: number) {
Copy link
Member

Choose a reason for hiding this comment

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

For anything new functions introduced, we need to discuss where they get exported from (i.e. most will need to be exposed from utilities.ts).

@mhegazy, this relates to the API test we need to reintroduce; Monday?

I'd also annotate the return types, but that's just me.

var lineNumber = binarySearch(lineStarts, position);
if (lineNumber < 0) {
// If the actual position was not found,
Expand All @@ -298,9 +306,8 @@ module ts {
};
}

export function positionToLineAndCharacter(text: string, pos: number) {
var lineStarts = computeLineStarts(text);
return getLineAndCharacterOfPosition(lineStarts, pos);
export function getLineAndCharacterOfPosition(sourceFile: SourceFile, position: number): LineAndCharacter {
return computeLineAndCharacterOfPosition(getLineStarts(sourceFile), position);
}

var hasOwnProperty = Object.prototype.hasOwnProperty;
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/tsc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ module ts {
function countLines(program: Program): number {
var count = 0;
forEach(program.getSourceFiles(), file => {
count += file.getLineAndCharacterFromPosition(file.end).line;
count += getLineAndCharacterOfPosition(file, file.end).line;
});
return count;
}
Expand All @@ -82,7 +82,7 @@ module ts {
var output = "";

if (diagnostic.file) {
var loc = diagnostic.file.getLineAndCharacterFromPosition(diagnostic.start);
var loc = getLineAndCharacterOfPosition(diagnostic.file, diagnostic.start);

output += diagnostic.file.filename + "(" + loc.line + "," + loc.character + "): ";
}
Expand Down
26 changes: 9 additions & 17 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -881,21 +881,6 @@ module ts {
filename: string;
text: string;

getLineAndCharacterFromPosition(position: number): LineAndCharacter;
getPositionFromLineAndCharacter(line: number, character: number): number;
getLineStarts(): number[];

// Produces a new SourceFile for the 'newText' provided. The 'textChangeRange' parameter
// indicates what changed between the 'text' that this SourceFile has and the 'newText'.
// The SourceFile will be created with the compiler attempting to reuse as many nodes from
// this file as possible.
//
// Note: this function mutates nodes from this SourceFile. That means any existing nodes
// from this SourceFile that are being held onto may change as a result (including
// becoming detached from any SourceFile). It is recommended that this SourceFile not
// be used once 'update' is called on it.
update(newText: string, textChangeRange: TextChangeRange): SourceFile;

amdDependencies: string[];
amdModuleName: string;
referencedFiles: FileReference[];
Expand All @@ -907,19 +892,26 @@ module ts {
// missing tokens, or tokens it didn't know how to deal with).
parseDiagnostics: Diagnostic[];

// Returns all syntactic diagnostics (i.e. the reference, parser and grammar diagnostics).
getSyntacticDiagnostics(): Diagnostic[];
//getSyntacticDiagnostics(): Diagnostic[];
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this commented out line.


// File level diagnostics reported by the binder.
semanticDiagnostics: Diagnostic[];

// Returns all syntactic diagnostics (i.e. the reference, parser and grammar diagnostics).
// This field should never be used directly, use getSyntacticDiagnostics function instead.
syntacticDiagnostics: Diagnostic[];

hasNoDefaultLib: boolean;
externalModuleIndicator: Node; // The first node that causes this file to be an external module
nodeCount: number;
identifierCount: number;
symbolCount: number;
languageVersion: ScriptTarget;
identifiers: Map<string>;

// Stores a line map for the file.
// This field should never be used directly to obtain line map, use getLineMap function instead.
lineMap: number[];
}

export interface ScriptReferenceHost {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ module ts {
// This is a useful function for debugging purposes.
export function nodePosToString(node: Node): string {
var file = getSourceFileOfNode(node);
var loc = file.getLineAndCharacterFromPosition(node.pos);
var loc = getLineAndCharacterOfPosition(file, node.pos);
return file.filename + "(" + loc.line + "," + loc.character + ")";
}

Expand Down
6 changes: 3 additions & 3 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ module FourSlash {
this.currentCaretPosition = pos;

var lineStarts = ts.computeLineStarts(this.getCurrentFileContent());
var lineCharPos = ts.getLineAndCharacterOfPosition(lineStarts, pos);
var lineCharPos = ts.computeLineAndCharacterOfPosition(lineStarts, pos);
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the difference between this and getLineAndCharacterOfPosition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

computeLineAndCharacterOfPosition accepts lineMap as number[] and use it to compute line and character. getLineAndCharacterOfPosition accepts SourceFile, picks lineMap from the file (and initializes it if it was not yet created) and then invokes computeLineAndCharacterOfPosition

this.scenarioActions.push('<MoveCaretToLineAndChar LineNumber="' + lineCharPos.line + '" CharNumber="' + lineCharPos.character + '" />');
}

Expand Down Expand Up @@ -1384,15 +1384,15 @@ module FourSlash {
var incrementalSourceFile = this.languageService.getSourceFile(this.activeFile.fileName);
Utils.assertInvariants(incrementalSourceFile, /*parent:*/ undefined);

var incrementalSyntaxDiagnostics = incrementalSourceFile.getSyntacticDiagnostics();
var incrementalSyntaxDiagnostics = ts.getSyntacticDiagnostics(incrementalSourceFile);

// Check syntactic structure
var snapshot = this.languageServiceShimHost.getScriptSnapshot(this.activeFile.fileName);
var content = snapshot.getText(0, snapshot.getLength());

var referenceSourceFile = ts.createLanguageServiceSourceFile(
this.activeFile.fileName, createScriptSnapShot(content), ts.ScriptTarget.Latest, /*version:*/ "0", /*isOpen:*/ false, /*setNodeParents:*/ false);
var referenceSyntaxDiagnostics = referenceSourceFile.getSyntacticDiagnostics();
var referenceSyntaxDiagnostics = ts.getSyntacticDiagnostics(referenceSourceFile);

Utils.assertDiagnosticsEquals(incrementalSyntaxDiagnostics, referenceSyntaxDiagnostics);
Utils.assertStructuralEquals(incrementalSourceFile, referenceSourceFile);
Expand Down
4 changes: 2 additions & 2 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ module Harness.LanguageService {
assert.isTrue(line >= 1);
assert.isTrue(col >= 1);

return ts.getPositionFromLineAndCharacter(script.lineMap, line, col);
return ts.computePositionFromLineAndCharacter(script.lineMap, line, col);
}

/**
Expand All @@ -303,7 +303,7 @@ module Harness.LanguageService {
var script: ScriptInfo = this.fileNameToScript[fileName];
assert.isNotNull(script);

var result = ts.getLineAndCharacterOfPosition(script.lineMap, position);
var result = ts.computeLineAndCharacterOfPosition(script.lineMap, position);

assert.isTrue(result.line >= 1);
assert.isTrue(result.character >= 1);
Expand Down
25 changes: 19 additions & 6 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ module ts {
scriptSnapshot: IScriptSnapshot;

getNamedDeclarations(): Declaration[];
getLineAndCharacterFromPosition(pos: number): LineAndCharacter;
getLineStarts(): number[];
getPositionFromLineAndCharacter(line: number, character: number): number;
}

/**
Expand Down Expand Up @@ -721,22 +724,20 @@ module ts {
public filename: string;
public text: string;
public scriptSnapshot: IScriptSnapshot;
public lineMap: number[];

public statements: NodeArray<Statement>;
public endOfFileToken: Node;

// These methods will have their implementation provided by the implementation the
// compiler actually exports off of SourceFile.
public getLineAndCharacterFromPosition: (position: number) => LineAndCharacter;
public getPositionFromLineAndCharacter: (line: number, character: number) => number;
public getLineStarts: () => number[];
public getSyntacticDiagnostics: () => Diagnostic[];
public update: (newText: string, textChangeRange: TextChangeRange) => SourceFile;

public amdDependencies: string[];
public amdModuleName: string;
public referencedFiles: FileReference[];

public syntacticDiagnostics: Diagnostic[];
public referenceDiagnostics: Diagnostic[];
public parseDiagnostics: Diagnostic[];
public semanticDiagnostics: Diagnostic[];
Expand All @@ -753,6 +754,18 @@ module ts {

private namedDeclarations: Declaration[];

public getLineAndCharacterFromPosition(position: number): LineAndCharacter {
return getLineAndCharacterOfPosition(this, position);
}

public getLineStarts(): number[] {
return getLineStarts(this);
}

public getPositionFromLineAndCharacter(line: number, character: number): number {
return getPositionFromLineAndCharacter(this, line, character);
}

public getNamedDeclarations() {
if (!this.namedDeclarations) {
var sourceFile = this;
Expand Down Expand Up @@ -1574,7 +1587,7 @@ module ts {
if (version !== sourceFile.version || isOpen != sourceFile.isOpen) {
// Once incremental parsing is ready, then just call into this function.
if (!disableIncrementalParsing) {
var newSourceFile = sourceFile.update(scriptSnapshot.getText(0, scriptSnapshot.getLength()), textChangeRange);
var newSourceFile = update(sourceFile, scriptSnapshot.getText(0, scriptSnapshot.getLength()), textChangeRange);
setSourceFileFields(newSourceFile, scriptSnapshot, version, isOpen);
return newSourceFile;
}
Expand Down
4 changes: 2 additions & 2 deletions tests/cases/unittests/incrementalParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ module ts {
}

function assertSameDiagnostics(file1: SourceFile, file2: SourceFile) {
var diagnostics1 = file1.getSyntacticDiagnostics();
var diagnostics2 = file2.getSyntacticDiagnostics();
var diagnostics1 = getSyntacticDiagnostics(file1);
var diagnostics2 = getSyntacticDiagnostics(file2);

assert.equal(diagnostics1.length, diagnostics2.length, "diagnostics1.length !== diagnostics2.length");
for (var i = 0, n = diagnostics1.length; i < n; i++) {
Expand Down