From 228ce06461fad0fb260f4dd3ca0c1f8a5abecfba Mon Sep 17 00:00:00 2001 From: Charles Pierce Date: Wed, 5 Jul 2017 10:03:56 -0700 Subject: [PATCH 01/13] #15214 Remove nonpublic members from destructuring completion lists --- src/services/completions.ts | 2 +- .../fourslash/completionListInObjectBindingPattern14.ts | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/completionListInObjectBindingPattern14.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index 410ba636d7b..f11130efa86 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1002,7 +1002,7 @@ namespace ts.Completions { const typeForObject = typeChecker.getTypeAtLocation(objectLikeContainer); if (!typeForObject) return false; // In a binding pattern, get only known properties. Everywhere else we will get all possible properties. - typeMembers = typeChecker.getPropertiesOfType(typeForObject); + typeMembers = typeChecker.getPropertiesOfType(typeForObject).filter((symbol) => !(getDeclarationModifierFlagsFromSymbol(symbol) & ModifierFlags.NonPublicAccessibilityModifier)); existingMembers = (objectLikeContainer).elements; } } diff --git a/tests/cases/fourslash/completionListInObjectBindingPattern14.ts b/tests/cases/fourslash/completionListInObjectBindingPattern14.ts new file mode 100644 index 00000000000..425813a5543 --- /dev/null +++ b/tests/cases/fourslash/completionListInObjectBindingPattern14.ts @@ -0,0 +1,9 @@ +/// + +////const { b/**/ } = new class { +//// private ab; +//// protected bc; +////} + +goTo.marker(); +verify.completionListIsEmpty(); From b747c2dd96ede26c853dee1d8882d73bb6d8ddf8 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Thu, 3 Aug 2017 18:30:09 -0700 Subject: [PATCH 02/13] exclude node_modules unless explicitly included --- src/compiler/commandLineParser.ts | 6 +- src/compiler/core.ts | 101 ++++++--- src/harness/unittests/matchFiles.ts | 193 ++++++++++++------ .../nodeModulesMaxDepthExceeded.errors.txt | 2 +- .../nodeModulesMaxDepthExceeded.errors.txt | 2 +- .../maxDepthExceeded/tsconfig.json | 2 +- 6 files changed, 200 insertions(+), 106 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 554b46dcdfe..b0cb7f94029 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1446,14 +1446,10 @@ namespace ts { } } else { - // If no includes were specified, exclude common package folders and the outDir - const specs = includeSpecs ? [] : ["node_modules", "bower_components", "jspm_packages"]; - const outDir = raw["compilerOptions"] && raw["compilerOptions"]["outDir"]; if (outDir) { - specs.push(outDir); + excludeSpecs = [outDir]; } - excludeSpecs = specs; } if (fileNames === undefined && includeSpecs === undefined) { diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 728fb433c04..07f979333b6 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -1889,14 +1889,54 @@ namespace ts { const reservedCharacterPattern = /[^\w\s\/]/g; const wildcardCharCodes = [CharacterCodes.asterisk, CharacterCodes.question]; - /** - * Matches any single directory segment unless it is the last segment and a .min.js file - * Breakdown: - * [^./] # matches everything up to the first . character (excluding directory seperators) - * (\\.(?!min\\.js$))? # matches . characters but not if they are part of the .min.js file extension - */ - const singleAsteriskRegexFragmentFiles = "([^./]|(\\.(?!min\\.js$))?)*"; - const singleAsteriskRegexFragmentOther = "[^/]*"; + /* @internal */ + export const commonPackageFolders: ReadonlyArray = ["node_modules", "bower_components", "jspm_packages"]; + + const implicitExcludePathRegexPattern = `(?!(${commonPackageFolders.join("|")})(/|$))`; + + interface WildcardMatcher { + singleAsteriskRegexFragment: string; + doubleAsteriskRegexFragment: string; + replaceWildcardCharacter: (match: string) => string; + } + + const filesMatcher: WildcardMatcher = { + /** + * Matches any single directory segment unless it is the last segment and a .min.js file + * Breakdown: + * [^./] # matches everything up to the first . character (excluding directory seperators) + * (\\.(?!min\\.js$))? # matches . characters but not if they are part of the .min.js file extension + */ + singleAsteriskRegexFragment: "([^./]|(\\.(?!min\\.js$))?)*", + /** + * Regex for the ** wildcard. Matches any number of subdirectories. When used for including + * files or directories, does not match subdirectories that start with a . character + */ + doubleAsteriskRegexFragment: `(/${implicitExcludePathRegexPattern}[^/.][^/]*)*?`, + replaceWildcardCharacter: match => replaceWildcardCharacter(match, filesMatcher.singleAsteriskRegexFragment) + }; + + const directoriesMatcher: WildcardMatcher = { + singleAsteriskRegexFragment: "[^/]*", + /** + * Regex for the ** wildcard. Matches any number of subdirectories. When used for including + * files or directories, does not match subdirectories that start with a . character + */ + doubleAsteriskRegexFragment: `(/${implicitExcludePathRegexPattern}[^/.][^/]*)*?`, + replaceWildcardCharacter: match => replaceWildcardCharacter(match, directoriesMatcher.singleAsteriskRegexFragment) + }; + + const excludeMatcher: WildcardMatcher = { + singleAsteriskRegexFragment: "[^/]*", + doubleAsteriskRegexFragment: "(/.+?)?", + replaceWildcardCharacter: match => replaceWildcardCharacter(match, excludeMatcher.singleAsteriskRegexFragment) + }; + + const wildcardMatchers = { + files: filesMatcher, + directories: directoriesMatcher, + exclude: excludeMatcher + }; export function getRegularExpressionForWildcard(specs: ReadonlyArray, basePath: string, usage: "files" | "directories" | "exclude"): string | undefined { const patterns = getRegularExpressionsForWildcards(specs, basePath, usage); @@ -1915,17 +1955,8 @@ namespace ts { return undefined; } - const replaceWildcardCharacter = usage === "files" ? replaceWildCardCharacterFiles : replaceWildCardCharacterOther; - const singleAsteriskRegexFragment = usage === "files" ? singleAsteriskRegexFragmentFiles : singleAsteriskRegexFragmentOther; - - /** - * Regex for the ** wildcard. Matches any number of subdirectories. When used for including - * files or directories, does not match subdirectories that start with a . character - */ - const doubleAsteriskRegexFragment = usage === "exclude" ? "(/.+?)?" : "(/[^/.][^/]*)*?"; - return flatMap(specs, spec => - spec && getSubPatternFromSpec(spec, basePath, usage, singleAsteriskRegexFragment, doubleAsteriskRegexFragment, replaceWildcardCharacter)); + spec && getSubPatternFromSpec(spec, basePath, usage, wildcardMatchers[usage])); } /** @@ -1936,7 +1967,7 @@ namespace ts { return !/[.*?]/.test(lastPathComponent); } - function getSubPatternFromSpec(spec: string, basePath: string, usage: "files" | "directories" | "exclude", singleAsteriskRegexFragment: string, doubleAsteriskRegexFragment: string, replaceWildcardCharacter: (match: string) => string): string | undefined { + function getSubPatternFromSpec(spec: string, basePath: string, usage: "files" | "directories" | "exclude", { singleAsteriskRegexFragment, doubleAsteriskRegexFragment, replaceWildcardCharacter }: WildcardMatcher): string | undefined { let subpattern = ""; let hasRecursiveDirectoryWildcard = false; let hasWrittenComponent = false; @@ -1975,20 +2006,36 @@ namespace ts { } if (usage !== "exclude") { + let componentPattern = ""; // The * and ? wildcards should not match directories or files that start with . if they // appear first in a component. Dotted directories and files can be included explicitly // like so: **/.*/.* if (component.charCodeAt(0) === CharacterCodes.asterisk) { - subpattern += "([^./]" + singleAsteriskRegexFragment + ")?"; + componentPattern += "([^./]" + singleAsteriskRegexFragment + ")?"; component = component.substr(1); } else if (component.charCodeAt(0) === CharacterCodes.question) { - subpattern += "[^./]"; + componentPattern += "[^./]"; component = component.substr(1); } - } - subpattern += component.replace(reservedCharacterPattern, replaceWildcardCharacter); + componentPattern += component.replace(reservedCharacterPattern, replaceWildcardCharacter); + + // Patterns should not include subfolders like node_modules unless they are + // explicitly included as part of the path. + // + // As an optimization, if the component pattern is the same as the component, + // then there definitely were no wildcard characters and we do not need to + // add the exclusion pattern. + if (componentPattern !== component) { + subpattern += implicitExcludePathRegexPattern; + } + + subpattern += componentPattern; + } + else { + subpattern += component.replace(reservedCharacterPattern, replaceWildcardCharacter); + } } hasWrittenComponent = true; @@ -2002,14 +2049,6 @@ namespace ts { return subpattern; } - function replaceWildCardCharacterFiles(match: string) { - return replaceWildcardCharacter(match, singleAsteriskRegexFragmentFiles); - } - - function replaceWildCardCharacterOther(match: string) { - return replaceWildcardCharacter(match, singleAsteriskRegexFragmentOther); - } - function replaceWildcardCharacter(match: string, singleAsteriskRegexFragment: string) { return match === "*" ? singleAsteriskRegexFragment : match === "?" ? "[^/]" : "\\" + match; } diff --git a/src/harness/unittests/matchFiles.ts b/src/harness/unittests/matchFiles.ts index e0454671930..71b1bfff11a 100644 --- a/src/harness/unittests/matchFiles.ts +++ b/src/harness/unittests/matchFiles.ts @@ -73,6 +73,7 @@ namespace ts { "c:/dev/a.d.ts", "c:/dev/a.js", "c:/dev/b.ts", + "c:/dev/x/a.ts", "c:/dev/node_modules/a.ts", "c:/dev/bower_components/a.ts", "c:/dev/jspm_packages/a.ts" @@ -141,7 +142,8 @@ namespace ts { errors: [], fileNames: [ "c:/dev/a.ts", - "c:/dev/b.ts" + "c:/dev/b.ts", + "c:/dev/x/a.ts" ], wildcardDirectories: { "c:/dev": ts.WatchDirectoryFlags.Recursive @@ -462,7 +464,6 @@ namespace ts { }; validateMatches(expected, json, caseInsensitiveHost, caseInsensitiveBasePath); }); - it("same named declarations are excluded", () => { const json = { include: [ @@ -651,71 +652,127 @@ namespace ts { }; validateMatches(expected, json, caseInsensitiveHost, caseInsensitiveBasePath); }); - it("with common package folders and no exclusions", () => { - const json = { - include: [ - "**/a.ts" - ] - }; - const expected: ts.ParsedCommandLine = { - options: {}, - errors: [], - fileNames: [ - "c:/dev/a.ts", - "c:/dev/bower_components/a.ts", - "c:/dev/jspm_packages/a.ts", - "c:/dev/node_modules/a.ts" - ], - wildcardDirectories: { - "c:/dev": ts.WatchDirectoryFlags.Recursive - }, - }; - validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); - }); - it("with common package folders and exclusions", () => { - const json = { - include: [ - "**/a.ts" - ], - exclude: [ - "a.ts" - ] - }; - const expected: ts.ParsedCommandLine = { - options: {}, - errors: [], - fileNames: [ - "c:/dev/bower_components/a.ts", - "c:/dev/jspm_packages/a.ts", - "c:/dev/node_modules/a.ts" - ], - wildcardDirectories: { - "c:/dev": ts.WatchDirectoryFlags.Recursive - }, - }; - validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); - }); - it("with common package folders and empty exclude", () => { - const json = { - include: [ - "**/a.ts" - ], - exclude: [] - }; - const expected: ts.ParsedCommandLine = { - options: {}, - errors: [], - fileNames: [ - "c:/dev/a.ts", - "c:/dev/bower_components/a.ts", - "c:/dev/jspm_packages/a.ts", - "c:/dev/node_modules/a.ts" - ], - wildcardDirectories: { - "c:/dev": ts.WatchDirectoryFlags.Recursive - }, - }; - validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); + describe("with common package folders", () => { + it("and no exclusions", () => { + const json = { + include: [ + "**/a.ts" + ] + }; + const expected: ts.ParsedCommandLine = { + options: {}, + errors: [], + fileNames: [ + "c:/dev/a.ts", + "c:/dev/x/a.ts" + ], + wildcardDirectories: { + "c:/dev": ts.WatchDirectoryFlags.Recursive + }, + }; + validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); + }); + it("and exclusions", () => { + const json = { + include: [ + "**/?.ts" + ], + exclude: [ + "a.ts" + ] + }; + const expected: ts.ParsedCommandLine = { + options: {}, + errors: [], + fileNames: [ + "c:/dev/b.ts", + "c:/dev/x/a.ts" + ], + wildcardDirectories: { + "c:/dev": ts.WatchDirectoryFlags.Recursive + }, + }; + validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); + }); + it("and empty exclude", () => { + const json = { + include: [ + "**/a.ts" + ], + exclude: [] + }; + const expected: ts.ParsedCommandLine = { + options: {}, + errors: [], + fileNames: [ + "c:/dev/a.ts", + "c:/dev/x/a.ts" + ], + wildcardDirectories: { + "c:/dev": ts.WatchDirectoryFlags.Recursive + }, + }; + validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); + }); + it("and explicit recursive include", () => { + const json = { + include: [ + "**/a.ts", + "**/node_modules/a.ts" + ] + }; + const expected: ts.ParsedCommandLine = { + options: {}, + errors: [], + fileNames: [ + "c:/dev/a.ts", + "c:/dev/x/a.ts", + "c:/dev/node_modules/a.ts" + ], + wildcardDirectories: { + "c:/dev": ts.WatchDirectoryFlags.Recursive + }, + }; + validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); + }); + it("and wildcard include", () => { + const json = { + include: [ + "*/a.ts" + ] + }; + const expected: ts.ParsedCommandLine = { + options: {}, + errors: [], + fileNames: [ + "c:/dev/x/a.ts" + ], + wildcardDirectories: { + "c:/dev": ts.WatchDirectoryFlags.Recursive + }, + }; + validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); + }); + it("and explicit wildcard include", () => { + const json = { + include: [ + "*/a.ts", + "node_modules/a.ts" + ] + }; + const expected: ts.ParsedCommandLine = { + options: {}, + errors: [], + fileNames: [ + "c:/dev/x/a.ts", + "c:/dev/node_modules/a.ts" + ], + wildcardDirectories: { + "c:/dev": ts.WatchDirectoryFlags.Recursive + }, + }; + validateMatches(expected, json, caseInsensitiveCommonFoldersHost, caseInsensitiveBasePath); + }); }); it("exclude .js files when allowJs=false", () => { const json = { @@ -1066,6 +1123,7 @@ namespace ts { }; validateMatches(expected, json, caseInsensitiveHost, caseInsensitiveBasePath); }); + describe("with trailing recursive directory", () => { it("in includes", () => { const json = { @@ -1264,6 +1322,7 @@ namespace ts { }); }); }); + describe("with files or folders that begin with a .", () => { it("that are not explicitly included", () => { const json = { diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt index a1b170ce665..bd2dd6231f3 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/amd/nodeModulesMaxDepthExceeded.errors.txt @@ -9,7 +9,7 @@ maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it i "maxNodeModuleJsDepth": 1, // Note: Module m1 is already included as a root file "outDir": "built" }, - "include": ["**/*"], + "include": ["**/*", "node_modules/**/*"], "exclude": ["node_modules/m2/**/*"] } diff --git a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt index a1b170ce665..bd2dd6231f3 100644 --- a/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt +++ b/tests/baselines/reference/project/nodeModulesMaxDepthExceeded/node/nodeModulesMaxDepthExceeded.errors.txt @@ -9,7 +9,7 @@ maxDepthExceeded/root.ts(4,4): error TS2540: Cannot assign to 'rel' because it i "maxNodeModuleJsDepth": 1, // Note: Module m1 is already included as a root file "outDir": "built" }, - "include": ["**/*"], + "include": ["**/*", "node_modules/**/*"], "exclude": ["node_modules/m2/**/*"] } diff --git a/tests/cases/projects/NodeModulesSearch/maxDepthExceeded/tsconfig.json b/tests/cases/projects/NodeModulesSearch/maxDepthExceeded/tsconfig.json index 52633bb5a98..b2ee28482ba 100644 --- a/tests/cases/projects/NodeModulesSearch/maxDepthExceeded/tsconfig.json +++ b/tests/cases/projects/NodeModulesSearch/maxDepthExceeded/tsconfig.json @@ -4,6 +4,6 @@ "maxNodeModuleJsDepth": 1, // Note: Module m1 is already included as a root file "outDir": "built" }, - "include": ["**/*"], + "include": ["**/*", "node_modules/**/*"], "exclude": ["node_modules/m2/**/*"] } From 44a6c6cc6ff0f3c9bfedb9e0a69d68232c10fdcf Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 5 Aug 2017 10:09:44 -0700 Subject: [PATCH 03/13] { [P in K]: T } is related to { [x: string]: U } if T is related to U --- src/compiler/checker.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7913520bfdb..2cbaf0e1992 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -9614,6 +9614,11 @@ namespace ts { if (sourceInfo) { return indexInfoRelatedTo(sourceInfo, targetInfo, reportErrors); } + if (isGenericMappedType(source)) { + // A generic mapped type { [P in K]: T } is related to an index signature { [x: string]: U } + // if T is related to U. + return kind === IndexKind.String && isRelatedTo(getTemplateTypeFromMappedType(source), targetInfo.type, reportErrors); + } if (isObjectLiteralType(source)) { let related = Ternary.True; if (kind === IndexKind.String) { From c938a2acdc4f7cfe19b628b074faa9b4e07ae736 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 5 Aug 2017 10:17:20 -0700 Subject: [PATCH 04/13] Add tests --- .../indexSignatureAndMappedType.errors.txt | 47 ++++++++++++ .../reference/indexSignatureAndMappedType.js | 73 +++++++++++++++++++ .../compiler/indexSignatureAndMappedType.ts | 35 +++++++++ 3 files changed, 155 insertions(+) create mode 100644 tests/baselines/reference/indexSignatureAndMappedType.errors.txt create mode 100644 tests/baselines/reference/indexSignatureAndMappedType.js create mode 100644 tests/cases/compiler/indexSignatureAndMappedType.ts diff --git a/tests/baselines/reference/indexSignatureAndMappedType.errors.txt b/tests/baselines/reference/indexSignatureAndMappedType.errors.txt new file mode 100644 index 00000000000..c8ae0494015 --- /dev/null +++ b/tests/baselines/reference/indexSignatureAndMappedType.errors.txt @@ -0,0 +1,47 @@ +tests/cases/compiler/indexSignatureAndMappedType.ts(6,5): error TS2322: Type '{ [key: string]: T; }' is not assignable to type 'Record'. +tests/cases/compiler/indexSignatureAndMappedType.ts(15,5): error TS2322: Type 'Record' is not assignable to type '{ [key: string]: T; }'. + Type 'U' is not assignable to type 'T'. +tests/cases/compiler/indexSignatureAndMappedType.ts(16,5): error TS2322: Type '{ [key: string]: T; }' is not assignable to type 'Record'. + + +==== tests/cases/compiler/indexSignatureAndMappedType.ts (3 errors) ==== + // A mapped type { [P in K]: X }, where K is a generic type, is related to + // { [key: string]: Y } if X is related to Y. + + function f1(x: { [key: string]: T }, y: Record) { + x = y; + y = x; // Error + ~ +!!! error TS2322: Type '{ [key: string]: T; }' is not assignable to type 'Record'. + } + + function f2(x: { [key: string]: T }, y: Record) { + x = y; + y = x; + } + + function f3(x: { [key: string]: T }, y: Record) { + x = y; // Error + ~ +!!! error TS2322: Type 'Record' is not assignable to type '{ [key: string]: T; }'. +!!! error TS2322: Type 'U' is not assignable to type 'T'. + y = x; // Error + ~ +!!! error TS2322: Type '{ [key: string]: T; }' is not assignable to type 'Record'. + } + + // Repro from #14548 + + type Dictionary = { + [key: string]: string; + }; + + interface IBaseEntity { + name: string; + properties: Dictionary; + } + + interface IEntity extends IBaseEntity { + properties: Record; + } + \ No newline at end of file diff --git a/tests/baselines/reference/indexSignatureAndMappedType.js b/tests/baselines/reference/indexSignatureAndMappedType.js new file mode 100644 index 00000000000..c35da4f4931 --- /dev/null +++ b/tests/baselines/reference/indexSignatureAndMappedType.js @@ -0,0 +1,73 @@ +//// [indexSignatureAndMappedType.ts] +// A mapped type { [P in K]: X }, where K is a generic type, is related to +// { [key: string]: Y } if X is related to Y. + +function f1(x: { [key: string]: T }, y: Record) { + x = y; + y = x; // Error +} + +function f2(x: { [key: string]: T }, y: Record) { + x = y; + y = x; +} + +function f3(x: { [key: string]: T }, y: Record) { + x = y; // Error + y = x; // Error +} + +// Repro from #14548 + +type Dictionary = { + [key: string]: string; +}; + +interface IBaseEntity { + name: string; + properties: Dictionary; +} + +interface IEntity extends IBaseEntity { + properties: Record; +} + + +//// [indexSignatureAndMappedType.js] +"use strict"; +// A mapped type { [P in K]: X }, where K is a generic type, is related to +// { [key: string]: Y } if X is related to Y. +function f1(x, y) { + x = y; + y = x; // Error +} +function f2(x, y) { + x = y; + y = x; +} +function f3(x, y) { + x = y; // Error + y = x; // Error +} + + +//// [indexSignatureAndMappedType.d.ts] +declare function f1(x: { + [key: string]: T; +}, y: Record): void; +declare function f2(x: { + [key: string]: T; +}, y: Record): void; +declare function f3(x: { + [key: string]: T; +}, y: Record): void; +declare type Dictionary = { + [key: string]: string; +}; +interface IBaseEntity { + name: string; + properties: Dictionary; +} +interface IEntity extends IBaseEntity { + properties: Record; +} diff --git a/tests/cases/compiler/indexSignatureAndMappedType.ts b/tests/cases/compiler/indexSignatureAndMappedType.ts new file mode 100644 index 00000000000..1070472a241 --- /dev/null +++ b/tests/cases/compiler/indexSignatureAndMappedType.ts @@ -0,0 +1,35 @@ +// @strict: true +// @declaration: true + +// A mapped type { [P in K]: X }, where K is a generic type, is related to +// { [key: string]: Y } if X is related to Y. + +function f1(x: { [key: string]: T }, y: Record) { + x = y; + y = x; // Error +} + +function f2(x: { [key: string]: T }, y: Record) { + x = y; + y = x; +} + +function f3(x: { [key: string]: T }, y: Record) { + x = y; // Error + y = x; // Error +} + +// Repro from #14548 + +type Dictionary = { + [key: string]: string; +}; + +interface IBaseEntity { + name: string; + properties: Dictionary; +} + +interface IEntity extends IBaseEntity { + properties: Record; +} From d0a195a3c5c6718f393071cc9a827905ac7a2f4c Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 5 Aug 2017 12:32:56 -0700 Subject: [PATCH 05/13] Propagate type comparer function in contextual signature instantiation --- src/compiler/checker.ts | 15 ++++++++------- src/compiler/core.ts | 14 -------------- src/compiler/types.ts | 18 ++++++++++++++++++ 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 1c0f15e27aa..f31c2573e36 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -8017,7 +8017,7 @@ namespace ts { function cloneTypeMapper(mapper: TypeMapper): TypeMapper { return mapper && isInferenceContext(mapper) ? - createInferenceContext(mapper.signature, mapper.flags | InferenceFlags.NoDefault, mapper.inferences) : + createInferenceContext(mapper.signature, mapper.flags | InferenceFlags.NoDefault, mapper.compareTypes, mapper.inferences) : mapper; } @@ -8458,7 +8458,7 @@ namespace ts { ignoreReturnTypes: boolean, reportErrors: boolean, errorReporter: ErrorReporter, - compareTypes: (s: Type, t: Type, reportErrors?: boolean) => Ternary): Ternary { + compareTypes: TypeComparer): Ternary { // TODO (drosen): De-duplicate code between related functions. if (source === target) { return Ternary.True; @@ -8468,7 +8468,7 @@ namespace ts { } if (source.typeParameters) { - source = instantiateSignatureInContextOf(source, target); + source = instantiateSignatureInContextOf(source, target, /*contextualMapper*/ undefined, compareTypes); } let result = Ternary.True; @@ -10216,13 +10216,14 @@ namespace ts { } } - function createInferenceContext(signature: Signature, flags: InferenceFlags, baseInferences?: InferenceInfo[]): InferenceContext { + function createInferenceContext(signature: Signature, flags: InferenceFlags, compareTypes?: TypeComparer, baseInferences?: InferenceInfo[]): InferenceContext { const inferences = baseInferences ? map(baseInferences, cloneInferenceInfo) : map(signature.typeParameters, createInferenceInfo); const context = mapper as InferenceContext; context.mappedTypes = signature.typeParameters; context.signature = signature; context.inferences = inferences; context.flags = flags; + context.compareTypes = compareTypes || compareTypesAssignable; return context; function mapper(t: Type): Type { @@ -10670,7 +10671,7 @@ namespace ts { const constraint = getConstraintOfTypeParameter(context.signature.typeParameters[index]); if (constraint) { const instantiatedConstraint = instantiateType(constraint, context); - if (!isTypeAssignableTo(inferredType, getTypeWithThisArgument(instantiatedConstraint, inferredType))) { + if (!context.compareTypes(inferredType, getTypeWithThisArgument(instantiatedConstraint, inferredType))) { inference.inferredType = inferredType = instantiatedConstraint; } } @@ -15071,8 +15072,8 @@ namespace ts { } // Instantiate a generic signature in the context of a non-generic signature (section 3.8.5 in TypeScript spec) - function instantiateSignatureInContextOf(signature: Signature, contextualSignature: Signature, contextualMapper?: TypeMapper): Signature { - const context = createInferenceContext(signature, InferenceFlags.InferUnionTypes); + function instantiateSignatureInContextOf(signature: Signature, contextualSignature: Signature, contextualMapper?: TypeMapper, compareTypes?: TypeComparer): Signature { + const context = createInferenceContext(signature, InferenceFlags.InferUnionTypes, compareTypes); forEachMatchingParameterType(contextualSignature, signature, (source, target) => { // Type parameters from outer context referenced by source type are fixed by instantiation of the source type inferTypes(context.inferences, instantiateType(source, contextualMapper || identityMapper), target); diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 728fb433c04..262564813e9 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -11,20 +11,6 @@ namespace ts { /* @internal */ namespace ts { - /** - * Ternary values are defined such that - * x & y is False if either x or y is False. - * x & y is Maybe if either x or y is Maybe, but neither x or y is False. - * x & y is True if both x and y are True. - * x | y is False if both x and y are False. - * x | y is Maybe if either x or y is Maybe, but neither x or y is True. - * x | y is True if either x or y is True. - */ - export const enum Ternary { - False = 0, - Maybe = 1, - True = -1 - } // More efficient to create a collator once and use its `compare` than to call `a.localeCompare(b)` many times. export const collator: { compare(a: string, b: string): number } = typeof Intl === "object" && typeof Intl.Collator === "function" ? new Intl.Collator(/*locales*/ undefined, { usage: "sort", sensitivity: "accent" }) : undefined; diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 09f1b5cfcc5..3b57608abc5 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3425,11 +3425,29 @@ namespace ts { AnyDefault = 1 << 2, // Infer anyType for no inferences (otherwise emptyObjectType) } + /** + * Ternary values are defined such that + * x & y is False if either x or y is False. + * x & y is Maybe if either x or y is Maybe, but neither x or y is False. + * x & y is True if both x and y are True. + * x | y is False if both x and y are False. + * x | y is Maybe if either x or y is Maybe, but neither x or y is True. + * x | y is True if either x or y is True. + */ + export const enum Ternary { + False = 0, + Maybe = 1, + True = -1 + } + + export type TypeComparer = (s: Type, t: Type, reportErrors?: boolean) => Ternary; + /* @internal */ export interface InferenceContext extends TypeMapper { signature: Signature; // Generic signature for which inferences are made inferences: InferenceInfo[]; // Inferences made for each type parameter flags: InferenceFlags; // Inference flags + compareTypes: TypeComparer; // Type comparer function } /* @internal */ From a4a37ea086abdad84c0a7aa882832c288ea7d59b Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Sat, 5 Aug 2017 12:40:40 -0700 Subject: [PATCH 06/13] Add regression test --- ...reInstantiationWithRecursiveConstraints.js | 30 ++++++++++++++++++ ...tantiationWithRecursiveConstraints.symbols | 30 ++++++++++++++++++ ...nstantiationWithRecursiveConstraints.types | 31 +++++++++++++++++++ ...reInstantiationWithRecursiveConstraints.ts | 13 ++++++++ 4 files changed, 104 insertions(+) create mode 100644 tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.js create mode 100644 tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.symbols create mode 100644 tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.types create mode 100644 tests/cases/compiler/signatureInstantiationWithRecursiveConstraints.ts diff --git a/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.js b/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.js new file mode 100644 index 00000000000..0be7fa1444d --- /dev/null +++ b/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.js @@ -0,0 +1,30 @@ +//// [signatureInstantiationWithRecursiveConstraints.ts] +// Repro from #17148 + +class Foo { + myFunc(arg: T) {} +} + +class Bar { + myFunc(arg: T) {} +} + +const myVar: Foo = new Bar(); + + +//// [signatureInstantiationWithRecursiveConstraints.js] +"use strict"; +// Repro from #17148 +var Foo = (function () { + function Foo() { + } + Foo.prototype.myFunc = function (arg) { }; + return Foo; +}()); +var Bar = (function () { + function Bar() { + } + Bar.prototype.myFunc = function (arg) { }; + return Bar; +}()); +var myVar = new Bar(); diff --git a/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.symbols b/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.symbols new file mode 100644 index 00000000000..ebc1b625d9e --- /dev/null +++ b/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.symbols @@ -0,0 +1,30 @@ +=== tests/cases/compiler/signatureInstantiationWithRecursiveConstraints.ts === +// Repro from #17148 + +class Foo { +>Foo : Symbol(Foo, Decl(signatureInstantiationWithRecursiveConstraints.ts, 0, 0)) + + myFunc(arg: T) {} +>myFunc : Symbol(Foo.myFunc, Decl(signatureInstantiationWithRecursiveConstraints.ts, 2, 11)) +>T : Symbol(T, Decl(signatureInstantiationWithRecursiveConstraints.ts, 3, 9)) +>Foo : Symbol(Foo, Decl(signatureInstantiationWithRecursiveConstraints.ts, 0, 0)) +>arg : Symbol(arg, Decl(signatureInstantiationWithRecursiveConstraints.ts, 3, 24)) +>T : Symbol(T, Decl(signatureInstantiationWithRecursiveConstraints.ts, 3, 9)) +} + +class Bar { +>Bar : Symbol(Bar, Decl(signatureInstantiationWithRecursiveConstraints.ts, 4, 1)) + + myFunc(arg: T) {} +>myFunc : Symbol(Bar.myFunc, Decl(signatureInstantiationWithRecursiveConstraints.ts, 6, 11)) +>T : Symbol(T, Decl(signatureInstantiationWithRecursiveConstraints.ts, 7, 9)) +>Bar : Symbol(Bar, Decl(signatureInstantiationWithRecursiveConstraints.ts, 4, 1)) +>arg : Symbol(arg, Decl(signatureInstantiationWithRecursiveConstraints.ts, 7, 24)) +>T : Symbol(T, Decl(signatureInstantiationWithRecursiveConstraints.ts, 7, 9)) +} + +const myVar: Foo = new Bar(); +>myVar : Symbol(myVar, Decl(signatureInstantiationWithRecursiveConstraints.ts, 10, 5)) +>Foo : Symbol(Foo, Decl(signatureInstantiationWithRecursiveConstraints.ts, 0, 0)) +>Bar : Symbol(Bar, Decl(signatureInstantiationWithRecursiveConstraints.ts, 4, 1)) + diff --git a/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.types b/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.types new file mode 100644 index 00000000000..2368835be08 --- /dev/null +++ b/tests/baselines/reference/signatureInstantiationWithRecursiveConstraints.types @@ -0,0 +1,31 @@ +=== tests/cases/compiler/signatureInstantiationWithRecursiveConstraints.ts === +// Repro from #17148 + +class Foo { +>Foo : Foo + + myFunc(arg: T) {} +>myFunc : (arg: T) => void +>T : T +>Foo : Foo +>arg : T +>T : T +} + +class Bar { +>Bar : Bar + + myFunc(arg: T) {} +>myFunc : (arg: T) => void +>T : T +>Bar : Bar +>arg : T +>T : T +} + +const myVar: Foo = new Bar(); +>myVar : Foo +>Foo : Foo +>new Bar() : Bar +>Bar : typeof Bar + diff --git a/tests/cases/compiler/signatureInstantiationWithRecursiveConstraints.ts b/tests/cases/compiler/signatureInstantiationWithRecursiveConstraints.ts new file mode 100644 index 00000000000..4f25446aad8 --- /dev/null +++ b/tests/cases/compiler/signatureInstantiationWithRecursiveConstraints.ts @@ -0,0 +1,13 @@ +// @strict: true + +// Repro from #17148 + +class Foo { + myFunc(arg: T) {} +} + +class Bar { + myFunc(arg: T) {} +} + +const myVar: Foo = new Bar(); From 3efeb1e27f60a95dad66c148295bfa5a65f56146 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 7 Aug 2017 13:59:52 -0700 Subject: [PATCH 07/13] Address CR feedback --- .../reference/indexSignatureAndMappedType.errors.txt | 2 +- tests/baselines/reference/indexSignatureAndMappedType.js | 4 ++-- tests/cases/compiler/indexSignatureAndMappedType.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/baselines/reference/indexSignatureAndMappedType.errors.txt b/tests/baselines/reference/indexSignatureAndMappedType.errors.txt index c8ae0494015..623fcd11bd0 100644 --- a/tests/baselines/reference/indexSignatureAndMappedType.errors.txt +++ b/tests/baselines/reference/indexSignatureAndMappedType.errors.txt @@ -15,7 +15,7 @@ tests/cases/compiler/indexSignatureAndMappedType.ts(16,5): error TS2322: Type '{ !!! error TS2322: Type '{ [key: string]: T; }' is not assignable to type 'Record'. } - function f2(x: { [key: string]: T }, y: Record) { + function f2(x: { [key: string]: T }, y: Record) { x = y; y = x; } diff --git a/tests/baselines/reference/indexSignatureAndMappedType.js b/tests/baselines/reference/indexSignatureAndMappedType.js index c35da4f4931..be58286fb46 100644 --- a/tests/baselines/reference/indexSignatureAndMappedType.js +++ b/tests/baselines/reference/indexSignatureAndMappedType.js @@ -7,7 +7,7 @@ function f1(x: { [key: string]: T }, y: Record) { y = x; // Error } -function f2(x: { [key: string]: T }, y: Record) { +function f2(x: { [key: string]: T }, y: Record) { x = y; y = x; } @@ -55,7 +55,7 @@ function f3(x, y) { declare function f1(x: { [key: string]: T; }, y: Record): void; -declare function f2(x: { +declare function f2(x: { [key: string]: T; }, y: Record): void; declare function f3(x: { diff --git a/tests/cases/compiler/indexSignatureAndMappedType.ts b/tests/cases/compiler/indexSignatureAndMappedType.ts index 1070472a241..b5f9e8a0030 100644 --- a/tests/cases/compiler/indexSignatureAndMappedType.ts +++ b/tests/cases/compiler/indexSignatureAndMappedType.ts @@ -9,7 +9,7 @@ function f1(x: { [key: string]: T }, y: Record) { y = x; // Error } -function f2(x: { [key: string]: T }, y: Record) { +function f2(x: { [key: string]: T }, y: Record) { x = y; y = x; } From b07aa0d97143cbcb5f15f005b00d0f4b36dbb981 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Mon, 7 Aug 2017 17:58:32 -0700 Subject: [PATCH 08/13] fix lint errors --- src/compiler/core.ts | 6 +++--- src/harness/unittests/matchFiles.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 07f979333b6..45602616640 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -2021,16 +2021,16 @@ namespace ts { componentPattern += component.replace(reservedCharacterPattern, replaceWildcardCharacter); - // Patterns should not include subfolders like node_modules unless they are + // Patterns should not include subfolders like node_modules unless they are // explicitly included as part of the path. // - // As an optimization, if the component pattern is the same as the component, + // As an optimization, if the component pattern is the same as the component, // then there definitely were no wildcard characters and we do not need to // add the exclusion pattern. if (componentPattern !== component) { subpattern += implicitExcludePathRegexPattern; } - + subpattern += componentPattern; } else { diff --git a/src/harness/unittests/matchFiles.ts b/src/harness/unittests/matchFiles.ts index 71b1bfff11a..9e2a5883754 100644 --- a/src/harness/unittests/matchFiles.ts +++ b/src/harness/unittests/matchFiles.ts @@ -1322,7 +1322,7 @@ namespace ts { }); }); }); - + describe("with files or folders that begin with a .", () => { it("that are not explicitly included", () => { const json = { From 813aaf40c01c4e46cb9a5477dcf1a65c031745d9 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Mon, 7 Aug 2017 18:20:57 -0700 Subject: [PATCH 09/13] fix lint errors --- src/harness/harness.ts | 2 +- src/lib/es2015.symbol.wellknown.d.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/harness/harness.ts b/src/harness/harness.ts index 55a8f3ebb4d..7122f55cae2 100644 --- a/src/harness/harness.ts +++ b/src/harness/harness.ts @@ -420,7 +420,7 @@ namespace Utils { const maxHarnessFrames = 1; - export function filterStack(error: Error, stackTraceLimit: number = Infinity) { + export function filterStack(error: Error, stackTraceLimit = Infinity) { const stack = (error).stack; if (stack) { const lines = stack.split(/\r\n?|\n/g); diff --git a/src/lib/es2015.symbol.wellknown.d.ts b/src/lib/es2015.symbol.wellknown.d.ts index 578cf0acbc2..b7c2610e652 100644 --- a/src/lib/es2015.symbol.wellknown.d.ts +++ b/src/lib/es2015.symbol.wellknown.d.ts @@ -110,7 +110,7 @@ interface Map { readonly [Symbol.toStringTag]: "Map"; } -interface WeakMap{ +interface WeakMap { readonly [Symbol.toStringTag]: "WeakMap"; } From 9ea2350a6d8119156674759327333e6e2082ad10 Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 8 Aug 2017 07:31:21 -0700 Subject: [PATCH 10/13] Simplify parameters to updateProjectStructure and updateErrorCheck (#17175) --- src/server/session.ts | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index 074ba4d6ca1..8fa580d7225 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -337,7 +337,7 @@ namespace ts.server { case ContextEvent: const { project, fileName } = event.data; this.projectService.logger.info(`got context event, updating diagnostics for ${fileName}`); - this.errorCheck.startNew(next => this.updateErrorCheck(next, [{ fileName, project }], this.changeSeq, (n) => n === this.changeSeq, 100)); + this.errorCheck.startNew(next => this.updateErrorCheck(next, [{ fileName, project }], 100)); break; case ConfigFileDiagEvent: const { triggerFile, configFileName, diagnostics } = event.data; @@ -453,22 +453,23 @@ namespace ts.server { } } - private updateProjectStructure(seq: number, matchSeq: (seq: number) => boolean, ms = 1500) { + private updateProjectStructure() { + const ms = 1500; + const seq = this.changeSeq; this.host.setTimeout(() => { - if (matchSeq(seq)) { + if (this.changeSeq === seq) { this.projectService.refreshInferredProjects(); } }, ms); } - private updateErrorCheck(next: NextStep, checkList: PendingErrorCheck[], seq: number, matchSeq: (seq: number) => boolean, ms = 1500, followMs = 200, requireOpen = true) { - if (followMs > ms) { - followMs = ms; - } + private updateErrorCheck(next: NextStep, checkList: PendingErrorCheck[], ms: number, requireOpen = true) { + const seq = this.changeSeq; + const followMs = Math.min(ms, 200); let index = 0; const checkOne = () => { - if (matchSeq(seq)) { + if (this.changeSeq === seq) { const checkSpec = checkList[index]; index++; if (checkSpec.project.containsFile(checkSpec.fileName, requireOpen)) { @@ -483,7 +484,7 @@ namespace ts.server { } }; - if ((checkList.length > index) && (matchSeq(seq))) { + if (checkList.length > index && this.changeSeq === seq) { next.delay(ms, checkOne); } } @@ -1262,14 +1263,14 @@ namespace ts.server { } private getDiagnostics(next: NextStep, delay: number, fileNames: string[]): void { - const checkList = mapDefined(fileNames, uncheckedFileName => { + const checkList = mapDefined(fileNames, uncheckedFileName => { const fileName = toNormalizedPath(uncheckedFileName); const project = this.projectService.getDefaultProjectForFile(fileName, /*refreshInferredProjects*/ true); return project && { fileName, project }; }); if (checkList.length > 0) { - this.updateErrorCheck(next, checkList, this.changeSeq, (n) => n === this.changeSeq, delay); + this.updateErrorCheck(next, checkList, delay); } } @@ -1283,7 +1284,7 @@ namespace ts.server { scriptInfo.editContent(start, end, args.insertString); this.changeSeq++; } - this.updateProjectStructure(this.changeSeq, n => n === this.changeSeq); + this.updateProjectStructure(); } } @@ -1638,7 +1639,7 @@ namespace ts.server { const checkList = fileNamesInProject.map(fileName => ({ fileName, project })); // Project level error analysis runs on background files too, therefore // doesn't require the file to be opened - this.updateErrorCheck(next, checkList, this.changeSeq, (n) => n === this.changeSeq, delay, 200, /*requireOpen*/ false); + this.updateErrorCheck(next, checkList, delay, /*requireOpen*/ false); } } From 382785a5282f6d49dae0f6af483c8acdaf89ed78 Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 8 Aug 2017 07:54:08 -0700 Subject: [PATCH 11/13] Fix logging of module resolution errors (#17144) --- src/compiler/moduleNameResolver.ts | 2 +- src/harness/harnessLanguageService.ts | 2 +- src/server/project.ts | 3 ++- src/server/server.ts | 2 -- src/server/types.ts | 2 +- 5 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 513c741ba09..be867db9743 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -678,7 +678,7 @@ namespace ts { const { resolvedModule, failedLookupLocations } = nodeModuleNameResolverWorker(moduleName, initialDir, { moduleResolution: ts.ModuleResolutionKind.NodeJs, allowJs: true }, host, /*cache*/ undefined, /*jsOnly*/ true); if (!resolvedModule) { - throw new Error(`Could not resolve JS module ${moduleName} starting at ${initialDir}. Looked in: ${failedLookupLocations.join(", ")}`); + throw new Error(`Could not resolve JS module '${moduleName}' starting at '${initialDir}'. Looked in: ${failedLookupLocations.join(", ")}`); } return resolvedModule.resolvedFileName; } diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index c604b224656..994ebe67e0c 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -795,7 +795,7 @@ namespace Harness.LanguageService { default: return { module: undefined, - error: "Could not resolve module" + error: new Error("Could not resolve module") }; } diff --git a/src/server/project.ts b/src/server/project.ts index 524b6c4d28d..5a92e6505e0 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -170,7 +170,8 @@ namespace ts.server { log(`Loading ${moduleName} from ${initialDir} (resolved to ${resolvedPath})`); const result = host.require(resolvedPath, moduleName); if (result.error) { - log(`Failed to load module: ${JSON.stringify(result.error)}`); + const err = result.error.stack || result.error.message || JSON.stringify(result.error); + log(`Failed to load module '${moduleName}': ${err}`); return undefined; } return result.module; diff --git a/src/server/server.ts b/src/server/server.ts index b72cc2f5a81..d0044153b91 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -116,8 +116,6 @@ namespace ts.server { birthtime: Date; } - type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; - const readline: { createInterface(options: ReadLineOptions): NodeJS.EventEmitter; } = require("readline"); diff --git a/src/server/types.ts b/src/server/types.ts index 07b94fe827e..4fc4356a4a9 100644 --- a/src/server/types.ts +++ b/src/server/types.ts @@ -9,7 +9,7 @@ declare namespace ts.server { data: any; } - type RequireResult = { module: {}, error: undefined } | { module: undefined, error: {} }; + type RequireResult = { module: {}, error: undefined } | { module: undefined, error: { stack?: string, message?: string } }; export interface ServerHost extends System { setTimeout(callback: (...args: any[]) => void, ms: number, ...args: any[]): any; clearTimeout(timeoutId: any): void; From a9a30d76fb39a55d91f633b3555d90c4f438d9ae Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 8 Aug 2017 07:55:03 -0700 Subject: [PATCH 12/13] Fix parsing of globalPlugins and pluginProbeLocations: Don't include empty string (#17143) --- src/server/server.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/server/server.ts b/src/server/server.ts index d0044153b91..2bf10964fa3 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -760,8 +760,16 @@ namespace ts.server { const typingSafeListLocation = findArgument(Arguments.TypingSafeListLocation); const npmLocation = findArgument(Arguments.NpmLocation); - const globalPlugins = (findArgument("--globalPlugins") || "").split(","); - const pluginProbeLocations = (findArgument("--pluginProbeLocations") || "").split(","); + function parseStringArray(argName: string): string[] { + const arg = findArgument(argName); + if (arg === undefined) { + return emptyArray as string[]; // TODO: https://github.com/Microsoft/TypeScript/issues/16312 + } + return arg.split(",").filter(name => name !== ""); + } + + const globalPlugins = parseStringArray("--globalPlugins"); + const pluginProbeLocations = parseStringArray("--pluginProbeLocations"); const allowLocalPluginLoads = hasArgument("--allowLocalPluginLoads"); const useSingleInferredProject = hasArgument("--useSingleInferredProject"); From ceae613e4c0ba36829a2381687883ecdc6b169c3 Mon Sep 17 00:00:00 2001 From: Andy Date: Tue, 8 Aug 2017 07:56:14 -0700 Subject: [PATCH 13/13] Add lint rule to check that `Debug.assert` calls do not eagerly interpolate strings (#17125) * And lint rule to check that `Debug.assert` calls do not eagerly interpolate strings * Use more specific 'assert' functions to avoid callbacks * Respond to PR feedback --- scripts/tslint/booleanTriviaRule.ts | 3 +- scripts/tslint/debugAssertRule.ts | 45 +++++++++++++++++++++++++ src/compiler/core.ts | 37 ++++++++++++++++---- src/compiler/transformers/generators.ts | 2 +- src/server/project.ts | 2 +- src/services/classifier.ts | 4 +-- src/services/services.ts | 2 +- src/services/signatureHelp.ts | 12 +++++-- src/services/transpile.ts | 4 +-- tslint.json | 1 + 10 files changed, 95 insertions(+), 17 deletions(-) create mode 100644 scripts/tslint/debugAssertRule.ts diff --git a/scripts/tslint/booleanTriviaRule.ts b/scripts/tslint/booleanTriviaRule.ts index 189dafac77e..c498131be16 100644 --- a/scripts/tslint/booleanTriviaRule.ts +++ b/scripts/tslint/booleanTriviaRule.ts @@ -34,6 +34,7 @@ function walk(ctx: Lint.WalkContext): void { switch (methodName) { case "apply": case "assert": + case "assertEqual": case "call": case "equal": case "fail": @@ -69,7 +70,7 @@ function walk(ctx: Lint.WalkContext): void { const ranges = ts.getTrailingCommentRanges(sourceFile.text, arg.pos) || ts.getLeadingCommentRanges(sourceFile.text, arg.pos); if (ranges === undefined || ranges.length !== 1 || ranges[0].kind !== ts.SyntaxKind.MultiLineCommentTrivia) { - ctx.addFailureAtNode(arg, "Tag boolean argument with parameter name"); + ctx.addFailureAtNode(arg, "Tag argument with parameter name"); return; } diff --git a/scripts/tslint/debugAssertRule.ts b/scripts/tslint/debugAssertRule.ts new file mode 100644 index 00000000000..933b27697b0 --- /dev/null +++ b/scripts/tslint/debugAssertRule.ts @@ -0,0 +1,45 @@ +import * as Lint from "tslint/lib"; +import * as ts from "typescript"; + +export class Rule extends Lint.Rules.AbstractRule { + public apply(sourceFile: ts.SourceFile): Lint.RuleFailure[] { + return this.applyWithFunction(sourceFile, ctx => walk(ctx)); + } +} + +function walk(ctx: Lint.WalkContext): void { + ts.forEachChild(ctx.sourceFile, function recur(node) { + if (ts.isCallExpression(node)) { + checkCall(node); + } + ts.forEachChild(node, recur); + }); + + function checkCall(node: ts.CallExpression) { + if (!isDebugAssert(node.expression) || node.arguments.length < 2) { + return; + } + + const message = node.arguments[1]; + if (!ts.isStringLiteral(message)) { + ctx.addFailureAtNode(message, "Second argument to 'Debug.assert' should be a string literal."); + } + + if (node.arguments.length < 3) { + return; + } + + const message2 = node.arguments[2]; + if (!ts.isStringLiteral(message2) && !ts.isArrowFunction(message2)) { + ctx.addFailureAtNode(message, "Third argument to 'Debug.assert' should be a string literal or arrow function."); + } + } + + function isDebugAssert(expr: ts.Node): boolean { + return ts.isPropertyAccessExpression(expr) && isName(expr.expression, "Debug") && isName(expr.name, "assert"); + } + + function isName(expr: ts.Node, text: string): boolean { + return ts.isIdentifier(expr) && expr.text === text; + } +} diff --git a/src/compiler/core.ts b/src/compiler/core.ts index f7ea0583670..bc131b201fd 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -1290,12 +1290,12 @@ namespace ts { export function createFileDiagnostic(file: SourceFile, start: number, length: number, message: DiagnosticMessage): Diagnostic { const end = start + length; - Debug.assert(start >= 0, "start must be non-negative, is " + start); - Debug.assert(length >= 0, "length must be non-negative, is " + length); + Debug.assertGreaterThanOrEqual(start, 0); + Debug.assertGreaterThanOrEqual(length, 0); if (file) { - Debug.assert(start <= file.text.length, `start must be within the bounds of the file. ${start} > ${file.text.length}`); - Debug.assert(end <= file.text.length, `end must be the bounds of the file. ${end} > ${file.text.length}`); + Debug.assertLessThanOrEqual(start, file.text.length); + Debug.assertLessThanOrEqual(end, file.text.length); } let text = getLocaleSpecificMessage(message); @@ -2389,15 +2389,40 @@ namespace ts { return currentAssertionLevel >= level; } - export function assert(expression: boolean, message?: string, verboseDebugInfo?: () => string, stackCrawlMark?: Function): void { + export function assert(expression: boolean, message?: string, verboseDebugInfo?: string | (() => string), stackCrawlMark?: Function): void { if (!expression) { if (verboseDebugInfo) { - message += "\r\nVerbose Debug Information: " + verboseDebugInfo(); + message += "\r\nVerbose Debug Information: " + (typeof verboseDebugInfo === "string" ? verboseDebugInfo : verboseDebugInfo()); } fail(message ? "False expression: " + message : "False expression.", stackCrawlMark || assert); } } + export function assertEqual(a: T, b: T, msg?: string, msg2?: string): void { + if (a !== b) { + const message = msg ? msg2 ? `${msg} ${msg2}` : msg : ""; + fail(`Expected ${a} === ${b}. ${message}`); + } + } + + export function assertLessThan(a: number, b: number, msg?: string): void { + if (a >= b) { + fail(`Expected ${a} < ${b}. ${msg || ""}`); + } + } + + export function assertLessThanOrEqual(a: number, b: number): void { + if (a > b) { + fail(`Expected ${a} <= ${b}`); + } + } + + export function assertGreaterThanOrEqual(a: number, b: number): void { + if (a < b) { + fail(`Expected ${a} >= ${b}`); + } + } + export function fail(message?: string, stackCrawlMark?: Function): void { debugger; const e = new Error(message ? `Debug Failure. ${message}` : "Debug Failure."); diff --git a/src/compiler/transformers/generators.ts b/src/compiler/transformers/generators.ts index 41edf24fe9e..5e2016ab590 100644 --- a/src/compiler/transformers/generators.ts +++ b/src/compiler/transformers/generators.ts @@ -2448,7 +2448,7 @@ namespace ts { * @param location An optional source map location for the statement. */ function createInlineBreak(label: Label, location?: TextRange): ReturnStatement { - Debug.assert(label > 0, `Invalid label: ${label}`); + Debug.assertLessThan(0, label, "Invalid label"); return setTextRange( createReturn( createArrayLiteral([ diff --git a/src/server/project.ts b/src/server/project.ts index 5a92e6505e0..844ded761d8 100644 --- a/src/server/project.ts +++ b/src/server/project.ts @@ -363,7 +363,7 @@ namespace ts.server { return map(this.program.getSourceFiles(), sourceFile => { const scriptInfo = this.projectService.getScriptInfoForPath(sourceFile.path); if (!scriptInfo) { - Debug.assert(false, `scriptInfo for a file '${sourceFile.fileName}' is missing.`); + Debug.fail(`scriptInfo for a file '${sourceFile.fileName}' is missing.`); } return scriptInfo; }); diff --git a/src/services/classifier.ts b/src/services/classifier.ts index dc5d99bc490..4552d8bf985 100644 --- a/src/services/classifier.ts +++ b/src/services/classifier.ts @@ -260,11 +260,11 @@ namespace ts { templateStack.pop(); } else { - Debug.assert(token === SyntaxKind.TemplateMiddle, "Should have been a template middle. Was " + token); + Debug.assertEqual(token, SyntaxKind.TemplateMiddle, "Should have been a template middle."); } } else { - Debug.assert(lastTemplateStackToken === SyntaxKind.OpenBraceToken, "Should have been an open brace. Was: " + token); + Debug.assertEqual(lastTemplateStackToken, SyntaxKind.OpenBraceToken, "Should have been an open brace"); templateStack.pop(); } } diff --git a/src/services/services.ts b/src/services/services.ts index 6a171ad9166..874c2feb107 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1258,7 +1258,7 @@ namespace ts { // We do not support the scenario where a host can modify a registered // file's script kind, i.e. in one project some file is treated as ".ts" // and in another as ".js" - Debug.assert(hostFileInformation.scriptKind === oldSourceFile.scriptKind, "Registered script kind (" + oldSourceFile.scriptKind + ") should match new script kind (" + hostFileInformation.scriptKind + ") for file: " + path); + Debug.assertEqual(hostFileInformation.scriptKind, oldSourceFile.scriptKind, "Registered script kind should match new script kind.", path); return documentRegistry.updateDocumentWithKey(fileName, path, newSettings, documentRegistryBucketKey, hostFileInformation.scriptSnapshot, hostFileInformation.version, hostFileInformation.scriptKind); } diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index f008c829116..2976b0d28ee 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -136,7 +136,9 @@ namespace ts.SignatureHelp { const kind = invocation.typeArguments && invocation.typeArguments.pos === list.pos ? ArgumentListKind.TypeArguments : ArgumentListKind.CallArguments; const argumentCount = getArgumentCount(list); - Debug.assert(argumentIndex === 0 || argumentIndex < argumentCount, `argumentCount < argumentIndex, ${argumentCount} < ${argumentIndex}`); + if (argumentIndex !== 0) { + Debug.assertLessThan(argumentIndex, argumentCount); + } const argumentsSpan = getApplicableSpanForArguments(list, sourceFile); return { kind, invocation, argumentsSpan, argumentIndex, argumentCount }; } @@ -270,7 +272,9 @@ namespace ts.SignatureHelp { ? 1 : (tagExpression.template).templateSpans.length + 1; - Debug.assert(argumentIndex === 0 || argumentIndex < argumentCount, `argumentCount < argumentIndex, ${argumentCount} < ${argumentIndex}`); + if (argumentIndex !== 0) { + Debug.assertLessThan(argumentIndex, argumentCount); + } return { kind: ArgumentListKind.TaggedTemplateArguments, invocation: tagExpression, @@ -402,7 +406,9 @@ namespace ts.SignatureHelp { }; }); - Debug.assert(argumentIndex === 0 || argumentIndex < argumentCount, `argumentCount < argumentIndex, ${argumentCount} < ${argumentIndex}`); + if (argumentIndex !== 0) { + Debug.assertLessThan(argumentIndex, argumentCount); + } const selectedItemIndex = candidates.indexOf(resolvedSignature); Debug.assert(selectedItemIndex !== -1); // If candidates is non-empty it should always include bestSignature. We check for an empty candidates before calling this function. diff --git a/src/services/transpile.ts b/src/services/transpile.ts index 561c188c6cd..79a69b886d9 100644 --- a/src/services/transpile.ts +++ b/src/services/transpile.ts @@ -78,11 +78,11 @@ namespace ts { getSourceFile: (fileName) => fileName === normalizePath(inputFileName) ? sourceFile : undefined, writeFile: (name, text) => { if (fileExtensionIs(name, ".map")) { - Debug.assert(sourceMapText === undefined, `Unexpected multiple source map outputs for the file '${name}'`); + Debug.assertEqual(sourceMapText, undefined, "Unexpected multiple source map outputs, file:", name); sourceMapText = text; } else { - Debug.assert(outputText === undefined, `Unexpected multiple outputs for the file: '${name}'`); + Debug.assertEqual(outputText, undefined, "Unexpected multiple outputs, file:", name); outputText = text; } }, diff --git a/tslint.json b/tslint.json index bcd4dfa2223..de60ad7683a 100644 --- a/tslint.json +++ b/tslint.json @@ -7,6 +7,7 @@ "check-space" ], "curly":[true, "ignore-same-line"], + "debug-assert": true, "indent": [true, "spaces" ],