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

[naming-convention] allow treating quoted properties differently #2761

Closed
ab-pm opened this issue Nov 12, 2020 · 1 comment · Fixed by #2813
Closed

[naming-convention] allow treating quoted properties differently #2761

ab-pm opened this issue Nov 12, 2020 · 1 comment · Fixed by #2813
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@ab-pm
Copy link

ab-pm commented Nov 12, 2020

This is essentially a duplicate of #1483, which unfortunately was closed without a complete solution.

We have a large code base that used the camelCase rule. We have the convention that whenever objects are used as key-value dictionaries not as records, properties would be quoted (string literal instead of identifier syntax). The camelCase rule would not complain about snake_case or PascalCase property names then. This is used in a lot of places (custom headers, database access layer, application-specific logic, external protocols, and more…) so that whitelisting allowed names is not an option.

Since we don't want to get rid of identifier linting completely, and cannot achieve the desired behavior with the naming-convention rule, we're currently blocked from upgrading typescript-eslint - very dissatisfying.

I'm not suggesting to just add a ignoreStringKeys option or even to enable it by default.
What is needed is a new selector to distinguish properties that are IdentifierNames from properties that are StringLiterals. For completeness, a third category might match properties that are NumericLiterals. I imagine this as an extension to propertyDefinition from #2244 (comment), which would become a group selector itself.
However, this could actually be a separate dimension for the matcher, like modifiers and types. While for my concrete use case the distinction is only necessary in object literals (not interface/class member declarations) and not (rarely, actually) for methods, it would provide more flexibility. What do you think?

@ab-pm ab-pm added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Nov 12, 2020
@bradzacher
Copy link
Member

Originally the problem with solving this was that it's actually a really, really, really difficult thing to catch 100% of the time because the rules for what is a valid "identifier" in JS is very, very complicated.

However since then, we learned that (thankfully) TypeScript exposes its functions which let it do this check, and we have got a usage of it this in the plugin:

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;
}


What solution would I accept for this rule?

I would be happy to accept a modifier for the property selector.
For example requires-quotes which denotes when the property name must be quoted.

Why a modifier which detects required quotes vs a selector that matches any quoted name?

I don't agree that all quoted names should ever just be ignored.
That is not the way to enforce a naming convention because it means that people can very easily create names that break your convention solely because they used quotes.

A modifier that only is applied when quotes are required allows you to choose to ignore these property names specifically, because they're clearly written this way on purpose.

@bradzacher bradzacher added enhancement: plugin rule option New rule option for an existing eslint-plugin rule and removed triage Waiting for maintainers to take a look labels Nov 13, 2020
bradzacher added a commit that referenced this issue Nov 25, 2020
Fixes #2761
Fixes #1483

This modifier simply matches any member name that requires quotes.
To clarify: this does not match names that have quotes - only names that ***require*** quotes.
bradzacher added a commit that referenced this issue Nov 25, 2020
Fixes #2761
Fixes #1483

This modifier simply matches any member name that requires quotes.
To clarify: this does not match names that have quotes - only names that ***require*** quotes.
bradzacher added a commit that referenced this issue Nov 25, 2020
#2813)

Fixes #2761
Fixes #1483

This modifier simply matches any member name that requires quotes.
To clarify: this does not match names that have quotes - only names that ***require*** quotes.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: plugin rule option New rule option for an existing eslint-plugin rule package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
2 participants