From 3664e0359cbb424a30cfc56331b709f81a57d043 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 13 Aug 2020 13:10:04 -0700 Subject: [PATCH 1/3] Test displaying failure when specs used are not strings --- .../unittests/config/tsconfigParsing.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/testRunner/unittests/config/tsconfigParsing.ts b/src/testRunner/unittests/config/tsconfigParsing.ts index 5e15bbc6e9f82..3c6dfb6a53636 100644 --- a/src/testRunner/unittests/config/tsconfigParsing.ts +++ b/src/testRunner/unittests/config/tsconfigParsing.ts @@ -381,5 +381,36 @@ namespace ts { const parsed = getParsedCommandJsonNode(jsonText, "/apath/tsconfig.json", "tests/cases/unittests", ["/apath/a.ts"]); assert.isTrue(parsed.errors.length >= 0); }); + + it("generates errors when files is not string", () => { + assertParseFileDiagnostics( + JSON.stringify({ + files: [{ + compilerOptions: { + experimentalDecorators: true, + allowJs: true + } + }] + }), + "/apath/tsconfig.json", + "tests/cases/unittests", + ["/apath/a.ts"], + Diagnostics.Compiler_option_0_requires_a_value_of_type_1.code, + /*noLocation*/ true); + }); + + it("generates errors when include is not string", () => { + assertParseFileDiagnostics( + JSON.stringify({ + include: [ + ["./**/*.ts"] + ] + }), + "/apath/tsconfig.json", + "tests/cases/unittests", + ["/apath/a.ts"], + Diagnostics.Compiler_option_0_requires_a_value_of_type_1.code, + /*noLocation*/ true); + }); }); } From 959dee43671ed670ba923b83e7c2366676d60b39 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 13 Aug 2020 14:01:29 -0700 Subject: [PATCH 2/3] Ensure specs used are strings Fixes #38164, #39856 --- src/compiler/commandLineParser.ts | 102 ++++++++++++++++-------------- src/compiler/types.ts | 9 +-- 2 files changed, 58 insertions(+), 53 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 26b8641e2d753..f079f8f50018b 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -2308,53 +2308,32 @@ namespace ts { }; function getFileNames(): ExpandResult { - let filesSpecs: readonly string[] | undefined; - if (hasProperty(raw, "files") && !isNullOrUndefined(raw.files)) { - if (isArray(raw.files)) { - filesSpecs = raw.files; - const hasReferences = hasProperty(raw, "references") && !isNullOrUndefined(raw.references); - const hasZeroOrNoReferences = !hasReferences || raw.references.length === 0; - const hasExtends = hasProperty(raw, "extends"); - if (filesSpecs.length === 0 && hasZeroOrNoReferences && !hasExtends) { - if (sourceFile) { - const fileName = configFileName || "tsconfig.json"; - const diagnosticMessage = Diagnostics.The_files_list_in_config_file_0_is_empty; - const nodeValue = firstDefined(getTsConfigPropArray(sourceFile, "files"), property => property.initializer); - const error = nodeValue - ? createDiagnosticForNodeInSourceFile(sourceFile, nodeValue, diagnosticMessage, fileName) - : createCompilerDiagnostic(diagnosticMessage, fileName); - errors.push(error); - } - else { - createCompilerDiagnosticOnlyIfJson(Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json"); - } + const filesSpecs = getSpecs("files")?.specs; + if (filesSpecs) { + const hasReferences = hasProperty(raw, "references") && !isNullOrUndefined(raw.references); + const hasZeroOrNoReferences = !hasReferences || raw.references.length === 0; + const hasExtends = hasProperty(raw, "extends"); + if (filesSpecs.length === 0 && hasZeroOrNoReferences && !hasExtends) { + if (sourceFile) { + const fileName = configFileName || "tsconfig.json"; + const diagnosticMessage = Diagnostics.The_files_list_in_config_file_0_is_empty; + const nodeValue = firstDefined(getTsConfigPropArray(sourceFile, "files"), property => property.initializer); + const error = nodeValue + ? createDiagnosticForNodeInSourceFile(sourceFile, nodeValue, diagnosticMessage, fileName) + : createCompilerDiagnostic(diagnosticMessage, fileName); + errors.push(error); + } + else { + createCompilerDiagnosticOnlyIfJson(Diagnostics.The_files_list_in_config_file_0_is_empty, configFileName || "tsconfig.json"); } - } - else { - createCompilerDiagnosticOnlyIfJson(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, "files", "Array"); } } - let includeSpecs: readonly string[] | undefined; - if (hasProperty(raw, "include") && !isNullOrUndefined(raw.include)) { - if (isArray(raw.include)) { - includeSpecs = raw.include; - } - else { - createCompilerDiagnosticOnlyIfJson(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, "include", "Array"); - } - } + let includeSpecs = getSpecs("include")?.specs; - let excludeSpecs: readonly string[] | undefined; - if (hasProperty(raw, "exclude") && !isNullOrUndefined(raw.exclude)) { - if (isArray(raw.exclude)) { - excludeSpecs = raw.exclude; - } - else { - createCompilerDiagnosticOnlyIfJson(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, "exclude", "Array"); - } - } - else if (raw.compilerOptions) { + const excludeSpecResult = getSpecs("exclude"); + let excludeSpecs = excludeSpecResult?.specs; + if (!excludeSpecResult && raw.compilerOptions) { const outDir = raw.compilerOptions.outDir; const declarationDir = raw.compilerOptions.declarationDir; @@ -2396,6 +2375,22 @@ namespace ts { return result; } + function getSpecs(prop: "files" | "include" | "exclude") { + if (hasProperty(raw, prop) && !isNullOrUndefined(raw[prop])) { + let specs: readonly string[] | undefined; + if (isArray(raw[prop])) { + specs = raw[prop]; + if (!sourceFile && !every(specs, isString)) { + errors.push(createCompilerDiagnostic(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, prop, "string")); + } + } + else { + createCompilerDiagnosticOnlyIfJson(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, prop, "Array"); + } + return { specs }; + } + } + function createCompilerDiagnosticOnlyIfJson(message: DiagnosticMessage, arg0?: string, arg1?: string) { if (!sourceFile) { errors.push(createCompilerDiagnostic(message, arg0, arg1)); @@ -2941,7 +2936,15 @@ namespace ts { // new entries in these paths. const wildcardDirectories = getWildcardDirectories(validatedIncludeSpecs, validatedExcludeSpecs, basePath, host.useCaseSensitiveFileNames); - const spec: ConfigFileSpecs = { filesSpecs, includeSpecs, excludeSpecs, validatedIncludeSpecs, validatedExcludeSpecs, wildcardDirectories }; + const spec: ConfigFileSpecs = { + filesSpecs, + includeSpecs, + excludeSpecs, + validatedFilesSpec: filter(filesSpecs, isString), + validatedIncludeSpecs, + validatedExcludeSpecs, + wildcardDirectories + }; return getFileNamesFromConfigSpecs(spec, basePath, options, host, extraFileExtensions); } @@ -2980,7 +2983,7 @@ namespace ts { // file map with a possibly case insensitive key. We use this map to store paths matched // via wildcard of *.json kind const wildCardJsonFileMap = new Map(); - const { filesSpecs, validatedIncludeSpecs, validatedExcludeSpecs, wildcardDirectories } = spec; + const { validatedFilesSpec, validatedIncludeSpecs, validatedExcludeSpecs, wildcardDirectories } = spec; // Rather than requery this for each file and filespec, we query the supported extensions // once and store it on the expansion context. @@ -2989,8 +2992,8 @@ namespace ts { // Literal files are always included verbatim. An "include" or "exclude" specification cannot // remove a literal file. - if (filesSpecs) { - for (const fileName of filesSpecs) { + if (validatedFilesSpec) { + for (const fileName of validatedFilesSpec) { const file = getNormalizedAbsolutePath(fileName, basePath); literalFileMap.set(keyMapper(file), file); } @@ -3056,14 +3059,14 @@ namespace ts { useCaseSensitiveFileNames: boolean, currentDirectory: string ): boolean { - const { filesSpecs, validatedIncludeSpecs, validatedExcludeSpecs } = spec; + const { validatedFilesSpec, validatedIncludeSpecs, validatedExcludeSpecs } = spec; if (!length(validatedIncludeSpecs) || !length(validatedExcludeSpecs)) return false; basePath = normalizePath(basePath); const keyMapper = createGetCanonicalFileName(useCaseSensitiveFileNames); - if (filesSpecs) { - for (const fileName of filesSpecs) { + if (validatedFilesSpec) { + for (const fileName of validatedFilesSpec) { if (keyMapper(getNormalizedAbsolutePath(fileName, basePath)) === pathToCheck) return false; } } @@ -3077,6 +3080,7 @@ namespace ts { function validateSpecs(specs: readonly string[], errors: Push, allowTrailingRecursion: boolean, jsonSourceFile: TsConfigSourceFile | undefined, specKey: string): readonly string[] { return specs.filter(spec => { + if (!isString(spec)) return false; const diag = specToDiagnostic(spec, allowTrailingRecursion); if (diag !== undefined) { errors.push(createDiagnostic(diag, spec)); diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 69d3e9b3d94d9..959ac410d294e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -5880,13 +5880,14 @@ namespace ts { /** * Present to report errors (user specified specs), validatedIncludeSpecs are used for file name matching */ - includeSpecs?: readonly string[]; + includeSpecs: readonly string[] | undefined; /** * Present to report errors (user specified specs), validatedExcludeSpecs are used for file name matching */ - excludeSpecs?: readonly string[]; - validatedIncludeSpecs?: readonly string[]; - validatedExcludeSpecs?: readonly string[]; + excludeSpecs: readonly string[] | undefined; + validatedFilesSpec: readonly string[] | undefined; + validatedIncludeSpecs: readonly string[] | undefined; + validatedExcludeSpecs: readonly string[] | undefined; wildcardDirectories: MapLike; } From 9d9f7e4ec0b7ea553fcd308affeb67142cf8c9e4 Mon Sep 17 00:00:00 2001 From: Sheetal Nandi Date: Thu, 13 Aug 2020 15:32:35 -0700 Subject: [PATCH 3/3] Feedback --- src/compiler/commandLineParser.ts | 73 +++++++++++++++++-------------- 1 file changed, 39 insertions(+), 34 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index f079f8f50018b..3095c9d7dc82e 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -2308,10 +2308,26 @@ namespace ts { }; function getFileNames(): ExpandResult { - const filesSpecs = getSpecs("files")?.specs; + const referencesOfRaw = getPropFromRaw("references", element => typeof element === "object", "object"); + if (isArray(referencesOfRaw)) { + for (const ref of referencesOfRaw) { + if (typeof ref.path !== "string") { + createCompilerDiagnosticOnlyIfJson(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, "reference.path", "string"); + } + else { + (projectReferences || (projectReferences = [])).push({ + path: getNormalizedAbsolutePath(ref.path, basePath), + originalPath: ref.path, + prepend: ref.prepend, + circular: ref.circular + }); + } + } + } + + const filesSpecs = toPropValue(getSpecsFromRaw("files")); if (filesSpecs) { - const hasReferences = hasProperty(raw, "references") && !isNullOrUndefined(raw.references); - const hasZeroOrNoReferences = !hasReferences || raw.references.length === 0; + const hasZeroOrNoReferences = referencesOfRaw === "no-prop" || isArray(referencesOfRaw) && referencesOfRaw.length === 0; const hasExtends = hasProperty(raw, "extends"); if (filesSpecs.length === 0 && hasZeroOrNoReferences && !hasExtends) { if (sourceFile) { @@ -2329,11 +2345,11 @@ namespace ts { } } - let includeSpecs = getSpecs("include")?.specs; + let includeSpecs = toPropValue(getSpecsFromRaw("include")); - const excludeSpecResult = getSpecs("exclude"); - let excludeSpecs = excludeSpecResult?.specs; - if (!excludeSpecResult && raw.compilerOptions) { + const excludeOfRaw = getSpecsFromRaw("exclude"); + let excludeSpecs = toPropValue(excludeOfRaw); + if (excludeOfRaw === "no-prop" && raw.compilerOptions) { const outDir = raw.compilerOptions.outDir; const declarationDir = raw.compilerOptions.declarationDir; @@ -2351,44 +2367,33 @@ namespace ts { errors.push(getErrorForNoInputFiles(result.spec, configFileName)); } - if (hasProperty(raw, "references") && !isNullOrUndefined(raw.references)) { - if (isArray(raw.references)) { - for (const ref of raw.references) { - if (typeof ref.path !== "string") { - createCompilerDiagnosticOnlyIfJson(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, "reference.path", "string"); - } - else { - (projectReferences || (projectReferences = [])).push({ - path: getNormalizedAbsolutePath(ref.path, basePath), - originalPath: ref.path, - prepend: ref.prepend, - circular: ref.circular - }); - } - } - } - else { - createCompilerDiagnosticOnlyIfJson(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, "references", "Array"); - } - } - return result; } - function getSpecs(prop: "files" | "include" | "exclude") { + type PropOfRaw = readonly T[] | "not-array" | "no-prop"; + function toPropValue(specResult: PropOfRaw) { + return isArray(specResult) ? specResult : undefined; + } + + function getSpecsFromRaw(prop: "files" | "include" | "exclude"): PropOfRaw { + return getPropFromRaw(prop, isString, "string"); + } + + function getPropFromRaw(prop: "files" | "include" | "exclude" | "references", validateElement: (value: unknown) => boolean, elementTypeName: string): PropOfRaw { if (hasProperty(raw, prop) && !isNullOrUndefined(raw[prop])) { - let specs: readonly string[] | undefined; if (isArray(raw[prop])) { - specs = raw[prop]; - if (!sourceFile && !every(specs, isString)) { - errors.push(createCompilerDiagnostic(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, prop, "string")); + const result = raw[prop]; + if (!sourceFile && !every(result, validateElement)) { + errors.push(createCompilerDiagnostic(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, prop, elementTypeName)); } + return result; } else { createCompilerDiagnosticOnlyIfJson(Diagnostics.Compiler_option_0_requires_a_value_of_type_1, prop, "Array"); + return "not-array"; } - return { specs }; } + return "no-prop"; } function createCompilerDiagnosticOnlyIfJson(message: DiagnosticMessage, arg0?: string, arg1?: string) {