Skip to content

Language Service host API cleanup #1735

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 31 commits into from
Feb 3, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
118b7a9
remove unnecessary call sourceFile.getSourceFile()
mhegazy Jan 13, 2015
3c59b9f
Add getDefaultLibraryFilePath to retrive the default lib file path fo…
mhegazy Jan 14, 2015
adee215
Make logger methods optional on the language service host
mhegazy Jan 15, 2015
3434aa0
Remove getDefaultLibFilename from the LS host interface as its result…
mhegazy Jan 15, 2015
9346651
Inline the return of createCompilerHost
mhegazy Jan 15, 2015
66f8257
Remove isOpen from souceFile and LanugageServiceHost interfaces
mhegazy Jan 17, 2015
c9b1309
Gracefully handel getChangeRange in Scriptsnapshot.fromString
mhegazy Jan 17, 2015
000206f
Update servicesVersion
mhegazy Jan 20, 2015
01267bc
Merge branch 'master' into LSAPICleanup
mhegazy Jan 20, 2015
1b1a45b
update unit test contents
mhegazy Jan 20, 2015
fe96258
revert serviceVersion change as the API is still compatible
mhegazy Jan 20, 2015
0257ace
Respond to code review comments
mhegazy Jan 27, 2015
04d8f5a
Merge branch 'master' into LSAPICleanup
mhegazy Jan 28, 2015
1f91322
Merge branch 'master' into LSAPICleanup
mhegazy Jan 28, 2015
1945e11
remove unused file
mhegazy Jan 28, 2015
9628191
Allow the LS API to resolve referenced files
mhegazy Jan 28, 2015
d6decf8
Expose underlying program from the LS
mhegazy Jan 28, 2015
fe836c5
make documentRegistry optional argument to createLanguageService
mhegazy Jan 28, 2015
e503f38
Remove unused IScriptSnapshot.getLineStartPositions
mhegazy Jan 28, 2015
d593902
Add documentation to DocumentRegistry
mhegazy Jan 28, 2015
cf0ee9e
Merge branch 'master' into LSAPICleanup
mhegazy Jan 28, 2015
fd3562b
Update ServicesVersion
mhegazy Jan 29, 2015
9f977af
Shorten library to lib in getDefaultLibFilePath for consistency
mhegazy Jan 29, 2015
5fa30e5
Add API sample tests
mhegazy Jan 29, 2015
0abef52
Update test failing by previous commit
mhegazy Jan 29, 2015
9a6e3ad
Merge branch 'master' into LSAPICleanup
mhegazy Jan 31, 2015
0ce51e6
Fix #1871 by ensuring the at we get the canonical filename before we …
mhegazy Jan 31, 2015
79a1457
Ensure that all LS requests are to an exiting soruceFile, and if we f…
mhegazy Jan 31, 2015
8524bfc
Merge branch 'master' into LSAPICleanup
mhegazy Feb 2, 2015
2772355
Merge branch 'master' into LSAPICleanup
mhegazy Feb 3, 2015
d6bd9f7
Merge branch 'master' into LSAPICleanup
mhegazy Feb 3, 2015
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: 4 additions & 0 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,10 @@ module ts {
}
}

export function getDefaultLibFilename(options: CompilerOptions): string {
return options.target === ScriptTarget.ES6 ? "lib.es6.d.ts" : "lib.d.ts";
}

Copy link
Member

Choose a reason for hiding this comment

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

We use some stuff like this in the harness - @yuit was questioning the logic a week ago. Not sure if the problem persists.

Copy link
Member

Choose a reason for hiding this comment

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

should we be more generic here for future targets? something that converts ScriptTaget into string and use that with lib.scriptTagetString.d.ts ?

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 think we should only have one lib file with some details inside on when declarations are relevant, e.g. decorators/attribute annotations that say if this declaration applies to the current compilation or not.
managing multiple files has been a hassle.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mhegazy I agree on that. Also, should we make sure that the target exist first? or we are guarantee to have default target as there was a problem in the harness when target isn't specified

export interface ObjectAllocator {
getNodeConstructor(kind: SyntaxKind): new () => Node;
getSymbolConstructor(): new (flags: SymbolFlags, name: string) => Symbol;
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ module ts {

return {
getSourceFile,
getDefaultLibFilename: options => combinePaths(getDirectoryPath(normalizePath(sys.getExecutingFilePath())), options.target === ScriptTarget.ES6 ? "lib.es6.d.ts" : "lib.d.ts"),
getDefaultLibFilename: options => combinePaths(getDirectoryPath(normalizePath(sys.getExecutingFilePath())), getDefaultLibFilename(options)),
writeFile,
getCurrentDirectory: () => currentDirectory || (currentDirectory = sys.getCurrentDirectory()),
useCaseSensitiveFileNames: () => sys.useCaseSensitiveFileNames,
Expand Down Expand Up @@ -174,7 +174,7 @@ module ts {
}
var diagnostic: DiagnosticMessage;
if (hasExtension(filename)) {
if (!options.allowNonTsExtensions && !fileExtensionIs(filename, ".ts")) {
if (!options.allowNonTsExtensions && !fileExtensionIs(host.getCanonicalFileName(filename), ".ts")) {
diagnostic = Diagnostics.File_0_must_have_extension_ts_or_d_ts;
}
else if (!findSourceFile(filename, isDefaultLib, refFile, refPos, refEnd)) {
Expand Down
5 changes: 1 addition & 4 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,6 @@ module FourSlash {
getLength: () => {
return sourceText.length;
},
getLineStartPositions: () => {
return <number[]>[];
},
getChangeRange: (oldSnapshot: ts.IScriptSnapshot) => {
return <ts.TextChangeRange>undefined;
}
Expand Down Expand Up @@ -1403,7 +1400,7 @@ module FourSlash {
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);
this.activeFile.fileName, createScriptSnapShot(content), ts.ScriptTarget.Latest, /*version:*/ "0", /*setNodeParents:*/ false);
var referenceSyntaxDiagnostics = referenceSourceFile.getSyntacticDiagnostics();

Utils.assertDiagnosticsEquals(incrementalSyntaxDiagnostics, referenceSyntaxDiagnostics);
Expand Down
16 changes: 13 additions & 3 deletions src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -930,6 +930,8 @@ module Harness {
settingsCallback(null);
}

var newLine = '\r\n';

var useCaseSensitiveFileNames = ts.sys.useCaseSensitiveFileNames;
this.settings.forEach(setting => {
switch (setting.flag.toLowerCase()) {
Expand Down Expand Up @@ -1008,7 +1010,7 @@ module Harness {

case 'newline':
case 'newlines':
ts.sys.newLine = setting.value;
newLine = setting.value;
break;

case 'comments':
Expand Down Expand Up @@ -1050,7 +1052,7 @@ module Harness {
break;

case 'includebuiltfile':
inputFiles.push({ unitName: setting.value, content: IO.readFile(libFolder + setting.value) });
inputFiles.push({ unitName: setting.value, content: normalizeLineEndings(IO.readFile(libFolder + setting.value), newLine) });
break;

default:
Expand Down Expand Up @@ -1096,7 +1098,7 @@ module Harness {
onComplete(result, program);

// reset what newline means in case the last test changed it
ts.sys.newLine = '\r\n';
ts.sys.newLine = newLine;
return options;
}

Expand Down Expand Up @@ -1168,6 +1170,14 @@ module Harness {
}
}

function normalizeLineEndings(text: string, lineEnding: string): string {
var normalized = text.replace(/\r\n?/g, '\n');
if (lineEnding !== '\n') {
normalized = normalized.replace(/\n/g, lineEnding);
}
return normalized;
}

export function getMinimalDiagnostic(err: ts.Diagnostic): HarnessDiagnostic {
var errorLineInfo = err.file ? err.file.getLineAndCharacterFromPosition(err.start) : { line: 0, character: 0 };
return {
Expand Down
40 changes: 17 additions & 23 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ module Harness.LanguageService {
public editRanges: { length: number; textChangeRange: ts.TextChangeRange; }[] = [];
public lineMap: number[] = null;

constructor(public fileName: string, public content: string, public isOpen = true) {
constructor(public fileName: string, public content: string) {
this.setContent(content);
}

Expand Down Expand Up @@ -72,14 +72,6 @@ module Harness.LanguageService {
return this.textSnapshot.length;
}

public getLineStartPositions(): string {
if (this.lineMap === null) {
this.lineMap = ts.computeLineStarts(this.textSnapshot);
}

return JSON.stringify(this.lineMap);
}

public getChangeRange(oldScript: ts.ScriptSnapshotShim): string {
var oldShim = <ScriptSnapshotShim>oldScript;
var range = this.scriptInfo.getTextChangeRangeBetweenVersions(oldShim.version, this.version);
Expand Down Expand Up @@ -109,11 +101,9 @@ module Harness.LanguageService {
fileName: string,
compilationSettings: ts.CompilerOptions,
scriptSnapshot: ts.IScriptSnapshot,
version: string,
isOpen: boolean): ts.SourceFile {
version: string): ts.SourceFile {
var sourceFile = ts.createSourceFile(fileName, scriptSnapshot.getText(0, scriptSnapshot.getLength()), compilationSettings.target);
sourceFile.version = version;
sourceFile.isOpen = isOpen;
return sourceFile;
}

Expand All @@ -123,10 +113,9 @@ module Harness.LanguageService {
compilationSettings: ts.CompilerOptions,
scriptSnapshot: ts.IScriptSnapshot,
version: string,
isOpen: boolean,
textChangeRange: ts.TextChangeRange
): ts.SourceFile {
return ts.updateLanguageServiceSourceFile(document, scriptSnapshot, version, isOpen, textChangeRange);
return ts.updateLanguageServiceSourceFile(document, scriptSnapshot, version, textChangeRange);
}

public releaseDocument(fileName: string, compilationSettings: ts.CompilerOptions): void {
Expand Down Expand Up @@ -159,13 +148,17 @@ module Harness.LanguageService {
}

private getScriptInfo(fileName: string): ScriptInfo {
return this.fileNameToScript[fileName];
return ts.lookUp(this.fileNameToScript, fileName);
}

public addScript(fileName: string, content: string) {
this.fileNameToScript[fileName] = new ScriptInfo(fileName, content);
}

private contains(fileName: string): boolean {
return ts.hasProperty(this.fileNameToScript, fileName);
}

public updateScript(fileName: string, content: string) {
var script = this.getScriptInfo(fileName);
if (script !== null) {
Expand Down Expand Up @@ -223,20 +216,22 @@ module Harness.LanguageService {

public getScriptFileNames(): string {
var fileNames: string[] = [];
ts.forEachKey(this.fileNameToScript, (fileName) => { fileNames.push(fileName); });
ts.forEachKey(this.fileNameToScript,(fileName) => { fileNames.push(fileName); });
return JSON.stringify(fileNames);
}

public getScriptSnapshot(fileName: string): ts.ScriptSnapshotShim {
return new ScriptSnapshotShim(this.getScriptInfo(fileName));
if (this.contains(fileName)) {
return new ScriptSnapshotShim(this.getScriptInfo(fileName));
}
return undefined;
}

public getScriptVersion(fileName: string): string {
return this.getScriptInfo(fileName).version.toString();
}

public getScriptIsOpen(fileName: string): boolean {
return this.getScriptInfo(fileName).isOpen;
if (this.contains(fileName)) {
return this.getScriptInfo(fileName).version.toString();
}
return undefined;
}

public getLocalizedDiagnosticMessages(): string {
Expand Down Expand Up @@ -272,7 +267,6 @@ module Harness.LanguageService {
public parseSourceText(fileName: string, sourceText: ts.IScriptSnapshot): ts.SourceFile {
var result = ts.createSourceFile(fileName, sourceText.getText(0, sourceText.getLength()), ts.ScriptTarget.Latest);
result.version = "1";
result.isOpen = true;
return result;
}

Expand Down
2 changes: 1 addition & 1 deletion src/services/formatting/rulesProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ module ts.formatting {
private activeRules: Rule[];
private rulesMap: RulesMap;

constructor(private logger: Logger) {
constructor() {
this.globalRules = new Rules();
}

Expand Down
Loading