From 0e1009d2e0ba586061e150cd6444f27c8fa86d05 Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Fri, 17 Nov 2017 22:36:08 +0200 Subject: [PATCH 01/12] Remove unneeded comment --- lib/rules/no-typos.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/rules/no-typos.js b/lib/rules/no-typos.js index 8d1e8845fc..280c7a6840 100644 --- a/lib/rules/no-typos.js +++ b/lib/rules/no-typos.js @@ -85,7 +85,6 @@ module.exports = { node.properties.forEach(prop => checkValidProp(prop.value)); } } - /* eslint-enable no-use-before-define */ function reportErrorIfClassPropertyCasingTypo(node, propertyName) { if (propertyName === 'propTypes' || propertyName === 'contextTypes' || propertyName === 'childContextTypes') { From 4abebc3e8320d1156fbe11b4ccbe4cfe2de3ad33 Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Thu, 1 Feb 2018 12:11:26 -0500 Subject: [PATCH 02/12] Detect import and require of PropTypes (rebased) --- lib/rules/no-typos.js | 38 ++++++- tests/lib/rules/no-typos.js | 206 +++++++++++++++++++++++++++--------- 2 files changed, 188 insertions(+), 56 deletions(-) diff --git a/lib/rules/no-typos.js b/lib/rules/no-typos.js index 280c7a6840..12f386eb9c 100644 --- a/lib/rules/no-typos.js +++ b/lib/rules/no-typos.js @@ -36,6 +36,8 @@ module.exports = { }, create: Components.detect((context, components, utils) => { + let propTypesPackageName; + function checkValidPropTypeQualfier(node) { if (node.name !== 'isRequired') { context.report({ @@ -56,14 +58,28 @@ module.exports = { /* eslint-disable no-use-before-define */ function checkValidProp(node) { - if (node && node.type === 'MemberExpression' && node.object.type === 'MemberExpression') { + if (!propTypesPackageName || !node) { + return; + } + + if ( + node.type === 'MemberExpression' && + node.object.type === 'MemberExpression' && + node.object.object.name === propTypesPackageName + ) { // PropTypes.myProp.isRequired checkValidPropType(node.object.property); checkValidPropTypeQualfier(node.property); - } else if (node && node.type === 'MemberExpression' && node.object.type === 'Identifier' && node.property.name !== 'isRequired') { + } else if ( + node.type === 'MemberExpression' && + node.object.type === 'Identifier' && + node.object.name === propTypesPackageName && + node.property.name !== 'isRequired' + ) { // PropTypes.myProp checkValidPropType(node.property); - } else if (node && ( - node.type === 'MemberExpression' && node.object.type === 'CallExpression' || node.type === 'CallExpression' - )) { + } else if ( + (node.type === 'MemberExpression' && node.object.type === 'CallExpression') || + node.type === 'CallExpression' + ) { // Shapes if (node.type === 'MemberExpression') { checkValidPropTypeQualfier(node.property); node = node.object; @@ -113,6 +129,18 @@ module.exports = { } return { + ImportDeclaration: function(node) { + if (node.source && node.source.value === 'prop-types') { // import PropType from "prop-types" + propTypesPackageName = node.specifiers[0].local.name; + } + }, + + CallExpression: function(node) { + if (node.callee.name === 'require' && node.arguments[0].value === 'prop-types') { + propTypesPackageName = node.parent.id.name; + } + }, + ClassProperty: function(node) { if (!node.static || !utils.isES6Component(node.parent.parent)) { return; diff --git a/tests/lib/rules/no-typos.js b/tests/lib/rules/no-typos.js index bd4bfc5a37..5987a5df16 100644 --- a/tests/lib/rules/no-typos.js +++ b/tests/lib/rules/no-typos.js @@ -260,81 +260,134 @@ ruleTester.run('no-typos', rule, { parser: 'babel-eslint', parserOptions: parserOptions }, { - code: `class Component extends React.Component {}; - Component.propTypes = { - a: PropTypes.number.isRequired - } + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.number.isRequired + } `, parser: 'babel-eslint', parserOptions: parserOptions }, { - code: `class Component extends React.Component {}; - Component.propTypes = { - e: PropTypes.shape({ - ea: PropTypes.string, - }) - } + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.propTypes = { + e: PropTypes.shape({ + ea: PropTypes.string, + }) + } `, parser: 'babel-eslint', parserOptions: parserOptions }, { - code: `class Component extends React.Component {}; - Component.propTypes = { - a: PropTypes.string, - b: PropTypes.string.isRequired, - c: PropTypes.shape({ - d: PropTypes.string, - e: PropTypes.number.isRequired, - }).isRequired - } + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.string, + b: PropTypes.string.isRequired, + c: PropTypes.shape({ + d: PropTypes.string, + e: PropTypes.number.isRequired, + }).isRequired + } `, parser: 'babel-eslint', parserOptions: parserOptions }, { - code: `class Component extends React.Component {}; - Component.propTypes = { - a: PropTypes.oneOfType([ - PropTypes.string, - PropTypes.number - ]) - } + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.oneOfType([ + PropTypes.string, + PropTypes.number + ]) + } `, parser: 'babel-eslint', parserOptions: parserOptions }, { - code: `class Component extends React.Component {}; - Component.propTypes = { - a: PropTypes.oneOf([ - 'hello', - 'hi' - ]) - } + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.oneOf([ + 'hello', + 'hi' + ]) + } `, parser: 'babel-eslint', parserOptions: parserOptions }, { - code: `class Component extends React.Component {}; - Component.childContextTypes = { - a: PropTypes.string, - b: PropTypes.string.isRequired, - c: PropTypes.shape({ - d: PropTypes.string, - e: PropTypes.number.isRequired, - }).isRequired - } + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.childContextTypes = { + a: PropTypes.string, + b: PropTypes.string.isRequired, + c: PropTypes.shape({ + d: PropTypes.string, + e: PropTypes.number.isRequired, + }).isRequired + } `, parser: 'babel-eslint', parserOptions: parserOptions }, { - code: `class Component extends React.Component {}; - Component.contextTypes = { - a: PropTypes.string, - b: PropTypes.string.isRequired, - c: PropTypes.shape({ - d: PropTypes.string, - e: PropTypes.number.isRequired, - }).isRequired - } + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.contextTypes = { + a: PropTypes.string, + b: PropTypes.string.isRequired, + c: PropTypes.shape({ + d: PropTypes.string, + e: PropTypes.number.isRequired, + }).isRequired + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions + }, { + code: ` + import PropTypes from 'prop-types' + import * as MyPropTypes from 'lib/my-prop-types' + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.string, + b: MyPropTypes.MYSTRING, + c: MyPropTypes.MYSTRING.isRequired, + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions + }, { + code: ` + const PropTypes = require('prop-types'); + const MyPropTypes = require('lib/my-prop-types') + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.string, + b: MyPropTypes.MYSTRING, + c: MyPropTypes.MYSTRING.isRequired, + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions + }, { + code: ` + const RealPropTypes = require('prop-types'); + const MyPropTypes = require('lib/my-prop-types') + class Component extends React.Component {}; + Component.propTypes = { + a: RealPropTypes.string, + b: MyPropTypes.MYSTRING, + c: MyPropTypes.MYSTRING.isRequired, + } `, parser: 'babel-eslint', parserOptions: parserOptions @@ -710,6 +763,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` + import PropTypes from "prop-types"; class Component extends React.Component {}; Component.propTypes = { a: PropTypes.Number.isRequired @@ -722,6 +776,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` + import PropTypes from "prop-types"; class Component extends React.Component {}; Component.propTypes = { a: PropTypes.number.isrequired @@ -734,6 +789,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` + import PropTypes from "prop-types"; class Component extends React.Component {}; Component.propTypes = { a: PropTypes.Number @@ -746,6 +802,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` + import PropTypes from "prop-types"; class Component extends React.Component {}; Component.propTypes = { a: PropTypes.shape({ @@ -761,6 +818,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` + import PropTypes from "prop-types"; class Component extends React.Component {}; Component.propTypes = { a: PropTypes.oneOfType([ @@ -776,6 +834,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` + import PropTypes from "prop-types"; class Component extends React.Component {}; Component.propTypes = { a: PropTypes.bools, @@ -797,6 +856,29 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.childContextTypes = { + a: PropTypes.bools, + b: PropTypes.Array, + c: PropTypes.function, + d: PropTypes.objectof, + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] + }, { + code: ` + const PropTypes = require('prop-types'); class Component extends React.Component {}; Component.childContextTypes = { a: PropTypes.bools, @@ -847,5 +929,27 @@ ruleTester.run('no-typos', rule, { }, { message: 'Typo in prop type chain qualifier: isrequired' }] + }, { + code: ` + const RealPropTypes = require('prop-types'); + class Component extends React.Component {}; + Component.childContextTypes = { + a: RealPropTypes.bools, + b: RealPropTypes.Array, + c: RealPropTypes.function, + d: RealPropTypes.objectof, + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] }] }); From ddba87677926d41e6e53dd615232c4cefc7bd868 Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Thu, 1 Feb 2018 12:12:02 -0500 Subject: [PATCH 03/12] Fix isRequired tests after custom proptypes changes The typos were being ignored, because without `PropTypes`, the un-namespaced identifiers looked like custom proptypes. --- tests/lib/rules/no-typos.js | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/lib/rules/no-typos.js b/tests/lib/rules/no-typos.js index 5987a5df16..72375aa70c 100644 --- a/tests/lib/rules/no-typos.js +++ b/tests/lib/rules/no-typos.js @@ -899,33 +899,37 @@ ruleTester.run('no-typos', rule, { message: 'Typo in declared prop type: objectof' }] }, { - code: `class Component extends React.Component {}; + code: ` + const PropTypes = require('prop-types'); + class Component extends React.Component {}; Component.propTypes = { - a: string.isrequired, - b: shape({ - c: number + a: PropTypes.string.isrequired, + b: PropTypes.shape({ + c: PropTypes.number }).isrequired } `, parserOptions: parserOptions, errors: [{ - message: 'Typo in declared prop type: isrequired' + message: 'Typo in prop type chain qualifier: isrequired' }, { message: 'Typo in prop type chain qualifier: isrequired' }] }, { - code: `class Component extends React.Component {}; + code: ` + const PropTypes = require('prop-types'); + class Component extends React.Component {}; Component.propTypes = { - a: string.isrequired, - b: shape({ - c: number + a: PropTypes.string.isrequired, + b: PropTypes.shape({ + c: PropTypes.number }).isrequired } `, parser: 'babel-eslint', parserOptions: parserOptions, errors: [{ - message: 'Typo in declared prop type: isrequired' + message: 'Typo in prop type chain qualifier: isrequired' }, { message: 'Typo in prop type chain qualifier: isrequired' }] From ec01d9acd2ef8579ee1e7b554d69208bead9aa45 Mon Sep 17 00:00:00 2001 From: Joachim Seminck Date: Sun, 26 Nov 2017 21:53:13 +0200 Subject: [PATCH 04/12] Add react import detection --- lib/rules/no-typos.js | 17 +++++---- tests/lib/rules/no-typos.js | 69 ++++++++++++++++++++++++++++--------- 2 files changed, 63 insertions(+), 23 deletions(-) diff --git a/lib/rules/no-typos.js b/lib/rules/no-typos.js index 12f386eb9c..2905f1cc1f 100644 --- a/lib/rules/no-typos.js +++ b/lib/rules/no-typos.js @@ -37,6 +37,7 @@ module.exports = { create: Components.detect((context, components, utils) => { let propTypesPackageName; + let reactPackageName; function checkValidPropTypeQualfier(node) { if (node.name !== 'isRequired') { @@ -58,7 +59,7 @@ module.exports = { /* eslint-disable no-use-before-define */ function checkValidProp(node) { - if (!propTypesPackageName || !node) { + if ((!propTypesPackageName && !reactPackageName) || !node) { return; } @@ -132,12 +133,14 @@ module.exports = { ImportDeclaration: function(node) { if (node.source && node.source.value === 'prop-types') { // import PropType from "prop-types" propTypesPackageName = node.specifiers[0].local.name; - } - }, - - CallExpression: function(node) { - if (node.callee.name === 'require' && node.arguments[0].value === 'prop-types') { - propTypesPackageName = node.parent.id.name; + } else if (node.source && node.source.value === 'react') { // import { PropTypes } from "react" + reactPackageName = node.specifiers[0].local.name; + + propTypesPackageName = node.specifiers.length > 1 && ( + node.specifiers + .filter(specifier => specifier.imported && specifier.imported.name === 'PropTypes') + .map(specifier => specifier.local.name) + ); } }, diff --git a/tests/lib/rules/no-typos.js b/tests/lib/rules/no-typos.js index 72375aa70c..ff06519348 100644 --- a/tests/lib/rules/no-typos.js +++ b/tests/lib/rules/no-typos.js @@ -14,7 +14,8 @@ const parserOptions = { ecmaVersion: 6, ecmaFeatures: { jsx: true - } + }, + sourceType: 'module' }; // ----------------------------------------------------------------------------- @@ -337,6 +338,34 @@ ruleTester.run('no-typos', rule, { `, parser: 'babel-eslint', parserOptions: parserOptions + }, { + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.oneOf([ + 'hello', + 'hi' + ]) + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions + }, { + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.childContextTypes = { + a: PropTypes.string, + b: PropTypes.string.isRequired, + c: PropTypes.shape({ + d: PropTypes.string, + e: PropTypes.number.isRequired, + }).isRequired + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions }, { code: ` import PropTypes from "prop-types"; @@ -367,26 +396,22 @@ ruleTester.run('no-typos', rule, { parserOptions: parserOptions }, { code: ` - const PropTypes = require('prop-types'); - const MyPropTypes = require('lib/my-prop-types') + import PropTypes from "prop-types" + import * as MyPropTypes from 'lib/my-prop-types' class Component extends React.Component {}; Component.propTypes = { - a: PropTypes.string, - b: MyPropTypes.MYSTRING, - c: MyPropTypes.MYSTRING.isRequired, + b: PropTypes.string, + a: MyPropTypes.MYSTRING, } `, parser: 'babel-eslint', parserOptions: parserOptions }, { code: ` - const RealPropTypes = require('prop-types'); - const MyPropTypes = require('lib/my-prop-types') + import CustomReact from "react" class Component extends React.Component {}; Component.propTypes = { - a: RealPropTypes.string, - b: MyPropTypes.MYSTRING, - c: MyPropTypes.MYSTRING.isRequired, + b: CustomReact.PropTypes.string, } `, parser: 'babel-eslint', @@ -878,7 +903,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` - const PropTypes = require('prop-types'); + import PropTypes from 'prop-types'; class Component extends React.Component {}; Component.childContextTypes = { a: PropTypes.bools, @@ -900,7 +925,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` - const PropTypes = require('prop-types'); + import PropTypes from 'prop-types'; class Component extends React.Component {}; Component.propTypes = { a: PropTypes.string.isrequired, @@ -908,7 +933,7 @@ ruleTester.run('no-typos', rule, { c: PropTypes.number }).isrequired } - `, + `, parserOptions: parserOptions, errors: [{ message: 'Typo in prop type chain qualifier: isrequired' @@ -917,7 +942,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` - const PropTypes = require('prop-types'); + import PropTypes from 'prop-types'; class Component extends React.Component {}; Component.propTypes = { a: PropTypes.string.isrequired, @@ -935,7 +960,7 @@ ruleTester.run('no-typos', rule, { }] }, { code: ` - const RealPropTypes = require('prop-types'); + import RealPropTypes from 'prop-types'; class Component extends React.Component {}; Component.childContextTypes = { a: RealPropTypes.bools, @@ -955,5 +980,17 @@ ruleTester.run('no-typos', rule, { }, { message: 'Typo in declared prop type: objectof' }] + }, { + code: ` + import RealReactDifferentName from "react" + Component.propTypes = { + b: RealReactDifferentName.PropTypes.STRING, + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: STRING' + }] }] }); From d318e52826d27b291edad0bf9dca0d3f09ea01aa Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Thu, 1 Feb 2018 10:37:43 -0500 Subject: [PATCH 05/12] Remove failing test for renamed React import It's a minor corner case, and removing it allows the major issue to be resolved. If it's really necessary to handle that corner case, I suggest creating a new issue to track just that case. --- tests/lib/rules/no-typos.js | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/tests/lib/rules/no-typos.js b/tests/lib/rules/no-typos.js index ff06519348..1a93e380bb 100644 --- a/tests/lib/rules/no-typos.js +++ b/tests/lib/rules/no-typos.js @@ -980,17 +980,5 @@ ruleTester.run('no-typos', rule, { }, { message: 'Typo in declared prop type: objectof' }] - }, { - code: ` - import RealReactDifferentName from "react" - Component.propTypes = { - b: RealReactDifferentName.PropTypes.STRING, - } - `, - parser: 'babel-eslint', - parserOptions: parserOptions, - errors: [{ - message: 'Typo in prop type chain qualifier: STRING' - }] }] }); From 352642ff43d77368dfc1b54b3206cdab37c0df46 Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Fri, 9 Feb 2018 18:09:08 -0500 Subject: [PATCH 06/12] Add common use-case tests and fix bugs Added tests for: - `import { PropTypes } from 'react';` - `React.PropTypes`o Fixed a couple subtle bugs that made these new tests fail, and made a small refactoring that made this code easier to understand and fix. --- lib/rules/no-typos.js | 76 ++++++++++++++++++++--------------- tests/lib/rules/no-typos.js | 80 +++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 33 deletions(-) diff --git a/lib/rules/no-typos.js b/lib/rules/no-typos.js index 2905f1cc1f..b5ceb56a9c 100644 --- a/lib/rules/no-typos.js +++ b/lib/rules/no-typos.js @@ -36,8 +36,8 @@ module.exports = { }, create: Components.detect((context, components, utils) => { - let propTypesPackageName; - let reactPackageName; + let propTypesPackageName = null; + let reactPackageName = null; function checkValidPropTypeQualfier(node) { if (node.name !== 'isRequired') { @@ -57,43 +57,53 @@ module.exports = { } } + function isPropTypesPackage(node) { + return ( + node.type === 'Identifier' && + node.name == propTypesPackageName + ) || ( + node.type === 'MemberExpression' && + node.property.name === 'PropTypes' && + node.object.name == reactPackageName + ); + } + + function checkValidCallExpression(node) { + const callee = node.callee; + if (callee.type === 'MemberExpression' && callee.property.name === 'shape') { + checkValidPropObject(node.arguments[0]); + } else if (callee.type === 'MemberExpression' && callee.property.name === 'oneOfType') { + const args = node.arguments[0]; + if (args && args.type === 'ArrayExpression') { + args.elements.forEach(el => checkValidProp(el)); + } + } + } + /* eslint-disable no-use-before-define */ function checkValidProp(node) { if ((!propTypesPackageName && !reactPackageName) || !node) { return; } - if ( - node.type === 'MemberExpression' && - node.object.type === 'MemberExpression' && - node.object.object.name === propTypesPackageName - ) { // PropTypes.myProp.isRequired - checkValidPropType(node.object.property); - checkValidPropTypeQualfier(node.property); - } else if ( - node.type === 'MemberExpression' && - node.object.type === 'Identifier' && - node.object.name === propTypesPackageName && - node.property.name !== 'isRequired' - ) { // PropTypes.myProp - checkValidPropType(node.property); - } else if ( - (node.type === 'MemberExpression' && node.object.type === 'CallExpression') || - node.type === 'CallExpression' - ) { // Shapes - if (node.type === 'MemberExpression') { + if (node.type === 'MemberExpression') { + if ( + node.object.type === 'MemberExpression' && + isPropTypesPackage(node.object.object) + ) { // PropTypes.myProp.isRequired + checkValidPropType(node.object.property); checkValidPropTypeQualfier(node.property); - node = node.object; - } - const callee = node.callee; - if (callee.type === 'MemberExpression' && callee.property.name === 'shape') { - checkValidPropObject(node.arguments[0]); - } else if (callee.type === 'MemberExpression' && callee.property.name === 'oneOfType') { - const args = node.arguments[0]; - if (args && args.type === 'ArrayExpression') { - args.elements.forEach(el => checkValidProp(el)); - } + } else if ( + isPropTypesPackage(node.object) && + node.property.name !== 'isRequired' + ) { // PropTypes.myProp + checkValidPropType(node.property); + } else if (node.object.type === 'CallExpression') { + checkValidPropTypeQualfier(node.property); + checkValidCallExpression(node.object); } + } else if (node.type === 'CallExpression') { + checkValidCallExpression(node); } } @@ -136,11 +146,11 @@ module.exports = { } else if (node.source && node.source.value === 'react') { // import { PropTypes } from "react" reactPackageName = node.specifiers[0].local.name; - propTypesPackageName = node.specifiers.length > 1 && ( + propTypesPackageName = node.specifiers.length >= 1 ? ( node.specifiers .filter(specifier => specifier.imported && specifier.imported.name === 'PropTypes') .map(specifier => specifier.local.name) - ); + ) : null; } }, diff --git a/tests/lib/rules/no-typos.js b/tests/lib/rules/no-typos.js index 1a93e380bb..8b1b1b55c5 100644 --- a/tests/lib/rules/no-typos.js +++ b/tests/lib/rules/no-typos.js @@ -980,5 +980,85 @@ ruleTester.run('no-typos', rule, { }, { message: 'Typo in declared prop type: objectof' }] + }, { + code: ` + import React from 'react'; + class Component extends React.Component {}; + Component.propTypes = { + a: React.PropTypes.string.isrequired, + b: React.PropTypes.shape({ + c: React.PropTypes.number + }).isrequired + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: isrequired' + }, { + message: 'Typo in prop type chain qualifier: isrequired' + }] + }, { + code: ` + import React from 'react'; + class Component extends React.Component {}; + Component.childContextTypes = { + a: React.PropTypes.bools, + b: React.PropTypes.Array, + c: React.PropTypes.function, + d: React.PropTypes.objectof, + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] + }, { + code: ` + import { PropTypes } from 'react'; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.string.isrequired, + b: PropTypes.shape({ + c: PropTypes.number + }).isrequired + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: isrequired' + }, { + message: 'Typo in prop type chain qualifier: isrequired' + }] + }, { + code: ` + import { PropTypes } from 'react'; + class Component extends React.Component {}; + Component.childContextTypes = { + a: PropTypes.bools, + b: PropTypes.Array, + c: PropTypes.function, + d: PropTypes.objectof, + } + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] }] }); From 830fc3d4a05d341234b6c0954b91e7938ca2ff4f Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Mon, 12 Feb 2018 08:13:28 -0500 Subject: [PATCH 07/12] Fix lint errors --- lib/rules/no-typos.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-typos.js b/lib/rules/no-typos.js index b5ceb56a9c..8e2469f948 100644 --- a/lib/rules/no-typos.js +++ b/lib/rules/no-typos.js @@ -58,6 +58,10 @@ module.exports = { } function isPropTypesPackage(node) { + // Note: we really do want == with the package names here, + // since we need value equality, not identity - and + // these values are always string or null. + /* eslint-disable eqeqeq */ return ( node.type === 'Identifier' && node.name == propTypesPackageName @@ -66,8 +70,11 @@ module.exports = { node.property.name === 'PropTypes' && node.object.name == reactPackageName ); + /* eslint-enable eqeqeq */ } + /* eslint-disable no-use-before-define */ + function checkValidCallExpression(node) { const callee = node.callee; if (callee.type === 'MemberExpression' && callee.property.name === 'shape') { @@ -80,7 +87,6 @@ module.exports = { } } - /* eslint-disable no-use-before-define */ function checkValidProp(node) { if ((!propTypesPackageName && !reactPackageName) || !node) { return; @@ -107,6 +113,8 @@ module.exports = { } } + /* eslint-enable no-use-before-define */ + function checkValidPropObject (node) { if (node && node.type === 'ObjectExpression') { node.properties.forEach(prop => checkValidProp(prop.value)); From 0f480f63600d6e8f4aae04196e0672609eae7bed Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Mon, 12 Feb 2018 08:40:01 -0500 Subject: [PATCH 08/12] Add commented-out createClass tests These two tests fail on master as well as this branch, so they expose a pre-existing bug with the no-typos rule. --- tests/lib/rules/no-typos.js | 51 +++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/lib/rules/no-typos.js b/tests/lib/rules/no-typos.js index 8b1b1b55c5..3657c0d47a 100644 --- a/tests/lib/rules/no-typos.js +++ b/tests/lib/rules/no-typos.js @@ -1061,4 +1061,55 @@ ruleTester.run('no-typos', rule, { message: 'Typo in declared prop type: objectof' }] }] +/* +// createClass tests below fail, so they're commented out +// --------- + }, { + code: ` + import React from 'react'; + import PropTypes from 'prop-types'; + const Component = React.createClass({ + propTypes: { + a: PropTypes.string.isrequired, + b: PropTypes.shape({ + c: PropTypes.number + }).isrequired + } + }); + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: isrequired' + }, { + message: 'Typo in prop type chain qualifier: isrequired' + }] + }, { + code: ` + import React from 'react'; + import PropTypes from 'prop-types'; + const Component = React.createClass({ + childContextTypes: { + a: PropTypes.bools, + b: PropTypes.Array, + c: PropTypes.function, + d: PropTypes.objectof, + } + }); + `, + parser: 'babel-eslint', + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] + }] +// --------- +// createClass tests above fail, so they're commented out +*/ }); From 8cc1aa8fd4ae21f1a6cdc6d5c1985f4be15641ba Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Wed, 21 Feb 2018 09:43:12 -0500 Subject: [PATCH 09/12] Fix array-to-string coercion bug --- lib/rules/no-typos.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/rules/no-typos.js b/lib/rules/no-typos.js index 8e2469f948..2f6ffeb9ad 100644 --- a/lib/rules/no-typos.js +++ b/lib/rules/no-typos.js @@ -58,19 +58,14 @@ module.exports = { } function isPropTypesPackage(node) { - // Note: we really do want == with the package names here, - // since we need value equality, not identity - and - // these values are always string or null. - /* eslint-disable eqeqeq */ return ( node.type === 'Identifier' && - node.name == propTypesPackageName + node.name === propTypesPackageName ) || ( node.type === 'MemberExpression' && node.property.name === 'PropTypes' && - node.object.name == reactPackageName + node.object.name === reactPackageName ); - /* eslint-enable eqeqeq */ } /* eslint-disable no-use-before-define */ @@ -158,7 +153,7 @@ module.exports = { node.specifiers .filter(specifier => specifier.imported && specifier.imported.name === 'PropTypes') .map(specifier => specifier.local.name) - ) : null; + )[0] : null; } }, From 26c73dee833aeb261b64bb0c1698cedfada950e9 Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Wed, 21 Feb 2018 09:56:59 -0500 Subject: [PATCH 10/12] Convert value-return arrow function to explicit-return --- lib/rules/no-typos.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rules/no-typos.js b/lib/rules/no-typos.js index 2f6ffeb9ad..c20fb20869 100644 --- a/lib/rules/no-typos.js +++ b/lib/rules/no-typos.js @@ -77,7 +77,9 @@ module.exports = { } else if (callee.type === 'MemberExpression' && callee.property.name === 'oneOfType') { const args = node.arguments[0]; if (args && args.type === 'ArrayExpression') { - args.elements.forEach(el => checkValidProp(el)); + args.elements.forEach(el => { + checkValidProp(el); + }); } } } From 3ddfd1c53fd9b56b9d51a51e935a085c1f8cae58 Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Fri, 23 Feb 2018 08:05:56 -0500 Subject: [PATCH 11/12] Add default parser tests --- tests/lib/rules/no-typos.js | 254 ++++++++++++++++++++++++++++++++++++ 1 file changed, 254 insertions(+) diff --git a/tests/lib/rules/no-typos.js b/tests/lib/rules/no-typos.js index 3657c0d47a..a6d50c033d 100644 --- a/tests/lib/rules/no-typos.js +++ b/tests/lib/rules/no-typos.js @@ -351,6 +351,66 @@ ruleTester.run('no-typos', rule, { `, parser: 'babel-eslint', parserOptions: parserOptions + }, { + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.childContextTypes = { + a: PropTypes.string, + b: PropTypes.string.isRequired, + c: PropTypes.shape({ + d: PropTypes.string, + e: PropTypes.number.isRequired, + }).isRequired + } + `, + parserOptions: parserOptions + }, { + code: ` + import PropTypes from "prop-types"; + class Component extends React.Component {}; + Component.contextTypes = { + a: PropTypes.string, + b: PropTypes.string.isRequired, + c: PropTypes.shape({ + d: PropTypes.string, + e: PropTypes.number.isRequired, + }).isRequired + } + `, + parserOptions: parserOptions + }, { + code: ` + import PropTypes from 'prop-types' + import * as MyPropTypes from 'lib/my-prop-types' + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.string, + b: MyPropTypes.MYSTRING, + c: MyPropTypes.MYSTRING.isRequired, + } + `, + parserOptions: parserOptions + }, { + code: ` + import PropTypes from "prop-types" + import * as MyPropTypes from 'lib/my-prop-types' + class Component extends React.Component {}; + Component.propTypes = { + b: PropTypes.string, + a: MyPropTypes.MYSTRING, + } + `, + parserOptions: parserOptions + }, { + code: ` + import CustomReact from "react" + class Component extends React.Component {}; + Component.propTypes = { + b: CustomReact.PropTypes.string, + } + `, + parserOptions: parserOptions }, { code: ` import PropTypes from "prop-types"; @@ -1060,6 +1120,158 @@ ruleTester.run('no-typos', rule, { }, { message: 'Typo in declared prop type: objectof' }] + }, { + code: ` + import PropTypes from 'prop-types'; + class Component extends React.Component {}; + Component.childContextTypes = { + a: PropTypes.bools, + b: PropTypes.Array, + c: PropTypes.function, + d: PropTypes.objectof, + } + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] + }, { + code: ` + import PropTypes from 'prop-types'; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.string.isrequired, + b: PropTypes.shape({ + c: PropTypes.number + }).isrequired + } + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: isrequired' + }, { + message: 'Typo in prop type chain qualifier: isrequired' + }] + }, { + code: ` + import PropTypes from 'prop-types'; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.string.isrequired, + b: PropTypes.shape({ + c: PropTypes.number + }).isrequired + } + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: isrequired' + }, { + message: 'Typo in prop type chain qualifier: isrequired' + }] + }, { + code: ` + import RealPropTypes from 'prop-types'; + class Component extends React.Component {}; + Component.childContextTypes = { + a: RealPropTypes.bools, + b: RealPropTypes.Array, + c: RealPropTypes.function, + d: RealPropTypes.objectof, + } + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] + }, { + code: ` + import React from 'react'; + class Component extends React.Component {}; + Component.propTypes = { + a: React.PropTypes.string.isrequired, + b: React.PropTypes.shape({ + c: React.PropTypes.number + }).isrequired + } + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: isrequired' + }, { + message: 'Typo in prop type chain qualifier: isrequired' + }] + }, { + code: ` + import React from 'react'; + class Component extends React.Component {}; + Component.childContextTypes = { + a: React.PropTypes.bools, + b: React.PropTypes.Array, + c: React.PropTypes.function, + d: React.PropTypes.objectof, + } + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] + }, { + code: ` + import { PropTypes } from 'react'; + class Component extends React.Component {}; + Component.propTypes = { + a: PropTypes.string.isrequired, + b: PropTypes.shape({ + c: PropTypes.number + }).isrequired + } + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: isrequired' + }, { + message: 'Typo in prop type chain qualifier: isrequired' + }] + }, { + code: ` + import { PropTypes } from 'react'; + class Component extends React.Component {}; + Component.childContextTypes = { + a: PropTypes.bools, + b: PropTypes.Array, + c: PropTypes.function, + d: PropTypes.objectof, + } + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] }] /* // createClass tests below fail, so they're commented out @@ -1108,6 +1320,48 @@ ruleTester.run('no-typos', rule, { }, { message: 'Typo in declared prop type: objectof' }] + }, { + code: ` + import React from 'react'; + import PropTypes from 'prop-types'; + const Component = React.createClass({ + propTypes: { + a: PropTypes.string.isrequired, + b: PropTypes.shape({ + c: PropTypes.number + }).isrequired + } + }); + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in prop type chain qualifier: isrequired' + }, { + message: 'Typo in prop type chain qualifier: isrequired' + }] + }, { + code: ` + import React from 'react'; + import PropTypes from 'prop-types'; + const Component = React.createClass({ + childContextTypes: { + a: PropTypes.bools, + b: PropTypes.Array, + c: PropTypes.function, + d: PropTypes.objectof, + } + }); + `, + parserOptions: parserOptions, + errors: [{ + message: 'Typo in declared prop type: bools' + }, { + message: 'Typo in declared prop type: Array' + }, { + message: 'Typo in declared prop type: function' + }, { + message: 'Typo in declared prop type: objectof' + }] }] // --------- // createClass tests above fail, so they're commented out From fc06f852b977f9bff23ae163270f555de136c9de Mon Sep 17 00:00:00 2001 From: Brett Higgins Date: Mon, 5 Mar 2018 09:00:51 -0500 Subject: [PATCH 12/12] Use find instead of filter --- lib/rules/no-typos.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/rules/no-typos.js b/lib/rules/no-typos.js index c20fb20869..1e350113bb 100644 --- a/lib/rules/no-typos.js +++ b/lib/rules/no-typos.js @@ -151,11 +151,14 @@ module.exports = { } else if (node.source && node.source.value === 'react') { // import { PropTypes } from "react" reactPackageName = node.specifiers[0].local.name; - propTypesPackageName = node.specifiers.length >= 1 ? ( - node.specifiers - .filter(specifier => specifier.imported && specifier.imported.name === 'PropTypes') - .map(specifier => specifier.local.name) - )[0] : null; + if (node.specifiers.length >= 1) { + const propTypesSpecifier = node.specifiers.find(specifier => ( + specifier.imported && specifier.imported.name === 'PropTypes' + )); + if (propTypesSpecifier) { + propTypesPackageName = propTypesSpecifier.local.name; + } + } } },