Repository: yannickcr/eslint-plugin-react
Pull request #793 (2016-08-30 14:27:28)

jsx-no-target-blank: Target only anchor elements
This pull request seeks to resolve lint errors which are raised when the `jsx-no-target-blank` rule is enabled when `target="_blank"` is applied to anything other than an anchor element. The [documentation for this rule](https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-no-target-blank.md) clearly states that `<p target="_blank"></p>` is not considered an error, but adding this missing test case reveals that the rule applies to all elements.

We observed this in https://github.com/Automattic/wp-calypso/pull/7680 where we have custom components which handle the incoming `target` prop and apply the `rel` as necessary through custom render logic.


Patch:
diff --git a/README.md b/README.md
index 62605800..a696ff08 100644
--- a/README.md
+++ b/README.md
@@ -69,7 +69,7 @@ With ESLint 2.x.x or 3.x.x:
 }
 ```
 
-Finally, enable all of the rules that you would like to use.  Use [our preset](#user-content-recommended-configuration) to get reasonable defaults quickly, and/or choose your own:
+Finally, enable all of the rules that you would like to use.  Use [our preset](#recommended) to get reasonable defaults quickly, and/or choose your own:
 
 ```json
   "rules": {
diff --git a/docs/rules/no-unused-prop-types.md b/docs/rules/no-unused-prop-types.md
index cea26a3d..de85b616 100644
--- a/docs/rules/no-unused-prop-types.md
+++ b/docs/rules/no-unused-prop-types.md
@@ -47,7 +47,7 @@ This rule can take one argument to ignore some specific props during validation.
 
 ```
 ...
-"prop-types": [<enabled>, { customValidators: <customValidator> }]
+"no-unused-prop-types": [<enabled>, { customValidators: <customValidator>, skipShapeProps: <skipShapeProps> }]
 ...
 ```
 
diff --git a/docs/rules/prop-types.md b/docs/rules/prop-types.md
index 2432ed30..c705a30d 100644
--- a/docs/rules/prop-types.md
+++ b/docs/rules/prop-types.md
@@ -29,6 +29,39 @@ function Hello({ name }) {
 }
 ```
 
+Examples of correct usage without warnings:
+
+```jsx
+var Hello = React.createClass({
+  propTypes: {
+    name: React.PropTypes.string.isRequired,
+  },
+  render: function() {
+    return <div>Hello {this.props.name}</div>;
+  },
+});
+
+// Or in ES6:
+class HelloEs6 extends React.Component {
+  render() {
+    return <div>Hello {this.props.name}</div>;
+  }
+}
+HelloEs6.propTypes = {
+  name: React.PropTypes.string.isRequired,
+};
+
+// ES6 + Public Class Fields (draft: https://tc39.github.io/proposal-class-public-fields/)
+class HelloEs6WithPublicClassField extends React.Component {
+  static propTypes = {
+    name: React.PropTypes.string.isRequired,
+  }
+  render() {
+    return <div>Hello {this.props.name}</div>;
+  }
+}
+```
+
 The following patterns are not considered warnings:
 
 ```jsx
diff --git a/lib/rules/jsx-no-target-blank.js b/lib/rules/jsx-no-target-blank.js
index 19ddd2a0..d6210037 100644
--- a/lib/rules/jsx-no-target-blank.js
+++ b/lib/rules/jsx-no-target-blank.js
@@ -21,6 +21,10 @@ module.exports = {
   create: function(context) {
     return {
       JSXAttribute: function(node) {
+        if (node.parent.name.name !== 'a') {
+          return;
+        }
+
         if (node.name.name === 'target' && node.value.value === '_blank') {
           var relFound = false;
           var attrs = node.parent.attributes;
diff --git a/lib/rules/no-unused-prop-types.js b/lib/rules/no-unused-prop-types.js
index e3b612c1..1d0bb94d 100644
--- a/lib/rules/no-unused-prop-types.js
+++ b/lib/rules/no-unused-prop-types.js
@@ -174,6 +174,53 @@ module.exports = {
       );
     }
 
+    /**
+     * Returns true if the given node is a React Component lifecycle method
+     * @param {ASTNode} node The AST node being checked.
+     * @return {Boolean} True if the node is a lifecycle method
+     */
+    function isNodeALifeCycleMethod(node) {
+      var nodeKeyName = (node.key || {}).name;
+      return (
+        node.kind === 'constructor' ||
+        nodeKeyName === 'componentWillReceiveProps' ||
+        nodeKeyName === 'shouldComponentUpdate' ||
+        nodeKeyName === 'componentWillUpdate' ||
+        nodeKeyName === 'componentDidUpdate'
+      );
+    }
+
+    /**
+     * Returns true if the given node is inside a React Component lifecycle
+     * method.
+     * @param {ASTNode} node The AST node being checked.
+     * @return {Boolean} True if the node is inside a lifecycle method
+     */
+    function isInLifeCycleMethod(node) {
+      if (node.type === 'MethodDefinition' && isNodeALifeCycleMethod(node)) {
+        return true;
+      }
+
+      if (node.parent) {
+        return isInLifeCycleMethod(node.parent);
+      }
+
+      return false;
+    }
+
+    /**
+     * Checks if a prop init name matches common naming patterns
+     * @param {ASTNode} node The AST node being checked.
+     * @returns {Boolean} True if the prop name matches
+     */
+    function isPropAttributeName (node) {
+      return (
+        node.init.name === 'props' ||
+        node.init.name === 'nextProps' ||
+        node.init.name === 'prevProps'
+      );
+    }
+
     /**
      * Checks if a prop is used
      * @param {ASTNode} node The AST node being checked.
@@ -536,11 +583,14 @@ module.exports = {
               node.id.properties[i].value.type === 'ObjectPattern'
             );
             // let {firstname} = props
-            var statelessDestructuring = node.init.name === 'props' && utils.getParentStatelessComponent();
+            var genericDestructuring = isPropAttributeName(node) && (
+                utils.getParentStatelessComponent() ||
+                isInLifeCycleMethod(node)
+            );
 
             if (thisDestructuring) {
               properties = node.id.properties[i].value.properties;
-            } else if (statelessDestructuring) {
+            } else if (genericDestructuring) {
               properties = node.id.properties;
             } else {
               continue;
@@ -776,7 +826,10 @@ module.exports = {
         // let {props: {firstname}} = this
         var thisDestructuring = destructuring && node.init.type === 'ThisExpression';
         // let {firstname} = props
-        var statelessDestructuring = destructuring && node.init.name === 'props' && utils.getParentStatelessComponent();
+        var statelessDestructuring = destructuring && isPropAttributeName(node) && (
+            utils.getParentStatelessComponent() ||
+            isInLifeCycleMethod(node)
+        );
 
         if (!thisDestructuring && !statelessDestructuring) {
           return;
@@ -831,6 +884,18 @@ module.exports = {
         }
       },
 
+      ObjectPattern: function(node) {
+        // If the object pattern is a destructured props object in a lifecycle
+        // method -- mark it for used props.
+        if (isNodeALifeCycleMethod(node.parent.parent)) {
+          node.properties.forEach(function(property, i) {
+            if (i === 0) {
+              markPropTypesAsUsed(node.parent);
+            }
+          });
+        }
+      },
+
       ObjectExpression: function(node) {
         // Search for the proptypes declaration
         node.properties.forEach(function(property) {


Test patch:
diff --git a/tests/lib/rules/jsx-no-target-blank.js b/tests/lib/rules/jsx-no-target-blank.js
index 48297eb2..06f2a496 100644
--- a/tests/lib/rules/jsx-no-target-blank.js
+++ b/tests/lib/rules/jsx-no-target-blank.js
@@ -29,7 +29,8 @@ ruleTester.run('jsx-no-target-blank', rule, {
     {code: '<a randomTag></a>', parserOptions: parserOptions},
     {code: '<a href="foobar" target="_blank" rel="noopener noreferrer"></a>', parserOptions: parserOptions},
     {code: '<a target="_blank" {...spreadProps} rel="noopener noreferrer"></a>', parserOptions: parserOptions},
-    {code: '<a target="_blank" rel="noopener noreferrer" {...spreadProps}></a>', parserOptions: parserOptions}
+    {code: '<a target="_blank" rel="noopener noreferrer" {...spreadProps}></a>', parserOptions: parserOptions},
+    {code: '<p target="_blank"></p>', parserOptions: parserOptions}
   ],
   invalid: [
     {code: '<a target="_blank"></a>', parserOptions: parserOptions,
diff --git a/tests/lib/rules/no-unused-prop-types.js b/tests/lib/rules/no-unused-prop-types.js
index 922498b5..986a42dd 100644
--- a/tests/lib/rules/no-unused-prop-types.js
+++ b/tests/lib/rules/no-unused-prop-types.js
@@ -1208,6 +1208,180 @@ ruleTester.run('no-unused-prop-types', rule, {
       ].join('\n'),
       options: [{skipShapeProps: true}],
       parser: 'babel-eslint'
+    }, {
+      // Destructured props in componentWillReceiveProps shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  componentWillReceiveProps (nextProps) {',
+        '    const {something} = nextProps;',
+        '    doSomething(something);',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    }, {
+      // Destructured function props in componentWillReceiveProps shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  componentWillReceiveProps ({something}) {',
+        '    doSomething(something);',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint'
+    }, {
+      // Destructured props in the constructor shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  constructor (props) {',
+        '    super(props);',
+        '    const {something} = props;',
+        '    doSomething(something);',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured function props in the constructor shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  constructor ({something}) {',
+        '    super({something});',
+        '    doSomething(something);',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured props in the `componentWillReceiveProps` method shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  componentWillReceiveProps (nextProps, nextState) {',
+        '    const {something} = nextProps;',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured function props in the `componentWillReceiveProps` method shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  componentWillReceiveProps ({something}, nextState) {',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured props in the `shouldComponentUpdate` method shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  shouldComponentUpdate (nextProps, nextState) {',
+        '    const {something} = nextProps;',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured function props in the `shouldComponentUpdate` method shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  shouldComponentUpdate ({something}, nextState) {',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured props in the `componentWillUpdate` method shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  componentWillUpdate (nextProps, nextState) {',
+        '    const {something} = nextProps;',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured function props in the `componentWillUpdate` method shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  componentWillUpdate ({something}, nextState) {',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured props in the `componentDidUpdate` method shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  componentDidUpdate (prevProps, nextState) {',
+        '    const {something} = prevProps;',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
+    }, {
+      // Destructured function props in the `componentDidUpdate` method shouldn't throw errors
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    something: PropTypes.bool',
+        '  }',
+        '  componentDidUpdate ({something}, nextState) {',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions
     }
   ],
 
@@ -1976,6 +2150,193 @@ ruleTester.run('no-unused-prop-types', rule, {
       errors: [{
         message: '\'unused\' PropType is defined but prop is never used'
       }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  constructor (props) {',
+        '    super(props);',
+        '    const {something} = props;',
+        '    doSomething(something);',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  constructor ({something}) {',
+        '    super({something});',
+        '    doSomething(something);',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  componentWillReceiveProps (nextProps, nextState) {',
+        '    const {something} = nextProps;',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  componentWillReceiveProps ({something}, nextState) {',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  shouldComponentUpdate (nextProps, nextState) {',
+        '    const {something} = nextProps;',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  shouldComponentUpdate ({something}, nextState) {',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  componentWillUpdate (nextProps, nextState) {',
+        '    const {something} = nextProps;',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  componentWillUpdate ({something}, nextState) {',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  componentDidUpdate (prevProps, prevState) {',
+        '    const {something} = prevProps;',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
+    }, {
+      code: [
+        'class Hello extends Component {',
+        '  static propTypes = {',
+        '    unused: PropTypes.bool',
+        '  }',
+        '  componentDidUpdate ({something}, prevState) {',
+        '    return something;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      parserOptions: parserOptions,
+      errors: [{
+        message: '\'unused\' PropType is defined but prop is never used',
+        line: 3,
+        column: 13
+      }]
     }/* , {
       // Enable this when the following issue is fixed
       // https://github.com/yannickcr/eslint-plugin-react/issues/296