Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): [naming-convention] support the auto-accessor syntax #8084

Merged
merged 30 commits into from Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1662bd3
fix: support auto-accessor for naming convention
arka1002 Dec 17, 2023
25feaba
chore: fix CI failures
arka1002 Dec 17, 2023
2d47b74
Merge branch 'main' into issue-7823
arka1002 Dec 19, 2023
c1b1796
Merge branch 'main' into issue-7823
arka1002 Dec 23, 2023
1fe2d01
Merge branch 'main' into issue-7823
arka1002 Jan 4, 2024
2bb04b7
Merge branch 'main' into issue-7823
arka1002 Jan 4, 2024
4636e6d
Merge branch 'main' into issue-7823
arka1002 Jan 5, 2024
0149a62
chore: making auto-accessor a modifier of accessor
arka1002 Jan 5, 2024
2351b09
chore: fix prettier issues
arka1002 Jan 6, 2024
45a5118
chore: docs for auto-accessor syntax
arka1002 Jan 6, 2024
320320e
Merge branch 'main' into issue-7823
arka1002 Jan 7, 2024
c08c4cf
fix: make autoAccessor a selector
arka1002 Jan 8, 2024
3762356
fix: prettier and docs
arka1002 Jan 8, 2024
c4ae266
fix: small typo
arka1002 Jan 8, 2024
ec931d8
Merge branch 'main' into issue-7823
arka1002 Jan 8, 2024
9a42efe
Merge branch 'main' into issue-7823
arka1002 Jan 11, 2024
47fd96a
chore: add more test coverage and docs
arka1002 Jan 12, 2024
3d05845
Merge branch 'main' into issue-7823
arka1002 Jan 12, 2024
bd7947a
fix: support the abstract accessor
arka1002 Jan 13, 2024
9148131
chore: change accessor to classicAccessor selector
arka1002 Jan 13, 2024
d8114e8
chore: group classic and auto selectors
arka1002 Jan 13, 2024
468549d
chore: fix a small type error
arka1002 Jan 13, 2024
9fbc50c
Merge branch 'main' into issue-7823
arka1002 Jan 13, 2024
e6a1d27
chore: add docs
arka1002 Jan 13, 2024
813b1ec
Merge branch 'main' into issue-7823
arka1002 Jan 16, 2024
11d81c1
chore: slight mistake
arka1002 Feb 5, 2024
16eb834
Merge branch 'issue-7823' of github.com:arka1002/typescript-eslint in…
arka1002 Feb 5, 2024
88acaae
chore: add accessor test file
arka1002 Feb 6, 2024
eb935d0
chore: move docs
arka1002 Feb 21, 2024
8f1c37d
fix selector
bradzacher Feb 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions packages/eslint-plugin/docs/rules/naming-convention.md
Expand Up @@ -200,7 +200,10 @@ There are two types of selectors, individual selectors, and grouped selectors.

Individual Selectors match specific, well-defined sets. There is no overlap between each of the individual selectors.

- `accessor` - matches any accessor.
- `classicAccessor` - matches any accessor. It refers to the methods attached to `get` and `set` syntax.
- Allowed `modifiers`: `abstract`, `override`, `private`, `protected`, `public`, `requiresQuotes`, `static`.
- Allowed `types`: `array`, `boolean`, `function`, `number`, `string`.
- `autoAccessor` - matches any auto-accessor. An auto-accessor is just a class field starting with an `accessor` keyword.
- Allowed `modifiers`: `abstract`, `override`, `private`, `protected`, `public`, `requiresQuotes`, `static`.
- Allowed `types`: `array`, `boolean`, `function`, `number`, `string`.
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
- `class` - matches any class declaration.
Expand Down Expand Up @@ -262,7 +265,7 @@ Group Selectors are provided for convenience, and essentially bundle up sets of
- `default` - matches everything.
- Allowed `modifiers`: all modifiers.
- Allowed `types`: none.
- `memberLike` - matches the same as `accessor`, `enumMember`, `method`, `parameterProperty`, `property`.
- `memberLike` - matches the same as `accessor`, `autoAccessor`, `enumMember`, `method`, `parameterProperty`, `property`.
- Allowed `modifiers`: `abstract`, `async`, `override`, `#private`, `private`, `protected`, `public`, `readonly`, `requiresQuotes`, `static`.
- Allowed `types`: none.
- `method` - matches the same as `classMethod`, `objectLiteralMethod`, `typeMethod`.
Expand All @@ -277,6 +280,9 @@ Group Selectors are provided for convenience, and essentially bundle up sets of
- `variableLike` - matches the same as `function`, `parameter` and `variable`.
- Allowed `modifiers`: `async`, `unused`.
- Allowed `types`: none.
- `accessor` - matches the same as `classicAccessor` and `autoAccessor`.
- Allowed `modifiers`: `abstract`, `override`, `private`, `protected`, `public`, `requiresQuotes`, `static`.
- Allowed `types`: `array`, `boolean`, `function`, `number`, `string`.
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

## FAQ

Expand Down
Expand Up @@ -28,20 +28,21 @@ enum Selectors {

// memberLike
parameterProperty = 1 << 3,
accessor = 1 << 4,
classicAccessor = 1 << 4,
enumMember = 1 << 5,
classMethod = 1 << 6,
objectLiteralMethod = 1 << 7,
typeMethod = 1 << 8,
classProperty = 1 << 9,
objectLiteralProperty = 1 << 10,
typeProperty = 1 << 11,
autoAccessor = 1 << 12,

// typeLike
class = 1 << 12,
interface = 1 << 13,
typeAlias = 1 << 14,
enum = 1 << 15,
class = 1 << 13,
interface = 1 << 14,
typeAlias = 1 << 15,
enum = 1 << 16,
typeParameter = 1 << 17,

// other
Expand All @@ -65,7 +66,8 @@ enum MetaSelectors {
Selectors.classMethod |
Selectors.objectLiteralMethod |
Selectors.typeMethod |
Selectors.accessor,
Selectors.classicAccessor |
Selectors.autoAccessor,
typeLike = 0 |
Selectors.class |
Selectors.interface |
Expand All @@ -80,6 +82,7 @@ enum MetaSelectors {
Selectors.classProperty |
Selectors.objectLiteralProperty |
Selectors.typeProperty,
accessor = 0 | Selectors.classicAccessor | Selectors.autoAccessor,
/* eslint-enable @typescript-eslint/prefer-literal-enum-member */
}
type MetaSelectorsString = keyof typeof MetaSelectors;
Expand Down
18 changes: 18 additions & 0 deletions packages/eslint-plugin/src/rules/naming-convention-utils/schema.ts
Expand Up @@ -290,6 +290,24 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'override',
'async',
]),
...selectorSchema('classicAccessor', true, [
'abstract',
'private',
'protected',
'public',
'requiresQuotes',
'static',
'override',
]),
...selectorSchema('autoAccessor', true, [
'abstract',
'private',
'protected',
'public',
'requiresQuotes',
'static',
'override',
]),
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
...selectorSchema('accessor', true, [
'abstract',
'private',
Expand Down
Expand Up @@ -419,7 +419,7 @@ const SelectorsAllowedToHaveTypes =
Selectors.objectLiteralProperty |
Selectors.typeProperty |
Selectors.parameterProperty |
Selectors.accessor;
Selectors.classicAccessor;

function isCorrectType(
node: TSESTree.Node,
Expand Down
53 changes: 39 additions & 14 deletions packages/eslint-plugin/src/rules/naming-convention.ts
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
Expand Up @@ -110,7 +110,8 @@ export default createRule<Options, MessageIds>({
| TSESTree.TSAbstractMethodDefinitionNonComputedName
| TSESTree.TSAbstractPropertyDefinitionNonComputedName
| TSESTree.TSMethodSignatureNonComputedName
| TSESTree.TSPropertySignatureNonComputedName,
| TSESTree.TSPropertySignatureNonComputedName
| TSESTree.AccessorPropertyNonComputedName,
modifiers: Set<Modifiers>,
): void {
const key = node.key;
Expand All @@ -127,7 +128,9 @@ export default createRule<Options, MessageIds>({
| TSESTree.PropertyDefinition
| TSESTree.TSAbstractMethodDefinition
| TSESTree.TSAbstractPropertyDefinition
| TSESTree.TSParameterProperty,
| TSESTree.TSParameterProperty
| TSESTree.AccessorProperty
| TSESTree.TSAbstractAccessorProperty,
): Set<Modifiers> {
const modifiers = new Set<Modifiers>();
if ('key' in node && node.key.type === AST_NODE_TYPES.PrivateIdentifier) {
Expand All @@ -148,7 +151,8 @@ export default createRule<Options, MessageIds>({
}
if (
node.type === AST_NODE_TYPES.TSAbstractPropertyDefinition ||
node.type === AST_NODE_TYPES.TSAbstractMethodDefinition
node.type === AST_NODE_TYPES.TSAbstractMethodDefinition ||
node.type === AST_NODE_TYPES.TSAbstractAccessorProperty
) {
modifiers.add(Modifiers.abstract);
}
Expand Down Expand Up @@ -519,27 +523,48 @@ export default createRule<Options, MessageIds>({
// #region accessor

'Property[computed = false]:matches([kind = "get"], [kind = "set"])': {
validator: validators.accessor,
validator: validators.classicAccessor,
handler: (node: TSESTree.PropertyNonComputedName, validator): void => {
const modifiers = new Set<Modifiers>([Modifiers.public]);
handleMember(validator, node, modifiers);
},
},

'MethodDefinition[computed = false]:matches([kind = "get"], [kind = "set"])':
{
validator: validators.accessor,
handler: (
node: TSESTree.MethodDefinitionNonComputedName,
validator,
): void => {
const modifiers = getMemberModifiers(node);
handleMember(validator, node, modifiers);
},
[[
'MethodDefinition[computed = false]:matches([kind = "get"], [kind = "set"])',
':matches(TSAbstractMethodDefinition)[computed = false][kind="get"]',
':matches(TSAbstractMethodDefinition)[computed = false][kind="set"]',
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
].join(', ')]: {
validator: validators.classicAccessor,
handler: (
node: TSESTree.MethodDefinitionNonComputedName,
validator,
): void => {
const modifiers = getMemberModifiers(node);
handleMember(validator, node, modifiers);
},
},

// #endregion accessor

// #region autoAccessor

[[
AST_NODE_TYPES.AccessorProperty,
AST_NODE_TYPES.TSAbstractAccessorProperty,
].join(', ')]: {
validator: validators.autoAccessor,
handler: (
node: TSESTree.AccessorPropertyNonComputedName,
validator,
): void => {
const modifiers = getMemberModifiers(node);
handleMember(validator, node, modifiers);
},
},

// #endregion autoAccessor

// #region enumMember

// computed is optional, so can't do [computed = false]
Expand Down
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
@@ -0,0 +1,22 @@
import { createTestCases } from './createTestCases';

createTestCases([
{
code: [
'class Ignored { accessor % = 10; }',
'class Ignored { accessor #% = 10; }',
'class Ignored { static accessor % = 10; }',
'class Ignored { static accessor #% = 10; }',
'class Ignored { private accessor % = 10; }',
'class Ignored { private static accessor % = 10; }',
'class Ignored { override accessor % = 10; }',
'class Ignored { accessor "%" = 10; }',
'class Ignored { protected accessor % = 10; }',
'class Ignored { public accessor % = 10; }',
'class Ignored { abstract accessor %; }',
],
options: {
selector: 'autoAccessor',
},
},
]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Testing] It's good to have created a test file for classicAccessor - but there should also be a test case file for accessor too! (so, three *.test.ts files around accessors, not two)

Copy link
Contributor Author

@arka1002 arka1002 Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes! Added here 88acaae. There are a bunch of tests in the other file too

class Ignored {
private static get some_name() {}
get IgnoredDueToModifiers() {}
}
`,
options: [
{
selector: 'default',
format: ['PascalCase'],
},
{
selector: 'accessor',
format: ['snake_case'],
modifiers: ['private', 'static'],
},
],

Expand Up @@ -9,9 +9,11 @@ createTestCases([
'class Ignored { private set "%"(ignored) {} }',
'class Ignored { private static get %() {} }',
'class Ignored { static get #%() {} }',
'abstract class Ignored { abstract get %(): number }',
'abstract class Ignored { abstract set %(ignored: number) }',
],
options: {
selector: 'accessor',
selector: 'classicAccessor',
},
},
]);
Expand Up @@ -291,7 +291,8 @@ export function createTestCases(cases: Cases): void {
selector !== 'memberLike' &&
selector !== 'typeLike' &&
selector !== 'property' &&
selector !== 'method'
selector !== 'method' &&
selector !== 'accessor'
? {
data: {
type: selectorTypeToMessageString(selector),
Expand Down
Expand Up @@ -241,7 +241,7 @@ ruleTester.run('naming-convention', rule, {
parserOptions,
options: [
{
selector: ['variable', 'parameter', 'property', 'accessor'],
selector: ['variable', 'parameter', 'property', 'classicAccessor'],
types: ['number'],
format: ['PascalCase'],
prefix: ['is', 'should', 'has', 'can', 'did', 'will'],
Expand All @@ -257,7 +257,7 @@ ruleTester.run('naming-convention', rule, {
parserOptions,
options: [
{
selector: ['property', 'accessor'],
selector: ['property', 'classicAccessor'],
types: ['boolean'],
modifiers: ['private', 'readonly'],
format: ['PascalCase'],
Expand All @@ -272,7 +272,7 @@ ruleTester.run('naming-convention', rule, {
`,
options: [
{
selector: ['property', 'accessor'],
selector: ['property', 'classicAccessor'],
modifiers: ['private'],
format: ['camelCase'],
},
Expand All @@ -289,7 +289,7 @@ ruleTester.run('naming-convention', rule, {
parserOptions,
options: [
{
selector: ['property', 'accessor'],
selector: ['property', 'classicAccessor'],
modifiers: ['private'],
format: ['StrictPascalCase'],
prefix: ['Van'],
Expand Down Expand Up @@ -618,7 +618,7 @@ ruleTester.run('naming-convention', rule, {
format: ['PascalCase'],
},
{
selector: 'accessor',
selector: 'classicAccessor',
format: ['snake_case'],
modifiers: ['private', 'static'],
},
Expand Down Expand Up @@ -766,7 +766,7 @@ ruleTester.run('naming-convention', rule, {
'classMethod',
'objectLiteralMethod',
'typeMethod',
'accessor',
'classicAccessor',
'enumMember',
],
format: null,
Expand All @@ -781,7 +781,7 @@ ruleTester.run('naming-convention', rule, {
'classMethod',
'objectLiteralMethod',
'typeMethod',
'accessor',
'classicAccessor',
'enumMember',
],
format: ['PascalCase'],
Expand Down Expand Up @@ -1347,7 +1347,7 @@ ruleTester.run('naming-convention', rule, {
`,
options: [
{
selector: ['property', 'accessor'],
selector: ['property', 'classicAccessor'],
modifiers: ['private', 'readonly'],
format: ['PascalCase'],
},
Expand Down Expand Up @@ -1643,7 +1643,7 @@ ruleTester.run('naming-convention', rule, {
format: ['PascalCase'],
},
{
selector: 'accessor',
selector: 'classicAccessor',
format: ['UPPER_CASE'],
modifiers: ['private', 'static'],
},
Expand Down Expand Up @@ -2088,15 +2088,15 @@ ruleTester.run('naming-convention', rule, {
{
messageId: 'doesNotMatchFormat',
data: {
type: 'Accessor',
type: 'Classic Accessor',
name: 'someGetterOverride',
formats: 'snake_case',
},
},
{
messageId: 'doesNotMatchFormat',
data: {
type: 'Accessor',
type: 'Classic Accessor',
name: 'someSetterOverride',
formats: 'snake_case',
},
Expand Down