Skip to content

Commit c25838a

Browse files
committed
chore(internals): add new rules for class inheritance and aria attribute management
- Introduced `no-deep-class-inheritance` rule to limit class inheritance depth, ensuring better maintainability. - Added `no-host-managed-aria-attributes` rule to prevent misuse of ARIA attributes that should be managed through ElementInternals. - Implemented `no-single-consumer-internal-base` rule to disallow internal base classes with fewer than two consumers, promoting better design practices. - Updated ESLint configurations to include the new rules and their respective error handling. Signed-off-by: Cory Rylan <crylan@nvidia.com>
1 parent f3906cb commit c25838a

8 files changed

Lines changed: 1181 additions & 1 deletion

projects/internals/eslint/src/configs/lit.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ import requireElementDefinitions from '../local/require-element-definitions.js';
1616
import requireTestCompleteness from '../local/require-test-completeness.js';
1717
import requireComposedEvents from '../local/require-composed-events.js';
1818
import noMissingBundleRegistration from '../local/no-missing-bundle-registration.js';
19+
import noHostManagedAriaAttributes from '../local/no-host-managed-aria-attributes.js';
20+
import noSingleConsumerInternalBase from '../local/no-single-consumer-internal-base.js';
1921

2022
const source = ['src/**/*.ts', 'src/**/*.tsx', 'src/**/*.d.ts'];
2123
const tests = [
@@ -64,7 +66,9 @@ export const litConfig = [
6466
'require-element-definitions': requireElementDefinitions,
6567
'require-test-completeness': requireTestCompleteness,
6668
'require-composed-events': requireComposedEvents,
67-
'no-missing-bundle-registration': noMissingBundleRegistration
69+
'no-missing-bundle-registration': noMissingBundleRegistration,
70+
'no-host-managed-aria-attributes': noHostManagedAriaAttributes,
71+
'no-single-consumer-internal-base': noSingleConsumerInternalBase
6872
}
6973
}
7074
},
@@ -134,6 +138,8 @@ export const litConfig = [
134138
'local/require-internal-host': ['error'],
135139
'local/require-element-definitions': ['error'],
136140
'local/require-composed-events': ['error'],
141+
'local/no-host-managed-aria-attributes': ['error'],
142+
'local/no-single-consumer-internal-base': ['error'],
137143
'local/no-missing-bundle-registration': [
138144
'error',
139145
{

projects/internals/eslint/src/configs/typescript.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import tseslint from 'typescript-eslint';
33
import importPlugin from 'eslint-plugin-import';
44
import jsdoc from 'eslint-plugin-jsdoc';
55
import deadCode from '../local/dead-code.js';
6+
import noDeepClassInheritance from '../local/no-deep-class-inheritance.js';
67
import exampleMetadata from '../local/example-metadata.js';
78
import exampleNaming from '../local/example-naming.js';
89
import exampleTemplateSize from '../local/example-template-size.js';
@@ -48,6 +49,7 @@ const config = {
4849
'local-typescript': {
4950
rules: {
5051
'no-dead-code': deadCode,
52+
'no-deep-class-inheritance': noDeepClassInheritance,
5153
'example-metadata': exampleMetadata,
5254
'example-naming': exampleNaming,
5355
'example-template-size': exampleTemplateSize,
@@ -106,6 +108,10 @@ const config = {
106108
'jsdoc/check-tag-names': 'error',
107109
'jsdoc/informative-docs': 'error',
108110
'local-typescript/no-dead-code': ['warn'], // todo, this should be migrated to the internal playground template config
111+
'local-typescript/no-deep-class-inheritance': [
112+
'error',
113+
{ maxDepth: 2, allowedRoots: ['HTMLElement', 'LitElement', 'FormControlMixin', 'BaseButton'] }
114+
],
109115
'local-typescript/require-listener-cleanup': 'error',
110116
'local-typescript/require-observer-cleanup': 'error',
111117
'local-typescript/require-timer-cleanup': 'error',
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
const DEFAULT_MAX_DEPTH = 2;
2+
const DEFAULT_ALLOWED_ROOTS = ['HTMLElement', 'LitElement'];
3+
4+
/**
5+
* ESLint rule that limits class inheritance depth. The rule counts superclass
6+
* hops until it reaches a configured allowed root class so local wrappers around
7+
* platform/framework bases stay possible without allowing hierarchy creep.
8+
*
9+
* @type {import('eslint').Rule.RuleModule}
10+
*/
11+
export default {
12+
meta: {
13+
type: 'problem',
14+
name: 'no-deep-class-inheritance',
15+
docs: {
16+
description: 'Disallows class inheritance chains deeper than the configured maximum.',
17+
category: 'Best Practice',
18+
recommended: true
19+
},
20+
schema: [
21+
{
22+
type: 'object',
23+
additionalProperties: false,
24+
properties: {
25+
maxDepth: {
26+
type: 'integer',
27+
minimum: 1
28+
},
29+
allowedRoots: {
30+
type: 'array',
31+
items: { type: 'string' },
32+
uniqueItems: true
33+
}
34+
}
35+
}
36+
],
37+
messages: {
38+
'too-deep':
39+
'`{{className}}` has inheritance depth {{depth}} (`{{chain}}`). Maximum allowed depth is {{maxDepth}}.'
40+
}
41+
},
42+
create(context) {
43+
const options = context.options[0] ?? {};
44+
const maxDepth = options.maxDepth ?? DEFAULT_MAX_DEPTH;
45+
const allowedRoots = new Set(options.allowedRoots ?? DEFAULT_ALLOWED_ROOTS);
46+
47+
return {
48+
ClassDeclaration(node) {
49+
if (!node.superClass) {
50+
return;
51+
}
52+
53+
const services = context.sourceCode.parserServices;
54+
if (!services?.program || !services.esTreeNodeToTSNodeMap) {
55+
return;
56+
}
57+
58+
const superClass = services.esTreeNodeToTSNodeMap.get(node.superClass);
59+
const chain = getInheritanceChain(superClass, services.program.getTypeChecker(), allowedRoots);
60+
61+
if (chain.length <= maxDepth) {
62+
return;
63+
}
64+
65+
context.report({
66+
node,
67+
messageId: 'too-deep',
68+
data: {
69+
className: node.id?.name ?? '<anonymous>',
70+
depth: String(chain.length),
71+
maxDepth: String(maxDepth),
72+
chain: `${node.id?.name ?? '<anonymous>'} -> ${chain.join(' -> ')}`
73+
}
74+
});
75+
}
76+
};
77+
}
78+
};
79+
80+
function getInheritanceChain(superClass, checker, allowedRoots) {
81+
const chain = [];
82+
const visited = new Set();
83+
let expression = superClass;
84+
85+
while (expression) {
86+
const symbol = checker.getTypeAtLocation(expression).symbol;
87+
const className = symbol?.getName() ?? expression.getText();
88+
89+
chain.push(className);
90+
91+
if (allowedRoots.has(className)) {
92+
break;
93+
}
94+
95+
const declaration = getClassDeclaration(symbol);
96+
if (!declaration || visited.has(declaration)) {
97+
break;
98+
}
99+
100+
visited.add(declaration);
101+
expression = getExtendsExpression(declaration);
102+
}
103+
104+
return chain;
105+
}
106+
107+
function getClassDeclaration(symbol) {
108+
return symbol?.declarations?.find(declaration => Array.isArray(declaration.members)) ?? null;
109+
}
110+
111+
function getExtendsExpression(classDeclaration) {
112+
const heritageClause = classDeclaration.heritageClauses?.find(clause => clause.getText().startsWith('extends '));
113+
return heritageClause?.types?.[0]?.expression ?? null;
114+
}
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { beforeEach, test } from 'node:test';
2+
import assert from 'node:assert/strict';
3+
import { RuleTester } from 'eslint';
4+
import tseslint from 'typescript-eslint';
5+
import noDeepClassInheritance from './no-deep-class-inheritance.js';
6+
7+
let tester;
8+
9+
beforeEach(() => {
10+
tester = new RuleTester({
11+
languageOptions: {
12+
parser: tseslint.parser,
13+
parserOptions: {
14+
ecmaVersion: 'latest',
15+
sourceType: 'module',
16+
projectService: { allowDefaultProject: ['*.ts'] },
17+
tsconfigRootDir: import.meta.dirname
18+
}
19+
}
20+
});
21+
});
22+
23+
test('defines rule metadata', () => {
24+
assert.equal(noDeepClassInheritance.meta.type, 'problem');
25+
assert.equal(noDeepClassInheritance.meta.name, 'no-deep-class-inheritance');
26+
assert.ok(noDeepClassInheritance.meta.messages['too-deep']);
27+
});
28+
29+
test('valid: allows direct and depth-2 class inheritance', () => {
30+
tester.run('no-deep-class-inheritance', noDeepClassInheritance, {
31+
valid: [
32+
{
33+
filename: 'direct-lit.ts',
34+
code: `
35+
declare class LitElement {}
36+
37+
class Badge extends LitElement {}
38+
`
39+
},
40+
{
41+
filename: 'depth-two.ts',
42+
code: `
43+
declare class LitElement {}
44+
45+
class BaseButton extends LitElement {}
46+
class SortButton extends BaseButton {}
47+
`
48+
},
49+
{
50+
filename: 'event-target-root.ts',
51+
code: `
52+
class VideoGroup extends EventTarget {}
53+
class ManagedVideoGroup extends VideoGroup {}
54+
`
55+
}
56+
],
57+
invalid: []
58+
});
59+
});
60+
61+
test('valid: supports custom maxDepth and allowedRoots options', () => {
62+
tester.run('no-deep-class-inheritance', noDeepClassInheritance, {
63+
valid: [
64+
{
65+
filename: 'custom-max-depth.ts',
66+
options: [{ maxDepth: 3 }],
67+
code: `
68+
declare class LitElement {}
69+
70+
class BaseButton extends LitElement {}
71+
class SortButton extends BaseButton {}
72+
class ToolbarSortButton extends SortButton {}
73+
`
74+
},
75+
{
76+
filename: 'custom-root.ts',
77+
options: [{ allowedRoots: ['BaseElement'] }],
78+
code: `
79+
class BaseElement {}
80+
class Control extends BaseElement {}
81+
class Color extends Control {}
82+
`
83+
}
84+
],
85+
invalid: []
86+
});
87+
});
88+
89+
test('invalid: reports classes deeper than maxDepth', () => {
90+
tester.run('no-deep-class-inheritance', noDeepClassInheritance, {
91+
valid: [],
92+
invalid: [
93+
{
94+
filename: 'too-deep.ts',
95+
code: `
96+
declare class LitElement {}
97+
98+
class BaseButton extends LitElement {}
99+
class SortButton extends BaseButton {}
100+
class ToolbarSortButton extends SortButton {}
101+
`,
102+
errors: [
103+
{
104+
messageId: 'too-deep',
105+
data: {
106+
className: 'ToolbarSortButton',
107+
depth: '3',
108+
maxDepth: '2',
109+
chain: 'ToolbarSortButton -> SortButton -> BaseButton -> LitElement'
110+
}
111+
}
112+
]
113+
},
114+
{
115+
filename: 'custom-max-depth-invalid.ts',
116+
options: [{ maxDepth: 1, allowedRoots: ['BaseElement'] }],
117+
code: `
118+
class BaseElement {}
119+
120+
class Control extends BaseElement {}
121+
class Radio extends Control {}
122+
`,
123+
errors: [
124+
{
125+
messageId: 'too-deep',
126+
data: {
127+
className: 'Radio',
128+
depth: '2',
129+
maxDepth: '1',
130+
chain: 'Radio -> Control -> BaseElement'
131+
}
132+
}
133+
]
134+
}
135+
]
136+
});
137+
});
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
const MANAGED_ATTRIBUTES = {
2+
'aria-current': 'stateCurrent',
3+
'aria-disabled': 'stateDisabled',
4+
'aria-expanded': 'stateExpanded',
5+
'aria-label': 'ElementInternals.ariaLabel',
6+
'aria-pressed': 'statePressed',
7+
'aria-selected': 'stateSelected',
8+
role: 'ElementInternals.role'
9+
};
10+
11+
/**
12+
* Prevent setting host ARIA state attributes that should be managed through
13+
* ElementInternals and the shared state/type controllers.
14+
*
15+
* @type {import('eslint').Rule.RuleModule}
16+
*/
17+
export default {
18+
meta: {
19+
type: 'problem',
20+
name: 'no-host-managed-aria-attributes',
21+
docs: {
22+
description: 'Prevent host ARIA state attributes that should be managed with ElementInternals.',
23+
category: 'Best Practice',
24+
recommended: true
25+
},
26+
schema: [],
27+
messages: {
28+
'managed-attribute':
29+
'Use {{controller}} instead of setting "{{attribute}}" on the custom element host with {{method}}().'
30+
}
31+
},
32+
create(context) {
33+
return {
34+
CallExpression(node) {
35+
if (!isHostAttributeMutation(node)) {
36+
return;
37+
}
38+
39+
const attribute = getStaticString(node.arguments[0])?.toLowerCase();
40+
const controller = MANAGED_ATTRIBUTES[attribute];
41+
42+
if (!controller) {
43+
return;
44+
}
45+
46+
context.report({
47+
node,
48+
messageId: 'managed-attribute',
49+
data: {
50+
attribute,
51+
controller,
52+
method: node.callee.property.name
53+
}
54+
});
55+
}
56+
};
57+
}
58+
};
59+
60+
function isHostAttributeMutation(node) {
61+
if (node.callee.type !== 'MemberExpression' || node.callee.computed) {
62+
return false;
63+
}
64+
65+
if (node.callee.object.type !== 'ThisExpression') {
66+
return false;
67+
}
68+
69+
return ['setAttribute', 'removeAttribute', 'toggleAttribute'].includes(node.callee.property.name);
70+
}
71+
72+
function getStaticString(node) {
73+
if (!node) {
74+
return null;
75+
}
76+
77+
if (node.type === 'Literal' && typeof node.value === 'string') {
78+
return node.value;
79+
}
80+
81+
if (node.type === 'TemplateLiteral' && node.expressions.length === 0) {
82+
return node.quasis[0]?.value.cooked ?? null;
83+
}
84+
85+
return null;
86+
}

0 commit comments

Comments
 (0)