Skip to content

Commit 7909462

Browse files
authored
tslint - rewrite globals rule to not use tslint (#87754)
* tslint - rewrite globals rule to not use tslint * comments
1 parent 8aa2191 commit 7909462

16 files changed

+404
-317
lines changed

.github/workflows/ci.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ jobs:
4141
name: Run TSLint Checks
4242
- run: yarn monaco-compile-check
4343
name: Run Monaco Editor Checks
44+
- run: yarn valid-globals-check
45+
name: Run Valid Globals Checks
4446
- run: yarn compile
4547
name: Compile Sources
4648
- run: yarn download-builtin-extensions
@@ -73,6 +75,8 @@ jobs:
7375
name: Run TSLint Checks
7476
- run: yarn monaco-compile-check
7577
name: Run Monaco Editor Checks
78+
- run: yarn valid-globals-check
79+
name: Run Valid Globals Checks
7680
- run: yarn compile
7781
name: Compile Sources
7882
- run: yarn download-builtin-extensions
@@ -102,6 +106,8 @@ jobs:
102106
name: Run TSLint Checks
103107
- run: yarn monaco-compile-check
104108
name: Run Monaco Editor Checks
109+
- run: yarn valid-globals-check
110+
name: Run Valid Globals Checks
105111
- run: yarn compile
106112
name: Compile Sources
107113
- run: yarn download-builtin-extensions

build/azure-pipelines/darwin/continuous-build-darwin.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ steps:
3232
- script: |
3333
yarn monaco-compile-check
3434
displayName: Run Monaco Editor Checks
35+
- script: |
36+
yarn valid-globals-check
37+
displayName: Run Valid Globals Checks
3538
- script: |
3639
yarn compile
3740
displayName: Compile Sources

build/azure-pipelines/linux/continuous-build-linux.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ steps:
4040
- script: |
4141
yarn monaco-compile-check
4242
displayName: Run Monaco Editor Checks
43+
- script: |
44+
yarn valid-globals-check
45+
displayName: Run Valid Globals Checks
4346
- script: |
4447
yarn compile
4548
displayName: Compile Sources

build/azure-pipelines/product-compile.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ steps:
9191
yarn gulp hygiene --skip-tslint
9292
yarn gulp tslint
9393
yarn monaco-compile-check
94-
displayName: Run hygiene, tslint and monaco compile checks
94+
yarn valid-globals-check
95+
displayName: Run hygiene, tslint, monaco compile & valid globals checks
9596
condition: and(succeeded(), ne(variables['CacheExists-Compilation'], 'true'), eq(variables['VSCODE_STEP_ON_IT'], 'false'))
9697

9798
- script: |

build/azure-pipelines/win32/continuous-build-win32.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ steps:
3737
- powershell: |
3838
yarn monaco-compile-check
3939
displayName: Run Monaco Editor Checks
40+
- script: |
41+
yarn valid-globals-check
42+
displayName: Run Valid Globals Checks
4043
- powershell: |
4144
yarn compile
4245
displayName: Compile Sources

build/lib/globalsLinter.js

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
"use strict";
2+
/*---------------------------------------------------------------------------------------------
3+
* Copyright (c) Microsoft Corporation. All rights reserved.
4+
* Licensed under the MIT License. See License.txt in the project root for license information.
5+
*--------------------------------------------------------------------------------------------*/
6+
Object.defineProperty(exports, "__esModule", { value: true });
7+
const ts = require("typescript");
8+
const fs_1 = require("fs");
9+
const path_1 = require("path");
10+
const minimatch_1 = require("minimatch");
11+
//
12+
// #############################################################################################
13+
//
14+
// A custom typescript linter for the specific task of detecting the use of certain globals in a
15+
// layer that does not allow the use. For example:
16+
// - using DOM globals in common/node/electron-main layer (e.g. HTMLElement)
17+
// - using node.js globals in common/browser layer (e.g. process)
18+
//
19+
// Make changes to below RULES to lift certain files from these checks only if absolutely needed
20+
//
21+
// #############################################################################################
22+
//
23+
const RULES = {
24+
"no-nodejs-globals": [
25+
{
26+
"target": "**/vs/**/test/{common,browser}/**",
27+
"allowed": [
28+
"process",
29+
"Buffer",
30+
"__filename",
31+
"__dirname"
32+
]
33+
},
34+
{
35+
"target": "**/vs/workbench/api/common/extHostExtensionService.ts",
36+
"allowed": [
37+
"global" // -> safe access to 'global'
38+
]
39+
},
40+
{
41+
"target": "**/vs/**/{common,browser}/**",
42+
"allowed": [ /* none */]
43+
}
44+
],
45+
"no-dom-globals": [
46+
{
47+
"target": "**/vs/base/parts/quickopen/common/quickOpen.ts",
48+
"allowed": [
49+
"HTMLElement" // quick open will be replaced with a different widget soon
50+
]
51+
},
52+
{
53+
"target": "**/vs/**/test/{common,node,electron-main}/**",
54+
"allowed": [
55+
"document",
56+
"HTMLElement",
57+
"createElement"
58+
]
59+
},
60+
{
61+
"target": "**/vs/**/{common,node,electron-main}/**",
62+
"allowed": [ /* none */]
63+
}
64+
]
65+
};
66+
const TS_CONFIG_PATH = path_1.join(__dirname, '../../', 'src', 'tsconfig.json');
67+
const DOM_GLOBALS_DEFINITION = 'lib.dom.d.ts';
68+
const DISALLOWED_DOM_GLOBALS = [
69+
"window",
70+
"document",
71+
"HTMLElement",
72+
"createElement"
73+
];
74+
const NODE_GLOBALS_DEFINITION = '@types/node';
75+
const DISALLOWED_NODE_GLOBALS = [
76+
// https://nodejs.org/api/globals.html#globals_global_objects
77+
"NodeJS",
78+
"Buffer",
79+
"__dirname",
80+
"__filename",
81+
"clearImmediate",
82+
"exports",
83+
"global",
84+
"module",
85+
"process",
86+
"setImmediate"
87+
];
88+
let hasErrors = false;
89+
function checkFile(program, sourceFile, rule) {
90+
checkNode(sourceFile);
91+
function checkNode(node) {
92+
if (node.kind !== ts.SyntaxKind.Identifier) {
93+
return ts.forEachChild(node, checkNode); // recurse down
94+
}
95+
const text = node.getText(sourceFile);
96+
if (!rule.disallowedGlobals.some(disallowedGlobal => disallowedGlobal === text)) {
97+
return; // only if disallowed
98+
}
99+
if (rule.allowedGlobals.some(allowed => allowed === text)) {
100+
return; // override
101+
}
102+
const checker = program.getTypeChecker();
103+
const symbol = checker.getSymbolAtLocation(node);
104+
if (symbol) {
105+
const declarations = symbol.declarations;
106+
if (Array.isArray(declarations) && symbol.declarations.some(declaration => {
107+
if (declaration) {
108+
const parent = declaration.parent;
109+
if (parent) {
110+
const sourceFile = parent.getSourceFile();
111+
if (sourceFile) {
112+
const fileName = sourceFile.fileName;
113+
if (fileName && fileName.indexOf(rule.definition) >= 0) {
114+
return true;
115+
}
116+
}
117+
}
118+
}
119+
return false;
120+
})) {
121+
const { line, character } = sourceFile.getLineAndCharacterOfPosition(node.getStart());
122+
console.log(`build/lib/globalsLinter.ts: Cannot use global '${text}' in ${sourceFile.fileName} (${line + 1},${character + 1})`);
123+
hasErrors = true;
124+
}
125+
}
126+
}
127+
}
128+
function createProgram(tsconfigPath) {
129+
const tsConfig = ts.readConfigFile(tsconfigPath, ts.sys.readFile);
130+
const configHostParser = { fileExists: fs_1.existsSync, readDirectory: ts.sys.readDirectory, readFile: file => fs_1.readFileSync(file, "utf8"), useCaseSensitiveFileNames: process.platform === 'linux' };
131+
const tsConfigParsed = ts.parseJsonConfigFileContent(tsConfig.config, configHostParser, path_1.resolve(path_1.dirname(tsconfigPath)), { noEmit: true });
132+
const compilerHost = ts.createCompilerHost(tsConfigParsed.options, true);
133+
return ts.createProgram(tsConfigParsed.fileNames, tsConfigParsed.options, compilerHost);
134+
}
135+
//
136+
// Create program and start checking
137+
//
138+
const program = createProgram(TS_CONFIG_PATH);
139+
for (const sourceFile of program.getSourceFiles()) {
140+
let noDomGlobalsLinter = undefined;
141+
let noNodeJSGlobalsLinter = undefined;
142+
for (const rules of RULES["no-dom-globals"]) {
143+
if (minimatch_1.match([sourceFile.fileName], rules.target).length > 0) {
144+
noDomGlobalsLinter = { allowed: rules.allowed };
145+
break;
146+
}
147+
}
148+
for (const rules of RULES["no-nodejs-globals"]) {
149+
if (minimatch_1.match([sourceFile.fileName], rules.target).length > 0) {
150+
noNodeJSGlobalsLinter = { allowed: rules.allowed };
151+
break;
152+
}
153+
}
154+
if (!noDomGlobalsLinter && !noNodeJSGlobalsLinter) {
155+
continue; // no rule to run
156+
}
157+
// No DOM Globals
158+
if (noDomGlobalsLinter) {
159+
checkFile(program, sourceFile, {
160+
definition: DOM_GLOBALS_DEFINITION,
161+
disallowedGlobals: DISALLOWED_DOM_GLOBALS,
162+
allowedGlobals: noDomGlobalsLinter.allowed
163+
});
164+
}
165+
// No node.js Globals
166+
if (noNodeJSGlobalsLinter) {
167+
checkFile(program, sourceFile, {
168+
definition: NODE_GLOBALS_DEFINITION,
169+
disallowedGlobals: DISALLOWED_NODE_GLOBALS,
170+
allowedGlobals: noNodeJSGlobalsLinter.allowed
171+
});
172+
}
173+
}
174+
if (hasErrors) {
175+
process.exit(1);
176+
}

0 commit comments

Comments
 (0)