Skip to content

Commit b2ea10e

Browse files
Merge pull request #2620 from NullVoxPopuli/nvp/template-lint-extract-rule-template-sort-invocations
Extract rule: template-sort-invocations
2 parents 9042859 + c41f0b2 commit b2ea10e

4 files changed

Lines changed: 1810 additions & 0 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ rules in templates can be disabled with eslint directives with mustache or html
283283
| [template-self-closing-void-elements](docs/rules/template-self-closing-void-elements.md) | require self-closing on void elements | | 🔧 | |
284284
| [template-simple-modifiers](docs/rules/template-simple-modifiers.md) | require simple modifier syntax | | | |
285285
| [template-simple-unless](docs/rules/template-simple-unless.md) | require simple conditions in unless blocks | | | |
286+
| [template-sort-invocations](docs/rules/template-sort-invocations.md) | require sorted attributes and modifiers | | | |
286287
| [template-splat-attributes-only](docs/rules/template-splat-attributes-only.md) | disallow ...spread other than ...attributes | | | |
287288
| [template-style-concatenation](docs/rules/template-style-concatenation.md) | disallow string concatenation in inline styles | | | |
288289

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# ember/template-sort-invocations
2+
3+
<!-- end auto-generated rule header -->
4+
5+
## Why use it?
6+
7+
The rule helps you standardize templates:
8+
9+
- Component invocations
10+
- Helper invocations
11+
- Modifier invocations
12+
13+
By sorting things that are order-independent, you can more easily refactor code. In addition, sorting removes style differences, so you can review another person's code more effectively.
14+
15+
> [!TIP]
16+
>
17+
> The `--fix` option for this rule doesn't preserve formatting. You can use `prettier`, [`prettier-plugin-ember-hbs-tag`](https://github.com/ijlee2/prettier-plugin-ember-hbs-tag), and [`prettier-plugin-ember-template-tag`](https://github.com/ember-tooling/prettier-plugin-ember-template-tag) to format templates in `*.hbs`, `hbs` tags, and `<template>` tags, respectively.
18+
19+
## Examples
20+
21+
### Components
22+
23+
When invoking a component, list things in the following order:
24+
25+
1. Arguments
26+
2. Attributes
27+
3. Modifiers
28+
4. Splattributes
29+
30+
The order clearly shows how the component is customized more and more. Things are alphabetized within each group.
31+
32+
```hbs
33+
<Ui::Button
34+
@label='Submit form'
35+
@type='submit'
36+
data-test-button
37+
{{on 'click' this.doSomething}}
38+
...attributes
39+
/>
40+
```
41+
42+
> [!NOTE]
43+
>
44+
> In rare cases, the order of [`...attributes`](https://guides.emberjs.com/release/components/component-arguments-and-html-attributes/#toc_html-attributes) can matter. Similarly, the order can matter when an [ARIA attribute has multiple values](https://github.com/ijlee2/ember-container-query/issues/38#issuecomment-647017665).
45+
>
46+
> Disable the rule per instance in either case.
47+
48+
### Helpers
49+
50+
When invoking a helper, list the named arguments in alphabetical order.
51+
52+
```hbs
53+
{{t
54+
'my-component.description'
55+
installedOn=this.installationDate
56+
packageName='ember-source'
57+
packageVersion='6.0.0'
58+
}}
59+
```
60+
61+
### Modifiers
62+
63+
Similarly to helpers, list the named arguments in alphabetical order.
64+
65+
## Limitations
66+
67+
It's intended that there are no options for sorting. Alphabetical sort is the simplest for everyone to understand and to apply across different projects. It's also the easiest to maintain.
68+
69+
To better meet your needs, consider creating a plugin for `ember-template-lint`.
70+
71+
## Known issues
72+
73+
1\. If you passed an empty string as an argument's value, it has been replaced with `{{""}}`. Let [`ember-template-lint`](https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-unnecessary-curly-strings.md) fix the formatting change.
74+
75+
```diff
76+
- <MyComponent @description={{""}} />
77+
+ <MyComponent @description="" />
78+
```
79+
80+
2\. Comments such as `{{! @glint-expect-error }}` may have shifted. Move them to the correct location.
81+
82+
## References
83+
84+
Source code and tests were copied from [`ember-codemod-sort-invocations`](https://github.com/ijlee2/ember-codemod-sort-invocations).
Lines changed: 289 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,289 @@
1+
/* eslint-disable unicorn/consistent-function-scoping, unicorn/prefer-at */
2+
/** @type {import('eslint').Rule.RuleModule} */
3+
module.exports = {
4+
meta: {
5+
type: 'suggestion',
6+
docs: {
7+
description: 'require sorted attributes and modifiers',
8+
category: 'Best Practices',
9+
url: 'https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules/template-sort-invocations.md',
10+
templateMode: 'both',
11+
},
12+
fixable: null,
13+
schema: [],
14+
messages: {
15+
attributeOrder: '`{{attributeName}}` must appear after `{{expectedAfter}}`',
16+
modifierOrder: '`{{{{modifierName}}}}` must appear after `{{{{expectedAfter}}}}`',
17+
hashPairOrder: '`{{hashPairName}}` must appear after `{{expectedAfter}}`',
18+
splattributesOrder: '`...attributes` must appear after modifiers',
19+
},
20+
originallyFrom: {
21+
name: 'ember-template-lint',
22+
rule: 'lib/rules/sort-invocations.js',
23+
docs: 'docs/rule/sort-invocations.md',
24+
tests: 'test/unit/rules/sort-invocations-test.js',
25+
},
26+
},
27+
28+
create(context) {
29+
function getAttributeName(node) {
30+
return node.name;
31+
}
32+
33+
function getAttributePosition(node) {
34+
const name = getAttributeName(node);
35+
36+
if (name.startsWith('@')) {
37+
return 1; // Arguments first
38+
}
39+
40+
if (name === '...attributes') {
41+
return 3; // Splattributes last
42+
}
43+
44+
return 2; // Regular attributes in the middle
45+
}
46+
47+
function getHashPairName(node) {
48+
return node.key;
49+
}
50+
51+
function getModifierName(node) {
52+
if (node.path.type !== 'GlimmerPathExpression') {
53+
return '';
54+
}
55+
56+
return node.path.original;
57+
}
58+
59+
function compareAttributes(a, b) {
60+
const positionA = getAttributePosition(a);
61+
const positionB = getAttributePosition(b);
62+
63+
if (positionA !== positionB) {
64+
return positionA - positionB;
65+
}
66+
67+
const nameA = getAttributeName(a);
68+
const nameB = getAttributeName(b);
69+
70+
return nameA.localeCompare(nameB);
71+
}
72+
73+
function compareHashPairs(a, b) {
74+
const nameA = getHashPairName(a);
75+
const nameB = getHashPairName(b);
76+
77+
return nameA.localeCompare(nameB);
78+
}
79+
80+
function compareModifiers(a, b) {
81+
const nameA = getModifierName(a);
82+
const nameB = getModifierName(b);
83+
84+
if (nameA !== nameB) {
85+
return nameA.localeCompare(nameB);
86+
}
87+
88+
// For 'on' modifiers, sort by event name
89+
if (nameA === 'on' && a.params && b.params && a.params.length > 0 && b.params.length > 0) {
90+
const eventA = a.params[0];
91+
const eventB = b.params[0];
92+
93+
if (eventA.type === 'GlimmerStringLiteral' && eventB.type === 'GlimmerStringLiteral') {
94+
return eventA.value.localeCompare(eventB.value);
95+
}
96+
}
97+
98+
return 0;
99+
}
100+
101+
function getUnsortedAttributeIndex(attributes) {
102+
return attributes.findIndex((attribute, index) => {
103+
if (index === attributes.length - 1) {
104+
return false;
105+
}
106+
107+
return compareAttributes(attribute, attributes[index + 1]) > 0;
108+
});
109+
}
110+
111+
function getUnsortedHashPairIndex(pairs) {
112+
return pairs.findIndex((hashPair, index) => {
113+
if (index === pairs.length - 1) {
114+
return false;
115+
}
116+
117+
return compareHashPairs(hashPair, pairs[index + 1]) > 0;
118+
});
119+
}
120+
121+
function getUnsortedModifierIndex(modifiers) {
122+
return modifiers.findIndex((modifier, index) => {
123+
if (index === modifiers.length - 1) {
124+
return false;
125+
}
126+
127+
return compareModifiers(modifier, modifiers[index + 1]) > 0;
128+
});
129+
}
130+
131+
function canSkipSplattributesLast(node) {
132+
const { attributes, modifiers } = node;
133+
134+
if (!attributes || attributes.length === 0 || !modifiers || modifiers.length === 0) {
135+
return true;
136+
}
137+
138+
const splattributes = attributes.at(-1);
139+
const lastModifier = modifiers.at(-1);
140+
141+
if (!splattributes || splattributes.name !== '...attributes' || !lastModifier) {
142+
return true;
143+
}
144+
145+
// Check that ...attributes appears after the last modifier
146+
const splattributesPosition = splattributes.loc.start;
147+
const lastModifierPosition = lastModifier.loc.start;
148+
149+
if (splattributesPosition.line > lastModifierPosition.line) {
150+
return true;
151+
}
152+
153+
return (
154+
splattributesPosition.line === lastModifierPosition.line &&
155+
splattributesPosition.column > lastModifierPosition.column
156+
);
157+
}
158+
159+
return {
160+
GlimmerElementNode(node) {
161+
const { attributes, modifiers } = node;
162+
163+
if (attributes && attributes.length > 1) {
164+
const index = getUnsortedAttributeIndex(attributes);
165+
166+
if (index !== -1) {
167+
context.report({
168+
node: attributes[index],
169+
messageId: 'attributeOrder',
170+
data: {
171+
attributeName: getAttributeName(attributes[index]),
172+
expectedAfter: getAttributeName(attributes[index + 1]),
173+
},
174+
});
175+
}
176+
}
177+
178+
if (modifiers && modifiers.length > 1) {
179+
const index = getUnsortedModifierIndex(modifiers);
180+
181+
if (index !== -1) {
182+
context.report({
183+
node: modifiers[index],
184+
messageId: 'modifierOrder',
185+
data: {
186+
modifierName: getModifierName(modifiers[index]),
187+
expectedAfter: getModifierName(modifiers[index + 1]),
188+
},
189+
});
190+
}
191+
}
192+
193+
if (!canSkipSplattributesLast(node)) {
194+
const splattributes = attributes.at(-1);
195+
196+
// When ...attributes is the only attribute, report as attributeOrder
197+
// (the ordering issue is that ...attributes should appear after modifiers)
198+
if (attributes.length === 1) {
199+
context.report({
200+
node: splattributes,
201+
messageId: 'attributeOrder',
202+
data: {
203+
attributeName: '...attributes',
204+
expectedAfter: 'modifiers',
205+
},
206+
});
207+
} else {
208+
context.report({
209+
node: splattributes,
210+
messageId: 'splattributesOrder',
211+
});
212+
}
213+
}
214+
},
215+
216+
GlimmerBlockStatement(node) {
217+
if (node.hash && node.hash.pairs && node.hash.pairs.length > 1) {
218+
const index = getUnsortedHashPairIndex(node.hash.pairs);
219+
220+
if (index !== -1) {
221+
context.report({
222+
node: node.hash.pairs[index],
223+
messageId: 'hashPairOrder',
224+
data: {
225+
hashPairName: getHashPairName(node.hash.pairs[index]),
226+
expectedAfter: getHashPairName(node.hash.pairs[index + 1]),
227+
},
228+
});
229+
}
230+
}
231+
},
232+
233+
GlimmerMustacheStatement(node) {
234+
if (node.hash && node.hash.pairs && node.hash.pairs.length > 1) {
235+
const index = getUnsortedHashPairIndex(node.hash.pairs);
236+
237+
if (index !== -1) {
238+
// Component invocations with a string positional param (e.g. {{component "ui/button" ...}})
239+
// treat hash pairs as component attributes
240+
const isComponentInvocation =
241+
node.path &&
242+
node.path.original === 'component' &&
243+
node.params &&
244+
node.params.length > 0 &&
245+
node.params[0].type === 'GlimmerStringLiteral';
246+
247+
if (isComponentInvocation) {
248+
context.report({
249+
node: node.hash.pairs[index],
250+
messageId: 'attributeOrder',
251+
data: {
252+
attributeName: getHashPairName(node.hash.pairs[index]),
253+
expectedAfter: getHashPairName(node.hash.pairs[index + 1]),
254+
},
255+
});
256+
} else {
257+
context.report({
258+
node: node.hash.pairs[index],
259+
messageId: 'hashPairOrder',
260+
data: {
261+
hashPairName: getHashPairName(node.hash.pairs[index]),
262+
expectedAfter: getHashPairName(node.hash.pairs[index + 1]),
263+
},
264+
});
265+
}
266+
}
267+
}
268+
},
269+
270+
GlimmerSubExpression(node) {
271+
if (node.hash && node.hash.pairs && node.hash.pairs.length > 1) {
272+
const index = getUnsortedHashPairIndex(node.hash.pairs);
273+
274+
if (index !== -1) {
275+
context.report({
276+
node: node.hash.pairs[index],
277+
messageId: 'hashPairOrder',
278+
data: {
279+
hashPairName: getHashPairName(node.hash.pairs[index]),
280+
expectedAfter: getHashPairName(node.hash.pairs[index + 1]),
281+
},
282+
});
283+
}
284+
}
285+
},
286+
};
287+
},
288+
};
289+
/* eslint-enable unicorn/consistent-function-scoping, unicorn/prefer-at */

0 commit comments

Comments
 (0)