From 4a830354f33a42a3c74f8012642b863f84c989d4 Mon Sep 17 00:00:00 2001
From: cyan33 <cyan.binary@gmail.com>
Date: Thu, 23 Apr 2020 14:07:27 -0700
Subject: [PATCH 1/5] Extend Namespace to PascalCase

---
 .../__tests__/ESLintRulesOfHooks-test.js      | 108 ++++++++++--------
 .../src/RulesOfHooks.js                       |   5 +-
 2 files changed, 65 insertions(+), 48 deletions(-)

diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
index 73bb86989c8bd..3c17b68b08fea 100644
--- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
+++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
@@ -106,6 +106,7 @@ const tests = {
       ({useHook() { useState(); }});
       const {useHook3 = () => { useState(); }} = {};
       ({useHook = () => { useState(); }} = {});
+      Namespace.useHook = () => { useState(); };
     `,
     `
       // Valid because hooks can call hooks.
@@ -191,29 +192,6 @@ const tests = {
         }
       }
     `,
-    `
-      // Currently valid.
-      // We *could* make this invalid if we want, but it creates false positives
-      // (see the FooStore case).
-      class C {
-        m() {
-          This.useHook();
-          Super.useHook();
-        }
-      }
-    `,
-    `
-      // Valid although we *could* consider these invalid.
-      // But it doesn't bring much benefit since it's an immediate runtime error anyway.
-      // So might as well allow it.
-      Hook.use();
-      Hook._use();
-      Hook.useState();
-      Hook._useState();
-      Hook.use42();
-      Hook.useHook();
-      Hook.use_hook();
-    `,
     `
       // Valid -- this is a regression test.
       jest.useFakeTimers();
@@ -221,17 +199,6 @@ const tests = {
         jest.useRealTimers();
       })
     `,
-    `
-      // Valid because that's a false positive we've seen quite a bit.
-      // This is a regression test.
-      class Foo extends Component {
-        render() {
-          if (cond) {
-            FooStore.useFeatureFlag();
-          }
-        }
-      }
-    `,
     `
       // Valid because they're not matching use[A-Z].
       fooState();
@@ -240,16 +207,6 @@ const tests = {
       _useState();
       use_hook();
     `,
-    `
-      // This is grey area.
-      // Currently it's valid (although React.useCallback would fail here).
-      // We could also get stricter and disallow it, just like we did
-      // with non-namespace use*() top-level calls.
-      const History = require('history-2.1.2');
-      const browserHistory = History.useBasename(History.createHistory)({
-        basename: '/',
-      });
-    `,
     `
       // Regression test for some internal code.
       // This shows how the "callback rule" is more relaxed,
@@ -352,7 +309,6 @@ const tests = {
         if (c) {} else {}
         if (c) {} else {}
         if (c) {} else {}
-
         // 10 hooks
         useHook();
         useHook();
@@ -392,6 +348,68 @@ const tests = {
       `,
       errors: [conditionalError('useConditionalHook')],
     },
+    {
+      code: `
+        Hook.use();
+        Hook._use();
+        Hook.useState();
+        Hook._useState();
+        Hook.use42();
+        Hook.useHook();
+        Hook.use_hook();
+      `,
+      errors: [
+        topLevelError('Hook.useState'),
+        topLevelError('Hook.use42'),
+        topLevelError('Hook.useHook'),
+      ],
+    },
+    {
+      code: `
+        class C {
+          m() {
+            This.useHook();
+            Super.useHook();
+          }
+        }
+      `,
+      errors: [classError('This.useHook'), classError('Super.useHook')],
+    },
+    {
+      code: `
+        class Foo extends Component {
+          render() {
+            if (cond) {
+              FooStore.useFeatureFlag();
+            }
+          }
+        }
+      `,
+      errors: [classError('FooStore.useFeatureFlag')],
+    },
+    {
+      code: `
+        // Invalid because it's dangerous and might not warn otherwise.
+        // This *must* be invalid.
+        function ComponentWithConditionalHook() {
+          if (cond) {
+            Namespace.useConditionalHook();
+          }
+        }
+      `,
+      errors: [conditionalError('Namespace.useConditionalHook')],
+    },
+    {
+      code: `
+        // Extending the isHook check to [A-Z].*\.use
+        // to not let it be called on top level
+        const History = require('history-2.1.2');
+        const browserHistory = History.useBasename(History.createHistory)({
+          basename: '/',
+        });
+      `,
+      errors: [topLevelError('History.useBasename')],
+    },
     {
       code: `
         // Invalid because it's dangerous and might not warn otherwise.
diff --git a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
index 71d2d42c938d6..dda5a1ee3c4f9 100644
--- a/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
+++ b/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
@@ -31,10 +31,9 @@ function isHook(node) {
     !node.computed &&
     isHook(node.property)
   ) {
-    // Only consider React.useFoo() to be namespace hooks for now to avoid false positives.
-    // We can expand this check later.
     const obj = node.object;
-    return obj.type === 'Identifier' && obj.name === 'React';
+    const isPascalCaseNameSpace = /^[A-Z].*/;
+    return obj.type === 'Identifier' && isPascalCaseNameSpace.test(obj.name);
   } else {
     return false;
   }

From 88c614ce66c5bb4bcf66d654ef8c773f6bcfd767 Mon Sep 17 00:00:00 2001
From: cyan33 <cyan.binary@gmail.com>
Date: Thu, 23 Apr 2020 14:21:08 -0700
Subject: [PATCH 2/5] Add valid case for jest.useFakeTimer

---
 .../__tests__/ESLintRulesOfHooks-test.js                        | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
index 3c17b68b08fea..27fc6631a13d4 100644
--- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
+++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
@@ -206,6 +206,8 @@ const tests = {
       _use();
       _useState();
       use_hook();
+      // also valid because it's not matching the PascalCase namespace
+      jest.useFakeTimer()
     `,
     `
       // Regression test for some internal code.

From e7fda9402f993399eba0a2ebdd63e75f869e5e90 Mon Sep 17 00:00:00 2001
From: cyan33 <cyan.binary@gmail.com>
Date: Thu, 23 Apr 2020 14:22:19 -0700
Subject: [PATCH 3/5] format

---
 .../__tests__/ESLintRulesOfHooks-test.js                         | 1 +
 1 file changed, 1 insertion(+)

diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
index 27fc6631a13d4..e2d0f60cbfec7 100644
--- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
+++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
@@ -311,6 +311,7 @@ const tests = {
         if (c) {} else {}
         if (c) {} else {}
         if (c) {} else {}
+        
         // 10 hooks
         useHook();
         useHook();

From 6fc2bab47d8768300cf0919d40dde1cbe1c4b702 Mon Sep 17 00:00:00 2001
From: cyan33 <cyan.binary@gmail.com>
Date: Thu, 23 Apr 2020 14:24:48 -0700
Subject: [PATCH 4/5] format :(

---
 .../__tests__/ESLintRulesOfHooks-test.js                        | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
index e2d0f60cbfec7..66325109228d2 100644
--- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
+++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
@@ -311,7 +311,7 @@ const tests = {
         if (c) {} else {}
         if (c) {} else {}
         if (c) {} else {}
-        
+
         // 10 hooks
         useHook();
         useHook();

From 7e3671a71319777ec16af937073549cb5f3bdcb0 Mon Sep 17 00:00:00 2001
From: cyan33 <cyan.binary@gmail.com>
Date: Thu, 23 Apr 2020 17:28:40 -0700
Subject: [PATCH 5/5] fix nits

---
 .../__tests__/ESLintRulesOfHooks-test.js            | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
index 66325109228d2..12c4f8ffb71fd 100644
--- a/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
+++ b/packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
@@ -380,6 +380,8 @@ const tests = {
     },
     {
       code: `
+        // This is a false positive (it's valid) that unfortunately 
+        // we cannot avoid. Prefer to rename it to not start with "use"
         class Foo extends Component {
           render() {
             if (cond) {
@@ -402,17 +404,6 @@ const tests = {
       `,
       errors: [conditionalError('Namespace.useConditionalHook')],
     },
-    {
-      code: `
-        // Extending the isHook check to [A-Z].*\.use
-        // to not let it be called on top level
-        const History = require('history-2.1.2');
-        const browserHistory = History.useBasename(History.createHistory)({
-          basename: '/',
-        });
-      `,
-      errors: [topLevelError('History.useBasename')],
-    },
     {
       code: `
         // Invalid because it's dangerous and might not warn otherwise.