History

Enforce WorkOS Vault style rules with oxlint (#481)

Add seven repo-local oxlint rules scoped to packages/plugins/workos-vault/src to keep the provider's Effect-only surface, tagged-error discipline, and typed boundary patterns in place. Rules added: - no-inline-object-type-assertion - no-instanceof-tagged-error - no-manual-tag-check - no-promise-client-surface - no-raw-error-throw - no-redundant-error-factory - no-unknown-shape-probing Adapted to the existing flat scripts/oxlint-plugin-executor/utils.js layout on main; no rules already on main were duplicated. Cleanup in workos-vault to satisfy the new rules: - WorkOSVaultClientError now captures status + message at construction so callers consume typed fields instead of probing unknown causes. - Renamed exported WorkOSVaultSdk to module-private WorkOSVaultPromiseApi and removed it from the public package surface. - Replaced unwrapVaultError + inline shape probes in secret-store with direct typed access on WorkOSVaultClientError. - Introduced a named row type in secret-store.test.ts instead of an inline object type assertion.

Rhys Sullivan committed 6 months ago

Commit e3ee27f1 parent c5d93a314 files · +399 −60Browse files

Showing the changes to scripts (9 of 14 files). Show all

Unified · Split

9 files
  1. scripts/oxlint-plugin-executor.js +14 −0
  2. scripts/oxlint-plugin-executor/rules/no-inline-object-type-assertion.js +40 −0
  3. scripts/oxlint-plugin-executor/rules/no-instanceof-tagged-error.js +27 −0
  4. scripts/oxlint-plugin-executor/rules/no-manual-tag-check.js +25 −0
  5. scripts/oxlint-plugin-executor/rules/no-promise-client-surface.js +42 −0
  6. scripts/oxlint-plugin-executor/rules/no-raw-error-throw.js +24 −0
  7. scripts/oxlint-plugin-executor/rules/no-redundant-error-factory.js +52 −0
  8. scripts/oxlint-plugin-executor/rules/no-unknown-shape-probing.js +32 −0
  9. scripts/oxlint-plugin-executor/utils.js +57 −0
scripts/oxlint-plugin-executor.js+14−0
@@ -2,7 +2,14 @@ import noConditionalTests from "./oxlint-plugin-executor/rules/no-conditional-te · 1 unmodified line
22 import noCrossPackageRelativeImports from "./oxlint-plugin-executor/rules/no-cross-package-relative-imports.js";
33 import noDoubleCast from "./oxlint-plugin-executor/rules/no-double-cast.js";
44 import noEffectInternalTags from "./oxlint-plugin-executor/rules/no-effect-internal-tags.js";
5+import noInlineObjectTypeAssertion from "./oxlint-plugin-executor/rules/no-inline-object-type-assertion.js";
6+import noInstanceofTaggedError from "./oxlint-plugin-executor/rules/no-instanceof-tagged-error.js";
7+import noManualTagCheck from "./oxlint-plugin-executor/rules/no-manual-tag-check.js";
8+import noPromiseClientSurface from "./oxlint-plugin-executor/rules/no-promise-client-surface.js";
9+import noRawErrorThrow from "./oxlint-plugin-executor/rules/no-raw-error-throw.js";
10+import noRedundantErrorFactory from "./oxlint-plugin-executor/rules/no-redundant-error-factory.js";
511 import noTsNocheck from "./oxlint-plugin-executor/rules/no-ts-nocheck.js";
12+import noUnknownShapeProbing from "./oxlint-plugin-executor/rules/no-unknown-shape-probing.js";
613 import noVitestImport from "./oxlint-plugin-executor/rules/no-vitest-import.js";
714 import requireReactivityKeys from "./oxlint-plugin-executor/rules/require-reactivity-keys.js";
815
@@ -18,5 +25,12 @@ export default { · 9 unmodified lines
1825 "require-reactivity-keys": requireReactivityKeys,
1926 "no-effect-internal-tags": noEffectInternalTags,
2027 "no-ts-nocheck": noTsNocheck,
28+ "no-inline-object-type-assertion": noInlineObjectTypeAssertion,
29+ "no-instanceof-tagged-error": noInstanceofTaggedError,
30+ "no-manual-tag-check": noManualTagCheck,
31+ "no-promise-client-surface": noPromiseClientSurface,
32+ "no-raw-error-throw": noRawErrorThrow,
33+ "no-redundant-error-factory": noRedundantErrorFactory,
34+ "no-unknown-shape-probing": noUnknownShapeProbing,
2135 },
2236 };
scripts/oxlint-plugin-executor/rules/no-inline-object-type-assertion.js+40−0
@@ -0,0 +1,40 @@
1+import { isIdentifier } from "../utils.js";
2+
3+const message =
4+ "Do not assert against inline object-shaped types. Use a named type, Schema, or a proper type guard.";
5+
6+const isUnknownKeyword = (node) => node?.type === "TSUnknownKeyword";
7+
8+const isStringKey = (node) =>
9+ node?.type === "TSStringKeyword" ||
10+ (node?.type === "TSLiteralType" && typeof node.literal?.value === "string");
11+
12+const isRecordUnknown = (node) =>
13+ node?.type === "TSTypeReference" &&
14+ isIdentifier(node.typeName, "Record") &&
15+ node.typeArguments?.params?.length === 2 &&
16+ isStringKey(node.typeArguments.params[0]) &&
17+ isUnknownKeyword(node.typeArguments.params[1]);
18+
19+const isBannedType = (node) => node?.type === "TSTypeLiteral" || isRecordUnknown(node);
20+
21+export default {
22+ meta: {
23+ type: "problem",
24+ docs: {
25+ description: message,
26+ },
27+ },
28+ create(context) {
29+ const check = (node) => {
30+ if (isBannedType(node.typeAnnotation)) {
31+ context.report({ node, message });
32+ }
33+ };
34+
35+ return {
36+ TSAsExpression: check,
37+ TSTypeAssertion: check,
38+ };
39+ },
40+};
scripts/oxlint-plugin-executor/rules/no-instanceof-tagged-error.js+27−0
@@ -0,0 +1,27 @@
1+import { isIdentifier, nodeName } from "../utils.js";
2+
3+const message =
4+ "Do not use instanceof for tagged errors. Use Effect.catchTag, Effect.catchTags, or a _tag-based guard.";
5+
6+const looksLikeTaggedErrorName = (name) =>
7+ typeof name === "string" && name !== "Error" && name.endsWith("Error");
8+
9+export default {
10+ meta: {
11+ type: "problem",
12+ docs: {
13+ description: message,
14+ },
15+ },
16+ create(context) {
17+ return {
18+ BinaryExpression(node) {
19+ if (node.operator !== "instanceof") return;
20+ const rightName = nodeName(node.right);
21+ if (isIdentifier(node.right) && looksLikeTaggedErrorName(rightName)) {
22+ context.report({ node, message });
23+ }
24+ },
25+ };
26+ },
27+};
scripts/oxlint-plugin-executor/rules/no-manual-tag-check.js+25−0
@@ -0,0 +1,25 @@
1+import { isIdentifier, isStringLiteral } from "../utils.js";
2+
3+const message =
4+ "Do not inspect _tag manually. Use Effect.catchTag, Effect.catchTags, Predicate.isTagged, or another Effect tagged-error API.";
5+
6+const isTagProperty = (node) =>
7+ isIdentifier(node, "_tag") || (isStringLiteral(node) && node.value === "_tag");
8+
9+export default {
10+ meta: {
11+ type: "problem",
12+ docs: {
13+ description: message,
14+ },
15+ },
16+ create(context) {
17+ return {
18+ MemberExpression(node) {
19+ if (isTagProperty(node.property)) {
20+ context.report({ node, message });
21+ }
22+ },
23+ };
24+ },
25+};
scripts/oxlint-plugin-executor/rules/no-promise-client-surface.js+42−0
@@ -0,0 +1,42 @@
1+import { containsPromiseType, nodeName } from "../utils.js";
2+
3+const message =
4+ "Do not expose Promise-shaped client surfaces. Wrap third-party SDK promises at the adapter boundary and expose Effect methods.";
5+
6+const isExported = (node) => node?.parent?.type === "ExportNamedDeclaration";
7+
8+const isClientInterface = (node) => {
9+ const name = nodeName(node.id);
10+ return (
11+ typeof name === "string" &&
12+ (name.endsWith("Client") || (isExported(node) && name.endsWith("Sdk")))
13+ );
14+};
15+
16+const methodReturnsPromise = (node) => containsPromiseType(node.returnType);
17+
18+const propertyReturnsPromise = (node) => containsPromiseType(node.typeAnnotation);
19+
20+export default {
21+ meta: {
22+ type: "problem",
23+ docs: {
24+ description: message,
25+ },
26+ },
27+ create(context) {
28+ return {
29+ TSInterfaceDeclaration(node) {
30+ if (!isClientInterface(node)) return;
31+ for (const member of node.body?.body ?? []) {
32+ if (
33+ (member.type === "TSMethodSignature" && methodReturnsPromise(member)) ||
34+ (member.type === "TSPropertySignature" && propertyReturnsPromise(member))
35+ ) {
36+ context.report({ node: member, message });
37+ }
38+ }
39+ },
40+ };
41+ },
42+};
scripts/oxlint-plugin-executor/rules/no-raw-error-throw.js+24−0
@@ -0,0 +1,24 @@
1+import { isIdentifier } from "../utils.js";
2+
3+const message =
4+ "Do not throw raw Error objects in Effect code. Return Effect.fail with a tagged error or assert directly in tests.";
5+
6+const isNewError = (node) => node?.type === "NewExpression" && isIdentifier(node.callee, "Error");
7+
8+export default {
9+ meta: {
10+ type: "problem",
11+ docs: {
12+ description: message,
13+ },
14+ },
15+ create(context) {
16+ return {
17+ ThrowStatement(node) {
18+ if (isNewError(node.argument)) {
19+ context.report({ node, message });
20+ }
21+ },
22+ };
23+ },
24+};
scripts/oxlint-plugin-executor/rules/no-redundant-error-factory.js+52−0
@@ -0,0 +1,52 @@
1+import { isIdentifier } from "../utils.js";
2+
3+const message =
4+ "Do not add redundant make*Error wrappers that only construct a tagged error. Construct the tagged error directly.";
5+
6+const isErrorFactoryName = (name) => /^make[A-Z].*Error$/.test(name);
7+
8+const isNewErrorExpression = (node) =>
9+ node?.type === "NewExpression" && isIdentifier(node.callee) && node.callee.name.endsWith("Error");
10+
11+const returnsOnlyNewError = (node) => {
12+ if (isNewErrorExpression(node)) return true;
13+ if (node?.type !== "BlockStatement") return false;
14+ const statements = node.body ?? [];
15+ return (
16+ statements.length === 1 &&
17+ statements[0]?.type === "ReturnStatement" &&
18+ isNewErrorExpression(statements[0].argument)
19+ );
20+};
21+
22+const reportIfRedundantFactory = (context, name, body, node) => {
23+ if (isErrorFactoryName(name) && returnsOnlyNewError(body)) {
24+ context.report({ node, message });
25+ }
26+};
27+
28+export default {
29+ meta: {
30+ type: "problem",
31+ docs: {
32+ description: message,
33+ },
34+ },
35+ create(context) {
36+ return {
37+ FunctionDeclaration(node) {
38+ reportIfRedundantFactory(context, node.id?.name, node.body, node);
39+ },
40+ VariableDeclarator(node) {
41+ if (!isIdentifier(node.id)) return;
42+ if (
43+ node.init?.type !== "ArrowFunctionExpression" &&
44+ node.init?.type !== "FunctionExpression"
45+ ) {
46+ return;
47+ }
48+ reportIfRedundantFactory(context, node.id.name, node.init.body, node);
49+ },
50+ };
51+ },
52+};
scripts/oxlint-plugin-executor/rules/no-unknown-shape-probing.js+32−0
@@ -0,0 +1,32 @@
1+import { isIdentifier, isStringLiteral } from "../utils.js";
2+
3+const message =
4+ "Do not probe unknown object shapes in domain code. Normalize at a boundary with Schema, a typed adapter, or a named guard.";
5+
6+const isReflectGet = (node) =>
7+ node?.type === "MemberExpression" &&
8+ isIdentifier(node.object, "Reflect") &&
9+ isIdentifier(node.property, "get");
10+
11+export default {
12+ meta: {
13+ type: "problem",
14+ docs: {
15+ description: message,
16+ },
17+ },
18+ create(context) {
19+ return {
20+ CallExpression(node) {
21+ if (isReflectGet(node.callee)) {
22+ context.report({ node, message });
23+ }
24+ },
25+ BinaryExpression(node) {
26+ if (node.operator === "in" && isStringLiteral(node.left)) {
27+ context.report({ node, message });
28+ }
29+ },
30+ };
31+ },
32+};
scripts/oxlint-plugin-executor/utils.js+57−0
@@ -71,3 +71,60 @@ export function getStringValue(node) { · 70 unmodified lines
7171 if (expression?.type === "StringLiteral") return expression.value;
7272 return undefined;
7373 }
74+
75+export function isIdentifier(node, name) {
76+ return node?.type === "Identifier" && (name === undefined || node.name === name);
77+}
78+
79+export function isStringLiteral(node) {
80+ return (
81+ (node?.type === "Literal" && typeof node.value === "string") ||
82+ node?.type === "StringLiteral"
83+ );
84+}
85+
86+export function typeName(node) {
87+ if (node?.type === "Identifier") return node.name;
88+ if (node?.type === "TSQualifiedName") {
89+ const left = typeName(node.left);
90+ const right = typeName(node.right);
91+ return left && right ? `${left}.${right}` : undefined;
92+ }
93+ return undefined;
94+}
95+
96+export function typeReferenceName(node) {
97+ return node?.type === "TSTypeReference" ? typeName(node.typeName) : undefined;
98+}
99+
100+export function isPromiseType(node) {
101+ return typeReferenceName(node) === "Promise";
102+}
103+
104+export function containsPromiseType(node) {
105+ if (!node || typeof node !== "object") return false;
106+ if (isPromiseType(node)) return true;
107+
108+ switch (node.type) {
109+ case "TSTypeAnnotation":
110+ return containsPromiseType(node.typeAnnotation);
111+ case "TSFunctionType":
112+ return containsPromiseType(node.returnType);
113+ case "TSParenthesizedType":
114+ return containsPromiseType(node.typeAnnotation);
115+ case "TSUnionType":
116+ case "TSIntersectionType":
117+ return (node.types ?? []).some(containsPromiseType);
118+ case "TSConditionalType":
119+ return containsPromiseType(node.trueType) || containsPromiseType(node.falseType);
120+ default:
121+ return false;
122+ }
123+}
124+
125+export function nodeName(node) {
126+ if (isIdentifier(node)) return node.name;
127+ if (node?.type === "PrivateIdentifier") return node.name;
128+ if (isStringLiteral(node)) return node.value;
129+ return undefined;
130+}