Skip to content

Commit 433e9b4

Browse files
author
Andy Hanson
committed
Code review
1 parent a9aaed5 commit 433e9b4

File tree

5 files changed

+42
-63
lines changed

5 files changed

+42
-63
lines changed

‎src/compiler/commandLineParser.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ namespace ts {
160160
esnext: ScriptTarget.ESNext,
161161
}),
162162
affectsSourceFile: true,
163+
affectsModuleResolution: true,
163164
paramType: Diagnostics.VERSION,
164165
showInSimplifiedHelpView: true,
165166
category: Diagnostics.Basic_Options,
@@ -191,6 +192,7 @@ namespace ts {
191192
name: "lib",
192193
type: libMap
193194
},
195+
affectsModuleResolution: true,
194196
showInSimplifiedHelpView: true,
195197
category: Diagnostics.Basic_Options,
196198
description: Diagnostics.Specify_library_files_to_be_included_in_the_compilation
@@ -327,6 +329,7 @@ namespace ts {
327329
{
328330
name: "noImplicitAny",
329331
type: "boolean",
332+
affectsSemanticDiagnostics: true,
330333
strictFlag: true,
331334
showInSimplifiedHelpView: true,
332335
category: Diagnostics.Strict_Type_Checking_Options,
@@ -335,6 +338,7 @@ namespace ts {
335338
{
336339
name: "strictNullChecks",
337340
type: "boolean",
341+
affectsSemanticDiagnostics: true,
338342
strictFlag: true,
339343
showInSimplifiedHelpView: true,
340344
category: Diagnostics.Strict_Type_Checking_Options,
@@ -343,6 +347,7 @@ namespace ts {
343347
{
344348
name: "strictFunctionTypes",
345349
type: "boolean",
350+
affectsSemanticDiagnostics: true,
346351
strictFlag: true,
347352
showInSimplifiedHelpView: true,
348353
category: Diagnostics.Strict_Type_Checking_Options,
@@ -351,6 +356,7 @@ namespace ts {
351356
{
352357
name: "strictPropertyInitialization",
353358
type: "boolean",
359+
affectsSemanticDiagnostics: true,
354360
strictFlag: true,
355361
showInSimplifiedHelpView: true,
356362
category: Diagnostics.Strict_Type_Checking_Options,
@@ -359,6 +365,7 @@ namespace ts {
359365
{
360366
name: "noImplicitThis",
361367
type: "boolean",
368+
affectsSemanticDiagnostics: true,
362369
strictFlag: true,
363370
showInSimplifiedHelpView: true,
364371
category: Diagnostics.Strict_Type_Checking_Options,
@@ -462,6 +469,7 @@ namespace ts {
462469
type: "string",
463470
isFilePath: true
464471
},
472+
affectsModuleResolution: true,
465473
category: Diagnostics.Module_Resolution_Options,
466474
description: Diagnostics.List_of_folders_to_include_type_definitions_from
467475
},
@@ -472,6 +480,7 @@ namespace ts {
472480
name: "types",
473481
type: "string"
474482
},
483+
affectsModuleResolution: true,
475484
showInSimplifiedHelpView: true,
476485
category: Diagnostics.Module_Resolution_Options,
477486
description: Diagnostics.Type_declaration_files_to_be_included_in_compilation
@@ -642,6 +651,7 @@ namespace ts {
642651
{
643652
name: "noLib",
644653
type: "boolean",
654+
affectsModuleResolution: true,
645655
category: Diagnostics.Advanced_Options,
646656
description: Diagnostics.Do_not_include_the_default_library_file_lib_d_ts
647657
},
@@ -743,6 +753,7 @@ namespace ts {
743753
{
744754
name: "maxNodeModuleJsDepth",
745755
type: "number",
756+
// TODO: GH#27108 affectsModuleResolution: true,
746757
category: Diagnostics.Advanced_Options,
747758
description: Diagnostics.The_maximum_dependency_depth_to_search_under_node_modules_and_load_JavaScript_files
748759
},
@@ -772,6 +783,18 @@ namespace ts {
772783
}
773784
];
774785

786+
/* @internal */
787+
export const semanticDiagnosticsOptionDeclarations: ReadonlyArray<CommandLineOption> =
788+
optionDeclarations.filter(option => !!option.affectsSemanticDiagnostics);
789+
790+
/* @internal */
791+
export const moduleResolutionOptionDeclarations: ReadonlyArray<CommandLineOption> =
792+
optionDeclarations.filter(option => !!option.affectsModuleResolution);
793+
794+
/* @internal */
795+
export const sourceFileAffectingCompilerOptions: ReadonlyArray<CommandLineOption> = optionDeclarations.filter(option =>
796+
!!option.affectsSourceFile || !!option.affectsModuleResolution || !!option.affectsBindDiagnostics);
797+
775798
/* @internal */
776799
export const buildOpts: CommandLineOption[] = [
777800
...commonOptionsWithBuild,

‎src/compiler/program.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -492,11 +492,13 @@ namespace ts {
492492
/**
493493
* Determine if source file needs to be re-created even if its text hasn't changed
494494
*/
495-
function shouldProgramCreateNewSourceFiles(program: Program | undefined, newOptions: CompilerOptions) {
495+
function shouldProgramCreateNewSourceFiles(program: Program | undefined, newOptions: CompilerOptions): boolean {
496+
if (!program) return false;
496497
// If any compiler options change, we can't reuse old source file even if version match
497498
// The change in options like these could result in change in syntax tree or `sourceFile.bindDiagnostics`.
498-
const oldOptions = program && program.getCompilerOptions();
499-
return !!oldOptions && !isJsonEqual(getSourceFileAffectingCompilerOptions(oldOptions), getSourceFileAffectingCompilerOptions(newOptions));
499+
const oldOptions = program.getCompilerOptions();
500+
return !!sourceFileAffectingCompilerOptions.some(option =>
501+
!isJsonEqual(getCompilerOptionValue(oldOptions, option), getCompilerOptionValue(newOptions, option)));
500502
}
501503

502504
function createCreateProgramOptions(rootNames: ReadonlyArray<string>, options: CompilerOptions, host?: CompilerHost, oldProgram?: Program, configFileParsingDiagnostics?: ReadonlyArray<Diagnostic>): CreateProgramOptions {

‎src/compiler/utilities.ts

Lines changed: 9 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -101,22 +101,8 @@ namespace ts {
101101
}
102102

103103
export function changesAffectModuleResolution(oldOptions: CompilerOptions, newOptions: CompilerOptions): boolean {
104-
return !oldOptions ||
105-
(oldOptions.module !== newOptions.module) ||
106-
(oldOptions.moduleResolution !== newOptions.moduleResolution) ||
107-
(oldOptions.noResolve !== newOptions.noResolve) ||
108-
(oldOptions.target !== newOptions.target) ||
109-
(oldOptions.noLib !== newOptions.noLib) ||
110-
(oldOptions.jsx !== newOptions.jsx) ||
111-
(oldOptions.allowJs !== newOptions.allowJs) ||
112-
(oldOptions.rootDir !== newOptions.rootDir) ||
113-
(oldOptions.configFilePath !== newOptions.configFilePath) ||
114-
(oldOptions.baseUrl !== newOptions.baseUrl) ||
115-
(oldOptions.maxNodeModuleJsDepth !== newOptions.maxNodeModuleJsDepth) ||
116-
!arrayIsEqualTo(oldOptions.lib, newOptions.lib) ||
117-
!arrayIsEqualTo(oldOptions.typeRoots, newOptions.typeRoots) ||
118-
!arrayIsEqualTo(oldOptions.rootDirs, newOptions.rootDirs) ||
119-
!equalOwnProperties(oldOptions.paths, newOptions.paths);
104+
return oldOptions.configFilePath !== newOptions.configFilePath || moduleResolutionOptionDeclarations.some(o =>
105+
!isJsonEqual(getCompilerOptionValue(oldOptions, o), getCompilerOptionValue(newOptions, o)));
120106
}
121107

122108
/**
@@ -7095,33 +7081,18 @@ namespace ts {
70957081
}
70967082

70977083
export type StrictOptionName = "noImplicitAny" | "noImplicitThis" | "strictNullChecks" | "strictFunctionTypes" | "strictPropertyInitialization" | "alwaysStrict";
7098-
export function isStrictOptionName(name: string): name is StrictOptionName {
7099-
const strictName = name as StrictOptionName;
7100-
switch (strictName) {
7101-
case "noImplicitAny":
7102-
case "noImplicitThis":
7103-
case "strictNullChecks":
7104-
case "strictFunctionTypes":
7105-
case "strictPropertyInitialization":
7106-
case "alwaysStrict":
7107-
return true;
7108-
default:
7109-
assertType<never>(strictName);
7110-
return false;
7111-
}
7112-
}
71137084

71147085
export function getStrictOptionValue(compilerOptions: CompilerOptions, flag: StrictOptionName): boolean {
71157086
return compilerOptions[flag] === undefined ? !!compilerOptions.strict : !!compilerOptions[flag];
71167087
}
71177088

7118-
export function compilerOptionsAffectSemanticDiagnostics(newOptions: CompilerOptions, oldOptions: CompilerOptions) {
7119-
if (oldOptions === newOptions) {
7120-
return false;
7121-
}
7089+
export function compilerOptionsAffectSemanticDiagnostics(newOptions: CompilerOptions, oldOptions: CompilerOptions): boolean {
7090+
return oldOptions !== newOptions &&
7091+
semanticDiagnosticsOptionDeclarations.some(option => !isJsonEqual(getCompilerOptionValue(oldOptions, option), getCompilerOptionValue(newOptions, option)));
7092+
}
71227093

7123-
return optionDeclarations.some(option => (!!option.strictFlag && getStrictOptionValue(newOptions, option.name as StrictOptionName) !== getStrictOptionValue(oldOptions, option.name as StrictOptionName)) ||
7124-
(!!option.affectsSemanticDiagnostics && !newOptions[option.name] !== !oldOptions[option.name]));
7094+
export function getCompilerOptionValue(options: CompilerOptions, option: CommandLineOption): unknown {
7095+
return option.strictFlag ? getStrictOptionValue(options, option.name as StrictOptionName) : options[option.name];
71257096
}
71267097

71277098
export function hasZeroOrOneAsteriskCharacter(str: string): boolean {
@@ -8391,17 +8362,6 @@ namespace ts {
83918362
}
83928363

83938364
export function isJsonEqual(a: unknown, b: unknown): boolean {
8394-
return a === b || typeof a === "object" && a !== null && typeof b === "object" && b !== null && equalOwnProperties(a as MapLike<unknown>, b as MapLike<unknown>);
8395-
}
8396-
8397-
export function getSourceFileAffectingCompilerOptions(compilerOptions: CompilerOptions): CompilerOptions {
8398-
const res: CompilerOptions = {};
8399-
for (const option of optionDeclarations) {
8400-
// SourceFile stores `resolvedModules` and `bindDiagnostics` so changing those means changing the SourceFile
8401-
if (option.name in compilerOptions && (!!option.affectsSourceFile || !!option.affectsModuleResolution || !!option.affectsBindDiagnostics)) {
8402-
res[option.name] = isStrictOptionName(option.name) ? getStrictOptionValue(compilerOptions, option.name) : compilerOptions[option.name];
8403-
}
8404-
}
8405-
return res;
8365+
return a === b || typeof a === "object" && a !== null && typeof b === "object" && b !== null && equalOwnProperties(a as MapLike<unknown>, b as MapLike<unknown>, isJsonEqual);
84068366
}
84078367
}

‎src/services/documentRegistry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,6 @@ namespace ts {
271271
}
272272

273273
function getKeyForCompilationSettings(settings: CompilerOptions): DocumentRegistryBucketKey {
274-
return <DocumentRegistryBucketKey>JSON.stringify(getSourceFileAffectingCompilerOptions(settings));
274+
return sourceFileAffectingCompilerOptions.map(option => getCompilerOptionValue(settings, option)).join("|") as DocumentRegistryBucketKey;
275275
}
276276
}

‎src/testRunner/unittests/tscWatchMode.ts

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,37 +1345,31 @@ export class B
13451345
path: `${currentDirectory}/a.ts`,
13461346
content: `declare function foo(): null | { hello: any };
13471347
foo().hello`
1348-
};
1349-
const compilerOptions: CompilerOptions = {
13501348
};
13511349
const config: File = {
13521350
path: `${currentDirectory}/tsconfig.json`,
1353-
content: JSON.stringify({ compilerOptions })
1351+
content: JSON.stringify({ compilerOptions: {} })
13541352
};
13551353
const files = [aFile, config, libFile];
13561354
const host = createWatchedSystem(files, { currentDirectory });
13571355
const watch = createWatchOfConfigFile("tsconfig.json", host);
13581356
checkProgramActualFiles(watch(), [aFile.path, libFile.path]);
13591357
checkOutputErrorsInitial(host, emptyArray);
13601358
const modifiedTimeOfAJs = host.getModifiedTime(`${currentDirectory}/a.js`);
1361-
compilerOptions.strictNullChecks = true;
1362-
host.writeFile(config.path, JSON.stringify({ compilerOptions }));
1359+
host.writeFile(config.path, JSON.stringify({ compilerOptions: { strictNullChecks: true } }));
13631360
host.runQueuedTimeoutCallbacks();
13641361
const expectedStrictNullErrors = [
13651362
getDiagnosticOfFileFromProgram(watch(), aFile.path, aFile.content.lastIndexOf("foo()"), 5, Diagnostics.Object_is_possibly_null)
13661363
];
13671364
checkOutputErrorsIncremental(host, expectedStrictNullErrors);
13681365
// File a need not be rewritten
13691366
assert.equal(host.getModifiedTime(`${currentDirectory}/a.js`), modifiedTimeOfAJs);
1370-
compilerOptions.strict = true;
1371-
delete (compilerOptions.strictNullChecks);
1372-
host.writeFile(config.path, JSON.stringify({ compilerOptions }));
1367+
host.writeFile(config.path, JSON.stringify({ compilerOptions: { strict: true, alwaysStrict: false } })); // Avoid changing 'alwaysStrict' or must re-bind
13731368
host.runQueuedTimeoutCallbacks();
13741369
checkOutputErrorsIncremental(host, expectedStrictNullErrors);
13751370
// File a need not be rewritten
13761371
assert.equal(host.getModifiedTime(`${currentDirectory}/a.js`), modifiedTimeOfAJs);
1377-
delete (compilerOptions.strict);
1378-
host.writeFile(config.path, JSON.stringify({ compilerOptions }));
1372+
host.writeFile(config.path, JSON.stringify({ compilerOptions: {} }));
13791373
host.runQueuedTimeoutCallbacks();
13801374
checkOutputErrorsIncremental(host, emptyArray);
13811375
// File a need not be rewritten

0 commit comments

Comments
 (0)