From a684ae152ccd75055cf87813d0f3f2013049a0d6 Mon Sep 17 00:00:00 2001 From: Jules Blom Date: Thu, 9 Feb 2023 21:26:18 +0100 Subject: [PATCH] [New] `display-name`: add `checkContextObjects` option --- CHANGELOG.md | 4 + docs/rules/display-name.md | 29 ++++- lib/rules/display-name.js | 41 +++++++ lib/util/isCreateContext.js | 53 +++++++++ tests/lib/rules/display-name.js | 195 ++++++++++++++++++++++++++++++++ 5 files changed, 321 insertions(+), 1 deletion(-) create mode 100644 lib/util/isCreateContext.js diff --git a/CHANGELOG.md b/CHANGELOG.md index fac36a0f3d..a0f4f100b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,10 +5,14 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange ## Unreleased +### Added +* [`display-name`]: add `checkContextObjects` option ([#3529][] @JulesBlm) + ### Fixed * [`no-array-index-key`]: consider flatMap ([#3530][] @k-yle) [#3530]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3530 +[#3529]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3529 ## [7.32.2] - 2023.01.28 diff --git a/docs/rules/display-name.md b/docs/rules/display-name.md index e1c7dd6599..90d5c856d4 100644 --- a/docs/rules/display-name.md +++ b/docs/rules/display-name.md @@ -45,7 +45,7 @@ const Hello = React.memo(function Hello({ a }) { ```js ... -"react/display-name": [, { "ignoreTranspilerName": }] +"react/display-name": [, { "ignoreTranspilerName": , "checkContextObjects": }] ... ``` @@ -128,6 +128,33 @@ function HelloComponent() { module.exports = HelloComponent(); ``` +### checkContextObjects (default: `false`) + +`displayName` allows you to [name your context](https://reactjs.org/docs/context.html#contextdisplayname) object. This name is used in the React dev tools for the context's `Provider` and `Consumer`. +When `true` this rule will warn on context objects without a `displayName`. + +Examples of **incorrect** code for this rule: + +```jsx +const Hello = React.createContext(); +``` + +```jsx +const Hello = createContext(); +``` + +Examples of **correct** code for this rule: + +```jsx +const Hello = React.createContext(); +Hello.displayName = "HelloContext"; +``` + +```jsx +const Hello = createContext(); +Hello.displayName = "HelloContext"; +``` + ## About component detection For this rule to work we need to detect React components, this could be very hard since components could be declared in a lot of ways. diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index b25dbb0142..3d69a13c87 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -8,6 +8,7 @@ const values = require('object.values'); const Components = require('../util/Components'); +const isCreateContext = require('../util/isCreateContext'); const astUtil = require('../util/ast'); const componentUtil = require('../util/componentUtil'); const docsUrl = require('../util/docsUrl'); @@ -21,6 +22,7 @@ const report = require('../util/report'); const messages = { noDisplayName: 'Component definition is missing display name', + noContextDisplayName: 'Context definition is missing display name', }; module.exports = { @@ -40,6 +42,9 @@ module.exports = { ignoreTranspilerName: { type: 'boolean', }, + checkContextObjects: { + type: 'boolean', + }, }, additionalProperties: false, }], @@ -48,6 +53,9 @@ module.exports = { create: Components.detect((context, components, utils) => { const config = context.options[0] || {}; const ignoreTranspilerName = config.ignoreTranspilerName || false; + const checkContextObjects = (config.checkContextObjects || false) && testReactVersion(context, '>= 16.3.0'); + + const contextObjects = new Map(); /** * Mark a prop type as declared @@ -87,6 +95,16 @@ module.exports = { }); } + /** + * Reports missing display name for a given context object + * @param {Object} contextObj The context object to process + */ + function reportMissingContextDisplayName(contextObj) { + report(context, messages.noContextDisplayName, 'noContextDisplayName', { + node: contextObj.node, + }); + } + /** * Checks if the component have a name set by the transpiler * @param {ASTNode} node The AST node being checked. @@ -144,6 +162,16 @@ module.exports = { // -------------------------------------------------------------------------- return { + ExpressionStatement(node) { + if (checkContextObjects && isCreateContext(node)) { + contextObjects.set(node.expression.left.name, { node, hasDisplayName: false }); + } + }, + VariableDeclarator(node) { + if (checkContextObjects && isCreateContext(node)) { + contextObjects.set(node.id.name, { node, hasDisplayName: false }); + } + }, 'ClassProperty, PropertyDefinition'(node) { if (!propsUtil.isDisplayNameDeclaration(node)) { return; @@ -155,6 +183,14 @@ module.exports = { if (!propsUtil.isDisplayNameDeclaration(node.property)) { return; } + if ( + checkContextObjects + && node.object + && node.object.name + && contextObjects.has(node.object.name) + ) { + contextObjects.get(node.object.name).hasDisplayName = true; + } const component = utils.getRelatedComponent(node); if (!component) { return; @@ -258,6 +294,11 @@ module.exports = { values(list).filter((component) => !component.hasDisplayName).forEach((component) => { reportMissingDisplayName(component); }); + if (checkContextObjects) { + // Report missing display name for all context objects + const contextsList = Array.from(contextObjects.values()).filter((v) => !v.hasDisplayName); + contextsList.forEach((contextObj) => reportMissingContextDisplayName(contextObj)); + } }, }; }), diff --git a/lib/util/isCreateContext.js b/lib/util/isCreateContext.js new file mode 100644 index 0000000000..d5fdfe4551 --- /dev/null +++ b/lib/util/isCreateContext.js @@ -0,0 +1,53 @@ +'use strict'; + +/** + * Checks if the node is a React.createContext call + * @param {ASTNode} node - The AST node being checked. + * @returns {Boolean} - True if node is a React.createContext call, false if not. + */ +module.exports = function isCreateContext(node) { + if ( + node.init + && node.init.type === 'CallExpression' + && node.init.callee + && node.init.callee.name === 'createContext' + ) { + return true; + } + + if ( + node.init + && node.init.callee + && node.init.callee.type === 'MemberExpression' + && node.init.callee.property + && node.init.callee.property.name === 'createContext' + ) { + return true; + } + + if ( + node.expression + && node.expression.type === 'AssignmentExpression' + && node.expression.operator === '=' + && node.expression.right.type === 'CallExpression' + && node.expression.right.callee + && node.expression.right.callee.name === 'createContext' + ) { + return true; + } + + if ( + node.expression + && node.expression.type === 'AssignmentExpression' + && node.expression.operator === '=' + && node.expression.right.type === 'CallExpression' + && node.expression.right.callee + && node.expression.right.callee.type === 'MemberExpression' + && node.expression.right.callee.property + && node.expression.right.callee.property.name === 'createContext' + ) { + return true; + } + + return false; +}; diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 420bc47c7b..b497ffbebe 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -755,6 +755,129 @@ ruleTester.run('display-name', rule, { }, }, }, + { + code: ` + import React from 'react'; + + const Hello = React.createContext(); + Hello.displayName = "HelloContext" + `, + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + const Hello = createContext(); + Hello.displayName = "HelloContext" + `, + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + const Hello = createContext(); + + const obj = {}; + obj.displayName = "False positive"; + + Hello.displayName = "HelloContext" + `, + options: [{ checkContextObjects: true }], + }, + { + code: ` + import * as React from 'react'; + + const Hello = React.createContext(); + + const obj = {}; + obj.displayName = "False positive"; + + Hello.displayName = "HelloContext"; + `, + options: [{ checkContextObjects: true }], + }, + { + code: ` + const obj = {}; + obj.displayName = "False positive"; + `, + options: [{ checkContextObjects: true }], + }, + // React.createContext should be accepted in React versions in the following range: + // >= 16.13.0 + { + code: ` + import { createContext } from 'react'; + + const Hello = createContext(); + `, + settings: { + react: { + version: '16.2.0', + }, + }, + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + const Hello = createContext(); + Hello.displayName = "HelloContext"; + `, + settings: { + react: { + version: '>16.3.0', + }, + }, + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + let Hello; + Hello = createContext(); + Hello.displayName = "HelloContext"; + `, + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + const Hello = createContext(); + `, + settings: { + react: { + version: '>16.3.0', + }, + }, + options: [{ checkContextObjects: false }], + }, + { + code: ` + import { createContext } from 'react'; + + var Hello; + Hello = createContext(); + Hello.displayName = "HelloContext"; + `, + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + var Hello; + Hello = React.createContext(); + Hello.displayName = "HelloContext"; + `, + options: [{ checkContextObjects: true }], + }, ]), invalid: parsers.all([ @@ -1180,5 +1303,77 @@ ruleTester.run('display-name', rule, { }, ], }, + { + code: ` + import React from 'react'; + + const Hello = React.createContext(); + `, + errors: [ + { + messageId: 'noContextDisplayName', + line: 4, + }, + ], + options: [{ checkContextObjects: true }], + }, + { + code: ` + import * as React from 'react'; + + const Hello = React.createContext(); + `, + errors: [ + { + messageId: 'noContextDisplayName', + line: 4, + }, + ], + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + const Hello = createContext(); + `, + errors: [ + { + messageId: 'noContextDisplayName', + line: 4, + }, + ], + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + var Hello; + Hello = createContext(); + `, + errors: [ + { + messageId: 'noContextDisplayName', + line: 5, + }, + ], + options: [{ checkContextObjects: true }], + }, + { + code: ` + import { createContext } from 'react'; + + var Hello; + Hello = React.createContext(); + `, + errors: [ + { + messageId: 'noContextDisplayName', + line: 5, + }, + ], + options: [{ checkContextObjects: true }], + }, ]), });