Skip to content

Commit a57467a

Browse files
author
Andy
authored
Fix bugs: Replace SourceFile if '--noUnusedLabels' changed (#27060)
* Fix bugs: Replace SourceFile if '--noUnusedLabels' changed * Use properties on CommandLineOptionBase * Handle "alwaysStrict" and better categorize options * Properly handle "strict" * Code review * fix test
1 parent c615718 commit a57467a

9 files changed

+128
-56
lines changed

‎src/compiler/commandLineParser.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ namespace ts {
172172
es2018: ScriptTarget.ES2018,
173173
esnext: ScriptTarget.ESNext,
174174
}),
175+
affectsSourceFile: true,
176+
affectsModuleResolution: true,
175177
paramType: Diagnostics.VERSION,
176178
showInSimplifiedHelpView: true,
177179
category: Diagnostics.Basic_Options,
@@ -190,6 +192,7 @@ namespace ts {
190192
es2015: ModuleKind.ES2015,
191193
esnext: ModuleKind.ESNext
192194
}),
195+
affectsModuleResolution: true,
193196
paramType: Diagnostics.KIND,
194197
showInSimplifiedHelpView: true,
195198
category: Diagnostics.Basic_Options,
@@ -202,13 +205,15 @@ namespace ts {
202205
name: "lib",
203206
type: libMap
204207
},
208+
affectsModuleResolution: true,
205209
showInSimplifiedHelpView: true,
206210
category: Diagnostics.Basic_Options,
207211
description: Diagnostics.Specify_library_files_to_be_included_in_the_compilation
208212
},
209213
{
210214
name: "allowJs",
211215
type: "boolean",
216+
affectsModuleResolution: true,
212217
showInSimplifiedHelpView: true,
213218
category: Diagnostics.Basic_Options,
214219
description: Diagnostics.Allow_javascript_files_to_be_compiled
@@ -226,6 +231,7 @@ namespace ts {
226231
"react-native": JsxEmit.ReactNative,
227232
"react": JsxEmit.React
228233
}),
234+
affectsSourceFile: true,
229235
paramType: Diagnostics.KIND,
230236
showInSimplifiedHelpView: true,
231237
category: Diagnostics.Basic_Options,
@@ -336,6 +342,7 @@ namespace ts {
336342
{
337343
name: "noImplicitAny",
338344
type: "boolean",
345+
affectsSemanticDiagnostics: true,
339346
strictFlag: true,
340347
showInSimplifiedHelpView: true,
341348
category: Diagnostics.Strict_Type_Checking_Options,
@@ -344,6 +351,7 @@ namespace ts {
344351
{
345352
name: "strictNullChecks",
346353
type: "boolean",
354+
affectsSemanticDiagnostics: true,
347355
strictFlag: true,
348356
showInSimplifiedHelpView: true,
349357
category: Diagnostics.Strict_Type_Checking_Options,
@@ -352,6 +360,7 @@ namespace ts {
352360
{
353361
name: "strictFunctionTypes",
354362
type: "boolean",
363+
affectsSemanticDiagnostics: true,
355364
strictFlag: true,
356365
showInSimplifiedHelpView: true,
357366
category: Diagnostics.Strict_Type_Checking_Options,
@@ -360,6 +369,7 @@ namespace ts {
360369
{
361370
name: "strictPropertyInitialization",
362371
type: "boolean",
372+
affectsSemanticDiagnostics: true,
363373
strictFlag: true,
364374
showInSimplifiedHelpView: true,
365375
category: Diagnostics.Strict_Type_Checking_Options,
@@ -368,6 +378,7 @@ namespace ts {
368378
{
369379
name: "noImplicitThis",
370380
type: "boolean",
381+
affectsSemanticDiagnostics: true,
371382
strictFlag: true,
372383
showInSimplifiedHelpView: true,
373384
category: Diagnostics.Strict_Type_Checking_Options,
@@ -376,6 +387,7 @@ namespace ts {
376387
{
377388
name: "alwaysStrict",
378389
type: "boolean",
390+
affectsSourceFile: true,
379391
strictFlag: true,
380392
showInSimplifiedHelpView: true,
381393
category: Diagnostics.Strict_Type_Checking_Options,
@@ -410,6 +422,7 @@ namespace ts {
410422
{
411423
name: "noFallthroughCasesInSwitch",
412424
type: "boolean",
425+
affectsBindDiagnostics: true,
413426
affectsSemanticDiagnostics: true,
414427
showInSimplifiedHelpView: true,
415428
category: Diagnostics.Additional_Checks,
@@ -423,13 +436,15 @@ namespace ts {
423436
node: ModuleResolutionKind.NodeJs,
424437
classic: ModuleResolutionKind.Classic,
425438
}),
439+
affectsModuleResolution: true,
426440
paramType: Diagnostics.STRATEGY,
427441
category: Diagnostics.Module_Resolution_Options,
428442
description: Diagnostics.Specify_module_resolution_strategy_Colon_node_Node_js_or_classic_TypeScript_pre_1_6,
429443
},
430444
{
431445
name: "baseUrl",
432446
type: "string",
447+
affectsModuleResolution: true,
433448
isFilePath: true,
434449
category: Diagnostics.Module_Resolution_Options,
435450
description: Diagnostics.Base_directory_to_resolve_non_absolute_module_names
@@ -439,6 +454,7 @@ namespace ts {
439454
// use type = object to copy the value as-is
440455
name: "paths",
441456
type: "object",
457+
affectsModuleResolution: true,
442458
isTSConfigOnly: true,
443459
category: Diagnostics.Module_Resolution_Options,
444460
description: Diagnostics.A_series_of_entries_which_re_map_imports_to_lookup_locations_relative_to_the_baseUrl
@@ -454,6 +470,7 @@ namespace ts {
454470
type: "string",
455471
isFilePath: true
456472
},
473+
affectsModuleResolution: true,
457474
category: Diagnostics.Module_Resolution_Options,
458475
description: Diagnostics.List_of_root_folders_whose_combined_content_represents_the_structure_of_the_project_at_runtime
459476
},
@@ -465,6 +482,7 @@ namespace ts {
465482
type: "string",
466483
isFilePath: true
467484
},
485+
affectsModuleResolution: true,
468486
category: Diagnostics.Module_Resolution_Options,
469487
description: Diagnostics.List_of_folders_to_include_type_definitions_from
470488
},
@@ -475,6 +493,7 @@ namespace ts {
475493
name: "types",
476494
type: "string"
477495
},
496+
affectsModuleResolution: true,
478497
showInSimplifiedHelpView: true,
479498
category: Diagnostics.Module_Resolution_Options,
480499
description: Diagnostics.Type_declaration_files_to_be_included_in_compilation
@@ -633,12 +652,14 @@ namespace ts {
633652
{
634653
name: "noLib",
635654
type: "boolean",
655+
affectsModuleResolution: true,
636656
category: Diagnostics.Advanced_Options,
637657
description: Diagnostics.Do_not_include_the_default_library_file_lib_d_ts
638658
},
639659
{
640660
name: "noResolve",
641661
type: "boolean",
662+
affectsModuleResolution: true,
642663
category: Diagnostics.Advanced_Options,
643664
description: Diagnostics.Do_not_add_triple_slash_references_or_imported_modules_to_the_list_of_compiled_files
644665
},
@@ -651,6 +672,7 @@ namespace ts {
651672
{
652673
name: "disableSizeLimit",
653674
type: "boolean",
675+
affectsSourceFile: true,
654676
category: Diagnostics.Advanced_Options,
655677
description: Diagnostics.Disable_size_limitations_on_JavaScript_projects
656678
},
@@ -696,13 +718,15 @@ namespace ts {
696718
{
697719
name: "allowUnusedLabels",
698720
type: "boolean",
721+
affectsBindDiagnostics: true,
699722
affectsSemanticDiagnostics: true,
700723
category: Diagnostics.Advanced_Options,
701724
description: Diagnostics.Do_not_report_errors_on_unused_labels
702725
},
703726
{
704727
name: "allowUnreachableCode",
705728
type: "boolean",
729+
affectsBindDiagnostics: true,
706730
affectsSemanticDiagnostics: true,
707731
category: Diagnostics.Advanced_Options,
708732
description: Diagnostics.Do_not_report_errors_on_unreachable_code
@@ -730,6 +754,7 @@ namespace ts {
730754
{
731755
name: "maxNodeModuleJsDepth",
732756
type: "number",
757+
// TODO: GH#27108 affectsModuleResolution: true,
733758
category: Diagnostics.Advanced_Options,
734759
description: Diagnostics.The_maximum_dependency_depth_to_search_under_node_modules_and_load_JavaScript_files
735760
},
@@ -759,6 +784,18 @@ namespace ts {
759784
}
760785
];
761786

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

‎src/compiler/program.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -496,23 +496,15 @@ namespace ts {
496496
}
497497

498498
/**
499-
* Determined if source file needs to be re-created even if its text hasn't changed
499+
* Determine if source file needs to be re-created even if its text hasn't changed
500500
*/
501-
function shouldProgramCreateNewSourceFiles(program: Program | undefined, newOptions: CompilerOptions) {
502-
// If any of these options change, we can't reuse old source file even if version match
503-
// The change in options like these could result in change in syntax tree change
504-
const oldOptions = program && program.getCompilerOptions();
505-
return oldOptions && (
506-
oldOptions.target !== newOptions.target ||
507-
oldOptions.module !== newOptions.module ||
508-
oldOptions.moduleResolution !== newOptions.moduleResolution ||
509-
oldOptions.noResolve !== newOptions.noResolve ||
510-
oldOptions.jsx !== newOptions.jsx ||
511-
oldOptions.allowJs !== newOptions.allowJs ||
512-
oldOptions.disableSizeLimit !== newOptions.disableSizeLimit ||
513-
oldOptions.baseUrl !== newOptions.baseUrl ||
514-
!equalOwnProperties(oldOptions.paths, newOptions.paths)
515-
);
501+
function shouldProgramCreateNewSourceFiles(program: Program | undefined, newOptions: CompilerOptions): boolean {
502+
if (!program) return false;
503+
// If any compiler options change, we can't reuse old source file even if version match
504+
// The change in options like these could result in change in syntax tree or `sourceFile.bindDiagnostics`.
505+
const oldOptions = program.getCompilerOptions();
506+
return !!sourceFileAffectingCompilerOptions.some(option =>
507+
!isJsonEqual(getCompilerOptionValue(oldOptions, option), getCompilerOptionValue(newOptions, option)));
516508
}
517509

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

‎src/compiler/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4569,6 +4569,9 @@ namespace ts {
45694569
showInSimplifiedHelpView?: boolean;
45704570
category?: DiagnosticMessage;
45714571
strictFlag?: true; // true if the option is one of the flag under strict
4572+
affectsSourceFile?: true; // true if we should recreate SourceFiles after this option changes
4573+
affectsModuleResolution?: true; // currently same effect as `affectsSourceFile`
4574+
affectsBindDiagnostics?: true; // true if this affects binding (currently same effect as `affectsSourceFile`)
45724575
affectsSemanticDiagnostics?: true; // true if option affects semantic diagnostics
45734576
}
45744577

‎src/compiler/utilities.ts

Lines changed: 12 additions & 22 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
/**
@@ -7106,13 +7092,13 @@ namespace ts {
71067092
return compilerOptions[flag] === undefined ? !!compilerOptions.strict : !!compilerOptions[flag];
71077093
}
71087094

7109-
export function compilerOptionsAffectSemanticDiagnostics(newOptions: CompilerOptions, oldOptions: CompilerOptions) {
7110-
if (oldOptions === newOptions) {
7111-
return false;
7112-
}
7095+
export function compilerOptionsAffectSemanticDiagnostics(newOptions: CompilerOptions, oldOptions: CompilerOptions): boolean {
7096+
return oldOptions !== newOptions &&
7097+
semanticDiagnosticsOptionDeclarations.some(option => !isJsonEqual(getCompilerOptionValue(oldOptions, option), getCompilerOptionValue(newOptions, option)));
7098+
}
71137099

7114-
return optionDeclarations.some(option => (!!option.strictFlag && getStrictOptionValue(newOptions, option.name as StrictOptionName) !== getStrictOptionValue(oldOptions, option.name as StrictOptionName)) ||
7115-
(!!option.affectsSemanticDiagnostics && !newOptions[option.name] !== !oldOptions[option.name]));
7100+
export function getCompilerOptionValue(options: CompilerOptions, option: CommandLineOption): unknown {
7101+
return option.strictFlag ? getStrictOptionValue(options, option.name as StrictOptionName) : options[option.name];
71167102
}
71177103

71187104
export function hasZeroOrOneAsteriskCharacter(str: string): boolean {
@@ -8380,4 +8366,8 @@ namespace ts {
83808366
// '/// <reference no-default-lib="true"/>' directive.
83818367
return options.skipLibCheck && sourceFile.isDeclarationFile || options.skipDefaultLibCheck && sourceFile.hasNoDefaultLib;
83828368
}
8369+
8370+
export function isJsonEqual(a: unknown, b: unknown): boolean {
8371+
return a === b || typeof a === "object" && a !== null && typeof b === "object" && b !== null && equalOwnProperties(a as MapLike<unknown>, b as MapLike<unknown>, isJsonEqual);
8372+
}
83838373
}

‎src/harness/virtualFileSystemWithWatch.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,7 @@ interface Array<T> {}`
654654
invokeWatcherCallbacks(this.watchedDirectoriesRecursive.get(this.toPath(folderFullPath))!, cb => this.directoryCallback(cb, relativePath));
655655
}
656656

657-
invokeFileWatcher(fileFullPath: string, eventKind: FileWatcherEventKind, useFileNameInCallback?: boolean) {
657+
private invokeFileWatcher(fileFullPath: string, eventKind: FileWatcherEventKind, useFileNameInCallback?: boolean) {
658658
invokeWatcherCallbacks(this.watchedFiles.get(this.toPath(fileFullPath))!, ({ cb, fileName }) => cb(useFileNameInCallback ? fileName : fileFullPath, eventKind));
659659
}
660660

‎src/services/documentRegistry.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,6 @@ namespace ts {
121121
const buckets = createMap<Map<DocumentRegistryEntry>>();
122122
const getCanonicalFileName = createGetCanonicalFileName(!!useCaseSensitiveFileNames);
123123

124-
function getKeyForCompilationSettings(settings: CompilerOptions): DocumentRegistryBucketKey {
125-
return <DocumentRegistryBucketKey>`_${settings.target}|${settings.module}|${settings.noResolve}|${settings.jsx}|${settings.allowJs}|${settings.baseUrl}|${JSON.stringify(settings.typeRoots)}|${JSON.stringify(settings.rootDirs)}|${JSON.stringify(settings.paths)}`;
126-
}
127-
128124
function getBucketForCompilationSettings(key: DocumentRegistryBucketKey, createIfMissing: boolean): Map<DocumentRegistryEntry> {
129125
let bucket = buckets.get(key);
130126
if (!bucket && createIfMissing) {
@@ -273,4 +269,8 @@ namespace ts {
273269
getKeyForCompilationSettings
274270
};
275271
}
272+
273+
function getKeyForCompilationSettings(settings: CompilerOptions): DocumentRegistryBucketKey {
274+
return sourceFileAffectingCompilerOptions.map(option => getCompilerOptionValue(settings, option)).join("|") as DocumentRegistryBucketKey;
275+
}
276276
}

‎src/testRunner/unittests/reuseProgramStructure.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,10 +305,10 @@ namespace ts {
305305
assert.equal(program1.structureIsReused, StructureIsReused.Not);
306306
});
307307

308-
it("fails if rootdir changes", () => {
308+
it("succeeds if rootdir changes", () => {
309309
const program1 = newProgram(files, ["a.ts"], { target, module: ModuleKind.CommonJS, rootDir: "/a/b" });
310310
updateProgram(program1, ["a.ts"], { target, module: ModuleKind.CommonJS, rootDir: "/a/c" }, noop);
311-
assert.equal(program1.structureIsReused, StructureIsReused.Not);
311+
assert.equal(program1.structureIsReused, StructureIsReused.Completely);
312312
});
313313

314314
it("fails if config path changes", () => {

0 commit comments

Comments
 (0)