From 168d92f5d254a0671a1c34ca79d7b5600084b139 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Wed, 15 Aug 2018 09:40:30 -0700 Subject: [PATCH] Use typescript strict mode (#505) --- js/main.ts | 4 +- js/os.ts | 25 +++++++++---- js/runtime.ts | 87 +++++++++++++++++++++++++++----------------- js/types.d.ts | 8 ++-- js/util.ts | 11 ++++-- js/v8_source_maps.ts | 16 ++++---- tsconfig.json | 3 +- 7 files changed, 96 insertions(+), 58 deletions(-) diff --git a/js/main.ts b/js/main.ts index 87fe30e6cb..d652b18bcb 100644 --- a/js/main.ts +++ b/js/main.ts @@ -72,5 +72,7 @@ export default function denoMain() { const inputFn = argv[1]; const mod = runtime.resolveModule(inputFn, `${cwd}/`); - mod.compileAndRun(); + assert(mod != null); + // TypeScript does not track assert, therefore not null assertion + mod!.compileAndRun(); } diff --git a/js/os.ts b/js/os.ts index a3f45e467d..e7d21db45f 100644 --- a/js/os.ts +++ b/js/os.ts @@ -37,11 +37,14 @@ export function codeFetch( fbs.Base.addMsgType(builder, fbs.Any.CodeFetch); builder.finish(fbs.Base.endBase(builder)); const resBuf = libdeno.send(builder.asUint8Array()); + assert(resBuf != null); // Process CodeFetchRes - const bb = new flatbuffers.ByteBuffer(new Uint8Array(resBuf)); + // TypeScript does not track `assert` from a CFA perspective, therefore not + // null assertion `!` + const bb = new flatbuffers.ByteBuffer(new Uint8Array(resBuf!)); const baseRes = fbs.Base.getRootAsBase(bb); if (fbs.Any.NONE === baseRes.msgType()) { - throw Error(baseRes.error()); + throw Error(baseRes.error()!); } assert(fbs.Any.CodeFetchRes === baseRes.msgType()); const codeFetchRes = new fbs.CodeFetchRes(); @@ -80,7 +83,9 @@ export function codeCache( const bb = new flatbuffers.ByteBuffer(new Uint8Array(resBuf)); const baseRes = fbs.Base.getRootAsBase(bb); assert(fbs.Any.NONE === baseRes.msgType()); - throw Error(baseRes.error()); + // undefined and null are incompatible in strict mode, but at runtime + // a null value is fine, therefore not null assertion + throw Error(baseRes.error()!); } } @@ -103,16 +108,22 @@ export function readFileSync(filename: string): Uint8Array { builder.finish(fbs.Base.endBase(builder)); const resBuf = libdeno.send(builder.asUint8Array()); assert(resBuf != null); - - const bb = new flatbuffers.ByteBuffer(new Uint8Array(resBuf)); + // TypeScript does not track `assert` from a CFA perspective, therefore not + // null assertion `!` + const bb = new flatbuffers.ByteBuffer(new Uint8Array(resBuf!)); const baseRes = fbs.Base.getRootAsBase(bb); if (fbs.Any.NONE === baseRes.msgType()) { - throw Error(baseRes.error()); + // undefined and null are incompatible in strict mode, but at runtime + // a null value is fine, therefore not null assertion + throw Error(baseRes.error()!); } assert(fbs.Any.ReadFileSyncRes === baseRes.msgType()); const res = new fbs.ReadFileSyncRes(); assert(baseRes.msg(res) != null); - return new Uint8Array(res.dataArray()); + const dataArray = res.dataArray(); + assert(dataArray != null); + // TypeScript cannot track assertion above, therefore not null assertion + return new Uint8Array(dataArray!); } export function writeFileSync( diff --git a/js/runtime.ts b/js/runtime.ts index 227d301c67..fdfbbe799a 100644 --- a/js/runtime.ts +++ b/js/runtime.ts @@ -57,7 +57,7 @@ export function setup(): void { const mod = FileModule.load(filename); if (!mod) { util.log("getGeneratedContents cannot find", filename); - return null; + return ""; } return mod.outputCode; } @@ -71,7 +71,7 @@ export function setup(): void { // FileModule.load(). FileModules are NOT executed upon first load, only when // compileAndRun is called. export class FileModule { - scriptVersion: string; + scriptVersion = ""; readonly exports = {}; private static readonly map = new Map(); @@ -105,7 +105,7 @@ export class FileModule { execute(this.fileName, this.outputCode); } - static load(fileName: string): FileModule { + static load(fileName: string): FileModule | undefined { return this.map.get(fileName); } @@ -113,7 +113,7 @@ export class FileModule { const out = []; for (const fn of this.map.keys()) { const m = this.map.get(fn); - if (m.sourceCode) { + if (m && m.sourceCode) { out.push(fn); } } @@ -127,7 +127,8 @@ export function makeDefine(fileName: string): AmdDefine { log("localRequire", x); }; const currentModule = FileModule.load(fileName); - const localExports = currentModule.exports; + util.assert(currentModule != null); + const localExports = currentModule!.exports; log("localDefine", fileName, deps, localExports); const args = deps.map(dep => { if (dep === "require") { @@ -140,9 +141,13 @@ export function makeDefine(fileName: string): AmdDefine { return deno; } else { const resolved = resolveModuleName(dep, fileName); - const depModule = FileModule.load(resolved); - depModule.compileAndRun(); - return depModule.exports; + util.assert(resolved != null); + const depModule = FileModule.load(resolved!); + if (depModule) { + depModule.compileAndRun(); + return depModule.exports; + } + return undefined; } }); factory(...args); @@ -156,10 +161,14 @@ export function resolveModule( ): null | FileModule { util.log("resolveModule", { moduleSpecifier, containingFile }); util.assert(moduleSpecifier != null && moduleSpecifier.length > 0); - let filename: string, sourceCode: string, outputCode: string; + let filename: string | null; + let sourceCode: string | null; + let outputCode: string | null; if (moduleSpecifier.startsWith(ASSETS) || containingFile.startsWith(ASSETS)) { // Assets are compiled into the runtime javascript bundle. - const moduleId = moduleSpecifier.split("/").pop(); + // we _know_ `.pop()` will return a string, but TypeScript doesn't so + // not null assertion + const moduleId = moduleSpecifier.split("/").pop()!; const assetName = moduleId.includes(".") ? moduleId : `${moduleId}.d.ts`; util.assert(assetName in assetSourceCode, `No such asset "${assetName}"`); sourceCode = assetSourceCode[assetName]; @@ -179,7 +188,7 @@ export function resolveModule( sourceCode = fetchResponse.sourceCode; outputCode = fetchResponse.outputCode; } - if (sourceCode == null || sourceCode.length === 0) { + if (sourceCode == null || sourceCode.length === 0 || filename == null) { return null; } util.log("resolveModule sourceCode length ", sourceCode.length); @@ -187,7 +196,9 @@ export function resolveModule( if (m != null) { return m; } else { - return new FileModule(filename, sourceCode, outputCode); + // null and undefined are incompatible in strict mode, but outputCode being + // null here has no runtime behavior impact, therefore not null assertion + return new FileModule(filename, sourceCode, outputCode!); } } @@ -204,7 +215,7 @@ function resolveModuleName( } function execute(fileName: string, outputCode: string): void { - util.assert(outputCode && outputCode.length > 0); + util.assert(outputCode != null && outputCode.length > 0); window["define"] = makeDefine(fileName); outputCode += `\n//# sourceURL=${fileName}`; globalEval(outputCode); @@ -278,7 +289,7 @@ class TypeScriptHost implements ts.LanguageServiceHost { getScriptVersion(fileName: string): string { util.log("getScriptVersion", fileName); const m = FileModule.load(fileName); - return m.scriptVersion; + return (m && m.scriptVersion) || ""; } getScriptSnapshot(fileName: string): ts.IScriptSnapshot | undefined { @@ -323,32 +334,40 @@ class TypeScriptHost implements ts.LanguageServiceHost { const fn = "lib.globals.d.ts"; // ts.getDefaultLibFileName(options); util.log("getDefaultLibFileName", fn); const m = resolveModule(fn, ASSETS); - return m.fileName; + util.assert(m != null); + // TypeScript cannot track assertions, therefore not null assertion + return m!.fileName; } resolveModuleNames( moduleNames: string[], - containingFile: string, - reusedNames?: string[] - ): Array { + containingFile: string + ): ts.ResolvedModule[] { //util.log("resolveModuleNames", { moduleNames, reusedNames }); - return moduleNames.map((name: string) => { - let resolvedFileName; - if (name === "deno") { - resolvedFileName = resolveModuleName("deno.d.ts", ASSETS); - } else if (name === "typescript") { - resolvedFileName = resolveModuleName("typescript.d.ts", ASSETS); - } else { - resolvedFileName = resolveModuleName(name, containingFile); - if (resolvedFileName == null) { - return undefined; + return moduleNames + .map(name => { + let resolvedFileName; + if (name === "deno") { + resolvedFileName = resolveModuleName("deno.d.ts", ASSETS); + } else if (name === "typescript") { + resolvedFileName = resolveModuleName("typescript.d.ts", ASSETS); + } else { + resolvedFileName = resolveModuleName(name, containingFile); } - } - // This flags to the compiler to not go looking to transpile functional - // code, anything that is in `/$asset$/` is just library code - const isExternalLibraryImport = resolvedFileName.startsWith(ASSETS); - return { resolvedFileName, isExternalLibraryImport }; - }); + // According to the interface we shouldn't return `undefined` but if we + // fail to return the same length of modules to those we cannot resolve + // then TypeScript fails on an assertion that the lengths can't be + // different, so we have to return an "empty" resolved module + // TODO: all this does is push the problem downstream, and TypeScript + // will complain it can't identify the type of the file and throw + // a runtime exception, so we need to handle missing modules better + resolvedFileName = resolvedFileName || ""; + // This flags to the compiler to not go looking to transpile functional + // code, anything that is in `/$asset$/` is just library code + const isExternalLibraryImport = resolvedFileName.startsWith(ASSETS); + // TODO: we should be returning a ts.ResolveModuleFull + return { resolvedFileName, isExternalLibraryImport }; + }); } } diff --git a/js/types.d.ts b/js/types.d.ts index 85eda0cd06..110873c6b4 100644 --- a/js/types.d.ts +++ b/js/types.d.ts @@ -2,10 +2,10 @@ export type TypedArray = Uint8Array | Float32Array | Int32Array; export interface ModuleInfo { - moduleName?: string; - filename?: string; - sourceCode?: string; - outputCode?: string; + moduleName: string | null; + filename: string | null; + sourceCode: string | null; + outputCode: string | null; } // Following definitions adapted from: diff --git a/js/util.ts b/js/util.ts index d4563936f0..c1e84610fb 100644 --- a/js/util.ts +++ b/js/util.ts @@ -49,15 +49,20 @@ export function arrayToStr(ui8: Uint8Array): string { // produces broken code when targeting ES5 code. // See https://github.com/Microsoft/TypeScript/issues/15202 // At the time of writing, the github issue is closed but the problem remains. -export interface Resolvable extends Promise { +export interface ResolvableMethods { resolve: (value?: T | PromiseLike) => void; // tslint:disable-next-line:no-any reject: (reason?: any) => void; } + +type Resolvable = Promise & ResolvableMethods; + export function createResolvable(): Resolvable { - let methods; + let methods: ResolvableMethods; const promise = new Promise((resolve, reject) => { methods = { resolve, reject }; }); - return Object.assign(promise, methods) as Resolvable; + // TypeScript doesn't know that the Promise callback occurs synchronously + // therefore use of not null assertion (`!`) + return Object.assign(promise, methods!) as Resolvable; } diff --git a/js/v8_source_maps.ts b/js/v8_source_maps.ts index ec706cd44f..8d1691b233 100644 --- a/js/v8_source_maps.ts +++ b/js/v8_source_maps.ts @@ -40,7 +40,7 @@ export function prepareStackTraceWrapper( try { return prepareStackTrace(error, stack); } catch (prepareStackError) { - Error.prepareStackTrace = null; + Error.prepareStackTrace = undefined; console.log("=====Error inside of prepareStackTrace===="); console.log(prepareStackError.stack.toString()); console.log("=====Original error======================="); @@ -66,8 +66,8 @@ export function wrapCallSite(frame: CallSite): CallSite { const source = frame.getFileName() || frame.getScriptNameOrSourceURL(); if (source) { - const line = frame.getLineNumber(); - const column = frame.getColumnNumber() - 1; + const line = frame.getLineNumber() || 0; + const column = (frame.getColumnNumber() || 1) - 1; const position = mapSourcePosition({ source, line, column }); frame = cloneCallSite(frame); frame.getFileName = () => position.source; @@ -79,7 +79,7 @@ export function wrapCallSite(frame: CallSite): CallSite { } // Code called using eval() needs special handling - let origin = frame.isEval() && frame.getEvalOrigin(); + let origin = (frame.isEval() && frame.getEvalOrigin()) || undefined; if (origin) { origin = mapEvalOrigin(origin); frame = cloneCallSite(frame); @@ -115,7 +115,7 @@ function CallSiteToString(frame: CallSite): string { } else { fileName = frame.getScriptNameOrSourceURL(); if (!fileName && frame.isEval()) { - fileLocation = frame.getEvalOrigin(); + fileLocation = frame.getEvalOrigin() || ""; fileLocation += ", "; // Expecting source position to follow. } @@ -162,7 +162,7 @@ function CallSiteToString(frame: CallSite): string { line += ` [as ${methodName} ]`; } } else { - line += typeName + "." + (methodName || ""); + line += `${typeName}.${methodName || ""}`; } } else if (isConstructor) { line += "new " + (functionName || ""); @@ -181,7 +181,7 @@ function CallSiteToString(frame: CallSite): string { // Regex for detecting source maps const reSourceMap = /^data:application\/json[^,]+base64,/; -function loadConsumer(source: string): SourceMapConsumer { +function loadConsumer(source: string): SourceMapConsumer | null { let consumer = consumers.get(source); if (consumer == null) { const code = getGeneratedContents(source); @@ -221,7 +221,7 @@ function loadConsumer(source: string): SourceMapConsumer { return consumer; } -function retrieveSourceMapURL(fileData: string): string { +function retrieveSourceMapURL(fileData: string): string | null { // Get the URL of the source map // tslint:disable-next-line:max-line-length const re = /(?:\/\/[@#][ \t]+sourceMappingURL=([^\s'"]+?)[ \t]*$)|(?:\/\*[@#][ \t]+sourceMappingURL=([^\*]+?)[ \t]*(?:\*\/)[ \t]*$)/gm; diff --git a/tsconfig.json b/tsconfig.json index e48f0fdf79..e6bf1688ba 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,10 +4,10 @@ "baseUrl": ".", "module": "esnext", "moduleResolution": "node", - "noImplicitAny": true, "noImplicitReturns": true, "noFallthroughCasesInSwitch": true, "noLib": true, + "noUnusedLocals": true, "paths": { "*": ["*", "out/debug/*", "out/default/*", "out/release/*"] }, @@ -15,6 +15,7 @@ "pretty": true, "removeComments": true, "sourceMap": true, + "strict": true, "target": "esnext", "types": [] },