Skip to content

Commit

Permalink
Merge branch 'feature/no-proxy-apis'
Browse files Browse the repository at this point in the history
  • Loading branch information
joshwilsonvu committed Nov 1, 2022
2 parents 197dbb5 + c20c14c commit 2b772f5
Show file tree
Hide file tree
Showing 8 changed files with 313 additions and 8 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
114 changes: 114 additions & 0 deletions docs/no-proxy-apis.md
Original file line number Diff line number Diff line change
@@ -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 -->
2 changes: 1 addition & 1 deletion docs/prefer-classlist.md
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
6 changes: 6 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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,
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
},
Expand Down
91 changes: 91 additions & 0 deletions src/rules/no-proxy-apis.ts
Original file line number Diff line number Diff line change
@@ -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;
36 changes: 30 additions & 6 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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 =>
Expand Down Expand Up @@ -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 };
};
Expand Down
69 changes: 69 additions & 0 deletions test/rules/no-proxy-apis.test.ts
Original file line number Diff line number Diff line change
@@ -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" }],
},
],
});
2 changes: 1 addition & 1 deletion test/rules/reactivity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
},
Expand Down

0 comments on commit 2b772f5

Please sign in to comment.