From 2d5888baa922c2083b8932245006dd0caa8bdba0 Mon Sep 17 00:00:00 2001 From: Aaron Rodriguez Date: Tue, 24 Oct 2023 21:39:04 -0700 Subject: [PATCH 1/2] make a selector in strict-camel-case more specific The selector should match `this.xyz = ...` and `window.xyz = ...`, but not `console.log(MyClass.A_CONSTANT)`, and now it doesn't. --- src/lib/rules/strict-camel-case.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/rules/strict-camel-case.js b/src/lib/rules/strict-camel-case.js index 9822f73..6c3954f 100644 --- a/src/lib/rules/strict-camel-case.js +++ b/src/lib/rules/strict-camel-case.js @@ -547,7 +547,7 @@ module.exports = { // 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, From 3d0293754f67f1a586253289f5a1473195bcc183 Mon Sep 17 00:00:00 2001 From: Aaron Rodriguez Date: Wed, 25 Oct 2023 22:15:52 -0700 Subject: [PATCH 2/2] - 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 --- package.json | 2 +- src/docs/strict-camel-case.md | 55 +++++++++++++++++++-- src/lib/rules/strict-camel-case.js | 46 ++++++++++++++---- src/tests/lib/rules/strict-camel-case.js | 61 +++++++++++++++++++++++- 4 files changed, 149 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 34a0086..a937f61 100644 --- a/package.json +++ b/package.json @@ -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": { diff --git a/src/docs/strict-camel-case.md b/src/docs/strict-camel-case.md index 2131d0b..7cd879b 100644 --- a/src/docs/strict-camel-case.md +++ b/src/docs/strict-camel-case.md @@ -71,7 +71,8 @@ Further reading: "ignoreImports": false, "ignoredIdentifiers": ["htmlToXML", "legacyAPI"], "allowOneCharWords": "last", - "ignoreSingleWords": false + "ignoreSingleWords": false, + "ignoreSingleWordsIn": ["enum_member", "static_class_field"] } ], ... @@ -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 @@ -179,14 +180,60 @@ 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`. @@ -194,4 +241,4 @@ Adding additional words demontstrates how the single-word versions are ambiguous `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`. diff --git a/src/lib/rules/strict-camel-case.js b/src/lib/rules/strict-camel-case.js index 6c3954f..58b451a 100644 --- a/src/lib/rules/strict-camel-case.js +++ b/src/lib/rules/strict-camel-case.js @@ -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 @@ -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"]; @@ -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) { @@ -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) { @@ -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)) { @@ -534,14 +562,14 @@ 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 = ...; } @@ -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, diff --git a/src/tests/lib/rules/strict-camel-case.js b/src/tests/lib/rules/strict-camel-case.js index 0cb6bd1..a38ac86 100644 --- a/src/tests/lib/rules/strict-camel-case.js +++ b/src/tests/lib/rules/strict-camel-case.js @@ -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 = () => {};`, @@ -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".', + }] }] }); @@ -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: [{ @@ -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" + }] }] });