From 6611c4b8f42520add983cc48fe4e14f7a02cc7cf Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Wed, 9 Oct 2019 11:46:08 -0700 Subject: [PATCH] Move error-subclass-name lint rule to GitHub Summary: Ports an internal ESLint rule used at Facebook, `error-subclass-name`, to cover the React Native codebase. This rule enforces that error classes ( = those with PascalCase names ending with `Error`) only extend other error classes, and that regular functions don't have names that could be mistaken for those of error classes. Reviewed By: rubennorte Differential Revision: D17829298 fbshipit-source-id: 834e457343034a0897ab394b6a2d941789953d2e --- .eslintrc | 3 +- package.json | 2 +- .../eslint-plugin-react-native-community/BUCK | 23 ++++ .../README.md | 12 +++ .../__tests__/error-subclass-name-test.js | 100 ++++++++++++++++++ .../__tests__/eslint-tester.js | 22 ++++ .../error-subclass-name.js | 66 ++++++++++++ .../index.js | 1 + yarn.lock | 4 +- 9 files changed, 228 insertions(+), 5 deletions(-) create mode 100644 packages/eslint-plugin-react-native-community/BUCK create mode 100644 packages/eslint-plugin-react-native-community/__tests__/error-subclass-name-test.js create mode 100644 packages/eslint-plugin-react-native-community/__tests__/eslint-tester.js create mode 100644 packages/eslint-plugin-react-native-community/error-subclass-name.js diff --git a/.eslintrc b/.eslintrc index 9c36dc1804..9385a7481c 100644 --- a/.eslintrc +++ b/.eslintrc @@ -11,7 +11,8 @@ "Libraries/**/*.js", ], rules: { - '@react-native-community/no-haste-imports': 2 + '@react-native-community/no-haste-imports': 2, + '@react-native-community/error-subclass-name': 2, } }, { diff --git a/package.json b/package.json index 314a47b7ce..682c86d426 100644 --- a/package.json +++ b/package.json @@ -119,7 +119,7 @@ "devDependencies": { "@babel/core": "^7.0.0", "@babel/generator": "^7.0.0", - "@react-native-community/eslint-plugin": "1.0.0", + "@react-native-community/eslint-plugin": "file:packages/eslint-plugin-react-native-community", "@reactions/component": "^2.0.2", "async": "^2.4.0", "babel-eslint": "10.0.1", diff --git a/packages/eslint-plugin-react-native-community/BUCK b/packages/eslint-plugin-react-native-community/BUCK new file mode 100644 index 0000000000..8fd3828943 --- /dev/null +++ b/packages/eslint-plugin-react-native-community/BUCK @@ -0,0 +1,23 @@ +load("@fbsource//tools/build_defs/third_party:yarn_defs.bzl", "yarn_workspace") + +yarn_workspace( + name = "yarn-workspace", + srcs = glob( + ["**/*.js"], + exclude = [ + "**/__fixtures__/**", + "**/__flowtests__/**", + "**/__mocks__/**", + "**/__server_snapshot_tests__/**", + "**/__tests__/**", + "**/node_modules/**", + "**/node_modules/.bin/**", + "**/.*", + "**/.*/**", + "**/.*/.*", + "**/*.xcodeproj/**", + "**/*.xcworkspace/**", + ], + ), + visibility = ["PUBLIC"], +) diff --git a/packages/eslint-plugin-react-native-community/README.md b/packages/eslint-plugin-react-native-community/README.md index 262c7e9217..702c35cd0c 100644 --- a/packages/eslint-plugin-react-native-community/README.md +++ b/packages/eslint-plugin-react-native-community/README.md @@ -19,3 +19,15 @@ Add to your eslint config (`.eslintrc`, or `eslintConfig` field in `package.json "plugins": ["@react-native-community"] } ``` + +## Rules + +### `error-subclass-name` + +**NOTE:** This rule is primarily used for developing React Native itself and is not generally applicable to other projects. + +Enforces that error classes ( = classes with PascalCase names ending with `Error`) only extend other error classes, and that regular functions don't have names that could be mistaken for those of error classes. + +### `no-haste-imports` + +Disallows Haste module names in `import` statements and `require()` calls. diff --git a/packages/eslint-plugin-react-native-community/__tests__/error-subclass-name-test.js b/packages/eslint-plugin-react-native-community/__tests__/error-subclass-name-test.js new file mode 100644 index 0000000000..6a11a6f3ae --- /dev/null +++ b/packages/eslint-plugin-react-native-community/__tests__/error-subclass-name-test.js @@ -0,0 +1,100 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails oncall+react_native + * @format + */ + +'use strict'; + +const ESLintTester = require('./eslint-tester.js'); + +const rule = require('../error-subclass-name.js'); + +const eslintTester = new ESLintTester(); + +const INVALID_SUPERCLASS_MESSAGE = + "'SomethingEndingWithError' must extend an error class (like 'Error') because its name is in PascalCase and ends with 'Error'."; +const INVALID_OWN_NAME_MESSAGE = + "'Foo' may not be the name of an error class. It should be in PascalCase and end with 'Error'."; +const MISSING_OWN_NAME_MESSAGE = + "An error class should have a PascalCase name ending with 'Error'."; +const INVALID_FUNCTION_NAME_MESSAGE = + "'SomethingEndingWithError' is a reserved name. PascalCase names ending with 'Error' are reserved for error classes and may not be used for regular functions. Either rename this function or convert it to a class that extends 'Error'."; + +eslintTester.run('../error-subclass-name', rule, { + valid: [ + 'class FooError extends Error {}', + '(class FooError extends Error {})', + 'class FooError extends SomethingEndingWithError {}', + '(class FooError extends SomethingEndingWithError {})', + 'function makeError() {}', + '(function () {})', + + // The following cases are currently allowed but could be disallowed in the + // future. This is technically an escape hatch. + 'class Foo extends SomeLibrary.FooError {}', + '(class extends SomeLibrary.FooError {})', + ], + invalid: [ + { + code: 'class SomethingEndingWithError {}', + errors: [{message: INVALID_SUPERCLASS_MESSAGE}], + }, + { + code: '(class SomethingEndingWithError {})', + errors: [{message: INVALID_SUPERCLASS_MESSAGE}], + }, + { + code: 'class Foo extends Error {}', + errors: [{message: INVALID_OWN_NAME_MESSAGE}], + }, + { + code: '(class Foo extends Error {})', + errors: [{message: INVALID_OWN_NAME_MESSAGE}], + }, + { + code: 'class Foo extends SomethingEndingWithError {}', + errors: [{message: INVALID_OWN_NAME_MESSAGE}], + }, + { + code: '(class Foo extends SomethingEndingWithError {})', + errors: [{message: INVALID_OWN_NAME_MESSAGE}], + }, + { + code: '(class extends Error {})', + errors: [{message: MISSING_OWN_NAME_MESSAGE}], + }, + { + code: 'class SomethingEndingWithError extends C {}', + errors: [{message: INVALID_SUPERCLASS_MESSAGE}], + }, + { + code: '(class SomethingEndingWithError extends C {})', + errors: [{message: INVALID_SUPERCLASS_MESSAGE}], + }, + { + code: 'function SomethingEndingWithError() {}', + errors: [{message: INVALID_FUNCTION_NAME_MESSAGE}], + }, + { + code: '(function SomethingEndingWithError() {})', + errors: [{message: INVALID_FUNCTION_NAME_MESSAGE}], + }, + + // The following cases are intentionally disallowed because the member + // expression `SomeLibrary.FooError` doesn't imply that the superclass is + // actually declared with the name `FooError`. + { + code: 'class SomethingEndingWithError extends SomeLibrary.FooError {}', + errors: [{message: INVALID_SUPERCLASS_MESSAGE}], + }, + { + code: '(class SomethingEndingWithError extends SomeLibrary.FooError {})', + errors: [{message: INVALID_SUPERCLASS_MESSAGE}], + }, + ], +}); diff --git a/packages/eslint-plugin-react-native-community/__tests__/eslint-tester.js b/packages/eslint-plugin-react-native-community/__tests__/eslint-tester.js new file mode 100644 index 0000000000..4218487f1d --- /dev/null +++ b/packages/eslint-plugin-react-native-community/__tests__/eslint-tester.js @@ -0,0 +1,22 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +'use strict'; + +const ESLintTester = require('eslint').RuleTester; + +ESLintTester.setDefaultConfig({ + parser: 'babel-eslint', + parserOptions: { + ecmaVersion: 6, + sourceType: 'module', + }, +}); + +module.exports = ESLintTester; diff --git a/packages/eslint-plugin-react-native-community/error-subclass-name.js b/packages/eslint-plugin-react-native-community/error-subclass-name.js new file mode 100644 index 0000000000..0c0c8f33f1 --- /dev/null +++ b/packages/eslint-plugin-react-native-community/error-subclass-name.js @@ -0,0 +1,66 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +'use strict'; + +module.exports = function rule(context) { + function classVisitor(node) { + const {superClass, id} = node; + const nodeIsError = isErrorLikeId(id); + const superIsError = isErrorLikeId(superClass); + if (nodeIsError && !superIsError) { + const idName = getNameFromId(id); + context.report({ + node: superClass || id, + message: `'${idName}' must extend an error class (like 'Error') because its name is in PascalCase and ends with 'Error'.`, + }); + } else if (superIsError && !nodeIsError) { + const idName = getNameFromId(id); + context.report({ + node: id || node, + message: idName + ? `'${idName}' may not be the name of an error class. It should be in PascalCase and end with 'Error'.` + : "An error class should have a PascalCase name ending with 'Error'.", + }); + } + } + + function functionVisitor(node) { + const {id} = node; + const nodeIsError = isErrorLikeId(id); + if (nodeIsError) { + const idName = getNameFromId(id); + context.report({ + node: id, + message: `'${idName}' is a reserved name. PascalCase names ending with 'Error' are reserved for error classes and may not be used for regular functions. Either rename this function or convert it to a class that extends 'Error'.`, + }); + } + } + + return { + ClassDeclaration: classVisitor, + ClassExpression: classVisitor, + FunctionExpression: functionVisitor, + FunctionDeclaration: functionVisitor, + }; +}; + +// Checks whether `node` is an identifier (or similar name node) with a +// PascalCase name ending with 'Error'. +function isErrorLikeId(node) { + return ( + node && node.type === 'Identifier' && /^([A-Z].*)?Error$/.test(node.name) + ); +} + +// If `node` is an identifier (or similar name node), returns its name as a +// string. Otherwise returns null. +function getNameFromId(node) { + return node ? node.name : null; +} diff --git a/packages/eslint-plugin-react-native-community/index.js b/packages/eslint-plugin-react-native-community/index.js index f016505750..aac041d58e 100644 --- a/packages/eslint-plugin-react-native-community/index.js +++ b/packages/eslint-plugin-react-native-community/index.js @@ -8,5 +8,6 @@ */ exports.rules = { + 'error-subclass-name': require('./error-subclass-name'), 'no-haste-imports': require('./no-haste-imports'), }; diff --git a/yarn.lock b/yarn.lock index 722094d43e..53583871e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1145,10 +1145,8 @@ shell-quote "1.6.1" ws "^1.1.0" -"@react-native-community/eslint-plugin@1.0.0": +"@react-native-community/eslint-plugin@file:packages/eslint-plugin-react-native-community": version "1.0.0" - resolved "https://registry.yarnpkg.com/@react-native-community/eslint-plugin/-/eslint-plugin-1.0.0.tgz#ae9a430f2c5795debca491f15a989fce86ea75a0" - integrity sha512-GLhSN8dRt4lpixPQh+8prSCy6PYk/MT/mvji/ojAd5yshowDo6HFsimCSTD/uWAdjpUq91XK9tVdTNWfGRlKQA== "@reactions/component@^2.0.2": version "2.0.2"