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

NEW(eslint): @W-17495039@: Allow users to specify other file extensions tha… #181

Merged
merged 1 commit into from
Dec 27, 2024
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
63 changes: 37 additions & 26 deletions packages/code-analyzer-eslint-engine/src/base-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,36 +53,24 @@ export class LegacyBaseConfigFactory {
}

private createJavascriptConfig(baseRuleset: BaseRuleset): Linter.ConfigOverride {
return {
const jsConfig: Linter.ConfigOverride = {
files: this.config.file_extensions.javascript.map(ext => `*${ext}`),
extends: [`eslint:${baseRuleset}`]
}
return this.addJavascriptParser(jsConfig);
Copy link
Collaborator Author

@stephen-carter-at-sf stephen-carter-at-sf Dec 23, 2024

Choose a reason for hiding this comment

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

[Fix as you go item] I realized that the base config needed a parser while doing some addition testing on just the base rules. I just assumed that the default would just work out of the box. I never caught this before because the LWC rules were always pulling in the parser. So this only comes up when you disable the lwc rules but keep the base rules enabled.

}

private createLwcConfig(): Linter.ConfigOverride {
return {
const lwcConfig: Linter.ConfigOverride = {
files: this.config.file_extensions.javascript.map(ext => `*${ext}`),
extends: [
"@salesforce/eslint-config-lwc/base" // Always using base for now. all and recommended both require additional plugins
],
plugins: [
"@lwc/eslint-plugin-lwc"
],
parser: "@babel/eslint-parser",
parserOptions: {
requireConfigFile: false,
babelOptions: {
babelrc: false,
configFile: false,
parserOpts: {
plugins: [
"classProperties",
["decorators", { "decoratorsBeforeExport": false }]
]
}
}
}
]
}
return this.addJavascriptParser(lwcConfig);
}

private createJavascriptPlusLwcConfig(baseRuleset: BaseRuleset): Linter.ConfigOverride {
Expand All @@ -92,23 +80,46 @@ export class LegacyBaseConfigFactory {
}

private createTypescriptConfig(baseRuleset: BaseRuleset): Linter.ConfigOverride {
return {
const tsConfig: Linter.ConfigOverride = {
files: this.config.file_extensions.typescript.map(ext => `*${ext}`),
extends: [
`eslint:${baseRuleset}`, // The typescript plugin applies the base rules to the typescript files, so we want this
`plugin:@typescript-eslint/${baseRuleset}`, // May override some rules from eslint:<all|recommended> as needed
],
plugins: [
"@typescript-eslint"
],
parser: '@typescript-eslint/parser',
parserOptions: {
// Finds the tsconfig.json file nearest to each source file. This should work for most users.
// If not, then we may consider letting user specify this via config or alternatively, users can just
// set disable_typescript_base_config=true and configure typescript in their own eslint config file.
// See https://typescript-eslint.io/packages/parser/#project
project: true
]
};
return this.addTypescriptParser(tsConfig);
}

private addJavascriptParser(config: Linter.ConfigOverride): Linter.ConfigOverride {
config.parser = "@babel/eslint-parser";
config.parserOptions = {
requireConfigFile: false,
babelOptions: {
babelrc: false,
configFile: false,
parserOpts: {
plugins: [
"classProperties",
["decorators", {"decoratorsBeforeExport": false}]
]
}
}
};
return config;
}

private addTypescriptParser(config: Linter.ConfigOverride): Linter.ConfigOverride {
config.parser = '@typescript-eslint/parser';
config.parserOptions = {
// Finds the tsconfig.json file nearest to each source file. This should work for most users.
// If not, then we may consider letting user specify this via config or alternatively, users can just
// set disable_typescript_base_config=true and configure typescript in their own eslint config file.
// See https://typescript-eslint.io/packages/parser/#project
project: true
}
return config;
}
}
19 changes: 11 additions & 8 deletions packages/code-analyzer-eslint-engine/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ export type ESLintEngineConfig = {
// Default: false
disable_typescript_base_config: boolean

// Extensions of the files in your workspace that will be used to discover rules for javascript and typescript.
// Each file extension can only be associated to one language. If a specific language is not specified, then the
// following list of default file extensions will be used:
// javascript: ['.js', '.cjs', '.mjs']
// typescript: ['.ts']
// Extensions of the files in your workspace that will be used to discover rules.
// To associate file extensions to the standard ESLint JavaScript rules, LWC rules, or custom JavaScript-based
// rules, add them under the 'javascript' language. To associate file extensions to the standard TypeScript
// rules or custom TypeScript-based rules, add them under the 'typescript' language. To allow for the
// discovery of custom rules that are associated with any other language, then add the associated file
// extensions under the 'other' language.
file_extensions: FileExtensionsObject

// (INTERNAL USE ONLY) Copy of the code analyzer config root.
Expand All @@ -46,7 +47,8 @@ export type ESLintEngineConfig = {

type FileExtensionsObject = {
javascript: string[],
typescript: string[]
typescript: string[],
other: string[]
};

export const DEFAULT_CONFIG: ESLintEngineConfig = {
Expand All @@ -58,7 +60,8 @@ export const DEFAULT_CONFIG: ESLintEngineConfig = {
disable_typescript_base_config: false,
file_extensions: {
javascript: ['.js', '.cjs', '.mjs'],
typescript: ['.ts']
typescript: ['.ts'],
other: []
},
config_root: process.cwd() // INTERNAL USE ONLY
}
Expand Down Expand Up @@ -169,7 +172,7 @@ class ESLintEngineConfigValueExtractor {
(element, elementFieldPath) => ValueValidator.validateString(element,
elementFieldPath, ESLintEngineConfigValueExtractor.FILE_EXT_PATTERN),
DEFAULT_CONFIG.file_extensions[language as keyof FileExtensionsObject]
)!).map(fileExt => fileExt.toLowerCase());
)!.map(fileExt => fileExt.toLowerCase()));

// Validate that none of the file extensions already exist in another language
for (const fileExt of fileExts) {
Expand Down
17 changes: 9 additions & 8 deletions packages/code-analyzer-eslint-engine/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,21 @@ const MESSAGE_CATALOG : { [key: string]: string } = {
`Whether to have Code Analyzer automatically discover/apply any ESLint configuration and ignore files from your workspace.`,

ConfigFieldDescription_disable_javascript_base_config:
`Whether to turn off the default base configuration that supplies the standard ESLint rules for javascript files.`,
`Whether to turn off the default base configuration that supplies the standard ESLint rules for JavaScript files.`,

ConfigFieldDescription_disable_lwc_base_config:
`Whether to turn off the default base configuration that supplies the LWC rules for javascript files.`,
`Whether to turn off the default base configuration that supplies the LWC rules for JavaScript files.`,

ConfigFieldDescription_disable_typescript_base_config:
`Whether to turn off the default base configuration that supplies the standard rules for typescript files.`,
`Whether to turn off the default base configuration that supplies the standard rules for TypeScript files.`,

ConfigFieldDescription_file_extensions:
`Extensions of the files in your workspace that will be used to discover rules for javascript and typescript.\n` +
`Each file extension can only be associated to one language. If a specific language is not specified, then the\n` +
`following list of default file extensions will be used:\n` +
` javascript: ['.js', '.cjs', '.mjs']\n` +
` typescript: ['.ts']`,
`Extensions of the files in your workspace that will be used to discover rules.\n` +
`To associate file extensions to the standard ESLint JavaScript rules, LWC rules, or custom JavaScript-based\n` +
`rules, add them under the 'javascript' language. To associate file extensions to the standard TypeScript\n` +
`rules or custom TypeScript-based rules, add them under the 'typescript' language. To allow for the\n` +
`discovery of custom rules that are associated with any other language, then add the associated file\n` +
`extensions under the 'other' language.`,

UnsupportedEngineName:
`The ESLintEnginePlugin does not support an engine with name '%s'.`,
Expand Down
13 changes: 10 additions & 3 deletions packages/code-analyzer-eslint-engine/src/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ export class MissingESLintWorkspace implements ESLintWorkspace {
async getCandidateFilesForBaseConfig(_filterFcn: AsyncFilterFnc<string>): Promise<string[]> {
return createPlaceholderCandidateFiles([
... this.config.file_extensions.javascript,
... this.config.file_extensions.typescript],
... this.config.file_extensions.typescript,
... this.config.file_extensions.other],
this.config.config_root
)
}
Expand All @@ -87,6 +88,7 @@ export class MissingESLintWorkspace implements ESLintWorkspace {
type FilesOfInterest = {
javascriptFiles: string[]
typescriptFiles: string[]
otherFiles: string[]
}

export class PresentESLintWorkspace implements ESLintWorkspace {
Expand All @@ -102,7 +104,9 @@ export class PresentESLintWorkspace implements ESLintWorkspace {

async getFilesToScan(filterFcn: AsyncFilterFnc<string>): Promise<string[]> {
const filesOfInterest: FilesOfInterest = await this.getFilesOfInterest(filterFcn);
return filesOfInterest.javascriptFiles.concat(filesOfInterest.typescriptFiles);
return filesOfInterest.javascriptFiles
.concat(filesOfInterest.typescriptFiles)
.concat(filesOfInterest.otherFiles);
}

async getCandidateFilesForUserConfig(filterFcn: AsyncFilterFnc<string>): Promise<string[]> {
Expand Down Expand Up @@ -140,17 +144,20 @@ export class PresentESLintWorkspace implements ESLintWorkspace {
return this.filesOfInterest;
}

this.filesOfInterest = { javascriptFiles: [], typescriptFiles: [] };
this.filesOfInterest = { javascriptFiles: [], typescriptFiles: [], otherFiles: [] };
for (const file of await this.delegateWorkspace.getExpandedFiles()) {
const fileExt = path.extname(file).toLowerCase();
if (this.config.file_extensions.javascript.includes(fileExt)) {
this.filesOfInterest.javascriptFiles.push(file);
} else if (this.config.file_extensions.typescript.includes(fileExt)) {
this.filesOfInterest.typescriptFiles.push(file);
} else if (this.config.file_extensions.other.includes(fileExt)) {
this.filesOfInterest.otherFiles.push(file);
}
}
this.filesOfInterest.javascriptFiles = await filterAsync(this.filesOfInterest.javascriptFiles, filterFcn);
this.filesOfInterest.typescriptFiles = await filterAsync(this.filesOfInterest.typescriptFiles, filterFcn);
this.filesOfInterest.otherFiles = await filterAsync(this.filesOfInterest.otherFiles, filterFcn);

return this.filesOfInterest;
}
Expand Down
63 changes: 63 additions & 0 deletions packages/code-analyzer-eslint-engine/test/engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,7 @@ describe('Tests for the describeRules method of ESLintEngine', () => {
it('When file_extensions.javascript and file_extensions.typescript are both empty, then no rules are returned', async () => {
const engine: ESLintEngine = new ESLintEngine({...DEFAULT_CONFIG,
file_extensions: {
...DEFAULT_CONFIG.file_extensions,
javascript: [],
typescript: []
}
Expand Down Expand Up @@ -384,6 +385,39 @@ describe('Tests for the describeRules method of ESLintEngine', () => {
{workspace: new Workspace([workspaceThatHasEslintIgnoreFile])});
expectRulesToMatchLegacyExpectationFile(ruleDescriptions, 'rules_DefaultConfig_NoJavascriptFilesInWorkspace.goldfile.json');
});

it('When custom rules only apply to file extensions that are not javascript or typescript based, then without specifying file extensions, they are not picked up', async () => {
const engine: ESLintEngine = new ESLintEngine({...DEFAULT_CONFIG,
config_root: __dirname,
disable_lwc_base_config: true,
disable_javascript_base_config: true,
disable_typescript_base_config: true,
eslint_config_file: path.join(workspaceThatHasCustomConfigWithNewRules, '.eslintrc_customLanguage.yml')
});
const ruleDescriptions: RuleDescription[] = await engine.describeRules(
{workspace: new Workspace([workspaceThatHasCustomConfigWithNewRules])});

expect(ruleDescriptions).toHaveLength(0);
});

it('When custom rules only apply to file extensions that are not javascript or typescript based, then when specifying file extensions, they are picked up', async () => {
const engine: ESLintEngine = new ESLintEngine({...DEFAULT_CONFIG,
config_root: __dirname,
disable_lwc_base_config: true,
disable_javascript_base_config: true,
disable_typescript_base_config: true,
eslint_config_file: path.join(workspaceThatHasCustomConfigWithNewRules, '.eslintrc_customLanguage.yml'),
file_extensions:{
... DEFAULT_CONFIG.file_extensions,
other: ['.html', '.cmp']
}
});
const ruleDescriptions: RuleDescription[] = await engine.describeRules(
{workspace: new Workspace([workspaceThatHasCustomConfigWithNewRules])});

expect(ruleDescriptions).toHaveLength(3);
expect(ruleDescriptions.map(rd => rd.name)).toEqual(["dummy/my-rule-1", "dummy/my-rule-2", "dummy/my-rule-3"]);
});
});

describe('Typical tests for the runRules method of ESLintEngine', () => {
Expand Down Expand Up @@ -493,6 +527,35 @@ describe('Typical tests for the runRules method of ESLintEngine', () => {
});
});

it('When custom rules only apply to file extensions that are not javascript or typescript based, then when specifying file extensions, the rules run', async () => {
const engine: ESLintEngine = new ESLintEngine({...DEFAULT_CONFIG,
config_root: __dirname,
eslint_config_file: path.join(workspaceThatHasCustomConfigWithNewRules, '.eslintrc_customLanguage.yml'),
file_extensions:{
... DEFAULT_CONFIG.file_extensions,
other: ['.html', '.cmp']
}
});

const runOptions: RunOptions = {workspace: new Workspace([path.join(workspaceThatHasCustomConfigWithNewRules)])};

const results: EngineRunResults = await engine.runRules(['dummy/my-rule-1'], runOptions);

expect(results.violations).toHaveLength(1);
expect(results.violations[0]).toEqual({
ruleName: "dummy/my-rule-1",
primaryLocationIndex: 0,
message: "Avoid using variables named 'forbidden'",
codeLocations: [{
file: path.join(workspaceThatHasCustomConfigWithNewRules, 'dummy.html'),
startLine: 2,
startColumn: 5,
endLine: 2,
endColumn: 14
}]
});
});

it('When custom eslint config exists but is not applied, then runRules emits info message', async () => {
const engine: ESLintEngine = new ESLintEngine({...DEFAULT_CONFIG,
config_root: __dirname
Expand Down
2 changes: 1 addition & 1 deletion packages/code-analyzer-eslint-engine/test/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ describe('Tests for the ESLintEnginePlugin', () => {
};
await expect(callCreateEngineConfig(plugin, userProvidedOverrides)).rejects.toThrow(
getMessageFromCatalog(SHARED_MESSAGE_CATALOG, 'ConfigObjectContainsInvalidKey', 'engines.eslint.file_extensions',
'oops', '["javascript","typescript"]'));
'oops', '["javascript","other","typescript"]'));
});

it('When a valid file_extensions.javascript value is passed to createEngineConfig, then it is set on the config', async () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
overrides:
- files:
- "*.html"
- "*.cmp"
plugins:
- "dummy"
parserOptions:
ecmaVersion: 2020
rules:
"dummy/my-rule-1":
- "error"
"dummy/my-rule-2":
- "error"
"dummy/my-rule-3":
- "error"
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Yes I know this isn't an html file. For testing purposes, this javascript file living in an html file is all I need.
let forbidden = 3;
Loading