From 077a22434f3139655dc28e82bd06552e24323ad8 Mon Sep 17 00:00:00 2001 From: James Garbutt <43081j@users.noreply.github.com> Date: Mon, 4 Dec 2023 20:39:08 +0000 Subject: [PATCH] feat: new rule attribute-names (#191) This introduces a new rule: `attribute-names`. The rule enforces that all attributes are lowercase if the associated property is not. For example: ```ts class Foo extends LitElement { @property() camelCase = 'foo'; } ``` This will fail since the default attribute will be `camelCase`, and therefore not lowercase. This should instead be: ```ts @property({attribute: 'camel-case'}) ``` Though in this rule, for now, the fact it is snake-case is a preference rather than being enforced by the rule. --- README.md | 1 + docs/rules/attribute-names.md | 43 +++++++++ src/rules/attribute-names.ts | 69 ++++++++++++++ src/test/rules/attribute-names_test.ts | 125 +++++++++++++++++++++++++ src/test/util_test.ts | 51 +++++++++- src/util.ts | 15 ++- 6 files changed, 296 insertions(+), 8 deletions(-) create mode 100644 docs/rules/attribute-names.md create mode 100644 src/rules/attribute-names.ts create mode 100644 src/test/rules/attribute-names_test.ts diff --git a/README.md b/README.md index fe747d0..c0eba26 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ If you want more fine-grained configuration, you can instead add a snippet like ## List of supported rules +- [lit/attribute-names](docs/rules/attribute-names.md) - [lit/attribute-value-entities](docs/rules/attribute-value-entities.md) - [lit/ban-attributes](docs/rules/ban-attributes.md) - [lit/binding-positions](docs/rules/binding-positions.md) diff --git a/docs/rules/attribute-names.md b/docs/rules/attribute-names.md new file mode 100644 index 0000000..f65e4c3 --- /dev/null +++ b/docs/rules/attribute-names.md @@ -0,0 +1,43 @@ +# Enforces attribute naming conventions + +Attributes are always treated lowercase, but it is common to have camelCase +property names. In these situations, an explicit lowercase attribute should +be supplied. + +Further, camelCase names should ideally be exposed as snake-case attributes. + +## Rule Details + +This rule enforces that all lit properties have equivalent lower case attributes +exposed. + +The following patterns are considered warnings: + +```ts +// Using decorators: + +@property() camelCaseName: string; + +// Using a getter: + +static get properties() { + return { + camelCaseName2: {type: String} + }; +} +``` + +The following patterns are not warnings: + +```ts +@property({attribute: 'camel-case-name'}) +camelCaseName: string; + +@property() +lower: string; +``` + +## When Not To Use It + +If you prefer other naming conventions for attributes, this rule should not +be used. diff --git a/src/rules/attribute-names.ts b/src/rules/attribute-names.ts new file mode 100644 index 0000000..c0b1ed8 --- /dev/null +++ b/src/rules/attribute-names.ts @@ -0,0 +1,69 @@ +/** + * @fileoverview Enforces attribute naming conventions + * @author James Garbutt + */ + +import {Rule} from 'eslint'; +import * as ESTree from 'estree'; +import {getPropertyMap, isLitClass} from '../util'; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +const rule: Rule.RuleModule = { + meta: { + docs: { + description: 'Enforces attribute naming conventions', + recommended: true, + url: 'https://github.com/43081j/eslint-plugin-lit/blob/master/docs/rules/attribute-names.md' + }, + schema: [], + messages: { + casedAttribute: + 'Attributes are case-insensitive and therefore should be ' + + 'defined in lower case', + casedPropertyWithoutAttribute: + 'Property has non-lowercase casing but no attribute. It should ' + + 'instead have an explicit `attribute` set to the lower case ' + + 'name (usually snake-case)' + } + }, + + create(context): Rule.RuleListener { + return { + ClassDeclaration: (node: ESTree.Class): void => { + if (isLitClass(node)) { + const propertyMap = getPropertyMap(node); + + for (const [prop, propConfig] of propertyMap.entries()) { + if (!propConfig.attribute) { + continue; + } + + if (!propConfig.attributeName) { + if (prop.toLowerCase() !== prop) { + context.report({ + node: propConfig.key, + messageId: 'casedPropertyWithoutAttribute' + }); + } + } else { + if ( + propConfig.attributeName.toLowerCase() !== + propConfig.attributeName + ) { + context.report({ + node: propConfig.expr ?? propConfig.key, + messageId: 'casedAttribute' + }); + } + } + } + } + } + }; + } +}; + +export = rule; diff --git a/src/test/rules/attribute-names_test.ts b/src/test/rules/attribute-names_test.ts new file mode 100644 index 0000000..08231c7 --- /dev/null +++ b/src/test/rules/attribute-names_test.ts @@ -0,0 +1,125 @@ +/** + * @fileoverview Enforces attribute naming conventions + * @author James Garbutt + */ + +import rule = require('../../rules/attribute-names'); +import {RuleTester} from 'eslint'; + +const ruleTester = new RuleTester({ + parserOptions: { + sourceType: 'module', + ecmaVersion: 2015 + } +}); + +const parser = require.resolve('@babel/eslint-parser'); +const parserOptions = { + requireConfigFile: false, + babelOptions: { + plugins: [ + ['@babel/plugin-proposal-decorators', {decoratorsBeforeExport: true}] + ] + } +}; + +ruleTester.run('attribute-names', rule, { + valid: [ + 'class Foo {}', + `class Foo { + static get properties() { + return { + whateverCaseYouWant: {type: String} + }; + } + }`, + `class Foo extends LitElement { + static get properties() { + return { + lowercase: {type: String} + }; + } + }`, + `class Foo extends Litelement { + static get properties() { + return { + camelCase: {type: String, attribute: 'lowercase'} + }; + } + }`, + { + code: `class Foo extends LitElement { + @property({ type: String }) + lowercase = 'foo'; + }`, + parser, + parserOptions + }, + { + code: `class Foo extends LitElement { + @property({ type: String, attribute: 'lowercase' }) + camelCase = 'foo'; + }`, + parser, + parserOptions + }, + { + code: `class Foo extends LitElement { + @property({ type: String, attribute: false }) + camelCase = 'foo'; + }`, + parser, + parserOptions + } + ], + + invalid: [ + { + code: `class Foo extends LitElement { + static get properties() { + return { + camelCase: {type: String} + }; + } + }`, + errors: [ + { + line: 4, + column: 13, + messageId: 'casedPropertyWithoutAttribute' + } + ] + }, + { + code: `class Foo extends LitElement { + static get properties() { + return { + camelCase: {type: String, attribute: 'stillCamelCase'} + }; + } + }`, + errors: [ + { + line: 4, + column: 24, + messageId: 'casedAttribute' + } + ] + }, + { + code: `class Foo extends LitElement { + @property({ type: String }) + camelCase = 'foo'; + }`, + parser, + parserOptions, + errors: [ + { + line: 3, + column: 9, + messageId: 'casedPropertyWithoutAttribute' + } + ] + } + ] +}); diff --git a/src/test/util_test.ts b/src/test/util_test.ts index d5d72bc..3a72b43 100644 --- a/src/test/util_test.ts +++ b/src/test/util_test.ts @@ -56,7 +56,8 @@ describe('util', () => { key, expr: node, state: false, - attribute: true + attribute: true, + attributeName: undefined }); }); @@ -76,7 +77,8 @@ describe('util', () => { key, expr: node, state: false, - attribute: true + attribute: true, + attributeName: undefined }); }); @@ -111,7 +113,8 @@ describe('util', () => { key, expr: node, state: true, - attribute: true + attribute: true, + attributeName: undefined }); }); @@ -146,7 +149,44 @@ describe('util', () => { key, expr: node, state: false, - attribute: false + attribute: false, + attributeName: undefined + }); + }); + + it('should extract attribute names', () => { + const node: ESTree.ObjectExpression = { + type: 'ObjectExpression', + properties: [ + { + type: 'Property', + kind: 'init', + method: false, + shorthand: false, + computed: false, + key: { + type: 'Identifier', + name: 'attribute' + }, + value: { + type: 'Literal', + value: 'boop' + } + } + ] + }; + const key: ESTree.Identifier = { + type: 'Identifier', + name: 'foo' + }; + const entry = util.extractPropertyEntry(key, node); + + expect(entry).to.deep.equal({ + key, + expr: node, + state: false, + attribute: true, + attributeName: 'boop' }); }); @@ -182,7 +222,8 @@ describe('util', () => { key, expr: node, state: false, - attribute: true + attribute: true, + attributeName: undefined }); }); }); diff --git a/src/util.ts b/src/util.ts index 3e6a36e..74a6d22 100644 --- a/src/util.ts +++ b/src/util.ts @@ -70,6 +70,7 @@ export interface PropertyMapEntry { expr: ESTree.ObjectExpression | null; state: boolean; attribute: boolean; + attributeName?: string; } /** @@ -84,6 +85,7 @@ export function extractPropertyEntry( ): PropertyMapEntry { let state = false; let attribute = true; + let attributeName: string | undefined = undefined; for (const prop of value.properties) { if ( @@ -93,8 +95,14 @@ export function extractPropertyEntry( ) { if (prop.key.name === 'state' && prop.value.value === true) { state = true; - } else if (prop.key.name === 'attribute' && prop.value.value === false) { - attribute = false; + } + + if (prop.key.name === 'attribute') { + if (prop.value.value === false) { + attribute = false; + } else if (typeof prop.value.value === 'string') { + attributeName = prop.value.value; + } } } } @@ -103,7 +111,8 @@ export function extractPropertyEntry( expr: value, key, state, - attribute + attribute, + attributeName }; }