Skip to content

Commit

Permalink
NEW(eslint): @W-17495039@: Allow users to specify file extensions tha…
Browse files Browse the repository at this point in the history
…t aren't associated with base plugins but only custom plugins
  • Loading branch information
stephen-carter-at-sf committed Dec 27, 2024
1 parent 6da23e8 commit 3e01253
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 46 deletions.
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);
}

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;

0 comments on commit 3e01253

Please sign in to comment.