fix: tighten tagged error linting

packages/core/src/npm.ts

@@ -11,7 +11,6 @@ import { LayerNode } from "./effect/layer-node"
 import { filesystem } from "./effect/layer-node-platform"
 import { makeRuntime } from "./effect/runtime"
 import { NpmConfig } from "./npm-config"
-import { errorMessage } from "./util/error"
 
 export class InstallFailedError extends Schema.TaggedErrorClass<InstallFailedError>()("NpmInstallFailedError", {
   add: Schema.Array(Schema.String).pipe(Schema.optional),
@@ -19,8 +18,7 @@ export class InstallFailedError extends Schema.TaggedErrorClass<InstallFailedErr
   cause: Schema.optional(Schema.Defect()),
 }) {
   override get message() {
-    const detail = this.cause === undefined ? undefined : errorMessage(this.cause)
-    return `Failed to install ${this.add?.join(", ") || "dependencies"} in ${this.dir}${detail ? `: ${detail}` : ""}`
+    return `Failed to install dependencies in ${this.dir}`
   }
 }
 

packages/opencode/src/skill/index.ts

@@ -60,7 +60,7 @@ function isSkillFrontmatter(data: unknown): data is { name: string; description?
 
 export class InvalidError extends Schema.TaggedErrorClass<InvalidError>()("SkillInvalidError", {
   path: Schema.String,
-  message: Schema.optional(Schema.String),
+  message: Schema.String,
   issues: Schema.optional(Schema.Array(Issue)),
 }) {}
 

script/lint/opencode.mjs

@@ -10,47 +10,78 @@ const taggedErrorMessage = {
     schema: [],
   },
   create(context) {
-    const schemas = new Set()
-
     function visitClass(node) {
-      const fields = taggedErrorFields(node.superClass, schemas)
-      if (
-        !fields ||
-        fields.properties.some((property) => property.type === "SpreadElement" || property.computed)
-      )
-        return
-      if (fields.properties.some((property) => propertyName(property) === "message")) return
+      const fields = taggedErrorFields(context, node.superClass)
+      if (!fields) return
+      if (fields.properties.some(hasRequiredMessage)) return
       if (node.body.body.some(hasInstanceMessage)) return
       context.report({ node, messageId: "missing" })
     }
 
     return {
-      ImportDeclaration(node) {
-        if (node.source.value !== "effect" && node.source.value !== "effect/Schema") return
-        for (const specifier of node.specifiers) {
-          if (specifier.type === "ImportSpecifier" && specifier.imported.name === "Schema") schemas.add(specifier.local.name)
-          if (specifier.type === "ImportNamespaceSpecifier" && node.source.value === "effect/Schema")
-            schemas.add(specifier.local.name)
-        }
-      },
       ClassDeclaration: visitClass,
       ClassExpression: visitClass,
     }
   },
 }
 
-function taggedErrorFields(superClass, schemas) {
+function taggedErrorFields(context, superClass) {
   if (superClass?.type !== "CallExpression" || superClass.arguments.length < 2) return null
   if (superClass.arguments[1].type !== "ObjectExpression") return null
   const factory = superClass.callee
   if (factory.type !== "CallExpression" || factory.arguments.length !== 0) return null
   const taggedError = factory.callee
   if (taggedError.type !== "MemberExpression" || taggedError.computed) return null
-  if (taggedError.object.type !== "Identifier" || !schemas.has(taggedError.object.name)) return null
+  if (taggedError.object.type !== "Identifier" || !isEffectSchema(context, taggedError.object)) return null
   if (taggedError.property.type !== "Identifier" || taggedError.property.name !== "TaggedErrorClass") return null
   return superClass.arguments[1]
 }
 
+function isEffectSchema(context, identifier) {
+  const variable = findVariable(context.sourceCode.getScope(identifier), identifier.name)
+  if (!variable || variable.defs.length !== 1 || variable.defs[0].type !== "ImportBinding") return false
+  const definition = variable.defs[0]
+  if (definition.parent.source.value === "effect")
+    return definition.node.type === "ImportSpecifier" && definition.node.imported.name === "Schema"
+  return definition.parent.source.value === "effect/Schema" && definition.node.type === "ImportNamespaceSpecifier"
+}
+
+function findVariable(scope, name) {
+  return scope.set.get(name) ?? (scope.upper ? findVariable(scope.upper, name) : null)
+}
+
+function hasRequiredMessage(property) {
+  if (propertyName(property) !== "message" || property.type !== "Property") return false
+  return isStringSchema(property.value) && !isOptional(property.value)
+}
+
+function isStringSchema(node) {
+  if (node.type === "MemberExpression")
+    return (
+      node.object.type === "Identifier" &&
+      node.object.name === "Schema" &&
+      node.property.type === "Identifier" &&
+      node.property.name === "String"
+    )
+  if (node.type !== "CallExpression" || node.callee.type !== "MemberExpression") return false
+  return isStringSchema(node.callee.object)
+}
+
+function isOptional(node) {
+  if (node.type !== "CallExpression" || node.callee.type !== "MemberExpression") return false
+  if (node.callee.object.type === "Identifier" && node.callee.object.name === "Schema")
+    return node.callee.property.type === "Identifier" && node.callee.property.name === "optional"
+  if (node.callee.property.type !== "Identifier" || node.callee.property.name !== "pipe") return false
+  return node.arguments.some(
+    (argument) =>
+      argument.type === "MemberExpression" &&
+      argument.object.type === "Identifier" &&
+      argument.object.name === "Schema" &&
+      argument.property.type === "Identifier" &&
+      argument.property.name === "optional",
+  )
+}
+
 function propertyName(property) {
   if (property.type !== "Property" && property.type !== "PropertyDefinition" && property.type !== "MethodDefinition")
     return null
@@ -61,8 +92,15 @@ function propertyName(property) {
 
 function hasInstanceMessage(member) {
   if (member.static || propertyName(member) !== "message") return false
-  if (member.type === "MethodDefinition") return member.kind === "get"
-  return member.type === "PropertyDefinition" && member.value !== null
+  if (member.type === "PropertyDefinition") return member.value !== null && !isEmptyMessage(member.value)
+  if (member.type !== "MethodDefinition" || member.kind !== "get") return false
+  if (member.value.body.body.length === 0) return false
+  const direct = member.value.body.body.length === 1 ? member.value.body.body[0] : null
+  return direct?.type !== "ReturnStatement" || (direct.argument !== null && !isEmptyMessage(direct.argument))
+}
+
+function isEmptyMessage(node) {
+  return (node.type === "Identifier" && node.name === "undefined") || (node.type === "Literal" && !node.value)
 }
 
 export default {

script/lint/test.ts

@@ -32,23 +32,44 @@ export namespace Example { export class Error extends Schema.TaggedErrorClass<Er
   },
   {
     name: "spread fields",
-    errors: 0,
+    errors: 1,
     source: `import { Schema } from "effect"
 const fields = { message: Schema.String }
 class Example extends Schema.TaggedErrorClass<Example>()("Example", { ...fields }) {}`,
   },
   {
     name: "computed fields",
-    errors: 0,
+    errors: 1,
     source: `import { Schema } from "effect"
 const key = "message"
 class Example extends Schema.TaggedErrorClass<Example>()("Example", { [key]: Schema.String }) {}`,
+  },
+  {
+    name: "optional message",
+    errors: 1,
+    source: `import { Schema } from "effect"
+class Example extends Schema.TaggedErrorClass<Example>()("Example", { message: Schema.optional(Schema.String) }) {}`,
+  },
+  {
+    name: "piped optional message",
+    errors: 1,
+    source: `import { Schema } from "effect"
+class Example extends Schema.TaggedErrorClass<Example>()("Example", { message: Schema.String.pipe(Schema.optional) }) {}`,
   },
   {
     name: "unrelated Schema binding",
     errors: 0,
     source: `const Schema = getSchema()
 class Example extends Schema.TaggedErrorClass()("Example", {}) {}`,
+  },
+  {
+    name: "shadowed Schema binding",
+    errors: 0,
+    source: `import { Schema } from "effect"
+function example() {
+  const Schema = getSchema()
+  return class extends Schema.TaggedErrorClass()("Example", {}) {}
+}`,
   },
   {
     name: "missing message",
@@ -67,6 +88,24 @@ class Example extends Schema.TaggedErrorClass<Example>()("Example", {}) { static
     errors: 1,
     source: `import { Schema } from "effect"
 class Example extends Schema.TaggedErrorClass<Example>()("Example", {}) { declare message: string }`,
+  },
+  {
+    name: "undefined property",
+    errors: 1,
+    source: `import { Schema } from "effect"
+class Example extends Schema.TaggedErrorClass<Example>()("Example", {}) { override message = undefined }`,
+  },
+  {
+    name: "empty getter",
+    errors: 1,
+    source: `import { Schema } from "effect"
+class Example extends Schema.TaggedErrorClass<Example>()("Example", {}) { override get message() { return "" } }`,
+  },
+  {
+    name: "non-string message schema",
+    errors: 1,
+    source: `import { Schema } from "effect"
+class Example extends Schema.TaggedErrorClass<Example>()("Example", { message: Schema.Unknown }) {}`,
   },
   {
     name: "documented disable",
@@ -88,28 +127,31 @@ try {
       rules: { "opencode/tagged-error-message": "error" },
     }),
   )
-  for (const [index, fixture] of cases.entries()) {
-    const file = path.join(directory, `${index}.ts`)
-    await Bun.write(file, fixture.source)
-    const result = Bun.spawnSync([
-      path.join(import.meta.dir, "../../node_modules/.bin/oxlint"),
-      "--config",
-      config,
-      "--format",
-      "json",
-      file,
-    ])
-    if (!result.stdout.length) throw new Error(`${fixture.name}: ${result.stderr.toString()}`)
-    const diagnostics: unknown = JSON.parse(result.stdout.toString())
-    if (!hasDiagnostics(diagnostics)) throw new Error(`${fixture.name}: invalid Oxlint output`)
-    const errors = diagnostics.diagnostics.filter(
-      (item) => typeof item === "object" && item !== null && "code" in item && item.code === "opencode(tagged-error-message)",
-    ).length
-    if (errors !== fixture.errors)
-      throw new Error(
-        `${fixture.name}: expected ${fixture.errors} errors, received ${errors}\n${result.stdout.toString()}`,
-      )
-  }
+  await Promise.all(
+    cases.map(async (fixture, index) => {
+      const file = path.join(directory, `${index}.ts`)
+      await Bun.write(file, fixture.source)
+      const result = Bun.spawnSync([
+        path.join(import.meta.dir, "../../node_modules/.bin/oxlint"),
+        "--config",
+        config,
+        "--format",
+        "json",
+        file,
+      ])
+      if (!result.stdout.length) throw new Error(`${fixture.name}: ${result.stderr.toString()}`)
+      const diagnostics: unknown = JSON.parse(result.stdout.toString())
+      if (!hasDiagnostics(diagnostics)) throw new Error(`${fixture.name}: invalid Oxlint output`)
+      const errors = diagnostics.diagnostics.filter(isErrorDiagnostic)
+      if (errors.length !== fixture.errors || !errors.every(isRuleDiagnostic))
+        throw new Error(
+          `${fixture.name}: expected ${fixture.errors} errors, received ${errors.length}\n${result.stdout.toString()}`,
+        )
+      const exitCode = fixture.errors === 0 ? 0 : 1
+      if (result.exitCode !== exitCode || result.signalCode)
+        throw new Error(`${fixture.name}: expected exit ${exitCode}, received ${result.exitCode}`)
+    }),
+  )
 } finally {
   await rm(directory, { recursive: true, force: true })
 }
@@ -119,3 +161,16 @@ console.log(`Validated ${cases.length} tagged error lint fixtures`)
 function hasDiagnostics(value: unknown): value is { diagnostics: unknown[] } {
   return typeof value === "object" && value !== null && "diagnostics" in value && Array.isArray(value.diagnostics)
 }
+
+function isRuleDiagnostic(value: unknown) {
+  return (
+    typeof value === "object" &&
+    value !== null &&
+    "code" in value &&
+    value.code === "opencode(tagged-error-message)"
+  )
+}
+
+function isErrorDiagnostic(value: unknown) {
+  return typeof value === "object" && value !== null && "severity" in value && value.severity === "error"
+}