Better error message for semijoin predicates (#4605)

# Description of Changes

`and` and `or` are scoped to only a single table. So using them in a
semijoin with two tables will fail to compile. Previously this resulted
in a gnarly error message (see #4586). This patch improves the error
message.

# API and ABI breaking changes

None

# Expected complexity level and risk

2

# Testing

Adds error message assertion tests.

Author: joshua-spacetime

crates/bindings-typescript/src/lib/query.ts

@@ -710,15 +710,38 @@ type BooleanExprData<Table extends TypedTableDef> = (
   _tableType?: Table;
 };
 
+type AndOrMixedTableScopeError = {
+  readonly 'Cannot combine predicates from different table scopes with and/or. In semijoin on(...), keep only the join equality and move extra predicates to .where(...).': never;
+};
+
+type RequireSameAndOrTable<
+  Expected extends TypedTableDef,
+  Actual extends TypedTableDef,
+> = [Expected] extends [Actual]
+  ? [Actual] extends [Expected]
+    ? unknown
+    : AndOrMixedTableScopeError
+  : AndOrMixedTableScopeError;
+
 export class BooleanExpr<Table extends TypedTableDef> {
   constructor(readonly data: BooleanExprData<Table>) {}
 
-  and(other: BooleanExpr<Table>): BooleanExpr<Table> {
-    return new BooleanExpr({ type: 'and', clauses: [this.data, other.data] });
+  and<OtherTable extends TypedTableDef>(
+    other: BooleanExpr<OtherTable> & RequireSameAndOrTable<Table, OtherTable>
+  ): BooleanExpr<Table> {
+    return new BooleanExpr({
+      type: 'and',
+      clauses: [this.data, other.data as BooleanExprData<Table>],
+    });
   }
 
-  or(other: BooleanExpr<Table>): BooleanExpr<Table> {
-    return new BooleanExpr({ type: 'or', clauses: [this.data, other.data] });
+  or<OtherTable extends TypedTableDef>(
+    other: BooleanExpr<OtherTable> & RequireSameAndOrTable<Table, OtherTable>
+  ): BooleanExpr<Table> {
+    return new BooleanExpr({
+      type: 'or',
+      clauses: [this.data, other.data as BooleanExprData<Table>],
+    });
   }
 
   not(): BooleanExpr<Table> {
@@ -732,28 +755,40 @@ export function not<T extends TypedTableDef>(
   return new BooleanExpr({ type: 'not', clause: clause.data });
 }
 
-export function and<T extends TypedTableDef>(
-  ...clauses: readonly [BooleanExpr<T>, BooleanExpr<T>, ...BooleanExpr<T>[]]
-): BooleanExpr<T> {
+export function and<
+  Table extends TypedTableDef,
+  OtherTable extends TypedTableDef,
+>(
+  first: BooleanExpr<Table>,
+  second: BooleanExpr<OtherTable> & RequireSameAndOrTable<Table, OtherTable>,
+  ...rest: readonly BooleanExpr<Table>[]
+): BooleanExpr<Table> {
+  const clauses = [first, second, ...rest];
   return new BooleanExpr({
     type: 'and',
     clauses: clauses.map(c => c.data) as [
-      BooleanExprData<T>,
-      BooleanExprData<T>,
-      ...BooleanExprData<T>[],
+      BooleanExprData<Table>,
+      BooleanExprData<Table>,
+      ...BooleanExprData<Table>[],
     ],
   });
 }
 
-export function or<T extends TypedTableDef>(
-  ...clauses: readonly [BooleanExpr<T>, BooleanExpr<T>, ...BooleanExpr<T>[]]
-): BooleanExpr<T> {
+export function or<
+  Table extends TypedTableDef,
+  OtherTable extends TypedTableDef,
+>(
+  first: BooleanExpr<Table>,
+  second: BooleanExpr<OtherTable> & RequireSameAndOrTable<Table, OtherTable>,
+  ...rest: readonly BooleanExpr<Table>[]
+): BooleanExpr<Table> {
+  const clauses = [first, second, ...rest];
   return new BooleanExpr({
     type: 'or',
     clauses: clauses.map(c => c.data) as [
-      BooleanExprData<T>,
-      BooleanExprData<T>,
-      ...BooleanExprData<T>[],
+      BooleanExprData<Table>,
+      BooleanExprData<Table>,
+      ...BooleanExprData<Table>[],
     ],
   });
 }

crates/bindings-typescript/src/server/view.test-d.ts

@@ -161,6 +161,10 @@ spacetime.anonymousView({ name: 'v5', public: true }, arrayRetValue, ctx => {
     .where(row => row.id.eq(5))
     .leftSemijoin(ctx.from.order, (p, o) => p.name.eq(o.person_name))
     .build();
+  const _mixedScopeAndInJoinPredicate = ctx.from.person
+    // @ts-expect-error semijoin on(...) only supports one table scope for and/or clauses.
+    .leftSemijoin(ctx.from.order, (p, o) => p.id.eq(o.id).and(o.id.eq(5)))
+    .build();
   return ctx.from.person
     .where(row => row.id.eq(5))
     .leftSemijoin(ctx.from.order, (p, o) => p.id.eq(o.id))

crates/bindings-typescript/tests/query_error_message.test.ts

@@ -0,0 +1,87 @@
+import { mkdtempSync, rmSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import * as ts from 'typescript';
+import { describe, expect, it } from 'vitest';
+
+const bindingsRoot = path.resolve(
+  path.dirname(fileURLToPath(import.meta.url)),
+  '..'
+);
+
+function runTypecheck(semijoinPredicateExpr: string) {
+  const tmpDir = mkdtempSync(path.join(tmpdir(), 'stdb-query-diag-'));
+  const reproPath = path.join(tmpDir, 'repro.ts');
+
+  const imports = {
+    query: path.join(bindingsRoot, 'src/lib/query.ts'),
+    moduleBindings: path.join(
+      bindingsRoot,
+      'test-app/src/module_bindings/index.ts'
+    ),
+    sys: path.join(bindingsRoot, 'src/server/sys.d.ts'),
+  };
+
+  const source = `
+import { and } from ${JSON.stringify(imports.query)};
+import { tables } from ${JSON.stringify(imports.moduleBindings)};
+
+tables.player
+  .leftSemijoin(tables.unindexed_player, (l, r) => ${semijoinPredicateExpr})
+  .build();
+`;
+
+  writeFileSync(reproPath, source);
+
+  try {
+    const options: ts.CompilerOptions = {
+      target: ts.ScriptTarget.ESNext,
+      module: ts.ModuleKind.ESNext,
+      strict: true,
+      noEmit: true,
+      skipLibCheck: true,
+      forceConsistentCasingInFileNames: true,
+      allowImportingTsExtensions: true,
+      noImplicitAny: true,
+      moduleResolution: ts.ModuleResolutionKind.Bundler,
+      useDefineForClassFields: true,
+      verbatimModuleSyntax: true,
+      isolatedModules: true,
+    };
+
+    const host = ts.createCompilerHost(options);
+    const program = ts.createProgram([reproPath, imports.sys], options, host);
+    const diagnostics = ts.getPreEmitDiagnostics(program);
+    const output = diagnostics
+      .map(d => ts.flattenDiagnosticMessageText(d.messageText, '\n'))
+      .join('\n');
+
+    return {
+      status: diagnostics.length === 0 ? 0 : 1,
+      output,
+    };
+  } finally {
+    rmSync(tmpDir, { recursive: true, force: true });
+  }
+}
+
+describe('query builder diagnostics', () => {
+  const messageStart =
+    'Cannot combine predicates from different table scopes with and/or.';
+  const messageHint = 'move extra predicates to .where(...)';
+
+  it('reports a clear message for free-floating and(...) in semijoin predicates', () => {
+    const { status, output } = runTypecheck('and(l.id.eq(r.id), r.id.eq(5))');
+    expect(status).not.toBe(0);
+    expect(output).toContain(messageStart);
+    expect(output).toContain(messageHint);
+  });
+
+  it('reports a clear message for method-style .and(...) in semijoin predicates', () => {
+    const { status, output } = runTypecheck('l.id.eq(r.id).and(r.id.eq(5))');
+    expect(status).not.toBe(0);
+    expect(output).toContain(messageStart);
+    expect(output).toContain(messageHint);
+  });
+});