diff --git a/.changeset/prepare-variables.md b/.changeset/prepare-variables.md new file mode 100644 index 0000000..587649a --- /dev/null +++ b/.changeset/prepare-variables.md @@ -0,0 +1,5 @@ +--- +"@shopify/shopify_function": minor +--- + +Fail the build when a JavaScript function's prepare target returns variables its run query doesn't accept, and warn about variables it can't check. Needs a Shopify CLI that runs this package's `check.js`. diff --git a/check.js b/check.js new file mode 100644 index 0000000..9ad1bd3 --- /dev/null +++ b/check.js @@ -0,0 +1,39 @@ +#!/usr/bin/env node +// Shopify CLI runs this, when the package has it, before bundling a JavaScript function: +// +// node check.js --function +// +// The function JSON is {handle, directory, entry, schema, targets: [{target, export, inputQuery}]}, where `export` is +// the JavaScript function the CLI bundles. It exits non-zero, failing the build, when a prepare target returns +// variables its run query doesn't accept, and warns about variables it can't check. +const fs = require("node:fs"); +const { parseArgs } = require("node:util"); +const { prepareVariablesProblems } = require("./prepare-variables.js"); + +try { + const { values } = parseArgs({ options: { function: { type: "string" } } }); + if (!values.function) throw new Error("Usage: check.js --function "); + const fn = { schema: "schema.graphql", targets: [], ...JSON.parse(fs.readFileSync(values.function, "utf8")) }; + const name = fn.handle ? `The function extension "${fn.handle}"` : "The function"; + let problems; + try { + problems = prepareVariablesProblems(fn); + } catch (error) { + // A problem in the check itself shouldn't stop the build + console.warn(`${name}'s prepare variables couldn't be checked: ${error.message}`); + problems = []; + } + const list = (warning) => problems.filter((problem) => problem.warning === warning).map((problem) => problem.message); + + const warnings = list(true); + if (warnings.length > 0) { + console.warn(`${name} returns prepare variables that can't be checked against its run query\n${warnings.join("\n")}`); + } + const errors = list(false); + if (errors.length > 0) { + throw new Error(`${name} returns prepare variables its run query doesn't accept\n${errors.join("\n")}`); + } +} catch (error) { + console.error(error.message); + process.exitCode = 1; +} diff --git a/prepare-variables.js b/prepare-variables.js new file mode 100644 index 0000000..97f3e4f --- /dev/null +++ b/prepare-variables.js @@ -0,0 +1,609 @@ +// Checks what each prepare target's export returns against the variables its run target's input query declares. +// Type annotations and casts are ignored, and values are followed back to where they're created. Values that can't be +// followed are warnings rather than passes. +const fs = require("node:fs"); +const path = require("node:path"); +const ts = require("typescript"); +const { + GraphQLInputObjectType, + GraphQLBoolean, + GraphQLFloat, + GraphQLNonNull, + GraphQLScalarType, + GraphQLString, + Kind, + buildSchema, + getNamedType, + isEnumType, + isInputObjectType, + isListType, + isNonNullType, + isScalarType, + isSpecifiedScalarType, + parse, + typeFromAST, +} = require("graphql"); + +const unknownScalar = new GraphQLScalarType({ name: "Unknown" }); + +// The TypeScript type Shopify CLI's codegen config gives each custom scalar, which a function's config can change +const codegenScalars = { + Date: "string", + DateTime: "string", + DateTimeWithoutTimezone: "string", + Decimal: "string", + Handle: "string", + JSON: "unknown", + TimeWithoutTimezone: "string", + URL: "string", + Void: "null", +}; +// The built-in scalar each of those TypeScript types is checked as. `null` is for Void, which only takes null. +const scalarsByType = { + string: GraphQLString, + number: GraphQLFloat, + boolean: GraphQLBoolean, + null: new GraphQLScalarType({ name: "Null", parseValue: () => undefined }), +}; + +/** The prepare targets whose run target has an input query. */ +function preparePairs(targets) { + return targets.flatMap(({ target, export: name }) => { + const run = targets.find((other) => other.target === target.replace(/\.prepare$/, ".run")); + return target.endsWith(".prepare") && name && run?.inputQuery ? [{ target, name, query: run.inputQuery }] : []; + }); +} + +/** + * Returns `{message, warning}` for each problem, where `message` is `file:line:column message`, relative to `directory`. + * Values that can't be checked are warnings, and mismatches are errors. + */ +function prepareVariablesProblems({ directory, entry: entryPath, schema: schemaPath, targets }) { + const pairs = preparePairs(targets); + if (pairs.length === 0) return []; + if (!entryPath) { + return [{ message: "The function has no entry file, such as src/index.js, to check its prepare targets in", warning: false }]; + } + // Shopify CLI merges the codegen config in the function's package.json with its own + const packageJson = path.join(directory, "package.json"); + const { codegen } = fs.existsSync(packageJson) ? JSON.parse(fs.readFileSync(packageJson, "utf8")) : {}; + const scalarTypes = { ...codegenScalars }; + for (const [name, type] of Object.entries(codegen?.config?.scalars ?? {})) { + scalarTypes[name] = typeof type === "object" ? type?.input : type; + } + const entry = path.resolve(directory, entryPath); + // Paths in messages use forward slashes, as Shopify CLI's do, on Windows too + const relative = (file) => path.relative(directory, file).split(path.sep).join("/"); + const locate = (file, position) => { + const { line, character } = file.getLineAndCharacterOfPosition(position); + return `${relative(file.fileName)}:${line + 1}:${character + 1}`; + }; + + const schema = buildSchema(fs.readFileSync(path.resolve(directory, schemaPath), "utf8")); + const options = { + allowJs: true, + strict: true, + noEmit: true, + skipLibCheck: true, + types: [], + lib: ["lib.es2022.d.ts"], + target: ts.ScriptTarget.ES2022, + module: ts.ModuleKind.ESNext, + moduleResolution: ts.ModuleResolutionKind.Bundler, + // An element of an array, like `list[0]`, can be undefined + noUncheckedIndexedAccess: true, + }; + const host = ts.createCompilerHost(options, true); + // Annotations and casts are claims nothing checks against the value (`@ts-ignore` silences the compiler), so they're + // blanked out of the function's own files, leaving types to come from the values. Parameter types and non-null + // assertions (`!`, like Rust's `unwrap`) are kept. Blanking keeps positions, so errors point at the original source. + const { getSourceFile } = host; + host.getSourceFile = (fileName, language, ...rest) => { + const file = getSourceFile(fileName, language, ...rest); + if (!file || file.isDeclarationFile || fileName.includes("/node_modules/")) return file; + const text = file.text.split(""); + const blank = (start, end) => { + for (let index = start; index < end; index++) if (text[index] !== "\n" && text[index] !== "\r") text[index] = " "; + }; + const visit = (node) => { + for (const tag of ts.getJSDocTags(node)) { + const typing = + ts.isJSDocTypeTag(tag) || ts.isJSDocReturnTag(tag) || ts.isJSDocSatisfiesTag(tag) || ts.isJSDocOverloadTag(tag); + if (typing && !ts.isParameter(tag.parent.parent)) blank(tag.pos, tag.end); + } + if (ts.isAsExpression(node) || ts.isSatisfiesExpression(node)) { + blank(node.expression.end, node.end); + } else if (ts.isTypeAssertionExpression(node)) { + // Unless a line break follows it, where blanking it would end a `return ` before what it returns + if (!/[\r\n]/.test(file.text.slice(node.expression.pos, node.expression.getStart()))) blank(node.type.pos - 1, node.expression.pos); + } else if ((ts.isCallExpression(node) || ts.isNewExpression(node)) && node.typeArguments) { + blank(node.typeArguments.pos - 1, node.typeArguments.end + 1); + } else if ( + (ts.isVariableDeclaration(node) || + ts.isPropertyDeclaration(node) || + ts.isFunctionDeclaration(node) || + ts.isFunctionExpression(node) || + ts.isArrowFunction(node) || + ts.isMethodDeclaration(node) || + ts.isGetAccessorDeclaration(node)) && + node.type + ) { + blank(node.type.pos - 1, node.type.end); + } + ts.forEachChild(node, visit); + }; + visit(file); + return ts.createSourceFile(fileName, text.join(""), language); + }; + const program = ts.createProgram([entry], options, host); + const isLocal = (file) => !file.isDeclarationFile && !program.isSourceFileFromExternalLibrary(file); + // A syntax error stops the check. The bundler reports it too, unless the bundle leaves the file out, such as one + // only imported with `import type`. + const syntaxErrors = program.getSourceFiles().filter(isLocal).flatMap((file) => program.getSyntacticDiagnostics(file).slice(0, 1)); + if (syntaxErrors.length > 0) { + return syntaxErrors.map(({ file, start, messageText }) => { + const error = ts.flattenDiagnosticMessageText(messageText, " "); + return { message: `${locate(file, start)} can't be checked because of a syntax error: ${error}`, warning: true }; + }); + } + const checker = program.getTypeChecker(); + const entryFile = program.getSourceFile(entry); + const entrySymbol = entryFile && checker.getSymbolAtLocation(entryFile); + const problems = new Map(); + let muted = false; + + const report = (node, message, warning = false) => { + if (!muted) problems.set(`${locate(node.getSourceFile(), node.getStart())} ${message}`, warning); + }; + const join = (base, key) => (base ? `${base}.${key}` : key); + const subject = (at) => (at ? `\`${at}\`` : "The result"); + // A type parameter, such as `T` in a generic helper, can be anything + const generic = (type) => + Boolean(type.flags & ts.TypeFlags.InstantiableNonPrimitive) || (type.isIntersection() && type.types.some(generic)); + const unchecked = (type) => Boolean(type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) || generic(type); + const members = (type) => (type.isUnion() ? type.types : [type]); + const isUndefined = (type) => Boolean(type.flags & (ts.TypeFlags.Undefined | ts.TypeFlags.Void)); + + const unwrap = (node) => + ts.isParenthesizedExpression(node) || ts.isTypeAssertionExpression(node) ? unwrap(node.expression) : node; + + const functionOf = (node) => { + if (!node) return; + if (ts.isVariableDeclaration(node)) return node.initializer && functionOf(unwrap(node.initializer)); + if (isLocal(node.getSourceFile()) && (ts.isFunctionDeclaration(node) || ts.isFunctionExpression(node) || ts.isArrowFunction(node))) { + return node.body && node; + } + }; + + const symbolOf = (node) => + ts.isShorthandPropertyAssignment(node.parent) && node.parent.name === node + ? checker.getShorthandAssignmentValueSymbol(node.parent) + : checker.getSymbolAtLocation(node); + + const functionAround = (node) => ts.findAncestor(node.parent, ts.isFunctionLike); + + // An assignment with a written-out key, `object.key = value` or `object["key"] = value`, to a reference to the object + const assignmentOf = (reference) => { + const access = reference.parent; + const assignment = access.parent; + const direct = (ts.isPropertyAccessExpression(access) || ts.isElementAccessExpression(access)) && access.expression === reference; + if (!direct || !ts.isBinaryExpression(assignment) || assignment.left !== access) return; + if (assignment.operatorToken.kind !== ts.SyntaxKind.EqualsToken) return; + const name = ts.isPropertyAccessExpression(access) ? access.name : access.argumentExpression; + // The key in brackets is its value, when TypeScript knows it, such as `"key"` or a `const` holding it + const keyType = ts.isElementAccessExpression(access) && checker.getTypeAtLocation(name); + const key = keyType ? (keyType.isStringLiteral() ? keyType.value : undefined) : name.text; + return key === undefined ? undefined : { key, name, value: assignment.right, node: assignment }; + }; + + // A reference that could change the value after it's created: assigning to it or a member, deleting or + // incrementing a member, calling a method on it, or passing, aliasing or reassigning the binding itself. + const changes = (reference) => { + let top = reference; + while ( + (ts.isPropertyAccessExpression(top.parent) || ts.isElementAccessExpression(top.parent)) && + top.parent.expression === top + ) { + top = top.parent; + } + const { parent } = top; + if (top === reference) { + return !(ts.isSpreadAssignment(parent) || ts.isSpreadElement(parent) || ts.isTypeOfExpression(parent)); + } + return written(top) || ts.isDeleteExpression(parent) || (ts.isCallExpression(parent) && parent.expression === top); + }; + + // Whether `node` is given a new value, as in `node = …`, `node += …` or `node++` + const written = (node) => { + const { parent } = node; + return ( + (ts.isBinaryExpression(parent) && + parent.left === node && + parent.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + parent.operatorToken.kind <= ts.SyntaxKind.LastAssignment) || + ((ts.isPrefixUnaryExpression(parent) || ts.isPostfixUnaryExpression(parent)) && + (parent.operator === ts.SyntaxKind.PlusPlusToken || parent.operator === ts.SyntaxKind.MinusMinusToken)) + ); + }; + // Whether a helper gives its parameter a new value + const reassigned = (parameter) => { + const symbol = checker.getSymbolAtLocation(parameter.name); + const visit = (node) => (ts.isIdentifier(node) && symbolOf(node) === symbol && written(node)) || ts.forEachChild(node, visit); + return Boolean(visit(parameter.parent)); + }; + + for (const { target, name, query } of pairs) { + const queryPath = path.resolve(directory, query); + const queryName = relative(queryPath); + const operation = parse(fs.readFileSync(queryPath, "utf8")).definitions.find( + (definition) => definition.kind === Kind.OPERATION_DEFINITION, + ); + const variables = new GraphQLInputObjectType({ + name: "Variables", + fields: Object.fromEntries( + (operation?.variableDefinitions ?? []).map((definition) => [ + definition.variable.name.value, + // A type the schema doesn't have is left to codegen, which reports it + { type: typeFromAST(schema, definition.type) ?? unknownScalar, defaultValue: definition.defaultValue }, + ]), + ), + }); + // The prepare target's result type, from its mutation field such as `cartValidationsGeneratePrepare(result: …)`, with + // `variables` typed as the run query's variables, as the Rust override does. Without one, it only has `variables`. + const field = schema.getMutationType()?.getFields()[target.replace(/[.-](\w)/g, (_, letter) => letter.toUpperCase())]; + const schemaResult = field && getNamedType(field.args.find((argument) => argument.name === "result")?.type); + const result = new GraphQLInputObjectType({ + name: isInputObjectType(schemaResult) ? schemaResult.name : "PrepareResult", + fields: { + ...(isInputObjectType(schemaResult) ? schemaResult.toConfig().fields : {}), + variables: { type: new GraphQLNonNull(variables) }, + }, + }); + const owner = (type) => (type === variables ? `declared by ${queryName}` : `a field of ${type.name}`); + const followed = new Set(); + const checkedReferences = new Set(); + const visiting = new Set(); + const changed = new Map(); + const assigned = new Map(); + const passed = new Map(); + const returning = []; + // An assignment can't change what the return being checked returns if a return or throw follows it in a block + // the return isn't in, as in `if (…) { x.a = 1; return … }`, or if it comes after the return, unless a loop around + // both runs it first + const reachesReturn = ({ node }) => { + const fn = functionAround(node); + const statement = returning.findLast((candidate) => functionAround(candidate) === fn); + if (!statement) return true; + for (let child = node; !(child.parent.pos <= statement.pos && statement.end <= child.parent.end); child = child.parent) { + const statements = ts.isBlock(child.parent) || ts.isCaseOrDefaultClause(child.parent) ? child.parent.statements : []; + const later = statements.includes(child) ? statements.slice(statements.indexOf(child) + 1) : []; + if (later.some((exit) => ts.isReturnStatement(exit) || ts.isThrowStatement(exit))) return false; + } + if (node.pos < statement.end) return true; + const loop = ts.findAncestor(statement, (ancestor) => (ancestor === fn ? "quit" : ts.isIterationStatement(ancestor, false))); + return Boolean(loop && node.end <= loop.end); + }; + + // The variable an object literal property is written in, such as `variables` in `const variables = {a: 1}` + const bindingOf = (property) => { + const declaration = property.valueDeclaration; + const assignment = + declaration && (ts.isPropertyAssignment(declaration) || ts.isShorthandPropertyAssignment(declaration)); + const binding = assignment && declaration.parent.parent; + return binding && ts.isVariableDeclaration(binding) && ts.isIdentifier(binding.name) ? binding : undefined; + }; + // A variable changed or passed on after it's created may not hold what it was created with, so it's not checked, + // except an object assigned to by name, which is checked with its assignments where it's created + const skipChanged = (declaration, at, object) => { + const assignments = assigned.get(declaration); + const changes = changed.get(declaration) ?? (assignments && assignments.object !== object ? assignments.nodes : undefined); + for (const change of changes ?? []) { + report(change, `${subject(at)} is changed or passed on after it's created, so it can't be checked`, true); + } + return Boolean(changes); + }; + + // Whether a built-in scalar or an enum accepts a value, by GraphQL's own coercion: "yes", "no", or "maybe" when it + // depends on which of its type's values it is. A type like `number` is tried with one of its values. + const scalarMatch = (type, named) => { + const accepts = (value) => { + try { + return named.parseValue(value) !== undefined; + } catch { + return false; + } + }; + if (type.isStringLiteral() || type.isNumberLiteral()) return accepts(type.value) ? "yes" : "no"; + if (type.flags & ts.TypeFlags.BooleanLiteral) return accepts(type === checker.getTrueType()) ? "yes" : "no"; + if (type.flags & ts.TypeFlags.StringLike) return isEnumType(named) ? "maybe" : accepts("") ? "yes" : "no"; + if (type.flags & ts.TypeFlags.NumberLike) return accepts(0) ? "yes" : "no"; + return "no"; + }; + + const typed = (type, expected, at, omittable, node) => { + const named = getNamedType(expected); + // A custom scalar is checked as the TypeScript type codegen gives it, so a `Handle` is a String. The server + // decides what other ones, such as JSON, accept, but not whether they can be null or a list. + const checkedAs = isScalarType(named) && !isSpecifiedScalarType(named) ? scalarsByType[scalarTypes[named.name]] : named; + if (unchecked(type)) { + if (checkedAs) report(node, `${subject(at)} is typed ${checker.typeToString(type)}, so it can't be checked`, true); + return; + } + // No value has the type never, such as an element of `[].slice()` + if (type.flags & ts.TypeFlags.Never) return; + if (type.isUnion()) return type.types.forEach((member) => typed(member, expected, at, omittable, node)); + if (isUndefined(type)) { + if (isNonNullType(expected) && !omittable) report(node, `${subject(at)} can be undefined, but ${expected} has no default`); + return; + } + if (type.flags & ts.TypeFlags.Null) { + if (isNonNullType(expected)) report(node, `${subject(at)} can be null, but ${expected} can't`); + return; + } + // A value with a toJSON method, such as a Date, is written out as what it returns, as JSON.stringify does + const toJSON = checker.getPropertyOfType(type, "toJSON"); + const [write] = toJSON ? checker.getSignaturesOfType(checker.getTypeOfSymbol(toJSON), ts.SignatureKind.Call) : []; + const json = write && checker.getReturnTypeOfSignature(write); + if (json && json !== type) return typed(json, expected, at, omittable, node); + const inner = isNonNullType(expected) ? expected.ofType : expected; + if (isListType(inner)) { + const element = + (checker.isArrayType(type) || checker.isTupleType(type)) && checker.getIndexTypeOfType(type, ts.IndexKind.Number); + return element ? typed(element, inner.ofType, `${at}[]`, false, node) : typed(type, inner.ofType, at, false, node); + } + // An intersection, such as what Object.assign returns, is checked like an object + if (isInputObjectType(inner) && (type.flags & ts.TypeFlags.Object || type.isIntersection()) && !checker.isArrayType(type)) { + if (checker.getIndexInfosOfType(type).length > 0) { + return report(node, `${subject(at)} can have any keys, so it can't be checked`, true); + } + const properties = checker.getPropertiesOfType(type); + const bindings = new Set(properties.map(bindingOf).filter(Boolean)); + // A spread object adds what it holds when it's spread, which may not be what it was created with + for (const spread of ts.isObjectLiteralExpression(node) ? node.properties.filter(ts.isSpreadAssignment) : []) { + const expression = unwrap(spread.expression); + const declaration = ts.isIdentifier(expression) && symbolOf(expression)?.valueDeclaration; + if (declaration && ts.isVariableDeclaration(declaration)) { + followed.add(declaration); + bindings.add(declaration); + } + const spreadType = checker.getTypeAtLocation(spread.expression); + if (unchecked(spreadType) || checker.getIndexInfosOfType(spreadType).length > 0) { + return report(spread, `${subject(at)} spreads an object that can have any keys, so it can't be checked`, true); + } + } + if ([...bindings].map((binding) => skipChanged(binding, at, node)).some(Boolean)) return; + const fields = inner.getFields(); + // A oneOf input object takes exactly one of its fields, which can't be null. The others are left out, or + // undefined, which JSON leaves out. + const fieldType = (field) => (inner.isOneOf ? new GraphQLNonNull(field.type) : field.type); + const canOmit = (field) => inner.isOneOf || field.defaultValue !== undefined; + const reaching = ([...assigned.values()].find((candidate) => candidate.object === node)?.assignments ?? []).filter( + reachesReturn, + ); + // A write that a later one to the same key always replaces isn't returned, and neither is a property one replaces + const assignments = reaching.filter( + (assignment, index) => !reaching.slice(index + 1).some((later) => later.key === assignment.key && later.always), + ); + const replaced = new Set(assignments.filter((assignment) => assignment.always).map((assignment) => assignment.key)); + for (const property of properties) { + if (replaced.has(property.name)) continue; + const field = fields[property.name]; + const key = join(at, property.name); + // A property written in an object literal is followed to its value, keeping literal types (like an enum + // value) that the object's type widens. A key written twice has its last value. + const declaration = property.declarations?.at(-1) ?? property.valueDeclaration; + const assignment = + declaration && (ts.isPropertyAssignment(declaration) || ts.isShorthandPropertyAssignment(declaration)) + ? declaration + : undefined; + if (!field) { + report(assignment ?? node, `${subject(key)} isn't ${owner(inner)}`); + } else if (assignment) { + const binding = bindingOf(property); + if (binding) followed.add(binding); + const initializer = ts.isPropertyAssignment(assignment) ? assignment.initializer : assignment.name; + value(initializer, fieldType(field), key, canOmit(field)); + } else { + typed(checker.getTypeOfSymbol(property), fieldType(field), key, canOmit(field), node); + } + } + for (const { key, name, value: assignedValue } of assignments) { + const field = fields[key]; + if (!field) report(name, `${subject(join(at, key))} isn't ${owner(inner)}`); + else value(assignedValue, fieldType(field), join(at, key), canOmit(field)); + } + if (inner.isOneOf) { + // A field is set where it can't be undefined, and maybe set where it can be + const presence = (type, optional) => + members(type).every(isUndefined) ? "no" : optional || members(type).some(isUndefined) ? "maybe" : "yes"; + const keys = new Set([...properties.map((property) => property.name), ...assignments.map(({ key }) => key)]); + const set = [...keys].map((key) => { + const writes = assignments.filter((assignment) => assignment.key === key); + const possible = writes.map((assignment) => presence(checker.getTypeAtLocation(assignment.value), false)); + const property = !replaced.has(key) && properties.find((candidate) => candidate.name === key); + if (property) possible.push(presence(checker.getTypeOfSymbol(property), property.flags & ts.SymbolFlags.Optional)); + else if (!replaced.has(key)) possible.push("no"); + return possible.every((kind) => kind === possible[0]) ? possible[0] : "maybe"; + }); + const count = (kind) => set.filter((presence) => presence === kind).length; + if (count("yes") > 1 || count("yes") + count("maybe") === 0) { + report(node, `${subject(at)} must set exactly one field of ${inner}`); + } else if (count("maybe") > 0) { + report(node, `${subject(at)} may not set exactly one field of ${inner}, so it can't be checked`, true); + } + } + for (const [key, field] of Object.entries(fields)) { + if (!isNonNullType(field.type) || field.defaultValue !== undefined || checker.getPropertyOfType(type, key)) continue; + const setBy = assignments.filter((assignment) => assignment.key === key); + if (setBy.some((assignment) => assignment.always)) continue; + if (setBy.length > 0) report(setBy[0].name, `${subject(join(at, key))} is only set on some paths, so it can't be checked`, true); + else report(node, `${subject(join(at, key))} is missing, and ${field.type} has no default`); + } + return; + } + if (!checkedAs) return; + const match = isInputObjectType(checkedAs) ? "no" : scalarMatch(type, checkedAs); + if (match === "no") { + report(node, `${subject(at)} must be ${expected}, not ${checker.typeToString(type)}`); + } else if (match === "maybe") { + report(node, `${subject(at)} is ${checker.typeToString(type)}, which ${inner} may not accept, so it can't be checked`, true); + } + }; + + const returns = (fn, expected, at, omittable) => { + const body = fn.body; + if (!body) return; + if (!ts.isBlock(body)) return value(body, expected, at, omittable); + const visit = (node) => { + if (ts.isReturnStatement(node)) { + returning.push(node); + if (node.expression) value(node.expression, expected, at, omittable); + else typed(checker.getUndefinedType(), expected, at, omittable, node); + returning.pop(); + } else if (!ts.isFunctionLike(node)) { + ts.forEachChild(node, visit); + } + }; + ts.forEachChild(body, visit); + }; + + // Checks the types `left` can have, apart from the ones `skip` matches, which use `right` instead + const orElse = (left, right, skip, expected, at, omittable) => { + for (const member of members(checker.getTypeAtLocation(left))) { + if (!skip(member)) typed(member, expected, at, omittable, left); + } + return value(right, expected, at, omittable); + }; + + const value = (node, expected, at, omittable) => { + const expression = unwrap(node); + const inner = isNonNullType(expected) ? expected.ofType : expected; + + if (ts.isIdentifier(expression)) { + const symbol = symbolOf(expression); + const declaration = symbol?.valueDeclaration; + // A use a check narrows, such as `x` in `typeof x === "string" ? x : …`, is checked as its narrowed type + const narrowed = symbol && checker.getTypeAtLocation(expression) !== checker.getTypeOfSymbol(symbol); + // A helper's parameter holds what the call passes for it, or its default where that's undefined, unless the + // helper narrows it or gives it a new value + if (declaration && passed.has(declaration) && !narrowed && !reassigned(declaration)) { + const { argument, initializer } = passed.get(declaration); + if (!argument) return value(initializer, expected, at, omittable); + if (!initializer || !members(checker.getTypeAtLocation(argument)).some(isUndefined)) { + return value(argument, expected, at, omittable); + } + return orElse(argument, initializer, isUndefined, expected, at, omittable); + } + if (declaration && ts.isVariableDeclaration(declaration) && declaration.initializer && !narrowed) { + followed.add(declaration); + checkedReferences.add(expression); + if (skipChanged(declaration, at, unwrap(declaration.initializer))) return; + return value(declaration.initializer, expected, at, omittable); + } + } else if (ts.isConditionalExpression(expression)) { + value(expression.whenTrue, expected, at, omittable); + return value(expression.whenFalse, expected, at, omittable); + } else if ( + ts.isBinaryExpression(expression) && + (expression.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken || + expression.operatorToken.kind === ts.SyntaxKind.BarBarToken) + ) { + // `??` uses the left side unless it's null or undefined, and `||` only when it's truthy + const or = expression.operatorToken.kind === ts.SyntaxKind.BarBarToken; + const falsy = (member) => + member === checker.getFalseType() || ((member.isStringLiteral() || member.isNumberLiteral()) && !member.value); + const skip = (member) => isUndefined(member) || Boolean(member.flags & ts.TypeFlags.Null) || (or && falsy(member)); + return orElse(expression.left, expression.right, skip, expected, at, omittable); + } else if (ts.isCallExpression(expression)) { + const fn = functionOf(checker.getResolvedSignature(expression)?.declaration); + // A generic helper is checked by what its call returns, where its type parameters are known + if (fn?.typeParameters) return typed(checker.getTypeAtLocation(expression), expected, at, omittable, expression); + if (fn && !visiting.has(fn)) { + visiting.add(fn); + const spread = expression.arguments.some(ts.isSpreadElement); + fn.parameters.forEach((parameter, index) => { + const argument = expression.arguments[index]; + const { initializer } = parameter; + if (spread || !ts.isIdentifier(parameter.name) || parameter.dotDotDotToken || !(argument || initializer)) return; + passed.set(parameter, { argument, initializer }); + }); + returns(fn, expected, at, omittable); + fn.parameters.forEach((parameter) => passed.delete(parameter)); + visiting.delete(fn); + return; + } + } else if (ts.isArrayLiteralExpression(expression) && isListType(inner)) { + for (const element of expression.elements) { + if (ts.isSpreadElement(element)) typed(checker.getTypeAtLocation(element), expected, at, omittable, element); + else value(element, inner.ofType, `${at}[]`, false); + } + return; + } + typed(checker.getTypeAtLocation(expression), expected, at, omittable, expression); + }; + + const exported = entrySymbol && checker.getExportsOfModule(entrySymbol).find((symbol) => symbol.name === name); + const resolved = exported && (exported.flags & ts.SymbolFlags.Alias ? checker.getAliasedSymbol(exported) : exported); + const fn = functionOf(resolved?.valueDeclaration); + if (fn && fn.modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.AsyncKeyword)) { + report(fn, `\`${name}\` is async, so it returns a promise instead of its result`); + continue; + } + if (!fn) { + // The bundler reports a missing export + if (resolved?.valueDeclaration) { + report(resolved.valueDeclaration, `\`${name}\` isn't a function it can follow, so it can't be checked`, true); + } + continue; + } + // Muted passes find the variables the result is built from and what changes them, until a pass finds nothing new, + // such as a variable only reached through an assignment's value. The last pass checks the result, skipping the + // variables changed after they're created. + muted = true; + for (let found = -1; found < followed.size + checkedReferences.size; ) { + found = followed.size + checkedReferences.size; + returns(fn, new GraphQLNonNull(result), "", false); + changed.clear(); + assigned.clear(); + for (const declaration of followed) { + const symbol = checker.getSymbolAtLocation(declaration.name); + const nodes = []; + const visit = (node) => { + if ( + ts.isIdentifier(node) && + node !== declaration.name && + !checkedReferences.has(node) && + symbolOf(node) === symbol && + changes(node) + ) { + nodes.push(node); + } + ts.forEachChild(node, visit); + }; + visit(declaration.getSourceFile()); + if (nodes.length === 0) continue; + const object = unwrap(declaration.initializer); + const assignments = nodes.map(assignmentOf); + // An assignment in a nested function, such as a callback, may run at any time or not at all + const nested = nodes.some((node) => functionAround(node) !== functionAround(declaration)); + if (ts.isObjectLiteralExpression(object) && assignments.every(Boolean) && !nested) { + // An assignment in the block the object is created in always runs, unlike one in an `if` or a loop + const block = declaration.parent.parent.parent; + for (const assignment of assignments) { + assignment.always = ts.isExpressionStatement(assignment.node.parent) && assignment.node.parent.parent === block; + } + assigned.set(declaration, { object, nodes, assignments }); + } else { + changed.set(declaration, nodes); + } + } + } + muted = false; + visiting.clear(); + returns(fn, new GraphQLNonNull(result), "", false); + } + + return [...problems].map(([message, warning]) => ({ message, warning })); +} + +module.exports = { prepareVariablesProblems }; diff --git a/tests/check.test.cjs b/tests/check.test.cjs new file mode 100644 index 0000000..7ee46c1 --- /dev/null +++ b/tests/check.test.cjs @@ -0,0 +1,87 @@ +const { test } = require("node:test"); +const assert = require("node:assert/strict"); +const { spawnSync } = require("node:child_process"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); + +const check = path.join(__dirname, "..", "check.js"); + +const schema = ` +scalar JSON +scalar Decimal +input LineSelector @oneOf { id: ID, handle: String } +type CartLine { id: ID! } +type Query { id: ID!, lines(selector: LineSelector!, first: Int): [CartLine!]! } +`; + +const targets = [ + { target: "test.target.prepare", export: "targetPrepare" }, + { target: "test.target.run", export: "targetRun", inputQuery: "src/run.graphql" }, +]; + +// Runs check.js for a function whose prepare target returns `variables`, as Shopify CLI does. +function runCheck(t, { variables = "{}", fn = {}, packageJson } = {}) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "function-")); + t.after(() => fs.rmSync(dir, { recursive: true, force: true })); + fs.mkdirSync(path.join(dir, "src")); + fs.writeFileSync(path.join(dir, "schema.graphql"), schema); + fs.writeFileSync(path.join(dir, "src/run.graphql"), "query Run($selector: LineSelector!, $first: Int, $price: Decimal) { lines(selector: $selector, first: $first) { id } }"); + if (packageJson) fs.writeFileSync(path.join(dir, "package.json"), JSON.stringify(packageJson)); + fs.writeFileSync(path.join(dir, "src/index.js"), `export function targetPrepare() {\n return { variables: ${variables} };\n}\n`); + const json = path.join(dir, "function.json"); + fs.writeFileSync(json, JSON.stringify({ handle: "prepare-test", directory: dir, entry: "src/index.js", schema: "schema.graphql", targets, ...fn })); + return spawnSync(process.execPath, [check, "--function", json], { cwd: dir, encoding: "utf8" }); +} + +test("passes variables the run query accepts", (t) => { + const result = runCheck(t, { variables: '{ selector: { id: "gid://shopify/CartLine/1" }, first: 5 }' }); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stderr, ""); +}); + +test("fails when a prepare target returns variables its run query doesn't accept", (t) => { + const result = runCheck(t, { variables: '{ selectr: { id: "gid://shopify/CartLine/1" }, first: "5" }' }); + assert.equal(result.status, 1); + assert.equal( + result.stderr, + [ + `The function extension "prepare-test" returns prepare variables its run query doesn't accept`, + "src/index.js:2:25 `variables.selectr` isn't declared by src/run.graphql", + 'src/index.js:2:77 `variables.first` must be Int, not "5"', + "src/index.js:2:23 `variables.selector` is missing, and LineSelector! has no default", + "", + ].join("\n"), + ); +}); + +test("warns, without failing, about variables it can't check", (t) => { + const result = runCheck(t, { variables: "{ selector: JSON.parse(\"{}\"), first: 5 }" }); + assert.equal(result.status, 0, result.stderr); + assert.equal( + result.stderr, + [ + `The function extension "prepare-test" returns prepare variables that can't be checked against its run query`, + "src/index.js:2:35 `variables.selector` is typed any, so it can't be checked", + "", + ].join("\n"), + ); +}); + +test("checks custom scalars with the function's codegen config", (t) => { + const variables = '{ selector: { id: "1" }, price: 1.5 }'; + assert.match(runCheck(t, { variables }).stderr, /`variables.price` must be Decimal, not 1.5/); + const packageJson = { codegen: { config: { scalars: { Decimal: "number" } } } }; + assert.equal(runCheck(t, { variables, packageJson }).status, 0); +}); + +test("warns, without failing, when the check itself fails", (t) => { + const result = runCheck(t, { fn: { schema: "missing.graphql" } }); + assert.equal(result.status, 0, result.stderr); + assert.match(result.stderr, /The function extension "prepare-test"'s prepare variables couldn't be checked: ENOENT/); +}); + +test("passes a function without prepare targets", (t) => { + const result = runCheck(t, { fn: { entry: undefined, targets: [targets[1]] } }); + assert.equal(result.status, 0, result.stderr); +}); diff --git a/tests/prepare-variables.test.cjs b/tests/prepare-variables.test.cjs new file mode 100644 index 0000000..b037ef9 --- /dev/null +++ b/tests/prepare-variables.test.cjs @@ -0,0 +1,476 @@ +const { describe, test } = require("node:test"); +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { prepareVariablesProblems } = require("../prepare-variables.js"); + +const schema = ` + scalar JSON + scalar Decimal + scalar Handle + scalar Void + enum Mode { FAST SLOW } + input Selector @oneOf { id: ID, handle: String } + input Rule { handle: String!, weight: Int } + type Query { shop: String, id: ID } + input CartValidationsGeneratePrepareResult { variables: JSON!, cacheKey: String } + type Mutation { cartValidationsGeneratePrepare(result: CartValidationsGeneratePrepareResult!): Void! } +`; +const runQuery = + "query Run($handle: String!, $limit: Int = 5, $mode: Mode, $rules: [Rule!], $data: JSON, $id: ID, $price: Decimal, $selector: Selector, $handles: [Handle!], $nothing: Void) { shop }"; + +const paired = [ + { target: "cart.validations.generate.prepare", export: "cartValidationsGeneratePrepare" }, + { target: "cart.validations.generate.run", inputQuery: "src/run.graphql", export: "cartValidationsGenerateRun" }, +]; + +// Writes a function with `files` in src/ and returns its problems, without their locations. Warnings start with "warning: ". +function check(files, targets = paired) { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "prepare-")); + try { + fs.mkdirSync(path.join(directory, "src")); + fs.writeFileSync(path.join(directory, "schema.graphql"), schema); + fs.writeFileSync(path.join(directory, "src/run.graphql"), runQuery); + for (const [file, content] of Object.entries(files)) fs.writeFileSync(path.join(directory, "src", file), content); + return prepareVariablesProblems({ directory, entry: "src/index.js", schema: "schema.graphql", targets }).map( + ({ message, warning }) => `${warning ? "warning: " : ""}${message.replace(/^\S+:\d+:\d+ /, "")}`, + ); + } finally { + fs.rmSync(directory, { recursive: true, force: true }); + } +} + +const prepare = (body) => ({ "index.js": `export function cartValidationsGeneratePrepare(input) {\n${body}\n}` }); +// A function whose prepare target is written in TypeScript, in src/prepare.ts +const typescript = (source) => ({ "index.js": "export * from './prepare.ts'", "prepare.ts": source }); +// A prepare target that creates `variables` from `object`, then runs `lines` before returning it +const assigned = (object, lines) => check(prepare(`const variables = ${object}\n${lines}\nreturn {variables}`)); + +describe("prepareVariablesProblems", () => { + test("accepts variables the run query declares", () => { + assert.deepEqual( + check( + prepare(` + const region = String(input.region) + return { + variables: { + handle: region ? \`rule-\${region.toLowerCase()}\` : 'default', + limit: undefined, + mode: region === 'eu' ? 'FAST' : null, + rules: [{handle: 'a', weight: 1}], + data: JSON.parse(input.data), + }, + }`), + ), + [], + ); + }); + + test("ignores functions without a prepare target", () => { + assert.deepEqual( + check({}, [{ target: "cart.validations.generate.run", inputQuery: "src/run.graphql", export: "run" }]), + [], + ); + }); + + test("rejects keys the run query does not declare", () => { + assert.deepEqual(check(prepare(`return {variables: {handel: 'a'}}`)), [ + "`variables.handel` isn't declared by src/run.graphql", + "`variables.handle` is missing, and String! has no default", + ]); + }); + + test("rejects values of the wrong type", () => { + assert.deepEqual( + check(prepare(`return {variables: {handle: 1, limit: 1.5, mode: 'MEDIUM', rules: [{handle: 'a', weight: '1'}]}}`)), + [ + "`variables.handle` must be String!, not 1", + "`variables.limit` must be Int, not 1.5", + '`variables.mode` must be Mode, not "MEDIUM"', + '`variables.rules[].weight` must be Int, not "1"', + ], + ); + // Each branch is checked on its own, since TypeScript widens 'FAST' and 'MEDIUM' to string and merges the branches + assert.deepEqual( + check( + prepare(`return input.a ? {variables: {handle: 'a', mode: 'FAST'}} : {variables: {handle: 'a', mode: 'MEDIUM'}}`), + ), + ['`variables.mode` must be Mode, not "MEDIUM"'], + ); + }); + + test("rejects null and undefined where the variable needs a value", () => { + assert.deepEqual(check(prepare(`return input.a ? {variables: {handle: null}} : {variables: {handle: undefined}}`)), [ + "`variables.handle` can be null, but String! can't", + "`variables.handle` can be undefined, but String! has no default", + ]); + assert.deepEqual(check(prepare(`return {}`)), ["`variables` is missing, and Variables! has no default"]); + }); + + test("only fails on values that are definitely wrong", () => { + assert.deepEqual( + check({ + "index.js": ` + const MODES = ['FAST', 'SLOW'] + /** @param {{a: boolean, m: string}} input */ + export function cartValidationsGeneratePrepare(input) { + return {variables: {handle: (input.a && 'x') || 'y', id: 5, mode: input.a ? input.m : MODES[0], price: '1.5'}} + }`, + }), + [ + // One for `input.m`, and one for `MODES[0]` + "warning: `variables.mode` is string, which Mode may not accept, so it can't be checked", + "warning: `variables.mode` is string, which Mode may not accept, so it can't be checked", + ], + ); + assert.deepEqual(check(prepare(`return {variables: {handle: (input.a && false) || true, id: 1.5}}`)), [ + "warning: `variables.handle` is typed any, so it can't be checked", + "`variables.handle` must be String!, not true", + "`variables.id` must be ID, not 1.5", + ]); + }); + + test("checks the prepare result's other fields from the schema", () => { + assert.deepEqual(check(prepare(`return {variables: {handle: 'a'}, cacheKey: 'k'}`)), []); + assert.deepEqual(check(prepare(`return {variables: {handle: 'a'}, cacheKey: 5}`)), ["`cacheKey` must be String, not 5"]); + assert.deepEqual(check(prepare(`return {variables: {handle: 'a'}, cacheKy: 'k'}`)), [ + "`cacheKy` isn't a field of CartValidationsGeneratePrepareResult", + ]); + }); + + test("rejects an async prepare target", () => { + assert.deepEqual(check({ "index.js": "export async function cartValidationsGeneratePrepare() { return {variables: {handle: 'a'}} }" }), [ + "`cartValidationsGeneratePrepare` is async, so it returns a promise instead of its result", + ]); + }); + + test("checks custom scalars as the type codegen gives them", () => { + const vars = (value, scalars) => + check({ + ...prepare(`return {variables: {handle: 'a', ${value}}}`), + "../package.json": JSON.stringify({ codegen: { config: { scalars } } }), + }); + assert.deepEqual(vars(`handles: ['h']`), []); + assert.deepEqual(vars(`handles: ['h', null]`), ["`variables.handles[]` can be null, but Handle! can't"]); + assert.deepEqual(vars(`handles: [5]`), ["`variables.handles[]` must be Handle!, not 5"]); + assert.deepEqual(vars(`price: '1.5'`), []); + assert.deepEqual(vars(`price: 1.5`), ["`variables.price` must be Decimal, not 1.5"]); + assert.deepEqual(vars(`price: 1.5`, { Decimal: "number" }), []); + assert.deepEqual(vars(`price: 1.5`, { Decimal: { input: "unknown", output: "string" } }), []); + assert.deepEqual(vars(`price: 1.5`, null), ["`variables.price` must be Decimal, not 1.5"]); + assert.deepEqual(vars(`data: {a: [1]}`), []); + assert.deepEqual(vars(`nothing: null`), []); + assert.deepEqual(vars(`nothing: 'x'`), ["`variables.nothing` must be Void, not \"x\""]); + }); + + test("checks a value with toJSON as what it returns", () => { + const vars = (value) => check(prepare(`return {variables: {handle: ${value}}}`)); + assert.deepEqual(vars(`new Date()`), []); + assert.deepEqual(vars(`{toJSON: () => 'a'}`), []); + assert.deepEqual(vars(`{toJSON: () => 5}`), ["`variables.handle` must be String!, not number"]); + }); + + test("applies oneOf input object rules", () => { + const selector = (value) => check(prepare(`return {variables: {handle: 'a', selector: ${value}}}`)); + assert.deepEqual(selector(`{id: '1'}`), []); + assert.deepEqual(selector(`{id: '1', handle: 'x'}`), ["`variables.selector` must set exactly one field of Selector"]); + assert.deepEqual(selector(`{}`), ["`variables.selector` must set exactly one field of Selector"]); + assert.deepEqual(selector(`{id: null}`), ["`variables.selector.id` can be null, but ID! can't"]); + assert.deepEqual(selector(`{id: '1', handle: undefined}`), []); + assert.deepEqual(selector(`{id: undefined, handle: undefined}`), ["`variables.selector` must set exactly one field of Selector"]); + assert.deepEqual(selector(`{id: input.a ? '1' : undefined, handle: 'x'}`), [ + "warning: `variables.selector` may not set exactly one field of Selector, so it can't be checked", + ]); + }); + + test("treats an array element as possibly undefined", () => { + assert.deepEqual( + check(typescript("export function cartValidationsGeneratePrepare(input: {list: string[]}) { return {variables: {handle: input.list[0]}} }")), + ["`variables.handle` can be undefined, but String! has no default"], + ); + }); + + test("keeps the narrowing of a variable where it's used", () => { + const narrowed = (body) => + check(typescript(`export function cartValidationsGeneratePrepare(input: {handle?: string}) {\nconst handle = input.handle\n${body}\n}`)); + assert.deepEqual(narrowed(`return {variables: {handle: typeof handle === 'string' ? handle : 'fallback'}}`), []); + assert.deepEqual(narrowed(`if (handle) return {variables: {handle}}\nreturn {variables: {handle: 'b'}}`), []); + assert.deepEqual(narrowed(`return {variables: {handle}}`), ["`variables.handle` can be undefined, but String! has no default"]); + assert.deepEqual(narrowed(`return {variables: {handle: notDeclared}}`), ["warning: `variables.handle` is typed any, so it can't be checked"]); + }); + + test("accepts an empty list typed never[]", () => { + const files = typescript("export function cartValidationsGeneratePrepare() { return {variables: {handle: 'a', rules: [].slice()}} }"); + assert.deepEqual(check(files), []); + }); + + test("trusts non-null assertions", () => { + assert.deepEqual( + check(typescript("export function cartValidationsGeneratePrepare(input: {list: string[]}) { return {variables: {handle: input.list.find((x) => x)!}} }")), + [], + ); + }); + + test("checks what is built, not what the return type declares", () => { + assert.deepEqual( + check({ + "index.js": ` + /** @returns {{variables: {handle: string}}} */ + export function cartValidationsGeneratePrepare() { + const variables = {handle: 'a', region: 'eu'} + return {variables} + }`, + }), + ["`variables.region` isn't declared by src/run.graphql"], + ); + }); + + test("ignores what annotations and casts claim", () => { + assert.deepEqual( + check( + prepare(` + const raw = /** @type {{handle: string}} */ (JSON.parse(input.raw)) + const {limit} = /** @type {{limit: number}} */ (JSON.parse(input.raw)) + return {variables: {handle: raw.handle, limit}}`), + ), + ["warning: `variables.handle` is typed any, so it can't be checked", "warning: `variables.limit` is typed any, so it can't be checked"], + ); + assert.deepEqual( + check({ + "index.js": `export * from './prepare.js'`, + "prepare.ts": ` + type Variables = {handle: string} + export function cartValidationsGeneratePrepare(input: {name?: string; mode: string}): {variables: Variables} { + if (input.mode) return {variables: {handle: input.name!, mode: <'FAST'>input.mode}} + return {variables: new Map().get(input.mode)! as Variables} + }`, + }), + [ + "warning: `variables.mode` is string, which Mode may not accept, so it can't be checked", + "warning: `variables` is typed any, so it can't be checked", + ], + ); + }); + + test("keeps what a return returns when it removes a cast", () => { + const files = typescript("export function cartValidationsGeneratePrepare() {\n return \n ({variables: {handle: 5}});\n}"); + assert.deepEqual(check(files), ["`variables.handle` must be String!, not 5"]); + }); + + test("sees through casts, @ts-ignore and helpers", () => { + assert.deepEqual( + check({ + "index.js": ` + export function cartValidationsGeneratePrepare(input) { + // @ts-ignore + if (input.a) return {variables: /** @type {{handle: string}} */ (JSON.parse(input.raw))} + return {variables: build()} + } + function build() { + return {handle: 2} + }`, + }), + ["warning: `variables` is typed any, so it can't be checked", "`variables.handle` must be String!, not 2"], + ); + }); + + test("checks the last value of a key written twice", () => { + assert.deepEqual(check(prepare(`return {variables: {handle: 'a', handle: 5}}`)), ["`variables.handle` must be String!, not 5"]); + }); + + test("checks objects combined with Object.assign", () => { + assert.deepEqual(check(prepare(`return {variables: Object.assign({}, {handle: 'a'}, {limit: 2})}`)), []); + assert.deepEqual(check(prepare(`return {variables: Object.assign({}, {handle: 'a'}, {region: 'eu'})}`)), [ + "`variables.region` isn't declared by src/run.graphql", + ]); + }); + + test("warns about spreading an object that can have any keys", () => { + assert.deepEqual( + check({ + "index.js": `/** @param {{json: Record}} input */ + export function cartValidationsGeneratePrepare(input) { return {variables: {handle: 'a', ...input.json}} }`, + }), + ["warning: `variables` spreads an object that can have any keys, so it can't be checked"], + ); + }); + + test("warns about, rather than checks, variables changed or passed on after they are created", () => { + assert.deepEqual( + check( + prepare(` + const variables = {handle: 'a'} + variables.limit = 'many' + tweak(variables) + return {variables}`), + ), + [ + "warning: `variables` is changed or passed on after it's created, so it can't be checked", + "warning: `variables` is changed or passed on after it's created, so it can't be checked", + ], + ); + assert.deepEqual( + check( + prepare(` + const base = {handle: 'a'} + base.handle = 1 + return {variables: {...base}}`), + ), + ["warning: `variables` is changed or passed on after it's created, so it can't be checked"], + ); + }); + + test("checks variables assigned by name after the object is created", () => { + assert.deepEqual(assigned(`{}`, `variables.handle = 'a'\nvariables['limit'] = 5`), []); + assert.deepEqual(assigned(`{handle: null}`, `variables.handle = 'a'`), []); + assert.deepEqual(assigned(`{handle: 'a'}`, `if (input.a) variables.region = 'eu'`), [ + "`variables.region` isn't declared by src/run.graphql", + ]); + assert.deepEqual(assigned(`{handle: 'a'}`, `variables.limit = 'many'`), ['`variables.limit` must be Int, not "many"']); + assert.deepEqual(assigned(`{}`, `variables.limit = 5`), ["`variables.handle` is missing, and String! has no default"]); + // Keys that aren't written out, and objects spread after they're assigned to, still can't be checked + assert.deepEqual(assigned(`{handle: 'a'}`, `variables[input.key] = 5`), [ + "warning: `variables` is changed or passed on after it's created, so it can't be checked", + ]); + assert.deepEqual(check(prepare(`const base = {}\nbase.handle = 'a'\nreturn {variables: {...base}}`)), [ + "warning: `variables` is changed or passed on after it's created, so it can't be checked", + ]); + }); + + test("uses the value of a key in brackets", () => { + assert.deepEqual(assigned(`{}`, `const key = 'handle'\nvariables[key] = 'a'`), []); + assert.deepEqual(assigned(`{}`, `let key = 'handle'\nvariables[key] = 'a'`), [ + "warning: `variables` is changed or passed on after it's created, so it can't be checked", + ]); + }); + + test("warns about a required variable that's only set on some paths", () => { + assert.deepEqual(assigned(`{}`, `variables.handle = 'a'`), []); + for (const lines of [ + `if (input.a) variables.handle = 'a'`, + `for (const x of input.list) variables.handle = 'a'`, + ]) { + assert.deepEqual(assigned(`{}`, lines), ["warning: `variables.handle` is only set on some paths, so it can't be checked"], lines); + } + }); + + test("follows a variable reached through an assignment's value", () => { + const body = `const variables = {}\nvariables.handle = ${"VALUE"}\nconst result = {}\nresult.variables = variables\nreturn result`; + assert.deepEqual(check(prepare(body.replace("VALUE", "'a'"))), []); + assert.deepEqual(check(prepare(body.replace("VALUE", "5"))), ["`variables.handle` must be String!, not 5"]); + }); + + test("checks the writes to a key that can be returned", () => { + assert.deepEqual(assigned(`{}`, `variables.handle = null\nvariables.handle = 'a'`), []); + assert.deepEqual(assigned(`{}`, `if (input.a) variables.handle = null\nvariables.handle = 'a'`), []); + assert.deepEqual(assigned(`{}`, `variables.handle = 'a'\nvariables.handle = null`), ["`variables.handle` can be null, but String! can't"]); + assert.deepEqual(assigned(`{handle: null}`, `if (input.a) variables.handle = 'a'`), ["`variables.handle` can be null, but String! can't"]); + }); + + test("skips a write in a block that returns or throws first", () => { + assert.deepEqual(assigned(`{handle: 'a'}`, `if (input.a) { variables.handle = null; return {variables: {handle: 'b'}} }`), []); + assert.deepEqual(assigned(`{handle: 'a'}`, `if (input.a) { variables.handle = null; throw new Error('x') }`), []); + assert.deepEqual(assigned(`{handle: 'a'}`, `if (input.a) { variables.handle = null; return {variables} }`), ["`variables.handle` can be null, but String! can't"]); + assert.deepEqual(assigned(`{handle: 'a'}`, `if (input.a) { variables.handle = null }`), ["`variables.handle` can be null, but String! can't"]); + }); + + test("only checks assignments that can change what's returned", () => { + const built = (lines) => check(prepare(`const variables = {handle: 'a'}\n${lines}`)); + assert.deepEqual(built(`const later = () => { variables.limit = 'x' }\nreturn {variables}`), [ + "warning: `variables` is changed or passed on after it's created, so it can't be checked", + ]); + assert.deepEqual(built(`if (input.a) return {variables}\nvariables.limit = 'x'\nreturn {variables: {handle: 'b'}}`), []); + assert.deepEqual(built(`const result = {variables}\nvariables.limit = 'x'\nreturn result`), ["`variables.limit` must be Int, not \"x\""]); + assert.deepEqual(built(`for (const x of input.list) { if (x) return {variables}; variables.limit = 'x' }\nreturn {variables: {handle: 'b'}}`), [ + "`variables.limit` must be Int, not \"x\"", + ]); + }); + + test("follows the arguments a helper is called with", () => { + assert.deepEqual( + check({ + "index.js": `/** @param {{n: number}} input */ + export function cartValidationsGeneratePrepare(input) { return {variables: build(input.n)} } + /** @param {string} handle */ + function build(handle) { return {handle} }`, + }), + ["`variables.handle` must be String!, not number"], + ); + assert.deepEqual( + check(typescript("function id(x: T): T { return x }\nexport function cartValidationsGeneratePrepare() { return {variables: id({handle: 'a'})} }")), + [], + ); + }); + + test("checks a parameter a helper narrows or changes as its type there", () => { + const helper = (source) => + check(typescript(`${source}\nexport function cartValidationsGeneratePrepare() { return {variables: build(7)} }`)); + assert.deepEqual(helper("function build(handle: string | number) { handle = String(handle); return {handle} }"), []); + assert.deepEqual(helper("function build(handle: unknown) { if (typeof handle === 'string') return {handle}; return {handle: 'x'} }"), []); + assert.deepEqual(helper("function build(handle: string | number) { handle = 5; return {handle} }"), ["`variables.handle` must be String!, not number"]); + assert.deepEqual(helper("function build(handle: unknown) { handle = String(handle); return {handle} }"), [ + "warning: `variables.handle` is typed unknown, so it can't be checked", + ]); + assert.deepEqual(check(prepare("function build(handle) { handle = String(handle); return {handle} }\nreturn {variables: build(7)}")), [ + "warning: `variables.handle` is typed any, so it can't be checked", + ]); + }); + + test("uses a helper's default where its argument is undefined", () => { + const build = (call) => + check(typescript(`function build(handle = 'a') { return {handle} }\nexport function cartValidationsGeneratePrepare(input: {h?: string, n?: number}) { return {variables: ${call}} }`)); + assert.deepEqual(build(`build(undefined)`), []); + assert.deepEqual(build(`build(input.h)`), []); + assert.deepEqual(build(`build(input.n)`), ["`variables.handle` must be String!, not number"]); + }); + + test("checks a generic helper by what its call returns", () => { + const generic = (variables) => + check(typescript(`function first(...xs: T[]): T { return xs[0]! }\nexport function cartValidationsGeneratePrepare(input: {t: T}) { return {variables: ${variables}} }`)); + assert.deepEqual(generic(`{handle: first(...['a'])}`), []); + assert.deepEqual(generic(`{handle: 'a', limit: first('x')}`), ["`variables.limit` must be Int, not \"x\""]); + assert.deepEqual(generic(`{handle: input.t}`), ["warning: `variables.handle` is typed T, so it can't be checked"]); + }); + + test("follows re-exports", () => { + assert.deepEqual( + check({ + "index.js": `export * from './prepare.js'`, + "prepare.js": `export const cartValidationsGeneratePrepare = () => ({variables: {handle: 1}})`, + }), + ["`variables.handle` must be String!, not 1"], + ); + }); + + test("warns about an export it can't follow, and leaves missing exports to the bundler", () => { + assert.deepEqual( + check({ + "index.js": ` + const wrap = (fn) => fn + export const cartValidationsGeneratePrepare = wrap(() => ({variables: {handle: 'a'}}))`, + }), + ["warning: `cartValidationsGeneratePrepare` isn't a function it can follow, so it can't be checked"], + ); + assert.deepEqual( + check({ + "index.js": ` + const handlers = {cartValidationsGeneratePrepare() { return {variables: {handle: 'a'}} }} + export const cartValidationsGeneratePrepare = handlers.cartValidationsGeneratePrepare`, + }), + ["warning: `cartValidationsGeneratePrepare` isn't a function it can follow, so it can't be checked"], + ); + assert.deepEqual(check({ "index.js": "export function other() {}" }), []); + }); + + test("warns about a syntax error, even in a file the bundle leaves out", () => { + // A function missing its closing brace + assert.deepEqual(check({ "index.js": "export function cartValidationsGeneratePrepare() {\n return {variables: {handle: 1}}" }), [ + "warning: can't be checked because of a syntax error: '}' expected.", + ]); + assert.deepEqual( + check({ ...typescript("import type { X } from './types.ts'\nexport function cartValidationsGeneratePrepare() { return {variables: {handle: 1}} }"), "types.ts": "export type X = { a: ; }" }), + ["warning: can't be checked because of a syntax error: Type expected."], + ); + }); +});