Merge branch 'feature/no-proxy-apis'

joshwilsonvu · joshwilsonvu · commit 2b772f53ed12 · 2022-11-01T12:24:56.000-04:00
diff --git a/README.md b/README.md
@@ -130,6 +130,7 @@ const [editedValue, setEditedValue] = createSignal(props.value);
 | ✔ |  | [solid/jsx-uses-vars](docs/jsx-uses-vars.md) | Prevent variables used in JSX from being marked as unused. |
 | ✔ | 🔧 | [solid/no-destructure](docs/no-destructure.md) | Disallow destructuring props. In Solid, props must be used with property accesses (`props.foo`) to preserve reactivity. This rule only tracks destructuring in the parameter list. |
 | ✔ | 🔧 | [solid/no-innerhtml](docs/no-innerhtml.md) | Disallow usage of the innerHTML attribute, which can often lead to security vulnerabilities. |
+| ✔ |  | [solid/no-proxy-apis](docs/no-proxy-apis.md) | Disallow usage of APIs that use ES6 Proxies, only to target environments that don't support them. |
 | ✔ | 🔧 | [solid/no-react-specific-props](docs/no-react-specific-props.md) | Disallow usage of React-specific `className`/`htmlFor` props, which were deprecated in v1.4.0. |
 | ✔ |  | [solid/no-unknown-namespaces](docs/no-unknown-namespaces.md) | Enforce using only Solid-specific namespaced attribute names (i.e. `'on:'` in `<div on:click={...} />`). |
 | ✔ | 🔧 | [solid/prefer-classlist](docs/prefer-classlist.md) | Enforce using the classlist prop over importing a classnames helper. The classlist prop accepts an object `{ [class: string]: boolean }` just like classnames. |
diff --git a/docs/no-proxy-apis.md b/docs/no-proxy-apis.md
@@ -0,0 +1,114 @@
+<!-- AUTO-GENERATED-CONTENT:START (HEADER) -->
+# solid/no-proxy-apis
+Disallow usage of APIs that use ES6 Proxies, only to target environments that don't support them.
+This rule is **off** by default.
+
+[View source](../src/rules/no-proxy-apis.ts) · [View tests](../test/rules/no-proxy-apis.test.ts)
+
+<!-- AUTO-GENERATED-CONTENT:END -->
+
+<!-- AUTO-GENERATED-CONTENT:START (OPTIONS) -->
+ 
+<!-- AUTO-GENERATED-CONTENT:END -->
+
+<!-- AUTO-GENERATED-CONTENT:START (CASES) -->
+## Tests
+
+### Invalid Examples
+
+These snippets cause lint errors, and some can be auto-fixed.
+
+```js
+let el = <div className="greeting">Hello world!</div>;
+// after eslint --fix:
+let el = <div class="greeting">Hello world!</div>;
+ 
+let el = <div className={"greeting"}>Hello world!</div>;
+// after eslint --fix:
+let el = <div class={"greeting"}>Hello world!</div>;
+ 
+let el = <div className="greeting" />;
+// after eslint --fix:
+let el = <div class="greeting" />;
+ 
+let el = (
+  <div many other attributes className="greeting">
+    Hello world!
+  </div>
+);
+// after eslint --fix:
+let el = (
+  <div many other attributes class="greeting">
+    Hello world!
+  </div>
+);
+ 
+let el = <PascalComponent className="greeting">Hello world!</PascalComponent>;
+// after eslint --fix:
+let el = <PascalComponent class="greeting">Hello world!</PascalComponent>;
+ 
+let el = <label htmlFor="id">Hello world!</label>;
+// after eslint --fix:
+let el = <label for="id">Hello world!</label>;
+ 
+let el = <label htmlFor={"id"}>Hello world!</label>;
+// after eslint --fix:
+let el = <label for={"id"}>Hello world!</label>;
+ 
+let el = (
+  <label many other attributes htmlFor="id">
+    Hello world!
+  </label>
+);
+// after eslint --fix:
+let el = (
+  <label many other attributes for="id">
+    Hello world!
+  </label>
+);
+ 
+let el = <PascalComponent htmlFor="id">Hello world!</PascalComponent>;
+// after eslint --fix:
+let el = <PascalComponent for="id">Hello world!</PascalComponent>;
+ 
+let el = <div key={item.id} />;
+// after eslint --fix:
+let el = <div />;
+ 
+```
+
+### Valid Examples
+
+These snippets don't cause lint errors.
+
+```js
+let el = <div>Hello world!</div>;
+
+let el = <div class="greeting">Hello world!</div>;
+
+let el = <div class={"greeting"}>Hello world!</div>;
+
+let el = (
+  <div many other attributes class="greeting">
+    Hello world!
+  </div>
+);
+
+let el = <label for="id">Hello world!</label>;
+
+let el = <label for="id">Hello world!</label>;
+
+let el = <label for={"id"}>Hello world!</label>;
+
+let el = (
+  <label many other attributes for="id">
+    Hello world!
+  </label>
+);
+
+let el = <PascalComponent class="greeting" for="id" />;
+
+let el = <PascalComponent key={item.id} />;
+
+```
+<!-- AUTO-GENERATED-CONTENT:END -->
diff --git a/docs/prefer-classlist.md b/docs/prefer-classlist.md
@@ -1,7 +1,7 @@
 <!-- AUTO-GENERATED-CONTENT:START (HEADER) -->
 # solid/prefer-classlist
 Enforce using the classlist prop over importing a classnames helper. The classlist prop accepts an object `{ [class: string]: boolean }` just like classnames.
-This rule is **off** by default.
+This rule is **deprecated** and **off** by default.
 
 [View source](../src/rules/prefer-classlist.ts) · [View tests](../test/rules/prefer-classlist.test.ts)
 
diff --git a/src/index.ts b/src/index.ts
@@ -7,6 +7,7 @@ import jsxNoUndef from "./rules/jsx-no-undef";
 import jsxUsesVars from "./rules/jsx-uses-vars";
 import noDestructure from "./rules/no-destructure";
 import noInnerHTML from "./rules/no-innerhtml";
+import noProxyApis from "./rules/no-proxy-apis";
 import noReactSpecificProps from "./rules/no-react-specific-props";
 import noUnknownNamespaces from "./rules/no-unknown-namespaces";
 import preferClasslist from "./rules/prefer-classlist";
@@ -27,6 +28,7 @@ const allRules = {
   "jsx-uses-vars": jsxUsesVars,
   "no-destructure": noDestructure,
   "no-innerhtml": noInnerHTML,
+  "no-proxy-apis": noProxyApis,
   "no-react-specific-props": noReactSpecificProps,
   "no-unknown-namespaces": noUnknownNamespaces,
   "prefer-classlist": preferClasslist,
@@ -78,6 +80,8 @@ const plugin = {
         "solid/self-closing-comp": 1,
         // handled by Solid compiler, opt-in style suggestion
         "solid/prefer-show": 0,
+        // only necessary for resource-constrained environments
+        "solid/no-proxy-apis": 0,
         // deprecated
         "solid/prefer-classlist": 0,
       },
@@ -111,6 +115,8 @@ const plugin = {
         "solid/no-unknown-namespaces": 0,
         // handled by Solid compiler, opt-in style suggestion
         "solid/prefer-show": 0,
+        // only necessary for resource-constrained environments
+        "solid/no-proxy-apis": 0,
         // deprecated
         "solid/prefer-classlist": 0,
       },
diff --git a/src/rules/no-proxy-apis.ts b/src/rules/no-proxy-apis.ts
@@ -0,0 +1,91 @@
+import { TSESTree as T, TSESLint } from "@typescript-eslint/utils";
+import { isFunctionNode, trackImports, isPropsByName, trace } from "../utils";
+
+const rule: TSESLint.RuleModule<
+  "noStore" | "spreadCall" | "spreadMember" | "proxyLiteral" | "mergeProps",
+  []
+> = {
+  meta: {
+    type: "problem",
+    docs: {
+      recommended: false,
+      description:
+        "Disallow usage of APIs that use ES6 Proxies, only to target environments that don't support them.",
+      url: "https://github.com/solidjs-community/eslint-plugin-solid/blob/main/docs/no-proxy-apis.md",
+    },
+    schema: [],
+    messages: {
+      noStore: "Solid Store APIs use Proxies, which are incompatible with your target environment.",
+      spreadCall:
+        "Using a function call in JSX spread makes Solid use Proxies, which are incompatible with your target environment.",
+      spreadMember:
+        "Using a property access in JSX spread makes Solid use Proxies, which are incompatible with your target environment.",
+      proxyLiteral: "Proxies are incompatible with your target environment.",
+      mergeProps:
+        "If you pass a function to `mergeProps`, it will create a Proxy, which are incompatible with your target environment.",
+    },
+  },
+  create(context) {
+    const { matchImport, handleImportDeclaration } = trackImports();
+
+    return {
+      ImportDeclaration(node) {
+        handleImportDeclaration(node); // track import aliases
+
+        const source = node.source.value;
+        if (source === "solid-js/store") {
+          context.report({
+            node,
+            messageId: "noStore",
+          });
+        }
+      },
+      "JSXSpreadAttribute MemberExpression"(node: T.MemberExpression) {
+        context.report({ node, messageId: "spreadMember" });
+      },
+      "JSXSpreadAttribute CallExpression"(node: T.CallExpression) {
+        context.report({ node, messageId: "spreadCall" });
+      },
+      CallExpression(node) {
+        if (node.callee.type === "Identifier") {
+          if (matchImport("mergeProps", node.callee.name)) {
+            node.arguments
+              .filter((arg) => {
+                if (arg.type === "SpreadElement") return true;
+                const traced = trace(arg, context.getScope());
+                return (
+                  (traced.type === "Identifier" && !isPropsByName(traced.name)) ||
+                  isFunctionNode(traced)
+                );
+              })
+              .forEach((badArg) => {
+                context.report({
+                  node: badArg,
+                  messageId: "mergeProps",
+                });
+              });
+          }
+        } else if (node.callee.type === "MemberExpression") {
+          if (
+            node.callee.object.type === "Identifier" &&
+            node.callee.object.name === "Proxy" &&
+            node.callee.property.type === "Identifier" &&
+            node.callee.property.name === "revocable"
+          ) {
+            context.report({
+              node,
+              messageId: "proxyLiteral",
+            });
+          }
+        }
+      },
+      NewExpression(node) {
+        if (node.callee.type === "Identifier" && node.callee.name === "Proxy") {
+          context.report({ node, messageId: "proxyLiteral" });
+        }
+      },
+    };
+  },
+};
+
+export default rule;
diff --git a/src/utils.ts b/src/utils.ts
@@ -1,4 +1,5 @@
-import type { TSESTree as T, TSESLint } from "@typescript-eslint/utils";
+import { TSESTree as T, TSESLint, ASTUtils } from "@typescript-eslint/utils";
+const { findVariable } = ASTUtils;
 
 const domElementRegex = /^[a-z]/;
 export const isDOMElementName = (name: string): boolean => domElementRegex.test(name);
@@ -42,6 +43,32 @@ export function findParent(node: T.Node, predicate: (node: T.Node) => boolean):
   return node.parent ? find(node.parent, predicate) : null;
 }
 
+// Try to resolve a variable to its definition
+export function trace(node: T.Node, initialScope: TSESLint.Scope.Scope): T.Node {
+  if (node.type === "Identifier") {
+    const variable = findVariable(initialScope, node);
+    if (!variable) return node;
+
+    const def = variable.defs[0];
+    switch (def.type) {
+      case "FunctionName":
+      case "ClassName":
+      case "ImportBinding":
+        return def.node;
+      case "Variable":
+        if (
+          ((def.node.parent as T.VariableDeclaration).kind === "const" ||
+            variable.references.every((ref) => ref.init || ref.isReadOnly())) &&
+          def.node.id.type === "Identifier" &&
+          def.node.init
+        ) {
+          return trace(def.node.init, initialScope);
+        }
+    }
+  }
+  return node;
+}
+
 export type FunctionNode = T.FunctionExpression | T.ArrowFunctionExpression | T.FunctionDeclaration;
 const FUNCTION_TYPES = ["FunctionExpression", "ArrowFunctionExpression", "FunctionDeclaration"];
 export const isFunctionNode = (node: T.Node | null | undefined): node is FunctionNode =>
@@ -95,12 +122,9 @@ export const trackImports = (fromModule = /^solid-js(?:\/?|\b)/) => {
       }
     }
   };
-  const matchImport = (imports: string | Array<string>, str: string) => {
+  const matchImport = (imports: string | Array<string>, str: string): string | undefined => {
     const importArr = Array.isArray(imports) ? imports : [imports];
-    return importArr
-      .map((i) => importMap.get(i))
-      .filter(Boolean)
-      .includes(str);
+    return importArr.find((i) => importMap.get(i) === str);
   };
   return { matchImport, handleImportDeclaration };
 };
diff --git a/test/rules/no-proxy-apis.test.ts b/test/rules/no-proxy-apis.test.ts
@@ -0,0 +1,69 @@
+import { AST_NODE_TYPES as T } from "@typescript-eslint/utils";
+import { run } from "../ruleTester";
+import rule from "../../src/rules/no-proxy-apis";
+
+// Don't bother checking for imports for every test
+jest.mock("../../src/utils", () => {
+  return {
+    ...jest.requireActual("../../src/utils"),
+    trackImports: () => {
+      // eslint-disable-next-line @typescript-eslint/no-empty-function
+      const handleImportDeclaration = () => {};
+      const matchImport = (imports: string | Array<string>, str: string) => {
+        const importArr = Array.isArray(imports) ? imports : [imports];
+        return importArr.find((i) => i === str);
+      };
+      return { matchImport, handleImportDeclaration };
+    },
+  };
+});
+
+export const cases = run("no-proxy-apis", rule, {
+  valid: [
+    `let merged = mergeProps({}, props);`,
+    `const obj = {}; let merged = mergeProps(obj, props);`,
+    `let obj = {}; let merged = mergeProps(obj, props);`,
+    `let merged = mergeProps({ get asdf() { signal() } }, props);`,
+    `let el = <div {...{ asdf: 'asdf' }} />`,
+    `let el = <div {...asdf} />`,
+    `let obj = { Proxy: 1 }`,
+  ],
+  invalid: [
+    {
+      code: `let proxy = new Proxy(asdf, {});`,
+      errors: [{ messageId: "proxyLiteral" }],
+    },
+    {
+      code: `let proxy = Proxy.revocable(asdf, {});`,
+      errors: [{ messageId: "proxyLiteral" }],
+    },
+    {
+      code: `import {} from 'solid-js/store';`,
+      errors: [{ messageId: "noStore", type: T.ImportDeclaration }],
+    },
+    {
+      code: `let el = <div {...maybeSignal()} />`,
+      errors: [{ messageId: "spreadCall" }],
+    },
+    {
+      code: `let el = <div {...{ ...maybeSignal() }} />`,
+      errors: [{ messageId: "spreadCall" }],
+    },
+    {
+      code: `let el = <div {...maybeProps.foo} />`,
+      errors: [{ messageId: "spreadMember" }],
+    },
+    {
+      code: `let el = <div {...{ ...maybeProps.foo }} />`,
+      errors: [{ messageId: "spreadMember" }],
+    },
+    {
+      code: `let merged = mergeProps(maybeSignal)`,
+      errors: [{ messageId: "mergeProps" }],
+    },
+    {
+      code: `let func = () => ({}); let merged = mergeProps(func, props)`,
+      errors: [{ messageId: "mergeProps" }],
+    },
+  ],
+});
diff --git a/test/rules/reactivity.test.ts b/test/rules/reactivity.test.ts
@@ -11,7 +11,7 @@ jest.mock("../../src/utils", () => {
       const handleImportDeclaration = () => {};
       const matchImport = (imports: string | Array<string>, str: string) => {
         const importArr = Array.isArray(imports) ? imports : [imports];
-        return importArr.includes(str);
+        return importArr.find((i) => i === str);
       };
       return { matchImport, handleImportDeclaration };
     },
