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.

Sign up for GitHub

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

Jump to bottom

feat(eslint-plugin): [naming-convention] add support for "override" and "async" modifiers (#5310) #5610

Merged
merged 4 commits into from Nov 7, 2022
Merged

feat(eslint-plugin): [naming-convention] add support for "override" and "async" modifiers (#5310) #5610

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
58dbdaf
feat(eslint-plugin): [naming-convention] add support for "override" a…
eliasm307 Sep 7, 2022
b3446d6
apply pr feedback ie remove test case util, split tests by type, remo…
eliasm307 Nov 5, 2022
f59a7f3
remove async parameter logic which is impossible
eliasm307 Nov 7, 2022
3a2086d
Merge branch 'main' into naming-convention-Add-override-modifier-support
JoshuaKGoldberg Nov 7, 2022
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
23 changes: 12 additions & 11 deletions packages/eslint-plugin/docs/rules/naming-convention.md
View file
Open in desktop
Expand Up @@ -173,7 +173,8 @@ If these are provided, the identifier must start with one of the provided values
- `unused` - matches anything that is not used.
- `requiresQuotes` - matches any name that requires quotes as it is not a valid identifier (i.e. has a space, a dash, etc in it).
- `public` - matches any member that is either explicitly declared as `public`, or has no visibility modifier (i.e. implicitly public).
- `readonly`, `static`, `abstract`, `protected`, `private` - matches any member explicitly declared with the given modifier.
- `readonly`, `static`, `abstract`, `protected`, `private`, `override` - matches any member explicitly declared with the given modifier.
- `async` - matches any method, function, or function variable which is async via the `async` keyword (e.g. does not match functions that return promises without using `async` keyword)
- `types` allows you to specify which types to match. This option supports simple, primitive types only (`boolean`, `string`, `number`, `array`, `function`).
- The name must match _one_ of the types.
- **_NOTE - Using this option will require that you lint with type information._**
Expand All @@ -196,16 +197,16 @@ 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.

- `variable` - matches any `var` / `let` / `const` variable name.
- Allowed `modifiers`: `const`, `destructured`, `global`, `exported`, `unused`.
- Allowed `modifiers`: `const`, `destructured`, `global`, `exported`, `unused`, `async`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `function` - matches any named function declaration or named function expression.
- Allowed `modifiers`: `global`, `exported`, `unused`.
- Allowed `modifiers`: `global`, `exported`, `unused`, `async`.
- Allowed `types`: none.
- `parameter` - matches any function parameter. Does not match parameter properties.
- Allowed `modifiers`: `destructured`, `unused`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `classProperty` - matches any class property. Does not match properties that have direct function expression or arrow function expression values.
- Allowed `modifiers`: `abstract`, `private`, `protected`, `public`, `readonly`, `requiresQuotes`, `static`.
- Allowed `modifiers`: `abstract`, `private`, `protected`, `public`, `readonly`, `requiresQuotes`, `static`, `override`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `objectLiteralProperty` - matches any object literal property. Does not match properties that have direct function expression or arrow function expression values.
- Allowed `modifiers`: `public`, `requiresQuotes`.
Expand All @@ -217,16 +218,16 @@ Individual Selectors match specific, well-defined sets. There is no overlap betw
- Allowed `modifiers`: `private`, `protected`, `public`, `readonly`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `classMethod` - matches any class method. Also matches properties that have direct function expression or arrow function expression values. Does not match accessors.
- Allowed `modifiers`: `abstract`, `private`, `protected`, `public`, `requiresQuotes`, `static`.
- Allowed `modifiers`: `abstract`, `private`, `protected`, `public`, `requiresQuotes`, `static`, `override`, `async`.
- Allowed `types`: none.
- `objectLiteralMethod` - matches any object literal method. Also matches properties that have direct function expression or arrow function expression values. Does not match accessors.
- Allowed `modifiers`: `public`, `requiresQuotes`.
- Allowed `modifiers`: `public`, `requiresQuotes`, `async`.
- Allowed `types`: none.
- `typeMethod` - matches any object type method. Also matches properties that have direct function expression or arrow function expression values. Does not match accessors.
- Allowed `modifiers`: `public`, `requiresQuotes`.
- Allowed `types`: none.
- `accessor` - matches any accessor.
- Allowed `modifiers`: `abstract`, `private`, `protected`, `public`, `requiresQuotes`, `static`.
- Allowed `modifiers`: `abstract`, `private`, `protected`, `public`, `requiresQuotes`, `static`, `override`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `enumMember` - matches any enum member.
- Allowed `modifiers`: `requiresQuotes`.
Expand Down Expand Up @@ -255,19 +256,19 @@ Group Selectors are provided for convenience, and essentially bundle up sets of
- Allowed `modifiers`: all modifiers.
- Allowed `types`: none.
- `variableLike` - matches the same as `variable`, `function` and `parameter`.
- Allowed `modifiers`: `unused`.
- Allowed `modifiers`: `unused`, `async`.
- Allowed `types`: none.
- `memberLike` - matches the same as `property`, `parameterProperty`, `method`, `accessor`, `enumMember`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`, `override`, `async`.
- Allowed `types`: none.
- `typeLike` - matches the same as `class`, `interface`, `typeAlias`, `enum`, `typeParameter`.
- Allowed `modifiers`: `abstract`, `unused`.
- Allowed `types`: none.
- `property` - matches the same as `classProperty`, `objectLiteralProperty`, `typeProperty`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`, `override`, `async`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `method` - matches the same as `classMethod`, `objectLiteralMethod`, `typeMethod`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`, `override`, `async`.
- Allowed `types`: none.

## FAQ
Expand Down
4 changes: 4 additions & 0 deletions packages/eslint-plugin/src/rules/naming-convention-utils/enums.ts
View file
Open in desktop
Expand Up @@ -102,6 +102,10 @@ enum Modifiers {
unused = 1 << 10,
// properties that require quoting
requiresQuotes = 1 << 11,
// class members that are overridden
override = 1 << 12,
// class methods, object function properties, or functions that are async via the `async` keyword
async = 1 << 13,

// make sure TypeModifiers starts at Modifiers + 1 or else sorting won't work
}
Expand Down
21 changes: 19 additions & 2 deletions packages/eslint-plugin/src/rules/naming-convention-utils/schema.ts
View file
Open in desktop
Expand Up @@ -167,15 +167,21 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
selectorsSchema(),
...selectorSchema('default', false, util.getEnumNames(Modifiers)),

...selectorSchema('variableLike', false, ['unused']),
...selectorSchema('variableLike', false, ['unused', 'async']),
...selectorSchema('variable', true, [
'const',
'destructured',
'exported',
'global',
'unused',
'async',
]),
...selectorSchema('function', false, [
'exported',
'global',
'unused',
'async',
]),
...selectorSchema('function', false, ['exported', 'global', 'unused']),
...selectorSchema('parameter', true, ['destructured', 'unused']),

...selectorSchema('memberLike', false, [
Expand All @@ -186,6 +192,8 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'readonly',
'requiresQuotes',
'static',
'override',
'async',
]),
...selectorSchema('classProperty', true, [
'abstract',
Expand All @@ -195,6 +203,7 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'readonly',
'requiresQuotes',
'static',
'override',
]),
...selectorSchema('objectLiteralProperty', true, [
'public',
Expand All @@ -219,6 +228,8 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'readonly',
'requiresQuotes',
'static',
'override',
'async',
]),

...selectorSchema('classMethod', false, [
Expand All @@ -228,10 +239,13 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'public',
'requiresQuotes',
'static',
'override',
'async',
]),
...selectorSchema('objectLiteralMethod', false, [
'public',
'requiresQuotes',
'async',
]),
...selectorSchema('typeMethod', false, ['public', 'requiresQuotes']),
...selectorSchema('method', false, [
Expand All @@ -241,6 +255,8 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'public',
'requiresQuotes',
'static',
'override',
'async',
]),
...selectorSchema('accessor', true, [
'abstract',
Expand All @@ -249,6 +265,7 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'public',
'requiresQuotes',
'static',
'override',
]),
...selectorSchema('enumMember', false, ['requiresQuotes']),

Expand Down
49 changes: 49 additions & 0 deletions packages/eslint-plugin/src/rules/naming-convention.ts
View file
Open in desktop
Expand Up @@ -138,6 +138,9 @@ export default util.createRule<Options, MessageIds>({
if ('readonly' in node && node.readonly) {
modifiers.add(Modifiers.readonly);
}
if ('override' in node && node.override) {
modifiers.add(Modifiers.override);
}
if (
node.type === AST_NODE_TYPES.TSAbstractPropertyDefinition ||
node.type === AST_NODE_TYPES.TSAbstractMethodDefinition
Expand Down Expand Up @@ -182,6 +185,34 @@ export default util.createRule<Options, MessageIds>({
);
}

function isAsyncMemberOrProperty(
propertyOrMemberNode:
| TSESTree.PropertyNonComputedName
| TSESTree.TSMethodSignatureNonComputedName
| TSESTree.PropertyDefinitionNonComputedName
| TSESTree.TSAbstractPropertyDefinitionNonComputedName
| TSESTree.MethodDefinitionNonComputedName
| TSESTree.TSAbstractMethodDefinitionNonComputedName,
): boolean {
return Boolean(
'value' in propertyOrMemberNode &&
Copy link
Member

Choose a reason for hiding this comment

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

Looking at this again: we generally try not to use this string in checking pattern, as it's a little imprecise. We normally check for specific types. But I can see why you did it here (much less code!), and there's already an instance of it in the file.

I think it's fine to check in as-is, and we should separately look into making a lint rule internally that flags this kind of check.

I also think I'd meant to post this in the first review but forgot 😄

Copy link
Contributor Author

@eliasm307 eliasm307 Nov 7, 2022 •
edited by JoshuaKGoldberg

Choose a reason for hiding this comment

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

as it's a little imprecise

I agree, and its not great for readability also, its not really clear what the intent of the in is, in terms of what node types its trying to check for, but I went with it since it was used everywhere in that file

I can see why you did it here (much less code!)

Yeah I don't do it like this usually, but I was also surprised how short the code gets, but I'm still a fan of the type utils for better communication (but please don't ask me change it now😅)

Copy link
Member

Choose a reason for hiding this comment

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

Haha don't worry, I won't 😄

Copy link
Member

Choose a reason for hiding this comment

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

eslint-community/eslint-plugin-eslint-plugin#326

propertyOrMemberNode.value &&
'async' in propertyOrMemberNode.value &&
propertyOrMemberNode.value.async,
);
}

function isAsyncVariableIdentifier(id: TSESTree.Identifier): boolean {
return Boolean(
id.parent &&
(('async' in id.parent && id.parent.async) ||
('init' in id.parent &&
id.parent.init &&
'async' in id.parent.init &&
id.parent.init.async)),
);
}

return {
// #region variable

Expand Down Expand Up @@ -219,6 +250,10 @@ export default util.createRule<Options, MessageIds>({
modifiers.add(Modifiers.unused);
}

if (isAsyncVariableIdentifier(id)) {
modifiers.add(Modifiers.async);
}

validator(id, modifiers);
});
},
Expand Down Expand Up @@ -254,6 +289,10 @@ export default util.createRule<Options, MessageIds>({
modifiers.add(Modifiers.unused);
}

if (node.async) {
modifiers.add(Modifiers.async);
}

validator(node.id, modifiers);
},

Expand Down Expand Up @@ -360,6 +399,11 @@ export default util.createRule<Options, MessageIds>({
| TSESTree.TSMethodSignatureNonComputedName,
): void {
const modifiers = new Set<Modifiers>([Modifiers.public]);

if (isAsyncMemberOrProperty(node)) {
modifiers.add(Modifiers.async);
}

handleMember(validators.objectLiteralMethod, node, modifiers);
},

Expand All @@ -376,6 +420,11 @@ export default util.createRule<Options, MessageIds>({
| TSESTree.TSAbstractMethodDefinitionNonComputedName,
): void {
const modifiers = getMemberModifiers(node);

if (isAsyncMemberOrProperty(node)) {
modifiers.add(Modifiers.async);
}

handleMember(validators.classMethod, node, modifiers);
},

Expand Down