Repository: yannickcr/eslint-plugin-react
Pull request #1509 (2017-10-31 04:57:10)

Add new rule for preventing `this` in SFCs
This rule will report errors when a stateless functional component is
attempting to use `this`.

This attempts to address the request in #1435.

jomasti: Okay, I can update the rule to report any usage of this. You mean all usages of `this` beyond just the typical React usages?
ljharb: Yes; an SFC has no reason to use `this` at all
jomasti: Updated

Patch:
diff --git a/README.md b/README.md
index 9084d6ad..35ad1074 100644
--- a/README.md
+++ b/README.md
@@ -108,6 +108,7 @@ Finally, enable all of the rules that you would like to use.  Use [our preset](#
 * [react/no-set-state](docs/rules/no-set-state.md): Prevent usage of `setState`
 * [react/no-typos](docs/rules/no-typos.md): Prevent common casing typos
 * [react/no-string-refs](docs/rules/no-string-refs.md): Prevent using string references in `ref` attribute.
+* [react/no-this-in-sfc](docs/rules/no-this-in-sfc.md): Prevent using `this` in stateless functional components
 * [react/no-unescaped-entities](docs/rules/no-unescaped-entities.md): Prevent invalid characters from appearing in markup
 * [react/no-unknown-property](docs/rules/no-unknown-property.md): Prevent usage of unknown DOM property (fixable)
 * [react/no-unused-prop-types](docs/rules/no-unused-prop-types.md): Prevent definitions of unused prop types
diff --git a/docs/rules/no-this-in-sfc.md b/docs/rules/no-this-in-sfc.md
new file mode 100644
index 00000000..e6224086
--- /dev/null
+++ b/docs/rules/no-this-in-sfc.md
@@ -0,0 +1,136 @@
+# Prevent `this` from being used in stateless functional components (react/no-this-in-sfc)
+
+When using a stateless functional component (SFC), props/context aren't accessed in the same way as a class component or the `create-react-class` format. Both props and context are passed as separate arguments to the component instead. Also, as the name suggests, a stateless component does not have state on `this.state`.
+
+Attempting to access properties on `this` can be a potential error if someone is unaware of the differences when writing a SFC or missed when converting a class component to a SFC.
+
+
+## Rule Details
+
+The following patterns are considered warnings:
+
+```jsx
+function Foo(props) {
+  return (
+    <div>{this.props.bar}</div>
+  );
+}
+```
+
+```jsx
+function Foo(props) {
+  const { bar } = this.props;
+  return (
+    <div>{bar}</div>
+  );
+}
+```
+
+```jsx
+function Foo(props, context) {
+  return (
+    <div>
+      {this.context.foo ? this.props.bar : ''}
+    </div>
+  );
+}
+```
+
+```jsx
+function Foo(props, context) {
+  const { foo } = this.context;
+  const { bar } = this.props;
+  return (
+    <div>
+      {foo ? bar : ''}
+    </div>
+  );
+}
+```
+
+```jsx
+function Foo(props) {
+  if (this.state.loading) {
+    return <Loader />;
+  }
+  return (
+    <div>
+      {this.props.bar}
+    </div>
+  );
+}
+```
+
+```jsx
+function Foo(props) {
+  const { loading } = this.state;
+  const { bar } = this.props;
+  if (loading) {
+    return <Loader />;
+  }
+  return (
+    <div>
+      {bar}
+    </div>
+  );
+}
+```
+
+The following patterns are **not** considered warnings:
+
+```jsx
+function Foo(props) {
+  return (
+    <div>{props.bar}</div>
+  );
+}
+```
+
+```jsx
+function Foo(props) {
+  const { bar } = props;
+  return (
+    <div>{bar}</div>
+  );
+}
+```
+
+```jsx
+function Foo({ bar }) {
+  return (
+    <div>{bar}</div>
+  );
+}
+```
+
+```jsx
+function Foo(props, context) {
+  return (
+    <div>
+      {context.foo ? props.bar : ''}
+    </div>
+  );
+}
+```
+
+```jsx
+function Foo(props, context) {
+  const { foo } = context;
+  const { bar } = props;
+  return (
+    <div>
+      {foo ? bar : ''}
+    </div>
+  );
+}
+```
+
+```jsx
+function Foo({ bar }, { foo }) {
+  return (
+    <div>
+      {foo ? bar : ''}
+    </div>
+  );
+}
+```
diff --git a/index.js b/index.js
index 6c1a933a..771b1643 100644
--- a/index.js
+++ b/index.js
@@ -55,6 +55,7 @@ const allRules = {
   'no-string-refs': require('./lib/rules/no-string-refs'),
   'no-redundant-should-component-update': require('./lib/rules/no-redundant-should-component-update'),
   'no-render-return-value': require('./lib/rules/no-render-return-value'),
+  'no-this-in-sfc': require('./lib/rules/no-this-in-sfc'),
   'no-typos': require('./lib/rules/no-typos'),
   'no-unescaped-entities': require('./lib/rules/no-unescaped-entities'),
   'no-unknown-property': require('./lib/rules/no-unknown-property'),
diff --git a/lib/rules/no-this-in-sfc.js b/lib/rules/no-this-in-sfc.js
new file mode 100644
index 00000000..443fbdcc
--- /dev/null
+++ b/lib/rules/no-this-in-sfc.js
@@ -0,0 +1,42 @@
+/**
+ * @fileoverview Report "this" being used in stateless functional components.
+ */
+'use strict';
+
+const Components = require('../util/Components');
+
+// ------------------------------------------------------------------------------
+// Constants
+// ------------------------------------------------------------------------------
+
+const ERROR_MESSAGE = 'Stateless functional components should not use this';
+
+// ------------------------------------------------------------------------------
+// Rule Definition
+// ------------------------------------------------------------------------------
+
+module.exports = {
+  meta: {
+    docs: {
+      description: 'Report "this" being used in stateless components',
+      category: 'Possible Errors',
+      recommended: false
+    },
+    schema: []
+  },
+
+  create: Components.detect((context, components, utils) => ({
+    MemberExpression(node) {
+      const component = components.get(utils.getParentStatelessComponent());
+      if (!component) {
+        return;
+      }
+      if (node.object.type === 'ThisExpression') {
+        context.report({
+          node: node,
+          message: ERROR_MESSAGE
+        });
+      }
+    }
+  }))
+};
diff --git a/lib/util/Components.js b/lib/util/Components.js
index 7ede0779..90ec3855 100644
--- a/lib/util/Components.js
+++ b/lib/util/Components.js
@@ -303,14 +303,7 @@ function componentRule(rule, context) {
       return calledOnReact;
     },
 
-    /**
-     * Check if the node is returning JSX
-     *
-     * @param {ASTNode} ASTnode The AST node being checked
-     * @param {Boolean} strict If true, in a ternary condition the node must return JSX in both cases
-     * @returns {Boolean} True if the node is returning JSX, false if not
-     */
-    isReturningJSX: function(ASTnode, strict) {
+    getReturnPropertyAndNode(ASTnode) {
       let property;
       let node = ASTnode;
       switch (node.type) {
@@ -319,14 +312,36 @@ function componentRule(rule, context) {
           break;
         case 'ArrowFunctionExpression':
           property = 'body';
+          if (node[property] && node[property].type === 'BlockStatement') {
+            node = utils.findReturnStatement(node);
+            property = 'argument';
+          }
           break;
         default:
           node = utils.findReturnStatement(node);
-          if (!node) {
-            return false;
-          }
           property = 'argument';
       }
+      return {
+        node: node,
+        property: property
+      };
+    },
+
+    /**
+     * Check if the node is returning JSX
+     *
+     * @param {ASTNode} ASTnode The AST node being checked
+     * @param {Boolean} strict If true, in a ternary condition the node must return JSX in both cases
+     * @returns {Boolean} True if the node is returning JSX, false if not
+     */
+    isReturningJSX: function(ASTnode, strict) {
+      const nodeAndProperty = utils.getReturnPropertyAndNode(ASTnode);
+      const node = nodeAndProperty.node;
+      const property = nodeAndProperty.property;
+
+      if (!node) {
+        return false;
+      }
 
       const returnsConditionalJSXConsequent =
         node[property] &&
@@ -356,6 +371,35 @@ function componentRule(rule, context) {
       );
     },
 
+    /**
+     * Check if the node is returning null
+     *
+     * @param {ASTNode} ASTnode The AST node being checked
+     * @returns {Boolean} True if the node is returning null, false if not
+     */
+    isReturningNull(ASTnode) {
+      const nodeAndProperty = utils.getReturnPropertyAndNode(ASTnode);
+      const property = nodeAndProperty.property;
+      const node = nodeAndProperty.node;
+
+      if (!node) {
+        return false;
+      }
+
+      return node[property] && node[property].value === null;
+    },
+
+    /**
+     * Check if the node is returning JSX or null
+     *
+     * @param {ASTNode} ASTnode The AST node being checked
+     * @param {Boolean} strict If true, in a ternary condition the node must return JSX in both cases
+     * @returns {Boolean} True if the node is returning JSX or null, false if not
+     */
+    isReturningJSXOrNull(ASTNode, strict) {
+      return utils.isReturningJSX(ASTNode, strict) || utils.isReturningNull(ASTNode);
+    },
+
     /**
      * Find a return statment in the current node
      *
@@ -430,7 +474,7 @@ function componentRule(rule, context) {
           return null;
         }
         // Return the node if it is a function that is not a class method and is not inside a JSX Element
-        if (isFunction && !isMethod && !isJSXExpressionContainer) {
+        if (isFunction && !isMethod && !isJSXExpressionContainer && utils.isReturningJSXOrNull(node)) {
           return node;
         }
         scope = scope.upper;


Test patch:
diff --git a/tests/lib/rules/no-this-in-sfc.js b/tests/lib/rules/no-this-in-sfc.js
new file mode 100644
index 00000000..350fb1fd
--- /dev/null
+++ b/tests/lib/rules/no-this-in-sfc.js
@@ -0,0 +1,170 @@
+/**
+ * @fileoverview Report "this" being used in stateless functional components.
+ */
+'use strict';
+
+// ------------------------------------------------------------------------------
+// Constants
+// ------------------------------------------------------------------------------
+
+const ERROR_MESSAGE = 'Stateless functional components should not use this';
+
+// ------------------------------------------------------------------------------
+// Requirements
+// ------------------------------------------------------------------------------
+
+const rule = require('../../../lib/rules/no-this-in-sfc');
+const RuleTester = require('eslint').RuleTester;
+
+const parserOptions = {
+  ecmaVersion: 8,
+  sourceType: 'module',
+  ecmaFeatures: {
+    experimentalObjectRestSpread: true,
+    jsx: true
+  }
+};
+
+const ruleTester = new RuleTester({parserOptions});
+ruleTester.run('no-this-in-sfc', rule, {
+  valid: [{
+    code: `
+    function Foo(props) {
+      const { foo } = props;
+      return <div bar={foo} />;
+    }`
+  }, {
+    code: `
+    function Foo({ foo }) {
+      return <div bar={foo} />;
+    }`
+  }, {
+    code: `
+    class Foo extends React.Component {
+      render() {
+        const { foo } = this.props;
+        return <div bar={foo} />;
+      }
+    }`
+  }, {
+    code: `
+    const Foo = createReactClass({
+      render: function() {
+        return <div>{this.props.foo}</div>;
+      }
+    });`
+  }, {
+    code: `
+    const Foo = React.createClass({
+      render: function() {
+        return <div>{this.props.foo}</div>;
+      }
+    });`,
+    settings: {react: {createClass: 'createClass'}}
+  }, {
+    code: `
+    function foo(bar) {
+      this.bar = bar;
+      this.props = 'baz';
+      this.getFoo = function() {
+        return this.bar + this.props;
+      }
+    }
+    `
+  }, {
+    code: `
+    function Foo(props) {
+      return props.foo ? <span>{props.bar}</span> : null;
+    }`
+  }, {
+    code: `
+    function Foo(props) {
+      if (props.foo) {
+        return <div>{props.bar}</div>;
+      }
+      return null;
+    }`
+  }, {
+    code: `
+    function Foo(props) {
+      if (props.foo) {
+        something();
+      }
+      return null;
+    }`
+  }, {
+    code: 'const Foo = (props) => <span>{props.foo}</span>'
+  }, {
+    code: 'const Foo = ({ foo }) => <span>{foo}</span>'
+  }, {
+    code: 'const Foo = (props) => props.foo ? <span>{props.bar}</span> : null;'
+  }, {
+    code: 'const Foo = ({ foo, bar }) => foo ? <span>{bar}</span> : null;'
+  }],
+  invalid: [{
+    code: `
+    function Foo(props) {
+      const { foo } = this.props;
+      return <div>{foo}</div>;
+    }`,
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: `
+    function Foo(props) {
+      return <div>{this.props.foo}</div>;
+    }`,
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: `
+    function Foo(props) {
+      return <div>{this.state.foo}</div>;
+    }`,
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: `
+    function Foo(props) {
+      const { foo } = this.state;
+      return <div>{foo}</div>;
+    }`,
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: `
+    function Foo(props) {
+      return props.foo ? <div>{this.props.bar}</div> : null;
+    }`,
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: `
+    function Foo(props) {
+      if (props.foo) {
+        return <div>{this.props.bar}</div>;
+      }
+      return null;
+    }`,
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: `
+    function Foo(props) {
+      if (this.props.foo) {
+        something();
+      }
+      return null;
+    }`,
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: 'const Foo = (props) => <span>{this.props.foo}</span>',
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: 'const Foo = (props) => this.props.foo ? <span>{props.bar}</span> : null;',
+    errors: [{message: ERROR_MESSAGE}]
+  }, {
+    code: `
+    function Foo(props) {
+      function onClick(bar) {
+        this.props.onClick();
+      }
+      return <div onClick={onClick}>{this.props.foo}</div>;
+    }`,
+    errors: [{message: ERROR_MESSAGE}, {message: ERROR_MESSAGE}]
+  }]
+});