From ddf2d42305de6ebb2d9ca811a0858f6c40f23f32 Mon Sep 17 00:00:00 2001 From: Patricio Trevino Date: Wed, 28 Nov 2018 22:02:59 -0600 Subject: [PATCH] Fix: camelcase false positives on interface properties (fixes #177) (#183) * Fix: camelcase properties in interfaces (fixes #177) --- README.md | 1 + docs/rules/camelcase.md | 195 +++++++++++++++++++++++++++++++++++ lib/rules/camelcase.js | 114 ++++++++++++++++++++ tests/lib/rules/camelcase.js | 190 ++++++++++++++++++++++++++++++++++ 4 files changed, 500 insertions(+) create mode 100644 docs/rules/camelcase.md create mode 100644 lib/rules/camelcase.js create mode 100644 tests/lib/rules/camelcase.js diff --git a/README.md b/README.md index aeca84d..c715e39 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ This guarantees 100% compatibility between the plugin and the parser. * [`typescript/adjacent-overload-signatures`](./docs/rules/adjacent-overload-signatures.md) — Require that member overloads be consecutive +* [`typescript/camelcase`](./docs/rules/camelcase.md) — Enforce camelCase naming convention * [`typescript/class-name-casing`](./docs/rules/class-name-casing.md) — Require PascalCased class and interface names (`class-name` from TSLint) * [`typescript/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) — Require explicit return types on functions and class methods * [`typescript/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) — Require explicit accessibility modifiers on class properties and methods (`member-access` from TSLint) diff --git a/docs/rules/camelcase.md b/docs/rules/camelcase.md new file mode 100644 index 0000000..c3af576 --- /dev/null +++ b/docs/rules/camelcase.md @@ -0,0 +1,195 @@ +# Enforce camelCase naming convention (camelcase) + +When it comes to naming variables, style guides generally fall into one of two +camps: camelcase (`variableName`) and underscores (`variable_name`). This rule +focuses on using the camelcase approach. If your style guide calls for +camelCasing your variable names, then this rule is for you! + +## Rule Details + +This rule looks for any underscores (`_`) located within the source code. +It ignores leading and trailing underscores and only checks those in the middle +of a variable name. If ESLint decides that the variable is a constant +(all uppercase), then no warning will be thrown. Otherwise, a warning will be +thrown. This rule only flags definitions and assignments but not function calls. +In case of ES6 `import` statements, this rule only targets the name of the +variable that will be imported into the local module scope. + +***This rule was taken from the ESLint core rule `camelcase`.*** +***Available options and test cases may vary depending on the version of ESLint installed in the system.*** + +## Options + +This rule has an object option: + +* `"properties": "always"` (default) enforces camelcase style for property names +* `"properties": "never"` does not check property names +* `"ignoreDestructuring": false` (default) enforces camelcase style for destructured identifiers +* `"ignoreDestructuring": true` does not check destructured identifiers +* `allow` (`string[]`) list of properties to accept. Accept regex. + +### properties: "always" + +Examples of **incorrect** code for this rule with the default `{ "properties": "always" }` option: + +```js +/*eslint camelcase: "error"*/ + +import { no_camelcased } from "external-module" + +var my_favorite_color = "#112C85"; + +function do_something() { + // ... +} + +obj.do_something = function() { + // ... +}; + +function foo({ no_camelcased }) { + // ... +}; + +function foo({ isCamelcased: no_camelcased }) { + // ... +} + +function foo({ no_camelcased = 'default value' }) { + // ... +}; + +var obj = { + my_pref: 1 +}; + +var { category_id = 1 } = query; + +var { foo: no_camelcased } = bar; + +var { foo: bar_baz = 1 } = quz; +``` + +Examples of **correct** code for this rule with the default `{ "properties": "always" }` option: + +```js +/*eslint camelcase: "error"*/ + +import { no_camelcased as camelCased } from "external-module"; + +var myFavoriteColor = "#112C85"; +var _myFavoriteColor = "#112C85"; +var myFavoriteColor_ = "#112C85"; +var MY_FAVORITE_COLOR = "#112C85"; +var foo = bar.baz_boom; +var foo = { qux: bar.baz_boom }; + +obj.do_something(); +do_something(); +new do_something(); + +var { category_id: category } = query; + +function foo({ isCamelCased }) { + // ... +}; + +function foo({ isCamelCased: isAlsoCamelCased }) { + // ... +} + +function foo({ isCamelCased = 'default value' }) { + // ... +}; + +var { categoryId = 1 } = query; + +var { foo: isCamelCased } = bar; + +var { foo: isCamelCased = 1 } = quz; + +``` + +### properties: "never" + +Examples of **correct** code for this rule with the `{ "properties": "never" }` option: + +```js +/*eslint camelcase: ["error", {properties: "never"}]*/ + +var obj = { + my_pref: 1 +}; +``` + +### ignoreDestructuring: false + +Examples of **incorrect** code for this rule with the default `{ "ignoreDestructuring": false }` option: + +```js +/*eslint camelcase: "error"*/ + +var { category_id } = query; + +var { category_id = 1 } = query; + +var { category_id: category_id } = query; + +var { category_id: category_alias } = query; + +var { category_id: categoryId, ...other_props } = query; +``` + +### ignoreDestructuring: true + +Examples of **incorrect** code for this rule with the `{ "ignoreDestructuring": true }` option: + +```js +/*eslint camelcase: ["error", {ignoreDestructuring: true}]*/ + +var { category_id: category_alias } = query; + +var { category_id, ...other_props } = query; +``` + +Examples of **correct** code for this rule with the `{ "ignoreDestructuring": true }` option: + +```js +/*eslint camelcase: ["error", {ignoreDestructuring: true}]*/ + +var { category_id } = query; + +var { category_id = 1 } = query; + +var { category_id: category_id } = query; +``` + +## allow + +Examples of **correct** code for this rule with the `allow` option: + +```js +/*eslint camelcase: ["error", {allow: ["UNSAFE_componentWillMount"]}]*/ + +function UNSAFE_componentWillMount() { + // ... +} +``` + +```js +/*eslint camelcase: ["error", {allow: ["^UNSAFE_"]}]*/ + +function UNSAFE_componentWillMount() { + // ... +} + +function UNSAFE_componentWillMount() { + // ... +} +``` + +## When Not To Use It + +If you have established coding standards using a different naming convention (separating words with underscores), turn this rule off. + +Taken with ❤️ [from ESLint core](https://github.com/eslint/eslint/blob/master/docs/rules/camelcase.md) diff --git a/lib/rules/camelcase.js b/lib/rules/camelcase.js new file mode 100644 index 0000000..997945e --- /dev/null +++ b/lib/rules/camelcase.js @@ -0,0 +1,114 @@ +/** + * @fileoverview Rule to flag non-camelcased identifiers + * @author Patricio Trevino + */ +"use strict"; + +const baseRule = require("eslint/lib/rules/camelcase"); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: Object.assign({}, baseRule.meta, { + docs: { + description: "Enforce camelCase naming convention", + }, + }), + + create(context) { + const rules = baseRule.create(context); + const TS_PROPERTY_TYPES = [ + "TSPropertySignature", + "ClassProperty", + "TSParameterProperty", + "TSAbstractClassProperty", + ]; + + const options = context.options[0] || {}; + let properties = options.properties || ""; + const allow = options.allow || []; + + if (properties !== "always" && properties !== "never") { + properties = "always"; + } + + /** + * Checks if a string contains an underscore and isn't all upper-case + * @param {string} name The string to check. + * @returns {boolean} if the string is underscored + * @private + */ + function isUnderscored(name) { + // if there's an underscore, it might be A_CONSTANT, which is okay + return name.indexOf("_") > -1 && name !== name.toUpperCase(); + } + + /** + * Checks if a string match the ignore list + * @param {string} name The string to check. + * @returns {boolean} if the string is ignored + * @private + */ + function isAllowed(name) { + return ( + allow.findIndex( + entry => name === entry || name.match(new RegExp(entry)) + ) !== -1 + ); + } + + /** + * Checks if the the node is a valid TypeScript property type. + * @param {Node} node the node to be validated. + * @returns {boolean} true if the node is a TypeScript property type. + * @private + */ + function isTSPropertyType(node) { + if (!node.parent) return false; + if (TS_PROPERTY_TYPES.includes(node.parent.type)) return true; + + if (node.parent.type === "AssignmentPattern") { + return ( + node.parent.parent && + TS_PROPERTY_TYPES.includes(node.parent.parent.type) + ); + } + + return false; + } + + return { + Identifier(node) { + /* + * Leading and trailing underscores are commonly used to flag + * private/protected identifiers, strip them + */ + const name = node.name.replace(/^_+|_+$/g, ""); + + // First, we ignore the node if it match the ignore list + if (isAllowed(name)) { + return; + } + + // Check TypeScript specific nodes + if (isTSPropertyType(node)) { + if (properties === "always" && isUnderscored(name)) { + context.report({ + node, + messageId: "notCamelCase", + data: { name: node.name }, + }); + } + + return; + } + + // Let the base rule deal with the rest + // eslint-disable-next-line new-cap + rules.Identifier(node); + }, + }; + }, +}; diff --git a/tests/lib/rules/camelcase.js b/tests/lib/rules/camelcase.js new file mode 100644 index 0000000..99af9aa --- /dev/null +++ b/tests/lib/rules/camelcase.js @@ -0,0 +1,190 @@ +/** + * @fileoverview Tests for camelcase rule + * @author Guy Lilian + * @author Shahar Or + * @author Patricio Trevino + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +const ruleCamelcase = require("../../../lib/rules/camelcase"); +const RuleTester = require("eslint").RuleTester; + +const ruleTester = new RuleTester({ + parser: "typescript-eslint-parser", +}); + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +ruleTester.run("camelcase", ruleCamelcase, { + valid: [ + { + code: "interface Foo { b_ar: number }", + options: [{ properties: "never" }], + }, + { + code: "interface Foo { bar: number }", + options: [{ properties: "always" }], + }, + { + code: "class Foo { b_ar: number; }", + options: [{ properties: "never" }], + }, + { + code: "class Foo { bar: number; }", + options: [{ properties: "always" }], + }, + { + code: "class Foo { b_ar: number = 0; }", + options: [{ properties: "never" }], + }, + { + code: "class Foo { bar: number = 0; }", + options: [{ properties: "always" }], + }, + { + code: "class Foo { constructor(private b_ar: number) {} }", + options: [{ properties: "never" }], + }, + { + code: "class Foo { constructor(private bar: number) {} }", + options: [{ properties: "always" }], + }, + { + code: "class Foo { constructor(private b_ar: number = 0) {} }", + options: [{ properties: "never" }], + }, + { + code: "class Foo { constructor(private bar: number = 0) {} }", + options: [{ properties: "always" }], + }, + { + code: "abstract class Foo { b_ar: number; }", + options: [{ properties: "never" }], + }, + { + code: "abstract class Foo { bar: number; }", + options: [{ properties: "always" }], + }, + { + code: "abstract class Foo { b_ar: number = 0; }", + options: [{ properties: "never" }], + }, + { + code: "abstract class Foo { bar: number = 0; }", + options: [{ properties: "always" }], + }, + { + code: "abstract class Foo { abstract b_ar: number; }", + options: [{ properties: "never" }], + }, + { + code: "abstract class Foo { abstract bar: number; }", + options: [{ properties: "always" }], + }, + { + code: "abstract class Foo { abstract b_ar: number = 0; }", + options: [{ properties: "never" }], + }, + { + code: "abstract class Foo { abstract bar: number = 0; }", + options: [{ properties: "always" }], + }, + ], + + invalid: [ + { + code: "interface Foo { b_ar: number }", + options: [{ properties: "always" }], + errors: [ + { + message: "Identifier 'b_ar' is not in camel case.", + line: 1, + column: 17, + }, + ], + }, + { + code: "class Foo { b_ar: number; }", + options: [{ properties: "always" }], + errors: [ + { + message: "Identifier 'b_ar' is not in camel case.", + line: 1, + column: 13, + }, + ], + }, + { + code: "class Foo { constructor(private b_ar: number) {} }", + options: [{ properties: "always" }], + errors: [ + { + message: "Identifier 'b_ar' is not in camel case.", + line: 1, + column: 33, + }, + ], + }, + { + code: "class Foo { constructor(private b_ar: number = 0) {} }", + options: [{ properties: "always" }], + errors: [ + { + message: "Identifier 'b_ar' is not in camel case.", + line: 1, + column: 33, + }, + ], + }, + { + code: "abstract class Foo { b_ar: number; }", + options: [{ properties: "always" }], + errors: [ + { + message: "Identifier 'b_ar' is not in camel case.", + line: 1, + column: 22, + }, + ], + }, + { + code: "abstract class Foo { b_ar: number = 0; }", + options: [{ properties: "always" }], + errors: [ + { + message: "Identifier 'b_ar' is not in camel case.", + line: 1, + column: 22, + }, + ], + }, + { + code: "abstract class Foo { abstract b_ar: number; }", + options: [{ properties: "always" }], + errors: [ + { + message: "Identifier 'b_ar' is not in camel case.", + line: 1, + column: 31, + }, + ], + }, + { + code: "abstract class Foo { abstract b_ar: number = 0; }", + options: [{ properties: "always" }], + errors: [ + { + message: "Identifier 'b_ar' is not in camel case.", + line: 1, + column: 31, + }, + ], + }, + ], +});