From ddd1f66455269b38534f2d75d91a01ecd4b6d4c0 Mon Sep 17 00:00:00 2001 From: Michael Ferris Date: Sun, 14 Jun 2015 22:51:26 -0400 Subject: [PATCH 01/11] Support nested prop types and use react propTypes to make further analysis. Minimal analyse of primitive propTypes. --- lib/rules/prop-types.js | 275 +++++++++++++++++++++++++++-- tests/lib/rules/prop-types.js | 323 ++++++++++++++++++++++++++++++++++ 2 files changed, 579 insertions(+), 19 deletions(-) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 421ff96cd9..28eb31015a 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -83,16 +83,74 @@ module.exports = function(context) { ); } + /** + * Internal: Checks if the prop is declared + * @param {Object} declaredPropTypes Description of propTypes declared in the current component + * @param {String[]} keyList Dot separated name of the prop to check. + * @returns {Boolean} True if the prop is declared, false if not. + */ + function _isDeclaredInComponent(declaredPropTypes, keyList) { + for (var i = 0, j = keyList.length; i < j; i++) { + var key = keyList[i]; + var propType = ( + // Check if this key is declared + declaredPropTypes[key] || + // If not, check if this type accepts any key + declaredPropTypes.__ANY_KEY__ + ); + + if (!propType) { + // If it's a computed property, we can't make any further analysis, but is valid + return key === '__COMPUTED_PROP__'; + } + if (propType === true) { + return true; + } + // Consider every children as declared + if (propType.children === true) { + return true; + } + if (propType.acceptedProperties) { + return key in propType.acceptedProperties; + } + if (propType.type === 'union') { + // If we fall in this case, we know there is at least one complex type in the union + if (i + 1 >= j) { + // this is the last key, accept everything + return true; + } + // non trivial, check all of them + var unionTypes = propType.children; + var unionPropType = {}; + for (var k = 0, z = unionTypes.length; k < z; k++) { + unionPropType[key] = unionTypes[k]; + var isValid = _isDeclaredInComponent( + unionPropType, + keyList.slice(i) + ); + if (isValid) { + return true; + } + } + + // every possible union were invalid + return false; + } + declaredPropTypes = propType.children; + } + return true; + } + /** * Checks if the prop is declared - * @param {String} name Name of the prop to check. * @param {Object} component The component to process + * @param {String} name Dot separated name of the prop to check. * @returns {Boolean} True if the prop is declared, false if not. */ function isDeclaredInComponent(component, name) { - return ( - component.declaredPropTypes && - component.declaredPropTypes.indexOf(name) !== -1 + return _isDeclaredInComponent( + component.declaredPropTypes || {}, + name.split('.') ); } @@ -106,15 +164,169 @@ module.exports = function(context) { return tokens.length && tokens[0].value === '...'; } + /** + * Iterates through a properties node, like a customized forEach. + * @param {Object[]} properties Array of properties to iterate. + * @param {Function} fn Function to call on each property, receives property key + and property value. (key, value) => void + */ + function iterateProperties(properties, fn) { + if (properties.length && typeof fn === 'function') { + for (var i = 0, j = properties.length; i < j; i++) { + var node = properties[i]; + var key = node.key; + var keyName = key.type === 'Identifier' ? key.name : key.value; + + var value = node.value; + fn(keyName, value); + } + } + } + + /** + * Creates the representation of the React propTypes for the component. + * The representation is used to verify nested used properties. + * @param {ASTNode} value Node of the React.PropTypes for the desired propery + * @return {Object|Boolean} The representation of the declaration, true means + * the property is declared without the need for further analysis. + */ + function buildReactDeclarationTypes(value) { + if ( + value.type === 'MemberExpression' && + value.property && + value.property.name && + value.property.name === 'isRequired' + ) { + value = value.object; + } + + // Verify React.PropTypes that are functions + if ( + value.type === 'CallExpression' && + value.callee && + value.callee.property && + value.callee.property.name && + value.arguments && + value.arguments.length > 0 + ) { + var callName = value.callee.property.name; + var argument = value.arguments[0]; + switch (callName) { + case 'shape': + if (argument.type !== 'ObjectExpression') { + // Invalid proptype or cannot analyse statically + return true; + } + var shapeTypeDefinition = { + type: 'shape', + children: {} + }; + iterateProperties(argument.properties, function(childKey, childValue) { + shapeTypeDefinition.children[childKey] = buildReactDeclarationTypes(childValue); + }); + return shapeTypeDefinition; + case 'arrayOf': + return { + type: 'array', + children: { + // Accept only array prototype and computed properties + __ANY_KEY__: { + acceptedProperties: Array.prototype + }, + __COMPUTED_PROP__: buildReactDeclarationTypes(argument) + } + }; + case 'objectOf': + return { + type: 'object', + children: { + __ANY_KEY__: buildReactDeclarationTypes(argument) + } + }; + case 'oneOfType': + if ( + !argument.elements || + !argument.elements.length + ) { + // Invalid proptype or cannot analyse statically + return true; + } + var unionTypeDefinition = { + type: 'union', + children: [] + }; + for (var i = 0, j = argument.elements.length; i < j; i++) { + var type = buildReactDeclarationTypes(argument.elements[i]); + // keep only complex type + if (type !== true) { + if (type.children === true) { + // every child is accepted for one type, abort type analysis + unionTypeDefinition.children = true; + return unionTypeDefinition; + } + unionTypeDefinition.children.push(type); + } + } + if (unionTypeDefinition.length === 0) { + // no complex type found, simply accept everything + return true; + } + return unionTypeDefinition; + case 'instanceOf': + return { + type: 'instance', + // Accept all children because we can't know what type they are + children: true + }; + case 'oneOf': + default: + return true; + } + } + if ( + value.type === 'MemberExpression' && + value.property && + value.property.name + ) { + var name = value.property.name; + // React propTypes with limited possible properties + var propertiesMap = { + array: Array.prototype, + bool: Boolean.prototype, + func: Function.prototype, + number: Number.prototype, + string: String.prototype + }; + if (name in propertiesMap) { + return { + type: name, + children: { + __ANY_KEY__: { + acceptedProperties: propertiesMap[name] + } + } + }; + } + } + // Unknown property or accepts everything (any, object, ...) + return true; + } + /** * Mark a prop type as used * @param {ASTNode} node The AST node being marked. */ - function markPropTypesAsUsed(node) { - var component = componentList.getByNode(context, node); - var usedPropTypes = component && component.usedPropTypes || []; + function markPropTypesAsUsed(node, parentName) { var type; - if (node.parent.property && node.parent.property.name && !node.parent.computed) { + var name = node.parent.computed ? + '__COMPUTED_PROP__' + : node.parent.property && node.parent.property.name; + var fullName = parentName ? parentName + '.' + name : name; + + if (node.parent.type === 'MemberExpression') { + markPropTypesAsUsed(node.parent, fullName); + } + if (name && !node.parent.computed) { type = 'direct'; } else if ( node.parent.parent.declarations && @@ -123,15 +335,17 @@ module.exports = function(context) { ) { type = 'destructuring'; } + var component = componentList.getByNode(context, node); + var usedPropTypes = component && component.usedPropTypes || []; switch (type) { case 'direct': // Ignore Object methods - if (Object.prototype[node.parent.property.name]) { + if (Object.prototype[name]) { break; } usedPropTypes.push({ - name: node.parent.property.name, + name: fullName, node: node.parent.property }); break; @@ -163,18 +377,39 @@ module.exports = function(context) { */ function markPropTypesAsDeclared(node, propTypes) { var component = componentList.getByNode(context, node); - var declaredPropTypes = component && component.declaredPropTypes || []; + var declaredPropTypes = component && component.declaredPropTypes || {}; var ignorePropsValidation = false; switch (propTypes && propTypes.type) { case 'ObjectExpression': - for (var i = 0, j = propTypes.properties.length; i < j; i++) { - var key = propTypes.properties[i].key; - declaredPropTypes.push(key.type === 'Identifier' ? key.name : key.value); - } + iterateProperties(propTypes.properties, function(key, value) { + declaredPropTypes[key] = buildReactDeclarationTypes(value); + }); break; case 'MemberExpression': - declaredPropTypes.push(propTypes.property.name); + var curDeclaredPropTypes = declaredPropTypes; + // Walk the list of properties, until we reach the assignment + // ie: ClassX.propTypes.a.b.c = ... + while ( + propTypes && + propTypes.parent.type !== 'AssignmentExpression' && + propTypes.property && + curDeclaredPropTypes + ) { + var propName = propTypes.property.name; + if (propName in curDeclaredPropTypes) { + curDeclaredPropTypes = curDeclaredPropTypes[propName].children; + propTypes = propTypes.parent; + } else { + // This will crash at runtime because we haven't seen this key before + // stop this and do not declare it + propTypes = null; + } + } + if (propTypes) { + curDeclaredPropTypes[propTypes.property.name] = + buildReactDeclarationTypes(propTypes.parent.right); + } break; case null: break; @@ -187,7 +422,6 @@ module.exports = function(context) { declaredPropTypes: declaredPropTypes, ignorePropsValidation: ignorePropsValidation }); - } /** @@ -198,13 +432,16 @@ module.exports = function(context) { var name; for (var i = 0, j = component.usedPropTypes.length; i < j; i++) { name = component.usedPropTypes[i].name; - if (isDeclaredInComponent(component, name) || isIgnored(name)) { + if ( + isIgnored(name.split('.').pop()) || + isDeclaredInComponent(component, name) + ) { continue; } context.report( component.usedPropTypes[i].node, component.name === componentUtil.DEFAULT_COMPONENT_NAME ? MISSING_MESSAGE : MISSING_MESSAGE_NAMED_COMP, { - name: name, + name: name.replace(/\.__COMPUTED_PROP__/g, '[]'), component: component.name } ); diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index f51fdaf311..ba11d4aeab 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -253,6 +253,168 @@ eslintTester.addRuleTest('lib/rules/prop-types', { classes: true, jsx: true } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {};', + 'Hello.propTypes.a = React.PropTypes.shape({', + ' b: React.PropTypes.string', + '});' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b.c;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.shape({', + ' b: React.PropTypes.shape({', + ' })', + ' })', + '};', + 'Hello.propTypes.a.b.c = React.PropTypes.number;' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b.c;', + ' this.props.a.__.d.length;', + ' this.props.a.anything.e[2];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.objectOf(', + ' React.PropTypes.shape({', + ' c: React.PropTypes.number,', + ' d: React.PropTypes.string,', + ' e: React.PropTypes.array', + ' })', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' var i = 3;', + ' this.props.a[2].c;', + ' this.props.a[i].d.length;', + ' this.props.a[i + 2].e[2];', + ' this.props.a.length;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.arrayOf(', + ' React.PropTypes.shape({', + ' c: React.PropTypes.number,', + ' d: React.PropTypes.string,', + ' e: React.PropTypes.array', + ' })', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.c;', + ' this.props.a[2] === true;', + ' this.props.a.e[2];', + ' this.props.a.length;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.oneOfType([', + ' React.PropTypes.shape({', + ' c: React.PropTypes.number,', + ' e: React.PropTypes.array', + ' }).isRequired,', + ' React.PropTypes.arrayOf(', + ' React.PropTypes.bool', + ' )', + ' ])', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.render;', + ' this.props.a.c;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.instanceOf(Hello)', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.arr;', + ' this.props.arr[3];', + ' this.props.arr.length;', + ' this.props.arr.push(3);', + ' this.props.bo;', + ' this.props.bo.toString();', + ' this.props.fu;', + ' this.props.fu.bind(this);', + ' this.props.numb;', + ' this.props.numb.toFixed();', + ' this.props.stri;', + ' this.props.stri.length();', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' arr: React.PropTypes.array,', + ' bo: React.PropTypes.bool.isRequired,', + ' fu: React.PropTypes.func,', + ' numb: React.PropTypes.number,', + ' stri: React.PropTypes.string', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } } ], @@ -396,6 +558,167 @@ eslintTester.addRuleTest('lib/rules/prop-types', { errors: [{ message: '\'firstname\' is missing in props validation for Hello' }] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.shape({', + ' })', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [{ + message: '\'a.b\' is missing in props validation for Hello' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b.c;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.shape({', + ' b: React.PropTypes.shape({', + ' })', + ' })', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [{ + message: '\'a.b.c\' is missing in props validation for Hello' + }] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.b.c;', + ' this.props.a.__.d.length;', + ' this.props.a.anything.e[2];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.objectOf(', + ' React.PropTypes.shape({', + ' })', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'a.b.c\' is missing in props validation for Hello'}, + {message: '\'a.__.d\' is missing in props validation for Hello'}, + {message: '\'a.__.d.length\' is missing in props validation for Hello'}, + {message: '\'a.anything.e\' is missing in props validation for Hello'} + ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' var i = 3;', + ' this.props.a[2].c;', + ' this.props.a[i].d.length;', + ' this.props.a[i + 2].e[2];', + ' this.props.a.length;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.arrayOf(', + ' React.PropTypes.shape({', + ' })', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'a[].c\' is missing in props validation for Hello'}, + {message: '\'a[].d\' is missing in props validation for Hello'}, + {message: '\'a[].d.length\' is missing in props validation for Hello'}, + {message: '\'a[].e\' is missing in props validation for Hello'} + ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.a.length;', + ' this.props.a.b;', + ' this.props.a.e.length;', + ' this.props.a.e.anyProp;', + ' this.props.a.c.toString();', + ' this.props.a.c.someThingElse();', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' a: React.PropTypes.oneOfType([', + ' React.PropTypes.shape({', + ' c: React.PropTypes.number,', + ' e: React.PropTypes.array', + ' })', + ' ])', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'a.length\' is missing in props validation for Hello'}, + {message: '\'a.b\' is missing in props validation for Hello'}, + {message: '\'a.e.anyProp\' is missing in props validation for Hello'}, + {message: '\'a.c.someThingElse\' is missing in props validation for Hello'} + ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props.arr.toFixed();', + ' this.props.bo.push();', + ' this.props.fu.push();', + ' this.props.numb.propX;', + ' this.props.stri.tooString();', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' arr: React.PropTypes.array,', + ' bo: React.PropTypes.bool,', + ' fu: React.PropTypes.func,', + ' numb: React.PropTypes.number,', + ' stri: React.PropTypes.string', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'arr.toFixed\' is missing in props validation for Hello'}, + {message: '\'bo.push\' is missing in props validation for Hello'}, + {message: '\'fu.push\' is missing in props validation for Hello'}, + {message: '\'numb.propX\' is missing in props validation for Hello'}, + {message: '\'stri.tooString\' is missing in props validation for Hello'} + ] } ] }); From 35bdea1d89ee2a7f93c280183b2888f2c0e6628d Mon Sep 17 00:00:00 2001 From: Michael Ferris Date: Mon, 15 Jun 2015 22:43:54 -0400 Subject: [PATCH 02/11] Point directly to mocha instead of symlink in .bin. On Windows the file in .bin is not a symlink. Inspired by solution in eslint --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index fae9b10475..6d0147c967 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "scripts": { "test": "npm run lint && npm run unit-test", "lint": "eslint ./", - "unit-test": "istanbul cover --dir reports/coverage _mocha tests/**/*.js -- --reporter dot", + "unit-test": "istanbul cover --dir reports/coverage node_modules/mocha/bin/_mocha tests/**/*.js -- --reporter dot", "coveralls": "cat ./reports/coverage/lcov.info | coveralls" }, "files": [ From 73f565d78b6e9aacd88364ba11347b48395dfc57 Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Wed, 17 Jun 2015 14:12:16 -0700 Subject: [PATCH 03/11] Allow blocking of '.jsx' extensions with `require()` Created a new rule, 'require-extension', which inspects the required module ID and warns if the file extension is .jsx. The particular disallowed file extensions are configurable, defaulting to just '.jsx'. If you wanted to also disallow '.js', the rule can be configured as: "rules": { "react/require-extension": [1, { "extensions": [".js", ".jsx"] }], } Note: this rule does not prevent required files from containing forbidden extensions, it merely prevents the extension from being included in the `require()` statement. This rule is inspired by the Airbnb react style guide: https://github.com/airbnb/javascript/tree/master/react#naming --- docs/rules/require-extenion.md | 38 +++++++++++ index.js | 6 +- lib/rules/require-extension.js | 72 +++++++++++++++++++++ tests/lib/rules/require-extension.js | 95 ++++++++++++++++++++++++++++ 4 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 docs/rules/require-extenion.md create mode 100644 lib/rules/require-extension.js create mode 100644 tests/lib/rules/require-extension.js diff --git a/docs/rules/require-extenion.md b/docs/rules/require-extenion.md new file mode 100644 index 0000000000..37c8fb2ded --- /dev/null +++ b/docs/rules/require-extenion.md @@ -0,0 +1,38 @@ +# Restrict file extensions that may be required (require-extension) + +`require()` statements should generally not include a file extension as there is a well defined mechanism for resolving a module ID to a specific file. This rule inspects the module ID being required and creates a warning if the ID contains a '.jsx' file extension. + +Note: this rule does not prevent required files from containing these extensions, it merely prevents the extension from being included in the `require()` statement. + +## Rule Details + +The following patterns are considered warnings: + +```js +var index = require('./index.jsx'); + +// When [1, {extensions: ['.js']}] +var index = require('./index.js'); +``` + +The following patterns are not considered warnings: + +```js +var index = require('./index'); + +var eslint = require('eslint'); +``` + +## Rule Options + +The set of forbidden extensions is configurable. By default '.jsx' is blocked. If you wanted to forbid both '.jsx' and '.js', the configuration would be: + +```js +"rules": { + "react/require-extension": [1, { "extensions": [".js", ".jsx"] }], +} +``` + +## When Not To Use It + +If you have file in your project with a '.jsx' file extension and do not have `require()` configured to automatically resolve '.jsx' files. diff --git a/index.js b/index.js index c467477b85..71ee48e8b8 100644 --- a/index.js +++ b/index.js @@ -18,7 +18,8 @@ module.exports = { 'jsx-sort-props': require('./lib/rules/jsx-sort-props'), 'jsx-sort-prop-types': require('./lib/rules/jsx-sort-prop-types'), 'jsx-boolean-value': require('./lib/rules/jsx-boolean-value'), - 'sort-comp': require('./lib/rules/sort-comp') + 'sort-comp': require('./lib/rules/sort-comp'), + 'require-extension': require('./lib/rules/require-extension') }, rulesConfig: { 'jsx-uses-react': 0, @@ -37,6 +38,7 @@ module.exports = { 'jsx-sort-props': 0, 'jsx-sort-prop-types': 0, 'jsx-boolean-value': 0, - 'sort-comp': 0 + 'sort-comp': 0, + 'require-extension': 0 } }; diff --git a/lib/rules/require-extension.js b/lib/rules/require-extension.js new file mode 100644 index 0000000000..2a5f7c673c --- /dev/null +++ b/lib/rules/require-extension.js @@ -0,0 +1,72 @@ +/** + * @fileoverview Restrict file extensions that may be required + * @author Scott Andrews + */ +'use strict'; + +var path = require('path'); + +// ------------------------------------------------------------------------------ +// Constants +// ------------------------------------------------------------------------------ + +var DEFAULTS = { + extentions: ['.jsx'] +}; + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +module.exports = function(context) { + + function isRequire(expression) { + return expression.callee.name === 'require'; + } + + function getId(expression) { + return expression.arguments[0] && expression.arguments[0].value; + } + + function getExtension(id) { + return path.extname(id || ''); + } + + function getExtentionsConfig() { + return context.options[0] && context.options[0].extensions || DEFAULTS.extentions; + } + + var forbiddenExtensions = getExtentionsConfig().reduce(function (extensions, extension) { + extensions[extension] = true; + return extensions; + }, Object.create(null)); + + function isForbiddenExtension(ext) { + return ext in forbiddenExtensions; + } + + // -------------------------------------------------------------------------- + // Public + // -------------------------------------------------------------------------- + + return { + + CallExpression: function(node) { + if (isRequire(node)) { + var ext = getExtension(getId(node)); + if (isForbiddenExtension(ext)) { + context.report(node, 'Unable to require module with extension \'' + ext + '\''); + } + } + } + + }; + +}; + +module.exports.schema = [{ + type: 'object', + properties: { + extensions: {type: 'array'} + } +}]; diff --git a/tests/lib/rules/require-extension.js b/tests/lib/rules/require-extension.js new file mode 100644 index 0000000000..3e48542a19 --- /dev/null +++ b/tests/lib/rules/require-extension.js @@ -0,0 +1,95 @@ +/** + * @fileoverview Restrict file extensions that may be required + * @author Scott Andrews + */ +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +var eslint = require('eslint').linter; +var ESLintTester = require('eslint-tester'); + +// ------------------------------------------------------------------------------ +// Code Snippets +// ------------------------------------------------------------------------------ + +var REQUIRE_PACKAGE = 'require(\'eslint\')'; + +var REQUIRE_JS = 'require(\'./index.js\')'; + +var REQUIRE_JSX = 'require(\'./index.jsx\')'; + +var REQUIRE_JSON = 'require(\'./index.json\')'; + +var REQUIRE_EMPTY = 'require()'; + +var REQUIRE_OBJECT = 'require({})'; + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +var eslintTester = new ESLintTester(eslint); +eslintTester.addRuleTest('lib/rules/require-extension', { + + valid: [ + { + code: REQUIRE_PACKAGE + }, { + code: REQUIRE_JS + }, { + code: REQUIRE_JSON + }, { + code: REQUIRE_EMPTY + }, { + code: REQUIRE_OBJECT + }, { + code: REQUIRE_PACKAGE, + args: [1] + }, { + code: REQUIRE_JS, + args: [1] + }, { + code: REQUIRE_JSON, + args: [1] + }, { + code: REQUIRE_EMPTY, + args: [1] + }, { + code: REQUIRE_OBJECT, + args: [1] + }, { + code: REQUIRE_JSON, + args: [1, {extensions: ['.js']}] + }, { + code: REQUIRE_JSX, + args: [1, {extensions: ['.js']}] + } + ], + + invalid: [ + { + code: REQUIRE_JSX, + errors: [{message: 'Unable to require module with extension \'.jsx\''}] + }, { + code: REQUIRE_JSX, + args: [1], + errors: [{message: 'Unable to require module with extension \'.jsx\''}] + }, { + code: REQUIRE_JS, + args: [1, {extensions: ['.js']}], + errors: [{message: 'Unable to require module with extension \'.js\''}] + }, { + code: REQUIRE_JS, + args: [1, {extensions: ['.js', '.jsx']}], + errors: [{message: 'Unable to require module with extension \'.js\''}] + }, { + code: REQUIRE_JSX, + args: [1, {extensions: ['.js', '.jsx']}], + errors: [{message: 'Unable to require module with extension \'.jsx\''}] + } + ] + +}); From 4a292d16085f5570fceab220f73a0dad4de7eb59 Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Thu, 18 Jun 2015 01:50:36 +0200 Subject: [PATCH 04/11] fix detect missing displayName in React class when ecmaFeatures.jsx is false --- lib/rules/display-name.js | 6 ++++++ tests/lib/rules/display-name.js | 14 ++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index 811e7ea10c..e6dfb4a1f6 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -117,6 +117,12 @@ module.exports = function(context) { } markDisplayNameAsDeclared(node); }); + + if (componentUtil.isComponentDefinition(node)) { + componentList.set(context, node, { + isReactComponent: true + }); + } }, 'Program:exit': function() { diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index b87a203f29..d64f3dbabf 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -89,6 +89,20 @@ eslintTester.addRuleTest('lib/rules/display-name', { }], invalid: [{ + code: [ + 'var Hello = React.createClass({', + ' render: function() {', + ' return React.createElement("div", {}, "text content");', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: false + }, + errors: [{ + message: 'Component definition is missing display name' + }] + }, { code: [ 'var Hello = React.createClass({', ' render: function() {', From 8626f722b9a79bbce188539e31fbb77c99b4ff0b Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Thu, 18 Jun 2015 02:10:23 +0200 Subject: [PATCH 05/11] fix detection of missing propTypes validations on ecmaFeatures.jsx false --- lib/rules/prop-types.js | 6 ++++++ tests/lib/rules/prop-types.js | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 28eb31015a..776fe75c9b 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -509,6 +509,12 @@ module.exports = function(context) { } markPropTypesAsDeclared(node, property.value); }); + + if (componentUtil.isComponentDefinition(node)) { + componentList.set(context, node, { + isReactComponent: true + }); + } }, 'Program:exit': function() { diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index ba11d4aeab..48b40ffbe9 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -420,6 +420,23 @@ eslintTester.addRuleTest('lib/rules/prop-types', { invalid: [ { + code: [ + 'var Hello = React.createClass({', + ' render: function() {', + ' return React.createElement("div", {}, this.props.name);', + ' }', + '});' + ].join('\n'), + ecmaFeatures: { + jsx: false + }, + errors: [{ + message: '\'name\' is missing in props validation', + line: 3, + column: 53, + type: 'Identifier' + }] + }, { code: [ 'var Hello = React.createClass({', ' render: function() {', From 1cd3686e927ade0a3b83995733ef1737e2d512a2 Mon Sep 17 00:00:00 2001 From: Michael Ferris Date: Fri, 19 Jun 2015 02:35:57 -0400 Subject: [PATCH 06/11] Added support for prop type identified by strings in rule `jsx-sort-prop-types` --- lib/rules/jsx-sort-prop-types.js | 8 ++++++-- tests/lib/rules/jsx-sort-prop-types.js | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/rules/jsx-sort-prop-types.js b/lib/rules/jsx-sort-prop-types.js index bce91a4dbd..be1eb0a614 100644 --- a/lib/rules/jsx-sort-prop-types.js +++ b/lib/rules/jsx-sort-prop-types.js @@ -35,6 +35,10 @@ module.exports = function(context) { ); } + function getKey(node) { + return node.key.type === 'Identifier' ? node.key.name : node.key.value; + } + /** * Checks if propTypes declarations are sorted * @param {Array} declarations The array of AST nodes being checked. @@ -42,8 +46,8 @@ module.exports = function(context) { */ function checkSorted(declarations) { declarations.reduce(function(prev, curr) { - var prevPropName = prev.key.name; - var currenPropName = curr.key.name; + var prevPropName = getKey(prev); + var currenPropName = getKey(curr); if (ignoreCase) { prevPropName = prevPropName.toLowerCase(); diff --git a/tests/lib/rules/jsx-sort-prop-types.js b/tests/lib/rules/jsx-sort-prop-types.js index 19ca8ed3ba..1f3f2f4bb4 100644 --- a/tests/lib/rules/jsx-sort-prop-types.js +++ b/tests/lib/rules/jsx-sort-prop-types.js @@ -161,6 +161,21 @@ eslintTester.addRuleTest('lib/rules/jsx-sort-prop-types', { classes: true, jsx: true } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' "aria-controls": React.PropTypes.string', + '};' + ].join('\n'), + parser: 'babel-eslint', + args: [1, { + ignoreCase: true + }] } ], From c73d9b8af01321776c87684ecdf181ac9692f048 Mon Sep 17 00:00:00 2001 From: Michael Ferris Date: Fri, 19 Jun 2015 03:06:28 -0400 Subject: [PATCH 07/11] Fix `prop-types` desctructuring with properties as string. --- lib/rules/prop-types.js | 28 +++++++++++++++++++-------- tests/lib/rules/prop-types.js | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 28eb31015a..6c83679f87 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -164,6 +164,16 @@ module.exports = function(context) { return tokens.length && tokens[0].value === '...'; } + function getKeyValue(node) { + var key = node.key; + if (key) { + if (key.type === 'Identifier') { + return key.name; + } + return key.value; + } + } + /** * Iterates through a properties node, like a customized forEach. * @param {Object[]} properties Array of properties to iterate. @@ -174,11 +184,10 @@ module.exports = function(context) { if (properties.length && typeof fn === 'function') { for (var i = 0, j = properties.length; i < j; i++) { var node = properties[i]; - var key = node.key; - var keyName = key.type === 'Identifier' ? key.name : key.value; + var key = getKeyValue(node); var value = node.value; - fn(keyName, value); + fn(key, value); } } } @@ -331,7 +340,7 @@ module.exports = function(context) { } else if ( node.parent.parent.declarations && node.parent.parent.declarations[0].id.properties && - node.parent.parent.declarations[0].id.properties[0].key.name + getKeyValue(node.parent.parent.declarations[0].id.properties[0]) ) { type = 'destructuring'; } @@ -355,10 +364,13 @@ module.exports = function(context) { if (hasSpreadOperator(properties[i])) { continue; } - usedPropTypes.push({ - name: properties[i].key.name, - node: properties[i] - }); + var propName = getKeyValue(properties[i]); + if (propName) { + usedPropTypes.push({ + name: propName, + node: properties[i] + }); + } } break; default: diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index ba11d4aeab..c444de0327 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -415,6 +415,23 @@ eslintTester.addRuleTest('lib/rules/prop-types', { classes: true, jsx: true } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' var { ', + ' propX,', + ' "aria-controls": ariaControls, ', + ' ...props } = this.props;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' "propX": React.PropTypes.string,', + ' "aria-controls": React.PropTypes.string', + '};' + ].join('\n'), + parser: 'babel-eslint' } ], @@ -719,6 +736,25 @@ eslintTester.addRuleTest('lib/rules/prop-types', { {message: '\'numb.propX\' is missing in props validation for Hello'}, {message: '\'stri.tooString\' is missing in props validation for Hello'} ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' var { ', + ' "aria-controls": ariaControls, ', + ' propX,', + ' ...props } = this.props;', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' "aria-controls": React.PropTypes.string', + '};' + ].join('\n'), + parser: 'babel-eslint', + errors: [ + {message: '\'propX\' is missing in props validation for Hello'} + ] } ] }); From 228548fb0cb469cfcc2fd73793e6c4235da2ce19 Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Fri, 19 Jun 2015 22:43:05 +0000 Subject: [PATCH 08/11] Fix crash if a ClassProperty has only one token (fixes #125) --- lib/rules/display-name.js | 5 ++++- lib/rules/prop-types.js | 5 ++++- tests/lib/rules/display-name.js | 11 +++++++++++ tests/lib/rules/prop-types.js | 11 +++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index e6dfb4a1f6..839b38b278 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -42,7 +42,10 @@ module.exports = function(context) { // (babel-eslint does not expose property name so we have to rely on tokens) if (node.type === 'ClassProperty') { var tokens = context.getFirstTokens(node, 2); - if (tokens[0].value === 'displayName' || tokens[1].value === 'displayName') { + if ( + tokens[0].value === 'displayName' || + (tokens[1] && tokens[1].value === 'displayName') + ) { return true; } return false; diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 426f64e2dc..a9888cf4b5 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -47,7 +47,10 @@ module.exports = function(context) { // (babel-eslint does not expose property name so we have to rely on tokens) if (node.type === 'ClassProperty') { var tokens = context.getFirstTokens(node, 2); - if (tokens[0].value === 'propTypes' || tokens[1].value === 'propTypes') { + if ( + tokens[0].value === 'propTypes' || + (tokens[1] && tokens[1].value === 'propTypes') + ) { return true; } return false; diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index d64f3dbabf..22fd58229c 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -57,6 +57,17 @@ eslintTester.addRuleTest('lib/rules/display-name', { classes: true, jsx: true } + }, { + code: [ + 'class Hello {', + ' method', + '}' + ].join('\n'), + parser: 'babel-eslint', + ecmaFeatures: { + classes: true, + jsx: true + } }, { code: [ 'class Hello extends React.Component {', diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index b3dce48e51..a901f3fda0 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -167,6 +167,17 @@ eslintTester.addRuleTest('lib/rules/prop-types', { classes: true, jsx: true } + }, { + code: [ + 'class Hello {', + ' method', + '}' + ].join('\n'), + parser: 'babel-eslint', + ecmaFeatures: { + classes: true, + jsx: true + } }, { code: [ 'class Hello extends React.Component {', From 144d45c854e484dabdb5783b0c408c7a4eb54909 Mon Sep 17 00:00:00 2001 From: Michael Ferris Date: Fri, 19 Jun 2015 18:45:55 -0400 Subject: [PATCH 09/11] Added support for prop-types check of type `this.props['this-format']`. Used list of nested names instead of dot-separated names to avoid conflict with properties like `this.props['this.format']` --- lib/rules/prop-types.js | 82 ++++++++++++++++++-------- tests/lib/rules/prop-types.js | 107 ++++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 25 deletions(-) diff --git a/lib/rules/prop-types.js b/lib/rules/prop-types.js index 426f64e2dc..ec81ea6ea3 100644 --- a/lib/rules/prop-types.js +++ b/lib/rules/prop-types.js @@ -144,13 +144,13 @@ module.exports = function(context) { /** * Checks if the prop is declared * @param {Object} component The component to process - * @param {String} name Dot separated name of the prop to check. + * @param {String[]} names List of names of the prop to check. * @returns {Boolean} True if the prop is declared, false if not. */ - function isDeclaredInComponent(component, name) { + function isDeclaredInComponent(component, names) { return _isDeclaredInComponent( component.declaredPropTypes || {}, - name.split('.') + names ); } @@ -164,14 +164,14 @@ module.exports = function(context) { return tokens.length && tokens[0].value === '...'; } + /** + * Retrieve the name of a key node + * @param {ASTNode} node The AST node with the key. + * @return {string} the name of the key + */ function getKeyValue(node) { var key = node.key; - if (key) { - if (key.type === 'Identifier') { - return key.name; - } - return key.value; - } + return key.type === 'Identifier' ? key.name : key.value; } /** @@ -321,22 +321,51 @@ module.exports = function(context) { return true; } + /** + * Retrieve the name of a property node + * @param {ASTNode} node The AST node with the property. + * @return {string} the name of the property or undefined if not found + */ + function getPropertyName(node) { + var property = node.property; + if (property) { + switch (property.type) { + case 'Identifier': + if (node.computed) { + return '__COMPUTED_PROP__'; + } + return property.name; + case 'Literal': + // Accept computed properties that are literal strings + if (typeof property.value === 'string') { + return property.value; + } + // falls through + default: + if (node.computed) { + return '__COMPUTED_PROP__'; + } + break; + } + } + } + /** * Mark a prop type as used * @param {ASTNode} node The AST node being marked. */ - function markPropTypesAsUsed(node, parentName) { + function markPropTypesAsUsed(node, parentNames) { + parentNames = parentNames || []; var type; - var name = node.parent.computed ? - '__COMPUTED_PROP__' - : node.parent.property && node.parent.property.name; - var fullName = parentName ? parentName + '.' + name : name; - - if (node.parent.type === 'MemberExpression') { - markPropTypesAsUsed(node.parent, fullName); - } - if (name && !node.parent.computed) { - type = 'direct'; + var name = getPropertyName(node.parent); + var allNames; + if (name) { + allNames = parentNames.concat(name); + if (node.parent.type === 'MemberExpression') { + markPropTypesAsUsed(node.parent, allNames); + } + // Do not mark computed props as used. + type = name !== '__COMPUTED_PROP__' ? 'direct' : null; } else if ( node.parent.parent.declarations && node.parent.parent.declarations[0].id.properties && @@ -354,7 +383,8 @@ module.exports = function(context) { break; } usedPropTypes.push({ - name: fullName, + name: name, + allNames: allNames, node: node.parent.property }); break; @@ -368,6 +398,7 @@ module.exports = function(context) { if (propName) { usedPropTypes.push({ name: propName, + allNames: [propName], node: properties[i] }); } @@ -441,19 +472,20 @@ module.exports = function(context) { * @param {Object} component The component to process */ function reportUndeclaredPropTypes(component) { - var name; + var allNames, name; for (var i = 0, j = component.usedPropTypes.length; i < j; i++) { name = component.usedPropTypes[i].name; + allNames = component.usedPropTypes[i].allNames; if ( - isIgnored(name.split('.').pop()) || - isDeclaredInComponent(component, name) + isIgnored(name) || + isDeclaredInComponent(component, allNames) ) { continue; } context.report( component.usedPropTypes[i].node, component.name === componentUtil.DEFAULT_COMPONENT_NAME ? MISSING_MESSAGE : MISSING_MESSAGE_NAMED_COMP, { - name: name.replace(/\.__COMPUTED_PROP__/g, '[]'), + name: allNames.join('.').replace(/\.__COMPUTED_PROP__/g, '[]'), component: component.name } ); diff --git a/tests/lib/rules/prop-types.js b/tests/lib/rules/prop-types.js index b3dce48e51..1a328c9d2b 100644 --- a/tests/lib/rules/prop-types.js +++ b/tests/lib/rules/prop-types.js @@ -432,6 +432,56 @@ eslintTester.addRuleTest('lib/rules/prop-types', { '};' ].join('\n'), parser: 'babel-eslint' + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props["some.value"];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' "some.value": React.PropTypes.string', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props["arr"][1];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' "arr": React.PropTypes.array', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props["arr"][1]["some.value"];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' "arr": React.PropTypes.arrayOf(', + ' React.PropTypes.shape({"some.value": React.PropTypes.string})', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + } } ], @@ -772,6 +822,63 @@ eslintTester.addRuleTest('lib/rules/prop-types', { errors: [ {message: '\'propX\' is missing in props validation for Hello'} ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props["some.value"];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'some.value\' is missing in props validation for Hello'} + ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props["arr"][1];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'arr\' is missing in props validation for Hello'} + ] + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' this.props["arr"][1]["some.value"];', + ' return
Hello
;', + ' }', + '}', + 'Hello.propTypes = {', + ' "arr": React.PropTypes.arrayOf(', + ' React.PropTypes.shape({})', + ' )', + '};' + ].join('\n'), + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [ + {message: '\'arr[].some.value\' is missing in props validation for Hello'} + ] } ] }); From 9b54b907d605bc4e0d99ce6fde1978255a8bb8dc Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Mon, 22 Jun 2015 00:33:55 +0000 Subject: [PATCH 10/11] Add acceptTranspilerName option to display-name rule (fixes #75) --- docs/rules/display-name.md | 60 +++++++++++++++++++++- lib/rules/display-name.js | 47 +++++++++++++++++ tests/lib/rules/display-name.js | 90 +++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 2 deletions(-) diff --git a/docs/rules/display-name.md b/docs/rules/display-name.md index c422b25d7e..8abc62ee7f 100644 --- a/docs/rules/display-name.md +++ b/docs/rules/display-name.md @@ -25,6 +25,62 @@ var Hello = React.createClass({ }); ``` -## When Not To Use It +## Rule Options -If you are using JSX this value is already automatically set and it is safe for you to disable this rule. +```js +... +"display-name": [, { "acceptTranspilerName": }] +... +``` + +### `acceptTranspilerName` + +When `true` the rule will accept the name set by the transpiler and does not require a `displayName` property in this case. + +The following patterns are considered okay and do not cause warnings: + +```js +var Hello = React.createClass({ + render: function() { + return
Hello {this.props.name}
; + } +}); +module.exports = Hello; +``` + +```js +export default class Hello extends React.Component { + render() { + return
Hello {this.props.name}
; + } +} +``` + +With the following patterns the transpiler can not assign a name for the component and therefore it will still cause warnings: + +```js +module.exports = React.createClass({ + render: function() { + return
Hello {this.props.name}
; + } +}); +``` + +```js +export default class extends React.Component { + render() { + return
Hello {this.props.name}
; + } +} +``` + +```js +function HelloComponent() { + return React.createClass({ + render: function() { + return
Hello {this.props.name}
; + } + }); +} +module.exports = HelloComponent(); +``` diff --git a/lib/rules/display-name.js b/lib/rules/display-name.js index 839b38b278..e6d43ec0d8 100644 --- a/lib/rules/display-name.js +++ b/lib/rules/display-name.js @@ -13,6 +13,9 @@ var ComponentList = componentUtil.List; module.exports = function(context) { + var config = context.options[0] || {}; + var acceptTranspilerName = config.acceptTranspilerName || false; + var componentList = new ComponentList(); var MISSING_MESSAGE = 'Component definition is missing display name'; @@ -80,6 +83,39 @@ module.exports = function(context) { ); } + /** + * Checks if the component have a name set by the transpiler + * @param {ASTNode} node The AST node being checked. + * @returns {Boolean} True ifcomponent have a name, false if not. + */ + function hasTranspilerName(node) { + var namedAssignment = ( + node.type === 'ObjectExpression' && + node.parent && + node.parent.parent && + node.parent.parent.type === 'AssignmentExpression' && ( + !node.parent.parent.left.object || + node.parent.parent.left.object.name !== 'module' || + node.parent.parent.left.property.name !== 'exports' + ) + ); + var namedDeclaration = ( + node.type === 'ObjectExpression' && + node.parent && + node.parent.parent && + node.parent.parent.type === 'VariableDeclarator' + ); + var namedClass = ( + node.type === 'ClassDeclaration' && + node.id && node.id.name + ); + + if (namedAssignment || namedDeclaration || namedClass) { + return true; + } + return false; + } + // -------------------------------------------------------------------------- // Public // -------------------------------------------------------------------------- @@ -112,6 +148,13 @@ module.exports = function(context) { markDisplayNameAsDeclared(node); }, + ClassDeclaration: function(node) { + if (!acceptTranspilerName || !hasTranspilerName(node)) { + return; + } + markDisplayNameAsDeclared(node); + }, + ObjectExpression: function(node) { // Search for the displayName declaration node.properties.forEach(function(property) { @@ -120,6 +163,10 @@ module.exports = function(context) { } markDisplayNameAsDeclared(node); }); + // Has transpiler name + if (acceptTranspilerName && hasTranspilerName(node)) { + markDisplayNameAsDeclared(node); + } if (componentUtil.isComponentDefinition(node)) { componentList.set(context, node, { diff --git a/tests/lib/rules/display-name.js b/tests/lib/rules/display-name.js index 22fd58229c..86b119d317 100644 --- a/tests/lib/rules/display-name.js +++ b/tests/lib/rules/display-name.js @@ -97,6 +97,60 @@ eslintTester.addRuleTest('lib/rules/display-name', { classes: true, jsx: true } + }, { + code: [ + 'var Hello = React.createClass({', + ' render: function() {', + ' return
Hello {this.props.name}
;', + ' }', + '});' + ].join('\n'), + args: [1, { + acceptTranspilerName: true + }], + ecmaFeatures: { + classes: true, + jsx: true + } + }, { + code: [ + 'class Hello extends React.Component {', + ' render() {', + ' return
Hello {this.props.name}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + args: [1, { + acceptTranspilerName: true + }] + }, { + code: [ + 'export default class Hello {', + ' render() {', + ' return
Hello {this.props.name}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + args: [1, { + acceptTranspilerName: true + }] + }, { + code: [ + 'var Hello;', + 'Hello = React.createClass({', + ' render: function() {', + ' return
Hello {this.props.name}
;', + ' }', + '});' + ].join('\n'), + args: [1, { + acceptTranspilerName: true + }], + ecmaFeatures: { + jsx: true + } }], invalid: [{ @@ -142,5 +196,41 @@ eslintTester.addRuleTest('lib/rules/display-name', { errors: [{ message: 'Hello component definition is missing display name' }] + }, { + code: [ + 'function HelloComponent() {', + ' return React.createClass({', + ' render: function() {', + ' return
Hello {this.props.name}
;', + ' }', + ' });', + '}', + 'module.exports = HelloComponent();' + ].join('\n'), + args: [1, { + acceptTranspilerName: true + }], + ecmaFeatures: { + classes: true, + jsx: true + }, + errors: [{ + message: 'Component definition is missing display name' + }] + }, { + code: [ + 'export default class {', + ' render() {', + ' return
Hello {this.props.name}
;', + ' }', + '}' + ].join('\n'), + parser: 'babel-eslint', + args: [1, { + acceptTranspilerName: true + }], + errors: [{ + message: 'Component definition is missing display name' + }] } ]}); From f6292bb14608bad37e273ad1f884a4e829e43f04 Mon Sep 17 00:00:00 2001 From: Yannick Croissant Date: Mon, 22 Jun 2015 00:41:33 +0000 Subject: [PATCH 11/11] Add require-extension rule to Readme --- README.md | 2 ++ docs/rules/{require-extenion.md => require-extension.md} | 0 2 files changed, 2 insertions(+) rename docs/rules/{require-extenion.md => require-extension.md} (100%) diff --git a/README.md b/README.md index 907d862fcd..4fe0991837 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ Finally, enable all of the rules that you would like to use. "react/no-unknown-property": 1, "react/prop-types": 1, "react/react-in-jsx-scope": 1, + "react/require-extension": 1, "react/self-closing-comp": 1, "react/sort-comp": 1, "react/wrap-multilines": 1 @@ -79,6 +80,7 @@ Finally, enable all of the rules that you would like to use. * [no-unknown-property](docs/rules/no-unknown-property.md): Prevent usage of unknown DOM property * [prop-types](docs/rules/prop-types.md): Prevent missing props validation in a React component definition * [react-in-jsx-scope](docs/rules/react-in-jsx-scope.md): Prevent missing React when using JSX +* [require-extension](docs/rules/require-extension.md): Restrict file extensions that may be required * [self-closing-comp](docs/rules/self-closing-comp.md): Prevent extra closing tags for components without children * [sort-comp](docs/rules/sort-comp.md): Enforce component methods order * [wrap-multilines](docs/rules/wrap-multilines.md): Prevent missing parentheses around multilines JSX diff --git a/docs/rules/require-extenion.md b/docs/rules/require-extension.md similarity index 100% rename from docs/rules/require-extenion.md rename to docs/rules/require-extension.md