Skip to content

Fix 10289: correctly generate tsconfig.json with --lib #10355

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

Merged
merged 7 commits into from
Aug 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Jakefile.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ var harnessSources = harnessCoreSources.concat([
"convertCompilerOptionsFromJson.ts",
"convertTypingOptionsFromJson.ts",
"tsserverProjectSystem.ts",
"matchFiles.ts"
"matchFiles.ts",
"initializeTSConfig.ts",
].map(function (f) {
return path.join(unittestsDirectory, f);
})).concat([
Expand Down
96 changes: 96 additions & 0 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,14 @@ namespace ts {
shortOptionNames: Map<string>;
}

/* @internal */
export const defaultInitCompilerOptions: CompilerOptions = {
module: ModuleKind.CommonJS,
target: ScriptTarget.ES5,
noImplicitAny: false,
sourceMap: false,
};

let optionNameMapCache: OptionNameMap;

/* @internal */
Expand Down Expand Up @@ -667,6 +675,94 @@ namespace ts {
}
}

/**
* Generate tsconfig configuration when running command line "--init"
* @param options commandlineOptions to be generated into tsconfig.json
* @param fileNames array of filenames to be generated into tsconfig.json
*/
/* @internal */
export function generateTSConfig(options: CompilerOptions, fileNames: string[]): { compilerOptions: Map<CompilerOptionsValue> } {
const compilerOptions = extend(options, defaultInitCompilerOptions);
const configurations: any = {
compilerOptions: serializeCompilerOptions(compilerOptions)
};
if (fileNames && fileNames.length) {
// only set the files property if we have at least one file
configurations.files = fileNames;
}

return configurations;

function getCustomTypeMapOfCommandLineOption(optionDefinition: CommandLineOption): Map<string | number> | undefined {
if (optionDefinition.type === "string" || optionDefinition.type === "number" || optionDefinition.type === "boolean") {
// this is of a type CommandLineOptionOfPrimitiveType
return undefined;
}
else if (optionDefinition.type === "list") {
return getCustomTypeMapOfCommandLineOption((<CommandLineOptionOfListType>optionDefinition).element);
}
else {
return (<CommandLineOptionOfCustomType>optionDefinition).type;
}
}

function getNameOfCompilerOptionValue(value: CompilerOptionsValue, customTypeMap: MapLike<string | number>): string | undefined {
// There is a typeMap associated with this command-line option so use it to map value back to its name
for (const key in customTypeMap) {
if (customTypeMap[key] === value) {
return key;
}
}
return undefined;
}

function serializeCompilerOptions(options: CompilerOptions): Map<CompilerOptionsValue> {
const result = createMap<CompilerOptionsValue>();
const optionsNameMap = getOptionNameMap().optionNameMap;

for (const name in options) {
if (hasProperty(options, name)) {
// tsconfig only options cannot be specified via command line,
// so we can assume that only types that can appear here string | number | boolean
switch (name) {
case "init":
case "watch":
case "version":
case "help":
case "project":
break;
default:
const value = options[name];
let optionDefinition = optionsNameMap[name.toLowerCase()];
if (optionDefinition) {
const customTypeMap = getCustomTypeMapOfCommandLineOption(optionDefinition);
if (!customTypeMap) {
// There is no map associated with this compiler option then use the value as-is
// This is the case if the value is expect to be string, number, boolean or list of string
result[name] = value;
}
else {
if (optionDefinition.type === "list") {
const convertedValue: string[] = [];
for (const element of value as (string | number)[]) {
convertedValue.push(getNameOfCompilerOptionValue(element, customTypeMap));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

customTypeMap is going to be undefined here for rootDirs, and i believe this will cause getNameOfCompilerOptionValue to throw.

Copy link
Contributor Author

@yuit yuit Aug 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it actually won't be because for rootDirs will have customTypeMap be undefined (as getCustomTypeMapOfCommandLineOption recursively call when you have "list" type and it will then return undefined) so it will already be catched here

The reason we still have check for type of "list" here is that we will know whether to iterate over input or not

Unrelated but your comment reminds me is to add tests for invalid option and make sure we generate something that make sense. Should error in the cases of incorrect input and generate results? Currently we will just ignore those incorrect inputs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it actually won't be because for rootDirs will have customTypeMap be undefined

i am a bit confused. the type of rootDirs is list, then we call getCustomTypeMapOfCommandLineOption with the element, the next time around the type of the element is string, so we return undefined. we can talk in person if you have time.

}
result[name] = convertedValue;
}
else {
// There is a typeMap associated with this command-line option so use it to map value back to its name
result[name] = getNameOfCompilerOptionValue(value, customTypeMap);
}
}
}
break;
}
}
}
return result;
}
}

/**
* Remove the comments from a json like text.
* Comments can be single line comments (starting with # or //) or multiline comments using / * * /
Expand Down
8 changes: 0 additions & 8 deletions src/compiler/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -828,14 +828,6 @@ namespace ts {
: { resolvedModule: undefined, failedLookupLocations };
}

/* @internal */
export const defaultInitCompilerOptions: CompilerOptions = {
module: ModuleKind.CommonJS,
target: ScriptTarget.ES5,
noImplicitAny: false,
sourceMap: false,
};

interface OutputFingerprint {
hash: string;
byteOrderMark: boolean;
Expand Down
58 changes: 1 addition & 57 deletions src/compiler/tsc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -763,67 +763,11 @@ namespace ts {
reportDiagnostic(createCompilerDiagnostic(Diagnostics.A_tsconfig_json_file_is_already_defined_at_Colon_0, file), /* host */ undefined);
}
else {
const compilerOptions = extend(options, defaultInitCompilerOptions);
const configurations: any = {
compilerOptions: serializeCompilerOptions(compilerOptions)
};

if (fileNames && fileNames.length) {
// only set the files property if we have at least one file
configurations.files = fileNames;
}
else {
configurations.exclude = ["node_modules"];
if (compilerOptions.outDir) {
configurations.exclude.push(compilerOptions.outDir);
}
}

sys.writeFile(file, JSON.stringify(configurations, undefined, 4));
sys.writeFile(file, JSON.stringify(generateTSConfig(options, fileNames), undefined, 4));
reportDiagnostic(createCompilerDiagnostic(Diagnostics.Successfully_created_a_tsconfig_json_file), /* host */ undefined);
}

return;

function serializeCompilerOptions(options: CompilerOptions): Map<string | number | boolean> {
const result = createMap<string | number | boolean>();
const optionsNameMap = getOptionNameMap().optionNameMap;

for (const name in options) {
if (hasProperty(options, name)) {
// tsconfig only options cannot be specified via command line,
// so we can assume that only types that can appear here string | number | boolean
const value = <string | number | boolean>options[name];
switch (name) {
case "init":
case "watch":
case "version":
case "help":
case "project":
break;
default:
let optionDefinition = optionsNameMap[name.toLowerCase()];
if (optionDefinition) {
if (typeof optionDefinition.type === "string") {
// string, number or boolean
result[name] = value;
}
else {
// Enum
const typeMap = <Map<number>>optionDefinition.type;
for (const key in typeMap) {
if (typeMap[key] === value) {
result[name] = key;
}
}
}
}
break;
}
}
}
return result;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2760,7 +2760,7 @@ namespace ts {

/* @internal */
export interface CommandLineOptionOfCustomType extends CommandLineOptionBase {
type: Map<number | string>; // an object literal mapping named values to actual values
type: Map<number | string>; // an object literal mapping named values to actual values
}

/* @internal */
Expand Down
3 changes: 2 additions & 1 deletion src/harness/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
"./unittests/convertCompilerOptionsFromJson.ts",
"./unittests/convertTypingOptionsFromJson.ts",
"./unittests/tsserverProjectSystem.ts",
"./unittests/matchFiles.ts"
"./unittests/matchFiles.ts",
"./unittests/initializeTSConfig.ts"
]
}
44 changes: 44 additions & 0 deletions src/harness/unittests/initializeTSConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/// <reference path="..\harness.ts" />
/// <reference path="..\..\compiler\commandLineParser.ts" />

namespace ts {
describe("initTSConfig", () => {
function initTSConfigCorrectly(name: string, commandLinesArgs: string[]) {
describe(name, () => {
const commandLine = parseCommandLine(commandLinesArgs);
const initResult = generateTSConfig(commandLine.options, commandLine.fileNames);
const outputFileName = `tsConfig/${name.replace(/[^a-z0-9\-. ]/ig, "")}/tsconfig.json`;

it(`Correct output for ${outputFileName}`, () => {
Harness.Baseline.runBaseline("Correct output", outputFileName, () => {
if (initResult) {
return JSON.stringify(initResult, undefined, 4);
}
else {
// This can happen if compiler recieve invalid compiler-options
/* tslint:disable:no-null-keyword */
return null;
/* tslint:enable:no-null-keyword */
}
});
});
});
}

initTSConfigCorrectly("Default initialized TSConfig", ["--init"]);

initTSConfigCorrectly("Initialized TSConfig with files options", ["--init", "file0.st", "file1.ts", "file2.ts"]);

initTSConfigCorrectly("Initialized TSConfig with boolean value compiler options", ["--init", "--noUnusedLocals"]);

initTSConfigCorrectly("Initialized TSConfig with enum value compiler options", ["--init", "--target", "es5", "--jsx", "react"]);

initTSConfigCorrectly("Initialized TSConfig with list compiler options", ["--init", "--types", "jquery,mocha"]);

initTSConfigCorrectly("Initialized TSConfig with list compiler options with enum value", ["--init", "--lib", "es5,es2015.core"]);

initTSConfigCorrectly("Initialized TSConfig with incorrect compiler option", ["--init", "--someNonExistOption"]);

initTSConfigCorrectly("Initialized TSConfig with incorrect compiler option value", ["--init", "--lib", "nonExistLib,es5,es2015.promise"]);
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"noImplicitAny": false,
"sourceMap": false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"noImplicitAny": false,
"sourceMap": false,
"noUnusedLocals": true
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"noImplicitAny": false,
"sourceMap": false,
"jsx": "react"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"noImplicitAny": false,
"sourceMap": false
},
"files": [
"file0.st",
"file1.ts",
"file2.ts"
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"noImplicitAny": false,
"sourceMap": false,
"lib": [
"es5",
"es2015.promise"
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"noImplicitAny": false,
"sourceMap": false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"noImplicitAny": false,
"sourceMap": false,
"lib": [
"es5",
"es2015.core"
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"compilerOptions": {
"module": "commonjs",
"target": "es5",
"noImplicitAny": false,
"sourceMap": false,
"types": [
"jquery",
"mocha"
]
}
}