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] add requiresQuotes modifier #2813

Merged
merged 1 commit into from Nov 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 45 additions & 16 deletions packages/eslint-plugin/docs/rules/naming-convention.md
Expand Up @@ -183,6 +183,7 @@ If these are provided, the identifier must start with one of the provided values
- `global` - matches a variable/function declared in the top-level scope.
- `exported` - matches anything that is exported from the module.
- `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.
- `types` allows you to specify which types to match. This option supports simple, primitive types only (`boolean`, `string`, `number`, `array`, `function`).
Expand Down Expand Up @@ -229,31 +230,31 @@ Individual Selectors match specific, well-defined sets. There is no overlap betw
- Allowed `modifiers`: `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`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- 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`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `typeProperty` - matches any object type property. Does not match properties that have direct function expression or arrow function expression values.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `parameterProperty` - matches any parameter property.
- 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`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- 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`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- 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`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `types`: none.
- `accessor` - matches any accessor.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `enumMember` - matches any enum member.
- Allowed `modifiers`: none.
- Allowed `modifiers`: `requiresQuotes`.
- Allowed `types`: none.
- `class` - matches any class declaration.
- Allowed `modifiers`: `abstract`, `exported`, `unused`.
Expand All @@ -276,22 +277,22 @@ Individual Selectors match specific, well-defined sets. There is no overlap betw
Group Selectors are provided for convenience, and essentially bundle up sets of individual selectors.

- `default` - matches everything.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: all modifiers.
- Allowed `types`: none.
- `variableLike` - matches the same as `variable`, `function` and `parameter`.
- Allowed `modifiers`: `unused`.
- Allowed `types`: none.
- `memberLike` - matches the same as `property`, `parameterProperty`, `method`, `accessor`, `enumMember`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- 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`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `types`: `boolean`, `string`, `number`, `function`, `array`.
- `method` - matches the same as `classMethod`, `objectLiteralMethod`, `typeMethod`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`.
- Allowed `modifiers`: `private`, `protected`, `public`, `static`, `readonly`, `abstract`, `requiresQuotes`.
- Allowed `types`: none.

## Examples
Expand Down Expand Up @@ -424,12 +425,36 @@ This allows you to lint multiple type with same pattern.
}
```

### Ignore properties that require quotes
### Ignore properties that **_require_** quotes

Sometimes you have to use a quoted name that breaks the convention (for example, HTTP headers).
If this is a common thing in your codebase, then you can use the `filter` option in one of two ways:
If this is a common thing in your codebase, then you have a few options.

You can use the `filter` option to ignore specific names only:
If you simply want to allow all property names that require quotes, you can use the `requiresQuotes` modifier to match any property name that _requires_ quoting, and use `format: null` to ignore the name.

```jsonc
{
"@typescript-eslint/naming-convention": [
"error",
{
"selector": [
"classProperty",
"objectLiteralProperty",
"typeProperty",
"classMethod",
"objectLiteralMethod",
"typeMethod",
"accessor",
"enumMember"
],
"format": null,
"modifiers": ["requiresQuotes"]
}
]
}
```

If you have a small and known list of exceptions, you can use the `filter` option to ignore these specific names only:

```jsonc
{
Expand All @@ -448,7 +473,7 @@ You can use the `filter` option to ignore specific names only:
}
```

You can use the `filter` option to ignore names that require quoting:
You can use the `filter` option to ignore names with specific characters:

```jsonc
{
Expand All @@ -467,6 +492,10 @@ You can use the `filter` option to ignore names that require quoting:
}
```

Note that there is no way to ignore any name that is quoted - only names that are required to be quoted.
This is intentional - adding quotes around a name is not an escape hatch for proper naming.
If you want an escape hatch for a specific name - you should can use an [`eslint-disable` comment](https://eslint.org/docs/user-guide/configuring#disabling-rules-with-inline-comments).

### Ignore destructured names

Sometimes you might want to allow destructured properties to retain their original name, even if it breaks your naming convention.
Expand Down
40 changes: 37 additions & 3 deletions packages/eslint-plugin/src/rules/naming-convention.ts
Expand Up @@ -120,6 +120,8 @@ enum Modifiers {
exported = 1 << 9,
// things that are unused
unused = 1 << 10,
// properties that require quoting
requiresQuotes = 1 << 11,
}
type ModifiersString = keyof typeof Modifiers;

Expand Down Expand Up @@ -359,6 +361,7 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'static',
'readonly',
'abstract',
'requiresQuotes',
]),
...selectorSchema('classProperty', true, [
'private',
Expand All @@ -367,6 +370,7 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'static',
'readonly',
'abstract',
'requiresQuotes',
]),
...selectorSchema('objectLiteralProperty', true, [
'private',
Expand All @@ -375,6 +379,7 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'static',
'readonly',
'abstract',
'requiresQuotes',
]),
...selectorSchema('typeProperty', true, [
'private',
Expand All @@ -383,6 +388,7 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'static',
'readonly',
'abstract',
'requiresQuotes',
]),
...selectorSchema('parameterProperty', true, [
'private',
Expand All @@ -397,6 +403,7 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'static',
'readonly',
'abstract',
'requiresQuotes',
]),

...selectorSchema('classMethod', false, [
Expand All @@ -405,36 +412,41 @@ const SCHEMA: JSONSchema.JSONSchema4 = {
'public',
'static',
'abstract',
'requiresQuotes',
]),
...selectorSchema('objectLiteralMethod', false, [
'private',
'protected',
'public',
'static',
'abstract',
'requiresQuotes',
]),
...selectorSchema('typeMethod', false, [
'private',
'protected',
'public',
'static',
'abstract',
'requiresQuotes',
]),
...selectorSchema('method', false, [
'private',
'protected',
'public',
'static',
'abstract',
'requiresQuotes',
]),
...selectorSchema('accessor', true, [
'private',
'protected',
'public',
'static',
'abstract',
'requiresQuotes',
]),
...selectorSchema('enumMember', false),
...selectorSchema('enumMember', false, ['requiresQuotes']),

...selectorSchema('typeLike', false, ['abstract', 'exported', 'unused']),
...selectorSchema('class', false, ['abstract', 'exported', 'unused']),
Expand Down Expand Up @@ -516,6 +528,9 @@ export default util.createRule<Options, MessageIds>({

const validators = parseOptions(context);

const compilerOptions = util
.getParserServices(context, true)
.program.getCompilerOptions();
function handleMember(
validator: ValidatorFunction | null,
node:
Expand All @@ -533,6 +548,10 @@ export default util.createRule<Options, MessageIds>({
}

const key = node.key;
if (requiresQuoting(key, compilerOptions.target)) {
modifiers.add(Modifiers.requiresQuotes);
}

validator(key, modifiers);
}

Expand Down Expand Up @@ -829,7 +848,13 @@ export default util.createRule<Options, MessageIds>({
}

const id = node.id;
validator(id);
const modifiers = new Set<Modifiers>();

if (requiresQuoting(id, compilerOptions.target)) {
modifiers.add(Modifiers.requiresQuotes);
}

validator(id, modifiers);
},

// #endregion enumMember
Expand Down Expand Up @@ -1020,8 +1045,17 @@ function isGlobal(scope: TSESLint.Scope.Scope | null): boolean {
);
}

type ValidatorFunction = (
function requiresQuoting(
node: TSESTree.Identifier | TSESTree.Literal,
target: ts.ScriptTarget | undefined,
): boolean {
const name =
node.type === AST_NODE_TYPES.Identifier ? node.name : `${node.value}`;
return util.requiresQuoting(name, target);
}

type ValidatorFunction = (
node: TSESTree.Identifier | TSESTree.StringLiteral | TSESTree.NumberLiteral,
modifiers?: Set<Modifiers>,
) => void;
type ParsedOptions = Record<SelectorsString, null | ValidatorFunction>;
Expand Down
21 changes: 2 additions & 19 deletions packages/eslint-plugin/src/rules/switch-exhaustiveness-check.ts
Expand Up @@ -6,6 +6,7 @@ import {
getParserServices,
isClosingBraceToken,
isOpeningBraceToken,
requiresQuoting,
} from '../util';
import { isTypeFlagSet, unionTypeParts } from 'tsutils';

Expand Down Expand Up @@ -34,24 +35,6 @@ export default createRule({
const checker = service.program.getTypeChecker();
const compilerOptions = service.program.getCompilerOptions();

function requiresQuoting(name: string): boolean {
if (name.length === 0) {
return true;
}

if (!ts.isIdentifierStart(name.charCodeAt(0), compilerOptions.target)) {
return true;
}

for (let i = 1; i < name.length; i += 1) {
if (!ts.isIdentifierPart(name.charCodeAt(i), compilerOptions.target)) {
return true;
}
}

return false;
}

function getNodeType(node: TSESTree.Node): ts.Type {
const tsNode = service.esTreeNodeToTSNodeMap.get(node);
return getConstrainedTypeAtLocation(checker, tsNode);
Expand Down Expand Up @@ -93,7 +76,7 @@ export default createRule({
if (
symbolName &&
(missingBranchName || missingBranchName === '') &&
requiresQuoting(missingBranchName.toString())
requiresQuoting(missingBranchName.toString(), compilerOptions.target)
) {
caseTest = `${symbolName}['${missingBranchName}']`;
}
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/util/index.ts
Expand Up @@ -8,6 +8,7 @@ export * from './misc';
export * from './nullThrows';
export * from './objectIterators';
export * from './propertyTypes';
export * from './requiresQuoting';
export * from './types';

// this is done for convenience - saves migrating all of the old rules
Expand Down
24 changes: 24 additions & 0 deletions packages/eslint-plugin/src/util/requiresQuoting.ts
@@ -0,0 +1,24 @@
import * as ts from 'typescript';

function requiresQuoting(
name: string,
target: ts.ScriptTarget = ts.ScriptTarget.ESNext,
): boolean {
if (name.length === 0) {
return true;
}

if (!ts.isIdentifierStart(name.charCodeAt(0), target)) {
return true;
}

for (let i = 1; i < name.length; i += 1) {
if (!ts.isIdentifierPart(name.charCodeAt(i), target)) {
return true;
}
}

return false;
}

export { requiresQuoting };