Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(29118): tsconfig.extends as array #50403

Merged
merged 15 commits into from
Dec 13, 2022
Merged
1 change: 1 addition & 0 deletions src/compiler/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,7 @@ function getBuildInfo(state: BuilderProgramState, bundle: BundleBuildInfo | unde

function convertToReusableCompilerOptionValue(option: CommandLineOption | undefined, value: CompilerOptionsValue, relativeToBuildInfo: (path: string) => string) {
if (option) {
Debug.assert(option.type !== "listOrElement");
if (option.type === "list") {
const values = value as readonly (string | number)[];
if (option.element.isFilePath && values.length) {
Expand Down
216 changes: 158 additions & 58 deletions src/compiler/commandLineParser.ts

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6977,7 +6977,7 @@ export interface CreateProgramOptions {
/** @internal */
export interface CommandLineOptionBase {
name: string;
type: "string" | "number" | "boolean" | "object" | "list" | Map<string, number | string>; // a value of a primitive type, or an object literal mapping named values to actual values
type: "string" | "number" | "boolean" | "object" | "list" | "listOrElement" | Map<string, number | string>; // a value of a primitive type, or an object literal mapping named values to actual values
isFilePath?: boolean; // True if option value is a path or fileName
shortName?: string; // A short mnemonic for convenience - for instance, 'h' can be used in place of 'help'
description?: DiagnosticMessage; // The message describing what the command line switch does.
Expand Down Expand Up @@ -7047,7 +7047,7 @@ export interface TsConfigOnlyOption extends CommandLineOptionBase {

/** @internal */
export interface CommandLineOptionOfListType extends CommandLineOptionBase {
type: "list";
type: "list" | "listOrElement";
element: CommandLineOptionOfCustomType | CommandLineOptionOfStringType | CommandLineOptionOfNumberType | CommandLineOptionOfBooleanType | TsConfigOnlyOption;
listPreserveFalsyValues?: boolean;
}
Expand Down
9 changes: 5 additions & 4 deletions src/executeCommandLine/executeCommandLine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,9 +253,9 @@ function generateOptionOutput(sys: System, option: CommandLineOption, rightAlign
typeof option.defaultValueDescription === "object"
? getDiagnosticText(option.defaultValueDescription)
: formatDefaultValue(
option.defaultValueDescription,
option.type === "list" ? option.element.type : option.type
);
option.defaultValueDescription,
option.type === "list" || option.type === "listOrElement" ? option.element.type : option.type
);
const terminalWidth = sys.getWidthOfTerminal?.() ?? 0;

// Note: child_process might return `terminalWidth` as undefined.
Expand Down Expand Up @@ -365,6 +365,7 @@ function generateOptionOutput(sys: System, option: CommandLineOption, rightAlign
};

function getValueType(option: CommandLineOption) {
Debug.assert(option.type !== "listOrElement");
switch (option.type) {
case "string":
case "number":
Expand All @@ -386,7 +387,7 @@ function generateOptionOutput(sys: System, option: CommandLineOption, rightAlign
possibleValues = option.type;
break;
case "list":
// TODO: check infinite loop
case "listOrElement":
possibleValues = getPossibleValues(option.element);
break;
case "object":
Expand Down
1 change: 1 addition & 0 deletions src/harness/harnessIO.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ export namespace Compiler {
}
// If not a primitive, the possible types are specified in what is effectively a map of options.
case "list":
case "listOrElement":
return ts.parseListTypeOption(option, value, errors);
default:
return ts.parseCustomTypeOption(option as ts.CommandLineOptionOfCustomType, value, errors);
Expand Down
3 changes: 2 additions & 1 deletion src/services/getEditsForFileRename.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function updateTsconfigFiles(program: Program, changeTracker: textChanges.Change
if (foundExactMatch || propertyName !== "include" || !isArrayLiteralExpression(property.initializer)) return;
const includes = mapDefined(property.initializer.elements, e => isStringLiteral(e) ? e.text : undefined);
if (includes.length === 0) return;
const matchers = getFileMatcherPatterns(configDir, /*excludes*/ [], includes, useCaseSensitiveFileNames, currentDirectory);
const matchers = getFileMatcherPatterns(configDir, /*excludes*/[], includes, useCaseSensitiveFileNames, currentDirectory);
// If there isn't some include for this, add a new one.
if (getRegexFromPattern(Debug.checkDefined(matchers.includeFilePattern), useCaseSensitiveFileNames).test(oldFileOrDirPath) &&
!getRegexFromPattern(Debug.checkDefined(matchers.includeFilePattern), useCaseSensitiveFileNames).test(newFileOrDirPath)) {
Expand All @@ -131,6 +131,7 @@ function updateTsconfigFiles(program: Program, changeTracker: textChanges.Change
case "compilerOptions":
forEachProperty(property.initializer, (property, propertyName) => {
const option = getOptionFromName(propertyName);
Debug.assert(option?.type !== "listOrElement");
if (option && (option.isFilePath || option.type === "list" && option.element.isFilePath)) {
updatePaths(property);
}
Expand Down
81 changes: 78 additions & 3 deletions src/testRunner/unittests/config/configurationExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,47 @@ function createFileSystem(ignoreCase: boolean, cwd: string, root: string) {
"dev/tests/unit/spec.ts": "",
"dev/tests/utils.ts": "",
"dev/tests/scenarios/first.json": "",
"dev/tests/baselines/first/output.ts": ""
"dev/tests/baselines/first/output.ts": "",
"dev/configs/extendsArrayFirst.json": JSON.stringify({
compilerOptions: {
allowJs: true,
noImplicitAny: true,
strictNullChecks: true
}
}),
"dev/configs/extendsArraySecond.json": JSON.stringify({
compilerOptions: {
module: "amd"
},
include: ["../supplemental.*"]
}),
"dev/configs/extendsArrayThird.json": JSON.stringify({
compilerOptions: {
module: null, // eslint-disable-line no-null/no-null
noImplicitAny: false
},
extends: "./extendsArrayFirst",
include: ["../supplemental.*"]
}),
"dev/configs/extendsArrayFourth.json": JSON.stringify({
compilerOptions: {
module: "system",
strictNullChecks: false
},
include: null, // eslint-disable-line no-null/no-null
files: ["../main.ts"]
}),
"dev/configs/extendsArrayFifth.json": JSON.stringify({
extends: ["./extendsArrayFirst", "./extendsArraySecond", "./extendsArrayThird", "./extendsArrayFourth"],
files: [],
}),
"dev/extendsArrayFails.json": JSON.stringify({
extends: ["./missingFile"],
compilerOptions: {
types: []
}
}),
"dev/extendsArrayFails2.json": JSON.stringify({ extends: [42] }),
}
}
});
Expand Down Expand Up @@ -295,9 +335,9 @@ describe("unittests:: config:: configurationExtension", () => {
messageText: `Unknown option 'excludes'. Did you mean 'exclude'?`
}]);

testFailure("can error when 'extends' is not a string", "extends.json", [{
testFailure("can error when 'extends' is not a string or Array", "extends.json", [{
code: 5024,
messageText: `Compiler option 'extends' requires a value of type string.`
messageText: `Compiler option 'extends' requires a value of type string or Array.`
}]);

testSuccess("can overwrite compiler options using extended 'null'", "configs/third.json", {
Expand Down Expand Up @@ -352,5 +392,40 @@ describe("unittests:: config:: configurationExtension", () => {
assert.deepEqual(sourceFile.extendedSourceFiles, expected);
});
});

describe(testName, () => {
it("adds extendedSourceFiles from an array only once", () => {
const sourceFile = ts.readJsonConfigFile("configs/extendsArrayFifth.json", (path) => host.readFile(path));
const dir = ts.combinePaths(basePath, "configs");
const expected = [
ts.combinePaths(dir, "extendsArrayFirst.json"),
ts.combinePaths(dir, "extendsArraySecond.json"),
ts.combinePaths(dir, "extendsArrayThird.json"),
ts.combinePaths(dir, "extendsArrayFourth.json"),
];
ts.parseJsonSourceFileConfigFileContent(sourceFile, host, dir, {}, "extendsArrayFifth.json");
assert.deepEqual(sourceFile.extendedSourceFiles, expected);
ts.parseJsonSourceFileConfigFileContent(sourceFile, host, dir, {}, "extendsArrayFifth.json");
assert.deepEqual(sourceFile.extendedSourceFiles, expected);
});

testSuccess("can overwrite top-level compilerOptions", "configs/extendsArrayFifth.json", {
allowJs: true,
noImplicitAny: false,
strictNullChecks: false,
module: ts.ModuleKind.System
}, []);

testFailure("can report missing configurations", "extendsArrayFails.json", [{
code: 6053,
messageText: `File './missingFile' not found.`
}]);

testFailure("can error when 'extends' is not a string or Array2", "extendsArrayFails2.json", [{
code: 5024,
messageText: `Compiler option 'extends' requires a value of type string.`
}]);
});
});
});

4 changes: 4 additions & 0 deletions src/testRunner/unittests/config/showConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,10 @@ describe("unittests:: config:: showConfig", () => {
}
break;
}
case "listOrElement": {
ts.Debug.fail();
break;
}
case "string": {
if (option.isTSConfigOnly) {
args = ["-p", "tsconfig.json"];
Expand Down
75 changes: 72 additions & 3 deletions src/testRunner/unittests/tsbuildWatch/programUpdates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ export function someFn() { }`),
verifyTscWatch({
scenario: "programUpdates",
subScenario: "works with extended source files",
commandLineArgs: ["-b", "-w", "-v", "project1.tsconfig.json", "project2.tsconfig.json"],
commandLineArgs: ["-b", "-w", "-v", "project1.tsconfig.json", "project2.tsconfig.json", "project3.tsconfig.json"],
sys: () => {
const alphaExtendedConfigFile: File = {
path: "/a/b/alpha.tsconfig.json",
Expand Down Expand Up @@ -633,10 +633,49 @@ export function someFn() { }`),
files: [otherFile.path]
})
};
const otherFile2: File = {
path: "/a/b/other2.ts",
content: "let k = 0;",
};
const extendsConfigFile1: File = {
path: "/a/b/extendsConfig1.tsconfig.json",
content: JSON.stringify({
compilerOptions: {
composite: true,
}
})
};
const extendsConfigFile2: File = {
path: "/a/b/extendsConfig2.tsconfig.json",
content: JSON.stringify({
compilerOptions: {
strictNullChecks: false,
}
})
};
const extendsConfigFile3: File = {
path: "/a/b/extendsConfig3.tsconfig.json",
content: JSON.stringify({
compilerOptions: {
noImplicitAny: true,
}
})
};
const project3Config: File = {
path: "/a/b/project3.tsconfig.json",
content: JSON.stringify({
extends: ["./extendsConfig1.tsconfig.json", "./extendsConfig2.tsconfig.json", "./extendsConfig3.tsconfig.json"],
compilerOptions: {
composite: false,
},
files: [otherFile2.path]
})
};
return createWatchedSystem([
libFile,
alphaExtendedConfigFile, project1Config, commonFile1, commonFile2,
bravoExtendedConfigFile, project2Config, otherFile
bravoExtendedConfigFile, project2Config, otherFile, otherFile2,
extendsConfigFile1, extendsConfigFile2, extendsConfigFile3, project3Config
], { currentDirectory: "/a/b" });
},
edits: [
Expand Down Expand Up @@ -689,7 +728,37 @@ export function someFn() { }`),
sys.checkTimeoutQueueLength(0);
},
},
]
{
caption: "Modify extendsConfigFile2",
edit: sys => sys.writeFile("/a/b/extendsConfig2.tsconfig.json", JSON.stringify({
compilerOptions: { strictNullChecks: true }
})),
timeouts: sys => { // Build project3
sys.checkTimeoutQueueLengthAndRun(1);
sys.checkTimeoutQueueLength(0);
},
},
{
caption: "Modify project 3",
edit: sys => sys.writeFile("/a/b/project3.tsconfig.json", JSON.stringify({
extends: ["./extendsConfig1.tsconfig.json", "./extendsConfig2.tsconfig.json"],
compilerOptions: { composite: false },
files: ["/a/b/other2.ts"]
sheetalkamat marked this conversation as resolved.
Show resolved Hide resolved
})),
timeouts: sys => { // Build project3
sys.checkTimeoutQueueLengthAndRun(1);
sys.checkTimeoutQueueLength(0);
},
},
{
caption: "Delete extendedConfigFile2 and report error",
edit: sys => sys.deleteFile("./extendsConfig2.tsconfig.json"),
timeouts: sys => { // Build project3
sys.checkTimeoutQueueLengthAndRun(1);
sys.checkTimeoutQueueLength(0);
},
}
],
});

verifyTscWatch({
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9170,7 +9170,7 @@ declare namespace ts {
/**
* Note that the case of the config path has not yet been normalized, as no files have been imported into the project yet
*/
extendedConfigPath?: string;
extendedConfigPath?: string | string[];
}
interface ExtendedConfigCacheEntry {
extendedResult: TsConfigSourceFile;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5236,7 +5236,7 @@ declare namespace ts {
/**
* Note that the case of the config path has not yet been normalized, as no files have been imported into the project yet
*/
extendedConfigPath?: string;
extendedConfigPath?: string | string[];
}
interface ExtendedConfigCacheEntry {
extendedResult: TsConfigSourceFile;
Expand Down
31 changes: 31 additions & 0 deletions tests/baselines/reference/configFileExtendsAsList.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/index.ts(1,12): error TS7006: Parameter 'x' implicitly has an 'any' type.
/index.ts(3,1): error TS2454: Variable 'y' is used before being assigned.


==== /tsconfig.json (0 errors) ====
{
"extends": ["./tsconfig1.json", "./tsconfig2.json"]
}

==== /tsconfig1.json (0 errors) ====
{
"compilerOptions": {
"strictNullChecks": true
}
}

==== /tsconfig2.json (0 errors) ====
{
"compilerOptions": {
"noImplicitAny": true
}
}

==== /index.ts (2 errors) ====
function f(x) { } // noImplicitAny error
~
!!! error TS7006: Parameter 'x' implicitly has an 'any' type.
let y: string;
y.toLowerCase(); // strictNullChecks error
~
!!! error TS2454: Variable 'y' is used before being assigned.
25 changes: 25 additions & 0 deletions tests/baselines/reference/configFileExtendsAsList.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//// [tests/cases/compiler/configFileExtendsAsList.ts] ////

//// [tsconfig1.json]
{
"compilerOptions": {
"strictNullChecks": true
}
}

//// [tsconfig2.json]
{
"compilerOptions": {
"noImplicitAny": true
}
}

//// [index.ts]
function f(x) { } // noImplicitAny error
let y: string;
y.toLowerCase(); // strictNullChecks error

//// [index.js]
function f(x) { } // noImplicitAny error
var y;
y.toLowerCase(); // strictNullChecks error
13 changes: 13 additions & 0 deletions tests/baselines/reference/configFileExtendsAsList.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
=== /index.ts ===
function f(x) { } // noImplicitAny error
>f : Symbol(f, Decl(index.ts, 0, 0))
>x : Symbol(x, Decl(index.ts, 0, 11))

let y: string;
>y : Symbol(y, Decl(index.ts, 1, 3))

y.toLowerCase(); // strictNullChecks error
>y.toLowerCase : Symbol(String.toLowerCase, Decl(lib.es5.d.ts, --, --))
>y : Symbol(y, Decl(index.ts, 1, 3))
>toLowerCase : Symbol(String.toLowerCase, Decl(lib.es5.d.ts, --, --))

14 changes: 14 additions & 0 deletions tests/baselines/reference/configFileExtendsAsList.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
=== /index.ts ===
function f(x) { } // noImplicitAny error
>f : (x: any) => void
>x : any

let y: string;
>y : string

y.toLowerCase(); // strictNullChecks error
>y.toLowerCase() : string
>y.toLowerCase : () => string
>y : string
>toLowerCase : () => string

Loading