Merge pull request #143 from code0-tech/feat/#142

Reference suggestions don't traverse union-typed properties

Author: nicosammito

package-lock.json

@@ -23,7 +23,7 @@
     "build": "vite build"
   },
   "devDependencies": {
-    "@code0-tech/sagittarius-graphql-types": "0.0.0-experimental-2668386432-315fff7e9283580109793944cb24db116d9cc264",
+    "@code0-tech/sagittarius-graphql-types": "0.0.0-experimental-2705887128-b8e126d1674035ed0f869775b533d7093e8aedbd",
     "@types/node": "^25.6.0",
     "@typescript/vfs": "^1.6.4",
     "typescript": "^6.0.3",
@@ -33,7 +33,7 @@
     "lossless-json": "^4.3.0"
   },
   "peerDependencies": {
-    "@code0-tech/sagittarius-graphql-types": "0.0.0-experimental-2668386432-315fff7e9283580109793944cb24db116d9cc264",
+    "@code0-tech/sagittarius-graphql-types": "0.0.0-experimental-2705887128-b8e126d1674035ed0f869775b533d7093e8aedbd",
     "@typescript/vfs": "^1.6.4",
     "typescript": "^5.9.3 || ^6.0.2",
     "lossless-json": "^4.3.0"

package.json

@@ -295,6 +295,59 @@ export const isRecursiveType = (
  * // For type {user: {name: string, age: number}} with expectedType = string
  * // Returns: [{path: [{path: 'user'}, {path: 'name'}], type: stringType}]
  */
+/**
+ * Determines whether a candidate type matches an expected parameter type.
+ *
+ * A plain type matches when it is (non-nullably) assignable to the expected type.
+ * A union matches when ANY of its constituents matches: a union such as
+ * `string | { deep: string }` is not assignable to `string` as a whole, yet its
+ * string branch makes the union key a valid suggestion for a string parameter.
+ *
+ * @param type - The candidate type (already non-nullable)
+ * @param checker - TypeScript TypeChecker for type operations
+ * @param expectedType - The target parameter type to match against
+ * @returns True if the type (or one of its union branches) matches the expected type
+ */
+const matchesExpectedType = (
+  type: ts.Type,
+  checker: ts.TypeChecker,
+  expectedType: ts.Type
+): boolean => {
+  if (type.isUnion()) {
+    return type.types.some((constituent) =>
+      matchesExpectedType(checker.getNonNullableType(constituent), checker, expectedType)
+    );
+  }
+
+  return (
+    (type.flags & ts.TypeFlags.Never) === 0 &&
+    checker.isTypeAssignableTo(type, expectedType)
+  );
+};
+
+/**
+ * Resolves the object types whose properties should be traversed for a candidate.
+ *
+ * For a plain object type this is simply the type itself. For a union it is each
+ * object-typed branch (non-nullable), since getProperties() on the union only
+ * exposes properties shared by every branch — the distinct keys of individual
+ * object branches (e.g. `deep` in `string | { deep: string }`) would otherwise be
+ * unreachable. Primitive branches are dropped.
+ *
+ * @param type - The candidate type (already non-nullable)
+ * @param checker - TypeScript TypeChecker for type operations
+ * @returns The object branches to traverse into
+ */
+const getObjectBranches = (type: ts.Type, checker: ts.TypeChecker): ts.Type[] => {
+  if (type.isUnion()) {
+    return type.types
+      .map((constituent) => checker.getNonNullableType(constituent))
+      .filter(isRealObjectType);
+  }
+
+  return isRealObjectType(type) ? [type] : [];
+};
+
 const extractObjectProperties = (
   type: ts.Type,
   checker: ts.TypeChecker,
@@ -311,32 +364,41 @@ const extractObjectProperties = (
   // `string` parameter — strict assignability would reject it under strictNullChecks.
   // A purely nullish candidate strips down to `never` (assignable to anything),
   // so it must be excluded explicitly.
+  //
+  // A union candidate (e.g. `TEXT | { deep: TEXT }`) matches when ANY of its
+  // constituents is assignable to the expected type: the whole union is not
+  // assignable to `string`, but its string branch is, so the union key is still a
+  // valid string suggestion.
   const nonNullableType = checker.getNonNullableType(type);
-  if (
-    (nonNullableType.flags & ts.TypeFlags.Never) === 0 &&
-    checker.isTypeAssignableTo(nonNullableType, expectedType)
-  ) {
+  if (matchesExpectedType(nonNullableType, checker, expectedType)) {
     results.push({ path: currentPath, type });
   }
 
   // Recursively traverse into object properties. Traversal also runs on the
   // non-nullable type: a nullable object in the chain (e.g. `{bla?: TEXT} | null`)
   // is a union whose getProperties() is empty, which would cut off all nested paths.
+  // Union candidates (e.g. `TEXT | { deep: TEXT }`) are traversed per object branch:
+  // getProperties() on the union itself only exposes properties common to every
+  // branch (none here), so `flexible.deep` would otherwise be unreachable.
   // Recursive data types (e.g. Order.delivery.order) are cut off via the visited
   // set — the checker caches type identities, so a cycle revisits the same ts.Type
   // object. The set only tracks the current branch (backtracked below) so the same
   // type may still appear on sibling paths. The depth cap only applies to types
   // that are part of a reference cycle (checked last, it's the expensive test);
   // purely nested non-recursive objects are traversed to arbitrary depth.
-  if (
-    isRealObjectType(nonNullableType) &&
-    !visited.has(nonNullableType) &&
-    (currentPath.length < MAX_REFERENCE_DEPTH ||
-      !isRecursiveType(nonNullableType, checker, recursionCache))
-  ) {
-    const properties = nonNullableType.getProperties();
+  const objectBranches = getObjectBranches(nonNullableType, checker);
+  objectBranches.forEach((branch) => {
+    if (
+      visited.has(branch) ||
+      (currentPath.length >= MAX_REFERENCE_DEPTH &&
+        isRecursiveType(branch, checker, recursionCache))
+    ) {
+      return;
+    }
+
+    const properties = branch.getProperties();
     if (properties && properties.length > 0) {
-      visited.add(nonNullableType);
+      visited.add(branch);
       properties.forEach((property) => {
         const propType = checker.getTypeOfSymbolAtLocation(property, property.valueDeclaration!);
         const propName = property.getName();
@@ -345,9 +407,9 @@ const extractObjectProperties = (
         // Recurse into nested properties
         results.push(...extractObjectProperties(propType, checker, expectedType, newPath, visited, recursionCache));
       });
-      visited.delete(nonNullableType);
+      visited.delete(branch);
     }
-  }
+  });
 
   return results;
 };

src/util/references.util.ts

@@ -113,7 +113,8 @@ export function generateFlowSourceCode(
     flow?: Flow,
     functions?: FunctionDefinition[],
     dataTypes?: DataType[],
-    isForInference: boolean = false
+    isForInference: boolean = false,
+    assertNonNullReferences: boolean = true
 ): string {
     const nodes = flow?.nodes?.nodes || [];
     const funcMap = new Map(functions?.map(f => [f.identifier, f]));
@@ -139,11 +140,15 @@ export function generateFlowSourceCode(
                 ref.referencePath?.forEach(pathObj => {
                     refCode += `?.${pathObj.path}`;
                 });
-                // Non-null assertion: a reference typed `string | null` (or an optional
-                // chain like `node_X?.text`) must still satisfy a plain `string` parameter
-                // under strictNullChecks — only the nullish part is waived, base type
-                // mismatches still fail validation.
-                return `/* @pos ${id} ${index} */ (${refCode})!`;
+                // A reference may be nullable (`string | null`) or reach through optional
+                // properties (an optional chain like `node_X?.text`). When `assertNonNullReferences`
+                // is set (inference/schema), the nullish part is waived with a `!` so the base
+                // type flows cleanly. During validation it is left in place so a possibly-null
+                // reference feeding a non-null parameter surfaces a diagnostic — which is then
+                // downgraded to a warning rather than being silently erased. Base type
+                // mismatches still fail validation in both modes.
+                const nonNull = assertNonNullReferences ? "!" : "";
+                return `/* @pos ${id} ${index} */ (${refCode})${nonNull}`;
             }
             if (val.__typename === "LiteralValue") {
                 const jsonString = val?.value !== null && val?.value !== undefined ? stringify(val?.value) : undefined

src/utils.ts

@@ -1,7 +1,78 @@
-import {flattenDiagnosticMessageText} from "typescript";
+import ts, {flattenDiagnosticMessageText} from "typescript";
 import {DataType, Flow, FunctionDefinition, NodeFunction} from "@code0-tech/sagittarius-graphql-types";
 import {createCompilerHost, generateFlowSourceCode, ValidationResult} from "../utils";
 
+// TypeScript diagnostic codes we may soften into warnings for union-branch references.
+const TS_ARGUMENT_NOT_ASSIGNABLE = 2345; // "Argument of type X is not assignable to parameter of type Y."
+const TS_PROPERTY_DOES_NOT_EXIST = 2339; // "Property 'p' does not exist on type X."
+
+/**
+ * Finds the innermost AST node that fully contains the [start, end) span.
+ */
+const findInnermostNode = (node: ts.Node, start: number, end: number): ts.Node | undefined => {
+    if (node.getStart() > start || node.getEnd() < end) return undefined;
+    let result: ts.Node = node;
+    node.forEachChild((child) => {
+        const found = findInnermostNode(child, start, end);
+        if (found) result = found;
+    });
+    return result;
+};
+
+/**
+ * Decides whether a type error stems from a reference whose value type is a union
+ * where at least one branch would satisfy the expected type, but not all of them do:
+ *
+ *  - a nullable reference — `TEXT | null` used for a plain TEXT parameter (the value
+ *    might be null/undefined at runtime); or
+ *  - a union-branch reference — `flexible: TEXT | { deep: TEXT }` used for a plain
+ *    TEXT parameter, or drilling into `flexible.deep` which only exists on the object
+ *    branch (the value might be the wrong branch at runtime).
+ *
+ * Both are references the schema engine offers as suggestions, so using them is a
+ * warning rather than a hard error — the flow stays valid. Genuine mismatches where
+ * no branch fits (e.g. `NUMBER | null` → TEXT) stay errors.
+ */
+const isSoftReferenceMismatch = (
+    diagnostic: ts.Diagnostic,
+    sourceFile: ts.SourceFile,
+    checker: ts.TypeChecker
+): boolean => {
+    if (diagnostic.start === undefined || diagnostic.length === undefined) return false;
+
+    const node = findInnermostNode(sourceFile, diagnostic.start, diagnostic.start + diagnostic.length);
+    if (!node) return false;
+
+    // Argument not assignable: the argument's type is a union and at least one of its
+    // non-nullish branches is assignable to the contextually expected parameter type.
+    // Nullish branches are excluded so that `NUMBER | null` (no assignable base branch)
+    // stays a hard error, while `TEXT | null` and `TEXT | { deep: TEXT }` soften.
+    if (diagnostic.code === TS_ARGUMENT_NOT_ASSIGNABLE && ts.isExpression(node)) {
+        const argType = checker.getTypeAtLocation(node);
+        if (!argType.isUnion()) return false;
+
+        const expectedType = checker.getContextualType(node);
+        if (!expectedType) return false;
+
+        return argType.types.some((branch) =>
+            (branch.flags & (ts.TypeFlags.Null | ts.TypeFlags.Undefined)) === 0 &&
+            checker.isTypeAssignableTo(branch, expectedType)
+        );
+    }
+
+    // Property access into a union branch: the accessed object is a union and the
+    // property exists on at least one of its branches.
+    if (diagnostic.code === TS_PROPERTY_DOES_NOT_EXIST && ts.isPropertyAccessExpression(node.parent)) {
+        const objectType = checker.getNonNullableType(checker.getTypeAtLocation(node.parent.expression));
+        if (!objectType.isUnion()) return false;
+
+        const propertyName = node.getText();
+        return objectType.types.some((branch) => branch.getProperty(propertyName) !== undefined);
+    }
+
+    return false;
+};
+
 /**
  * Validates a flow by generating virtual TypeScript code and running it through the TS compiler.
  */
@@ -58,14 +129,18 @@ export const getFlowValidation = (
         }
     }
 
-    const sourceCode = generateFlowSourceCode(flow, functions, dataTypes);
+    // Validation keeps reference nullability (assertNonNullReferences = false) so that a
+    // possibly-null reference into a non-null parameter surfaces as a diagnostic, which is
+    // then downgraded to a warning below instead of being silently suppressed by a `!`.
+    const sourceCode = generateFlowSourceCode(flow, functions, dataTypes, false, false);
 
     // 3. Virtual TypeScript Compilation
     const fileName = "index.ts";
     const host = createCompilerHost(fileName, sourceCode);
     const sourceFile = host.getSourceFile(fileName)!;
 
     const program = host.languageService.getProgram()!;
+    const checker = program.getTypeChecker();
     const diagnostics = program.getSemanticDiagnostics(sourceFile);
 
     const errors = diagnostics.map(d => {
@@ -113,10 +188,15 @@ export const getFlowValidation = (
             }
         }
 
+        // A nullable reference (`TEXT | null`) or a union-branch reference
+        // (`TEXT | { deep: TEXT }`) into a non-null parameter is a valid suggestion, so its
+        // mismatch is a warning rather than a hard error — the flow stays valid.
+        const severity: "error" | "warning" = isSoftReferenceMismatch(d, sourceFile, checker) ? "warning" : "error";
+
         return {
             message,
             code: d.code,
-            severity: "error" as const,
+            severity,
             nodeId,
             parameterIndex: typeof parameterIndex == "number" && Number.isSafeInteger(parameterIndex) ? parameterIndex : null,
         };