Skip to content

Commit

Permalink
Merge pull request #1 from adashrod/ignore-single-word-refactor
Browse files Browse the repository at this point in the history
    - introduce new option in `strict-camel-case`, `ignoreSingleWordsIn`, which allows all-caps single words in specific contexts that use constants
    - deprecate option `ignoreSingleWords` in favor of new option
  • Loading branch information
adashrod authored Oct 26, 2023
2 parents 5d2c543 + 3d02937 commit 1c4bae8
Show file tree
Hide file tree
Showing 4 changed files with 150 additions and 16 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "eslint-plugin-sequence",
"version": "0.4.0",
"version": "0.5.0",
"description": "A collection of EsLint rules variously related to sequences: import sorting, ordering, etc",
"main": "index.js",
"devDependencies": {
Expand Down
55 changes: 51 additions & 4 deletions src/docs/strict-camel-case.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ Further reading:
"ignoreImports": false,
"ignoredIdentifiers": ["htmlToXML", "legacyAPI"],
"allowOneCharWords": "last",
"ignoreSingleWords": false
"ignoreSingleWords": false,
"ignoreSingleWordsIn": ["enum_member", "static_class_field"]
}
],
...
Expand Down Expand Up @@ -149,7 +150,7 @@ values: `"never" | "always" | "last"`
default `"never"`
Since tokens in camel case have one letter capitalized and the rest lowercase, a 1-letter word can create problems for camel case because, even following strict camel case, there could be contiguous uppercase letters, e.g. `ThisIsAClass`. This option allows some control over how to handle that.
Since tokens in camel case have one letter capitalized and the rest lowercase, a 1-letter word can create problems for camel case because, even following strict camel case, there could be contiguous uppercase letters, e.g. `ThisIsAClass`. This option allows some control over how to handle that. Note: when identifiers trigger an error due to this configuration, the lint rule will not provide a suggestion, because none exists.
`"never"`: 1-letter, uppercase words are considered invalid
```javascript
Expand Down Expand Up @@ -179,19 +180,65 @@ function getX() {}
class A {}
```
## ignoreSingleWordsIn
---
type: `enum[]`
values: `"enum_member" | "first_class_constant" | "object_field" | "static_class_field"`
default: `[]`
Identifiers that are all-caps and contain only one word are inherently ambiguous, e.g. `HTML, JSON, PI, TAU, EPSILON`. Any of these could be names of classes that are in loose camel case, or names of constants that are in all-caps snake case. There's no way to make that determination without knowing the semantic meaning of them.
Adding additional words demontstrates how the single-word versions are ambiguous: `HTMLTags, JSONSerializer, TAU_IS_2_PI, EPSILON_UNCERTAINTY`.
As opposed to `ignoreSingleWords`, which is very broad, this option lets you allow all-caps single words in certain contexts that are likely to have constant members. For example, you can allowlist 1st-class constants (`export const MAX = 10`) or static class fields (`class Util { public static MAX = 10; }`), but not identifiers that are not usually used as constants (`class API {}` or `type HTML = {...}`).
#### Example allowed single-word identifiers with each option:
`enum_member`:
```
enum Direction {
NORTH, EAST, SOUTH, WEST
}
```
`first_class_constant`:
```
export const VERSION = "1.0"
```
## ignoreSingleWords
`object_field`:
```
const myEs6EnumDirection = {
NORTH: "north",
EAST: "east",
SOUTH: "south",
WEST: "west"
}
```
`static_class_field`:
```
class MyUtil {
public static VERSION = "1.0";
}
```
## ignoreSingleWords (deprecated)
---
type: `boolean`
default: `false`
#### This option will be removed in a future release. Please use `ignoredIdentifiers` and/or `ignoreSingleWordsIn` instead
Identifiers that are all-caps and contain only one word are inherently ambiguous, e.g. `HTML, JSON, PI, TAU, EPSILON`. Any of these could be names of classes that are in loose camel case, or names of constants that are in all-caps snake case. There's no way to make that determination without knowing the semantic meaning of them.
Adding additional words demontstrates how the single-word versions are ambiguous: `HTMLTags, JSONSerializer, TAU_IS_2_PI, EPSILON_UNCERTAINTY`.
`true`: all-caps single-word identifiers are ignored and don't trigger errors (assumed to be all-caps snake case, i.e. constants)
`false`: all-caps single-word identifiers trigger errors (assumed to be loose camel case)
If you'd like to enforce `strict-camel-case` on single-word identifiers, but not trigger errors for single-word all-caps constants, consider keeping this option set to `false` and either adding your constant names to the `ignoredIdentifiers` option, or using `/* eslint-disable sequence/strict-camel-case */` around your constants.
If you'd like to enforce `strict-camel-case` on single-word identifiers, but not trigger errors for single-word all-caps constants, ~~consider keeping this option set to `false` and either adding your constant names to the `ignoredIdentifiers` option, or using `/* eslint-disable sequence/strict-camel-case */` around your constants.~~ use `ignoreSingleWordsIn` to specify how you are using constants or put your identifiers in `ignoredIdentifiers`.
48 changes: 38 additions & 10 deletions src/lib/rules/strict-camel-case.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,20 @@ module.exports = {
ignoreSingleWords: {
type: "boolean",
default: false
},
ignoreSingleWordsIn: {
type: "array",
items: [{
type: "string",
enum: [
"enum_member",
"first_class_constant",
"object_field",
"static_class_field"
]
}],
minItems: 0,
uniqueItems: true
}
},
additionalProperties: false
Expand All @@ -67,7 +81,8 @@ module.exports = {
ignoreImports = options.ignoreImports ?? false,
ignoredIdentifiers = options.ignoredIdentifiers || [],
allowOneCharWords = options.allowOneCharWords || "never",
ignoreSingleWords = options.ignoreSingleWords ?? false;
ignoreSingleWords = options.ignoreSingleWords ?? false,
ignoreSingleWordsIn = options.ignoreSingleWordsIn ?? [];

const LOG_RULE_PREFIX = "eslint-plugin-sequence/strict-camel-case ";
const LEVELS = ["OFF", "FATAL", "ERROR", "WARN", "INFO", "DEBUG", "TRACE"];
Expand Down Expand Up @@ -435,8 +450,12 @@ module.exports = {
}
}

function checkDeclarations(node) {
function checkDeclarations(node, exemptionType) {
for (const variable of context.getDeclaredVariables(node)) { // funcName+params, mult var decl in one stmt
if (ignoreSingleWordsIn.includes(exemptionType) && node.kind === "const" && isAllCaps(variable.name)) {
log("TRACE", `variable declarations skipping node.name=${node.name} due to config`);
continue;
}
log("DEBUG", `*Declaration checking variable.name=${variable.name}`);
const response = checkValidityAndGetSuggestion(variable.name);
if (response.valid) {
Expand All @@ -461,8 +480,8 @@ module.exports = {
}
}

function checkClassFieldsMethodsAndObjectFieldsMethods(node) {
if (!ignoreProperties) {
function checkClassFieldsMethodsAndObjectFieldsMethods(node, exemptionType) {
if (!ignoreProperties && !(ignoreSingleWordsIn.includes(exemptionType) && isAllCaps(node.name))) {
log("DEBUG", `class/object field/method declarations checking ${node.name}`);
const response = checkValidityAndGetSuggestion(node.name);
if (!response.valid) {
Expand All @@ -476,6 +495,15 @@ module.exports = {
}
}

function checkClassFields(node) {
const propDef = node.parent;
if (ignoreSingleWordsIn.includes("static_class_field") && propDef.static && isAllCaps(node.name)) {
log("TRACE", `static field declarations skipping node.name=${node.name} due to config`);
return;
}
checkClassFieldsMethodsAndObjectFieldsMethods(node);
}

function checkImportDeclarations(node) {
if (!ignoreImports) {
for (const variable of context.getDeclaredVariables(node)) {
Expand Down Expand Up @@ -534,20 +562,20 @@ module.exports = {
TSEnumDeclaration: checkDeclarations,
FunctionDeclaration: checkDeclarations,
FunctionExpression: checkDeclarations,
VariableDeclaration: checkDeclarations,
VariableDeclaration: node => checkDeclarations(node, "first_class_constant"),

// ---object literals---
"ObjectExpression > Property > Identifier.key": checkClassFieldsMethodsAndObjectFieldsMethods,
"ObjectExpression > Property > Identifier.key": node => checkClassFieldsMethodsAndObjectFieldsMethods(node, "object_field"),
// ---instance functions on classes---
"MethodDefinition > Identifier.key": checkClassFieldsMethodsAndObjectFieldsMethods,
// ---TS-only: class instance props---
"PropertyDefinition > Identifier.key": checkClassFieldsMethodsAndObjectFieldsMethods,
// ---TS-only: class instance and static props---
"PropertyDefinition > Identifier.key": checkClassFields,
// class { #privFunc() {...} }
"MethodDefinition > PrivateIdentifier.key": checkClassFieldsMethodsAndObjectFieldsMethods,
// class { #privPropName = ...; }
"PropertyDefinition > PrivateIdentifier.key": checkClassFieldsMethodsAndObjectFieldsMethods,
// ---class props `this.xyz = ...`, `window.abc = ...`---
"MemberExpression > Identifier.property": checkClassFieldsMethodsAndObjectFieldsMethods,
"AssignmentExpression > MemberExpression > Identifier.property": checkClassFieldsMethodsAndObjectFieldsMethods,
// TS interface fields
"TSInterfaceDeclaration > TSInterfaceBody > TSPropertySignature > Identifier.key":
checkClassFieldsMethodsAndObjectFieldsMethods,
Expand All @@ -557,7 +585,7 @@ module.exports = {
"TSTypeAliasDeclaration > TSTypeLiteral > TSPropertySignature > Identifier.key":
checkClassFieldsMethodsAndObjectFieldsMethods,
// TS enum members
"TSEnumDeclaration > TSEnumMember > Identifier.id": checkClassFieldsMethodsAndObjectFieldsMethods,
"TSEnumDeclaration > TSEnumMember > Identifier.id": node => checkClassFieldsMethodsAndObjectFieldsMethods(node, "enum_member"),

ImportDeclaration: checkImportDeclarations,

Expand Down
61 changes: 60 additions & 1 deletion src/tests/lib/rules/strict-camel-case.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,14 @@ es13RuleTester.run("strict-camel-case", rule, {
`import * as fs from "fs";`,
`import { exec as doExec } from "child_process";`,
`import { notMyFAULT } from "badNames";`,
`import { notMyFAULT as betterName } from "badNames";`
`import { notMyFAULT as betterName } from "badNames";`,
{
code: `export const MAX = 10;`,
options: [{ ignoreSingleWordsIn: [ "first_class_constant" ] }]
}, {
code: `const esEnumDirection = { NORTH: 0, EAST: 1, SOUTH: 2, WEST: 3 }`,
options: [{ ignoreSingleWordsIn: [ "object_field" ] }]
}
],
invalid: [{
code: `let xmlToHTML = () => {};`,
Expand Down Expand Up @@ -247,6 +254,32 @@ es13RuleTester.run("strict-camel-case", rule, {
errors: [{
messageId: "notCamelCaseWithSuggestion"
}]
}, {
code: `export const MAX = 10;`,
errors: [{
messageId: "notCamelCaseWithSuggestion"
}]
}, {
code: `export let MAX = 10;`,
errors: [{
messageId: "notCamelCaseWithSuggestion"
}]
}, {
code: `export var MAX = 10;`,
errors: [{
messageId: "notCamelCaseWithSuggestion"
}]
}, {
code: `const esEnumDirection = { NORTH: 0, EAST: 1, SOUTH: 2, WEST: 3 }`,
errors: [{
message: 'Identifier "NORTH" is not in strict camel case, should be "North".',
}, {
message: 'Identifier "EAST" is not in strict camel case, should be "East".',
}, {
message: 'Identifier "SOUTH" is not in strict camel case, should be "South".',
}, {
message: 'Identifier "WEST" is not in strict camel case, should be "West".',
}]
}]
});

Expand All @@ -262,6 +295,14 @@ tsRuleTester.run("strict-camel-case", rule, {
{
code: `enum HtmlTags { HEAD, BODY, DIV }`,
options: [{ ignoredIdentifiers: ["HEAD", "BODY", "DIV"] }]
},
{
code: `enum Direction { NORTH, EAST, SOUTH, WEST }`,
options: [{ ignoreSingleWordsIn: ["enum_member"]}]
},
{
code: `class Util { public static NAME = "TheUtil"; }`,
options: [{ ignoreSingleWordsIn: ["static_class_field"]}]
}
],
invalid: [{
Expand Down Expand Up @@ -305,5 +346,23 @@ tsRuleTester.run("strict-camel-case", rule, {
errors: [{
messageId: "notCamelCaseWithSuggestion"
}]
},
{
code: `enum Direction { NORTH, EAST, SOUTH, WEST }`,
errors: [{
message: 'Identifier "NORTH" is not in strict camel case, should be "North".',
}, {
message: 'Identifier "EAST" is not in strict camel case, should be "East".',
}, {
message: 'Identifier "SOUTH" is not in strict camel case, should be "South".',
}, {
message: 'Identifier "WEST" is not in strict camel case, should be "West".',
}]
},
{
code: `class Util { public static NAME = "TheUtil"; }`,
errors: [{
messageId: "notCamelCaseWithSuggestion"
}]
}]
});

0 comments on commit 1c4bae8

Please sign in to comment.