From e02172ef5b7b2468024d3de6b1ccdef5b8471872 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Wed, 28 Oct 2026 10:45:12 +0200 Subject: [PATCH 1/3] feat(analyzer): analyse composables in .uf.ts modules A `.uf.ts` module's named `use*` exports are composables (ADR-0069). Each body is declared and lowered by the component setup's own pass, with parameters read as `argument` bindings in place of props and a returned shorthand object in place of JSX. The module's IR holds `UfComposable`, and its API a `ComposableApi` per composable, with each member's kind, the flags of its functions and `hasEffects`. UF2035 reports each form ADR-0069 rejects, at its span: the APIs that need a component, a call of another composable, a default export, a return of any other shape, and parameters a target cannot read by kind. Co-authored-by: multica-agent --- packages/analyzer/README.md | 25 +- packages/analyzer/src/analyze.ts | 207 ++++++- packages/analyzer/src/composables.ts | 608 +++++++++++++++++++++ packages/analyzer/src/expressions.ts | 11 + packages/analyzer/src/render.ts | 2 + packages/analyzer/src/setup.ts | 60 +- packages/analyzer/test/composables.test.ts | 387 +++++++++++++ 7 files changed, 1279 insertions(+), 21 deletions(-) create mode 100644 packages/analyzer/src/composables.ts create mode 100644 packages/analyzer/test/composables.test.ts diff --git a/packages/analyzer/README.md b/packages/analyzer/README.md index 0e1762e6..9f862523 100644 --- a/packages/analyzer/README.md +++ b/packages/analyzer/README.md @@ -3,7 +3,7 @@ Passes P2 (analyse) and P3 (lower) of the Unframework compiler (plan §5.1): - `analyze(parsed)` checks a parsed `.uf.tsx` module and lowers each component's returned JSX into - the portable IR. It reports every construct it cannot lower as a diagnostic, and never copies + the portable IR, or a `.uf.ts` module and its composables (ADR-0069). It reports every construct it cannot lower as a diagnostic, and never copies code through unanalysed (P2). It never throws. A syntax error stops it. Any other module-level error (an import, a top-level declaration) drops every component, which are still checked, so that fixing it reveals nothing new; an error in a component drops that component only. @@ -115,6 +115,29 @@ checked as the declaration its likely fix writes (`analyze.ts`). or a template without holes; `unfollowedImports(parsed)` lists what it loads that the compiler cannot follow (any other `import()`, `import.meta.glob`), which is UF1202. +- **Composables** (`composables.ts`, ADR-0069): a `.uf.ts` module holds no component, and each + named export `use[A-Z]…`, a function declaration, is a composable, lowered into + `UfModule.composables`. Its body is a setup under a component's rules (`declareSetup`), with + parameters in place of props and a returned object in place of JSX. + - Each parameter is a name with a type, an `argument` binding read as a destructured prop is. + Its type gives its kind: a function type without parameters is a getter, any other function + type a callback, which only client code reads, and anything else a value. + - The last statement returns a shorthand object of the setup's state, derived values, local + functions and constants. + - The module's API lists a `ComposableApi` per composable: each parameter's kind and type, each + member's kind and its type where the source writes one, whether a returned function is + `async`, and `pure` or `client` by its summary, and `hasEffects` where the setup holds a + watcher or a lifecycle hook. A composable with an error is left out of it. + - UF2035 rejects a call of `useId`, `useTemplateRef`, `provide`, `inject` or a `define*` macro, + a call of another composable (one the module declares, or imports from a `.uf.ts`), a default + export, an export named otherwise, a `use*` function the module does not export or writes as + a value, an `async` or generator function, a parameter without a type, with a default value or + written as a pattern, a callback read outside client code, a return of any other shape, and a + module that exports no composable. An injection key in a `.uf.ts` module is UF1002: it stays + in the `.uf.tsx` module that provides it (ADR-0054). + - The call site, `const { on, toggle } = useToggle(false)`, and `compile()`'s handling of a + `.uf.ts` source come later. + - **The authoring API** (`authoring.ts`): named imports from `"unframework"`, recognised by the binding each declares (ADR-0006), as the package exports them (a test reads its `index.ts`). Anything else is UF2016. diff --git a/packages/analyzer/src/analyze.ts b/packages/analyzer/src/analyze.ts index 0706b0cb..4e6b9ffd 100644 --- a/packages/analyzer/src/analyze.ts +++ b/packages/analyzer/src/analyze.ts @@ -20,6 +20,7 @@ import { walk, } from "@unframework/ir"; import type { + Binding, ComponentApi, ImportedName, InjectionKeyDeclaration, @@ -51,6 +52,8 @@ import { componentApi, withReferences } from "./api.ts"; import type { ApiContext } from "./api.ts"; import { authoringBindings, checkAuthoringImport } from "./authoring.ts"; import type { AuthoringApi } from "./authoring.ts"; +import { analyzeComposable, COMPOSABLE_FILE, findComposables } from "./composables.ts"; +import type { AnalyzedComposable, ComposableCandidate } from "./composables.ts"; import { Reporter } from "./context.ts"; import { checkTypeDeclaration, closure, collectTypes, setupTypes } from "./declarations.ts"; import type { ModuleTypes } from "./declarations.ts"; @@ -77,6 +80,7 @@ import { scopeIdOf, STYLESHEET_SPECIFIER, stylesheetImport } from "./styles.ts"; import type { LoweredSheet, StylesheetSource } from "./styles.ts"; import { syntaxError } from "./syntax.ts"; import { kindsFromShape } from "./types/from-shape.ts"; +import type { Kinds } from "./types/kinds.ts"; /** The result of analysing one module. */ export interface AnalyzeResult { @@ -166,7 +170,10 @@ interface Candidate { value?: AST.VariableDeclaration; } -/** Analyses a parsed `.uf.tsx` module and lowers its components into IR (passes P2 and P3). */ +/** + * Analyses a parsed `.uf.tsx` module and lowers its components into IR (passes P2 and P3), or a + * `.uf.ts` module and its composables (ADR-0069). + */ export function analyze(parsed: ParsedModule, options: AnalyzeOptions = {}): AnalyzeResult { return analyzeModule(parsed, undefined, options); } @@ -199,11 +206,24 @@ export function analyzeModule( return { module: undefined, diagnostics: reporter.diagnostics, api: undefined }; } - const candidates: Candidate[] = [ - ...findComponents(parsed.program), - ...findValueComponents(parsed.program), - ].toSorted((a, b) => a.span.start - b.span.start); - const componentStatements = new Set(candidates.map((candidate) => candidate.span.start)); + // A `.uf.ts` module holds composables and no component (ADR-0069). + const composableModule = COMPOSABLE_FILE.test(parsed.file); + const candidates: Candidate[] = composableModule + ? [] + : [...findComponents(parsed.program), ...findValueComponents(parsed.program)].toSorted( + (a, b) => a.span.start - b.span.start, + ); + // Its top-level forms are the module's: an error in one drops every composable (UF2035). + const foundMark = reporter.diagnostics.length; + const found = composableModule + ? findComposables(parsed.program, exportedNames(parsed.program), reporter) + : undefined; + const foundErrors = reporter.hasErrorsSince(foundMark); + const composableCandidates = found?.candidates ?? []; + const componentStatements = new Set([ + ...candidates.map((candidate) => candidate.span.start), + ...(found?.statements ?? []), + ]); const listed = listedTypeExports(parsed.program); const declarations = findTypeDeclarations(parsed.program).map((declaration) => listed.has(declaration.name) ? { ...declaration, exported: true } : declaration, @@ -227,6 +247,8 @@ export function analyzeModule( const types = collectTypes(declarations, candidates, [ ...[...keyCandidates.values()].flatMap((candidate) => (candidate.type ? [candidate.type] : [])), ...constantTypes.values(), + // A composable's types are its setup's, its parameters' included (ADR-0069). + ...composableCandidates.map(({ node }) => [node.typeParameters, node.params, node.body]), ]); const scopes = new Scopes(parsed.program); const authoring = authoringBindings(parsed.program); @@ -252,6 +274,15 @@ export function analyzeModule( const keyDeclarations: InjectionKeyDeclaration[] = []; const keys = new Map(); for (const candidate of keyCandidates.values()) { + if (composableModule) { + // `provide` and `inject` are a component's, so a key stays in the module of the component + // that provides it (ADR-0054, ADR-0069). + reporter.unsupported( + candidate.id, + "An injection key is declared in the `.uf.tsx` module of the component that provides it, and a composable neither provides nor injects.", + ); + continue; + } // A key reported here is still one `provide` and `inject` may name: their uses are checked // as its fix would leave them, and report nothing more (one code per misuse). const ref = refTypeOf(candidate.type); @@ -295,6 +326,8 @@ export function analyzeModule( moduleImports: [], shared: { values: new Map(), failed: new Set() }, constants: [], + composableModule, + composableImports: new Set(), }; const moduleMark = reporter.diagnostics.length; for (const statement of parsed.program.body) { @@ -312,6 +345,30 @@ export function analyzeModule( // A module-level error drops every component, but they are still checked: fixing it must // reveal nothing new (the harness's L1). const moduleErrors = keyErrors || reporter.hasErrorsSince(moduleMark); + if (composableModule) { + const errors = moduleErrors || foundErrors; + return composableResult( + composableCandidates, + errors, + constants, + constantTypes, + { + parsed, + types, + shapes, + scopes, + authoring, + reporter, + components: new Map(), + keys, + passThrough: context.passThrough, + shared: context.shared, + composableImports: context.composableImports, + writtenType: writtenTypeIn({ parsed, types, shapes, localShapes }), + }, + context, + ); + } // The module's one component owns its sheets (ADR-0063): its API carries their scope id, as // the one a parent reads through the resolver, and so does its own render of itself. const [sheetOwner] = candidates; @@ -443,13 +500,98 @@ export function analyzeModule( }; } +/** + * Lowers a `.uf.ts` module's composables, once its top-level statements are checked (ADR-0069): + * its IR holds them and no component, and its API what a caller reads of each. + */ +function composableResult( + candidates: readonly ComposableCandidate[], + moduleErrors: boolean, + constants: readonly ModuleConstant[], + constantTypes: ReadonlyMap, + module: Parameters[2], + context: ModuleContext, +): AnalyzeResult { + const { parsed, reporter, types } = module; + const lowered: AnalyzedComposable[] = []; + for (const candidate of candidates) { + const result = analyzeComposable( + candidate, + typeParametersOf(candidate.node, parsed.source), + module, + ); + if (result) lowered.push(result); + } + const api: ModuleApi = { + file: parsed.file, + components: [], + keys: [], + ...(lowered.length ? { composables: lowered.map(({ api }) => api) } : {}), + }; + const uncalled = + !reporter.diagnostics.some(({ severity }) => severity === "error") && + reportUnread(parsed.program.body, context.passThrough, reporter); + if (moduleErrors || uncalled) + return { module: undefined, diagnostics: reporter.diagnostics, api }; + if (!candidates.length) { + reporter.report("UF2035", { start: 0, end: 0 }, "This file exports no composable.", { + help: "A module of composables exports functions named `use` and an upper-case letter, each returning a shorthand object.", + }); + return { module: undefined, diagnostics: reporter.diagnostics, api }; + } + if (lowered.length < candidates.length) { + return { module: undefined, diagnostics: reporter.diagnostics, api }; + } + const composables = lowered.map(({ composable }) => composable); + const read = readConstants(constants, composables); + const used = new Set([ + ...composables.flatMap((composable) => composable.types), + ...closure( + read.map(({ binding }) => constantTypes.get(binding)), + types.table, + ), + ]); + return { + module: createModule( + parsed.file, + [], + [], + types.declarations + .filter((declaration) => used.has(declaration.name)) + .map((declaration) => + createTypeDeclaration( + declaration.name, + declaration.exported, + parsed.source.slice(declaration.node.start, declaration.node.end), + spanOf(declaration.node), + ), + ), + [], + [], + context.passThroughImports, + { + typeImports: context.typeImports, + typeExports: [ + ...context.typeExports, + ...declaredTypeExports(types, parsed.source), + ].toSorted((a, b) => a.span.start - b.span.start), + moduleImports: context.moduleImports, + constants: read, + composables, + }, + ), + diagnostics: reporter.diagnostics, + api, + }; +} + /** * The module's constants its components read, and those they read in turn, in source order * (ADR-0070): what every output declares. */ function readConstants( constants: readonly ModuleConstant[], - components: readonly UfComponent[], + components: readonly { bindings: readonly Binding[] }[], ): ModuleConstant[] { const byId = new Map(constants.map((constant) => [constant.binding, constant])); const read = new Set(); @@ -625,6 +767,10 @@ interface ModuleContext { shared: SharedValues; /** The module's top-level `const`s, in source order, lowered once every import is known. */ constants: ConstantCandidate[]; + /** Whether the module is a `.uf.ts` module of composables (ADR-0069). */ + composableModule: boolean; + /** In a `.uf.ts` module, the names it imports from other `.uf.ts` modules (UF2035). */ + composableImports: Set; } /** The local names a module exports as values in `export { name }` and `export default name`. */ @@ -717,6 +863,19 @@ function checkTopLevelStatement( ); return; } + if ( + context.composableModule && + COMPOSABLE_FILE.test(statement.source.value) && + statement.importKind !== "type" + ) { + // A composable that calls another is reported at the call (UF2035). + for (const specifier of statement.specifiers) { + if (specifier.type !== "ImportSpecifier" || specifier.importKind !== "type") { + context.composableImports.add(specifier.local); + } + } + return; + } if (STYLESHEET_SPECIFIER.test(statement.source.value)) { stylesheetImport( statement, @@ -1476,6 +1635,27 @@ interface ComponentModule { observe?: SetupObserver; } +/** + * What a written type gives a value (`SetupContext.writtenType`). A written type that reaches an + * imported type is one `shapeQueries` asks the oracle for; any other resolves in the module, + * which follows its `extends` and aliases as the oracle does. + */ +function writtenTypeIn( + module: Pick, +): (type: AST.TSType) => { kinds: Kinds; complete: boolean } { + const { parsed, types, shapes } = module; + return (type) => { + const at = { start: type.start, end: type.end }; + const shape = shapes(at) ?? module.localShapes(at); + // A shape that does not resolve in full keeps what it could not evaluate as an object (a + // branded `string & {}`), so its kinds are read as the module's own types give them, which + // leave such a type `unknown` (ADR-0035). + return shape && !shape.unresolved.length + ? { kinds: kindsFromShape(shape, shapeReading(parsed, types.table, at)), complete: true } + : { kinds: types.table.kindsOf(type), complete: false }; + }; +} + /** Checks a component's shape, its props and its setup, and lowers its returned JSX. */ function analyzeComponent(candidate: Candidate, module: ComponentModule): UfComponent | undefined { const { parsed, types, shapes, scopes, authoring, reporter } = module; @@ -1504,18 +1684,7 @@ function analyzeComponent(candidate: Candidate, module: ComponentModule): UfComp : last?.type === "ReturnStatement" ? last.argument : undefined; - // A written type that reaches an imported type is one `shapeQueries` asks the oracle for; any - // other resolves in the module, which follows its `extends` and aliases as the oracle does. - const writtenType = (type: AST.TSType) => { - const at = { start: type.start, end: type.end }; - const shape = shapes(at) ?? module.localShapes(at); - // A shape that does not resolve in full keeps what it could not evaluate as an object (a - // branded `string & {}`), so its kinds are read as the module's own types give them, which - // leave such a type `unknown` (ADR-0035). - return shape && !shape.unresolved.length - ? { kinds: kindsFromShape(shape, shapeReading(parsed, types.table, at)), complete: true } - : { kinds: types.table.kindsOf(type), complete: false }; - }; + const writtenType = writtenTypeIn(module); // The setup (ADR-0045): every statement before the return, declared before any code is walked. const setup = declareSetup({ source: parsed.source, diff --git a/packages/analyzer/src/composables.ts b/packages/analyzer/src/composables.ts new file mode 100644 index 00000000..a1b973c2 --- /dev/null +++ b/packages/analyzer/src/composables.ts @@ -0,0 +1,608 @@ +// Composables (ADR-0069): the named `use*` exports of a `.uf.ts` module, each a setup under a +// component's rules (ADR-0045), with parameters in place of props and a returned shorthand object +// in place of JSX. The setup is declared and lowered by `declareSetup`, as a component's is, with +// the APIs that need a component rejected (UF2035); the parameters are read as a component's +// destructured props are, through `argument` bindings. + +import { + createBinding, + createComponent, + createComposable, + createComposableParameter, + createFragment, + createReturnedMember, + createTypeText, + impurity, + summarize, +} from "@unframework/ir"; +import type { + Binding, + ComposableApi, + ComposableMemberApi, + ComposableParameter, + ComposableParameterKind, + ReturnedMember, + TypeParameter, + UfComposable, +} from "@unframework/ir"; +import { exportName, visitorKeys } from "@unframework/parser"; +import type { AST, ParsedModule } from "@unframework/parser"; + +import type { AuthoringApi } from "./authoring.ts"; +import type { Reporter } from "./context.ts"; +import { closure } from "./declarations.ts"; +import type { ModuleTypes } from "./declarations.ts"; +import type { PassThrough } from "./pass-through.ts"; +import { checkTypeParameters } from "./props.ts"; +import type { PropsAnalysis } from "./props.ts"; +import type { ComponentInfo, PropBinding, RenderContext, SetupBinding } from "./render.ts"; +import { checkRules } from "./rules.ts"; +import type { Scopes } from "./scope.ts"; +import { declareSetup } from "./setup.ts"; +import type { InjectionKeyInfo } from "./setup.ts"; +import type { Shapes } from "./shapes.ts"; +import type { SharedValues } from "./shared.ts"; +import { FUNCTION, union, UNDEFINED } from "./types/kinds.ts"; +import type { Kinds } from "./types/kinds.ts"; + +/** A module of composables is a `.uf.ts` file (D1, ADR-0005, ADR-0069). */ +export const COMPOSABLE_FILE: RegExp = /\.uf\.ts$/; + +/** A composable's name: `use` and an upper-case letter, in ASCII letters and digits. */ +const COMPOSABLE_NAME = /^use[A-Z][A-Za-z0-9]*$/; + +/** The help of UF2035 for a composable that is not a named `use*` function declaration. */ +const DECLARATION_HELP = + "Declare it as a named export: `export function useToggle(initial: boolean) { …; return { on, toggle }; }`."; + +/** A composable candidate: an exported `use*` function declaration of the module. */ +export interface ComposableCandidate { + name: string; + node: AST.Function; + /** The statement that declares it, including any `export` keyword. */ + span: { start: number; end: number }; +} + +/** The top-level statements of a `.uf.ts` module that declare or export composables. */ +export interface FoundComposables { + candidates: ComposableCandidate[]; + /** The starts of the statements read here, reported or not, which no other check reads. */ + statements: Set; +} + +/** + * Finds a `.uf.ts` module's composables: its function declarations named `use*` and exported by + * name. A default export, a `use*` function the module keeps to itself, an exported function + * named otherwise, and a composable written as a value are UF2035. + */ +export function findComposables( + program: AST.Program, + exported: ReadonlySet, + reporter: Reporter, +): FoundComposables { + const candidates: ComposableCandidate[] = []; + const statements = new Set(); + const functions = new Map(); + for (const statement of program.body) { + const fn = + statement.type === "FunctionDeclaration" + ? statement + : statement.type === "ExportNamedDeclaration" && + statement.declaration?.type === "FunctionDeclaration" + ? statement.declaration + : undefined; + if (fn?.id) functions.set(fn.id.name, fn); + } + for (const statement of program.body) { + switch (statement.type) { + case "ExportDefaultDeclaration": + statements.add(statement.start); + reporter.report( + "UF2035", + statement, + "A module of composables has no default export: each composable is a named export, which a component imports by its name.", + { help: DECLARATION_HELP }, + ); + continue; + case "FunctionDeclaration": + case "ExportNamedDeclaration": { + let fn: AST.Function | undefined; + if (statement.type === "FunctionDeclaration") fn = statement; + else { + const named = statement as AST.ExportNamedDeclaration; + if (named.declaration?.type !== "FunctionDeclaration") { + exportedValue(named); + continue; + } + fn = named.declaration; + } + if (!fn?.id) continue; + const { name } = fn.id; + const isExported = statement.type === "ExportNamedDeclaration" || exported.has(name); + if (COMPOSABLE_NAME.test(name) && isExported) { + statements.add(statement.start); + candidates.push({ name, node: fn, span: { start: statement.start, end: statement.end } }); + } else if (COMPOSABLE_NAME.test(name)) { + statements.add(statement.start); + reporter.report( + "UF2035", + fn.id, + `\`${name}\` is named as a composable, and the module does not export it: a composable is a named export, compiled to a module of its own on every target.`, + { help: `Export it: \`export function ${name}(…) { … }\`.` }, + ); + } else if (isExported) { + statements.add(statement.start); + reporter.report( + "UF2035", + fn.id, + `\`${name}\` is exported from a module of composables, and a composable's name is \`use\` and an upper-case letter.`, + { + help: "Name it as a composable, as in `useToggle`, or move it to a shared `.ts` module.", + }, + ); + } + continue; + } + default: + continue; + } + } + return { candidates, statements }; + + /** `export const useX = …` and `export { useX }`: what an export statement without a function exports. */ + function exportedValue(statement: AST.ExportNamedDeclaration): void { + const { declaration } = statement; + if (declaration?.type === "VariableDeclaration") { + const values = declaration.declarations.filter( + (declarator) => + declarator.id.type === "Identifier" && + (declarator.init?.type === "ArrowFunctionExpression" || + declarator.init?.type === "FunctionExpression"), + ); + if (!values.length) return; + statements.add(statement.start); + for (const declarator of values) { + reporter.report( + "UF2035", + declarator.id, + `\`${(declarator.id as AST.BindingIdentifier).name}\` is a function written as a value, and a composable is a function declaration.`, + { help: DECLARATION_HELP }, + ); + } + return; + } + if (declaration || statement.source || statement.exportKind === "type") return; + const own = statement.specifiers.filter( + (specifier) => + specifier.exportKind !== "type" && + specifier.local.type === "Identifier" && + functions.has(specifier.local.name), + ); + if (!own.length || own.length !== statement.specifiers.length) return; + statements.add(statement.start); + for (const specifier of own) { + const local = (specifier.local as AST.IdentifierReference).name; + const name = exportName(specifier.exported); + if (name === "default") { + reporter.report( + "UF2035", + specifier, + `\`${local}\` is exported as the default: each composable is a named export, which a component imports by its name.`, + { help: `Export it by its name: \`export { ${local} };\`.` }, + ); + } else if (name !== local && COMPOSABLE_NAME.test(local)) { + reporter.report( + "UF2035", + specifier, + `\`${local}\` is exported as \`${name}\`: a composable is exported under its own name, which names its module on every target.`, + { help: `Name the function \`${name}\`, and export it as declared.` }, + ); + } + } + } +} + +/** What analysing a composable needs to know about its module. */ +export interface ComposableModule { + parsed: ParsedModule; + types: ModuleTypes; + shapes: Shapes; + scopes: Scopes; + authoring: ReadonlyMap; + reporter: Reporter; + /** The components a template may render: none, since a composable renders nothing. */ + components: ReadonlyMap; + keys: ReadonlyMap; + passThrough: PassThrough; + shared: SharedValues; + /** The names the module imports from other `.uf.ts` modules, by the identifier of each. */ + composableImports: ReadonlySet; + /** What a written type gives a value (`SetupContext.writtenType`). */ + readonly writtenType: (type: AST.TSType) => { kinds: Kinds; complete: boolean }; +} + +/** A composable, lowered, with the API a caller reads of it. */ +export interface AnalyzedComposable { + composable: UfComposable; + api: ComposableApi; +} + +/** + * Checks a composable's parameters, its setup and its return, and lowers it into IR (ADR-0069): + * `undefined` when it has an error. + */ +export function analyzeComposable( + candidate: ComposableCandidate, + typeParameters: TypeParameter[], + module: ComposableModule, +): AnalyzedComposable | undefined { + const { parsed, types, reporter, scopes } = module; + const { source } = parsed; + const mark = reporter.diagnostics.length; + const fn = candidate.node; + const id = fn.id!; + if (fn.async || fn.generator) { + reporter.report( + "UF2035", + id, + `\`${candidate.name}\` is ${fn.async ? "asynchronous" : "a generator"}, and a composable is a plain function whose setup runs once, when the component that calls it is set up.`, + { help: "Remove the keyword, and await in a returned function or a lifecycle hook." }, + ); + } + checkTypeParameters(fn, types, parsed, reporter); + const parameters = composableParameters(fn, module); + const block = fn.body; + const body = (block?.body ?? []).filter((statement) => statement.type !== "EmptyStatement"); + const last = body.at(-1); + const returned = last?.type === "ReturnStatement" ? last : undefined; + if (!returned?.argument || returned.argument.type !== "ObjectExpression") { + reporter.report( + "UF2035", + returned ?? last ?? id, + `\`${candidate.name}\`'s last statement must return a shorthand object of what a caller uses: its state, derived values, functions and constants.`, + { help: "End the composable with `return { on, toggle };`." }, + ); + } + if (calledComposables(fn, candidate, module)) return undefined; + const props: PropsAnalysis = { + props: [], + propsParameter: undefined, + bindings: parameters.bindings, + types: [], + byName: new Map(parameters.reads.map((read) => [read.name, read])), + byDeclaration: parameters.byDeclaration, + object: undefined, + failed: false, + }; + const setup = declareSetup({ + source, + reporter, + scopes, + types: types.table, + shapes: module.shapes, + writtenType: module.writtenType, + comments: parsed.comments, + authoring: module.authoring, + component: fn, + props, + statements: body.slice(0, -1), + returned: undefined, + keys: module.keys, + composable: true, + }); + const render: RenderContext = { + source, + reporter, + scopes, + types: types.table, + shapes: module.shapes, + component: fn, + props: props.byName, + propsByDeclaration: props.byDeclaration, + propsObject: undefined, + loopVariables: new Map(), + enclosing: [], + setup: setup.scope, + attached: new Map(), + bindings: [...props.bindings, ...setup.bindings], + comments: parsed.comments, + facts: { nestedCalls: [], getterKinds: new Map(), tickCallbacks: [], passed: new Map() }, + components: module.components, + slots: undefined, + passThrough: module.passThrough, + shared: module.shared, + }; + setup.lower(render); + const returns = + returned?.argument?.type === "ObjectExpression" + ? returnedMembers(returned.argument, candidate, render) + : []; + setup.finish(render); + checkRules({ + name: candidate.name, + span: candidate.span, + render, + setup, + props, + root: undefined, + program: parsed.program, + failed: reporter.hasErrorsSince(mark), + }); + if (reporter.hasErrorsSince(mark)) return undefined; + const bindings = render.bindings.toSorted((a, b) => a.span.start - b.span.start); + const names = new Set(closure([fn.typeParameters, fn.params, fn.body], types.table)); + const composable = createComposable(candidate.name, candidate.span, parameters.parameters, { + typeParameters, + types: types.declarations + .filter((declaration) => names.has(declaration.name)) + .map((declaration) => declaration.name), + bindings, + setup: setup.items, + returns: returns.map(({ member }) => member), + }); + return { + composable, + api: composableApi(composable, returns, bindings), + }; +} + +/** A composable's parameters, each an `argument` binding read as a destructured prop is. */ +function composableParameters( + fn: AST.Function, + module: ComposableModule, +): { + parameters: ComposableParameter[]; + bindings: Binding[]; + reads: PropBinding[]; + byDeclaration: Map; +} { + const { reporter, parsed } = module; + const parameters: ComposableParameter[] = []; + const bindings: Binding[] = []; + const reads: PropBinding[] = []; + const byDeclaration = new Map(); + for (const parameter of fn.params) { + if (parameter.type !== "Identifier") { + reporter.report( + "UF2035", + parameter, + parameter.type === "AssignmentPattern" + ? "A composable's parameter has no default value: each target reads its arguments as the caller passes them." + : "A composable's parameter is a name of its own, never a pattern or a rest parameter: its kind follows from its annotation.", + { + help: + parameter.type === "AssignmentPattern" + ? "Make it optional, `initial?: boolean`, and read `initial ?? false` where the body needs the default." + : "Declare each parameter by its name, with its type: `initial: boolean`.", + }, + ); + continue; + } + const annotation = parameter.typeAnnotation?.typeAnnotation; + if (!annotation) { + reporter.report( + "UF2035", + parameter, + `\`${parameter.name}\` has no type: a composable's parameter is annotated, and its type says how the body reads it, a value once, a getter (\`() => T\`) as it changes, a callback from client code.`, + { help: `Annotate it: \`${parameter.name}: boolean\`.` }, + ); + continue; + } + const kind = parameterKind(annotation); + // oxc's types leave `optional` out of a binding identifier, which a parameter sets. + const optional = (parameter as { optional?: boolean }).optional === true; + const at = { start: parameter.start, end: parameter.end }; + const binding = createBinding(parameter.name, "argument", { + start: parameter.start, + end: parameter.start + parameter.name.length, + }); + const kinds: Kinds = kind === "value" ? module.writtenType(annotation).kinds : FUNCTION; + const read: PropBinding = { + name: parameter.name, + id: binding.id, + kinds: optional ? union(kinds, UNDEFINED) : kinds, + ...(kind === "callback" ? { callback: true as const } : {}), + }; + bindings.push(binding); + reads.push(read); + byDeclaration.set(parameter, read); + parameters.push( + createComposableParameter( + parameter.name, + kind, + createTypeText(parsed.source.slice(annotation.start, annotation.end), { + start: annotation.start, + end: annotation.end, + }), + binding.id, + at, + optional, + ), + ); + } + return { parameters, bindings, reads, byDeclaration }; +} + +/** + * How a parameter is read, by its annotation (ADR-0069): a function type without parameters is a + * getter, any other function type a callback, and anything else a value. + */ +function parameterKind(annotation: AST.TSType): ComposableParameterKind { + if (annotation.type !== "TSFunctionType") return "value"; + return annotation.params.length ? "callback" : "getter"; +} + +/** + * Reports each call of another composable in a composable's body (UF2035): of one the module + * imports from a `.uf.ts` module, or declares itself. Returns whether it reported, since the + * walk of the body would report the call again. + */ +function calledComposables( + fn: AST.Function, + candidate: ComposableCandidate, + module: ComposableModule, +): boolean { + let found = false; + visit(fn.body, (node) => { + if (node.type !== "CallExpression" || node.callee.type !== "Identifier") return; + const { callee } = node; + const resolution = module.scopes.resolve(callee); + const imported = + resolution.kind === "import" && module.composableImports.has(resolution.declaration); + const own = + resolution.kind === "variable" && + resolution.declares === "function" && + (resolution.scope as AST.Node).type === "Program" && + COMPOSABLE_NAME.test(callee.name); + if (!imported && !own) return; + found = true; + module.reporter.report( + "UF2035", + callee, + `\`${candidate.name}\` calls the composable \`${callee.name}\`: a composable compiles to one module per target, and one that calls another is not supported.`, + { + help: `Call \`${callee.name}\` in the component, and pass what \`${candidate.name}\` needs from it as an argument.`, + }, + ); + }); + return found; +} + +/** The binding kinds a composable returns, with the member kind each gives its API. */ +const RETURNED: Readonly>> = { + state: "state", + derived: "derived", + localFn: "function", + localConst: "constant", +}; + +/** A returned member, with the binding it names. */ +interface Returned { + member: ReturnedMember; + binding: SetupBinding; +} + +/** + * The members of a composable's returned object (ADR-0069): shorthand properties, each naming + * state, a derived value, a local function or a constant of its setup (UF2035 otherwise). + */ +function returnedMembers( + object: AST.ObjectExpression, + candidate: ComposableCandidate, + render: RenderContext, +): Returned[] { + const { reporter, scopes, setup } = render; + const members: Returned[] = []; + for (const property of object.properties) { + if ( + property.type !== "Property" || + !property.shorthand || + property.value.type !== "Identifier" + ) { + reporter.report( + "UF2035", + property, + `\`${candidate.name}\` returns an object with a member that is not a shorthand property: each member names a binding of its setup, which every target returns by its name.`, + { help: "Return each binding by its name: `return { on, toggle };`." }, + ); + continue; + } + const value = property.value; + const resolution = scopes.resolve(value); + const own = resolution.kind === "variable" && resolution.scope === render.component; + const binding = own ? setup.bindings.get(resolution.declaration) : undefined; + // A declaration the setup rejected is reported where it is written. + if (own && !binding) continue; + if (!binding || RETURNED[binding.kind] === undefined) { + reporter.report( + "UF2035", + value, + `\`${value.name}\` is returned, and is not state, a derived value, a function or a constant of \`${candidate.name}\`'s setup: what a composable returns is its own.`, + { help: "Return the setup's own bindings, and let the caller keep its arguments." }, + ); + continue; + } + members.push({ + member: createReturnedMember(binding.name, binding.id, { + start: property.start, + end: property.end, + }), + binding, + }); + } + return members; +} + +/** + * What a caller reads of a composable (ADR-0069): its parameters, its members by their kinds and + * types, each function's flags from its summary, and whether its setup runs code in the browser. + */ +function composableApi( + composable: UfComposable, + returns: readonly Returned[], + bindings: Binding[], +): ComposableApi { + // Its setup, as a component's without props or a template, for the summaries of its functions. + const owner = createComponent( + composable.name, + createFragment([], { start: composable.span.end, end: composable.span.end }), + composable.span, + [], + undefined, + [], + bindings, + composable.setup, + ); + const summaries = summarize(owner); + const items = new Map( + composable.setup.flatMap((item) => ("binding" in item ? [[item.binding, item] as const] : [])), + ); + const members = returns.map(({ binding }): ComposableMemberApi => { + const kind = RETURNED[binding.kind]!; + const item = items.get(binding.id); + if (item?.kind === "Function") { + const summary = summaries.get(binding.id); + const pure = summary !== undefined && impurity(summary) === undefined; + return { + name: binding.name, + kind, + ...(item.function.async ? { async: true as const } : {}), + ...(pure ? { pure: true as const } : { client: true as const }), + }; + } + const type = item && "type" in item ? item.type?.code : undefined; + return { name: binding.name, kind, ...(type !== undefined ? { type } : {}) }; + }); + const hasEffects = composable.setup.some( + (item) => item.kind === "Watch" || item.kind === "WatchEffect" || item.kind === "Lifecycle", + ); + return { + name: composable.name, + ...(composable.typeParameters + ? { typeParameters: composable.typeParameters.map(({ name }) => name) } + : {}), + parameters: composable.parameters.map((parameter) => ({ + name: parameter.name, + kind: parameter.kind, + ...(parameter.optional ? { optional: true as const } : {}), + type: parameter.type.code, + })), + members, + ...(hasEffects ? { hasEffects: true as const } : {}), + }; +} + +/** Visits every node under a node, in source order. */ +function visit(node: unknown, enter: (node: AST.Node) => void): void { + if (!node || typeof node !== "object") return; + if (Array.isArray(node)) { + for (const item of node) visit(item, enter); + return; + } + const typed = node as AST.Node; + if (typeof typed.type !== "string") return; + enter(typed); + for (const key of visitorKeys[typed.type] ?? []) { + visit((typed as unknown as Record)[key], enter); + } +} diff --git a/packages/analyzer/src/expressions.ts b/packages/analyzer/src/expressions.ts index 03c75b76..c999d997 100644 --- a/packages/analyzer/src/expressions.ts +++ b/packages/analyzer/src/expressions.ts @@ -930,6 +930,17 @@ class Walk { } const prop = context.propsByDeclaration.get(resolution.declaration); if (prop?.id === undefined) return UNKNOWN; + if (prop.callback && this.#mode !== "client") { + context.reporter.report( + "UF2035", + node, + `\`${node.name}\` is a callback the composable is passed, read in ${this.#mode === "render" ? "a template expression" : "a getter or an initial value"}: a callback is called from client code, a handler, a watcher's callback or a lifecycle hook.`, + { + help: "Call it from client code, or type the parameter as a getter, `() => T`, to read it as it changes.", + }, + ); + return UNKNOWN; + } const narrowed = render ? [] : this.#narrowingRule(node, { kind: "prop", prop }, true); this.refs.push(createBindingReference(prop.id, span(node), shorthand, false, narrowed)); return this.#narrowed(node, prop.kinds); diff --git a/packages/analyzer/src/render.ts b/packages/analyzer/src/render.ts index c8527a0e..e8e5e135 100644 --- a/packages/analyzer/src/render.ts +++ b/packages/analyzer/src/render.ts @@ -23,6 +23,8 @@ export interface PropBinding { id: BindingId | undefined; /** Its kinds: the member's type, with `undefined` when optional without a default. */ kinds: Kinds; + /** Set on a composable's callback argument, which only client code reads (ADR-0069). */ + callback?: true; } /** A component a template may render (ADR-0053): its local name, and its API. */ diff --git a/packages/analyzer/src/setup.ts b/packages/analyzer/src/setup.ts index 7715f0ec..0492ea35 100644 --- a/packages/analyzer/src/setup.ts +++ b/packages/analyzer/src/setup.ts @@ -115,6 +115,11 @@ export interface SetupContext { readonly returned: AST.Expression | undefined; /** The injection keys the module declares or imports, by the identifier that declares each. */ readonly keys: ReadonlyMap; + /** + * Set where the setup is a composable's body (ADR-0069): the APIs that need a component, such + * as `useId`, `provide` and the macros, are UF2035 there. + */ + readonly composable?: true; } /** A component's setup, declared: its bindings, and what lowers its items. */ @@ -215,6 +220,22 @@ const BOUND_MACROS: ReadonlySet = new Set([ "inject", ]); +/** + * The APIs a composable cannot call (ADR-0069): each needs the component it runs in, its id, its + * template, its API or its place in the tree. + */ +const COMPONENT_APIS: ReadonlySet = new Set([ + "useId", + "useTemplateRef", + "provide", + "inject", + "defineEmits", + "defineSlots", + "defineModel", + "defineExpose", + "defineOptions", +]); + /** * Declares a component's setup (ADR-0045): classifies each statement before the return, creates * the bindings, reads the events `defineEmits` declares, and finds the parameters that receive an @@ -310,6 +331,7 @@ export function declareSetup(context: SetupContext): Setup { continue; } const call = expression as AST.CallExpression; + if (needsComponent(call, api)) continue; if (api === "watch" || api === "watchEffect") declared.push({ kind: api, statement, call }); else if (api === "onMounted" || api === "onUnmounted") { declared.push({ @@ -343,6 +365,24 @@ export function declareSetup(context: SetupContext): Setup { } } + /** + * A call in a composable of an API that needs a component (UF2035): reported where it is + * called. Returns whether it reported. + */ + function needsComponent(call: AST.CallExpression, api: AuthoringApi): boolean { + if (!context.composable || !COMPONENT_APIS.has(api)) return false; + const name = (call.callee as AST.IdentifierReference).name; + reporter.report( + "UF2035", + call.callee, + `\`${name}\` needs the component it runs in, and a composable runs in whichever component calls it: it calls no \`useId\`, \`useTemplateRef\`, \`provide\`, \`inject\` or \`define*\` macro.`, + { + help: `Call \`${name}\` in the component, and pass what the composable needs from it as an argument.`, + }, + ); + return true; + } + /** A `const` or `let` at the top level of the body. */ function variable(statement: AST.VariableDeclaration): void { if (statement.kind !== "const" && statement.kind !== "let") { @@ -382,6 +422,7 @@ export function declareSetup(context: SetupContext): Setup { const api = call ? apiOf(call.callee) : null; if (api === undefined) return; if (api !== null) { + if (needsComponent(call!, api)) return; macro(statement, declarator, call!, api); return; } @@ -2163,7 +2204,24 @@ function unwatchable( if (node?.type === "Identifier") { const resolution = render.scopes.resolve(node); const binding = setupBindingOf(node, render); - if (resolution.kind === "parameter" && render.propsByDeclaration.has(resolution.declaration)) { + const prop = + resolution.kind === "parameter" + ? render.propsByDeclaration.get(resolution.declaration) + : undefined; + if (prop && render.bindings.some(({ id, kind }) => id === prop.id && kind === "argument")) { + // A composable's parameter (ADR-0069): a getter is watched through a call of it. + const annotation = (resolution as { declaration: AST.BindingIdentifier }).declaration + .typeAnnotation?.typeAnnotation; + const getter = annotation?.type === "TSFunctionType" && !annotation.params.length; + what = `\`${node.name}\` is ${getter ? "a getter the composable is passed, which a watcher calls in its own getter" : "an argument of the composable, which nothing can watch"}`; + if (getter) { + fix = { + title: `Watch \`() => ${node.name}()\``, + confidence: "safe", + edits: [{ span: span(node), text: `() => ${node.name}()` }], + }; + } + } else if (prop) { what = `\`${node.name}\` is a prop's value, which nothing can watch`; fix = getterFix(node, text); } else if (binding?.kind === "templateRef") { diff --git a/packages/analyzer/test/composables.test.ts b/packages/analyzer/test/composables.test.ts new file mode 100644 index 00000000..6ad8fa49 --- /dev/null +++ b/packages/analyzer/test/composables.test.ts @@ -0,0 +1,387 @@ +import { describe, expect, it } from "vitest"; + +// Composables (ADR-0069): the named `use*` exports of a `.uf.ts` module, each a setup under a +// component's rules, with parameters in place of props and a returned shorthand object in place +// of JSX. Analysing one gives its IR, `UfComposable`, and what a caller reads of it, +// `ComposableApi`. +import { problems, run } from "./helpers.ts"; + +/** Analyses a module of composables. */ +function composables(lines: readonly string[]) { + const source = lines.join("\n"); + return { source, ...run(source, "useToggle.uf.ts") }; +} + +const TOGGLE = [ + 'import { ref } from "unframework";', + "export function useToggle(initial: boolean) {", + " const on = ref(initial);", + " function toggle() {", + " on.value = !on.value;", + " }", + " return { on, toggle };", + "}", +]; + +describe("a composable", () => { + it("lowers its parameters, its setup and its returned members", () => { + const { source, module, diagnostics } = composables(TOGGLE); + expect(diagnostics).toEqual([]); + expect(module!.components).toEqual([]); + expect(module!.exports).toEqual([]); + const [composable] = module!.composables!; + const at = (text: string, from = 0) => source.indexOf(text, from); + expect(composable).toMatchObject({ + name: "useToggle", + span: { start: at("export"), end: source.length }, + parameters: [ + { + name: "initial", + kind: "value", + type: { code: "boolean" }, + binding: `initial@${at("initial")}`, + }, + ], + types: [], + returns: [ + { name: "on", binding: `on@${at("on =")}` }, + { name: "toggle", binding: `toggle@${at("toggle()")}` }, + ], + }); + expect(composable!.bindings.map(({ name, kind }) => [name, kind])).toEqual([ + ["initial", "argument"], + ["on", "state"], + ["toggle", "localFn"], + ]); + expect(composable!.setup.map((item) => item.kind)).toEqual(["State", "Function"]); + // The state's initial value reads the argument through its binding. + expect(JSON.stringify(composable!.setup[0])).toContain(`"binding":"initial@${at("initial")}"`); + }); + + it("gives its API: each parameter's kind, each member's kind and type, and the function flags", () => { + const { api, diagnostics } = composables([ + 'import { computed, ref } from "unframework";', + "export function useCounter(start: number, step: () => number, onChange?: (count: number) => void) {", + " const count = ref(start);", + " const double = computed(() => count.value * 2);", + " const max: number = 10;", + " function increment() {", + " count.value += step();", + " onChange?.(count.value);", + " }", + " async function reset() {", + " count.value = start;", + " }", + " function isMax() {", + " return count.value >= max;", + " }", + " return { count, double, max, increment, reset, isMax };", + "}", + ]); + expect(diagnostics).toEqual([]); + expect(api!.composables).toEqual([ + { + name: "useCounter", + parameters: [ + { name: "start", kind: "value", type: "number" }, + { name: "step", kind: "getter", type: "() => number" }, + { + name: "onChange", + kind: "callback", + optional: true, + type: "(count: number) => void", + }, + ], + members: [ + { name: "count", kind: "state", type: "number" }, + { name: "double", kind: "derived", type: "number" }, + { name: "max", kind: "constant", type: "number" }, + { name: "increment", kind: "function", client: true }, + { name: "reset", kind: "function", async: true, client: true }, + { name: "isMax", kind: "function", pure: true }, + ], + }, + ]); + }); + + it("sets `hasEffects` where its setup runs code in the browser", () => { + const { api, diagnostics } = composables([ + 'import { onMounted, onUnmounted, ref, watch } from "unframework";', + "export function useMediaQuery(query: () => string) {", + " const matches = ref(false);", + " let list: MediaQueryList | undefined;", + " function update() {", + " matches.value = list?.matches ?? false;", + " }", + " watch(() => query(), () => update());", + " onMounted(() => {", + " list = window.matchMedia(query());", + ' list.addEventListener("change", update);', + " update();", + " });", + ' onUnmounted(() => list?.removeEventListener("change", update));', + " return { matches };", + "}", + "export function useFlag(initial: boolean) {", + " const flag = ref(initial);", + " return { flag };", + "}", + ]); + expect(diagnostics).toEqual([]); + expect(api!.composables!.map(({ name, hasEffects }) => [name, hasEffects])).toEqual([ + ["useMediaQuery", true], + ["useFlag", undefined], + ]); + }); + + it("keeps its type parameters, and the module's types and constants it reaches", () => { + const { module, api, diagnostics } = composables([ + 'import { ref } from "unframework";', + "interface Item { id: string }", + "const LIMIT = 3;", + "export function useList(items: T[]) {", + " const list = ref(items.slice(0, LIMIT));", + " return { list };", + "}", + ]); + expect(diagnostics).toEqual([]); + expect(module!.types.map(({ name }) => name)).toEqual(["Item"]); + expect(module!.constants!.map(({ name }) => name)).toEqual(["LIMIT"]); + const [composable] = module!.composables!; + expect(composable!.types).toEqual(["Item"]); + expect(composable!.typeParameters).toMatchObject([{ name: "T", constraint: { code: "Item" } }]); + expect(api!.composables![0]).toMatchObject({ + typeParameters: ["T"], + members: [{ name: "list", kind: "state", type: "T[]" }], + }); + }); + + it("is exported by name in a declaration or a list", () => { + const { api, diagnostics } = composables([ + "function useA() {", + " return {};", + "}", + "export function useB() {", + " return {};", + "}", + "export { useA };", + ]); + expect(diagnostics).toEqual([]); + expect(api!.composables!.map(({ name }) => name)).toEqual(["useA", "useB"]); + }); + + it("reads a callback in client code, and a getter in a derived value", () => { + const { diagnostics } = composables([ + 'import { computed } from "unframework";', + "export function useLabel(text: () => string, onPick: (value: string) => void) {", + " const label = computed(() => text().toUpperCase());", + " function pick() {", + " onPick(label.value);", + " }", + " return { label, pick };", + "}", + ]); + expect(diagnostics).toEqual([]); + }); + + it("watches a getter argument through a call of it, with a safe fix", () => { + const { source, diagnostics } = composables([ + 'import { ref, watch } from "unframework";', + "export function useSeen(value: () => number) {", + " const seen = ref(0);", + " watch(value, () => {", + " seen.value++;", + " });", + " return { seen };", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2020 value"]); + expect(diagnostics[0]!.fixes?.[0]?.edits).toEqual([ + { span: diagnostics[0]!.span, text: "() => value()" }, + ]); + }); + + it("leaves a `.uf.tsx` module's analysis as it was", () => { + const { module, diagnostics } = run( + [ + 'import { ref } from "unframework";', + "export function Toggle() {", + " const count = ref(0);", + " return ;", + "}", + ].join("\n"), + ); + expect(diagnostics).toEqual([]); + expect(module!.composables).toBeUndefined(); + expect(module!.components.map(({ name }) => name)).toEqual(["Toggle"]); + }); +}); + +/** A composable `useA` whose setup is `setup`, returning `returned`. */ +function useA(setup: readonly string[], returned = "return { n };", imports = "ref") { + return composables([ + `import { ${imports} } from "unframework";`, + "export function useA() {", + " const n = ref(1);", + ...setup.map((line) => ` ${line}`), + ` ${returned}`, + "}", + ]); +} + +// Each form ADR-0069 rejects is UF2035 (`invalid-composable`), at the exact span, once. +describe("UF2035", () => { + it.each([ + ["useId", ["const id = useId();"], "return { n, id };"], + ["useTemplateRef", ['const el = useTemplateRef("el");'], "return { n };"], + ["defineEmits", ["const emit = defineEmits<{ change: [value: number] }>();"], "return { n };"], + ["defineSlots", ["const slots = defineSlots<{ default(): unknown }>();"], "return { n };"], + ["defineModel", ['const value = defineModel("value");'], "return { n, value };"], + ["defineExpose", ["defineExpose({});"], "return { n };"], + ["defineOptions", ["defineOptions({ inheritAttrs: false });"], "return { n };"], + ])("rejects `%s`, which needs a component", (api, setup, returned) => { + const { source, module, diagnostics } = useA(setup, returned, `${api}, ref`); + expect(problems(source, diagnostics)).toEqual([`UF2035 ${api}`]); + expect(module).toBeUndefined(); + }); + + it.each([ + ["provide", ["provide(ThemeKey, n);"], "return { n };"], + ["inject", ["const theme = inject(ThemeKey);"], "return { n, theme };"], + ])("rejects `%s`, which needs a place in the tree", (api, setup, returned) => { + const { source, diagnostics } = composables([ + `import { ${api}, ref } from "unframework";`, + 'import { ThemeKey } from "./Theme.uf.tsx";', + "export function useA() {", + " const n = ref(1);", + ...setup.map((line) => ` ${line}`), + ` ${returned}`, + "}", + ]); + // The key's module is no resolver's here (UF1202): the call is still reported, once. + expect(problems(source, diagnostics).filter((each) => each.startsWith("UF2035"))).toEqual([ + `UF2035 ${api}`, + ]); + }); + + it("rejects a call of a composable another `.uf.ts` module exports", () => { + const { source, module, diagnostics } = composables([ + 'import { ref } from "unframework";', + 'import { useOther } from "./useOther.uf.ts";', + "export function useA() {", + " const n = ref(1);", + " const { m } = useOther();", + " return { n };", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2035 useOther"]); + expect(module).toBeUndefined(); + }); + + it("rejects a call of a composable the module declares, wherever the body makes it", () => { + const { source, diagnostics } = composables([ + 'import { ref } from "unframework";', + "export function useB() {", + " const m = ref(1);", + " return { m };", + "}", + "export function useA() {", + " const n = ref(1);", + " function grow() {", + " n.value += useB().m.value;", + " }", + " return { n, grow };", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2035 useB"]); + }); + + it("rejects a default export, declared or listed", () => { + const declared = composables(["export default function useA() {", " return {};", "}"]); + expect(problems(declared.source, declared.diagnostics)).toEqual([`UF2035 ${declared.source}`]); + const listed = composables([ + "function useA() {", + " return {};", + "}", + "export { useA as default };", + ]); + expect(problems(listed.source, listed.diagnostics)).toEqual(["UF2035 useA as default"]); + expect(listed.module).toBeUndefined(); + }); + + it.each([ + ["a ref", "return n;", "return n;"], + ["an array", "return [n];", "return [n];"], + ["nothing", "return;", "return;"], + ])("rejects a return of %s", (_, returned, reported) => { + const { source, diagnostics } = useA([], returned); + expect(problems(source, diagnostics)).toEqual([`UF2035 ${reported}`]); + }); + + it("rejects a body that does not end with a return", () => { + const { source, diagnostics } = useA([], "n.value++;"); + expect(problems(source, diagnostics)).toEqual(["UF2035 n.value++;"]); + }); + + it("rejects a member that is not a shorthand property", () => { + const { source, diagnostics } = useA([], "return { count: n, ...{} };"); + expect(problems(source, diagnostics)).toEqual(["UF2035 count: n", "UF2035 ...{}"]); + }); + + it("rejects a returned name that is not the setup's own", () => { + const { source, diagnostics } = composables([ + "export function useA(start: number) {", + " return { start };", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2035 start"]); + }); + + it("rejects a parameter without a type, with a default value, or written as a pattern", () => { + const { source, diagnostics } = composables([ + "export function useA(a, b: number = 1, { c }: { c: number }, ...d: number[]) {", + " return {};", + "}", + ]); + expect(problems(source, diagnostics)).toEqual([ + "UF2035 a", + "UF2035 b: number = 1", + "UF2035 { c }: { c: number }", + "UF2035 ...d: number[]", + ]); + }); + + it("rejects a callback read outside client code", () => { + const { source, diagnostics } = composables([ + 'import { computed } from "unframework";', + "export function useA(onPick: (value: number) => void) {", + " const picked = computed(() => onPick);", + " return { picked };", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2035 onPick"]); + }); + + it("rejects a composable the module keeps, one named otherwise, and one written as a value", () => { + const local = composables(["function useA() {", " return {};", "}"]); + expect(problems(local.source, local.diagnostics)).toEqual(["UF2035 useA"]); + const misnamed = composables(["export function toggle() {", " return {};", "}"]); + expect(problems(misnamed.source, misnamed.diagnostics)).toEqual(["UF2035 toggle"]); + const value = composables(["export const useA = () => {", " return {};", "};"]); + expect(problems(value.source, value.diagnostics)).toEqual(["UF2035 useA"]); + }); + + it("rejects an asynchronous composable", () => { + const { source, diagnostics } = composables([ + "export async function useA() {", + " return {};", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2035 useA"]); + }); + + it("rejects a module that exports no composable", () => { + const { source, diagnostics } = composables(['import { ref } from "unframework";']); + expect(problems(source, diagnostics)).toEqual(["UF2035 "]); + }); +}); From 286f009a70b04a294eef44578e86c9f1ec98d78d Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Wed, 28 Oct 2026 11:25:07 +0200 Subject: [PATCH 2/3] fix(analyzer): report reserved, repeated and aliased composable forms A reserved parameter name is UF2003, a member returned twice, a function type written through an alias or a union, a `this` parameter and a value import of a `.uf.ts` module that no composable calls are UF2035. Type specifiers of a `.uf.ts` import are re-imported. A function that calls a callback is client code: impure where a getter or an initial value calls it (UF2014), and `client` in the API. Co-authored-by: multica-agent --- packages/analyzer/README.md | 6 +- packages/analyzer/src/analyze.ts | 47 +++++- packages/analyzer/src/composables.ts | 87 ++++++++++- packages/analyzer/src/rules.ts | 12 ++ packages/analyzer/test/composables.test.ts | 171 +++++++++++++++++++++ 5 files changed, 306 insertions(+), 17 deletions(-) diff --git a/packages/analyzer/README.md b/packages/analyzer/README.md index 9f862523..b6ba3c67 100644 --- a/packages/analyzer/README.md +++ b/packages/analyzer/README.md @@ -3,8 +3,8 @@ Passes P2 (analyse) and P3 (lower) of the Unframework compiler (plan §5.1): - `analyze(parsed)` checks a parsed `.uf.tsx` module and lowers each component's returned JSX into - the portable IR, or a `.uf.ts` module and its composables (ADR-0069). It reports every construct it cannot lower as a diagnostic, and never copies - code through unanalysed (P2). It never throws. A syntax error stops it. Any other module-level + the portable IR, or a `.uf.ts` module and its composables (ADR-0069). It reports every construct + it cannot lower as a diagnostic, and never copies code through unanalysed (P2). It never throws. A syntax error stops it. Any other module-level error (an import, a top-level declaration) drops every component, which are still checked, so that fixing it reveals nothing new; an error in a component drops that component only. - JSX text means what every JSX implementation agrees it means. Babel (Vue's and Solid's JSX) @@ -29,7 +29,7 @@ Passes P2 (analyse) and P3 (lower) of the Unframework compiler (plan §5.1): Components with typed props, a setup, listeners and JSX (plan §9 M1 and M2, ADR-0034 to ADR-0040, ADR-0045 onwards), and the composition of M3's core lane (ADR-0053 to ADR-0055): components that -render each other, slots, fallthrough and `defineExpose`. A module exports at least one component; +render each other, slots, fallthrough and `defineExpose`. A `.uf.tsx` module exports at least one component; one it does not export is lowered too, and each output writes it as a sibling file. A component is a function declaration; one written as a value (`export const Card = (props) => …`) is UF1102, checked as the declaration its likely fix writes (`analyze.ts`). diff --git a/packages/analyzer/src/analyze.ts b/packages/analyzer/src/analyze.ts index 4e6b9ffd..a77fe6e1 100644 --- a/packages/analyzer/src/analyze.ts +++ b/packages/analyzer/src/analyze.ts @@ -327,7 +327,7 @@ export function analyzeModule( shared: { values: new Map(), failed: new Set() }, constants: [], composableModule, - composableImports: new Set(), + composableImports: new Map(), }; const moduleMark = reporter.diagnostics.length; for (const statement of parsed.program.body) { @@ -364,6 +364,7 @@ export function analyzeModule( passThrough: context.passThrough, shared: context.shared, composableImports: context.composableImports, + called: new Set(), writtenType: writtenTypeIn({ parsed, types, shapes, localShapes }), }, context, @@ -513,6 +514,7 @@ function composableResult( context: ModuleContext, ): AnalyzeResult { const { parsed, reporter, types } = module; + const mark = reporter.diagnostics.length; const lowered: AnalyzedComposable[] = []; for (const candidate of candidates) { const result = analyzeComposable( @@ -522,6 +524,32 @@ function composableResult( ); if (result) lowered.push(result); } + // A value import no composable calls names a type, or nothing a composable may read: either + // way its outputs would import a value the module never had (UF2035). + for (const [local, specifier] of module.composableImports) { + if (module.called.has(local)) continue; + reporter.report( + "UF2035", + specifier, + `\`${specifier.local.name}\` is imported as a value from a module of composables, which a composable never calls: a type is imported with \`type\`.`, + { + help: `Import it as a type: \`import { type ${specifier.local.name} } from "…";\`.`, + ...(specifier.type === "ImportSpecifier" + ? { + fixes: [ + { + title: "Import it as a type", + confidence: "likely" as const, + edits: [ + { span: { start: specifier.start, end: specifier.start }, text: "type " }, + ], + }, + ], + } + : {}), + }, + ); + } const api: ModuleApi = { file: parsed.file, components: [], @@ -531,8 +559,9 @@ function composableResult( const uncalled = !reporter.diagnostics.some(({ severity }) => severity === "error") && reportUnread(parsed.program.body, context.passThrough, reporter); - if (moduleErrors || uncalled) + if (moduleErrors || uncalled || reporter.hasErrorsSince(mark)) { return { module: undefined, diagnostics: reporter.diagnostics, api }; + } if (!candidates.length) { reporter.report("UF2035", { start: 0, end: 0 }, "This file exports no composable.", { help: "A module of composables exports functions named `use` and an upper-case letter, each returning a shorthand object.", @@ -770,7 +799,7 @@ interface ModuleContext { /** Whether the module is a `.uf.ts` module of composables (ADR-0069). */ composableModule: boolean; /** In a `.uf.ts` module, the names it imports from other `.uf.ts` modules (UF2035). */ - composableImports: Set; + composableImports: Map; } /** The local names a module exports as values in `export { name }` and `export default name`. */ @@ -868,10 +897,16 @@ function checkTopLevelStatement( COMPOSABLE_FILE.test(statement.source.value) && statement.importKind !== "type" ) { - // A composable that calls another is reported at the call (UF2035). + // Its types are re-imported by name (ADR-0067); a composable that calls another is + // reported at the call (UF2035). + const types = statement.specifiers.filter( + (item): item is AST.ImportSpecifier => + item.type === "ImportSpecifier" && item.importKind === "type", + ); + if (types.length) typeImport(statement, types, context); for (const specifier of statement.specifiers) { - if (specifier.type !== "ImportSpecifier" || specifier.importKind !== "type") { - context.composableImports.add(specifier.local); + if (!types.includes(specifier as AST.ImportSpecifier)) { + context.composableImports.set(specifier.local, specifier); } } return; diff --git a/packages/analyzer/src/composables.ts b/packages/analyzer/src/composables.ts index a1b973c2..80c74744 100644 --- a/packages/analyzer/src/composables.ts +++ b/packages/analyzer/src/composables.ts @@ -13,6 +13,7 @@ import { createReturnedMember, createTypeText, impurity, + reservedSetupName, summarize, } from "@unframework/ir"; import type { @@ -42,7 +43,7 @@ import { declareSetup } from "./setup.ts"; import type { InjectionKeyInfo } from "./setup.ts"; import type { Shapes } from "./shapes.ts"; import type { SharedValues } from "./shared.ts"; -import { FUNCTION, union, UNDEFINED } from "./types/kinds.ts"; +import { FUNCTION, has, union, UNDEFINED } from "./types/kinds.ts"; import type { Kinds } from "./types/kinds.ts"; /** A module of composables is a `.uf.ts` file (D1, ADR-0005, ADR-0069). */ @@ -216,7 +217,9 @@ export interface ComposableModule { passThrough: PassThrough; shared: SharedValues; /** The names the module imports from other `.uf.ts` modules, by the identifier of each. */ - composableImports: ReadonlySet; + composableImports: ReadonlyMap; + /** The imported composables a composable calls, which `calledComposables` fills. */ + called: Set; /** What a written type gives a value (`SetupContext.writtenType`). */ readonly writtenType: (type: AST.TSType) => { kinds: Kinds; complete: boolean }; } @@ -378,6 +381,27 @@ function composableParameters( ); continue; } + const name = { start: parameter.start, end: parameter.start + parameter.name.length }; + if (parameter.name === "this") { + reporter.report( + "UF2035", + name, + "A composable has no `this`: it is a plain function, which each target calls as one.", + { help: "Remove the `this` parameter, and pass what the body reads as an argument." }, + ); + continue; + } + // Its `argument` binding is a binding of the setup, which every target declares (UF2003). + const reserved = reservedSetupName(parameter.name); + if (reserved) { + reporter.report( + "UF2003", + name, + `\`${parameter.name}\` cannot name a composable's parameter: ${reserved}.`, + { help: "Rename it." }, + ); + continue; + } const annotation = parameter.typeAnnotation?.typeAnnotation; if (!annotation) { reporter.report( @@ -389,14 +413,25 @@ function composableParameters( continue; } const kind = parameterKind(annotation); + const written = module.writtenType(annotation); + if (kind === "value" && has(written.kinds, "function")) { + // The kind decides each target's output (a getter is Qwik's `useComputed$`, a callback a + // QRL), so it is read from the annotation as written, never guessed through an alias. + reporter.report( + "UF2035", + annotation, + `\`${parameter.name}\`'s type may be a function, written through a name or a union: a composable's parameter writes a function type in place, which says whether it is a getter (\`() => T\`) or a callback.`, + { + help: `Write the function type in place, as in \`${parameter.name}: () => number\` or \`${parameter.name}?: (value: number) => void\`.`, + }, + ); + continue; + } // oxc's types leave `optional` out of a binding identifier, which a parameter sets. const optional = (parameter as { optional?: boolean }).optional === true; const at = { start: parameter.start, end: parameter.end }; - const binding = createBinding(parameter.name, "argument", { - start: parameter.start, - end: parameter.start + parameter.name.length, - }); - const kinds: Kinds = kind === "value" ? module.writtenType(annotation).kinds : FUNCTION; + const binding = createBinding(parameter.name, "argument", name); + const kinds: Kinds = kind === "value" ? written.kinds : FUNCTION; const read: PropBinding = { name: parameter.name, id: binding.id, @@ -455,6 +490,7 @@ function calledComposables( (resolution.scope as AST.Node).type === "Program" && COMPOSABLE_NAME.test(callee.name); if (!imported && !own) return; + if (imported) module.called.add(resolution.declaration); found = true; module.reporter.report( "UF2035", @@ -493,6 +529,7 @@ function returnedMembers( ): Returned[] { const { reporter, scopes, setup } = render; const members: Returned[] = []; + const names = new Set(); for (const property of object.properties) { if ( property.type !== "Property" || @@ -522,6 +559,33 @@ function returnedMembers( ); continue; } + if (names.has(binding.name)) { + reporter.report( + "UF2035", + property, + `\`${binding.name}\` is returned twice: each member of a composable's result is named once, which every target returns by its name.`, + { + help: `Remove the second \`${binding.name}\`.`, + fixes: [ + { + title: `Remove the second \`${binding.name}\``, + confidence: "safe", + edits: [ + { + span: { + start: members.at(-1)?.member.span.end ?? property.start, + end: property.end, + }, + text: "", + }, + ], + }, + ], + }, + ); + continue; + } + names.add(binding.name); members.push({ member: createReturnedMember(binding.name, binding.id, { start: property.start, @@ -554,6 +618,10 @@ function composableApi( composable.setup, ); const summaries = summarize(owner); + // A callback is called from client code (ADR-0069), so a function that reads one is too. + const callbacks = new Set( + composable.parameters.flatMap(({ kind, binding }) => (kind === "callback" ? [binding] : [])), + ); const items = new Map( composable.setup.flatMap((item) => ("binding" in item ? [[item.binding, item] as const] : [])), ); @@ -562,7 +630,10 @@ function composableApi( const item = items.get(binding.id); if (item?.kind === "Function") { const summary = summaries.get(binding.id); - const pure = summary !== undefined && impurity(summary) === undefined; + const pure = + summary !== undefined && + impurity(summary) === undefined && + ![...summary.reads].some((read) => callbacks.has(read)); return { name: binding.name, kind, diff --git a/packages/analyzer/src/rules.ts b/packages/analyzer/src/rules.ts index b709bfc9..2c40e8c8 100644 --- a/packages/analyzer/src/rules.ts +++ b/packages/analyzer/src/rules.ts @@ -102,6 +102,8 @@ class Rules { readonly #shared: ReadonlyMap; /** The functions client code hands on to run later (ADR-0048). */ readonly #deferred: ReadonlySet; + /** A composable's callback arguments, which only client code calls (ADR-0069). */ + readonly #callbacks: ReadonlySet; constructor(input: RulesInput) { this.#input = input; @@ -126,6 +128,11 @@ class Rules { [...render.setup.bindings.values()].map((binding) => [binding.id, binding]), ); this.#shared = new Map([...render.shared.values.values()].map((value) => [value.id, value])); + this.#callbacks = new Set( + [...props.byDeclaration.values()].flatMap(({ id, callback }) => + callback && id !== undefined ? [id] : [], + ), + ); // A module's constants and imports are static too: Qwik reads them where they are declared, // at the module's top level (ADR-0070). for (const binding of this.#component.bindings) { @@ -209,6 +216,11 @@ class Rules { if (summary.readsLocalVar) return "it reads a setup `let`, which is not reactive"; const global = [...summary.clientGlobals].find((name) => !NONDETERMINISTIC.has(name)); if (global !== undefined) return `it reads \`${global}\`, which only client code may`; + // A composable's callback, which only client code calls (ADR-0069). + const callback = [...summary.reads].find((read) => this.#callbacks.has(read)); + if (callback !== undefined) { + return `it calls the callback \`${this.#name(callback)}\`, which only client code calls`; + } // A function of the module it calls, which its summary counts as a read (ADR-0070). for (const read of summary.reads) { const shared = this.#shared.get(read); diff --git a/packages/analyzer/test/composables.test.ts b/packages/analyzer/test/composables.test.ts index 6ad8fa49..2e321753 100644 --- a/packages/analyzer/test/composables.test.ts +++ b/packages/analyzer/test/composables.test.ts @@ -1,3 +1,4 @@ +import { applyFixes } from "@unframework/diagnostics"; import { describe, expect, it } from "vitest"; // Composables (ADR-0069): the named `use*` exports of a `.uf.ts` module, each a setup under a @@ -134,6 +135,87 @@ describe("a composable", () => { ]); }); + it.each([ + ["watch", "watch(n, () => {});"], + ["watchEffect", "watchEffect(() => {\n void n.value;\n });"], + ["onMounted", "onMounted(() => {});"], + ["onUnmounted", "onUnmounted(() => {});"], + ])("sets `hasEffects` for `%s` alone", (api, statement) => { + const { api: moduleApi, diagnostics } = composables([ + `import { ${api}, ref } from "unframework";`, + "export function useA() {", + " const n = ref(0);", + ` ${statement}`, + " return { n };", + "}", + ]); + expect(diagnostics).toEqual([]); + expect(moduleApi!.composables![0]!.hasEffects).toBe(true); + }); + + it("marks a function that calls a callback as client code", () => { + const { api, diagnostics } = composables([ + "export function useA(onPick: (value: number) => void) {", + " function pick() {", + " onPick(1);", + " }", + " return { pick };", + "}", + ]); + expect(diagnostics).toEqual([]); + expect(api!.composables![0]!.members).toEqual([ + { name: "pick", kind: "function", client: true }, + ]); + }); + + it("keeps each composable's own types, and the constants it reads", () => { + const { module, diagnostics } = composables([ + 'import { ref } from "unframework";', + "interface Item { id: string }", + "interface Tag { name: string }", + "const LIMIT = 3;", + "const UNREAD = 4;", + "export function useItems(items: Item[]) {", + " const list = ref(items.slice(0, LIMIT));", + " return { list };", + "}", + "export function useTags(tags: Tag[]) {", + " const list = ref(tags);", + " return { list };", + "}", + ]); + expect(diagnostics).toEqual([]); + expect(module!.composables!.map(({ name, types }) => [name, types])).toEqual([ + ["useItems", ["Item"]], + ["useTags", ["Tag"]], + ]); + expect(module!.types.map(({ name }) => name)).toEqual(["Item", "Tag"]); + expect(module!.constants!.map(({ name }) => name)).toEqual(["LIMIT"]); + }); + + it("re-imports the types it imports from a `.uf.ts` module, inline or whole", () => { + const { module, diagnostics } = composables([ + 'import { ref } from "unframework";', + 'import { type Item } from "./useItem.uf.ts";', + 'import type { Tag } from "./useTag.uf.ts";', + "export function useA(item: Item, tag: Tag) {", + " const state = ref(item);", + " const label = ref(tag);", + " return { state, label };", + "}", + ]); + expect(diagnostics).toEqual([]); + expect( + module!.typeImports!.map(({ specifier, names }) => [ + specifier, + names.map(({ local }) => local), + ]), + ).toEqual([ + ["./useItem.uf.ts", ["Item"]], + ["./useTag.uf.ts", ["Tag"]], + ]); + }); + it("keeps its type parameters, and the module's types and constants it reaches", () => { const { module, api, diagnostics } = composables([ 'import { ref } from "unframework";', @@ -328,6 +410,17 @@ describe("UF2035", () => { expect(problems(source, diagnostics)).toEqual(["UF2035 count: n", "UF2035 ...{}"]); }); + it("rejects a member returned twice, with a safe fix", () => { + const { source, diagnostics } = useA([], "return { n, n };"); + expect(problems(source, diagnostics)).toEqual(["UF2035 n"]); + expect(applyFixes(source, diagnostics[0]!.fixes!)).toContain("return { n };"); + }); + + it("rejects a returned setup `let`", () => { + const { source, diagnostics } = useA(["let timer = 0;"], "return { n, timer };"); + expect(problems(source, diagnostics)).toEqual(["UF2035 timer"]); + }); + it("rejects a returned name that is not the setup's own", () => { const { source, diagnostics } = composables([ "export function useA(start: number) {", @@ -351,6 +444,68 @@ describe("UF2035", () => { ]); }); + it("rejects a parameter name a target reserves, as a setup binding's (UF2003)", () => { + const { source, module, diagnostics } = composables([ + "export function useA(ngValue: number, props: string) {", + " return {};", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2003 ngValue", "UF2003 props"]); + expect(module).toBeUndefined(); + }); + + it("rejects a `this` parameter", () => { + const { source, diagnostics } = composables([ + "export function useA(this: Window) {", + " return {};", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2035 this"]); + }); + + it("rejects a function type written through an alias or a union", () => { + const { source, diagnostics } = composables([ + "type Get = () => number;", + "type OnPick = (value: number) => void;", + "export function useA(get: Get, pick: OnPick | undefined, both: number | (() => number)) {", + " return {};", + "}", + ]); + expect(problems(source, diagnostics)).toEqual([ + "UF2035 Get", + "UF2035 OnPick | undefined", + "UF2035 number | (() => number)", + ]); + }); + + it("rejects a function that calls a callback where an initial value or a getter calls it (UF2014)", () => { + const { source, diagnostics } = composables([ + 'import { computed, ref } from "unframework";', + "export function useA(onPick: (value: number) => number) {", + " function pick() {", + " return onPick(1);", + " }", + " const first = ref(pick());", + " const next = computed(() => pick());", + " return { first, next };", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2014 pick", "UF2014 pick"]); + }); + + it("rejects a value import from a `.uf.ts` module that no composable calls, with a fix", () => { + const { source, diagnostics } = composables([ + 'import { ref } from "unframework";', + 'import { Item } from "./useItem.uf.ts";', + "export function useA(item: Item) {", + " const state = ref(item);", + " return { state };", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2035 Item"]); + expect(applyFixes(source, diagnostics[0]!.fixes!)).toContain("import { type Item }"); + }); + it("rejects a callback read outside client code", () => { const { source, diagnostics } = composables([ 'import { computed } from "unframework";', @@ -371,6 +526,22 @@ describe("UF2035", () => { expect(problems(value.source, value.diagnostics)).toEqual(["UF2035 useA"]); }); + it("rejects a generator composable", () => { + const { source, diagnostics } = composables(["export function* useA() {", " return {};", "}"]); + expect(problems(source, diagnostics)).toEqual(["UF2035 useA"]); + }); + + it("rejects an injection key in a `.uf.ts` module (UF1002)", () => { + const { source, diagnostics } = composables([ + 'import type { InjectionKey } from "unframework";', + 'export const ThemeKey: InjectionKey = Symbol("theme");', + "export function useA() {", + " return {};", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF1002 ThemeKey: InjectionKey"]); + }); + it("rejects an asynchronous composable", () => { const { source, diagnostics } = composables([ "export async function useA() {", From d5b273b7787be54793dfa800e2e29ee16fc4f0f8 Mon Sep 17 00:00:00 2001 From: Alex Grozav Date: Wed, 28 Oct 2026 11:46:56 +0200 Subject: [PATCH 3/3] fix(analyzer): read composable parameters as plain locals A read of a composable's parameter records no narrowing: only a prop's or a ref's value's read narrows in the IR. A parameter typed `Function` or a call signature, in place or through the module's aliases and interfaces, is UF2035, as a function type through an alias is. The fix for a member returned twice starts at the property before it, and UF2020's fix calls an optional getter with `?.()`. Co-authored-by: multica-agent --- packages/analyzer/src/composables.ts | 69 ++++++++++++++++- packages/analyzer/src/expressions.ts | 12 ++- packages/analyzer/src/render.ts | 2 + packages/analyzer/src/setup.ts | 6 +- packages/analyzer/test/composables.test.ts | 88 ++++++++++++++++++++++ 5 files changed, 170 insertions(+), 7 deletions(-) diff --git a/packages/analyzer/src/composables.ts b/packages/analyzer/src/composables.ts index 80c74744..a854856a 100644 --- a/packages/analyzer/src/composables.ts +++ b/packages/analyzer/src/composables.ts @@ -43,6 +43,7 @@ import { declareSetup } from "./setup.ts"; import type { InjectionKeyInfo } from "./setup.ts"; import type { Shapes } from "./shapes.ts"; import type { SharedValues } from "./shared.ts"; +import type { TypeTable } from "./types/from-type.ts"; import { FUNCTION, has, union, UNDEFINED } from "./types/kinds.ts"; import type { Kinds } from "./types/kinds.ts"; @@ -414,13 +415,16 @@ function composableParameters( } const kind = parameterKind(annotation); const written = module.writtenType(annotation); - if (kind === "value" && has(written.kinds, "function")) { + if ( + kind === "value" && + (has(written.kinds, "function") || functionLike(annotation, module.types.table)) + ) { // The kind decides each target's output (a getter is Qwik's `useComputed$`, a callback a // QRL), so it is read from the annotation as written, never guessed through an alias. reporter.report( "UF2035", annotation, - `\`${parameter.name}\`'s type may be a function, written through a name or a union: a composable's parameter writes a function type in place, which says whether it is a getter (\`() => T\`) or a callback.`, + `\`${parameter.name}\`'s type may be a function, written other than as a function type in place (through a name, a union, \`Function\` or a call signature): a composable's parameter writes a function type in place, which says whether it is a getter (\`() => T\`) or a callback.`, { help: `Write the function type in place, as in \`${parameter.name}: () => number\` or \`${parameter.name}?: (value: number) => void\`.`, }, @@ -436,6 +440,7 @@ function composableParameters( name: parameter.name, id: binding.id, kinds: optional ? union(kinds, UNDEFINED) : kinds, + argument: true, ...(kind === "callback" ? { callback: true as const } : {}), }; bindings.push(binding); @@ -467,6 +472,62 @@ function parameterKind(annotation: AST.TSType): ComposableParameterKind { return annotation.params.length ? "callback" : "getter"; } +/** + * Whether a type may be a function in a spelling other than a function type written in place: + * `Function`, a call or construct signature, a constructor type, or one of these through the + * module's aliases and interfaces, a union or an intersection (UF2035). + */ +function functionLike(type: AST.TSType, table: TypeTable, seen = new Set()): boolean { + switch (type.type) { + case "TSFunctionType": + case "TSConstructorType": + return true; + case "TSTypeLiteral": + return type.members.some(isSignature); + case "TSUnionType": + case "TSIntersectionType": + return type.types.some((each) => functionLike(each, table, seen)); + case "TSTypeReference": { + if (type.typeName.type !== "Identifier") return false; + const { name } = type.typeName; + if (name === "Function" || name === "CallableFunction" || name === "NewableFunction") { + return true; + } + const declaration = table.declaration(name)?.node; + if (!declaration || seen.has(name)) return false; + seen.add(name); + return declaration.type === "TSTypeAliasDeclaration" + ? functionLike(declaration.typeAnnotation, table, seen) + : declaration.type === "TSInterfaceDeclaration" && + (declaration.body.body.some(isSignature) || + declaration.extends.some( + (heritage) => + heritage.expression.type === "Identifier" && + functionLike( + { + type: "TSTypeReference", + typeName: heritage.expression, + start: heritage.start, + end: heritage.end, + } as AST.TSTypeReference, + table, + seen, + ), + )); + } + default: + return false; + } +} + +/** A call or construct signature, which makes an object type a function's. */ +function isSignature(member: AST.TSSignature): boolean { + return ( + member.type === "TSCallSignatureDeclaration" || + member.type === "TSConstructSignatureDeclaration" + ); +} + /** * Reports each call of another composable in a composable's body (UF2035): of one the module * imports from a `.uf.ts` module, or declares itself. Returns whether it reported, since the @@ -530,7 +591,7 @@ function returnedMembers( const { reporter, scopes, setup } = render; const members: Returned[] = []; const names = new Set(); - for (const property of object.properties) { + for (const [index, property] of object.properties.entries()) { if ( property.type !== "Property" || !property.shorthand || @@ -573,7 +634,7 @@ function returnedMembers( edits: [ { span: { - start: members.at(-1)?.member.span.end ?? property.start, + start: object.properties[index - 1]!.end, end: property.end, }, text: "", diff --git a/packages/analyzer/src/expressions.ts b/packages/analyzer/src/expressions.ts index c999d997..c2d0fc41 100644 --- a/packages/analyzer/src/expressions.ts +++ b/packages/analyzer/src/expressions.ts @@ -942,7 +942,17 @@ class Walk { return UNKNOWN; } const narrowed = render ? [] : this.#narrowingRule(node, { kind: "prop", prop }, true); - this.refs.push(createBindingReference(prop.id, span(node), shorthand, false, narrowed)); + // Only a prop's or a ref's value's read narrows in the IR: a composable's parameter is a + // plain local on every target, read as TypeScript narrows it. + this.refs.push( + createBindingReference( + prop.id, + span(node), + shorthand, + false, + prop.argument ? [] : narrowed, + ), + ); return this.#narrowed(node, prop.kinds); } const loop = context.loopVariables.get(resolution.declaration); diff --git a/packages/analyzer/src/render.ts b/packages/analyzer/src/render.ts index e8e5e135..d919d2ec 100644 --- a/packages/analyzer/src/render.ts +++ b/packages/analyzer/src/render.ts @@ -23,6 +23,8 @@ export interface PropBinding { id: BindingId | undefined; /** Its kinds: the member's type, with `undefined` when optional without a default. */ kinds: Kinds; + /** Set on a composable's parameter (ADR-0069), a plain local on every target: no read narrows. */ + argument?: true; /** Set on a composable's callback argument, which only client code reads (ADR-0069). */ callback?: true; } diff --git a/packages/analyzer/src/setup.ts b/packages/analyzer/src/setup.ts index 0492ea35..b12c7b46 100644 --- a/packages/analyzer/src/setup.ts +++ b/packages/analyzer/src/setup.ts @@ -2213,12 +2213,14 @@ function unwatchable( const annotation = (resolution as { declaration: AST.BindingIdentifier }).declaration .typeAnnotation?.typeAnnotation; const getter = annotation?.type === "TSFunctionType" && !annotation.params.length; + // An optional getter is called only where it is given. + const call = has(prop.kinds, "undefined") ? "?.()" : "()"; what = `\`${node.name}\` is ${getter ? "a getter the composable is passed, which a watcher calls in its own getter" : "an argument of the composable, which nothing can watch"}`; if (getter) { fix = { - title: `Watch \`() => ${node.name}()\``, + title: `Watch \`() => ${node.name}${call}\``, confidence: "safe", - edits: [{ span: span(node), text: `() => ${node.name}()` }], + edits: [{ span: span(node), text: `() => ${node.name}${call}` }], }; } } else if (prop) { diff --git a/packages/analyzer/test/composables.test.ts b/packages/analyzer/test/composables.test.ts index 2e321753..594e40eb 100644 --- a/packages/analyzer/test/composables.test.ts +++ b/packages/analyzer/test/composables.test.ts @@ -283,6 +283,61 @@ describe("a composable", () => { ]); }); + it("watches an optional getter through an optional call of it", () => { + const { source, diagnostics } = composables([ + 'import { ref, watch } from "unframework";', + "export function useSeen(value?: () => number) {", + " const seen = ref(0);", + " watch(value, () => {", + " seen.value++;", + " });", + " return { seen };", + "}", + ]); + expect(problems(source, diagnostics)).toEqual(["UF2020 value"]); + const fixed = applyFixes(source, diagnostics[0]!.fixes!); + expect(fixed).toContain("watch(() => value?.(), () => {"); + expect(run(fixed, "useSeen.uf.ts").diagnostics).toEqual([]); + }); + + it.each([ + [ + "a callback tested before its call", + "(v: number) => void", + "function pick() {\n if (p) p(1);\n }", + "pick", + ], + [ + "a value tested against `undefined`", + "number", + "const v = ref(p !== undefined ? p : 0);", + "v", + ], + [ + "a getter tested in a derived value", + "() => number", + "const v = computed(() => (p ? p() : 0));", + "v", + ], + [ + "a getter tested in a watcher's getter", + "() => number", + "const v = ref(0);\n watch(() => (p ? p() : 0), () => {\n v.value++;\n });", + "v", + ], + ])("reads an optional parameter as TypeScript narrows it: %s", (_, type, setup, returned) => { + const { module, diagnostics } = composables([ + 'import { computed, ref, watch } from "unframework";', + `export function useA(p?: ${type}) {`, + ` ${setup}`, + ` return { ${returned} };`, + "}", + ]); + expect(diagnostics).toEqual([]); + // No read of a parameter narrows in the IR (`run` checks the invariants). + expect(JSON.stringify(module!.composables)).not.toContain('"narrowed"'); + }); + it("leaves a `.uf.tsx` module's analysis as it was", () => { const { module, diagnostics } = run( [ @@ -416,6 +471,21 @@ describe("UF2035", () => { expect(applyFixes(source, diagnostics[0]!.fixes!)).toContain("return { n };"); }); + it("removes only the repeated member, whatever stands between", () => { + const between = useA(["const m = ref(2);"], "return { n, count: m, n };"); + expect(problems(between.source, between.diagnostics)).toEqual(["UF2035 count: m", "UF2035 n"]); + expect(applyFixes(between.source, between.diagnostics[1]!.fixes!)).toContain( + "return { n, count: m };", + ); + const thrice = useA([], "return { n, n, n };"); + expect( + applyFixes( + thrice.source, + thrice.diagnostics.flatMap(({ fixes }) => fixes!), + ), + ).toContain("return { n };"); + }); + it("rejects a returned setup `let`", () => { const { source, diagnostics } = useA(["let timer = 0;"], "return { n, timer };"); expect(problems(source, diagnostics)).toEqual(["UF2035 timer"]); @@ -463,6 +533,24 @@ describe("UF2035", () => { expect(problems(source, diagnostics)).toEqual(["UF2035 this"]); }); + it("rejects `Function` and a call signature, in place or through an alias or an interface", () => { + const { source, diagnostics } = composables([ + "type Any = Function;", + "interface OnPick { (value: number): void }", + "interface Picker extends OnPick { label: string }", + "export function useA(a: Function, b: { (value: number): void }, c: Any, d: OnPick, e: Picker) {", + " return {};", + "}", + ]); + expect(problems(source, diagnostics)).toEqual([ + "UF2035 Function", + "UF2035 { (value: number): void }", + "UF2035 Any", + "UF2035 OnPick", + "UF2035 Picker", + ]); + }); + it("rejects a function type written through an alias or a union", () => { const { source, diagnostics } = composables([ "type Get = () => number;",