From 498c4fec4d566cbbd2171ec9d1625a95a2ab6b52 Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Thu, 23 Apr 2026 15:33:19 +0400 Subject: [PATCH 01/10] feat(js): service-scoped type resolution + generic substitution on TypeResolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #1317. Adds three public methods on `TypeResolver` and a convenience accessor on `SailsProgram` so downstream consumers (wallets, explorers, custom codegen) stop re-implementing type-scope + generic-substitution out-of-band. - `TypeResolver.resolveNamed(typeDecl)` — look up a named user `Type` from the resolver's scope, honoring ambient-vs-local shadowing. - `TypeResolver.substituteGenerics(typeDecl, substitutions)` — pure, idempotent tree walk that substitutes bare `{kind:'named', name:'T'}` leaves through chains (`{T: U, U: u32}` resolves `T` to `u32`). Runtime cycle guard on self-referential or cyclic maps. Same-reference return on unchanged subtrees avoids pointless allocation in walkers. - `TypeResolver.genericsSubstitutions(userType, generics)` — zip a user type's `type_params` with a concrete generics list into a sub map. `getTypeDeclString` now uses this helper instead of an inline copy. - `TypeResolver` constructor gained an `ambientTypes` parameter; `SailsProgram` threads `program.types` into every `SailsService` (including the `extends` chain) so service-local types shadow program-level ambients inside each per-service resolver. - `SailsProgram.resolveInService(serviceName, typeDecl)` — direct AST lookup that skips `SailsService`/`TypeResolver` construction, safe to call in tight walker loops. Tests: 21 new (17 unit + 4 parser-driven integration) covering all five `TypeDecl` variants, shadowing with registry-level collision, chain resolution, cycle detection, unknown-kind rejection, and service-scoped collision across two services declaring the same type name with incompatible shapes. --- js/src/sails-idl-v2.ts | 29 ++- js/src/type-resolver-idl-v2.ts | 111 ++++++++++- js/test/idl-v2-parser-type-resolver.test.ts | 129 +++++++++++++ js/test/idl-v2-type-resolver.test.ts | 197 ++++++++++++++++++++ 4 files changed, 455 insertions(+), 11 deletions(-) diff --git a/js/src/sails-idl-v2.ts b/js/src/sails-idl-v2.ts index bc4a889bb..02e2845ca 100644 --- a/js/src/sails-idl-v2.ts +++ b/js/src/sails-idl-v2.ts @@ -2,6 +2,7 @@ import { GearApi, HexString, UserMessageSent } from '@gear-js/api'; import { u8aToHex, u8aToU8a } from '@polkadot/util'; import type { + Type, TypeDecl, IIdlDoc, IServiceExpo, @@ -141,7 +142,7 @@ export class SailsProgram { constructor(doc: IIdlDoc) { this._doc = doc; if (this._doc.program) { - this._typeResolver = new TypeResolver(this._doc.program.types); + this._typeResolver = new TypeResolver(this._doc.program.types ?? []); } this._services = this._initServices(); } @@ -215,11 +216,31 @@ export class SailsProgram { this._programId, expo.route_idx, this._resolveServiceUnit, + program.types ?? [], ); } return services; } + /** + * Resolve a `TypeDecl` to its user-type definition in the scope of a named service. + * Service-local types shadow program-level types; program-level types are visible to + * every service. Returns `undefined` when the service doesn't exist, or the `TypeDecl` + * isn't a named user type. + * + * Looks up the AST directly (does not build a `SailsService` or `TypeResolver`) so it + * is safe to call in tight loops while walking large IDL trees. + */ + resolveInService(serviceName: string, typeDecl: TypeDecl): Type | undefined { + if (typeof typeDecl === 'string' || typeDecl.kind !== 'named') return undefined; + const serviceUnit = this._doc.services?.find((s) => s.name === serviceName); + if (!serviceUnit) return undefined; + return ( + serviceUnit.types?.find((t) => t.name === typeDecl.name) ?? + this._doc.program?.types?.find((t) => t.name === typeDecl.name) + ); + } + /** #### Constructor functions with arguments from the parsed IDL */ get ctors(): Record | null { if (!this._doc.program) { @@ -332,6 +353,7 @@ export class SailsService implements ISailsService { private _typeResolver: TypeResolver; private _api?: GearApi; private _routeIdx: number; + private _ambientTypes: Type[] = []; private _resolveServiceUnit?: (ident: IServiceIdent) => IServiceUnit | undefined; @@ -341,12 +363,14 @@ export class SailsService implements ISailsService { programId?: HexString, routeIdx = 0, resolveServiceUnit?: (ident: IServiceIdent) => IServiceUnit | undefined, + ambientTypes: Type[] = [], ) { this._service = service; this._api = api; this._programId = programId; this._resolveServiceUnit = resolveServiceUnit; - this._typeResolver = new TypeResolver(service.types); + this._typeResolver = new TypeResolver(service.types ?? [], ambientTypes); + this._ambientTypes = ambientTypes; this._routeIdx = routeIdx; this.events = this._getEvents(service); @@ -569,6 +593,7 @@ export class SailsService implements ISailsService { this._programId, this.routeIdx, this._resolveServiceUnit, + this._ambientTypes, ); } diff --git a/js/src/type-resolver-idl-v2.ts b/js/src/type-resolver-idl-v2.ts index 0a0b23162..451a11d3a 100644 --- a/js/src/type-resolver-idl-v2.ts +++ b/js/src/type-resolver-idl-v2.ts @@ -16,23 +16,120 @@ export class TypeResolver { registry: TypeRegistry; private _userTypes: Record = {}; - constructor(types: Type[]) { + /** + * @param types service-local (or program-level) user types. Takes precedence over `ambientTypes` on name collision. + * @param ambientTypes program-level types visible to every service. Shadowed by `types` when names collide. + */ + constructor(types: Type[], ambientTypes: Type[] = []) { this.registry = new TypeRegistry(); const scaleTypes: Record = {}; const userTypes: Record = {}; - for (const type of types) { + for (const type of [...ambientTypes, ...types]) { userTypes[type.name] = type; + } + this._userTypes = userTypes; + for (const type of Object.values(userTypes)) { if (!type.type_params?.length) { - // register non-generic by name scaleTypes[type.name] = this.getTypeDef(type); } } - this._userTypes = userTypes; this.registry.setKnownTypes({ types: scaleTypes }); this.registry.register(scaleTypes); } + /** + * Resolve a `TypeDecl`'s named user type to its `Type` definition. + * + * Returns `undefined` for primitives, slices, arrays, tuples, unknown names, and bare + * type parameters (a `{ kind: 'named', name: 'T' }` that isn't a registered user type). + * Does not recurse into generics — callers that want the substituted inner shape should + * pair this with {@link substituteGenerics}. + * + * The returned `Type` is shared with the resolver's internal state — do not mutate it. + */ + resolveNamed(type: TypeDecl): Type | undefined { + if (typeof type === 'string') return undefined; + if (type.kind !== 'named') return undefined; + return this._userTypes[type.name]; + } + + /** + * Recursively substitute type parameters through a `TypeDecl` tree. + * + * Pure: does not mutate inputs. Idempotent: passing an already-substituted tree yields an + * equivalent tree. Only bare `{ kind: 'named', name: 'T' }` leaves whose `name` appears in + * `substitutions` are replaced; wrapper shapes (`Option`, `Vec`, `Result`, custom + * generics) are preserved and their inner types substituted in place. + * + * The function recurses through replacement chains (`{ T: U, U: u32 }` resolves `T` to `u32`). + * Cyclic maps (`{ T: { kind: 'named', name: 'T' } }` or `{ T: U, U: T }`) are detected at + * runtime and cause an error to be thrown rather than an unbounded recursion. Maps produced by + * {@link genericsSubstitutions} from a parsed IDL cannot create cycles. + */ + substituteGenerics(type: TypeDecl, substitutions: Record = {}): TypeDecl { + return this._substituteGenerics(type, substitutions, new Set()); + } + + private _substituteGenerics( + type: TypeDecl, + substitutions: Record, + visited: Set, + ): TypeDecl { + if (typeof type === 'string') return type; + if (type.kind === 'slice') { + const item = this._substituteGenerics(type.item, substitutions, visited); + return item === type.item ? type : { kind: 'slice', item }; + } + if (type.kind === 'array') { + const item = this._substituteGenerics(type.item, substitutions, visited); + return item === type.item ? type : { kind: 'array', item, len: type.len }; + } + if (type.kind === 'tuple') { + const next = type.types.map((t) => this._substituteGenerics(t, substitutions, visited)); + return next.every((t, i) => t === type.types[i]) ? type : { kind: 'tuple', types: next }; + } + if (type.kind === 'named') { + if (type.generics?.length) { + const next = type.generics.map((g) => this._substituteGenerics(g, substitutions, visited)); + return next.every((g, i) => g === type.generics![i]) + ? type + : { kind: 'named', name: type.name, generics: next }; + } + // Bare named reference may be a type parameter. Track visited names so a cyclic map + // (`{ T: T }`, `{ T: U, U: T }`) throws instead of stack-overflowing. + const replacement = substitutions[type.name]; + if (replacement !== undefined) { + if (visited.has(type.name)) { + throw new Error( + `Cyclic substitution detected while resolving type parameter "${type.name}" — ` + + `substitution chain: ${[...visited, type.name].join(' → ')}`, + ); + } + const nextVisited = new Set(visited); + nextVisited.add(type.name); + return this._substituteGenerics(replacement, substitutions, nextVisited); + } + return type; + } + throw new Error('Unknown TypeDecl kind :: ' + JSON.stringify(type)); + } + + /** + * Build a substitution map from a user type's declared `type_params` and a concrete + * generics list (typically the `generics` field of a `{ kind: 'named', generics: [...] }` + * `TypeDecl`). Missing positions are omitted. + */ + genericsSubstitutions(userType: Type, generics: TypeDecl[] = []): Record { + const map: Record = {}; + const params = userType.type_params ?? []; + const len = Math.min(params.length, generics.length); + for (let i = 0; i < len; i++) { + map[params[i].name] = generics[i]; + } + return map; + } + /** * Convert a `TypeDecl` into a concrete string name, resolving generic parameters. * @@ -131,11 +228,7 @@ export class TypeResolver { .map((t: TypeDecl) => this.getTypeDeclString(t, generics, 'canonical')) .join('')}`; if (!this.registry.hasType(canonicalName)) { - // type param to generic map, i.e, { "T": "String", "U": { "kind": "named", "name": "Option", "generics": ["u32"]} } - const generics_map: Record = {}; - for (let i = 0; i < userType.type_params.length; i++) { - generics_map[userType.type_params[i].name] = type.generics[i]; - } + const generics_map = this.genericsSubstitutions(userType, type.generics); const typeDef = this.getTypeDef(userType, generics_map); /// When a user type with generics is resolved, the resolver constructs two names: // - genericName: readable, type-like syntax (example MyType>). diff --git a/js/test/idl-v2-parser-type-resolver.test.ts b/js/test/idl-v2-parser-type-resolver.test.ts index 6cd6fba93..6cdb36df4 100644 --- a/js/test/idl-v2-parser-type-resolver.test.ts +++ b/js/test/idl-v2-parser-type-resolver.test.ts @@ -341,3 +341,132 @@ describe('type-resolver-v2 generics', () => { ).toEqual({ three: { p1: arrayTupleU8, p2: [['a', 'b', 'c', 'd'], null] } }); }); }); + +describe('sails v2 service-scoped type resolution', () => { + const TWO_SERVICES_SAME_NAME = ` + !@sails: 1.0.0-beta.3 + + service A@0xa667a3b129e57f5c { + functions { + Set(p: Packet); + } + types { + struct Packet { + payload: [u8; 4], + } + } + } + + service B@0x8b02064fa4f2f602 { + functions { + Set(p: Packet); + } + types { + struct Packet { + payload: [u8; 8], + } + } + } + + program Test { + services { + A@0xa667a3b129e57f5c, + B@0x8b02064fa4f2f602, + } + } + `; + + test('resolveInService returns the service-local Type on name collision', () => { + const program = new SailsProgram(parser.parse(TWO_SERVICES_SAME_NAME)); + + const a = program.resolveInService('A', { kind: 'named', name: 'Packet' }); + const b = program.resolveInService('B', { kind: 'named', name: 'Packet' }); + expect(a?.kind).toBe('struct'); + expect(b?.kind).toBe('struct'); + // Differentiate by the array length on the single field. + const aField = (a as any).fields[0].type; + const bField = (b as any).fields[0].type; + expect(aField).toEqual({ kind: 'array', item: 'u8', len: 4 }); + expect(bField).toEqual({ kind: 'array', item: 'u8', len: 8 }); + }); + + test('resolveInService returns undefined for unknown service names', () => { + const program = new SailsProgram(parser.parse(TWO_SERVICES_SAME_NAME)); + expect(program.resolveInService('Nonexistent', { kind: 'named', name: 'Packet' })).toBeUndefined(); + }); + + test('program-level (ambient) types are visible inside service resolvers', () => { + // Program-level `Shared` is referenced by the ctor (parser rejects it in service signatures) + // but must still resolve through the service's resolver for consumers walking ctor args. + const text = ` + !@sails: 1.0.0-beta.3 + + service A@0x4071744d7e684110 { + functions { + Ping() -> u32; + } + } + + program Test { + constructors { + Default(shared: Shared); + } + services { + A@0x4071744d7e684110, + } + types { + struct Shared { + v: u32, + } + } + } + `; + const program = new SailsProgram(parser.parse(text)); + const t = program.resolveInService('A', { kind: 'named', name: 'Shared' }); + expect(t?.kind).toBe('struct'); + expect(t?.name).toBe('Shared'); + }); + + test('generic substitution: Envelope<[u8]>.payload resolves to [u8]', () => { + const text = ` + !@sails: 1.0.0-beta.3 + + service Gen@0x8c5db6384e4cf753 { + functions { + SetPayload(p: Envelope<[u8]>); + } + types { + struct Envelope { + id: u32, + payload: T, + } + } + } + + program Test { + services { + Gen@0x8c5db6384e4cf753, + } + } + `; + const program = new SailsProgram(parser.parse(text)); + const service = program.services['Gen']; + const envelope = service.typeResolver.resolveNamed({ + kind: 'named', + name: 'Envelope', + generics: [{ kind: 'slice', item: 'u8' }], + }); + expect(envelope?.kind).toBe('struct'); + // Build substitution map from the concrete generics list on the arg. + const subs = service.typeResolver.genericsSubstitutions(envelope!, [ + { kind: 'slice', item: 'u8' }, + ]); + expect(subs).toEqual({ T: { kind: 'slice', item: 'u8' } }); + + // Walk the struct fields through substituteGenerics — payload's T becomes [u8]. + const payloadField = (envelope as any).fields.find((f: any) => f.name === 'payload'); + expect(payloadField).toBeDefined(); + const resolvedPayloadType = service.typeResolver.substituteGenerics(payloadField.type, subs); + expect(resolvedPayloadType).toEqual({ kind: 'slice', item: 'u8' }); + }); +}); diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index 1ee369301..4c72054b9 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -288,6 +288,203 @@ describe('type-resolver-v2 structs', () => { }); }); +describe('type-resolver-v2 substituteGenerics', () => { + const resolver = new TypeResolver([]); + + test('replaces a bare named type_param with a primitive', () => { + expect(resolver.substituteGenerics(named('T'), { T: 'u32' })).toBe('u32'); + }); + + test('recurses through slice / array / tuple', () => { + const input: TypeDecl = { + kind: 'tuple', + types: [ + { kind: 'slice', item: named('T') }, + { kind: 'array', item: named('T'), len: 4 }, + ], + }; + expect(resolver.substituteGenerics(input, { T: 'u8' })).toEqual({ + kind: 'tuple', + types: [ + { kind: 'slice', item: 'u8' }, + { kind: 'array', item: 'u8', len: 4 }, + ], + }); + }); + + test('recurses through named-with-generics (Option, custom wrappers)', () => { + const input: TypeDecl = named('Envelope', [named('Option', [named('T')])]); + expect( + resolver.substituteGenerics(input, { T: { kind: 'slice', item: 'u8' } }), + ).toEqual(named('Envelope', [named('Option', [{ kind: 'slice', item: 'u8' }])])); + }); + + test('passes through unknown named refs that are not in substitutions', () => { + expect(resolver.substituteGenerics(named('Unknown'), {})).toEqual(named('Unknown')); + }); + + test('is a no-op on primitives and on inputs with no type_params', () => { + expect(resolver.substituteGenerics('u32')).toBe('u32'); + expect(resolver.substituteGenerics({ kind: 'slice', item: 'u8' }, { T: 'u64' })).toEqual({ + kind: 'slice', + item: 'u8', + }); + }); + + test('is idempotent', () => { + const input: TypeDecl = named('Envelope', [named('T')]); + const once = resolver.substituteGenerics(input, { T: 'u32' }); + expect(resolver.substituteGenerics(once, { T: 'u32' })).toEqual(once); + }); + + test('does not mutate the input tree', () => { + const input: TypeDecl = { kind: 'slice', item: named('T') }; + const snapshot = JSON.parse(JSON.stringify(input)); + resolver.substituteGenerics(input, { T: 'u8' }); + expect(input).toEqual(snapshot); + }); + + test('resolves substitution chains (T -> U -> u32)', () => { + expect( + resolver.substituteGenerics(named('T'), { T: named('U'), U: 'u32' }), + ).toBe('u32'); + }); + + test('throws on self-referential substitution map', () => { + expect(() => resolver.substituteGenerics(named('T'), { T: named('T') })).toThrow(/[Cc]yclic/); + }); + + test('throws on cyclic substitution chain (T -> U -> T)', () => { + expect(() => + resolver.substituteGenerics(named('T'), { T: named('U'), U: named('T') }), + ).toThrow(/[Cc]yclic/); + }); + + test('throws on unknown TypeDecl kind', () => { + // A `Type` (kind: 'struct') is not a valid TypeDecl — catch the misuse loudly. + const bogus = { kind: 'struct', name: 'X', fields: [] } as unknown as TypeDecl; + expect(() => resolver.substituteGenerics(bogus)).toThrow(/Unknown TypeDecl kind/); + }); +}); + +describe('type-resolver-v2 resolveNamed', () => { + const packet: Type = { + kind: 'struct', + name: 'Packet', + fields: [{ name: 'payload', type: { kind: 'array', item: 'u8', len: 4 } }], + }; + const envelope: Type = { + kind: 'struct', + name: 'Envelope', + type_params: [{ name: 'T' }], + fields: [ + { name: 'id', type: 'u32' }, + { name: 'payload', type: named('T') }, + ], + }; + + test('returns the user Type for a known named decl', () => { + const resolver = new TypeResolver([packet]); + expect(resolver.resolveNamed(named('Packet'))).toBe(packet); + }); + + test('returns the user Type for a generic named decl (ignoring generics)', () => { + const resolver = new TypeResolver([envelope]); + expect(resolver.resolveNamed(named('Envelope', ['u32']))).toBe(envelope); + }); + + test('returns undefined for primitives, slices, arrays, tuples', () => { + const resolver = new TypeResolver([]); + expect(resolver.resolveNamed('u32')).toBeUndefined(); + expect(resolver.resolveNamed({ kind: 'slice', item: 'u8' })).toBeUndefined(); + expect(resolver.resolveNamed({ kind: 'array', item: 'u8', len: 4 })).toBeUndefined(); + expect(resolver.resolveNamed({ kind: 'tuple', types: ['u8', 'u16'] })).toBeUndefined(); + }); + + test('returns undefined for unknown names and bare type_params', () => { + const resolver = new TypeResolver([]); + expect(resolver.resolveNamed(named('Unknown'))).toBeUndefined(); + expect(resolver.resolveNamed(named('T'))).toBeUndefined(); + }); +}); + +describe('type-resolver-v2 genericsSubstitutions', () => { + test('zips type_params with concrete generics', () => { + const resolver = new TypeResolver([]); + const userType: Type = { + kind: 'struct', + name: 'Pair', + type_params: [{ name: 'T' }, { name: 'U' }], + fields: [ + { name: 'left', type: named('T') }, + { name: 'right', type: named('U') }, + ], + }; + expect(resolver.genericsSubstitutions(userType, ['u32', 'String'])).toEqual({ + T: 'u32', + U: 'String', + }); + }); + + test('returns empty map when userType has no type_params', () => { + const resolver = new TypeResolver([]); + const userType: Type = { + kind: 'struct', + name: 'Plain', + fields: [{ name: 'x', type: 'u32' }], + }; + expect(resolver.genericsSubstitutions(userType, ['u32'])).toEqual({}); + }); + + test('ignores extra concrete generics past the declared params', () => { + const resolver = new TypeResolver([]); + const userType: Type = { + kind: 'struct', + name: 'One', + type_params: [{ name: 'T' }], + fields: [{ name: 'v', type: named('T') }], + }; + expect(resolver.genericsSubstitutions(userType, ['u8', 'u16'])).toEqual({ T: 'u8' }); + }); +}); + +describe('type-resolver-v2 ambient types', () => { + const ambientPacket: Type = { + kind: 'struct', + name: 'Packet', + fields: [{ name: 'payload', type: { kind: 'array', item: 'u8', len: 4 } }], + }; + const localPacket: Type = { + kind: 'struct', + name: 'Packet', + fields: [{ name: 'payload', type: { kind: 'array', item: 'u8', len: 8 } }], + }; + const ambientOnly: Type = { + kind: 'struct', + name: 'AmbientOnly', + fields: [{ name: 'v', type: 'u32' }], + }; + + test('ambient types are resolvable when not shadowed', () => { + const resolver = new TypeResolver([], [ambientOnly]); + expect(resolver.resolveNamed(named('AmbientOnly'))).toBe(ambientOnly); + }); + + test('service-local types shadow ambients on name collision', () => { + const resolver = new TypeResolver([localPacket], [ambientPacket]); + expect(resolver.resolveNamed(named('Packet'))).toBe(localPacket); + }); + + test('registry reflects shadowing (local wins)', () => { + const resolver = new TypeResolver([localPacket], [ambientPacket]); + // With localPacket (len 8), an 8-byte array round-trips; a 4-byte payload would fail. + const encoded = resolver.registry.createType('Packet', { payload: [1, 2, 3, 4, 5, 6, 7, 8] }); + expect(encoded.toJSON()).toEqual({ payload: '0x0102030405060708' }); + // Attempting to create with only 4 bytes should fail (ambient shape was overridden). + expect(() => resolver.registry.createType('Packet', { payload: [1, 2, 3, 4] })).toThrow(); + }); +}); + describe('type-resolver-v2 aliases', () => { test('simple alias', () => { const userType: any = { From f7f63b2a4b236c8c727a37749a6c02695556c247 Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Thu, 23 Apr 2026 15:51:22 +0400 Subject: [PATCH 02/10] =?UTF-8?q?fix(js):=20address=20#1323=20review=20?= =?UTF-8?q?=E2=80=94=20O(1)=20resolveInService=20+=20structuredClone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Build a lazy `Map>` index in `SailsProgram` on first `resolveInService` call. O(1) per call after that, no cost for consumers who never use the method. Addresses the gemini-code-assist comment on sails-idl-v2.ts:242. - Swap `JSON.parse(JSON.stringify(input))` in the "does not mutate" test for `structuredClone(input)` to satisfy `unicorn/prefer-structured-clone` (fixes the lint job failure on the v1.6.1.0 JS CI). --- js/src/sails-idl-v2.ts | 28 ++++++++++++++++++++++------ js/test/idl-v2-type-resolver.test.ts | 2 +- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/js/src/sails-idl-v2.ts b/js/src/sails-idl-v2.ts index 02e2845ca..f82f2c708 100644 --- a/js/src/sails-idl-v2.ts +++ b/js/src/sails-idl-v2.ts @@ -127,6 +127,10 @@ export class SailsProgram { private _api?: GearApi; private _programId?: HexString; private _services: Map; + // Lazy indices for resolveInService. Populated on first call; not invalidated because + // `_doc` is immutable after parse. + private _serviceTypeIndex?: Map>; + private _programTypeIndex?: Map; private _resolveServiceUnit = (ident: IServiceIdent): IServiceUnit | undefined => { if (!ident.interface_id) { throw new Error(`Service "${ident.name}" is missing interface_id in IDL`); @@ -228,19 +232,31 @@ export class SailsProgram { * every service. Returns `undefined` when the service doesn't exist, or the `TypeDecl` * isn't a named user type. * - * Looks up the AST directly (does not build a `SailsService` or `TypeResolver`) so it - * is safe to call in tight loops while walking large IDL trees. + * Backed by a lazy `Map`-based index — O(1) per call after the first invocation. + * Safe to call in tight loops while walking large IDL trees. */ resolveInService(serviceName: string, typeDecl: TypeDecl): Type | undefined { if (typeof typeDecl === 'string' || typeDecl.kind !== 'named') return undefined; - const serviceUnit = this._doc.services?.find((s) => s.name === serviceName); - if (!serviceUnit) return undefined; + if (!this._serviceTypeIndex) this._buildTypeIndex(); return ( - serviceUnit.types?.find((t) => t.name === typeDecl.name) ?? - this._doc.program?.types?.find((t) => t.name === typeDecl.name) + this._serviceTypeIndex!.get(serviceName)?.get(typeDecl.name) ?? + this._programTypeIndex!.get(typeDecl.name) ); } + private _buildTypeIndex(): void { + const serviceIndex = new Map>(); + for (const unit of this._doc.services ?? []) { + const local = new Map(); + for (const t of unit.types ?? []) local.set(t.name, t); + serviceIndex.set(unit.name, local); + } + const programIndex = new Map(); + for (const t of this._doc.program?.types ?? []) programIndex.set(t.name, t); + this._serviceTypeIndex = serviceIndex; + this._programTypeIndex = programIndex; + } + /** #### Constructor functions with arguments from the parsed IDL */ get ctors(): Record | null { if (!this._doc.program) { diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index 4c72054b9..fe227453c 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -339,7 +339,7 @@ describe('type-resolver-v2 substituteGenerics', () => { test('does not mutate the input tree', () => { const input: TypeDecl = { kind: 'slice', item: named('T') }; - const snapshot = JSON.parse(JSON.stringify(input)); + const snapshot = structuredClone(input); resolver.substituteGenerics(input, { T: 'u8' }); expect(input).toEqual(snapshot); }); From dd74a924d7afab6f91c63cdb197db74dd382af5a Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Thu, 23 Apr 2026 18:32:27 +0400 Subject: [PATCH 03/10] =?UTF-8?q?refactor(js):=20simplify=20TypeResolver?= =?UTF-8?q?=20=E2=80=94=20single-arg=20constructor=20+=20TypeDecl::Generic?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per maintainer feedback on #1323: - `TypeResolver` constructor reverts to `(types: Type[])`. Callers that want ambient-vs-local shadowing pass `[...ambientTypes, ...localTypes]` — the merge is the caller's concern, not the resolver's. Moved the merge to `SailsService`, which is the only caller that needs it. - `substituteGenerics` and `resolveNamed` now target the explicit `TypeDecl::Generic` AST variant (landed in #1314). A bare `{kind:'named'}` decl is always a concrete user-type reference and is no longer treated as a type-parameter fallback; substitution only fires on `{kind:'generic'}` leaves. Cycle detection, chain resolution, and same-reference returns on unchanged subtrees all unchanged. - Tests updated to use `{kind:'generic', name:'T'}` for type parameters. Ambient-types describe block renamed to "last-write-wins merge (shadowing via call-site merge)" to reflect the new API shape. --- js/src/sails-idl-v2.ts | 3 +- js/src/type-resolver-idl-v2.ts | 60 +++++++++++------------- js/test/idl-v2-type-resolver.test.ts | 70 +++++++++++++--------------- 3 files changed, 62 insertions(+), 71 deletions(-) diff --git a/js/src/sails-idl-v2.ts b/js/src/sails-idl-v2.ts index f82f2c708..2cca50024 100644 --- a/js/src/sails-idl-v2.ts +++ b/js/src/sails-idl-v2.ts @@ -385,7 +385,8 @@ export class SailsService implements ISailsService { this._api = api; this._programId = programId; this._resolveServiceUnit = resolveServiceUnit; - this._typeResolver = new TypeResolver(service.types ?? [], ambientTypes); + // Ambient types come first so service-local types shadow them on name collision. + this._typeResolver = new TypeResolver([...ambientTypes, ...(service.types ?? [])]); this._ambientTypes = ambientTypes; this._routeIdx = routeIdx; diff --git a/js/src/type-resolver-idl-v2.ts b/js/src/type-resolver-idl-v2.ts index 451a11d3a..2442d2228 100644 --- a/js/src/type-resolver-idl-v2.ts +++ b/js/src/type-resolver-idl-v2.ts @@ -16,16 +16,14 @@ export class TypeResolver { registry: TypeRegistry; private _userTypes: Record = {}; - /** - * @param types service-local (or program-level) user types. Takes precedence over `ambientTypes` on name collision. - * @param ambientTypes program-level types visible to every service. Shadowed by `types` when names collide. - */ - constructor(types: Type[], ambientTypes: Type[] = []) { + constructor(types: Type[]) { this.registry = new TypeRegistry(); const scaleTypes: Record = {}; const userTypes: Record = {}; - for (const type of [...ambientTypes, ...types]) { + // Iteration order is last-write-wins: callers that want ambient-type shadowing should + // pass `[...ambientTypes, ...localTypes]` so locals overwrite on name collision. + for (const type of types) { userTypes[type.name] = type; } this._userTypes = userTypes; @@ -41,10 +39,9 @@ export class TypeResolver { /** * Resolve a `TypeDecl`'s named user type to its `Type` definition. * - * Returns `undefined` for primitives, slices, arrays, tuples, unknown names, and bare - * type parameters (a `{ kind: 'named', name: 'T' }` that isn't a registered user type). - * Does not recurse into generics — callers that want the substituted inner shape should - * pair this with {@link substituteGenerics}. + * Returns `undefined` for primitives, slices, arrays, tuples, type parameters + * (`{ kind: 'generic' }`), and unknown names. Does not recurse into generics — callers + * that want the substituted inner shape should pair this with {@link substituteGenerics}. * * The returned `Type` is shared with the resolver's internal state — do not mutate it. */ @@ -58,12 +55,12 @@ export class TypeResolver { * Recursively substitute type parameters through a `TypeDecl` tree. * * Pure: does not mutate inputs. Idempotent: passing an already-substituted tree yields an - * equivalent tree. Only bare `{ kind: 'named', name: 'T' }` leaves whose `name` appears in + * equivalent tree. Only `{ kind: 'generic', name: 'T' }` leaves whose `name` appears in * `substitutions` are replaced; wrapper shapes (`Option`, `Vec`, `Result`, custom - * generics) are preserved and their inner types substituted in place. + * generics, and any `named` decl) are preserved and their inner types substituted in place. * * The function recurses through replacement chains (`{ T: U, U: u32 }` resolves `T` to `u32`). - * Cyclic maps (`{ T: { kind: 'named', name: 'T' } }` or `{ T: U, U: T }`) are detected at + * Cyclic maps (`{ T: { kind: 'generic', name: 'T' } }` or `{ T: U, U: T }`) are detected at * runtime and cause an error to be thrown rather than an unbounded recursion. Maps produced by * {@link genericsSubstitutions} from a parsed IDL cannot create cycles. */ @@ -89,28 +86,27 @@ export class TypeResolver { const next = type.types.map((t) => this._substituteGenerics(t, substitutions, visited)); return next.every((t, i) => t === type.types[i]) ? type : { kind: 'tuple', types: next }; } - if (type.kind === 'named') { - if (type.generics?.length) { - const next = type.generics.map((g) => this._substituteGenerics(g, substitutions, visited)); - return next.every((g, i) => g === type.generics![i]) - ? type - : { kind: 'named', name: type.name, generics: next }; - } - // Bare named reference may be a type parameter. Track visited names so a cyclic map + if (type.kind === 'generic') { + // Explicit type-parameter leaf. Track visited names so a cyclic map // (`{ T: T }`, `{ T: U, U: T }`) throws instead of stack-overflowing. const replacement = substitutions[type.name]; - if (replacement !== undefined) { - if (visited.has(type.name)) { - throw new Error( - `Cyclic substitution detected while resolving type parameter "${type.name}" — ` + - `substitution chain: ${[...visited, type.name].join(' → ')}`, - ); - } - const nextVisited = new Set(visited); - nextVisited.add(type.name); - return this._substituteGenerics(replacement, substitutions, nextVisited); + if (replacement === undefined) return type; + if (visited.has(type.name)) { + throw new Error( + `Cyclic substitution detected while resolving type parameter "${type.name}" — ` + + `substitution chain: ${[...visited, type.name].join(' → ')}`, + ); } - return type; + const nextVisited = new Set(visited); + nextVisited.add(type.name); + return this._substituteGenerics(replacement, substitutions, nextVisited); + } + if (type.kind === 'named') { + if (!type.generics?.length) return type; + const next = type.generics.map((g) => this._substituteGenerics(g, substitutions, visited)); + return next.every((g, i) => g === type.generics![i]) + ? type + : { kind: 'named', name: type.name, generics: next }; } throw new Error('Unknown TypeDecl kind :: ' + JSON.stringify(type)); } diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index fe227453c..6a28a5efe 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -291,16 +291,16 @@ describe('type-resolver-v2 structs', () => { describe('type-resolver-v2 substituteGenerics', () => { const resolver = new TypeResolver([]); - test('replaces a bare named type_param with a primitive', () => { - expect(resolver.substituteGenerics(named('T'), { T: 'u32' })).toBe('u32'); + test('replaces a type parameter leaf with a primitive', () => { + expect(resolver.substituteGenerics(generic('T'), { T: 'u32' })).toBe('u32'); }); test('recurses through slice / array / tuple', () => { const input: TypeDecl = { kind: 'tuple', types: [ - { kind: 'slice', item: named('T') }, - { kind: 'array', item: named('T'), len: 4 }, + { kind: 'slice', item: generic('T') }, + { kind: 'array', item: generic('T'), len: 4 }, ], }; expect(resolver.substituteGenerics(input, { T: 'u8' })).toEqual({ @@ -313,17 +313,21 @@ describe('type-resolver-v2 substituteGenerics', () => { }); test('recurses through named-with-generics (Option, custom wrappers)', () => { - const input: TypeDecl = named('Envelope', [named('Option', [named('T')])]); + const input: TypeDecl = named('Envelope', [named('Option', [generic('T')])]); expect( resolver.substituteGenerics(input, { T: { kind: 'slice', item: 'u8' } }), ).toEqual(named('Envelope', [named('Option', [{ kind: 'slice', item: 'u8' }])])); }); - test('passes through unknown named refs that are not in substitutions', () => { + test('passes through named refs unchanged (named is a concrete user type, never a param)', () => { expect(resolver.substituteGenerics(named('Unknown'), {})).toEqual(named('Unknown')); + // Even if substitutions map has a matching name, a `named` decl is not substituted. + expect(resolver.substituteGenerics(named('Unknown'), { Unknown: 'u32' })).toEqual( + named('Unknown'), + ); }); - test('is a no-op on primitives and on inputs with no type_params', () => { + test('is a no-op on primitives and on inputs with no type params', () => { expect(resolver.substituteGenerics('u32')).toBe('u32'); expect(resolver.substituteGenerics({ kind: 'slice', item: 'u8' }, { T: 'u64' })).toEqual({ kind: 'slice', @@ -332,31 +336,31 @@ describe('type-resolver-v2 substituteGenerics', () => { }); test('is idempotent', () => { - const input: TypeDecl = named('Envelope', [named('T')]); + const input: TypeDecl = named('Envelope', [generic('T')]); const once = resolver.substituteGenerics(input, { T: 'u32' }); expect(resolver.substituteGenerics(once, { T: 'u32' })).toEqual(once); }); test('does not mutate the input tree', () => { - const input: TypeDecl = { kind: 'slice', item: named('T') }; + const input: TypeDecl = { kind: 'slice', item: generic('T') }; const snapshot = structuredClone(input); resolver.substituteGenerics(input, { T: 'u8' }); expect(input).toEqual(snapshot); }); test('resolves substitution chains (T -> U -> u32)', () => { - expect( - resolver.substituteGenerics(named('T'), { T: named('U'), U: 'u32' }), - ).toBe('u32'); + expect(resolver.substituteGenerics(generic('T'), { T: generic('U'), U: 'u32' })).toBe('u32'); }); test('throws on self-referential substitution map', () => { - expect(() => resolver.substituteGenerics(named('T'), { T: named('T') })).toThrow(/[Cc]yclic/); + expect(() => resolver.substituteGenerics(generic('T'), { T: generic('T') })).toThrow( + /[Cc]yclic/, + ); }); test('throws on cyclic substitution chain (T -> U -> T)', () => { expect(() => - resolver.substituteGenerics(named('T'), { T: named('U'), U: named('T') }), + resolver.substituteGenerics(generic('T'), { T: generic('U'), U: generic('T') }), ).toThrow(/[Cc]yclic/); }); @@ -379,7 +383,7 @@ describe('type-resolver-v2 resolveNamed', () => { type_params: [{ name: 'T' }], fields: [ { name: 'id', type: 'u32' }, - { name: 'payload', type: named('T') }, + { name: 'payload', type: generic('T') }, ], }; @@ -393,18 +397,18 @@ describe('type-resolver-v2 resolveNamed', () => { expect(resolver.resolveNamed(named('Envelope', ['u32']))).toBe(envelope); }); - test('returns undefined for primitives, slices, arrays, tuples', () => { + test('returns undefined for primitives, slices, arrays, tuples, type params', () => { const resolver = new TypeResolver([]); expect(resolver.resolveNamed('u32')).toBeUndefined(); expect(resolver.resolveNamed({ kind: 'slice', item: 'u8' })).toBeUndefined(); expect(resolver.resolveNamed({ kind: 'array', item: 'u8', len: 4 })).toBeUndefined(); expect(resolver.resolveNamed({ kind: 'tuple', types: ['u8', 'u16'] })).toBeUndefined(); + expect(resolver.resolveNamed(generic('T'))).toBeUndefined(); }); - test('returns undefined for unknown names and bare type_params', () => { + test('returns undefined for unknown names', () => { const resolver = new TypeResolver([]); expect(resolver.resolveNamed(named('Unknown'))).toBeUndefined(); - expect(resolver.resolveNamed(named('T'))).toBeUndefined(); }); }); @@ -416,8 +420,8 @@ describe('type-resolver-v2 genericsSubstitutions', () => { name: 'Pair', type_params: [{ name: 'T' }, { name: 'U' }], fields: [ - { name: 'left', type: named('T') }, - { name: 'right', type: named('U') }, + { name: 'left', type: generic('T') }, + { name: 'right', type: generic('U') }, ], }; expect(resolver.genericsSubstitutions(userType, ['u32', 'String'])).toEqual({ @@ -442,13 +446,15 @@ describe('type-resolver-v2 genericsSubstitutions', () => { kind: 'struct', name: 'One', type_params: [{ name: 'T' }], - fields: [{ name: 'v', type: named('T') }], + fields: [{ name: 'v', type: generic('T') }], }; expect(resolver.genericsSubstitutions(userType, ['u8', 'u16'])).toEqual({ T: 'u8' }); }); }); -describe('type-resolver-v2 ambient types', () => { +describe('type-resolver-v2 last-write-wins merge (shadowing via call-site merge)', () => { + // Callers wire ambient-vs-local shadowing by passing `[...ambient, ...local]` so locals + // come last and the constructor's last-write-wins loop registers the local shape. const ambientPacket: Type = { kind: 'struct', name: 'Packet', @@ -459,28 +465,16 @@ describe('type-resolver-v2 ambient types', () => { name: 'Packet', fields: [{ name: 'payload', type: { kind: 'array', item: 'u8', len: 8 } }], }; - const ambientOnly: Type = { - kind: 'struct', - name: 'AmbientOnly', - fields: [{ name: 'v', type: 'u32' }], - }; - - test('ambient types are resolvable when not shadowed', () => { - const resolver = new TypeResolver([], [ambientOnly]); - expect(resolver.resolveNamed(named('AmbientOnly'))).toBe(ambientOnly); - }); - test('service-local types shadow ambients on name collision', () => { - const resolver = new TypeResolver([localPacket], [ambientPacket]); + test('last-declared type wins on name collision', () => { + const resolver = new TypeResolver([ambientPacket, localPacket]); expect(resolver.resolveNamed(named('Packet'))).toBe(localPacket); }); - test('registry reflects shadowing (local wins)', () => { - const resolver = new TypeResolver([localPacket], [ambientPacket]); - // With localPacket (len 8), an 8-byte array round-trips; a 4-byte payload would fail. + test('registry reflects the last-declared shape', () => { + const resolver = new TypeResolver([ambientPacket, localPacket]); const encoded = resolver.registry.createType('Packet', { payload: [1, 2, 3, 4, 5, 6, 7, 8] }); expect(encoded.toJSON()).toEqual({ payload: '0x0102030405060708' }); - // Attempting to create with only 4 bytes should fail (ambient shape was overridden). expect(() => resolver.registry.createType('Packet', { payload: [1, 2, 3, 4] })).toThrow(); }); }); From 16091a1191f4dc512a03ca0c31b7d6cdf942fc86 Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Thu, 23 Apr 2026 18:40:33 +0400 Subject: [PATCH 04/10] test(js): add extends-chain ambient-type propagation coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Graph review surfaced SailsService.extends as a test gap — it propagates ambient types to child services at sails-idl-v2.ts:612, but no offline test exercised that path. Adds a parser-driven test: Child extends Base, program declares Shared, verify baseThroughExtends.typeResolver resolves Shared through the propagated ambient types. --- js/test/idl-v2-parser-type-resolver.test.ts | 39 +++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/js/test/idl-v2-parser-type-resolver.test.ts b/js/test/idl-v2-parser-type-resolver.test.ts index 6cdb36df4..aafb092ba 100644 --- a/js/test/idl-v2-parser-type-resolver.test.ts +++ b/js/test/idl-v2-parser-type-resolver.test.ts @@ -427,6 +427,45 @@ describe('sails v2 service-scoped type resolution', () => { expect(t?.name).toBe('Shared'); }); + test('extended services see program-level (ambient) types through the extends chain', () => { + const text = ` + !@sails: 1.0.0-beta.3 + + service Base@0x4071744d7e684110 { + functions { + Ping() -> u32; + } + } + + service Child@0x1f2c78d96df31861 { + extends { + Base@0x4071744d7e684110, + } + } + + program Test { + constructors { + Default(shared: Shared); + } + services { + Child@0x1f2c78d96df31861, + } + types { + struct Shared { + v: u32, + } + } + } + `; + const program = new SailsProgram(parser.parse(text)); + const child = program.services['Child']; + const baseThroughExtends = child.extends['Base']; + // Program-level Shared must be resolvable via the extended service's own resolver. + const t = baseThroughExtends.typeResolver.resolveNamed({ kind: 'named', name: 'Shared' }); + expect(t?.kind).toBe('struct'); + expect(t?.name).toBe('Shared'); + }); + test('generic substitution: Envelope<[u8]>.payload resolves to [u8]', () => { const text = ` !@sails: 1.0.0-beta.3 From b416c4f4a401f577fa55302704ef551c29e53b0b Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Fri, 24 Apr 2026 18:09:20 +0400 Subject: [PATCH 05/10] =?UTF-8?q?refactor(js):=20address=20#1323=20review?= =?UTF-8?q?=20=E2=80=94=20resolveNamed=20overload,=20rename=20resolveGener?= =?UTF-8?q?ics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename `substituteGenerics` → `resolveGenerics`. - Hide `genericsSubstitutions` (now `_genericsSubstitutions`). Consumers no longer manually zip `type_params` with a `TypeDecl.generics` list — that's resolver logic. - Extend `resolveNamed` with a second overload `(name: string, generics?: TypeDecl[])`. When concrete generics are provided (via either overload or via `typeDecl.generics`), the result is a concrete substituted `Type` with `type_params` stripped: `Envelope` → `{ kind: 'struct', fields: [{id, u32}, {payload, u32}] }`. Without generics, the raw user `Type` is returned unchanged. Per vobradovich feedback on https://github.com/gear-tech/sails/pull/1323. Co-Authored-By: Claude Opus 4.7 (1M context) --- js/src/type-resolver-idl-v2.ts | 125 ++++++++++++---- js/test/idl-v2-parser-type-resolver.test.ts | 14 +- js/test/idl-v2-type-resolver.test.ts | 153 ++++++++++++-------- 3 files changed, 194 insertions(+), 98 deletions(-) diff --git a/js/src/type-resolver-idl-v2.ts b/js/src/type-resolver-idl-v2.ts index 2442d2228..2f5b36af8 100644 --- a/js/src/type-resolver-idl-v2.ts +++ b/js/src/type-resolver-idl-v2.ts @@ -37,53 +37,79 @@ export class TypeResolver { } /** - * Resolve a `TypeDecl`'s named user type to its `Type` definition. + * Resolve a named user type to its `Type` definition. * - * Returns `undefined` for primitives, slices, arrays, tuples, type parameters - * (`{ kind: 'generic' }`), and unknown names. Does not recurse into generics — callers - * that want the substituted inner shape should pair this with {@link substituteGenerics}. + * Two call shapes: + * - `resolveNamed(typeDecl)` — pass a `TypeDecl`; returns the user type for a + * `{ kind: 'named', name, generics? }` decl, or `undefined` for primitives, slices, + * arrays, tuples, type parameters, and unknown names. + * - `resolveNamed(name, generics?)` — pass the user type's name directly, with an + * optional concrete generics list. * - * The returned `Type` is shared with the resolver's internal state — do not mutate it. + * When concrete generics are provided (either via the overload or via `typeDecl.generics`), + * the returned `Type` is a **concrete** substituted copy: every `{ kind: 'generic', name }` + * leaf is replaced according to the user type's declared `type_params`, and `type_params` + * is omitted from the result. When no generics are provided, the raw user `Type` is + * returned as-is (shared with the resolver's internal state — do not mutate). */ - resolveNamed(type: TypeDecl): Type | undefined { - if (typeof type === 'string') return undefined; - if (type.kind !== 'named') return undefined; - return this._userTypes[type.name]; + resolveNamed(type: TypeDecl): Type | undefined; + resolveNamed(name: string, generics?: TypeDecl[]): Type | undefined; + resolveNamed(typeOrName: TypeDecl | string, generics?: TypeDecl[]): Type | undefined { + let name: string; + let concrete: TypeDecl[] | undefined; + + if (typeof typeOrName === 'string') { + // String is ambiguous: it's either a primitive (`'u32'`) or a user-type name. + // Primitives aren't in `_userTypes`, so lookup naturally returns `undefined` for them. + name = typeOrName; + concrete = generics; + } else if (typeOrName.kind === 'named') { + name = typeOrName.name; + concrete = generics ?? typeOrName.generics; + } else { + return undefined; + } + + const userType = this._userTypes[name]; + if (!userType) return undefined; + if (!concrete?.length) return userType; + + const substitutions = this._genericsSubstitutions(userType, concrete); + return this._resolveTypeGenerics(userType, substitutions); } /** - * Recursively substitute type parameters through a `TypeDecl` tree. + * Recursively resolve type parameters through a `TypeDecl` tree. * - * Pure: does not mutate inputs. Idempotent: passing an already-substituted tree yields an + * Pure: does not mutate inputs. Idempotent: passing an already-resolved tree yields an * equivalent tree. Only `{ kind: 'generic', name: 'T' }` leaves whose `name` appears in * `substitutions` are replaced; wrapper shapes (`Option`, `Vec`, `Result`, custom - * generics, and any `named` decl) are preserved and their inner types substituted in place. + * generics, and any `named` decl) are preserved and their inner types resolved in place. * - * The function recurses through replacement chains (`{ T: U, U: u32 }` resolves `T` to `u32`). + * Recurses through replacement chains (`{ T: U, U: u32 }` resolves `T` to `u32`). * Cyclic maps (`{ T: { kind: 'generic', name: 'T' } }` or `{ T: U, U: T }`) are detected at - * runtime and cause an error to be thrown rather than an unbounded recursion. Maps produced by - * {@link genericsSubstitutions} from a parsed IDL cannot create cycles. + * runtime and throw rather than stack-overflowing. */ - substituteGenerics(type: TypeDecl, substitutions: Record = {}): TypeDecl { - return this._substituteGenerics(type, substitutions, new Set()); + resolveGenerics(type: TypeDecl, substitutions: Record = {}): TypeDecl { + return this._resolveGenerics(type, substitutions, new Set()); } - private _substituteGenerics( + private _resolveGenerics( type: TypeDecl, substitutions: Record, visited: Set, ): TypeDecl { if (typeof type === 'string') return type; if (type.kind === 'slice') { - const item = this._substituteGenerics(type.item, substitutions, visited); + const item = this._resolveGenerics(type.item, substitutions, visited); return item === type.item ? type : { kind: 'slice', item }; } if (type.kind === 'array') { - const item = this._substituteGenerics(type.item, substitutions, visited); + const item = this._resolveGenerics(type.item, substitutions, visited); return item === type.item ? type : { kind: 'array', item, len: type.len }; } if (type.kind === 'tuple') { - const next = type.types.map((t) => this._substituteGenerics(t, substitutions, visited)); + const next = type.types.map((t) => this._resolveGenerics(t, substitutions, visited)); return next.every((t, i) => t === type.types[i]) ? type : { kind: 'tuple', types: next }; } if (type.kind === 'generic') { @@ -99,11 +125,11 @@ export class TypeResolver { } const nextVisited = new Set(visited); nextVisited.add(type.name); - return this._substituteGenerics(replacement, substitutions, nextVisited); + return this._resolveGenerics(replacement, substitutions, nextVisited); } if (type.kind === 'named') { if (!type.generics?.length) return type; - const next = type.generics.map((g) => this._substituteGenerics(g, substitutions, visited)); + const next = type.generics.map((g) => this._resolveGenerics(g, substitutions, visited)); return next.every((g, i) => g === type.generics![i]) ? type : { kind: 'named', name: type.name, generics: next }; @@ -111,12 +137,10 @@ export class TypeResolver { throw new Error('Unknown TypeDecl kind :: ' + JSON.stringify(type)); } - /** - * Build a substitution map from a user type's declared `type_params` and a concrete - * generics list (typically the `generics` field of a `{ kind: 'named', generics: [...] }` - * `TypeDecl`). Missing positions are omitted. - */ - genericsSubstitutions(userType: Type, generics: TypeDecl[] = []): Record { + // Build a substitution map by zipping a user type's declared `type_params` with a concrete + // generics list. Internal: callers should use `resolveNamed(name, generics)` instead of + // building substitution maps by hand. + private _genericsSubstitutions(userType: Type, generics: TypeDecl[] = []): Record { const map: Record = {}; const params = userType.type_params ?? []; const len = Math.min(params.length, generics.length); @@ -126,6 +150,47 @@ export class TypeResolver { return map; } + // Return a `Type` with every `generic` leaf substituted and `type_params` stripped. The result + // is a shallow copy — fields/variants/targets are recursively resolved via `resolveGenerics`, + // which preserves unchanged subtrees by reference. + private _resolveTypeGenerics(type: Type, substitutions: Record): Type { + if (type.kind === 'struct') { + return { + kind: 'struct', + name: type.name, + docs: type.docs, + annotations: type.annotations, + fields: type.fields.map((f) => ({ + ...f, + type: this.resolveGenerics(f.type, substitutions), + })), + }; + } + if (type.kind === 'enum') { + return { + kind: 'enum', + name: type.name, + docs: type.docs, + annotations: type.annotations, + variants: type.variants.map((v) => ({ + ...v, + fields: v.fields.map((f) => ({ + ...f, + type: this.resolveGenerics(f.type, substitutions), + })), + })), + }; + } + // alias + return { + kind: 'alias', + name: type.name, + docs: type.docs, + annotations: type.annotations, + target: this.resolveGenerics(type.target, substitutions), + }; + } + /** * Convert a `TypeDecl` into a concrete string name, resolving generic parameters. * @@ -224,7 +289,7 @@ export class TypeResolver { .map((t: TypeDecl) => this.getTypeDeclString(t, generics, 'canonical')) .join('')}`; if (!this.registry.hasType(canonicalName)) { - const generics_map = this.genericsSubstitutions(userType, type.generics); + const generics_map = this._genericsSubstitutions(userType, type.generics); const typeDef = this.getTypeDef(userType, generics_map); /// When a user type with generics is resolved, the resolver constructs two names: // - genericName: readable, type-like syntax (example MyType>). diff --git a/js/test/idl-v2-parser-type-resolver.test.ts b/js/test/idl-v2-parser-type-resolver.test.ts index aafb092ba..249727454 100644 --- a/js/test/idl-v2-parser-type-resolver.test.ts +++ b/js/test/idl-v2-parser-type-resolver.test.ts @@ -490,22 +490,16 @@ describe('sails v2 service-scoped type resolution', () => { `; const program = new SailsProgram(parser.parse(text)); const service = program.services['Gen']; + // resolveNamed(name, generics) returns a concrete substituted Type. const envelope = service.typeResolver.resolveNamed({ kind: 'named', name: 'Envelope', generics: [{ kind: 'slice', item: 'u8' }], }); expect(envelope?.kind).toBe('struct'); - // Build substitution map from the concrete generics list on the arg. - const subs = service.typeResolver.genericsSubstitutions(envelope!, [ - { kind: 'slice', item: 'u8' }, - ]); - expect(subs).toEqual({ T: { kind: 'slice', item: 'u8' } }); - - // Walk the struct fields through substituteGenerics — payload's T becomes [u8]. + // type_params are stripped from the concrete result. + expect(envelope?.type_params).toBeUndefined(); const payloadField = (envelope as any).fields.find((f: any) => f.name === 'payload'); - expect(payloadField).toBeDefined(); - const resolvedPayloadType = service.typeResolver.substituteGenerics(payloadField.type, subs); - expect(resolvedPayloadType).toEqual({ kind: 'slice', item: 'u8' }); + expect(payloadField?.type).toEqual({ kind: 'slice', item: 'u8' }); }); }); diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index 6a28a5efe..98e0951e8 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -288,11 +288,11 @@ describe('type-resolver-v2 structs', () => { }); }); -describe('type-resolver-v2 substituteGenerics', () => { +describe('type-resolver-v2 resolveGenerics', () => { const resolver = new TypeResolver([]); test('replaces a type parameter leaf with a primitive', () => { - expect(resolver.substituteGenerics(generic('T'), { T: 'u32' })).toBe('u32'); + expect(resolver.resolveGenerics(generic('T'), { T: 'u32' })).toBe('u32'); }); test('recurses through slice / array / tuple', () => { @@ -303,7 +303,7 @@ describe('type-resolver-v2 substituteGenerics', () => { { kind: 'array', item: generic('T'), len: 4 }, ], }; - expect(resolver.substituteGenerics(input, { T: 'u8' })).toEqual({ + expect(resolver.resolveGenerics(input, { T: 'u8' })).toEqual({ kind: 'tuple', types: [ { kind: 'slice', item: 'u8' }, @@ -315,21 +315,21 @@ describe('type-resolver-v2 substituteGenerics', () => { test('recurses through named-with-generics (Option, custom wrappers)', () => { const input: TypeDecl = named('Envelope', [named('Option', [generic('T')])]); expect( - resolver.substituteGenerics(input, { T: { kind: 'slice', item: 'u8' } }), + resolver.resolveGenerics(input, { T: { kind: 'slice', item: 'u8' } }), ).toEqual(named('Envelope', [named('Option', [{ kind: 'slice', item: 'u8' }])])); }); test('passes through named refs unchanged (named is a concrete user type, never a param)', () => { - expect(resolver.substituteGenerics(named('Unknown'), {})).toEqual(named('Unknown')); + expect(resolver.resolveGenerics(named('Unknown'), {})).toEqual(named('Unknown')); // Even if substitutions map has a matching name, a `named` decl is not substituted. - expect(resolver.substituteGenerics(named('Unknown'), { Unknown: 'u32' })).toEqual( + expect(resolver.resolveGenerics(named('Unknown'), { Unknown: 'u32' })).toEqual( named('Unknown'), ); }); test('is a no-op on primitives and on inputs with no type params', () => { - expect(resolver.substituteGenerics('u32')).toBe('u32'); - expect(resolver.substituteGenerics({ kind: 'slice', item: 'u8' }, { T: 'u64' })).toEqual({ + expect(resolver.resolveGenerics('u32')).toBe('u32'); + expect(resolver.resolveGenerics({ kind: 'slice', item: 'u8' }, { T: 'u64' })).toEqual({ kind: 'slice', item: 'u8', }); @@ -337,37 +337,37 @@ describe('type-resolver-v2 substituteGenerics', () => { test('is idempotent', () => { const input: TypeDecl = named('Envelope', [generic('T')]); - const once = resolver.substituteGenerics(input, { T: 'u32' }); - expect(resolver.substituteGenerics(once, { T: 'u32' })).toEqual(once); + const once = resolver.resolveGenerics(input, { T: 'u32' }); + expect(resolver.resolveGenerics(once, { T: 'u32' })).toEqual(once); }); test('does not mutate the input tree', () => { const input: TypeDecl = { kind: 'slice', item: generic('T') }; const snapshot = structuredClone(input); - resolver.substituteGenerics(input, { T: 'u8' }); + resolver.resolveGenerics(input, { T: 'u8' }); expect(input).toEqual(snapshot); }); test('resolves substitution chains (T -> U -> u32)', () => { - expect(resolver.substituteGenerics(generic('T'), { T: generic('U'), U: 'u32' })).toBe('u32'); + expect(resolver.resolveGenerics(generic('T'), { T: generic('U'), U: 'u32' })).toBe('u32'); }); test('throws on self-referential substitution map', () => { - expect(() => resolver.substituteGenerics(generic('T'), { T: generic('T') })).toThrow( + expect(() => resolver.resolveGenerics(generic('T'), { T: generic('T') })).toThrow( /[Cc]yclic/, ); }); test('throws on cyclic substitution chain (T -> U -> T)', () => { expect(() => - resolver.substituteGenerics(generic('T'), { T: generic('U'), U: generic('T') }), + resolver.resolveGenerics(generic('T'), { T: generic('U'), U: generic('T') }), ).toThrow(/[Cc]yclic/); }); test('throws on unknown TypeDecl kind', () => { // A `Type` (kind: 'struct') is not a valid TypeDecl — catch the misuse loudly. const bogus = { kind: 'struct', name: 'X', fields: [] } as unknown as TypeDecl; - expect(() => resolver.substituteGenerics(bogus)).toThrow(/Unknown TypeDecl kind/); + expect(() => resolver.resolveGenerics(bogus)).toThrow(/Unknown TypeDecl kind/); }); }); @@ -386,15 +386,90 @@ describe('type-resolver-v2 resolveNamed', () => { { name: 'payload', type: generic('T') }, ], }; + const maybe: Type = { + kind: 'enum', + name: 'Maybe', + type_params: [{ name: 'T' }], + variants: [ + { name: 'None', fields: [] }, + { name: 'Some', fields: [{ type: generic('T') }] }, + ], + }; + const genericAlias: Type = { + kind: 'alias', + name: 'MaybeOpt', + type_params: [{ name: 'T' }], + target: named('Option', [generic('T')]), + }; - test('returns the user Type for a known named decl', () => { + test('returns the raw user Type for a known named decl with no generics', () => { const resolver = new TypeResolver([packet]); expect(resolver.resolveNamed(named('Packet'))).toBe(packet); }); - test('returns the user Type for a generic named decl (ignoring generics)', () => { + test('returns a concrete substituted Type when the named decl carries generics', () => { + const resolver = new TypeResolver([envelope]); + const result = resolver.resolveNamed(named('Envelope', ['u32'])); + expect(result).toEqual({ + kind: 'struct', + name: 'Envelope', + docs: undefined, + annotations: undefined, + fields: [ + { name: 'id', type: 'u32' }, + { name: 'payload', type: 'u32' }, + ], + }); + // type_params must be omitted on the concrete result. + expect(result?.type_params).toBeUndefined(); + }); + + test('substitutes generics across enum variants', () => { + const resolver = new TypeResolver([maybe]); + const result = resolver.resolveNamed(named('Maybe', [{ kind: 'slice', item: 'u8' }])); + expect(result?.kind).toBe('enum'); + const variants = (result as any).variants; + expect(variants[0]).toEqual({ name: 'None', fields: [] }); + expect(variants[1].fields[0].type).toEqual({ kind: 'slice', item: 'u8' }); + expect(result?.type_params).toBeUndefined(); + }); + + test('substitutes generics through alias targets', () => { + const resolver = new TypeResolver([genericAlias]); + const result = resolver.resolveNamed(named('MaybeOpt', ['u32'])); + expect(result).toEqual({ + kind: 'alias', + name: 'MaybeOpt', + docs: undefined, + annotations: undefined, + target: named('Option', ['u32']), + }); + }); + + test('string overload: by name returns the raw user Type', () => { + const resolver = new TypeResolver([packet]); + expect(resolver.resolveNamed('Packet')).toBe(packet); + }); + + test('string overload: name + concrete generics returns substituted Type', () => { + const resolver = new TypeResolver([envelope]); + const result = resolver.resolveNamed('Envelope', [{ kind: 'slice', item: 'u8' }]); + expect((result as any).fields[1].type).toEqual({ kind: 'slice', item: 'u8' }); + expect(result?.type_params).toBeUndefined(); + }); + + test('string overload prefers the explicit generics list over any on the decl', () => { + const resolver = new TypeResolver([envelope]); + // Caller has a name string, provides its own generics. + const result = resolver.resolveNamed('Envelope', ['u64']); + expect((result as any).fields[1].type).toBe('u64'); + }); + + test('explicit generics arg on TypeDecl overload overrides the decl.generics field', () => { const resolver = new TypeResolver([envelope]); - expect(resolver.resolveNamed(named('Envelope', ['u32']))).toBe(envelope); + // Pass generics as a separate argument — should win over typeDecl.generics. + const result = resolver.resolveNamed(named('Envelope', ['u32']), ['String']); + expect((result as any).fields[1].type).toBe('String'); }); test('returns undefined for primitives, slices, arrays, tuples, type params', () => { @@ -409,46 +484,8 @@ describe('type-resolver-v2 resolveNamed', () => { test('returns undefined for unknown names', () => { const resolver = new TypeResolver([]); expect(resolver.resolveNamed(named('Unknown'))).toBeUndefined(); - }); -}); - -describe('type-resolver-v2 genericsSubstitutions', () => { - test('zips type_params with concrete generics', () => { - const resolver = new TypeResolver([]); - const userType: Type = { - kind: 'struct', - name: 'Pair', - type_params: [{ name: 'T' }, { name: 'U' }], - fields: [ - { name: 'left', type: generic('T') }, - { name: 'right', type: generic('U') }, - ], - }; - expect(resolver.genericsSubstitutions(userType, ['u32', 'String'])).toEqual({ - T: 'u32', - U: 'String', - }); - }); - - test('returns empty map when userType has no type_params', () => { - const resolver = new TypeResolver([]); - const userType: Type = { - kind: 'struct', - name: 'Plain', - fields: [{ name: 'x', type: 'u32' }], - }; - expect(resolver.genericsSubstitutions(userType, ['u32'])).toEqual({}); - }); - - test('ignores extra concrete generics past the declared params', () => { - const resolver = new TypeResolver([]); - const userType: Type = { - kind: 'struct', - name: 'One', - type_params: [{ name: 'T' }], - fields: [{ name: 'v', type: generic('T') }], - }; - expect(resolver.genericsSubstitutions(userType, ['u8', 'u16'])).toEqual({ T: 'u8' }); + expect(resolver.resolveNamed('Unknown')).toBeUndefined(); + expect(resolver.resolveNamed('Unknown', ['u32'])).toBeUndefined(); }); }); From 6cbe4f910f4c1feaa94d82c41944be65f34f5a43 Mon Sep 17 00:00:00 2001 From: vobradovich Date: Tue, 5 May 2026 15:41:53 +0200 Subject: [PATCH 06/10] fix: remove overloaded `resolver.resolveNamed` call --- js/test/idl-v2-type-resolver.test.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index 98e0951e8..12128d047 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -465,13 +465,6 @@ describe('type-resolver-v2 resolveNamed', () => { expect((result as any).fields[1].type).toBe('u64'); }); - test('explicit generics arg on TypeDecl overload overrides the decl.generics field', () => { - const resolver = new TypeResolver([envelope]); - // Pass generics as a separate argument — should win over typeDecl.generics. - const result = resolver.resolveNamed(named('Envelope', ['u32']), ['String']); - expect((result as any).fields[1].type).toBe('String'); - }); - test('returns undefined for primitives, slices, arrays, tuples, type params', () => { const resolver = new TypeResolver([]); expect(resolver.resolveNamed('u32')).toBeUndefined(); From e00dca9c75ee2ab8fa3df6c6d31e1dc6b1c8660f Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Wed, 6 May 2026 19:47:00 +0400 Subject: [PATCH 07/10] refactor(js): drop program.types ambient scope from service resolvers Aligns with the self-sufficient service IDL contract being formalized in #1341: a service IDL is independently distributable and resolves its function/event/throws payload types from its own `types` block plus the types of every service it explicitly extends, transitively. Program-level `types` are scoped to program/ctor declarations and are no longer threaded into per-service resolvers. - SailsService: remove `ambientTypes` constructor param + `_ambientTypes` field. Resolver is now seeded by `_collectServiceScopeTypes`, a module- level helper that walks the `extends` chain depth-first with cycle detection (mirrors `_substituteGenerics`'s visited-set pattern). - SailsProgram: stop passing `program.types` into services. Rewrite `resolveInService` to return only hits from the merged service-local + extends-chain scope. Drop `_programTypeIndex`; the lazy index is now per-service `Map` with extends pre-merged. - Tests: flip the two ambient-themed assertions (now require `resolveInService(...)` and `registry.hasType(...)` to fail for a program-only type, matching #1341's parallel JS tests). Rewrite the extends-chain test to assert base-service types instead. Add a sibling- bleed test. Reframe the call-site shadowing block as a generic last-write-wins test. - TypeResolver constructor JSDoc: drop the ambient/local framing in favour of a generic last-write-wins description. Closes #1317. Co-Authored-By: Claude Opus 4.7 (1M context) --- js/src/sails-idl-v2.ts | 91 +++++++++++++++------ js/src/type-resolver-idl-v2.ts | 5 +- js/test/idl-v2-parser-type-resolver.test.ts | 63 +++++++++----- js/test/idl-v2-type-resolver.test.ts | 17 ++-- 4 files changed, 122 insertions(+), 54 deletions(-) diff --git a/js/src/sails-idl-v2.ts b/js/src/sails-idl-v2.ts index 2cca50024..ab93378a9 100644 --- a/js/src/sails-idl-v2.ts +++ b/js/src/sails-idl-v2.ts @@ -99,6 +99,46 @@ const _getArgsForTxBuilder = (args: any[], params: ISailsFuncArg[]) => { return args.slice(0, params.length); }; +/** + * Collect every `Type` in scope for a service per the self-sufficient service IDL + * contract: depth-first across the `extends` chain (with cycle detection), then the + * service's own `types` last so locals shadow base definitions on name collision. + * + * Throws on a cyclic `extends` graph (`A → B → A`), reporting the chain. + * Used by both `SailsService` (to seed its `TypeResolver`) and `SailsProgram` + * (to build `resolveInService`'s lazy index). + */ +const _collectServiceScopeTypes = ( + service: IServiceUnit, + resolveServiceUnit?: (ident: IServiceIdent) => IServiceUnit | undefined, +): Type[] => { + const out: Type[] = []; + const walk = (unit: IServiceUnit, visited: Set) => { + if (visited.has(unit.name)) { + throw new Error( + `Cyclic service-extends chain detected at "${unit.name}" — chain: ` + + `${[...visited, unit.name].join(' → ')}`, + ); + } + const nextVisited = new Set(visited); + nextVisited.add(unit.name); + + if (resolveServiceUnit && unit.extends?.length) { + for (const ident of unit.extends as IServiceIdent[]) { + const baseUnit = resolveServiceUnit(ident); + if (!baseUnit) { + throw new Error(`Service definition for "${ident.name}" not found in IDL`); + } + walk(baseUnit, nextVisited); + } + } + + for (const t of unit.types ?? []) out.push(t); + }; + walk(service, new Set()); + return out; +}; + const _assertMatchingHeader = ( payload: Uint8Array | HexString, expected: SailsMessageHeader, @@ -127,10 +167,10 @@ export class SailsProgram { private _api?: GearApi; private _programId?: HexString; private _services: Map; - // Lazy indices for resolveInService. Populated on first call; not invalidated because - // `_doc` is immutable after parse. + // Lazy index for resolveInService: per-service `Map`, pre-merged + // with the service's transitive extends chain. Populated on first call; not + // invalidated because `_doc` is immutable after parse. private _serviceTypeIndex?: Map>; - private _programTypeIndex?: Map; private _resolveServiceUnit = (ident: IServiceIdent): IServiceUnit | undefined => { if (!ident.interface_id) { throw new Error(`Service "${ident.name}" is missing interface_id in IDL`); @@ -220,7 +260,6 @@ export class SailsProgram { this._programId, expo.route_idx, this._resolveServiceUnit, - program.types ?? [], ); } return services; @@ -228,9 +267,14 @@ export class SailsProgram { /** * Resolve a `TypeDecl` to its user-type definition in the scope of a named service. - * Service-local types shadow program-level types; program-level types are visible to - * every service. Returns `undefined` when the service doesn't exist, or the `TypeDecl` - * isn't a named user type. + * + * Per the self-sufficient service IDL contract (see `docs/idl-v2-spec.md`), scope is + * the service's own `types` plus the types of every service it extends, transitively. + * Service-local definitions shadow base-service definitions on name collision. + * Program-level `types` are NOT in scope — they belong to program/ctor declarations. + * + * Returns `undefined` when the service doesn't exist, the `TypeDecl` isn't a named + * user type, or the name is not declared in the service's transitive scope. * * Backed by a lazy `Map`-based index — O(1) per call after the first invocation. * Safe to call in tight loops while walking large IDL trees. @@ -238,23 +282,26 @@ export class SailsProgram { resolveInService(serviceName: string, typeDecl: TypeDecl): Type | undefined { if (typeof typeDecl === 'string' || typeDecl.kind !== 'named') return undefined; if (!this._serviceTypeIndex) this._buildTypeIndex(); - return ( - this._serviceTypeIndex!.get(serviceName)?.get(typeDecl.name) ?? - this._programTypeIndex!.get(typeDecl.name) - ); + return this._serviceTypeIndex!.get(serviceName)?.get(typeDecl.name); } private _buildTypeIndex(): void { const serviceIndex = new Map>(); + const unitByName = new Map(); + for (const unit of this._doc.services ?? []) unitByName.set(unit.name, unit); + + const lookupBase = (ident: IServiceIdent): IServiceUnit | undefined => unitByName.get(ident.name); + for (const unit of this._doc.services ?? []) { - const local = new Map(); - for (const t of unit.types ?? []) local.set(t.name, t); - serviceIndex.set(unit.name, local); + const merged = new Map(); + // Walk extends-chain depth-first; service-local types come last so they win + // on collision with base types (Map.set is last-write-wins). + for (const t of _collectServiceScopeTypes(unit, lookupBase)) { + merged.set(t.name, t); + } + serviceIndex.set(unit.name, merged); } - const programIndex = new Map(); - for (const t of this._doc.program?.types ?? []) programIndex.set(t.name, t); this._serviceTypeIndex = serviceIndex; - this._programTypeIndex = programIndex; } /** #### Constructor functions with arguments from the parsed IDL */ @@ -369,7 +416,6 @@ export class SailsService implements ISailsService { private _typeResolver: TypeResolver; private _api?: GearApi; private _routeIdx: number; - private _ambientTypes: Type[] = []; private _resolveServiceUnit?: (ident: IServiceIdent) => IServiceUnit | undefined; @@ -379,15 +425,15 @@ export class SailsService implements ISailsService { programId?: HexString, routeIdx = 0, resolveServiceUnit?: (ident: IServiceIdent) => IServiceUnit | undefined, - ambientTypes: Type[] = [], ) { this._service = service; this._api = api; this._programId = programId; this._resolveServiceUnit = resolveServiceUnit; - // Ambient types come first so service-local types shadow them on name collision. - this._typeResolver = new TypeResolver([...ambientTypes, ...(service.types ?? [])]); - this._ambientTypes = ambientTypes; + // Service IDL is self-contained: types in scope are this service's own `types` + // plus the types of every service it extends, transitively. Base types come first + // so service-local definitions shadow them on name collision. + this._typeResolver = new TypeResolver(_collectServiceScopeTypes(service, resolveServiceUnit)); this._routeIdx = routeIdx; this.events = this._getEvents(service); @@ -610,7 +656,6 @@ export class SailsService implements ISailsService { this._programId, this.routeIdx, this._resolveServiceUnit, - this._ambientTypes, ); } diff --git a/js/src/type-resolver-idl-v2.ts b/js/src/type-resolver-idl-v2.ts index 2f5b36af8..8dbb5c897 100644 --- a/js/src/type-resolver-idl-v2.ts +++ b/js/src/type-resolver-idl-v2.ts @@ -21,8 +21,9 @@ export class TypeResolver { const scaleTypes: Record = {}; const userTypes: Record = {}; - // Iteration order is last-write-wins: callers that want ambient-type shadowing should - // pass `[...ambientTypes, ...localTypes]` so locals overwrite on name collision. + // Iteration order is last-write-wins: when two entries share a name, the later one + // overrides the earlier. Callers that want a "shadowing" merge pass already-merged + // arrays in the desired override order (e.g. base-service types first, locals last). for (const type of types) { userTypes[type.name] = type; } diff --git a/js/test/idl-v2-parser-type-resolver.test.ts b/js/test/idl-v2-parser-type-resolver.test.ts index 249727454..6246f95ad 100644 --- a/js/test/idl-v2-parser-type-resolver.test.ts +++ b/js/test/idl-v2-parser-type-resolver.test.ts @@ -395,9 +395,10 @@ describe('sails v2 service-scoped type resolution', () => { expect(program.resolveInService('Nonexistent', { kind: 'named', name: 'Packet' })).toBeUndefined(); }); - test('program-level (ambient) types are visible inside service resolvers', () => { - // Program-level `Shared` is referenced by the ctor (parser rejects it in service signatures) - // but must still resolve through the service's resolver for consumers walking ctor args. + test('service resolver does NOT see program-level types (self-sufficient service IDL)', () => { + // Per the self-sufficient service IDL contract (docs/idl-v2-spec.md), program.types + // are scoped to program/ctor declarations and are NOT ambient for services. A service + // IDL must be resolvable from its own `types` plus its extends chain. const text = ` !@sails: 1.0.0-beta.3 @@ -422,48 +423,68 @@ describe('sails v2 service-scoped type resolution', () => { } `; const program = new SailsProgram(parser.parse(text)); - const t = program.resolveInService('A', { kind: 'named', name: 'Shared' }); - expect(t?.kind).toBe('struct'); - expect(t?.name).toBe('Shared'); + expect(program.resolveInService('A', { kind: 'named', name: 'Shared' })).toBeUndefined(); + // The service's own resolver must not register the program-level type either. + expect(program.services['A'].registry.hasType('Shared')).toBe(false); }); - test('extended services see program-level (ambient) types through the extends chain', () => { + test('extended services see base service types through the extends chain', () => { + // Per the spec: a service resolves types from its own `types` plus from + // explicitly extended service interfaces. Here `Child` has no `Shared` of its own, + // but its base `Base` does — and the extension must surface it. const text = ` !@sails: 1.0.0-beta.3 - service Base@0x4071744d7e684110 { + service Base@0xb45ddc41cf66e2da { functions { - Ping() -> u32; + GetShared() -> Shared; + } + types { + struct Shared { + v: u32, + } } } service Child@0x1f2c78d96df31861 { extends { - Base@0x4071744d7e684110, + Base@0xb45ddc41cf66e2da, } } program Test { constructors { - Default(shared: Shared); + Default(); } services { Child@0x1f2c78d96df31861, } - types { - struct Shared { - v: u32, - } - } } `; const program = new SailsProgram(parser.parse(text)); + // Direct lookup: extends-merged scope is reachable through resolveInService. + const fromProgram = program.resolveInService('Child', { kind: 'named', name: 'Shared' }); + expect(fromProgram?.kind).toBe('struct'); + expect(fromProgram?.name).toBe('Shared'); + + // The extender's own TypeResolver also has the base's type folded in. const child = program.services['Child']; - const baseThroughExtends = child.extends['Base']; - // Program-level Shared must be resolvable via the extended service's own resolver. - const t = baseThroughExtends.typeResolver.resolveNamed({ kind: 'named', name: 'Shared' }); - expect(t?.kind).toBe('struct'); - expect(t?.name).toBe('Shared'); + const fromResolver = child.typeResolver.resolveNamed({ kind: 'named', name: 'Shared' }); + expect(fromResolver?.kind).toBe('struct'); + expect(fromResolver?.name).toBe('Shared'); + }); + + test('sibling services do not bleed types via SailsProgram.resolveInService', () => { + // A and B both declare `Packet` with different shapes. resolveInService must + // return each service's own definition, never the sibling's. + const program = new SailsProgram(parser.parse(TWO_SERVICES_SAME_NAME)); + const a = program.resolveInService('A', { kind: 'named', name: 'Packet' }) as any; + const b = program.resolveInService('B', { kind: 'named', name: 'Packet' }) as any; + expect(a.fields[0].type).toEqual({ kind: 'array', item: 'u8', len: 4 }); + expect(b.fields[0].type).toEqual({ kind: 'array', item: 'u8', len: 8 }); + // And neither service should see a same-named type that exists only on the other. + // (Already proven by the differing shapes above; this is a redundant invariant.) + expect(a.fields[0].type).not.toEqual(b.fields[0].type); }); test('generic substitution: Envelope<[u8]>.payload resolves to [u8]', () => { diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index 12128d047..bf5ca2703 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -482,27 +482,28 @@ describe('type-resolver-v2 resolveNamed', () => { }); }); -describe('type-resolver-v2 last-write-wins merge (shadowing via call-site merge)', () => { - // Callers wire ambient-vs-local shadowing by passing `[...ambient, ...local]` so locals - // come last and the constructor's last-write-wins loop registers the local shape. - const ambientPacket: Type = { +describe('type-resolver-v2 last-write-wins merge', () => { + // The constructor takes a single pre-merged Type[]; later entries override earlier + // ones on name collision. Callers compose the input array in the desired override + // order (e.g. base-service types first, service-local last for IDL extends-merging). + const firstPacket: Type = { kind: 'struct', name: 'Packet', fields: [{ name: 'payload', type: { kind: 'array', item: 'u8', len: 4 } }], }; - const localPacket: Type = { + const secondPacket: Type = { kind: 'struct', name: 'Packet', fields: [{ name: 'payload', type: { kind: 'array', item: 'u8', len: 8 } }], }; test('last-declared type wins on name collision', () => { - const resolver = new TypeResolver([ambientPacket, localPacket]); - expect(resolver.resolveNamed(named('Packet'))).toBe(localPacket); + const resolver = new TypeResolver([firstPacket, secondPacket]); + expect(resolver.resolveNamed(named('Packet'))).toBe(secondPacket); }); test('registry reflects the last-declared shape', () => { - const resolver = new TypeResolver([ambientPacket, localPacket]); + const resolver = new TypeResolver([firstPacket, secondPacket]); const encoded = resolver.registry.createType('Packet', { payload: [1, 2, 3, 4, 5, 6, 7, 8] }); expect(encoded.toJSON()).toEqual({ payload: '0x0102030405060708' }); expect(() => resolver.registry.createType('Packet', { payload: [1, 2, 3, 4] })).toThrow(); From cdf7215ec197d4bb52889dd38d695813231b9d17 Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Wed, 6 May 2026 20:06:33 +0400 Subject: [PATCH 08/10] test(js): cycle-guard tests + simplify-pass cleanups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add SailsService extends-chain cycle-guard tests: self-cycle (`A → A`) and mutual cycle (`A → B → A`) throw with the chain reported; diamond inheritance (A → [B, C], B → C) does not throw. - Trim two over-explained inline comments in `_collectServiceScopeTypes` call sites flagged by /simplify quality review. - Make the extends-chain test fixture parser-agnostic: Base now exposes `Ping() -> u32` instead of `GetShared() -> Shared`. Keeps Base.types populated as the merge target without referencing extender-resolved types, so the IDL parses cleanly under both current master and the tightened validator in #1341. Co-Authored-By: Claude Opus 4.7 (1M context) --- js/src/sails-idl-v2.ts | 7 +--- js/test/idl-v2-parser-type-resolver.test.ts | 2 +- js/test/idl-v2-type-resolver.test.ts | 45 ++++++++++++++++++++- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/js/src/sails-idl-v2.ts b/js/src/sails-idl-v2.ts index ab93378a9..7381714ed 100644 --- a/js/src/sails-idl-v2.ts +++ b/js/src/sails-idl-v2.ts @@ -294,8 +294,7 @@ export class SailsProgram { for (const unit of this._doc.services ?? []) { const merged = new Map(); - // Walk extends-chain depth-first; service-local types come last so they win - // on collision with base types (Map.set is last-write-wins). + // Locals come last so they win on Map.set last-write-wins. for (const t of _collectServiceScopeTypes(unit, lookupBase)) { merged.set(t.name, t); } @@ -430,9 +429,7 @@ export class SailsService implements ISailsService { this._api = api; this._programId = programId; this._resolveServiceUnit = resolveServiceUnit; - // Service IDL is self-contained: types in scope are this service's own `types` - // plus the types of every service it extends, transitively. Base types come first - // so service-local definitions shadow them on name collision. + // Self-contained scope: bases first so locals shadow on collision. this._typeResolver = new TypeResolver(_collectServiceScopeTypes(service, resolveServiceUnit)); this._routeIdx = routeIdx; diff --git a/js/test/idl-v2-parser-type-resolver.test.ts b/js/test/idl-v2-parser-type-resolver.test.ts index 6246f95ad..5766f812b 100644 --- a/js/test/idl-v2-parser-type-resolver.test.ts +++ b/js/test/idl-v2-parser-type-resolver.test.ts @@ -437,7 +437,7 @@ describe('sails v2 service-scoped type resolution', () => { service Base@0xb45ddc41cf66e2da { functions { - GetShared() -> Shared; + Ping() -> u32; } types { struct Shared { diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index bf5ca2703..595e7d911 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -1,6 +1,7 @@ import { TypeRegistry } from '@polkadot/types/create'; -import type { Type, TypeDecl } from 'sails-js-types'; +import type { IServiceIdent, IServiceUnit, Type, TypeDecl } from 'sails-js-types'; +import { SailsService } from '../src/sails-idl-v2.js'; import { TypeResolver } from '../src/type-resolver-idl-v2.js'; const named = (name: string, generics?: TypeDecl[]): TypeDecl => ({ @@ -566,3 +567,45 @@ describe('type-resolver-v2 aliases', () => { expect(encodedNull.toJSON()).toBe(null); }); }); + +describe('SailsService extends-chain cycle detection', () => { + // _collectServiceScopeTypes (module-private) walks the extends graph depth-first + // with a visited-set guard. Pathological IDLs with cyclic extends graphs + // (`A extends B`, `B extends A`) must throw with the chain in the message rather + // than stack-overflow. The parser would normally reject these, but hand-built or + // wire-sourced ASTs can still produce them — the runtime guard is the safety net. + const ident = (name: string): IServiceIdent => ({ name, interface_id: undefined }); + const unit = (name: string, extendsList: IServiceIdent[]): IServiceUnit => ({ + name, + extends: extendsList, + }); + + test('throws on self-cycle (A extends A)', () => { + const a = unit('A', [ident('A')]); + const lookup = (i: IServiceIdent): IServiceUnit | undefined => (i.name === 'A' ? a : undefined); + expect(() => new SailsService(a, undefined, undefined, 0, lookup)).toThrow(/Cyclic service-extends chain detected at "A".*A → A/s); + }); + + test('throws on mutual cycle (A extends B, B extends A)', () => { + const a = unit('A', [ident('B')]); + const b = unit('B', [ident('A')]); + const lookup = (i: IServiceIdent): IServiceUnit | undefined => + i.name === 'A' ? a : i.name === 'B' ? b : undefined; + // Walk starts from A → visits B → tries to revisit A. Chain is reported. + expect(() => new SailsService(a, undefined, undefined, 0, lookup)).toThrow(/Cyclic service-extends chain detected at "A".*A → B → A/s); + }); + + test('does not throw when the same service appears as a sibling base twice (diamond)', () => { + // A extends [B, C]; B extends C; C is a leaf. Not a cycle — `C` is reachable + // from A through two paths, but the visited-set is per-recursion, not global. + const c: IServiceUnit = { + name: 'C', + types: [{ kind: 'struct', name: 'Leaf', fields: [{ name: 'v', type: 'u32' }] }], + }; + const b: IServiceUnit = { name: 'B', extends: [ident('C')] }; + const a: IServiceUnit = { name: 'A', extends: [ident('B'), ident('C')] }; + const lookup = (i: IServiceIdent): IServiceUnit | undefined => + i.name === 'A' ? a : i.name === 'B' ? b : i.name === 'C' ? c : undefined; + expect(() => new SailsService(a, undefined, undefined, 0, lookup)).not.toThrow(); + }); +}); From 7e08898f5fac8e015e5670631d6e9b1af3b1cb2e Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Wed, 6 May 2026 20:10:46 +0400 Subject: [PATCH 09/10] fix(js,test): satisfy eslint unicorn rules in cycle-guard tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI lint flagged 4 errors from unicorn: - consistent-function-scoping: ident/unit helpers were inside the describe block; lifted them to module scope as mockIdent/mockUnit. - no-nested-ternary: replaced the inline `i.name === 'A' ? a : ...` ternary chains in the lookup closures with a tiny `lookupFromMap` helper that takes a `Record`. Each test now builds its name → unit map explicitly. Tests are otherwise unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- js/test/idl-v2-type-resolver.test.ts | 43 +++++++++++++++++----------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index 595e7d911..210a5339a 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -568,31 +568,41 @@ describe('type-resolver-v2 aliases', () => { }); }); +const mockIdent = (name: string): IServiceIdent => ({ name, interface_id: undefined }); + +const mockUnit = (name: string, extendsList: IServiceIdent[]): IServiceUnit => ({ + name, + extends: extendsList, +}); + +const lookupFromMap = + (units: Record) => + (i: IServiceIdent): IServiceUnit | undefined => + units[i.name]; + describe('SailsService extends-chain cycle detection', () => { // _collectServiceScopeTypes (module-private) walks the extends graph depth-first // with a visited-set guard. Pathological IDLs with cyclic extends graphs // (`A extends B`, `B extends A`) must throw with the chain in the message rather // than stack-overflow. The parser would normally reject these, but hand-built or // wire-sourced ASTs can still produce them — the runtime guard is the safety net. - const ident = (name: string): IServiceIdent => ({ name, interface_id: undefined }); - const unit = (name: string, extendsList: IServiceIdent[]): IServiceUnit => ({ - name, - extends: extendsList, - }); test('throws on self-cycle (A extends A)', () => { - const a = unit('A', [ident('A')]); - const lookup = (i: IServiceIdent): IServiceUnit | undefined => (i.name === 'A' ? a : undefined); - expect(() => new SailsService(a, undefined, undefined, 0, lookup)).toThrow(/Cyclic service-extends chain detected at "A".*A → A/s); + const a = mockUnit('A', [mockIdent('A')]); + const lookup = lookupFromMap({ A: a }); + expect(() => new SailsService(a, undefined, undefined, 0, lookup)).toThrow( + /Cyclic service-extends chain detected at "A".*A → A/s, + ); }); test('throws on mutual cycle (A extends B, B extends A)', () => { - const a = unit('A', [ident('B')]); - const b = unit('B', [ident('A')]); - const lookup = (i: IServiceIdent): IServiceUnit | undefined => - i.name === 'A' ? a : i.name === 'B' ? b : undefined; + const a = mockUnit('A', [mockIdent('B')]); + const b = mockUnit('B', [mockIdent('A')]); + const lookup = lookupFromMap({ A: a, B: b }); // Walk starts from A → visits B → tries to revisit A. Chain is reported. - expect(() => new SailsService(a, undefined, undefined, 0, lookup)).toThrow(/Cyclic service-extends chain detected at "A".*A → B → A/s); + expect(() => new SailsService(a, undefined, undefined, 0, lookup)).toThrow( + /Cyclic service-extends chain detected at "A".*A → B → A/s, + ); }); test('does not throw when the same service appears as a sibling base twice (diamond)', () => { @@ -602,10 +612,9 @@ describe('SailsService extends-chain cycle detection', () => { name: 'C', types: [{ kind: 'struct', name: 'Leaf', fields: [{ name: 'v', type: 'u32' }] }], }; - const b: IServiceUnit = { name: 'B', extends: [ident('C')] }; - const a: IServiceUnit = { name: 'A', extends: [ident('B'), ident('C')] }; - const lookup = (i: IServiceIdent): IServiceUnit | undefined => - i.name === 'A' ? a : i.name === 'B' ? b : i.name === 'C' ? c : undefined; + const b: IServiceUnit = { name: 'B', extends: [mockIdent('C')] }; + const a: IServiceUnit = { name: 'A', extends: [mockIdent('B'), mockIdent('C')] }; + const lookup = lookupFromMap({ A: a, B: b, C: c }); expect(() => new SailsService(a, undefined, undefined, 0, lookup)).not.toThrow(); }); }); From e36dce185c70d6dd62469bacb127ae06a1a2e6e0 Mon Sep 17 00:00:00 2001 From: Vadim Smirnov Date: Wed, 6 May 2026 20:34:56 +0400 Subject: [PATCH 10/10] fix(js,test): unblock CI test failures from prior cycle-guard commit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two tests broke after the simplify-pass cleanups landed: 1. Cycle-guard "diamond" test — `SailsService.constructor` calls `_getEvents` which unconditionally invokes `InterfaceId.from(service.interface_id)` at line 585 (before iterating events). The mock units left `interface_id` undefined, so `InterfaceId.fromBytes(undefined)` threw TypeError. Fixed by giving mocks an 8-byte zero placeholder (`ZERO_IFACE`) and explicit empty `funcs`/`events` arrays. Self-cycle and mutual-cycle tests were unaffected because `_collectServiceScopeTypes` throws inside the resolver init, before `_getEvents` runs. 2. Extends-chain integration test — fixture pinned `Base@0xb45ddc41cf66e2da` but the parser computes interface_ids from interface contents (per `docs/interface-id-spec.md`), and the prior fixture rewrite (`GetShared() -> Shared` → `Ping() -> u32`) changed the content hash. Following the convention used by #1341's added tests, drop explicit `@0x...` annotations and let the parser compute them. Co-Authored-By: Claude Opus 4.7 (1M context) --- js/test/idl-v2-parser-type-resolver.test.ts | 8 ++++---- js/test/idl-v2-type-resolver.test.ts | 17 ++++++++++++++--- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/js/test/idl-v2-parser-type-resolver.test.ts b/js/test/idl-v2-parser-type-resolver.test.ts index 5766f812b..20bd7994c 100644 --- a/js/test/idl-v2-parser-type-resolver.test.ts +++ b/js/test/idl-v2-parser-type-resolver.test.ts @@ -435,7 +435,7 @@ describe('sails v2 service-scoped type resolution', () => { const text = ` !@sails: 1.0.0-beta.3 - service Base@0xb45ddc41cf66e2da { + service Base { functions { Ping() -> u32; } @@ -446,9 +446,9 @@ describe('sails v2 service-scoped type resolution', () => { } } - service Child@0x1f2c78d96df31861 { + service Child { extends { - Base@0xb45ddc41cf66e2da, + Base, } } @@ -457,7 +457,7 @@ describe('sails v2 service-scoped type resolution', () => { Default(); } services { - Child@0x1f2c78d96df31861, + Child, } } `; diff --git a/js/test/idl-v2-type-resolver.test.ts b/js/test/idl-v2-type-resolver.test.ts index 210a5339a..27852c6b9 100644 --- a/js/test/idl-v2-type-resolver.test.ts +++ b/js/test/idl-v2-type-resolver.test.ts @@ -568,11 +568,19 @@ describe('type-resolver-v2 aliases', () => { }); }); -const mockIdent = (name: string): IServiceIdent => ({ name, interface_id: undefined }); +// Minimal mock service-units for direct-construction tests. The SailsService +// constructor reads `interface_id` via `InterfaceId.from(...)` in `_getEvents` +// regardless of whether events exist, so we pass an 8-byte zero placeholder. +const ZERO_IFACE = new Uint8Array(8); + +const mockIdent = (name: string): IServiceIdent => ({ name, interface_id: ZERO_IFACE }); const mockUnit = (name: string, extendsList: IServiceIdent[]): IServiceUnit => ({ name, + interface_id: ZERO_IFACE, extends: extendsList, + funcs: [], + events: [], }); const lookupFromMap = @@ -610,10 +618,13 @@ describe('SailsService extends-chain cycle detection', () => { // from A through two paths, but the visited-set is per-recursion, not global. const c: IServiceUnit = { name: 'C', + interface_id: ZERO_IFACE, + funcs: [], + events: [], types: [{ kind: 'struct', name: 'Leaf', fields: [{ name: 'v', type: 'u32' }] }], }; - const b: IServiceUnit = { name: 'B', extends: [mockIdent('C')] }; - const a: IServiceUnit = { name: 'A', extends: [mockIdent('B'), mockIdent('C')] }; + const b: IServiceUnit = { ...mockUnit('B', [mockIdent('C')]) }; + const a: IServiceUnit = { ...mockUnit('A', [mockIdent('B'), mockIdent('C')]) }; const lookup = lookupFromMap({ A: a, B: b, C: c }); expect(() => new SailsService(a, undefined, undefined, 0, lookup)).not.toThrow(); });