Skip to content

Commit 6daa27e

Browse files
andrewbranchandrewbranch
committed
Merge branch 'master' into bug/27294
2 parents ac768d0 + 2458c8a commit 6daa27e

17 files changed

+495
-82
lines changed

‎src/compiler/checker.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -15415,7 +15415,9 @@ namespace ts {
1541515415
//
1541615416
// - For a primitive type or type parameter (such as 'number = A & B') there is no point in
1541715417
// breaking the intersection apart.
15418-
result = someTypeRelatedToType(<IntersectionType>source, target, /*reportErrors*/ false, IntersectionState.Source);
15418+
if (!isNonGenericObjectType(target) || !every((<IntersectionType>source).types, t => isNonGenericObjectType(t) && !(getObjectFlags(t) & ObjectFlags.NonInferrableType))) {
15419+
result = someTypeRelatedToType(<IntersectionType>source, target, /*reportErrors*/ false, IntersectionState.Source);
15420+
}
1541915421
}
1542015422
if (!result && (source.flags & TypeFlags.StructuredOrInstantiable || target.flags & TypeFlags.StructuredOrInstantiable)) {
1542115423
if (result = recursiveTypeRelatedTo(source, target, reportErrors, intersectionState)) {

‎src/compiler/moduleSpecifiers.ts

+43-13
Original file line numberDiff line numberDiff line change
@@ -178,40 +178,70 @@ namespace ts.moduleSpecifiers {
178178
);
179179
}
180180

181-
/**
182-
* Looks for existing imports that use symlinks to this module.
183-
* Symlinks will be returned first so they are preferred over the real path.
184-
*/
185-
function getAllModulePaths(files: readonly SourceFile[], importingFileName: string, importedFileName: string, getCanonicalFileName: GetCanonicalFileName, host: ModuleSpecifierResolutionHost, redirectTargetsMap: RedirectTargetsMap): readonly string[] {
181+
export function forEachFileNameOfModule<T>(
182+
files: readonly SourceFile[],
183+
importingFileName: string,
184+
importedFileName: string,
185+
getCanonicalFileName: GetCanonicalFileName,
186+
host: ModuleSpecifierResolutionHost,
187+
redirectTargetsMap: RedirectTargetsMap,
188+
preferSymlinks: boolean,
189+
cb: (fileName: string) => T | undefined
190+
): T | undefined {
186191
const redirects = redirectTargetsMap.get(importedFileName);
187192
const importedFileNames = redirects ? [...redirects, importedFileName] : [importedFileName];
188193
const cwd = host.getCurrentDirectory ? host.getCurrentDirectory() : "";
189194
const targets = importedFileNames.map(f => getNormalizedAbsolutePath(f, cwd));
195+
if (!preferSymlinks) {
196+
const result = forEach(targets, cb);
197+
if (result) return result;
198+
}
190199
const links = host.getProbableSymlinks
191200
? host.getProbableSymlinks(files)
192201
: discoverProbableSymlinks(files, getCanonicalFileName, cwd);
193202

194-
const result: string[] = [];
195203
const compareStrings = (!host.useCaseSensitiveFileNames || host.useCaseSensitiveFileNames()) ? compareStringsCaseSensitive : compareStringsCaseInsensitive;
196-
links.forEach((resolved, path) => {
204+
const result = forEachEntry(links, (resolved, path) => {
197205
if (startsWithDirectory(importingFileName, resolved, getCanonicalFileName)) {
198-
return; // Don't want to a package to globally import from itself
206+
return undefined; // Don't want to a package to globally import from itself
199207
}
200208

201209
const target = find(targets, t => compareStrings(t.slice(0, resolved.length + 1), resolved + "/") === Comparison.EqualTo);
202-
if (target === undefined) return;
210+
if (target === undefined) return undefined;
203211

204212
const relative = getRelativePathFromDirectory(resolved, target, getCanonicalFileName);
205213
const option = resolvePath(path, relative);
206214
if (!host.fileExists || host.fileExists(option)) {
207-
result.push(option);
215+
const result = cb(option);
216+
if (result) return result;
208217
}
209218
});
210-
result.push(...targets);
211-
if (result.length < 2) return result;
219+
return result ||
220+
(preferSymlinks ? forEach(targets, cb) : undefined);
221+
}
222+
223+
/**
224+
* Looks for existing imports that use symlinks to this module.
225+
* Symlinks will be returned first so they are preferred over the real path.
226+
*/
227+
function getAllModulePaths(files: readonly SourceFile[], importingFileName: string, importedFileName: string, getCanonicalFileName: GetCanonicalFileName, host: ModuleSpecifierResolutionHost, redirectTargetsMap: RedirectTargetsMap): readonly string[] {
228+
const cwd = host.getCurrentDirectory ? host.getCurrentDirectory() : "";
229+
const allFileNames = createMap<string>();
230+
forEachFileNameOfModule(
231+
files,
232+
importingFileName,
233+
importedFileName,
234+
getCanonicalFileName,
235+
host,
236+
redirectTargetsMap,
237+
/*preferSymlinks*/ true,
238+
path => {
239+
// dont return value, so we collect everything
240+
allFileNames.set(path, getCanonicalFileName(path));
241+
}
242+
);
212243

213244
// Sort by paths closest to importing file Name directory
214-
const allFileNames = arrayToMap(result, identity, getCanonicalFileName);
215245
const sortedPaths: string[] = [];
216246
for (
217247
let directory = getDirectoryPath(toPath(importingFileName, cwd, getCanonicalFileName));

‎src/harness/fourslashImpl.ts

+39-25
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ namespace FourSlash {
2626

2727
files: FourSlashFile[];
2828

29+
symlinks: vfs.FileSet | undefined;
30+
2931
// A mapping from marker names to name/position pairs
3032
markerPositions: ts.Map<Marker>;
3133

@@ -342,6 +344,10 @@ namespace FourSlash {
342344
});
343345
}
344346

347+
if (testData.symlinks) {
348+
this.languageServiceAdapterHost.vfs.apply(testData.symlinks);
349+
}
350+
345351
this.formatCodeSettings = ts.testFormatSettings;
346352

347353
// Open the first file by default
@@ -3767,6 +3773,7 @@ namespace FourSlash {
37673773
const files: FourSlashFile[] = [];
37683774
// Global options
37693775
const globalOptions: { [s: string]: string; } = {};
3776+
let symlinks: vfs.FileSet | undefined;
37703777
// Marker positions
37713778

37723779
// Split up the input file by line
@@ -3815,32 +3822,38 @@ namespace FourSlash {
38153822
throw new Error("Three-slash line in the middle of four-slash region at line " + i);
38163823
}
38173824
else if (line.substr(0, 2) === "//") {
3818-
// Comment line, check for global/file @options and record them
3819-
const match = optionRegex.exec(line.substr(2));
3820-
if (match) {
3821-
const key = match[1].toLowerCase();
3822-
const value = match[2];
3823-
if (!ts.contains(fileMetadataNames, key)) {
3824-
// Check if the match is already existed in the global options
3825-
if (globalOptions[key] !== undefined) {
3826-
throw new Error(`Global option '${key}' already exists`);
3825+
const possiblySymlinks = Harness.TestCaseParser.parseSymlinkFromTest(line, symlinks);
3826+
if (possiblySymlinks) {
3827+
symlinks = possiblySymlinks;
3828+
}
3829+
else {
3830+
// Comment line, check for global/file @options and record them
3831+
const match = optionRegex.exec(line.substr(2));
3832+
if (match) {
3833+
const key = match[1].toLowerCase();
3834+
const value = match[2];
3835+
if (!ts.contains(fileMetadataNames, key)) {
3836+
// Check if the match is already existed in the global options
3837+
if (globalOptions[key] !== undefined) {
3838+
throw new Error(`Global option '${key}' already exists`);
3839+
}
3840+
globalOptions[key] = value;
38273841
}
3828-
globalOptions[key] = value;
3829-
}
3830-
else {
3831-
switch (key) {
3832-
case MetadataOptionNames.fileName:
3833-
// Found an @FileName directive, if this is not the first then create a new subfile
3834-
nextFile();
3835-
currentFileName = ts.isRootedDiskPath(value) ? value : basePath + "/" + value;
3836-
currentFileOptions[key] = value;
3837-
break;
3838-
case MetadataOptionNames.symlink:
3839-
currentFileSymlinks = ts.append(currentFileSymlinks, value);
3840-
break;
3841-
default:
3842-
// Add other fileMetadata flag
3843-
currentFileOptions[key] = value;
3842+
else {
3843+
switch (key) {
3844+
case MetadataOptionNames.fileName:
3845+
// Found an @FileName directive, if this is not the first then create a new subfile
3846+
nextFile();
3847+
currentFileName = ts.isRootedDiskPath(value) ? value : basePath + "/" + value;
3848+
currentFileOptions[key] = value;
3849+
break;
3850+
case MetadataOptionNames.symlink:
3851+
currentFileSymlinks = ts.append(currentFileSymlinks, value);
3852+
break;
3853+
default:
3854+
// Add other fileMetadata flag
3855+
currentFileOptions[key] = value;
3856+
}
38443857
}
38453858
}
38463859
}
@@ -3870,6 +3883,7 @@ namespace FourSlash {
38703883
markers,
38713884
globalOptions,
38723885
files,
3886+
symlinks,
38733887
ranges
38743888
};
38753889
}

‎src/harness/harnessIO.ts

+13-5
Original file line numberDiff line numberDiff line change
@@ -1128,6 +1128,16 @@ namespace Harness {
11281128
const optionRegex = /^[\/]{2}\s*@(\w+)\s*:\s*([^\r\n]*)/gm; // multiple matches on multiple lines
11291129
const linkRegex = /^[\/]{2}\s*@link\s*:\s*([^\r\n]*)\s*->\s*([^\r\n]*)/gm; // multiple matches on multiple lines
11301130

1131+
export function parseSymlinkFromTest(line: string, symlinks: vfs.FileSet | undefined) {
1132+
const linkMetaData = linkRegex.exec(line);
1133+
linkRegex.lastIndex = 0;
1134+
if (!linkMetaData) return undefined;
1135+
1136+
if (!symlinks) symlinks = {};
1137+
symlinks[linkMetaData[2].trim()] = new vfs.Symlink(linkMetaData[1].trim());
1138+
return symlinks;
1139+
}
1140+
11311141
export function extractCompilerSettings(content: string): CompilerSettings {
11321142
const opts: CompilerSettings = {};
11331143

@@ -1163,11 +1173,9 @@ namespace Harness {
11631173

11641174
for (const line of lines) {
11651175
let testMetaData: RegExpExecArray | null;
1166-
const linkMetaData = linkRegex.exec(line);
1167-
linkRegex.lastIndex = 0;
1168-
if (linkMetaData) {
1169-
if (!symlinks) symlinks = {};
1170-
symlinks[linkMetaData[2].trim()] = new vfs.Symlink(linkMetaData[1].trim());
1176+
const possiblySymlinks = parseSymlinkFromTest(line, symlinks);
1177+
if (possiblySymlinks) {
1178+
symlinks = possiblySymlinks;
11711179
}
11721180
else if (testMetaData = optionRegex.exec(line)) {
11731181
// Comment line, check for global/file @options and record them

‎src/services/codefixes/importFixes.ts

+37-8
Original file line numberDiff line numberDiff line change
@@ -786,9 +786,9 @@ namespace ts.codefix {
786786
cb: (module: Symbol) => void,
787787
) {
788788
let filteredCount = 0;
789-
const packageJson = filterByPackageJson && createAutoImportFilter(from, program, host);
789+
const moduleSpecifierResolutionHost = createModuleSpecifierResolutionHost(program, host);
790+
const packageJson = filterByPackageJson && createAutoImportFilter(from, program, host, moduleSpecifierResolutionHost);
790791
const allSourceFiles = program.getSourceFiles();
791-
const globalTypingsCache = host.getGlobalTypingsCacheLocation && host.getGlobalTypingsCacheLocation();
792792
forEachExternalModule(program.getTypeChecker(), allSourceFiles, (module, sourceFile) => {
793793
if (sourceFile === undefined) {
794794
if (!packageJson || packageJson.allowsImportingAmbientModule(module, allSourceFiles)) {
@@ -800,7 +800,7 @@ namespace ts.codefix {
800800
}
801801
else if (sourceFile &&
802802
sourceFile !== from &&
803-
isImportablePath(from.fileName, sourceFile.fileName, hostGetCanonicalFileName(host), globalTypingsCache)
803+
isImportableFile(program, from, sourceFile, moduleSpecifierResolutionHost)
804804
) {
805805
if (!packageJson || packageJson.allowsImportingSourceFile(sourceFile, allSourceFiles)) {
806806
cb(module);
@@ -826,6 +826,32 @@ namespace ts.codefix {
826826
}
827827
}
828828

829+
function isImportableFile(
830+
program: Program,
831+
from: SourceFile,
832+
to: SourceFile,
833+
moduleSpecifierResolutionHost: ModuleSpecifierResolutionHost
834+
) {
835+
const getCanonicalFileName = hostGetCanonicalFileName(moduleSpecifierResolutionHost);
836+
const globalTypingsCache = moduleSpecifierResolutionHost.getGlobalTypingsCacheLocation?.();
837+
return !!moduleSpecifiers.forEachFileNameOfModule(
838+
program.getSourceFiles(),
839+
from.fileName,
840+
to.fileName,
841+
hostGetCanonicalFileName(moduleSpecifierResolutionHost),
842+
moduleSpecifierResolutionHost,
843+
program.redirectTargetsMap,
844+
/*preferSymlinks*/ false,
845+
toPath => {
846+
const toFile = program.getSourceFile(toPath);
847+
// Determine to import using toPath only if toPath is what we were looking at
848+
// or there doesnt exist the file in the program by the symlink
849+
return (toFile === to || !toFile) &&
850+
isImportablePath(from.fileName, toPath, getCanonicalFileName, globalTypingsCache);
851+
}
852+
);
853+
}
854+
829855
/**
830856
* Don't include something from a `node_modules` that isn't actually reachable by a global import.
831857
* A relative import to node_modules is usually a bad idea.
@@ -870,12 +896,10 @@ namespace ts.codefix {
870896
return !isStringANonContextualKeyword(res) ? res || "_" : `_${res}`;
871897
}
872898

873-
function createAutoImportFilter(fromFile: SourceFile, program: Program, host: LanguageServiceHost) {
874-
const packageJsons = host.getPackageJsonsVisibleToFile && host.getPackageJsonsVisibleToFile(fromFile.fileName) || getPackageJsonsVisibleToFile(fromFile.fileName, host);
875-
const dependencyGroups = PackageJsonDependencyGroup.Dependencies | PackageJsonDependencyGroup.DevDependencies | PackageJsonDependencyGroup.OptionalDependencies;
899+
function createModuleSpecifierResolutionHost(program: Program, host: LanguageServiceHost): ModuleSpecifierResolutionHost {
876900
// Mix in `getProbablySymlinks` from Program when host doesn't have it
877901
// in order for non-Project hosts to have a symlinks cache.
878-
const moduleSpecifierResolutionHost: ModuleSpecifierResolutionHost = {
902+
return {
879903
directoryExists: maybeBind(host, host.directoryExists),
880904
fileExists: maybeBind(host, host.fileExists),
881905
getCurrentDirectory: maybeBind(host, host.getCurrentDirectory),
@@ -884,9 +908,14 @@ namespace ts.codefix {
884908
getProbableSymlinks: maybeBind(host, host.getProbableSymlinks) || program.getProbableSymlinks,
885909
getGlobalTypingsCacheLocation: maybeBind(host, host.getGlobalTypingsCacheLocation),
886910
};
911+
}
912+
913+
function createAutoImportFilter(fromFile: SourceFile, program: Program, host: LanguageServiceHost, moduleSpecifierResolutionHost = createModuleSpecifierResolutionHost(program, host)) {
914+
const packageJsons = host.getPackageJsonsVisibleToFile && host.getPackageJsonsVisibleToFile(fromFile.fileName) || getPackageJsonsVisibleToFile(fromFile.fileName, host);
915+
const dependencyGroups = PackageJsonDependencyGroup.Dependencies | PackageJsonDependencyGroup.DevDependencies | PackageJsonDependencyGroup.OptionalDependencies;
887916

888917
let usesNodeCoreModules: boolean | undefined;
889-
return { allowsImportingAmbientModule, allowsImportingSourceFile, allowsImportingSpecifier };
918+
return { allowsImportingAmbientModule, allowsImportingSourceFile, allowsImportingSpecifier, moduleSpecifierResolutionHost };
890919

891920
function moduleSpecifierIsCoveredByPackageJson(specifier: string) {
892921
const packageName = getNodeModuleRootSpecifier(specifier);

‎tests/baselines/reference/intersectionsAndOptionalProperties.errors.txt

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
tests/cases/compiler/intersectionsAndOptionalProperties.ts(5,1): error TS2322: Type '{ a: null; b: string; }' is not assignable to type '{ a?: number | undefined; b: string; }'.
2+
Types of property 'a' are incompatible.
3+
Type 'null' is not assignable to type 'number | undefined'.
4+
tests/cases/compiler/intersectionsAndOptionalProperties.ts(6,1): error TS2322: Type '{ a: null; } & { b: string; }' is not assignable to type '{ a?: number | undefined; b: string; }'.
5+
Types of property 'a' are incompatible.
6+
Type 'null' is not assignable to type 'number | undefined'.
7+
tests/cases/compiler/intersectionsAndOptionalProperties.ts(19,5): error TS2322: Type 'From' is not assignable to type 'To'.
8+
Types of property 'field' are incompatible.
9+
Type 'null' is not assignable to type 'number | undefined'.
10+
tests/cases/compiler/intersectionsAndOptionalProperties.ts(20,5): error TS2322: Type 'null' is not assignable to type 'number | undefined'.
11+
12+
13+
==== tests/cases/compiler/intersectionsAndOptionalProperties.ts (4 errors) ====
14+
declare let x: { a?: number, b: string };
15+
declare let y: { a: null, b: string };
16+
declare let z: { a: null } & { b: string };
17+
18+
x = y; // Error
19+
~
20+
!!! error TS2322: Type '{ a: null; b: string; }' is not assignable to type '{ a?: number | undefined; b: string; }'.
21+
!!! error TS2322: Types of property 'a' are incompatible.
22+
!!! error TS2322: Type 'null' is not assignable to type 'number | undefined'.
23+
x = z; // Error
24+
~
25+
!!! error TS2322: Type '{ a: null; } & { b: string; }' is not assignable to type '{ a?: number | undefined; b: string; }'.
26+
!!! error TS2322: Types of property 'a' are incompatible.
27+
!!! error TS2322: Type 'null' is not assignable to type 'number | undefined'.
28+
29+
// Repro from #36604
30+
31+
interface To {
32+
field?: number;
33+
anotherField: string;
34+
}
35+
36+
type From = { field: null } & Omit<To, 'field'>;
37+
38+
function foo(v: From) {
39+
let x: To;
40+
x = v; // Error
41+
~
42+
!!! error TS2322: Type 'From' is not assignable to type 'To'.
43+
!!! error TS2322: Types of property 'field' are incompatible.
44+
!!! error TS2322: Type 'null' is not assignable to type 'number | undefined'.
45+
x.field = v.field; // Error
46+
~~~~~~~
47+
!!! error TS2322: Type 'null' is not assignable to type 'number | undefined'.
48+
}
49+

‎tests/baselines/reference/intersectionsAndOptionalProperties.js

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//// [intersectionsAndOptionalProperties.ts]
2+
declare let x: { a?: number, b: string };
3+
declare let y: { a: null, b: string };
4+
declare let z: { a: null } & { b: string };
5+
6+
x = y; // Error
7+
x = z; // Error
8+
9+
// Repro from #36604
10+
11+
interface To {
12+
field?: number;
13+
anotherField: string;
14+
}
15+
16+
type From = { field: null } & Omit<To, 'field'>;
17+
18+
function foo(v: From) {
19+
let x: To;
20+
x = v; // Error
21+
x.field = v.field; // Error
22+
}
23+
24+
25+
//// [intersectionsAndOptionalProperties.js]
26+
"use strict";
27+
x = y; // Error
28+
x = z; // Error
29+
function foo(v) {
30+
var x;
31+
x = v; // Error
32+
x.field = v.field; // Error
33+
}

0 commit comments

Comments
 (0)