Skip to content

improve type safety with regards to config files #601

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 2 commits into from
Jun 1, 2020
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
14 changes: 14 additions & 0 deletions src/creation/eslint/createEnv.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,20 @@ describe("createEnv", () => {
});
});

it("handles an environment where typescript configuration files are mostly undefined", () => {
// Arrange
const packages = undefined;
const typescript = {};

// Act
const env = createEnv({ packages, typescript });

// Assert
expect(env).not.toContain({
browser: expect.any(Boolean),
});
});

it("returns browser as true if a typescript lib is dom", () => {
// Arrange
const packages = undefined;
Expand Down
4 changes: 2 additions & 2 deletions src/creation/eslint/createEnv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ export const createEnv = ({
}: Pick<AllOriginalConfigurations, "packages" | "typescript">) => {
const browser =
typescript === undefined ||
typescript.compilerOptions.lib === undefined ||
typescript.compilerOptions?.lib === undefined ||
typescript.compilerOptions.lib.includes("dom");

const es6 =
typescript === undefined ||
!["es3", "es5"].includes(typescript.compilerOptions.target.toLowerCase());
!["es3", "es5"].includes(typescript.compilerOptions?.target?.toLowerCase() ?? "");

const node =
packages === undefined ||
Expand Down
10 changes: 5 additions & 5 deletions src/input/findESLintConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import { OriginalConfigurations } from "./findOriginalConfigurations";
import { importer } from "./importer";

export type ESLintConfiguration = {
env: Record<string, boolean>;
extends: string | string[];
globals?: Record<string, boolean>;
rules: ESLintConfigurationRules;
env?: Record<string, boolean | undefined>;
extends?: string | string[];
globals?: Record<string, boolean | undefined>;
rules?: ESLintConfigurationRules;
};

export type ESLintConfigurationRules = {
Expand Down Expand Up @@ -43,7 +43,7 @@ export const findESLintConfiguration = async (
findRawConfiguration<ESLintConfiguration>(dependencies.importer, filePath, {
extends: [],
}),
findReportedConfiguration<Partial<ESLintConfiguration>>(
findReportedConfiguration<ESLintConfiguration>(
dependencies.exec,
"eslint --print-config",
filePath,
Expand Down
3 changes: 2 additions & 1 deletion src/input/findOriginalConfigurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from "./findTypeScriptConfiguration";
import { findTSLintConfiguration, TSLintConfiguration } from "./findTSLintConfiguration";
import { mergeLintConfigurations } from "./mergeLintConfigurations";
import { DeepPartial } from "./findReportedConfiguration";

export type FindOriginalConfigurationsDependencies = {
findESLintConfiguration: SansDependencies<typeof findESLintConfiguration>;
Expand All @@ -35,7 +36,7 @@ export type OriginalConfigurations<Configuration> = {
/**
* Raw import results from `import`ing the configuration file.
*/
raw: Partial<Configuration>;
raw: DeepPartial<Configuration>;
};

export type AllOriginalConfigurations = {
Expand Down
4 changes: 2 additions & 2 deletions src/input/findPackagesConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {
} from "./findReportedConfiguration";

export type PackagesConfiguration = {
dependencies: Record<string, string>;
devDependencies: Record<string, string>;
dependencies: Record<string, string | undefined>;
devDependencies: Record<string, string | undefined>;
};

export const findPackagesConfiguration = async (
Expand Down
2 changes: 1 addition & 1 deletion src/input/findReportedConfiguration.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Exec } from "../adapters/exec";

export type DeepPartial<T> = {
[P in keyof T]?: T[P] extends Record<P, T[P]> ? DeepPartial<T[P]> : T[P];
[P in keyof T]?: T[P] extends Record<string, unknown> ? DeepPartial<T[P]> : T[P];
};

export type FindReportedConfigurationDependencies = {
Expand Down
2 changes: 1 addition & 1 deletion src/input/findTSLintConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { importer } from "./importer";
export type TSLintConfiguration = {
extends?: string[];
rulesDirectory?: string[];
rules: TSLintConfigurationRules;
rules?: TSLintConfigurationRules;
};

export type TSLintConfigurationRules = Record<string, any>;
Expand Down
4 changes: 2 additions & 2 deletions src/input/findTypeScriptConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ import {
} from "./findReportedConfiguration";

export type TypeScriptConfiguration = {
compilerOptions: {
compilerOptions?: {
lib?: string[];
target: string;
target?: string;
};
};

Expand Down
2 changes: 1 addition & 1 deletion src/input/mergeLintConfigurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const mergeLintConfigurations = (
return tslint;
}

const mappedConfig = eslint.full.rules["@typescript-eslint/tslint/config"];
const mappedConfig = eslint.full.rules?.["@typescript-eslint/tslint/config"];
if (!(mappedConfig instanceof Array) || mappedConfig[0] === "off") {
return tslint;
}
Expand Down
13 changes: 13 additions & 0 deletions src/rules/convertRules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ import { RuleConverter, ConversionResult } from "./converter";
import { RuleMerger } from "./merger";

describe("convertRules", () => {
it("doesn't crash when passed an undefined configuration", () => {
// Arrange
const { converters, mergers } = setupConversionEnvironment({
ruleSeverity: "off",
});

// Act
const { missing } = convertRules({ converters, mergers }, undefined);

// Assert
expect(missing).toEqual([]);
});

it("doesn't marks a disabled rule as missing when its converter returns undefined", () => {
// Arrange
const { tslintRule, converters, mergers } = setupConversionEnvironment({
Expand Down
87 changes: 44 additions & 43 deletions src/rules/convertRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,66 +22,67 @@ export type RuleConversionResults = {

export const convertRules = (
dependencies: ConvertRulesDependencies,
rawTslintRules: TSLintConfigurationRules,
rawTslintRules?: TSLintConfigurationRules,
): RuleConversionResults => {
const converted = new Map<string, ESLintRuleOptions>();
const failed: ConversionError[] = [];
const missing: TSLintRuleOptions[] = [];
const plugins = new Set<string>();

for (const [ruleName, value] of Object.entries(rawTslintRules)) {
const tslintRule = formatRawTslintRule(ruleName, value);
const conversion = convertRule(tslintRule, dependencies.converters);
if (rawTslintRules !== undefined) {
for (const [ruleName, value] of Object.entries(rawTslintRules)) {
const tslintRule = formatRawTslintRule(ruleName, value);
const conversion = convertRule(tslintRule, dependencies.converters);

if (conversion === undefined) {
if (tslintRule.ruleSeverity !== "off") {
missing.push(tslintRule);
}

continue;
}
if (conversion === undefined) {
if (tslintRule.ruleSeverity !== "off") {
missing.push(tslintRule);
}

if (conversion instanceof ConversionError) {
failed.push(conversion);
continue;
}

for (const changes of conversion.rules) {
const existingConversion = converted.get(changes.ruleName);
const newConversion = {
...changes,
ruleSeverity: convertTSLintRuleSeverity(tslintRule.ruleSeverity),
};
continue;
}

if (existingConversion === undefined) {
converted.set(changes.ruleName, newConversion);
if (conversion instanceof ConversionError) {
failed.push(conversion);
continue;
}

const merger = dependencies.mergers.get(changes.ruleName);
if (merger === undefined) {
failed.push(ConversionError.forMerger(changes.ruleName));
} else {
const existingNotices = existingConversion.notices ?? [];
const newNotices = newConversion.notices ?? [];
for (const changes of conversion.rules) {
const existingConversion = converted.get(changes.ruleName);
const newConversion = {
...changes,
ruleSeverity: convertTSLintRuleSeverity(tslintRule.ruleSeverity),
};

if (existingConversion === undefined) {
converted.set(changes.ruleName, newConversion);
continue;
}

const merger = dependencies.mergers.get(changes.ruleName);
if (merger === undefined) {
failed.push(ConversionError.forMerger(changes.ruleName));
} else {
const existingNotices = existingConversion.notices ?? [];
const newNotices = newConversion.notices ?? [];

converted.set(changes.ruleName, {
...existingConversion,
ruleArguments: merger(
existingConversion.ruleArguments,
newConversion.ruleArguments,
),
notices: Array.from(new Set([...existingNotices, ...newNotices])),
});
converted.set(changes.ruleName, {
...existingConversion,
ruleArguments: merger(
existingConversion.ruleArguments,
newConversion.ruleArguments,
),
notices: Array.from(new Set([...existingNotices, ...newNotices])),
});
}
}
}

if (conversion.plugins !== undefined) {
for (const newPlugin of conversion.plugins) {
plugins.add(newPlugin);
if (conversion.plugins !== undefined) {
for (const newPlugin of conversion.plugins) {
plugins.add(newPlugin);
}
}
}
}

return { converted, failed, missing, plugins };
};