Merge pull request #362 from Intellicode/no-refs

Add no-string-refs rule (fixes #341)

README.md

@@ -87,6 +87,7 @@ Finally, enable all of the rules that you would like to use.
     "react/no-direct-mutation-state": 1,
     "react/no-multi-comp": 1,
     "react/no-set-state": 1,
+    "react/no-string-refs": 1,
     "react/no-unknown-property": 1,
     "react/prefer-es6-class": 1,
     "react/prop-types": 1,
@@ -128,6 +129,7 @@ Finally, enable all of the rules that you would like to use.
 * [no-is-mounted](docs/rules/no-is-mounted.md): Prevent usage of `isMounted`
 * [no-multi-comp](docs/rules/no-multi-comp.md): Prevent multiple component definition per file
 * [no-set-state](docs/rules/no-set-state.md): Prevent usage of `setState`
+* [no-string-refs](docs/rules/no-string-refs.md): Prevent using string references in `ref` attribute.
 * [no-unknown-property](docs/rules/no-unknown-property.md): Prevent usage of unknown DOM property
 * [prefer-es6-class](docs/rules/prefer-es6-class.md): Enforce ES5 or ES6 class for React Components
 * [prop-types](docs/rules/prop-types.md): Prevent missing props validation in a React component definition

docs/rules/no-string-refs.md

@@ -0,0 +1,41 @@
+# Prevent using string references (no-string-refs)
+
+Currently, two ways are supported by React to refer to components. The first one, providing a string identifier is considered legacy in the official documentation. Referring to components by setting an property on the `this` object in the reference callback is preferred.
+
+## Rule Details
+
+Invalid:
+
+```js
+var Hello = React.createClass({
+ render: function() {
+  return <div ref="hello">Hello, world.</div>;
+ }
+});
+```
+
+```js
+var Hello = React.createClass({
+  componentDidMount: function() {
+    var component = this.refs.hello;
+    // ...do something with component
+  },
+  render: function() {
+    return <div ref="hello">Hello, world.</div>;
+  }
+});
+```
+
+Valid:
+
+```js
+var Hello = React.createClass({
+  componentDidMount: function() {
+    var component = this.hello;
+    // ...do something with component
+  },
+  render() {
+    return <div ref={c => this.hello = c}>Hello, world.</div>;
+  }
+});
+```

index.js

@@ -36,7 +36,8 @@ module.exports = {
     'no-direct-mutation-state': require('./lib/rules/no-direct-mutation-state'),
     'forbid-prop-types': require('./lib/rules/forbid-prop-types'),
     'prefer-es6-class': require('./lib/rules/prefer-es6-class'),
-    'jsx-key': require('./lib/rules/jsx-key')
+    'jsx-key': require('./lib/rules/jsx-key'),
+    'no-string-refs': require('./lib/rules/no-string-refs')
   },
   rulesConfig: {
     'jsx-uses-react': 0,
@@ -73,6 +74,7 @@ module.exports = {
     'no-direct-mutation-state': 0,
     'forbid-prop-types': 0,
     'prefer-es6-class': 0,
-    'jsx-key': 0
+    'jsx-key': 0,
+    'no-string-refs': 0
   }
 };

lib/rules/no-string-refs.js

@@ -0,0 +1,88 @@
+/**
+ * @fileoverview Prevent string definitions for references and prevent referencing this.refs
+ * @author Tom Hastjarjanto
+ */
+'use strict';
+
+var Components = require('../util/Components');
+
+// ------------------------------------------------------------------------------
+// Rule Definition
+// ------------------------------------------------------------------------------
+
+module.exports = Components.detect(function(context, components, utils) {
+  /**
+   * Checks if we are using refs
+   * @param {ASTNode} node The AST node being checked.
+   * @returns {Boolean} True if we are using refs, false if not.
+   */
+  function isRefsUsage(node) {
+    return Boolean(
+      (
+        utils.getParentES6Component() ||
+        utils.getParentES5Component()
+      ) &&
+      node.object.type === 'ThisExpression' &&
+      node.property.name === 'refs'
+    );
+  }
+
+  /**
+   * Checks if we are using a ref attribute
+   * @param {ASTNode} node The AST node being checked.
+   * @returns {Boolean} True if we are using a ref attribute, false if not.
+   */
+  function isRefAttribute(node) {
+    return Boolean(
+      node.type === 'JSXAttribute' &&
+      node.name &&
+      node.name.name === 'ref'
+    );
+  }
+
+  /**
+   * Checks if a node contains a string value
+   * @param {ASTNode} node The AST node being checked.
+   * @returns {Boolean} True if the node contains a string value, false if not.
+   */
+  function containsStringLiteral(node) {
+    return Boolean(
+      node.value &&
+      node.value.type === 'Literal' &&
+      typeof node.value.value === 'string'
+    );
+  }
+
+  /**
+   * Checks if a node contains a string value within a jsx expression
+   * @param {ASTNode} node The AST node being checked.
+   * @returns {Boolean} True if the node contains a string value within a jsx expression, false if not.
+   */
+  function containsStringExpressionContainer(node) {
+    return Boolean(
+      node.value &&
+      node.value.type === 'JSXExpressionContainer' &&
+      node.value.expression &&
+      node.value.expression.type === 'Literal' &&
+      typeof node.value.expression.value === 'string'
+    );
+  }
+
+  return {
+    MemberExpression: function(node) {
+      if (isRefsUsage(node)) {
+        context.report(node, 'Using this.refs is deprecated.');
+      }
+    },
+    JSXAttribute: function(node) {
+      if (
+        isRefAttribute(node) &&
+        (containsStringLiteral(node) || containsStringExpressionContainer(node))
+      ) {
+        context.report(node, 'Using string literals in ref attributes is deprecated.');
+      }
+    }
+  };
+});
+
+module.exports.schema = [];

tests/lib/rules/no-string-refs.js

@@ -0,0 +1,114 @@
+/**
+ * @fileoverview Prevent string definitions for references and prevent referencing this.refs
+ * @author Tom Hastjarjanto
+ */
+'use strict';
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+var rule = require('../../../lib/rules/no-string-refs');
+var RuleTester = require('eslint').RuleTester;
+
+require('babel-eslint');
+
+// ------------------------------------------------------------------------------
+// Tests
+// ------------------------------------------------------------------------------
+
+var ruleTester = new RuleTester();
+ruleTester.run('no-refs', rule, {
+
+  valid: [{
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+      '     var component = this.hello;',
+      '  },',
+      '  render: function() {',
+      '    return <div ref={c => this.hello = c}>Hello {this.props.name}</div>;',
+      '  }',
+      '});'
+    ].join('\n'),
+    parser: 'babel-eslint',
+    ecmaFeatures: {
+      jsx: true
+    }
+  }
+  ],
+
+  invalid: [{
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+      '     var component = this.refs.hello;',
+      '  },',
+      '  render: function() {',
+      '    return <div>Hello {this.props.name}</div>;',
+      '  }',
+      '});'
+    ].join('\n'),
+    parser: 'babel-eslint',
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    },
+    errors: [{
+      message: 'Using this.refs is deprecated.'
+    }]
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  render: function() {',
+      '    return <div ref="hello">Hello {this.props.name}</div>;',
+      '  }',
+      '});'
+    ].join('\n'),
+    parser: 'babel-eslint',
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    },
+    errors: [{
+      message: 'Using string literals in ref attributes is deprecated.'
+    }]
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  render: function() {',
+      '    return <div ref={\'hello\'}>Hello {this.props.name}</div>;',
+      '  }',
+      '});'
+    ].join('\n'),
+    parser: 'babel-eslint',
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    },
+    errors: [{
+      message: 'Using string literals in ref attributes is deprecated.'
+    }]
+  }, {
+    code: [
+      'var Hello = React.createClass({',
+      '  componentDidMount: function() {',
+      '     var component = this.refs.hello;',
+      '  },',
+      '  render: function() {',
+      '    return <div ref="hello">Hello {this.props.name}</div>;',
+      '  }',
+      '});'
+    ].join('\n'),
+    parser: 'babel-eslint',
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    },
+    errors: [{
+      message: 'Using this.refs is deprecated.'
+    }, {
+      message: 'Using string literals in ref attributes is deprecated.'
+    }]
+  }
+]});