From 078b82d428b60ddee5537edb8b3ba99605731b90 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Tue, 7 Jan 2020 11:03:59 +0100 Subject: [PATCH] debt - rewrite layers checker to catch more cases --- .github/workflows/ci.yml | 12 +- .../darwin/continuous-build-darwin.yml | 4 +- .../linux/continuous-build-linux.yml | 4 +- build/azure-pipelines/product-compile.yml | 4 +- .../win32/continuous-build-win32.yml | 4 +- build/lib/globalsLinter.js | 176 ------------- build/lib/globalsLinter.ts | 209 --------------- build/lib/layersChecker.js | 205 +++++++++++++++ build/lib/layersChecker.ts | 241 ++++++++++++++++++ package.json | 2 +- src/vs/base/common/worker/simpleWorker.ts | 4 +- .../base/parts/quickopen/common/quickOpen.ts | 4 +- 12 files changed, 464 insertions(+), 405 deletions(-) delete mode 100644 build/lib/globalsLinter.js delete mode 100644 build/lib/globalsLinter.ts create mode 100644 build/lib/layersChecker.js create mode 100644 build/lib/layersChecker.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9fbfa2f16b4..5c9a7a74c52 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,8 +39,8 @@ jobs: name: Run Hygiene Checks - run: yarn monaco-compile-check name: Run Monaco Editor Checks - - run: yarn valid-globals-check - name: Run Valid Globals Checks + - run: yarn valid-layers-check + name: Run Valid Layers Checks - run: yarn compile name: Compile Sources - run: yarn download-builtin-extensions @@ -71,8 +71,8 @@ jobs: name: Run Hygiene Checks - run: yarn monaco-compile-check name: Run Monaco Editor Checks - - run: yarn valid-globals-check - name: Run Valid Globals Checks + - run: yarn valid-layers-check + name: Run Valid Layers Checks - run: yarn compile name: Compile Sources - run: yarn download-builtin-extensions @@ -100,8 +100,8 @@ jobs: name: Run Hygiene Checks - run: yarn monaco-compile-check name: Run Monaco Editor Checks - - run: yarn valid-globals-check - name: Run Valid Globals Checks + - run: yarn valid-layers-check + name: Run Valid Layers Checks - run: yarn compile name: Compile Sources - run: yarn download-builtin-extensions diff --git a/build/azure-pipelines/darwin/continuous-build-darwin.yml b/build/azure-pipelines/darwin/continuous-build-darwin.yml index 72265e035e0..d0aafb2a3e6 100644 --- a/build/azure-pipelines/darwin/continuous-build-darwin.yml +++ b/build/azure-pipelines/darwin/continuous-build-darwin.yml @@ -30,8 +30,8 @@ steps: yarn monaco-compile-check displayName: Run Monaco Editor Checks - script: | - yarn valid-globals-check - displayName: Run Valid Globals Checks + yarn valid-layers-check + displayName: Run Valid Layers Checks - script: | yarn compile displayName: Compile Sources diff --git a/build/azure-pipelines/linux/continuous-build-linux.yml b/build/azure-pipelines/linux/continuous-build-linux.yml index dac42d4ffad..24e05537d9b 100644 --- a/build/azure-pipelines/linux/continuous-build-linux.yml +++ b/build/azure-pipelines/linux/continuous-build-linux.yml @@ -38,8 +38,8 @@ steps: yarn monaco-compile-check displayName: Run Monaco Editor Checks - script: | - yarn valid-globals-check - displayName: Run Valid Globals Checks + yarn valid-layers-check + displayName: Run Valid Layers Checks - script: | yarn compile displayName: Compile Sources diff --git a/build/azure-pipelines/product-compile.yml b/build/azure-pipelines/product-compile.yml index 0055d48f0e4..db6524be03b 100644 --- a/build/azure-pipelines/product-compile.yml +++ b/build/azure-pipelines/product-compile.yml @@ -90,8 +90,8 @@ steps: set -e yarn gulp hygiene yarn monaco-compile-check - yarn valid-globals-check - displayName: Run hygiene, monaco compile & valid globals checks + yarn valid-layers-check + displayName: Run hygiene, monaco compile & valid layers checks condition: and(succeeded(), ne(variables['CacheExists-Compilation'], 'true'), eq(variables['VSCODE_STEP_ON_IT'], 'false')) - script: | diff --git a/build/azure-pipelines/win32/continuous-build-win32.yml b/build/azure-pipelines/win32/continuous-build-win32.yml index 17cb23d406f..8e55440c3a8 100644 --- a/build/azure-pipelines/win32/continuous-build-win32.yml +++ b/build/azure-pipelines/win32/continuous-build-win32.yml @@ -35,8 +35,8 @@ steps: yarn monaco-compile-check displayName: Run Monaco Editor Checks - script: | - yarn valid-globals-check - displayName: Run Valid Globals Checks + yarn valid-layers-check + displayName: Run Valid Layers Checks - powershell: | yarn compile displayName: Compile Sources diff --git a/build/lib/globalsLinter.js b/build/lib/globalsLinter.js deleted file mode 100644 index 1a71d0f4a20..00000000000 --- a/build/lib/globalsLinter.js +++ /dev/null @@ -1,176 +0,0 @@ -"use strict"; -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ -Object.defineProperty(exports, "__esModule", { value: true }); -const ts = require("typescript"); -const fs_1 = require("fs"); -const path_1 = require("path"); -const minimatch_1 = require("minimatch"); -// -// ############################################################################################# -// -// A custom typescript linter for the specific task of detecting the use of certain globals in a -// layer that does not allow the use. For example: -// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement) -// - using node.js globals in common/browser layer (e.g. process) -// -// Make changes to below RULES to lift certain files from these checks only if absolutely needed -// -// ############################################################################################# -// -const RULES = { - 'no-nodejs-globals': [ - { - 'target': '**/vs/**/test/{common,browser}/**', - 'allowed': [ - 'process', - 'Buffer', - '__filename', - '__dirname' - ] - }, - { - 'target': '**/vs/workbench/api/common/extHostExtensionService.ts', - 'allowed': [ - 'global' // -> safe access to 'global' - ] - }, - { - 'target': '**/vs/**/{common,browser}/**', - 'allowed': [ /* none */] - } - ], - 'no-dom-globals': [ - { - 'target': '**/vs/base/parts/quickopen/common/quickOpen.ts', - 'allowed': [ - 'HTMLElement' // quick open will be replaced with a different widget soon - ] - }, - { - 'target': '**/vs/**/test/{common,node,electron-main}/**', - 'allowed': [ - 'document', - 'HTMLElement', - 'createElement' - ] - }, - { - 'target': '**/vs/**/{common,node,electron-main}/**', - 'allowed': [ /* none */] - } - ] -}; -const TS_CONFIG_PATH = path_1.join(__dirname, '../../', 'src', 'tsconfig.json'); -const DOM_GLOBALS_DEFINITION = 'lib.dom.d.ts'; -const DISALLOWED_DOM_GLOBALS = [ - 'window', - 'document', - 'HTMLElement', - 'createElement' -]; -const NODE_GLOBALS_DEFINITION = '@types/node'; -const DISALLOWED_NODE_GLOBALS = [ - // https://nodejs.org/api/globals.html#globals_global_objects - 'NodeJS', - 'Buffer', - '__dirname', - '__filename', - 'clearImmediate', - 'exports', - 'global', - 'module', - 'process', - 'setImmediate' -]; -let hasErrors = false; -function checkFile(program, sourceFile, rule) { - checkNode(sourceFile); - function checkNode(node) { - if (node.kind !== ts.SyntaxKind.Identifier) { - return ts.forEachChild(node, checkNode); // recurse down - } - const text = node.getText(sourceFile); - if (!rule.disallowedGlobals.some(disallowedGlobal => disallowedGlobal === text)) { - return; // only if disallowed - } - if (rule.allowedGlobals.some(allowed => allowed === text)) { - return; // override - } - const checker = program.getTypeChecker(); - const symbol = checker.getSymbolAtLocation(node); - if (symbol) { - const declarations = symbol.declarations; - if (Array.isArray(declarations) && symbol.declarations.some(declaration => { - if (declaration) { - const parent = declaration.parent; - if (parent) { - const sourceFile = parent.getSourceFile(); - if (sourceFile) { - const fileName = sourceFile.fileName; - if (fileName && fileName.indexOf(rule.definition) >= 0) { - return true; - } - } - } - } - return false; - })) { - const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); - console.log(`build/lib/globalsLinter.ts: Cannot use global '${text}' in ${sourceFile.fileName} (${line + 1},${character + 1})`); - hasErrors = true; - } - } - } -} -function createProgram(tsconfigPath) { - const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile); - const configHostParser = { fileExists: fs_1.existsSync, readDirectory: ts.sys.readDirectory, readFile: file => fs_1.readFileSync(file, 'utf8'), useCaseSensitiveFileNames: process.platform === 'linux' }; - const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, path_1.resolve(path_1.dirname(tsconfigPath)), { noEmit: true }); - const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true); - return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost); -} -// -// Create program and start checking -// -const program = createProgram(TS_CONFIG_PATH); -for (const sourceFile of program.getSourceFiles()) { - let noDomGlobalsLinter = undefined; - let noNodeJSGlobalsLinter = undefined; - for (const rules of RULES['no-dom-globals']) { - if (minimatch_1.match([sourceFile.fileName], rules.target).length > 0) { - noDomGlobalsLinter = { allowed: rules.allowed }; - break; - } - } - for (const rules of RULES['no-nodejs-globals']) { - if (minimatch_1.match([sourceFile.fileName], rules.target).length > 0) { - noNodeJSGlobalsLinter = { allowed: rules.allowed }; - break; - } - } - if (!noDomGlobalsLinter && !noNodeJSGlobalsLinter) { - continue; // no rule to run - } - // No DOM Globals - if (noDomGlobalsLinter) { - checkFile(program, sourceFile, { - definition: DOM_GLOBALS_DEFINITION, - disallowedGlobals: DISALLOWED_DOM_GLOBALS, - allowedGlobals: noDomGlobalsLinter.allowed - }); - } - // No node.js Globals - if (noNodeJSGlobalsLinter) { - checkFile(program, sourceFile, { - definition: NODE_GLOBALS_DEFINITION, - disallowedGlobals: DISALLOWED_NODE_GLOBALS, - allowedGlobals: noNodeJSGlobalsLinter.allowed - }); - } -} -if (hasErrors) { - process.exit(1); -} diff --git a/build/lib/globalsLinter.ts b/build/lib/globalsLinter.ts deleted file mode 100644 index 07e87f3c685..00000000000 --- a/build/lib/globalsLinter.ts +++ /dev/null @@ -1,209 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as ts from 'typescript'; -import { readFileSync, existsSync } from 'fs'; -import { resolve, dirname, join } from 'path'; -import { match } from 'minimatch'; - -// -// ############################################################################################# -// -// A custom typescript linter for the specific task of detecting the use of certain globals in a -// layer that does not allow the use. For example: -// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement) -// - using node.js globals in common/browser layer (e.g. process) -// -// Make changes to below RULES to lift certain files from these checks only if absolutely needed -// -// ############################################################################################# -// - -const RULES = { - 'no-nodejs-globals': [ - { - 'target': '**/vs/**/test/{common,browser}/**', - 'allowed': [ // -> less strict for test files - 'process', - 'Buffer', - '__filename', - '__dirname' - ] - }, - { - 'target': '**/vs/workbench/api/common/extHostExtensionService.ts', - 'allowed': [ - 'global' // -> safe access to 'global' - ] - }, - { - 'target': '**/vs/**/{common,browser}/**', - 'allowed': [ /* none */] - } - ], - 'no-dom-globals': [ - { - 'target': '**/vs/base/parts/quickopen/common/quickOpen.ts', - 'allowed': [ - 'HTMLElement' // quick open will be replaced with a different widget soon - ] - }, - { - 'target': '**/vs/**/test/{common,node,electron-main}/**', - 'allowed': [ // -> less strict for test files - 'document', - 'HTMLElement', - 'createElement' - ] - }, - { - 'target': '**/vs/**/{common,node,electron-main}/**', - 'allowed': [ /* none */] - } - ] -}; - -const TS_CONFIG_PATH = join(__dirname, '../../', 'src', 'tsconfig.json'); - -const DOM_GLOBALS_DEFINITION = 'lib.dom.d.ts'; - -const DISALLOWED_DOM_GLOBALS = [ - 'window', - 'document', - 'HTMLElement', - 'createElement' -]; - -const NODE_GLOBALS_DEFINITION = '@types/node'; - -const DISALLOWED_NODE_GLOBALS = [ - // https://nodejs.org/api/globals.html#globals_global_objects - 'NodeJS', - 'Buffer', - '__dirname', - '__filename', - 'clearImmediate', - 'exports', - 'global', - 'module', - 'process', - 'setImmediate' -]; - -interface IRule { - definition: string; - disallowedGlobals: string[]; - allowedGlobals: string[]; -} - -let hasErrors = false; - -function checkFile(program: ts.Program, sourceFile: ts.SourceFile, rule: IRule) { - checkNode(sourceFile); - - function checkNode(node: ts.Node): void { - if (node.kind !== ts.SyntaxKind.Identifier) { - return ts.forEachChild(node, checkNode); // recurse down - } - - const text = node.getText(sourceFile); - - if (!rule.disallowedGlobals.some(disallowedGlobal => disallowedGlobal === text)) { - return; // only if disallowed - } - - if (rule.allowedGlobals.some(allowed => allowed === text)) { - return; // override - } - - const checker = program.getTypeChecker(); - const symbol = checker.getSymbolAtLocation(node); - if (symbol) { - const declarations = symbol.declarations; - if (Array.isArray(declarations) && symbol.declarations.some(declaration => { - if (declaration) { - const parent = declaration.parent; - if (parent) { - const sourceFile = parent.getSourceFile(); - if (sourceFile) { - const fileName = sourceFile.fileName; - if (fileName && fileName.indexOf(rule.definition) >= 0) { - return true; - } - } - } - } - - return false; - })) { - const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); - console.log(`build/lib/globalsLinter.ts: Cannot use global '${text}' in ${sourceFile.fileName} (${line + 1},${character + 1})`); - - hasErrors = true; - } - } - } -} - -function createProgram(tsconfigPath: string): ts.Program { - const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile); - - const configHostParser: ts.ParseConfigHost = { fileExists: existsSync, readDirectory: ts.sys.readDirectory, readFile: file => readFileSync(file, 'utf8'), useCaseSensitiveFileNames: process.platform === 'linux' }; - const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, resolve(dirname(tsconfigPath)), { noEmit: true }); - - const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true); - - return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost); -} - -// -// Create program and start checking -// -const program = createProgram(TS_CONFIG_PATH); - -for (const sourceFile of program.getSourceFiles()) { - let noDomGlobalsLinter: { allowed: string[] } | undefined = undefined; - let noNodeJSGlobalsLinter: { allowed: string[] } | undefined = undefined; - - for (const rules of RULES['no-dom-globals']) { - if (match([sourceFile.fileName], rules.target).length > 0) { - noDomGlobalsLinter = { allowed: rules.allowed }; - break; - } - } - - for (const rules of RULES['no-nodejs-globals']) { - if (match([sourceFile.fileName], rules.target).length > 0) { - noNodeJSGlobalsLinter = { allowed: rules.allowed }; - break; - } - } - - if (!noDomGlobalsLinter && !noNodeJSGlobalsLinter) { - continue; // no rule to run - } - - // No DOM Globals - if (noDomGlobalsLinter) { - checkFile(program, sourceFile, { - definition: DOM_GLOBALS_DEFINITION, - disallowedGlobals: DISALLOWED_DOM_GLOBALS, - allowedGlobals: noDomGlobalsLinter.allowed - }); - } - - // No node.js Globals - if (noNodeJSGlobalsLinter) { - checkFile(program, sourceFile, { - definition: NODE_GLOBALS_DEFINITION, - disallowedGlobals: DISALLOWED_NODE_GLOBALS, - allowedGlobals: noNodeJSGlobalsLinter.allowed - }); - } -} - -if (hasErrors) { - process.exit(1); -} diff --git a/build/lib/layersChecker.js b/build/lib/layersChecker.js new file mode 100644 index 00000000000..be871ae5228 --- /dev/null +++ b/build/lib/layersChecker.js @@ -0,0 +1,205 @@ +"use strict"; +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ +Object.defineProperty(exports, "__esModule", { value: true }); +const ts = require("typescript"); +const fs_1 = require("fs"); +const path_1 = require("path"); +const minimatch_1 = require("minimatch"); +// +// ############################################################################################# +// +// A custom typescript checker for the specific task of detecting the use of certain types in a +// layer that does not allow such use. For example: +// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement) +// - using node.js globals in common/browser layer (e.g. process) +// +// Make changes to below RULES to lift certain files from these checks only if absolutely needed +// +// ############################################################################################# +// +// Types we assume are present in all implementations of JS VMs (node.js, browsers) +// Feel free to add more core types as you see needed if present in node.js and browsers +const CORE_TYPES = [ + 'require', + 'atob', + 'btoa', + 'setTimeout', + 'clearTimeout', + 'setInterval', + 'clearInterval', + 'console', + 'log', + 'info', + 'warn', + 'error', + 'group', + 'groupEnd', + 'table', + 'Error', + 'String', + 'throws', + 'stack', + 'captureStackTrace', + 'stackTraceLimit', + 'TextDecoder', + 'TextEncoder', + 'encode', + 'decode', + 'self', + 'trimLeft', + 'trimRight' +]; +const RULES = [ + // Tests: skip + { + target: '**/vs/**/test/**', + skip: true // -> skip all test files + }, + // Common: vs/base/common/platform.ts + { + target: '**/vs/base/common/platform.ts', + allowedTypes: [ + ...CORE_TYPES, + // Safe access to postMessage() and friends + 'MessageEvent', + 'data' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts', + '@types' // no node.js + ] + }, + // Common: vs/workbench/api/common/extHostExtensionService.ts + { + target: '**/vs/workbench/api/common/extHostExtensionService.ts', + allowedTypes: [ + ...CORE_TYPES, + // Safe access to global + 'global' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts', + '@types' // no node.js + ] + }, + // Common + { + target: '**/vs/**/common/**', + allowedTypes: CORE_TYPES, + disallowedDefinitions: [ + 'lib.dom.d.ts', + '@types' // no node.js + ] + }, + // Browser + { + target: '**/vs/**/browser/**', + allowedTypes: CORE_TYPES, + disallowedDefinitions: [ + '@types' // no node.js + ] + }, + // node.js + { + target: '**/vs/**/node/**', + allowedTypes: [ + ...CORE_TYPES, + // --> types from node.d.ts that duplicate from lib.dom.d.ts + 'URL', + 'protocol', + 'hostname', + 'port', + 'pathname', + 'search', + 'username', + 'password' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts' // no DOM + ] + }, + // Electron (renderer): skip + { + target: '**/vs/**/electron-browser/**', + skip: true // -> supports all types + }, + // Electron (main) + { + target: '**/vs/**/electron-main/**', + allowedTypes: [ + ...CORE_TYPES, + // --> types from electron.d.ts that duplicate from lib.dom.d.ts + 'Event', + 'Request' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts' // no DOM + ] + } +]; +const TS_CONFIG_PATH = path_1.join(__dirname, '../../', 'src', 'tsconfig.json'); +let hasErrors = false; +function checkFile(program, sourceFile, rule) { + checkNode(sourceFile); + function checkNode(node) { + var _a, _b; + if (node.kind !== ts.SyntaxKind.Identifier) { + return ts.forEachChild(node, checkNode); // recurse down + } + const text = node.getText(sourceFile); + if ((_a = rule.allowedTypes) === null || _a === void 0 ? void 0 : _a.some(allowed => allowed === text)) { + return; // override + } + const checker = program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + const declarations = symbol.declarations; + if (Array.isArray(declarations)) { + for (const declaration of declarations) { + if (declaration) { + const parent = declaration.parent; + if (parent) { + const parentSourceFile = parent.getSourceFile(); + if (parentSourceFile) { + const definitionFileName = parentSourceFile.fileName; + if ((_b = rule.disallowedDefinitions) === null || _b === void 0 ? void 0 : _b.some(disallowedDefinition => definitionFileName.indexOf(disallowedDefinition) >= 0)) { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from ${path_1.basename(definitionFileName)} violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); + hasErrors = true; + break; + } + } + } + } + } + } + } + } +} +function createProgram(tsconfigPath) { + const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile); + const configHostParser = { fileExists: fs_1.existsSync, readDirectory: ts.sys.readDirectory, readFile: file => fs_1.readFileSync(file, 'utf8'), useCaseSensitiveFileNames: process.platform === 'linux' }; + const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, path_1.resolve(path_1.dirname(tsconfigPath)), { noEmit: true }); + const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true); + return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost); +} +// +// Create program and start checking +// +const program = createProgram(TS_CONFIG_PATH); +for (const sourceFile of program.getSourceFiles()) { + for (const rule of RULES) { + if (minimatch_1.match([sourceFile.fileName], rule.target).length > 0) { + if (!rule.skip) { + checkFile(program, sourceFile, rule); + } + break; + } + } +} +if (hasErrors) { + process.exit(1); +} diff --git a/build/lib/layersChecker.ts b/build/lib/layersChecker.ts new file mode 100644 index 00000000000..800fc3590a6 --- /dev/null +++ b/build/lib/layersChecker.ts @@ -0,0 +1,241 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as ts from 'typescript'; +import { readFileSync, existsSync } from 'fs'; +import { resolve, dirname, join, basename } from 'path'; +import { match } from 'minimatch'; + +// +// ############################################################################################# +// +// A custom typescript checker for the specific task of detecting the use of certain types in a +// layer that does not allow such use. For example: +// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement) +// - using node.js globals in common/browser layer (e.g. process) +// +// Make changes to below RULES to lift certain files from these checks only if absolutely needed +// +// ############################################################################################# +// + +// Types we assume are present in all implementations of JS VMs (node.js, browsers) +// Feel free to add more core types as you see needed if present in node.js and browsers +const CORE_TYPES = [ + 'require', // from our AMD loader + 'atob', + 'btoa', + 'setTimeout', + 'clearTimeout', + 'setInterval', + 'clearInterval', + 'console', + 'log', + 'info', + 'warn', + 'error', + 'group', + 'groupEnd', + 'table', + 'Error', + 'String', + 'throws', + 'stack', + 'captureStackTrace', + 'stackTraceLimit', + 'TextDecoder', + 'TextEncoder', + 'encode', + 'decode', + 'self', + 'trimLeft', + 'trimRight' +]; + +const RULES = [ + + // Tests: skip + { + target: '**/vs/**/test/**', + skip: true // -> skip all test files + }, + + // Common: vs/base/common/platform.ts + { + target: '**/vs/base/common/platform.ts', + allowedTypes: [ + ...CORE_TYPES, + + // Safe access to postMessage() and friends + 'MessageEvent', + 'data' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts', // no DOM + '@types' // no node.js + ] + }, + + // Common: vs/workbench/api/common/extHostExtensionService.ts + { + target: '**/vs/workbench/api/common/extHostExtensionService.ts', + allowedTypes: [ + ...CORE_TYPES, + + // Safe access to global + 'global' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts', // no DOM + '@types' // no node.js + ] + }, + + // Common + { + target: '**/vs/**/common/**', + allowedTypes: CORE_TYPES, + disallowedDefinitions: [ + 'lib.dom.d.ts', // no DOM + '@types' // no node.js + ] + }, + + // Browser + { + target: '**/vs/**/browser/**', + allowedTypes: CORE_TYPES, + disallowedDefinitions: [ + '@types' // no node.js + ] + }, + + // node.js + { + target: '**/vs/**/node/**', + allowedTypes: [ + ...CORE_TYPES, + + // --> types from node.d.ts that duplicate from lib.dom.d.ts + 'URL', + 'protocol', + 'hostname', + 'port', + 'pathname', + 'search', + 'username', + 'password' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts' // no DOM + ] + }, + + // Electron (renderer): skip + { + target: '**/vs/**/electron-browser/**', + skip: true // -> supports all types + }, + + // Electron (main) + { + target: '**/vs/**/electron-main/**', + allowedTypes: [ + ...CORE_TYPES, + + // --> types from electron.d.ts that duplicate from lib.dom.d.ts + 'Event', + 'Request' + ], + disallowedDefinitions: [ + 'lib.dom.d.ts' // no DOM + ] + } +]; + +const TS_CONFIG_PATH = join(__dirname, '../../', 'src', 'tsconfig.json'); + +interface IRule { + target: string; + skip?: boolean; + allowedTypes?: string[]; + disallowedDefinitions?: string[]; +} + +let hasErrors = false; + +function checkFile(program: ts.Program, sourceFile: ts.SourceFile, rule: IRule) { + checkNode(sourceFile); + + function checkNode(node: ts.Node): void { + if (node.kind !== ts.SyntaxKind.Identifier) { + return ts.forEachChild(node, checkNode); // recurse down + } + + const text = node.getText(sourceFile); + + if (rule.allowedTypes?.some(allowed => allowed === text)) { + return; // override + } + + const checker = program.getTypeChecker(); + const symbol = checker.getSymbolAtLocation(node); + if (symbol) { + const declarations = symbol.declarations; + if (Array.isArray(declarations)) { + for (const declaration of declarations) { + if (declaration) { + const parent = declaration.parent; + if (parent) { + const parentSourceFile = parent.getSourceFile(); + if (parentSourceFile) { + const definitionFileName = parentSourceFile.fileName; + if (rule.disallowedDefinitions?.some(disallowedDefinition => definitionFileName.indexOf(disallowedDefinition) >= 0)) { + const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart()); + console.log(`[build/lib/layersChecker.ts]: Reference to '${text}' from ${basename(definitionFileName)} violates layers (${sourceFile.fileName} (${line + 1},${character + 1})`); + + hasErrors = true; + break; + } + } + } + } + } + } + } + } +} + +function createProgram(tsconfigPath: string): ts.Program { + const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile); + + const configHostParser: ts.ParseConfigHost = { fileExists: existsSync, readDirectory: ts.sys.readDirectory, readFile: file => readFileSync(file, 'utf8'), useCaseSensitiveFileNames: process.platform === 'linux' }; + const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, resolve(dirname(tsconfigPath)), { noEmit: true }); + + const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true); + + return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost); +} + +// +// Create program and start checking +// +const program = createProgram(TS_CONFIG_PATH); + +for (const sourceFile of program.getSourceFiles()) { + for (const rule of RULES) { + if (match([sourceFile.fileName], rule.target).length > 0) { + if (!rule.skip) { + checkFile(program, sourceFile, rule); + } + + break; + } + } +} + +if (hasErrors) { + process.exit(1); +} diff --git a/package.json b/package.json index 5a5d54ce626..aa95fce241d 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,7 @@ "smoketest": "cd test/smoke && node test/index.js", "download-builtin-extensions": "node build/lib/builtInExtensions.js", "monaco-compile-check": "tsc -p src/tsconfig.monaco.json --noEmit", - "valid-globals-check": "node build/lib/globalsLinter.js", + "valid-layers-check": "node build/lib/layersChecker.js", "strict-function-types-watch": "tsc --watch -p src/tsconfig.json --noEmit --strictFunctionTypes", "update-distro": "node build/npm/update-distro.js", "web": "node scripts/code-web.js", diff --git a/src/vs/base/common/worker/simpleWorker.ts b/src/vs/base/common/worker/simpleWorker.ts index 6c9815d3aaa..479c2ceb32c 100644 --- a/src/vs/base/common/worker/simpleWorker.ts +++ b/src/vs/base/common/worker/simpleWorker.ts @@ -12,7 +12,7 @@ const INITIALIZE = '$initialize'; export interface IWorker extends IDisposable { getId(): number; - postMessage(message: any, transfer: Transferable[]): void; + postMessage(message: any, transfer: ArrayBuffer[]): void; } export interface IWorkerCallback { @@ -302,7 +302,7 @@ export class SimpleWorkerServer { private _requestHandler: IRequestHandler | null; private _protocol: SimpleWorkerProtocol; - constructor(postMessage: (msg: any, transfer?: Transferable[]) => void, requestHandlerFactory: IRequestHandlerFactory | null) { + constructor(postMessage: (msg: any, transfer?: ArrayBuffer[]) => void, requestHandlerFactory: IRequestHandlerFactory | null) { this._requestHandlerFactory = requestHandlerFactory; this._requestHandler = null; this._protocol = new SimpleWorkerProtocol({ diff --git a/src/vs/base/parts/quickopen/common/quickOpen.ts b/src/vs/base/parts/quickopen/common/quickOpen.ts index d6039c68802..582ddf56ee6 100644 --- a/src/vs/base/parts/quickopen/common/quickOpen.ts +++ b/src/vs/base/parts/quickopen/common/quickOpen.ts @@ -68,9 +68,7 @@ export interface IDataSource { export interface IRenderer { getHeight(entry: T): number; getTemplateId(entry: T): string; - // rationale: will be replaced by quickinput later - // tslint:disable-next-line: no-dom-globals - renderTemplate(templateId: string, container: HTMLElement, styles: any): any; + renderTemplate(templateId: string, container: any /* HTMLElement */, styles: any): any; renderElement(entry: T, templateId: string, templateData: any, styles: any): void; disposeTemplate(templateId: string, templateData: any): void; }