Skip to content

Opt-in Atom editor conversion #728

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.

Sign up for GitHub

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 6 commits into from
Oct 17, 2020
Merged

Opt-in Atom editor conversion #728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8b84a3d
WIP: Atom editor conversion
JoshuaKGoldberg Oct 2, 2020
b6affc8
Continue refactor: merging, and reporting updates
JoshuaKGoldberg Oct 13, 2020
1d5106f
Added back missing reporting
JoshuaKGoldberg Oct 15, 2020
1c20b2e
Use a more tolerant endsWith
JoshuaKGoldberg Oct 15, 2020
b470605
Merge branch 'main'
JoshuaKGoldberg Oct 15, 2020
459981c
Merge branch 'main'
JoshuaKGoldberg Oct 17, 2020
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
26 changes: 22 additions & 4 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
"dependencies": {
"chalk": "4.1.0",
"commander": "6.1.0",
"cson-parser": "^4.0.5",
Copy link

@skovalyov skovalyov Nov 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I get an error npm ERR! 404 Not Found - GET http://my-registry-url/coffeescript/-/coffeescript-1.12.8.tgz during installation on Windows. It comes from cson-parser. There are some mentions of this problem, for example at jashkenas/coffeescript/issues/4805, but it's not clear how to fix it while installing Angular Eslint with ng add @angular-eslint/schematics

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello, can you open a new issue in order to track the error with Angular ESLint + this package? Thank you :)

Copy link

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you, we will look into these and provide a proper fix soon.

"eslint-config-prettier": "6.13.0",
"glob": "7.1.6",
"lodash": "^4.17.20",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Psss, you could have used lodash-es which is tree-shakable hence reduces bundle size. 🍸

Copy link
Member Author

Choose a reason for hiding this comment

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

Aha! I can never keep track of which Lodash is the right one to use now 😄. Will do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, https://www.npmjs.com/package/lodash-es is a year out of date 😕 ... I'll skip this for now, but if it ends up getting regularly published it'd be great to include.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh fun, this is an actively discussed topic: https://github.com/lodash/lodash/issues/4879

"minimatch": "3.0.4",
"strip-json-comments": "3.1.1",
"tslint": "6.1.3",
Expand All @@ -28,6 +30,7 @@
"@types/eslint-config-prettier": "6.11.0",
"@types/glob": "7.1.3",
"@types/jest": "26.0.14",
"@types/lodash": "^4.14.162",
"@types/minimatch": "3.0.3",
"@types/node": "12.12.21",
"@typescript-eslint/eslint-plugin": "4.4.1",
Expand Down
63 changes: 27 additions & 36 deletions src/cli/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,7 @@ import {
ReportCommentResultsDependencies,
reportCommentResults,
} from "../converters/comments/reporting/reportCommentResults";
import {
ConvertEditorConfigDependencies,
convertEditorConfig,
} from "../converters/editorConfigs/convertEditorConfig";
import {
ConvertEditorSettingsDependencies,
convertEditorSettings,
} from "../converters/editorConfigs/convertEditorSettings";
import { editorSettingsConverters } from "../converters/editorConfigs/editorSettingsConverters";
import { reportEditorSettingConversionResults } from "../converters/editorConfigs/reporting/reportEditorSettingConversionResults";
import { convertEditorConfig } from "../converters/editorConfigs/convertEditorConfig";
import {
ConvertLintConfigDependencies,
convertLintConfig,
Expand All @@ -44,6 +35,14 @@ import {
ConvertFileCommentsDependencies,
convertFileComments,
} from "../converters/comments/convertFileComments";
import {
convertEditorConfigs,
ConvertEditorConfigsDependencies,
} from "../converters/editorConfigs/convertEditorConfigs";
import { convertAtomConfig } from "../converters/editorConfigs/converters/convertAtomConfig";
import { convertVSCodeConfig } from "../converters/editorConfigs/converters/convertVSCodeConfig";
import { reportEditorConfigConversionResults } from "../converters/editorConfigs/reporting/reportEditorConfigConversionResults";
import { EditorConfigDescriptor } from "../converters/editorConfigs/types";
import {
ConvertRulesDependencies,
convertRules,
Expand All @@ -67,17 +66,12 @@ import {
} from "../converters/lintConfigs/reporting/packages/logMissingPackages";
import { runCli, RunCliDependencies } from "./runCli";
import { ruleMergers } from "../converters/lintConfigs/rules/ruleMergers";
import { writeEditorConfigConversionResults } from "../converters/lintConfigs/writeEditorConfigConversionResults";
import { checkPrettierExtension } from "../converters/lintConfigs/summarization/prettier/checkPrettierExtension";
import { removeExtendsDuplicatedRules } from "../converters/lintConfigs/pruning/removeExtendsDuplicatedRules";
import {
ExtractGlobPathsDependencies,
extractGlobPaths,
} from "../converters/comments/extractGlobPaths";
import {
findEditorConfiguration,
FindEditorConfigurationDependencies,
} from "../input/findEditorConfiguration";
import { findESLintConfiguration } from "../input/findESLintConfiguration";
import {
findOriginalConfigurations,
Expand All @@ -103,10 +97,6 @@ const convertRulesDependencies: ConvertRulesDependencies = {
ruleMergers,
};

const convertEditorSettingsDependencies: ConvertEditorSettingsDependencies = {
converters: editorSettingsConverters,
};

const nativeImporterDependencies: ImporterDependencies = {
fileSystem: fsFileSystem,
getCwd: () => process.cwd(),
Expand All @@ -120,11 +110,6 @@ const findConfigurationDependencies = {
importer: boundImporter,
};

const findEditorConfigurationDependencies: FindEditorConfigurationDependencies = {
fileSystem: fsFileSystem,
importer: boundImporter,
};

const findOriginalConfigurationsDependencies: FindOriginalConfigurationsDependencies = {
findESLintConfiguration: bind(findESLintConfiguration, findConfigurationDependencies),
findPackagesConfiguration: bind(findPackagesConfiguration, findConfigurationDependencies),
Expand Down Expand Up @@ -175,20 +160,26 @@ const writeConversionResultsDependencies: WriteConversionResultsDependencies = {
fileSystem: fsFileSystem,
};

const reportEditorSettingConversionResultsDependencies = {
const editorConfigDescriptors: EditorConfigDescriptor[] = [
[".atom/config.cson", convertAtomConfig],
[".vscode/settings.json", convertVSCodeConfig],
];

const convertEditorConfigDependencies = {
editorConfigDescriptors,
fileSystem: fsFileSystem,
};

const reportEditorConfigConversionResultsDependencies = {
logger: processLogger,
};

const convertEditorConfigDependencies: ConvertEditorConfigDependencies = {
findEditorConfiguration: bind(findEditorConfiguration, findEditorConfigurationDependencies),
convertEditorSettings: bind(convertEditorSettings, convertEditorSettingsDependencies),
reportEditorSettingConversionResults: bind(
reportEditorSettingConversionResults,
reportEditorSettingConversionResultsDependencies,
),
writeEditorConfigConversionResults: bind(
writeEditorConfigConversionResults,
writeConversionResultsDependencies,
const convertEditorConfigsDependencies: ConvertEditorConfigsDependencies = {
convertEditorConfig: bind(convertEditorConfig, convertEditorConfigDependencies),
editorConfigDescriptors,
reportEditorConfigConversionResults: bind(
reportEditorConfigConversionResults,
reportEditorConfigConversionResultsDependencies,
),
};

Expand All @@ -209,7 +200,7 @@ const convertLintConfigDependencies: ConvertLintConfigDependencies = {
const runCliDependencies: RunCliDependencies = {
converters: [
bind(convertLintConfig, convertLintConfigDependencies),
bind(convertEditorConfig, convertEditorConfigDependencies),
bind(convertEditorConfigs, convertEditorConfigsDependencies),
bind(convertComments, convertCommentsDependencies),
],
findOriginalConfigurations: bind(
Expand Down
146 changes: 39 additions & 107 deletions src/converters/editorConfigs/convertEditorConfig.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,137 +1,69 @@
import { ResultStatus, FailedResult } from "../../types";
import { convertEditorConfig, ConvertEditorConfigDependencies } from "./convertEditorConfig";
import { createEmptyEditorSettingConversionResults } from "./editorConversionResults.stubs";
import { EditorSetting } from "./types";
import { convertEditorConfig } from "./convertEditorConfig";

const stubPath = "./vscode/settings.json";

const stubSettings = {
config: "./eslintrc.js",
editor: "./my-editor/settings.json",
config: ".eslintrc.js",
};

const createStubDependencies = (
overrides: Partial<ConvertEditorConfigDependencies> = {},
): ConvertEditorConfigDependencies => ({
convertEditorSettings: jest.fn(),
findEditorConfiguration: jest.fn().mockResolvedValue({}),
reportEditorSettingConversionResults: jest.fn(),
writeEditorConfigConversionResults: jest.fn().mockReturnValue(Promise.resolve()),
...overrides,
});

describe("convertEditorConfig", () => {
it("returns a success result when there is no original configuration", async () => {
// Arrange
const dependencies = createStubDependencies({
findEditorConfiguration: async () => undefined,
});

// Act
const result = await convertEditorConfig(dependencies, stubSettings);

// Assert
expect(result).toEqual({
status: ResultStatus.Succeeded,
});
});

it("returns the failure result when finding the original configurations fails", async () => {
it("returns an error when reading the file fails", async () => {
// Arrange
const error = new Error();
const findError: FailedResult = {
errors: [error],
status: ResultStatus.Failed,
const error = new Error("Oh no");
const dependencies = {
fileSystem: {
readFile: jest.fn().mockResolvedValue(error),
writeFile: jest.fn(),
},
};

const dependencies = createStubDependencies({
findEditorConfiguration: async () => ({
configPath: "",
result: error,
}),
});

// Act
const result = await convertEditorConfig(dependencies, stubSettings);
const result = await convertEditorConfig(dependencies, jest.fn(), stubPath, stubSettings);

// Assert
expect(result).toEqual(findError);
expect(result).toEqual(error);
});

it("returns the failure result when writing to the configuration file fails", async () => {
it("returns an error when writing to a file fails", async () => {
// Arrange
const fileWriteError = new Error();
const dependencies = createStubDependencies({
writeEditorConfigConversionResults: jest.fn().mockResolvedValueOnce(fileWriteError),
const originalFileContents = "Hello";
const error = new Error("Oh no!");
const converter = (input: string) => ({
contents: `${input} world!`,
missing: [],
});

// Act
const result = await convertEditorConfig(dependencies, stubSettings);

// Assert
expect(result).toEqual({
errors: [fileWriteError],
status: ResultStatus.Failed,
});
});

it("converts conversion results when finding the original configurations succeeds", async () => {
// Arrange
const originalConfig = {
"typescript.tsdk": "node_modules/typescript/lib",
const dependencies = {
fileSystem: {
readFile: jest.fn().mockResolvedValue(originalFileContents),
writeFile: jest.fn().mockResolvedValue(error),
},
};

const dependencies = createStubDependencies({
findEditorConfiguration: jest.fn().mockResolvedValue({
result: originalConfig,
}),
});

// Act
await convertEditorConfig(dependencies, stubSettings);
const result = await convertEditorConfig(dependencies, converter, stubPath, stubSettings);

// Assert
expect(dependencies.convertEditorSettings).toHaveBeenCalledWith(
originalConfig,
stubSettings,
);
expect(result).toEqual(error);
});

it("reports conversion results when settings are converted successfully", async () => {
it("returns the conversion data when writing to a file succeeds", async () => {
// Arrange
const conversionResults = createEmptyEditorSettingConversionResults({
converted: new Map<string, EditorSetting>([
[
"tslint-editor-setting-one",
{
editorSettingName: "tslint-editor-setting-one",
value: 42,
},
],
]),
});

const dependencies = createStubDependencies({
convertEditorSettings: jest.fn().mockReturnValue(conversionResults),
const originalFileContents = "Hello";
const converter = (input: string) => ({
contents: `${input} world!`,
missing: [],
});
const dependencies = {
fileSystem: {
readFile: jest.fn().mockResolvedValue(originalFileContents),
writeFile: jest.fn().mockResolvedValue(undefined),
},
};

// Act
await convertEditorConfig(dependencies, stubSettings);

// Assert
expect(dependencies.reportEditorSettingConversionResults).toHaveBeenCalledWith(
conversionResults,
);
});

it("returns a successful result when finding the original configurations succeeds", async () => {
// Arrange
const dependencies = createStubDependencies();

// Act
const result = await convertEditorConfig(dependencies, stubSettings);
const result = await convertEditorConfig(dependencies, converter, stubPath, stubSettings);

// Assert
expect(result).toEqual({
status: ResultStatus.Succeeded,
});
expect(result).toEqual(converter(originalFileContents));
});
});
Loading