From 3c200b7736266f15948b09cc865b4de988f27308 Mon Sep 17 00:00:00 2001 From: Christian Svensson Date: Mon, 20 Nov 2023 17:06:33 +0100 Subject: [PATCH 01/10] feat(eslint-plugin): add rule to disallow single array `styles` --- packages/eslint-plugin/README.md | 1 + .../docs/rules/no-single-styles-array.md | 426 ++++++++++++++++++ packages/eslint-plugin/src/configs/all.json | 1 + packages/eslint-plugin/src/index.ts | 4 + .../src/rules/no-single-styles-array.ts | 70 +++ .../rules/no-single-styles-array/cases.ts | 179 ++++++++ .../rules/no-single-styles-array/spec.ts | 12 + 7 files changed, 693 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-single-styles-array.md create mode 100644 packages/eslint-plugin/src/rules/no-single-styles-array.ts create mode 100644 packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts create mode 100644 packages/eslint-plugin/tests/rules/no-single-styles-array/spec.ts diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 9063affc6..44767d484 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -60,6 +60,7 @@ | [`no-outputs-metadata-property`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/no-outputs-metadata-property.md) | Disallows usage of the `outputs` metadata property. See more at https://angular.io/styleguide#style-05-12 | :white_check_mark: | | | | [`no-pipe-impure`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/no-pipe-impure.md) | Disallows the declaration of impure pipes | | | :bulb: | | [`no-queries-metadata-property`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/no-queries-metadata-property.md) | Disallows usage of the `queries` metadata property. See more at https://angular.io/styleguide#style-05-12. | | | | +| [`no-single-styles-array`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/no-single-styles-array.md) | Disallows usage `styles`/`styleUrls` with a single array string. | | :wrench: | | | [`pipe-prefix`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/pipe-prefix.md) | Enforce consistent prefix for pipes. | | | | | [`prefer-on-push-component-change-detection`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/prefer-on-push-component-change-detection.md) | Ensures component's `changeDetection` is set to `ChangeDetectionStrategy.OnPush` | | | :bulb: | | [`prefer-output-readonly`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/prefer-output-readonly.md) | Prefer to declare `@Output` as `readonly` since they are not supposed to be reassigned | | | :bulb: | diff --git a/packages/eslint-plugin/docs/rules/no-single-styles-array.md b/packages/eslint-plugin/docs/rules/no-single-styles-array.md new file mode 100644 index 000000000..35fc63694 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-single-styles-array.md @@ -0,0 +1,426 @@ + + +
+ +# `@angular-eslint/no-single-styles-array` + +Ensures component `styles`/`styleUrl` with `string` is used over `styles`/`styleUrls` when there is only a single string in the array + +- Type: suggestion +- 🔧 Supports autofix (`--fix`) + +
+ +## Rule Options + +The rule does not have any configuration options. + +
+ +## Usage Examples + +> The following examples are generated automatically from the actual unit tests within the plugin, so you can be assured that their behavior is accurate based on the current commit. + +
+ +
+❌ - Toggle examples of incorrect code for this rule + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: [':host { display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + standalone: true, + imports: [MatButtonModule], + styles: [':host { display: block; }'], + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styleUrls: ['./test.component.css'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + standalone: true, + imports: [MatButtonModule], + styleUrls: ['./test.component.css'], + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: [`:host { display: block; }`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styleUrls: [`./test.component.css`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +
+ +--- + +
+ +
+✅ - Toggle examples of correct code for this rule + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: ':host { display: block; }', +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: ` + :host { display: block; } + `, +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styles: ` + :host { display: block; } + `, + providers: [FooService] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: [ + ':host { display: block; }', + `.foo { color: red; }` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styleUrls: [ + '../shared.css', + `./test.component.css` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/no-single-styles-array": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styleUrls: [ + '../shared.css', + `./test.component.css` + ], + providers: [FooService] +}) +class Test {} +``` + +
+ +
diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 8c043b2c5..2143bb3b2 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -25,6 +25,7 @@ "@angular-eslint/no-outputs-metadata-property": "error", "@angular-eslint/no-pipe-impure": "error", "@angular-eslint/no-queries-metadata-property": "error", + "@angular-eslint/no-single-styles-array": "error", "@angular-eslint/pipe-prefix": "error", "@angular-eslint/prefer-on-push-component-change-detection": "error", "@angular-eslint/prefer-output-readonly": "error", diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index f64911150..584ac901d 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -70,6 +70,9 @@ import noPipeImpure, { import noQueriesMetadataProperty, { RULE_NAME as noQueriesMetadataPropertyRuleName, } from './rules/no-queries-metadata-property'; +import noSingleStylesArray, { + RULE_NAME as noSingleStylesArrayRuleName, +} from './rules/no-single-styles-array'; import pipePrefix, { RULE_NAME as pipePrefixRuleName, } from './rules/pipe-prefix'; @@ -139,6 +142,7 @@ export = { [noOutputsMetadataPropertyRuleName]: noOutputsMetadataProperty, [noPipeImpureRuleName]: noPipeImpure, [noQueriesMetadataPropertyRuleName]: noQueriesMetadataProperty, + [noSingleStylesArrayRuleName]: noSingleStylesArray, [pipePrefixRuleName]: pipePrefix, [preferOnPushComponentChangeDetectionRuleName]: preferOnPushComponentChangeDetection, diff --git a/packages/eslint-plugin/src/rules/no-single-styles-array.ts b/packages/eslint-plugin/src/rules/no-single-styles-array.ts new file mode 100644 index 000000000..db8a9f3b0 --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-single-styles-array.ts @@ -0,0 +1,70 @@ +import { ASTUtils, Selectors } from '@angular-eslint/utils'; +import type { TSESTree } from '@typescript-eslint/utils'; +import { createESLintRule } from '../utils/create-eslint-rule'; + +type Options = []; +export type MessageIds = 'noSingleStylesArray' | 'noSingleStyleUrl'; +export const RULE_NAME = 'no-single-styles-array'; + +export default createESLintRule({ + name: RULE_NAME, + meta: { + type: 'suggestion', + docs: { + description: + 'Ensures component `styles`/`styleUrl` with `string` is used over `styles`/`styleUrls` when there is only a single string in the array', + }, + fixable: 'code', + schema: [], + messages: { + noSingleStyleUrl: + 'Use `styleUrl` over `styleUrls` for a single stylesheet', + noSingleStylesArray: + 'Use a `string` over `[string]` for the `styles` property', + }, + }, + defaultOptions: [], + create(context) { + const { + COMPONENT_CLASS_DECORATOR, + LITERAL_OR_TEMPLATE_ELEMENT, + metadataProperty, + } = Selectors; + const singleArrayStringLiteral = `ArrayExpression:matches([elements.length=1]:has(${LITERAL_OR_TEMPLATE_ELEMENT}))`; + const singleStylesArrayExpression = `${COMPONENT_CLASS_DECORATOR} ${metadataProperty( + 'styles', + )} > ${singleArrayStringLiteral}`; + const singleStyleUrlsProperty = `${COMPONENT_CLASS_DECORATOR} ${metadataProperty( + 'styleUrls', + )}:has(${singleArrayStringLiteral})`; + + return { + [singleStylesArrayExpression](node: TSESTree.ArrayExpression) { + context.report({ + node, + messageId: 'noSingleStylesArray', + fix: (fixer) => { + const [el] = node.elements; + // TODO: Do we need the fix to apply for template strings? + if (!el || !ASTUtils.isStringLiteral(el)) return []; + return [fixer.replaceText(node, `'${el.value}'`)]; + }, + }); + }, + [singleStyleUrlsProperty](node: TSESTree.Property) { + if (!ASTUtils.isArrayExpression(node.value)) return; + const [el] = node.value.elements; + + context.report({ + node, + messageId: 'noSingleStyleUrl', + fix: (fixer) => { + // TODO: Do we need to fix for using template strings? + if (!el || !ASTUtils.isStringLiteral(el)) return []; + return [fixer.replaceText(node, `styleUrl: '${el.value}'`)]; + }, + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts b/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts new file mode 100644 index 000000000..c5f82aa6f --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts @@ -0,0 +1,179 @@ +import { convertAnnotatedSourceToFailureCase } from '@angular-eslint/utils'; +import type { MessageIds } from '../../../src/rules/no-single-styles-array'; + +const stylesMessageId: MessageIds = 'noSingleStylesArray'; +const urlMessageId: MessageIds = 'noSingleStyleUrl'; + +export const valid = [ + ` + @Component({ + styles: ':host { display: block; }', + }) + class Test {} + `, + ` + @Component({ + styles: \` + :host { display: block; } + \`, + }) + class Test {} + `, + ` + @Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styles: \` + :host { display: block; } + \`, + providers: [FooService] + }) + class Test {} + `, + ` + @Component({ + styles: [ + ':host { display: block; }', + \`.foo { color: red; }\` + ], + }) + class Test {} + `, + ` + @Component({ + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], + }) + class Test {} + `, + ` + @Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], + providers: [FooService] + }) + class Test {} + `, +]; + +export const invalid = [ + convertAnnotatedSourceToFailureCase({ + description: 'should fail when a component has a single style array value', + annotatedSource: ` + @Component({ + styles: [':host { display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + messageId: stylesMessageId, + annotatedOutput: ` + @Component({ + styles: ':host { display: block; }' + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: + 'should fail when a component has a single styles array value with multiple decorator properties', + annotatedSource: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styles: [':host { display: block; }'], + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] + }) + class Test {} + `, + messageId: stylesMessageId, + annotatedOutput: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styles: ':host { display: block; }', + + providers: [] + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: + 'should fail when a component has a single styleUrls array value', + annotatedSource: ` + @Component({ + styleUrls: ['./test.component.css'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + messageId: urlMessageId, + annotatedOutput: ` + @Component({ + styleUrl: './test.component.css' + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: + 'should fail when a component has a single styleUrls array value with multiple decorator properties', + annotatedSource: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styleUrls: ['./test.component.css'], + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] + }) + class Test {} + `, + messageId: urlMessageId, + annotatedOutput: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styleUrl: './test.component.css', + + providers: [] + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: + 'should fail when a component has a single style template string array value', + annotatedSource: ` + @Component({ + styles: [\`:host { display: block; }\`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + messageId: stylesMessageId, + }), + convertAnnotatedSourceToFailureCase({ + description: + 'should fail when a component has a single styleUrls template string array value', + annotatedSource: ` + @Component({ + styleUrls: [\`./test.component.css\`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + messageId: urlMessageId, + }), +]; diff --git a/packages/eslint-plugin/tests/rules/no-single-styles-array/spec.ts b/packages/eslint-plugin/tests/rules/no-single-styles-array/spec.ts new file mode 100644 index 000000000..63205d528 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-single-styles-array/spec.ts @@ -0,0 +1,12 @@ +import { RuleTester } from '@angular-eslint/utils'; +import rule, { RULE_NAME } from '../../../src/rules/no-single-styles-array'; +import { invalid, valid } from './cases'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run(RULE_NAME, rule, { + valid, + invalid, +}); From 94d9863c2347e7686ec1c710db634303a6981f15 Mon Sep 17 00:00:00 2001 From: reduckted Date: Wed, 17 Jan 2024 20:13:50 +1000 Subject: [PATCH 02/10] feat(eslint-plugin): handle template strings and different quote types --- .../src/rules/no-single-styles-array.ts | 34 ++- .../rules/no-single-styles-array/cases.ts | 227 ++++++++++++------ 2 files changed, 177 insertions(+), 84 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-single-styles-array.ts b/packages/eslint-plugin/src/rules/no-single-styles-array.ts index db8a9f3b0..5b59f9a17 100644 --- a/packages/eslint-plugin/src/rules/no-single-styles-array.ts +++ b/packages/eslint-plugin/src/rules/no-single-styles-array.ts @@ -45,9 +45,20 @@ export default createESLintRule({ messageId: 'noSingleStylesArray', fix: (fixer) => { const [el] = node.elements; - // TODO: Do we need the fix to apply for template strings? - if (!el || !ASTUtils.isStringLiteral(el)) return []; - return [fixer.replaceText(node, `'${el.value}'`)]; + if (el) { + if (ASTUtils.isStringLiteral(el)) { + return [fixer.replaceText(node, el.raw)]; + } + if (ASTUtils.isTemplateLiteral(el)) { + return [ + fixer.replaceText( + node, + `${context.getSourceCode().getText(el)}`, + ), + ]; + } + } + return []; }, }); }, @@ -59,9 +70,20 @@ export default createESLintRule({ node, messageId: 'noSingleStyleUrl', fix: (fixer) => { - // TODO: Do we need to fix for using template strings? - if (!el || !ASTUtils.isStringLiteral(el)) return []; - return [fixer.replaceText(node, `styleUrl: '${el.value}'`)]; + if (el) { + if (ASTUtils.isStringLiteral(el)) { + return [fixer.replaceText(node, `styleUrl: ${el.raw}`)]; + } + if (ASTUtils.isTemplateLiteral(el)) { + return [ + fixer.replaceText( + node, + `styleUrl: ${context.getSourceCode().getText(el)}`, + ), + ]; + } + } + return []; }, }); }, diff --git a/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts b/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts index c5f82aa6f..88af7e8db 100644 --- a/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts +++ b/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts @@ -40,6 +40,12 @@ export const valid = [ }) class Test {} `, + ` + @Component({ + styleUrl: \`./test.component.css\`, + }) + class Test {} + `, ` @Component({ styleUrls: [ @@ -66,114 +72,179 @@ export const valid = [ export const invalid = [ convertAnnotatedSourceToFailureCase({ - description: 'should fail when a component has a single style array value', + description: `should fail when a component has a single style array value`, annotatedSource: ` - @Component({ - styles: [':host { display: block; }'] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - }) - class Test {} - `, + @Component({ + styles: [':host { display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, messageId: stylesMessageId, annotatedOutput: ` - @Component({ - styles: ':host { display: block; }' - - }) - class Test {} - `, + @Component({ + styles: ':host { display: block; }' + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should fail when a component has a single styles array value with multiple decorator properties`, + annotatedSource: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styles: [':host { display: block; }'], + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] + }) + class Test {} + `, + messageId: stylesMessageId, + annotatedOutput: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styles: ':host { display: block; }', + + providers: [] + }) + class Test {} + `, }), convertAnnotatedSourceToFailureCase({ - description: - 'should fail when a component has a single styles array value with multiple decorator properties', + description: `should fail when a component has a single styleUrls array value`, annotatedSource: ` - @Component({ - standalone: true, - imports: [MatButtonModule], - styles: [':host { display: block; }'], - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - providers: [] - }) - class Test {} + @Component({ + styleUrls: ['./test.component.css'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + messageId: urlMessageId, + annotatedOutput: ` + @Component({ + styleUrl: './test.component.css' + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should fail when a component has a single styleUrls array value with multiple decorator properties`, + annotatedSource: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styleUrls: ['./test.component.css'], + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] + }) + class Test {} + `, + messageId: urlMessageId, + annotatedOutput: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styleUrl: './test.component.css', + + providers: [] + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should keep template strings when fixing`, + annotatedSource: ` + const type = 'block'; + @Component({ + styles: [\`:host\\t{ display: \${type}; }\`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} `, messageId: stylesMessageId, annotatedOutput: ` - @Component({ - standalone: true, - imports: [MatButtonModule], - styles: ':host { display: block; }', - - providers: [] - }) - class Test {} + const type = 'block'; + @Component({ + styles: \`:host\\t{ display: \${type}; }\` + + }) + class Test {} `, }), convertAnnotatedSourceToFailureCase({ - description: - 'should fail when a component has a single styleUrls array value', + description: `should keep escaped characters when fixing`, annotatedSource: ` - @Component({ - styleUrls: ['./test.component.css'] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - }) - class Test {} + @Component({ + styles: [':host\\t{ display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} `, - messageId: urlMessageId, + messageId: stylesMessageId, annotatedOutput: ` - @Component({ - styleUrl: './test.component.css' - - }) - class Test {} + @Component({ + styles: ':host\\t{ display: block; }' + + }) + class Test {} `, }), convertAnnotatedSourceToFailureCase({ - description: - 'should fail when a component has a single styleUrls array value with multiple decorator properties', + description: `should keep single quotes when fixing`, annotatedSource: ` - @Component({ - standalone: true, - imports: [MatButtonModule], - styleUrls: ['./test.component.css'], - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - providers: [] - }) - class Test {} + @Component({ + styles: [':host{ display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} `, - messageId: urlMessageId, + messageId: stylesMessageId, annotatedOutput: ` - @Component({ - standalone: true, - imports: [MatButtonModule], - styleUrl: './test.component.css', - - providers: [] - }) - class Test {} + @Component({ + styles: ':host{ display: block; }' + + }) + class Test {} `, }), convertAnnotatedSourceToFailureCase({ - description: - 'should fail when a component has a single style template string array value', + description: `should keep double quotes when fixing`, annotatedSource: ` - @Component({ - styles: [\`:host { display: block; }\`] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - }) - class Test {} + @Component({ + styles: [":host{ display: block; }"] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} `, messageId: stylesMessageId, + annotatedOutput: ` + @Component({ + styles: ":host{ display: block; }" + + }) + class Test {} + `, }), convertAnnotatedSourceToFailureCase({ - description: - 'should fail when a component has a single styleUrls template string array value', + description: `should keep backtick quotes when fixing`, annotatedSource: ` - @Component({ - styleUrls: [\`./test.component.css\`] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - }) - class Test {} + @Component({ + styles: [\`:host{ display: block; }\`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + messageId: stylesMessageId, + annotatedOutput: ` + @Component({ + styles: \`:host{ display: block; }\` + + }) + class Test {} `, - messageId: urlMessageId, }), ]; From 8e6f80d50424000a67e190204ab1672b6edc9bcd Mon Sep 17 00:00:00 2001 From: reduckted Date: Sun, 21 Jan 2024 21:56:46 +1000 Subject: [PATCH 03/10] feat(eslint-plugin): rename no-single-styles-array to consistent-component-styles --- packages/eslint-plugin/README.md | 2 +- packages/eslint-plugin/src/configs/all.json | 2 +- packages/eslint-plugin/src/index.ts | 8 +++---- ...rray.ts => consistent-component-styles.ts} | 23 +++++++++++------- .../cases.ts | 24 +++++++++---------- .../spec.ts | 4 +++- 6 files changed, 36 insertions(+), 27 deletions(-) rename packages/eslint-plugin/src/rules/{no-single-styles-array.ts => consistent-component-styles.ts} (81%) rename packages/eslint-plugin/tests/rules/{no-single-styles-array => consistent-component-styles}/cases.ts (90%) rename packages/eslint-plugin/tests/rules/{no-single-styles-array => consistent-component-styles}/spec.ts (73%) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 44767d484..16a88d0f8 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -43,6 +43,7 @@ | [`component-class-suffix`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/component-class-suffix.md) | Classes decorated with @Component must have suffix "Component" (or custom) in their name. See more at https://angular.io/styleguide#style-02-03 | :white_check_mark: | | | | [`component-max-inline-declarations`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/component-max-inline-declarations.md) | Enforces a maximum number of lines in inline template, styles and animations. See more at https://angular.io/guide/styleguide#style-05-04 | | | | | [`component-selector`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/component-selector.md) | Component selectors should follow given naming rules. See more at https://angular.io/guide/styleguide#style-02-07, https://angular.io/guide/styleguide#style-05-02 and https://angular.io/guide/styleguide#style-05-03. | | | | +| [`consistent-component-styles`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/consistent-component-styles.md) | Ensures component `styles`/`styleUrl` with `string` is used over `styles`/`styleUrls` when there is only a single string in the array | | :wrench: | | | [`contextual-decorator`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/contextual-decorator.md) | Ensures that classes use contextual decorators in its body | | | | | [`directive-class-suffix`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/directive-class-suffix.md) | Classes decorated with @Directive must have suffix "Directive" (or custom) in their name. See more at https://angular.io/styleguide#style-02-03 | :white_check_mark: | | | | [`directive-selector`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/directive-selector.md) | Directive selectors should follow given naming rules. See more at https://angular.io/guide/styleguide#style-02-06 and https://angular.io/guide/styleguide#style-02-08. | | | | @@ -60,7 +61,6 @@ | [`no-outputs-metadata-property`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/no-outputs-metadata-property.md) | Disallows usage of the `outputs` metadata property. See more at https://angular.io/styleguide#style-05-12 | :white_check_mark: | | | | [`no-pipe-impure`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/no-pipe-impure.md) | Disallows the declaration of impure pipes | | | :bulb: | | [`no-queries-metadata-property`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/no-queries-metadata-property.md) | Disallows usage of the `queries` metadata property. See more at https://angular.io/styleguide#style-05-12. | | | | -| [`no-single-styles-array`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/no-single-styles-array.md) | Disallows usage `styles`/`styleUrls` with a single array string. | | :wrench: | | | [`pipe-prefix`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/pipe-prefix.md) | Enforce consistent prefix for pipes. | | | | | [`prefer-on-push-component-change-detection`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/prefer-on-push-component-change-detection.md) | Ensures component's `changeDetection` is set to `ChangeDetectionStrategy.OnPush` | | | :bulb: | | [`prefer-output-readonly`](https://github.com/angular-eslint/angular-eslint/blob/main/packages/eslint-plugin/docs/rules/prefer-output-readonly.md) | Prefer to declare `@Output` as `readonly` since they are not supposed to be reassigned | | | :bulb: | diff --git a/packages/eslint-plugin/src/configs/all.json b/packages/eslint-plugin/src/configs/all.json index 2143bb3b2..5f0fdc680 100644 --- a/packages/eslint-plugin/src/configs/all.json +++ b/packages/eslint-plugin/src/configs/all.json @@ -5,6 +5,7 @@ "@angular-eslint/component-class-suffix": "error", "@angular-eslint/component-max-inline-declarations": "error", "@angular-eslint/component-selector": "error", + "@angular-eslint/consistent-component-styles": "error", "@angular-eslint/contextual-decorator": "error", "@angular-eslint/contextual-lifecycle": "error", "@angular-eslint/directive-class-suffix": "error", @@ -25,7 +26,6 @@ "@angular-eslint/no-outputs-metadata-property": "error", "@angular-eslint/no-pipe-impure": "error", "@angular-eslint/no-queries-metadata-property": "error", - "@angular-eslint/no-single-styles-array": "error", "@angular-eslint/pipe-prefix": "error", "@angular-eslint/prefer-on-push-component-change-detection": "error", "@angular-eslint/prefer-output-readonly": "error", diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts index 584ac901d..ce72f8e0f 100644 --- a/packages/eslint-plugin/src/index.ts +++ b/packages/eslint-plugin/src/index.ts @@ -10,6 +10,9 @@ import componentMaxInlineDeclarations, { import componentSelector, { RULE_NAME as componentSelectorRuleName, } from './rules/component-selector'; +import consistentComponentStyles, { + RULE_NAME as consistentComponentStylesRuleName, +} from './rules/consistent-component-styles'; import contextualDecorator, { RULE_NAME as contextualDecoratorRuleName, } from './rules/contextual-decorator'; @@ -70,9 +73,6 @@ import noPipeImpure, { import noQueriesMetadataProperty, { RULE_NAME as noQueriesMetadataPropertyRuleName, } from './rules/no-queries-metadata-property'; -import noSingleStylesArray, { - RULE_NAME as noSingleStylesArrayRuleName, -} from './rules/no-single-styles-array'; import pipePrefix, { RULE_NAME as pipePrefixRuleName, } from './rules/pipe-prefix'; @@ -122,6 +122,7 @@ export = { [componentClassSuffixRuleName]: componentClassSuffix, [componentMaxInlineDeclarationsRuleName]: componentMaxInlineDeclarations, [componentSelectorRuleName]: componentSelector, + [consistentComponentStylesRuleName]: consistentComponentStyles, [contextualDecoratorRuleName]: contextualDecorator, [contextualLifecycleRuleName]: contextualLifecycle, [directiveClassSuffixRuleName]: directiveClassSuffix, @@ -142,7 +143,6 @@ export = { [noOutputsMetadataPropertyRuleName]: noOutputsMetadataProperty, [noPipeImpureRuleName]: noPipeImpure, [noQueriesMetadataPropertyRuleName]: noQueriesMetadataProperty, - [noSingleStylesArrayRuleName]: noSingleStylesArray, [pipePrefixRuleName]: pipePrefix, [preferOnPushComponentChangeDetectionRuleName]: preferOnPushComponentChangeDetection, diff --git a/packages/eslint-plugin/src/rules/no-single-styles-array.ts b/packages/eslint-plugin/src/rules/consistent-component-styles.ts similarity index 81% rename from packages/eslint-plugin/src/rules/no-single-styles-array.ts rename to packages/eslint-plugin/src/rules/consistent-component-styles.ts index 5b59f9a17..847667d1c 100644 --- a/packages/eslint-plugin/src/rules/no-single-styles-array.ts +++ b/packages/eslint-plugin/src/rules/consistent-component-styles.ts @@ -3,8 +3,12 @@ import type { TSESTree } from '@typescript-eslint/utils'; import { createESLintRule } from '../utils/create-eslint-rule'; type Options = []; -export type MessageIds = 'noSingleStylesArray' | 'noSingleStyleUrl'; -export const RULE_NAME = 'no-single-styles-array'; +export type MessageIds = + | 'useStylesArray' + | 'useStylesString' + | 'useStyleUrl' + | 'useStyleUrls'; +export const RULE_NAME = 'consistent-component-styles'; export default createESLintRule({ name: RULE_NAME, @@ -17,10 +21,13 @@ export default createESLintRule({ fixable: 'code', schema: [], messages: { - noSingleStyleUrl: - 'Use `styleUrl` over `styleUrls` for a single stylesheet', - noSingleStylesArray: - 'Use a `string` over `[string]` for the `styles` property', + useStyleUrl: + 'Use `styleUrl` instead of `styleUrls` for a single stylesheet', + useStyleUrls: 'Use `styleUrls` instead of `styleUrl`', + useStylesArray: + 'Use a `string[]` instead of a `string` for the `styles` property', + useStylesString: + 'Use a `string` instead of a `string[]` for the `styles` property', }, }, defaultOptions: [], @@ -42,7 +49,7 @@ export default createESLintRule({ [singleStylesArrayExpression](node: TSESTree.ArrayExpression) { context.report({ node, - messageId: 'noSingleStylesArray', + messageId: 'useStylesString', fix: (fixer) => { const [el] = node.elements; if (el) { @@ -68,7 +75,7 @@ export default createESLintRule({ context.report({ node, - messageId: 'noSingleStyleUrl', + messageId: 'useStyleUrl', fix: (fixer) => { if (el) { if (ASTUtils.isStringLiteral(el)) { diff --git a/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts b/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts similarity index 90% rename from packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts rename to packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts index 88af7e8db..edbcc2f6e 100644 --- a/packages/eslint-plugin/tests/rules/no-single-styles-array/cases.ts +++ b/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts @@ -1,8 +1,8 @@ import { convertAnnotatedSourceToFailureCase } from '@angular-eslint/utils'; -import type { MessageIds } from '../../../src/rules/no-single-styles-array'; +import type { MessageIds } from '../../../src/rules/consistent-component-styles'; -const stylesMessageId: MessageIds = 'noSingleStylesArray'; -const urlMessageId: MessageIds = 'noSingleStyleUrl'; +const messageIdUseStylesString: MessageIds = 'useStylesString'; +const messageIdUseStyleUrl: MessageIds = 'useStyleUrl'; export const valid = [ ` @@ -80,7 +80,7 @@ export const invalid = [ }) class Test {} `, - messageId: stylesMessageId, + messageId: messageIdUseStylesString, annotatedOutput: ` @Component({ styles: ':host { display: block; }' @@ -101,7 +101,7 @@ export const invalid = [ }) class Test {} `, - messageId: stylesMessageId, + messageId: messageIdUseStylesString, annotatedOutput: ` @Component({ standalone: true, @@ -122,7 +122,7 @@ export const invalid = [ }) class Test {} `, - messageId: urlMessageId, + messageId: messageIdUseStyleUrl, annotatedOutput: ` @Component({ styleUrl: './test.component.css' @@ -143,7 +143,7 @@ export const invalid = [ }) class Test {} `, - messageId: urlMessageId, + messageId: messageIdUseStyleUrl, annotatedOutput: ` @Component({ standalone: true, @@ -165,7 +165,7 @@ export const invalid = [ }) class Test {} `, - messageId: stylesMessageId, + messageId: messageIdUseStylesString, annotatedOutput: ` const type = 'block'; @Component({ @@ -184,7 +184,7 @@ export const invalid = [ }) class Test {} `, - messageId: stylesMessageId, + messageId: messageIdUseStylesString, annotatedOutput: ` @Component({ styles: ':host\\t{ display: block; }' @@ -202,7 +202,7 @@ export const invalid = [ }) class Test {} `, - messageId: stylesMessageId, + messageId: messageIdUseStylesString, annotatedOutput: ` @Component({ styles: ':host{ display: block; }' @@ -220,7 +220,7 @@ export const invalid = [ }) class Test {} `, - messageId: stylesMessageId, + messageId: messageIdUseStylesString, annotatedOutput: ` @Component({ styles: ":host{ display: block; }" @@ -238,7 +238,7 @@ export const invalid = [ }) class Test {} `, - messageId: stylesMessageId, + messageId: messageIdUseStylesString, annotatedOutput: ` @Component({ styles: \`:host{ display: block; }\` diff --git a/packages/eslint-plugin/tests/rules/no-single-styles-array/spec.ts b/packages/eslint-plugin/tests/rules/consistent-component-styles/spec.ts similarity index 73% rename from packages/eslint-plugin/tests/rules/no-single-styles-array/spec.ts rename to packages/eslint-plugin/tests/rules/consistent-component-styles/spec.ts index 63205d528..02a007fa1 100644 --- a/packages/eslint-plugin/tests/rules/no-single-styles-array/spec.ts +++ b/packages/eslint-plugin/tests/rules/consistent-component-styles/spec.ts @@ -1,5 +1,7 @@ import { RuleTester } from '@angular-eslint/utils'; -import rule, { RULE_NAME } from '../../../src/rules/no-single-styles-array'; +import rule, { + RULE_NAME, +} from '../../../src/rules/consistent-component-styles'; import { invalid, valid } from './cases'; const ruleTester = new RuleTester({ From 3d141a180b5d7bbf66b0e1338f90a684b8c64f4b Mon Sep 17 00:00:00 2001 From: reduckted Date: Sun, 21 Jan 2024 21:59:10 +1000 Subject: [PATCH 04/10] feat(eslint-plugin): add option for consistent-component-styles --- .../docs/rules/consistent-component-styles.md | 1331 +++++++++++++++++ .../src/rules/consistent-component-styles.ts | 165 +- .../consistent-component-styles/cases.ts | 388 ++++- 3 files changed, 1828 insertions(+), 56 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/consistent-component-styles.md diff --git a/packages/eslint-plugin/docs/rules/consistent-component-styles.md b/packages/eslint-plugin/docs/rules/consistent-component-styles.md new file mode 100644 index 000000000..e2eb33b38 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/consistent-component-styles.md @@ -0,0 +1,1331 @@ + + +
+ +# `@angular-eslint/consistent-component-styles` + +Ensures component `styles`/`styleUrl` with `string` is used over `styles`/`styleUrls` when there is only a single string in the array + +- Type: suggestion +- 🔧 Supports autofix (`--fix`) + +
+ +## Rule Options + +The rule accepts an options object with the following properties: + +```ts +type Options = "array" | "string"; + +``` + +
+ +## Usage Examples + +> The following examples are generated automatically from the actual unit tests within the plugin, so you can be assured that their behavior is accurate based on the current commit. + +
+ +
+❌ - Toggle examples of incorrect code for this rule + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: [':host { display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + standalone: true, + imports: [MatButtonModule], + styles: [':host { display: block; }'], + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styleUrls: ['./test.component.css'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + standalone: true, + imports: [MatButtonModule], + styleUrls: ['./test.component.css'], + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +const type = 'block'; +@Component({ + styles: [`:host\t{ display: ${type}; }`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: [':host\t{ display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: [':host{ display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: [":host{ display: block; }"] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: [`:host{ display: block; }`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: [':host { display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styleUrls: ['./test.component.css'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: ':host { display: block; }' + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + standalone: true, + imports: [MatButtonModule], + styles: ':host { display: block; }', + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styleUrl: './test.component.css', + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +const type = 'block'; +@Component({ + styles: `:host\t{ display: ${type}; }` + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: ':host\t{ display: block; }' + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: ':host{ display: block; }' + ~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: ":host{ display: block; }" + ~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styles: `:host{ display: block; }` + ~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +
+ +--- + +
+ +
+✅ - Toggle examples of correct code for this rule + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: ':host { display: block; }', +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: ` + :host { display: block; } + `, +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styles: ` + :host { display: block; } + `, + providers: [FooService] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: [ + ':host { display: block; }', + `.foo { color: red; }` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styleUrl: `./test.component.css`, +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styleUrls: [ + '../shared.css', + `./test.component.css` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styleUrls: [ + '../shared.css', + `./test.component.css` + ], + providers: [FooService] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: ':host { display: block; }', +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: \` + :host { display: block; } + \`, +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styles: \` + :host { display: block; } + \`, + providers: [FooService] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: [ + ':host { display: block; }', + \`.foo { color: red; }\` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styleUrl: \`./test.component.css\`, +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "string" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], + providers: [FooService] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: [':host { display: block; }'], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: [ + \` + :host { display: block; } + \` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styles: [ + \` + :host { display: block; } + \` + ], + providers: [FooService] +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styles: [ + ':host { display: block; }', + \`.foo { color: red; }\` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styleUrls: [\`./test.component.css\`], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], +}) +class Test {} +``` + +
+ +--- + +
+ +#### Custom Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error", + "array" + ] + } +} +``` + +
+ +#### ✅ Valid Code + +```ts +@Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], + providers: [FooService] +}) +class Test {} +``` + +
+ +
diff --git a/packages/eslint-plugin/src/rules/consistent-component-styles.ts b/packages/eslint-plugin/src/rules/consistent-component-styles.ts index 847667d1c..1dbb90bbc 100644 --- a/packages/eslint-plugin/src/rules/consistent-component-styles.ts +++ b/packages/eslint-plugin/src/rules/consistent-component-styles.ts @@ -2,7 +2,8 @@ import { ASTUtils, Selectors } from '@angular-eslint/utils'; import type { TSESTree } from '@typescript-eslint/utils'; import { createESLintRule } from '../utils/create-eslint-rule'; -type Options = []; +type Mode = 'array' | 'string'; +type Options = [mode: Mode]; export type MessageIds = | 'useStylesArray' | 'useStylesString' @@ -19,7 +20,12 @@ export default createESLintRule({ 'Ensures component `styles`/`styleUrl` with `string` is used over `styles`/`styleUrls` when there is only a single string in the array', }, fixable: 'code', - schema: [], + schema: [ + { + type: 'string', + enum: ['array', 'string'], + }, + ], messages: { useStyleUrl: 'Use `styleUrl` instead of `styleUrls` for a single stylesheet', @@ -30,70 +36,127 @@ export default createESLintRule({ 'Use a `string` instead of a `string[]` for the `styles` property', }, }, - defaultOptions: [], - create(context) { - const { - COMPONENT_CLASS_DECORATOR, - LITERAL_OR_TEMPLATE_ELEMENT, - metadataProperty, - } = Selectors; - const singleArrayStringLiteral = `ArrayExpression:matches([elements.length=1]:has(${LITERAL_OR_TEMPLATE_ELEMENT}))`; - const singleStylesArrayExpression = `${COMPONENT_CLASS_DECORATOR} ${metadataProperty( - 'styles', - )} > ${singleArrayStringLiteral}`; - const singleStyleUrlsProperty = `${COMPONENT_CLASS_DECORATOR} ${metadataProperty( - 'styleUrls', - )}:has(${singleArrayStringLiteral})`; + defaultOptions: ['string'], + create(context, [mode]) { + const { COMPONENT_CLASS_DECORATOR, metadataProperty } = Selectors; + const LITERAL_OR_TEMPLATE_LITERAL = ':matches(Literal, TemplateLiteral)'; + + if (mode === 'array') { + const stylesStringExpression = `${COMPONENT_CLASS_DECORATOR} ${metadataProperty( + 'styles', + )} > ${LITERAL_OR_TEMPLATE_LITERAL}`; + const styleUrlProperty = `${COMPONENT_CLASS_DECORATOR} ${metadataProperty( + 'styleUrl', + )}:has(:matches(Literal, TemplateElement))`; - return { - [singleStylesArrayExpression](node: TSESTree.ArrayExpression) { - context.report({ - node, - messageId: 'useStylesString', - fix: (fixer) => { - const [el] = node.elements; - if (el) { - if (ASTUtils.isStringLiteral(el)) { - return [fixer.replaceText(node, el.raw)]; + return { + [stylesStringExpression]( + node: TSESTree.Literal | TSESTree.TemplateLiteral, + ) { + context.report({ + node, + messageId: 'useStylesArray', + fix: (fixer) => { + if (ASTUtils.isStringLiteral(node)) { + return [fixer.replaceText(node, `[${node.raw}]`)]; } - if (ASTUtils.isTemplateLiteral(el)) { + if (ASTUtils.isTemplateLiteral(node)) { return [ fixer.replaceText( node, - `${context.getSourceCode().getText(el)}`, + `[${context.getSourceCode().getText(node)}]`, ), ]; } - } - return []; - }, - }); - }, - [singleStyleUrlsProperty](node: TSESTree.Property) { - if (!ASTUtils.isArrayExpression(node.value)) return; - const [el] = node.value.elements; + return []; + }, + }); + }, - context.report({ - node, - messageId: 'useStyleUrl', - fix: (fixer) => { - if (el) { - if (ASTUtils.isStringLiteral(el)) { - return [fixer.replaceText(node, `styleUrl: ${el.raw}`)]; + [styleUrlProperty](node: TSESTree.Property) { + context.report({ + node, + messageId: 'useStyleUrls', + fix: (fixer) => { + if (ASTUtils.isStringLiteral(node.value)) { + return [ + fixer.replaceText(node, `styleUrls: [${node.value.raw}]`), + ]; } - if (ASTUtils.isTemplateLiteral(el)) { + if (ASTUtils.isTemplateLiteral(node.value)) { return [ fixer.replaceText( node, - `styleUrl: ${context.getSourceCode().getText(el)}`, + `styleUrls: [${context + .getSourceCode() + .getText(node.value)}]`, ), ]; } - } - return []; - }, - }); - }, - }; + return []; + }, + }); + }, + }; + } else { + const singleArrayStringLiteral = `ArrayExpression:matches([elements.length=1]:has(${LITERAL_OR_TEMPLATE_LITERAL}))`; + const singleStylesArrayExpression = `${COMPONENT_CLASS_DECORATOR} ${metadataProperty( + 'styles', + )} > ${singleArrayStringLiteral}`; + const singleStyleUrlsProperty = `${COMPONENT_CLASS_DECORATOR} ${metadataProperty( + 'styleUrls', + )}:has(${singleArrayStringLiteral})`; + + return { + [singleStylesArrayExpression](node: TSESTree.ArrayExpression) { + context.report({ + node, + messageId: 'useStylesString', + fix: (fixer) => { + const [el] = node.elements; + if (el) { + if (ASTUtils.isStringLiteral(el)) { + return [fixer.replaceText(node, el.raw)]; + } + if (ASTUtils.isTemplateLiteral(el)) { + return [ + fixer.replaceText( + node, + context.getSourceCode().getText(el), + ), + ]; + } + } + return []; + }, + }); + }, + [singleStyleUrlsProperty](node: TSESTree.Property) { + if (!ASTUtils.isArrayExpression(node.value)) return; + const [el] = node.value.elements; + + context.report({ + node, + messageId: 'useStyleUrl', + fix: (fixer) => { + if (el) { + if (ASTUtils.isStringLiteral(el)) { + return [fixer.replaceText(node, `styleUrl: ${el.raw}`)]; + } + if (ASTUtils.isTemplateLiteral(el)) { + return [ + fixer.replaceText( + node, + `styleUrl: ${context.getSourceCode().getText(el)}`, + ), + ]; + } + } + return []; + }, + }); + }, + }; + } }, }); diff --git a/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts b/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts index edbcc2f6e..251007c1c 100644 --- a/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts +++ b/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts @@ -1,10 +1,13 @@ import { convertAnnotatedSourceToFailureCase } from '@angular-eslint/utils'; import type { MessageIds } from '../../../src/rules/consistent-component-styles'; +const messageIdUseStylesArray: MessageIds = 'useStylesArray'; +const messageIdUseStyleUrls: MessageIds = 'useStyleUrls'; const messageIdUseStylesString: MessageIds = 'useStylesString'; const messageIdUseStyleUrl: MessageIds = 'useStyleUrl'; export const valid = [ + // Default. ` @Component({ styles: ':host { display: block; }', @@ -68,9 +71,184 @@ export const valid = [ }) class Test {} `, + // String. + { + code: ` + @Component({ + styles: ':host { display: block; }', + }) + class Test {} + `, + options: ['string'], + }, + { + code: ` + @Component({ + styles: \` + :host { display: block; } + \`, + }) + class Test {} + `, + options: ['string'], + }, + { + code: ` + @Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styles: \` + :host { display: block; } + \`, + providers: [FooService] + }) + class Test {} + `, + options: ['string'], + }, + { + code: ` + @Component({ + styles: [ + ':host { display: block; }', + \`.foo { color: red; }\` + ], + }) + class Test {} + `, + options: ['string'], + }, + { + code: ` + @Component({ + styleUrl: \`./test.component.css\`, + }) + class Test {} + `, + options: ['string'], + }, + { + code: ` + @Component({ + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], + }) + class Test {} + `, + options: ['string'], + }, + { + code: ` + @Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], + providers: [FooService] + }) + class Test {} + `, + options: ['string'], + }, + // Array. + { + code: ` + @Component({ + styles: [':host { display: block; }'], + }) + class Test {} + `, + options: ['array'], + }, + { + code: ` + @Component({ + styles: [ + \` + :host { display: block; } + \` + ], + }) + class Test {} + `, + options: ['array'], + }, + { + code: ` + @Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styles: [ + \` + :host { display: block; } + \` + ], + providers: [FooService] + }) + class Test {} + `, + options: ['array'], + }, + { + code: ` + @Component({ + styles: [ + ':host { display: block; }', + \`.foo { color: red; }\` + ], + }) + class Test {} + `, + options: ['array'], + }, + { + code: ` + @Component({ + styleUrls: [\`./test.component.css\`], + }) + class Test {} + `, + options: ['array'], + }, + { + code: ` + @Component({ + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], + }) + class Test {} + `, + options: ['array'], + }, + { + code: ` + @Component({ + selector: 'my-test', + standalone: true, + imports: [CommonModule], + styleUrls: [ + '../shared.css', + \`./test.component.css\` + ], + providers: [FooService] + }) + class Test {} + `, + options: ['array'], + }, ]; export const invalid = [ + // Default convertAnnotatedSourceToFailureCase({ description: `should fail when a component has a single style array value`, annotatedSource: ` @@ -156,7 +334,7 @@ export const invalid = [ `, }), convertAnnotatedSourceToFailureCase({ - description: `should keep template strings when fixing`, + description: `should keep template strings when fixing styles array to string`, annotatedSource: ` const type = 'block'; @Component({ @@ -176,7 +354,7 @@ export const invalid = [ `, }), convertAnnotatedSourceToFailureCase({ - description: `should keep escaped characters when fixing`, + description: `should keep escaped characters when fixing styles array to string`, annotatedSource: ` @Component({ styles: [':host\\t{ display: block; }'] @@ -194,7 +372,7 @@ export const invalid = [ `, }), convertAnnotatedSourceToFailureCase({ - description: `should keep single quotes when fixing`, + description: `should keep single quotes when fixing styles array to string`, annotatedSource: ` @Component({ styles: [':host{ display: block; }'] @@ -212,7 +390,7 @@ export const invalid = [ `, }), convertAnnotatedSourceToFailureCase({ - description: `should keep double quotes when fixing`, + description: `should keep double quotes when fixing styles array to string`, annotatedSource: ` @Component({ styles: [":host{ display: block; }"] @@ -230,7 +408,7 @@ export const invalid = [ `, }), convertAnnotatedSourceToFailureCase({ - description: `should keep backtick quotes when fixing`, + description: `should keep backtick quotes when fixing styles array to string`, annotatedSource: ` @Component({ styles: [\`:host{ display: block; }\`] @@ -247,4 +425,204 @@ export const invalid = [ class Test {} `, }), + // String + convertAnnotatedSourceToFailureCase({ + description: `should fail when a component has a single style array value when string is preferred`, + annotatedSource: ` + @Component({ + styles: [':host { display: block; }'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['string'], + messageId: messageIdUseStylesString, + annotatedOutput: ` + @Component({ + styles: ':host { display: block; }' + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should fail when a component has a single styleUrls array value when string is preferred`, + annotatedSource: ` + @Component({ + styleUrls: ['./test.component.css'] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['string'], + messageId: messageIdUseStyleUrl, + annotatedOutput: ` + @Component({ + styleUrl: './test.component.css' + + }) + class Test {} + `, + }), + // Array + convertAnnotatedSourceToFailureCase({ + description: `should fail when a component has a styles string when an array is preferred`, + annotatedSource: ` + @Component({ + styles: ':host { display: block; }' + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['array'], + messageId: messageIdUseStylesArray, + annotatedOutput: ` + @Component({ + styles: [':host { display: block; }'] + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should fail when a component has a styles string with multiple decorator properties when an array is preferred`, + annotatedSource: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styles: ':host { display: block; }', + ~~~~~~~~~~~~~~~~~~~~~~~~~~~ + providers: [] + }) + class Test {} + `, + options: ['array'], + messageId: messageIdUseStylesArray, + annotatedOutput: ` + @Component({ + standalone: true, + imports: [MatButtonModule], + styles: [':host { display: block; }'], + + providers: [] + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should fail when a component has a styleUrl string value when an array is preferred`, + annotatedSource: ` + @Component({ + styleUrl: './test.component.css', + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['array'], + messageId: messageIdUseStyleUrls, + annotatedOutput: ` + @Component({ + styleUrls: ['./test.component.css'], + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should keep template strings when fixing styles string to array`, + annotatedSource: ` + const type = 'block'; + @Component({ + styles: \`:host\\t{ display: \${type}; }\` + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['array'], + messageId: messageIdUseStylesArray, + annotatedOutput: ` + const type = 'block'; + @Component({ + styles: [\`:host\\t{ display: \${type}; }\`] + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should keep escaped characters when fixing styles string to array`, + annotatedSource: ` + @Component({ + styles: ':host\\t{ display: block; }' + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['array'], + messageId: messageIdUseStylesArray, + annotatedOutput: ` + @Component({ + styles: [':host\\t{ display: block; }'] + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should keep single quotes when fixing styles string to array`, + annotatedSource: ` + @Component({ + styles: ':host{ display: block; }' + ~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['array'], + messageId: messageIdUseStylesArray, + annotatedOutput: ` + @Component({ + styles: [':host{ display: block; }'] + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should keep double quotes when fixing styles string to array`, + annotatedSource: ` + @Component({ + styles: ":host{ display: block; }" + ~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['array'], + messageId: messageIdUseStylesArray, + annotatedOutput: ` + @Component({ + styles: [":host{ display: block; }"] + + }) + class Test {} + `, + }), + convertAnnotatedSourceToFailureCase({ + description: `should keep backtick quotes when fixing styles string to array`, + annotatedSource: ` + @Component({ + styles: \`:host{ display: block; }\` + ~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + options: ['array'], + messageId: messageIdUseStylesArray, + annotatedOutput: ` + @Component({ + styles: [\`:host{ display: block; }\`] + + }) + class Test {} + `, + }), ]; From 4e267604217e4675dd2d9514ef3924f9950f9ea6 Mon Sep 17 00:00:00 2001 From: reduckted Date: Mon, 29 Jan 2024 21:10:30 +1000 Subject: [PATCH 05/10] feat(eslint-plugin): improve coverage of consistent-component-styles --- .../src/rules/consistent-component-styles.ts | 86 +++++++------------ .../consistent-component-styles/cases.ts | 18 ++++ 2 files changed, 47 insertions(+), 57 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-component-styles.ts b/packages/eslint-plugin/src/rules/consistent-component-styles.ts index 1dbb90bbc..a0e1efdbc 100644 --- a/packages/eslint-plugin/src/rules/consistent-component-styles.ts +++ b/packages/eslint-plugin/src/rules/consistent-component-styles.ts @@ -57,18 +57,12 @@ export default createESLintRule({ node, messageId: 'useStylesArray', fix: (fixer) => { - if (ASTUtils.isStringLiteral(node)) { - return [fixer.replaceText(node, `[${node.raw}]`)]; - } - if (ASTUtils.isTemplateLiteral(node)) { - return [ - fixer.replaceText( - node, - `[${context.getSourceCode().getText(node)}]`, - ), - ]; - } - return []; + return fixer.replaceText( + node, + ASTUtils.isStringLiteral(node) + ? `[${node.raw}]` + : `[${context.getSourceCode().getText(node)}]`, + ); }, }); }, @@ -78,22 +72,14 @@ export default createESLintRule({ node, messageId: 'useStyleUrls', fix: (fixer) => { - if (ASTUtils.isStringLiteral(node.value)) { - return [ - fixer.replaceText(node, `styleUrls: [${node.value.raw}]`), - ]; - } - if (ASTUtils.isTemplateLiteral(node.value)) { - return [ - fixer.replaceText( - node, - `styleUrls: [${context + return fixer.replaceText( + node, + ASTUtils.isStringLiteral(node.value) + ? `styleUrls: [${node.value.raw}]` + : `styleUrls: [${context .getSourceCode() .getText(node.value)}]`, - ), - ]; - } - return []; + ); }, }); }, @@ -109,50 +95,36 @@ export default createESLintRule({ return { [singleStylesArrayExpression](node: TSESTree.ArrayExpression) { + // The selector ensures the element is not null. + const el = node.elements[0]!; + context.report({ node, messageId: 'useStylesString', fix: (fixer) => { - const [el] = node.elements; - if (el) { - if (ASTUtils.isStringLiteral(el)) { - return [fixer.replaceText(node, el.raw)]; - } - if (ASTUtils.isTemplateLiteral(el)) { - return [ - fixer.replaceText( - node, - context.getSourceCode().getText(el), - ), - ]; - } - } - return []; + return fixer.replaceText( + node, + ASTUtils.isStringLiteral(el) + ? el.raw + : context.getSourceCode().getText(el), + ); }, }); }, [singleStyleUrlsProperty](node: TSESTree.Property) { - if (!ASTUtils.isArrayExpression(node.value)) return; - const [el] = node.value.elements; + // The selector ensures the value is an array with a single non-null element. + const el = (node.value as TSESTree.ArrayExpression).elements[0]!; context.report({ node, messageId: 'useStyleUrl', fix: (fixer) => { - if (el) { - if (ASTUtils.isStringLiteral(el)) { - return [fixer.replaceText(node, `styleUrl: ${el.raw}`)]; - } - if (ASTUtils.isTemplateLiteral(el)) { - return [ - fixer.replaceText( - node, - `styleUrl: ${context.getSourceCode().getText(el)}`, - ), - ]; - } - } - return []; + return fixer.replaceText( + node, + ASTUtils.isStringLiteral(el) + ? `styleUrl: ${el.raw}` + : `styleUrl: ${context.getSourceCode().getText(el)}`, + ); }, }); }, diff --git a/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts b/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts index 251007c1c..0dc9912ec 100644 --- a/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts +++ b/packages/eslint-plugin/tests/rules/consistent-component-styles/cases.ts @@ -425,6 +425,24 @@ export const invalid = [ class Test {} `, }), + convertAnnotatedSourceToFailureCase({ + description: `should keep backtick quotes when fixing styleUrls array to styleUrl string`, + annotatedSource: ` + @Component({ + styleUrls: [\`./test.component.css\`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + }) + class Test {} + `, + messageId: messageIdUseStyleUrl, + annotatedOutput: ` + @Component({ + styleUrl: \`./test.component.css\` + + }) + class Test {} + `, + }), // String convertAnnotatedSourceToFailureCase({ description: `should fail when a component has a single style array value when string is preferred`, From 0ac33703899b7fe1dc1acdb10322a550bbcfc02d Mon Sep 17 00:00:00 2001 From: reduckted Date: Thu, 8 Feb 2024 19:37:59 +1000 Subject: [PATCH 06/10] feat(eslint-plugin): remove old docs --- .../docs/rules/no-single-styles-array.md | 426 ------------------ 1 file changed, 426 deletions(-) delete mode 100644 packages/eslint-plugin/docs/rules/no-single-styles-array.md diff --git a/packages/eslint-plugin/docs/rules/no-single-styles-array.md b/packages/eslint-plugin/docs/rules/no-single-styles-array.md deleted file mode 100644 index 35fc63694..000000000 --- a/packages/eslint-plugin/docs/rules/no-single-styles-array.md +++ /dev/null @@ -1,426 +0,0 @@ - - -
- -# `@angular-eslint/no-single-styles-array` - -Ensures component `styles`/`styleUrl` with `string` is used over `styles`/`styleUrls` when there is only a single string in the array - -- Type: suggestion -- 🔧 Supports autofix (`--fix`) - -
- -## Rule Options - -The rule does not have any configuration options. - -
- -## Usage Examples - -> The following examples are generated automatically from the actual unit tests within the plugin, so you can be assured that their behavior is accurate based on the current commit. - -
- -
-❌ - Toggle examples of incorrect code for this rule - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ❌ Invalid Code - -```ts -@Component({ - styles: [':host { display: block; }'] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ❌ Invalid Code - -```ts -@Component({ - standalone: true, - imports: [MatButtonModule], - styles: [':host { display: block; }'], - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - providers: [] -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ❌ Invalid Code - -```ts -@Component({ - styleUrls: ['./test.component.css'] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ❌ Invalid Code - -```ts -@Component({ - standalone: true, - imports: [MatButtonModule], - styleUrls: ['./test.component.css'], - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - providers: [] -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ❌ Invalid Code - -```ts -@Component({ - styles: [`:host { display: block; }`] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ❌ Invalid Code - -```ts -@Component({ - styleUrls: [`./test.component.css`] - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -}) -class Test {} -``` - -
- -
- ---- - -
- -
-✅ - Toggle examples of correct code for this rule - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ✅ Valid Code - -```ts -@Component({ - styles: ':host { display: block; }', -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ✅ Valid Code - -```ts -@Component({ - styles: ` - :host { display: block; } - `, -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ✅ Valid Code - -```ts -@Component({ - selector: 'my-test', - standalone: true, - imports: [CommonModule], - styles: ` - :host { display: block; } - `, - providers: [FooService] -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ✅ Valid Code - -```ts -@Component({ - styles: [ - ':host { display: block; }', - `.foo { color: red; }` - ], -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ✅ Valid Code - -```ts -@Component({ - styleUrls: [ - '../shared.css', - `./test.component.css` - ], -}) -class Test {} -``` - -
- ---- - -
- -#### Default Config - -```json -{ - "rules": { - "@angular-eslint/no-single-styles-array": [ - "error" - ] - } -} -``` - -
- -#### ✅ Valid Code - -```ts -@Component({ - selector: 'my-test', - standalone: true, - imports: [CommonModule], - styleUrls: [ - '../shared.css', - `./test.component.css` - ], - providers: [FooService] -}) -class Test {} -``` - -
- -
From ce0adf894808653d2f5116edd7a8f5998eeeb5e5 Mon Sep 17 00:00:00 2001 From: reduckted Date: Thu, 8 Feb 2024 19:48:23 +1000 Subject: [PATCH 07/10] feat(eslint-plugin): update consistent-component-styles docs --- .../docs/rules/consistent-component-styles.md | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/consistent-component-styles.md b/packages/eslint-plugin/docs/rules/consistent-component-styles.md index e2eb33b38..36f93fc5a 100644 --- a/packages/eslint-plugin/docs/rules/consistent-component-styles.md +++ b/packages/eslint-plugin/docs/rules/consistent-component-styles.md @@ -321,6 +321,36 @@ class Test {}
+#### Default Config + +```json +{ + "rules": { + "@angular-eslint/consistent-component-styles": [ + "error" + ] + } +} +``` + +
+ +#### ❌ Invalid Code + +```ts +@Component({ + styleUrls: [`./test.component.css`] + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +}) +class Test {} +``` + +
+ +--- + +
+ #### Custom Config ```json From c0918b64bf3700dd15da841f08b75798d0ea5570 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CJamesHenry=E2=80=9D?= Date: Sat, 16 Mar 2024 00:38:00 +0400 Subject: [PATCH 08/10] chore: update rule description --- .../eslint-plugin/docs/rules/consistent-component-styles.md | 2 +- packages/eslint-plugin/src/rules/consistent-component-styles.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/consistent-component-styles.md b/packages/eslint-plugin/docs/rules/consistent-component-styles.md index 36f93fc5a..42aa88b68 100644 --- a/packages/eslint-plugin/docs/rules/consistent-component-styles.md +++ b/packages/eslint-plugin/docs/rules/consistent-component-styles.md @@ -15,7 +15,7 @@ # `@angular-eslint/consistent-component-styles` -Ensures component `styles`/`styleUrl` with `string` is used over `styles`/`styleUrls` when there is only a single string in the array +Ensures consistent usage of `styles`/`styleUrls`/`styleUrl` within Component metadata - Type: suggestion - 🔧 Supports autofix (`--fix`) diff --git a/packages/eslint-plugin/src/rules/consistent-component-styles.ts b/packages/eslint-plugin/src/rules/consistent-component-styles.ts index a0e1efdbc..a21a14d0b 100644 --- a/packages/eslint-plugin/src/rules/consistent-component-styles.ts +++ b/packages/eslint-plugin/src/rules/consistent-component-styles.ts @@ -17,7 +17,7 @@ export default createESLintRule({ type: 'suggestion', docs: { description: - 'Ensures component `styles`/`styleUrl` with `string` is used over `styles`/`styleUrls` when there is only a single string in the array', + 'Ensures consistent usage of `styles`/`styleUrls`/`styleUrl` within Component metadata', }, fixable: 'code', schema: [ From 07fe5412df84cc7778a80533df724840b6bc193f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CJamesHenry=E2=80=9D?= Date: Sat, 16 Mar 2024 00:49:08 +0400 Subject: [PATCH 09/10] chore: update ci config --- .github/workflows/ci.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 07b92f460..50776dce4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: echo "NX_CI_EXECUTION_ENV=Node $(node --version) -" >> $GITHUB_ENV - name: Start Nx Cloud CI Run - run: npx nx-cloud start-ci-run --distributes-on="6 custom-linux-medium-plus-js" + run: npx nx-cloud start-ci-run --distributes-on="6 custom-linux-medium-plus-js" --stop-agents-after="e2e-suite" - uses: actions/cache@v4 id: yarn-cache # use this to check for `cache-hit` (`steps.yarn-cache.outputs.cache-hit != 'true'`) @@ -61,8 +61,10 @@ jobs: cmd2: yarn nx run-many -t test --codeCoverage cmd3: yarn nx-cloud record -- yarn lint cmd4: yarn nx-cloud record -- yarn format-check - # Run distributed e2e test suites with independent local registries (max 1 per agent via parallel=1) - cmd5: yarn nx run-many -t e2e-suite --parallel 1 + + # Run distributed e2e test suites with independent local registries (max 1 per agent via parallel=1) + - name: Run e2e test suites + run: yarn nx run-many -t e2e-suite --parallel 1 - name: Publish code coverage report uses: codecov/codecov-action@v4 From 761f43580a84c0d7651321fae5674656d6b7c91d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CJamesHenry=E2=80=9D?= Date: Sat, 16 Mar 2024 00:55:50 +0400 Subject: [PATCH 10/10] chore: update ci config --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 50776dce4..8e6309650 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,14 +57,14 @@ jobs: with: # Note that the typecheck target *also* typechecks tests and tools, # whereas the build only checks src files - cmd1: yarn nx run-many -t build,typecheck,check-rule-docs,lint - cmd2: yarn nx run-many -t test --codeCoverage - cmd3: yarn nx-cloud record -- yarn lint - cmd4: yarn nx-cloud record -- yarn format-check + cmd1: npx nx run-many -t build,typecheck,check-rule-docs,lint + cmd2: npx nx run-many -t test --codeCoverage + cmd3: npx nx-cloud record -- yarn lint + cmd4: npx nx-cloud record -- yarn format-check # Run distributed e2e test suites with independent local registries (max 1 per agent via parallel=1) - name: Run e2e test suites - run: yarn nx run-many -t e2e-suite --parallel 1 + run: npx nx run-many -t e2e-suite --parallel 1 - name: Publish code coverage report uses: codecov/codecov-action@v4