Explorar o código

feat(effect-zod): translate Schema.check filters into zod .superRefine + promote LSP refinement to Effect layer (#23173)

Kit Langton hai 3 días
pai
achega
e7686dbd64

+ 20 - 18
packages/opencode/src/config/lsp.ts

@@ -20,24 +20,26 @@ export const Entry = Schema.Union([
   }),
 ]).pipe(withStatics((s) => ({ zod: zod(s) })))
 
-export const Info = Schema.Union([Schema.Boolean, Schema.Record(Schema.String, Entry)]).pipe(
-  withStatics((s) => ({
-    zod: zod(s).refine(
-      (data) => {
-        if (typeof data === "boolean") return true
-        const serverIds = new Set(Object.values(LSPServer).map((server) => server.id))
-
-        return Object.entries(data).every(([id, config]) => {
-          if (config.disabled) return true
-          if (serverIds.has(id)) return true
-          return Boolean(config.extensions)
-        })
-      },
-      {
-        error: "For custom LSP servers, 'extensions' array is required.",
-      },
-    ),
-  })),
+/**
+ * For custom (non-builtin) LSP server entries, `extensions` is required so the
+ * client knows which files the server should attach to. Builtin server IDs and
+ * explicitly disabled entries are exempt.
+ */
+export const requiresExtensionsForCustomServers = Schema.makeFilter<boolean | Record<string, Schema.Schema.Type<typeof Entry>>>(
+  (data) => {
+    if (typeof data === "boolean") return undefined
+    const serverIds = new Set(Object.values(LSPServer).map((server) => server.id))
+    const ok = Object.entries(data).every(([id, config]) => {
+      if ("disabled" in config && config.disabled) return true
+      if (serverIds.has(id)) return true
+      return "extensions" in config && Boolean(config.extensions)
+    })
+    return ok ? undefined : "For custom LSP servers, 'extensions' array is required."
+  },
 )
 
+export const Info = Schema.Union([Schema.Boolean, Schema.Record(Schema.String, Entry)])
+  .check(requiresExtensionsForCustomServers)
+  .pipe(withStatics((s) => ({ zod: zod(s) })))
+
 export type Info = Schema.Schema.Type<typeof Info>

+ 22 - 1
packages/opencode/src/util/effect-zod.ts

@@ -16,13 +16,34 @@ function walk(ast: SchemaAST.AST): z.ZodTypeAny {
   const override = (ast.annotations as any)?.[ZodOverride] as z.ZodTypeAny | undefined
   if (override) return override
 
-  const out = body(ast)
+  let out = body(ast)
+  for (const check of ast.checks ?? []) {
+    out = applyCheck(out, check, ast)
+  }
   const desc = SchemaAST.resolveDescription(ast)
   const ref = SchemaAST.resolveIdentifier(ast)
   const next = desc ? out.describe(desc) : out
   return ref ? next.meta({ ref }) : next
 }
 
+function applyCheck(out: z.ZodTypeAny, check: SchemaAST.Check<any>, ast: SchemaAST.AST): z.ZodTypeAny {
+  if (check._tag === "FilterGroup") {
+    return check.checks.reduce((acc, sub) => applyCheck(acc, sub, ast), out)
+  }
+  return out.superRefine((value, ctx) => {
+    const issue = check.run(value, ast, {} as any)
+    if (!issue) return
+    const message = issueMessage(issue) ?? (check.annotations as any)?.message ?? "Validation failed"
+    ctx.addIssue({ code: "custom", message })
+  })
+}
+
+function issueMessage(issue: any): string | undefined {
+  if (typeof issue?.annotations?.message === "string") return issue.annotations.message
+  if (typeof issue?.message === "string") return issue.message
+  return undefined
+}
+
 function body(ast: SchemaAST.AST): z.ZodTypeAny {
   if (SchemaAST.isOptional(ast)) return opt(ast)
 

+ 87 - 0
packages/opencode/test/config/lsp.test.ts

@@ -0,0 +1,87 @@
+import { describe, expect, test } from "bun:test"
+import { Schema } from "effect"
+import { ConfigLSP } from "../../src/config/lsp"
+
+// The LSP config refinement enforces: any custom (non-builtin) LSP server
+// entry must declare an `extensions` array so the client knows which files
+// the server should attach to. Builtin server IDs and explicitly disabled
+// entries are exempt.
+//
+// Both validation paths must honor this rule:
+//   - `Schema.decodeUnknownSync(ConfigLSP.Info)` (Effect layer)
+//   - `ConfigLSP.Info.zod.parse(...)`            (derived Zod)
+//
+// `typescript` is a builtin server id (see src/lsp/server.ts).
+describe("ConfigLSP.Info refinement", () => {
+  const decodeEffect = Schema.decodeUnknownSync(ConfigLSP.Info)
+
+  describe("accepted inputs", () => {
+    test("true and false pass (top-level toggle)", () => {
+      expect(decodeEffect(true)).toBe(true)
+      expect(decodeEffect(false)).toBe(false)
+      expect(ConfigLSP.Info.zod.parse(true)).toBe(true)
+      expect(ConfigLSP.Info.zod.parse(false)).toBe(false)
+    })
+
+    test("builtin server with no extensions passes", () => {
+      const input = { typescript: { command: ["typescript-language-server", "--stdio"] } }
+      expect(decodeEffect(input)).toEqual(input)
+      expect(ConfigLSP.Info.zod.parse(input)).toEqual(input)
+    })
+
+    test("custom server WITH extensions passes", () => {
+      const input = {
+        "my-lsp": { command: ["my-lsp-bin"], extensions: [".ml"] },
+      }
+      expect(decodeEffect(input)).toEqual(input)
+      expect(ConfigLSP.Info.zod.parse(input)).toEqual(input)
+    })
+
+    test("disabled custom server passes (no extensions needed)", () => {
+      const input = { "my-lsp": { disabled: true as const } }
+      expect(decodeEffect(input)).toEqual(input)
+      expect(ConfigLSP.Info.zod.parse(input)).toEqual(input)
+    })
+
+    test("mix of builtin and custom with extensions passes", () => {
+      const input = {
+        typescript: { command: ["typescript-language-server", "--stdio"] },
+        "my-lsp": { command: ["my-lsp-bin"], extensions: [".ml"] },
+      }
+      expect(decodeEffect(input)).toEqual(input)
+      expect(ConfigLSP.Info.zod.parse(input)).toEqual(input)
+    })
+  })
+
+  describe("rejected inputs", () => {
+    const expectedMessage = "For custom LSP servers, 'extensions' array is required."
+
+    test("custom server WITHOUT extensions fails via Effect decode", () => {
+      expect(() => decodeEffect({ "my-lsp": { command: ["my-lsp-bin"] } })).toThrow(expectedMessage)
+    })
+
+    test("custom server WITHOUT extensions fails via derived Zod", () => {
+      const result = ConfigLSP.Info.zod.safeParse({ "my-lsp": { command: ["my-lsp-bin"] } })
+      expect(result.success).toBe(false)
+      expect(result.error!.issues.some((i) => i.message === expectedMessage)).toBe(true)
+    })
+
+    test("custom server with empty extensions array fails (extensions must be non-empty-truthy)", () => {
+      // Boolean(['']) is true, so a non-empty array of strings is fine.
+      // Boolean([]) is also true in JS, so empty arrays are accepted by the
+      // refinement. This test documents current behavior.
+      const input = { "my-lsp": { command: ["my-lsp-bin"], extensions: [] } }
+      expect(decodeEffect(input)).toEqual(input)
+      expect(ConfigLSP.Info.zod.parse(input)).toEqual(input)
+    })
+
+    test("custom server without extensions mixed with a valid builtin still fails", () => {
+      const input = {
+        typescript: { command: ["typescript-language-server", "--stdio"] },
+        "my-lsp": { command: ["my-lsp-bin"] },
+      }
+      expect(() => decodeEffect(input)).toThrow(expectedMessage)
+      expect(ConfigLSP.Info.zod.safeParse(input).success).toBe(false)
+    })
+  })
+})

+ 61 - 0
packages/opencode/test/util/effect-zod.test.ts

@@ -186,4 +186,65 @@ describe("util.effect-zod", () => {
     const schema = json(zod(Parent)) as any
     expect(schema.properties.sessionID).toEqual({ type: "string", pattern: "^ses.*" })
   })
+
+  describe("Schema.check translation", () => {
+    test("filter returning string triggers refinement with that message", () => {
+      const isEven = Schema.makeFilter((n: number) =>
+        n % 2 === 0 ? undefined : "expected an even number",
+      )
+      const schema = zod(Schema.Number.check(isEven))
+
+      expect(schema.parse(4)).toBe(4)
+      const result = schema.safeParse(3)
+      expect(result.success).toBe(false)
+      expect(result.error!.issues[0].message).toBe("expected an even number")
+    })
+
+    test("filter returning false triggers refinement with fallback message", () => {
+      const nonEmpty = Schema.makeFilter((s: string) => s.length > 0)
+      const schema = zod(Schema.String.check(nonEmpty))
+
+      expect(schema.parse("hi")).toBe("hi")
+      const result = schema.safeParse("")
+      expect(result.success).toBe(false)
+      expect(result.error!.issues[0].message).toMatch(/./)
+    })
+
+    test("filter returning undefined passes validation", () => {
+      const alwaysOk = Schema.makeFilter(() => undefined)
+      const schema = zod(Schema.Number.check(alwaysOk))
+
+      expect(schema.parse(42)).toBe(42)
+    })
+
+    test("annotations.message on the filter is used when filter returns false", () => {
+      const positive = Schema.makeFilter(
+        (n: number) => n > 0,
+        { message: "must be positive" },
+      )
+      const schema = zod(Schema.Number.check(positive))
+
+      const result = schema.safeParse(-1)
+      expect(result.success).toBe(false)
+      expect(result.error!.issues[0].message).toBe("must be positive")
+    })
+
+    test("cross-field check on a record flags missing key", () => {
+      const hasKey = Schema.makeFilter(
+        (data: Record<string, { enabled: boolean }>) =>
+          "required" in data ? undefined : "missing 'required' key",
+      )
+      const schema = zod(
+        Schema.Record(Schema.String, Schema.Struct({ enabled: Schema.Boolean })).check(hasKey),
+      )
+
+      expect(schema.parse({ required: { enabled: true } })).toEqual({
+        required: { enabled: true },
+      })
+
+      const result = schema.safeParse({ other: { enabled: true } })
+      expect(result.success).toBe(false)
+      expect(result.error!.issues[0].message).toBe("missing 'required' key")
+    })
+  })
 })