From 0a206d8855c3667c6ca3723a034bad18c653d333 Mon Sep 17 00:00:00 2001 From: Mohamed Hegazy Date: Wed, 30 Jul 2014 14:46:33 -0700 Subject: [PATCH] Respond to code review comments: * Change comment to say "noresolve=false" in shims.ts https://github.com/Microsoft/TypeScript/commit/05eeba5bc95f9a39ceb9c17b5674ed7135d74aed * Switch newline to "\r\n" https://github.com/Microsoft/TypeScript/commit/9395eeaedbb4279480c306f88e7916fa9694de31 * Use hasOwnProperty for Map types https://github.com/Microsoft/TypeScript/commit/212c18460281cac5b5be5239d45f7d04212f62d8 * Switch "s" to "S" in typescriptServices.ts filename https://github.com/Microsoft/TypeScript/commit/9061e58dffbeaf3f72cee1c6ca6d8fc0b98b1ea5 * Change method names in Node to be more detailed --- Jakefile | 2 +- src/compiler/core.ts | 4 + src/services/compiler/bloomFilter.ts | 2 +- .../getScriptLexicalStructureWalker.ts | 6 +- src/services/services.ts | 88 ++++++++++++------- src/services/shims.ts | 2 +- 6 files changed, 68 insertions(+), 36 deletions(-) diff --git a/Jakefile b/Jakefile index d738359e22..a36c3f4a9d 100644 --- a/Jakefile +++ b/Jakefile @@ -231,7 +231,7 @@ task("generate-diagnostics", [diagnosticInfoMapTs]) var tcFile = path.join(builtLocalDirectory, "tc.js"); compileFile(tcFile, compilerSources, [builtLocalDirectory, copyright].concat(compilerSources), [copyright], /*useBuiltCompiler:*/ false); -var tcServicesFile = path.join(builtLocalDirectory, "typescriptservices.js"); +var tcServicesFile = path.join(builtLocalDirectory, "typescriptServices.js"); compileFile(tcServicesFile, servicesSources, [builtLocalDirectory, copyright].concat(servicesSources), [copyright], /*useBuiltCompiler:*/ true); // Local target to build the compiler and services diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 824aaca5a4..2aa6ae6e9a 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -142,6 +142,10 @@ module ts { return result; } + export function lookUp(map: Map, key: string): T { + return hasProperty(map, key) ? map[key] : undefined; + } + export function mapToArray(map: Map): T[] { var result: T[] = []; for (var id in map) result.push(map[id]); diff --git a/src/services/compiler/bloomFilter.ts b/src/services/compiler/bloomFilter.ts index 8b0e3674e0..44d72bf5bf 100644 --- a/src/services/compiler/bloomFilter.ts +++ b/src/services/compiler/bloomFilter.ts @@ -83,7 +83,7 @@ module TypeScript { public addKeys(keys: ts.Map) { for (var name in keys) { - if (keys[name]) { + if (ts.lookUp(keys, name)) { this.add(name); } } diff --git a/src/services/getScriptLexicalStructureWalker.ts b/src/services/getScriptLexicalStructureWalker.ts index df182d3b40..b42f34fa92 100644 --- a/src/services/getScriptLexicalStructureWalker.ts +++ b/src/services/getScriptLexicalStructureWalker.ts @@ -34,7 +34,7 @@ module TypeScript.Services { var parentScope = this.currentScope; this.parentScopes.push(parentScope); - var scope = parentScope.childScopes[key]; + var scope = ts.lookUp(parentScope.childScopes, key); if (!scope) { scope = this.createScope() parentScope.childScopes[key] = scope; @@ -76,7 +76,7 @@ module TypeScript.Services { private createItem(node: TypeScript.ISyntaxNode, modifiers: ISyntaxToken[], kind: string, name: string): void { var key = kind + "+" + name; - if (this.currentScope.items[key] !== undefined) { + if (ts.lookUp(this.currentScope.items, key) !== undefined) { this.addAdditionalSpan(node, key); return; } @@ -100,7 +100,7 @@ module TypeScript.Services { node: TypeScript.ISyntaxNode, key: string) { - var item = this.currentScope.items[key] + var item = ts.lookUp(this.currentScope.items, key); Debug.assert(item !== undefined); var start = TypeScript.start(node); diff --git a/src/services/services.ts b/src/services/services.ts index f0be2fa0c2..c3359b1004 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -32,8 +32,12 @@ module ts { getChildCount(): number; getChildAt(index: number): Node; getChildren(): Node[]; + getStart(): number; + getFullStart(): number; + getEnd(): number; + getWidth(): number; getFullWidth(): number; - getTriviaWidth(): number; + getLeadingTriviaWidth(): number; getFullText(): string; getFirstToken(): Node; getLastToken(): Node; @@ -91,16 +95,28 @@ module ts { return node; } - public getTextPos(): number { + public getStart(): number { return getTokenPosOfNode(this); } - - public getFullWidth(): number { - return this.end - this.pos; + + public getFullStart(): number { + return this.pos; } - public getTriviaWidth(): number { - return getTokenPosOfNode(this) - this.pos; + public getEnd(): number { + return this.end; + } + + public getWidth(): number { + return this.getStart() - this.getEnd(); + } + + public getFullWidth(): number { + return this.end - this.getFullStart(); + } + + public getLeadingTriviaWidth(): number { + return this.getStart() - this.pos; } public getFullText(): string { @@ -919,40 +935,48 @@ module ts { return this._compilationSettings; } + public getEntry(filename: string): HostFileInformation { + filename = TypeScript.switchToForwardSlashes(filename); + return lookUp(this.filenameToEntry, filename); + } + public contains(filename: string): boolean { - return !!this.filenameToEntry[TypeScript.switchToForwardSlashes(filename)]; + return !!this.getEntry(filename); } public getHostfilename(filename: string) { - var hostCacheEntry = this.filenameToEntry[TypeScript.switchToForwardSlashes(filename)]; + var hostCacheEntry = this.getEntry(filename); if (hostCacheEntry) { return hostCacheEntry.filename; } return filename; } - public getfilenames(): string[] { + public getFilenames(): string[] { var fileNames: string[] = []; - for (var id in this.filenameToEntry) { - fileNames.push(id); - } + + forEachKey(this.filenameToEntry, key => { + if (hasProperty(this.filenameToEntry, key)) + fileNames.push(key); + }); + return fileNames; } public getVersion(filename: string): number { - return this.filenameToEntry[TypeScript.switchToForwardSlashes(filename)].version; + return this.getEntry(filename).version; } public isOpen(filename: string): boolean { - return this.filenameToEntry[TypeScript.switchToForwardSlashes(filename)].isOpen; + return this.getEntry(filename).isOpen; } public getByteOrderMark(filename: string): ByteOrderMark { - return this.filenameToEntry[TypeScript.switchToForwardSlashes(filename)].byteOrderMark; + return this.getEntry(filename).byteOrderMark; } public getScriptSnapshot(filename: string): TypeScript.IScriptSnapshot { - var file = this.filenameToEntry[TypeScript.switchToForwardSlashes(filename)]; + var file = this.getEntry(filename); if (!file.sourceText) { file.sourceText = this.host.getScriptSnapshot(file.filename); } @@ -1117,7 +1141,7 @@ module ts { function getBucketForCompilationSettings(settings: CompilerOptions, createIfMissing: boolean): Map { var key = getKeyFromCompilationSettings(settings); - var bucket = buckets[key]; + var bucket = lookUp(buckets, key); if (!bucket && createIfMissing) { buckets[key] = bucket = {}; } @@ -1126,7 +1150,7 @@ module ts { function reportStats() { var bucketInfoArray = Object.keys(buckets).filter(name => name && name.charAt(0) === '_').map(name => { - var entries = buckets[name]; + var entries = lookUp(buckets, name); var documents: { name: string; refCount: number; references: string[]; }[] = []; for (var i in entries) { var entry = entries[i]; @@ -1152,7 +1176,7 @@ module ts { referencedFiles: string[]= []): Document { var bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ true); - var entry = bucket[filename]; + var entry = lookUp(bucket, filename); if (!entry) { var document = createDocument(compilationSettings, filename, scriptSnapshot, byteOrderMark, version, isOpen, referencedFiles); @@ -1179,7 +1203,7 @@ module ts { var bucket = getBucketForCompilationSettings(compilationSettings, /*createIfMissing*/ false); Debug.assert(bucket); - var entry = bucket[filename]; + var entry = lookUp(bucket, filename); Debug.assert(entry); if (entry.document.isOpen() === isOpen && entry.document.getVersion() === version) { @@ -1194,7 +1218,7 @@ module ts { var bucket = getBucketForCompilationSettings(compilationSettings, false); Debug.assert(bucket); - var entry = bucket[filename]; + var entry = lookUp(bucket, filename); entry.refCount--; Debug.assert(entry.refCount >= 0); @@ -1229,10 +1253,14 @@ module ts { TypeScript.LocalizedDiagnosticMessages = host.getLocalizedDiagnosticMessages(); } + function getDocument(filename: string): Document { + return lookUp(documentsByName, filename); + } + function createCompilerHost(): CompilerHost { return { getSourceFile: (filename, languageVersion) => { - var document = documentsByName[filename]; + var document = getDocument(filename); Debug.assert(!!document, "document can not be undefined"); @@ -1241,7 +1269,7 @@ module ts { getCancellationToken: () => cancellationToken, getCanonicalFileName: (filename) => useCaseSensitivefilenames ? filename : filename.toLowerCase(), useCaseSensitiveFileNames: () => useCaseSensitivefilenames, - getNewLine: () => "\n", + getNewLine: () => "\r\n", // Need something that doesn't depend on sys.ts here getDefaultLibFilename: (): string => { throw Error("TOD:: getDefaultLibfilename"); @@ -1292,7 +1320,7 @@ module ts { // Now, for every file the host knows about, either add the file (if the compiler // doesn't know about it.). Or notify the compiler about any changes (if it does // know about it.) - var hostfilenames = hostCache.getfilenames(); + var hostfilenames = hostCache.getFilenames(); for (var i = 0, n = hostfilenames.length; i < n; i++) { var filename = hostfilenames[i]; @@ -1301,7 +1329,7 @@ module ts { var isOpen = hostCache.isOpen(filename); var scriptSnapshot = hostCache.getScriptSnapshot(filename); - var document: Document = documentsByName[filename]; + var document: Document = getDocument(filename); if (document) { // // If the document is the same, assume no update @@ -1564,7 +1592,7 @@ module ts { filename = TypeScript.switchToForwardSlashes(filename); - var document = documentsByName[filename]; + var document = getDocument(filename); var sourceUnit = document.getSourceUnit(); if (isCompletionListBlocker(document.getSyntaxTree().sourceUnit(), position)) { @@ -1706,7 +1734,7 @@ module ts { return undefined; } - var symbol = activeCompletionSession.symbols[entryName]; + var symbol = lookUp(activeCompletionSession.symbols, entryName); if (symbol) { var type = session.typeChecker.getTypeOfSymbol(symbol); Debug.assert(type, "Could not find type for symbol"); @@ -1739,7 +1767,7 @@ module ts { // find the child that has this for (var i = 0, n = current.getChildCount(); i < n; i++) { var child = current.getChildAt(i); - if (getTokenPosOfNode(child) <= position && position < child.end) { + if (child.getStart() <= position && position < child.getEnd()) { current = child; continue outer; } @@ -1824,7 +1852,7 @@ module ts { synchronizeHostData(); filename = TypeScript.switchToForwardSlashes(filename); - var document = documentsByName[filename]; + var document = getDocument(filename); var node = getNodeAtPosition(document.getSourceFile(), position); if (!node) return undefined; diff --git a/src/services/shims.ts b/src/services/shims.ts index 8debe8f973..f02b9b6208 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -331,7 +331,7 @@ module ts { /// TODO: this should be pushed into VS. /// We can not ask the LS instance to resolve, as this will lead to asking the host about files it does not know about, - /// something it is not desinged to handle. for now make sure we never get a noresolve=true. + /// something it is not desinged to handle. for now make sure we never get a "noresolve == false". /// This value should not matter, as the host runs resolution logic independentlly options.noResolve = true;