Skip to content

Commit eeb84f5

Browse files
Omar Qureshiclaude
andcommitted
refactor(ruby): replace pervasive any with jsii-reflect/spec types
The generator passed reflect objects around as `any` throughout (typeSpec, members, parameters, type references), which is how bugs like the derived-fqn statics issue survived review — typed members would have failed compilation. - emitClassType/emitInterfaceType/emitEnumType take reflect.ClassType/ InterfaceType/EnumType; the topo sort works on reflect.Type and leans on the API's `this is X` narrowing predicates. - Dual-shape boundaries get named types: RubyTypeRef (reflect.TypeReference | spec.TypeReference, normalized by typeRefSpec via instanceof), ParamLike (reflect.Parameter, spec.Parameter and reflect.Property used as ctor kwargs), and MemberLike (the collision passes' structural requirement, with `deprecated?: unknown` covering the reflect-boolean/spec-string duality). - dedupCrossCategory is generic over both member kinds; emitDocs opts are fully typed (spec.OptionalValue returns, readonly ParamLike[]). Two `any`s remain by design, each with a comment: rubyJsonLiteral (arbitrary JSON-able input) and rawDocs (genuinely dual-shape). No behavior change: regenerated jsii-calc bindings are byte-identical; pacmak tests 18 passing; full suite 240 examples, 0 failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 28452c6 commit eeb84f5

1 file changed

Lines changed: 105 additions & 54 deletions

File tree

  • packages/jsii-pacmak/lib/targets

packages/jsii-pacmak/lib/targets/ruby.ts

Lines changed: 105 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as spec from '@jsii/spec';
22
import { toSnakeCase, toPascalCase } from 'codemaker';
33
import * as fs from 'fs-extra';
4+
import * as reflect from 'jsii-reflect';
45
import * as path from 'path';
56

67
import { Generator, Legalese } from '../generator';
@@ -97,6 +98,37 @@ function isDeprecated(member: { docs?: { deprecated?: unknown } }): boolean {
9798
return !!member.docs?.deprecated;
9899
}
99100

101+
/**
102+
* A type reference in either of the two shapes this generator handles:
103+
* jsii-reflect wrappers (members coming off `allProperties`/`allMethods`)
104+
* or raw spec objects (initializer parameters). Normalize with
105+
* {@link RubyGenerator.typeRefSpec} before introspecting.
106+
*/
107+
type RubyTypeRef = reflect.TypeReference | spec.TypeReference;
108+
109+
/**
110+
* Minimal structural shape of a documentable, typed parameter — satisfied
111+
* by `reflect.Parameter`, raw `spec.Parameter`, and `reflect.Property`
112+
* (struct members doubling as constructor keyword arguments).
113+
*/
114+
interface ParamLike {
115+
readonly name: string;
116+
readonly type?: RubyTypeRef;
117+
readonly optional?: boolean;
118+
readonly variadic?: boolean;
119+
}
120+
121+
/**
122+
* Minimal structural shape required by the member-collision passes —
123+
* satisfied by reflect members (whose `docs.deprecated` is a boolean) and
124+
* raw spec members (where it is a string reason).
125+
*/
126+
interface MemberLike {
127+
readonly name: string;
128+
readonly static?: boolean;
129+
readonly docs?: { readonly deprecated?: unknown };
130+
}
131+
100132
/**
101133
* Names that must be renamed (with a leading underscore) when used as Ruby
102134
* method/parameter identifiers. Includes:
@@ -191,8 +223,13 @@ export class RubyGenerator extends Generator {
191223
* `typeSpec.spec.initializer.parameters`. Collection/union introspection
192224
* only works on the raw shape.
193225
*/
194-
private typeRefSpec(type: any): spec.TypeReference | undefined {
195-
return type?.spec ?? type;
226+
private typeRefSpec(
227+
type: RubyTypeRef | undefined,
228+
): spec.TypeReference | undefined {
229+
if (type instanceof reflect.TypeReference) {
230+
return type.spec;
231+
}
232+
return type;
196233
}
197234

198235
private isStructFqn(fqn: string): boolean {
@@ -203,7 +240,10 @@ export class RubyGenerator extends Generator {
203240
/**
204241
* Extract the raw `spec.Docs` from either shape we hold: jsii-reflect
205242
* objects wrap it under `.spec.docs`; plain spec objects (enum members,
206-
* initializer parameters) carry `.docs` directly.
243+
* initializer parameters) carry `.docs` directly. Genuinely dual-shape,
244+
* hence the `any` — reflect `Docs` instances also satisfy the returned
245+
* type structurally (their getters mirror spec.Docs, except `deprecated`
246+
* which is a boolean; emitDocs handles both).
207247
*/
208248
private rawDocs(obj: any): spec.Docs | undefined {
209249
return obj?.spec?.docs ?? obj?.docs;
@@ -278,16 +318,16 @@ export class RubyGenerator extends Generator {
278318
* there are no docs and no tags to write.
279319
*/
280320
private emitDocs(
281-
docsSource: any,
321+
docsSource: unknown,
282322
opts: {
283323
/** Parameters (raw spec or reflect) to render as @param tags. */
284-
params?: any[];
324+
params?: readonly ParamLike[];
285325
/** The method's raw `returns` OptionalValue; pass with isMethod. */
286-
returns?: any;
326+
returns?: spec.OptionalValue;
287327
/** Emit `@return [void]` when a method declares no return type. */
288328
isMethod?: boolean;
289329
/** Property getter: emit an @return of the property's type. */
290-
propertyType?: any;
330+
propertyType?: RubyTypeRef;
291331
propertyOptional?: boolean;
292332
} = {},
293333
): void {
@@ -396,10 +436,10 @@ export class RubyGenerator extends Generator {
396436

397437
// Loop through the Abstract Syntax Tree (AST) metadata types
398438
const types = assembly.allTypes.slice();
399-
const sortedTypes: any[] = [];
439+
const sortedTypes: reflect.Type[] = [];
400440
const visited = new Set<string>();
401441

402-
const visit = (type: any) => {
442+
const visit = (type: reflect.Type) => {
403443
if (visited.has(type.fqn)) return;
404444
visited.add(type.fqn);
405445

@@ -519,10 +559,10 @@ export class RubyGenerator extends Generator {
519559
this.code.line('');
520560
}
521561

522-
private emitEnumType(typeSpec: any, prefix: string): void {
562+
private emitEnumType(typeSpec: reflect.EnumType, prefix: string): void {
523563
const resolvedMembers = this.dedupByRubyName(
524-
(typeSpec.members ?? []) as any[],
525-
(m: any) => this.rubyConstName(m.name),
564+
typeSpec.members,
565+
(m) => this.rubyConstName(m.name),
526566
typeSpec.fqn,
527567
);
528568
this.emitDocs(typeSpec);
@@ -537,28 +577,31 @@ export class RubyGenerator extends Generator {
537577
this.code.line('');
538578
}
539579

540-
private emitInterfaceType(typeSpec: any, prefix: string): void {
580+
private emitInterfaceType(
581+
typeSpec: reflect.InterfaceType,
582+
prefix: string,
583+
): void {
541584
const { props: resolvedAllProperties, methods: resolvedAllMethods } =
542585
this.dedupCrossCategory(
543586
this.dedupByRubyName(
544-
typeSpec.allProperties as any[],
545-
(p: any) => this.rubyName(p.name),
587+
typeSpec.allProperties,
588+
(p) => this.rubyName(p.name),
546589
typeSpec.fqn,
547590
),
548591
this.dedupByRubyName(
549-
typeSpec.allMethods as any[],
550-
(m: any) => this.rubyName(m.name),
592+
typeSpec.allMethods,
593+
(m) => this.rubyName(m.name),
551594
typeSpec.fqn,
552595
),
553-
(p: any) => this.rubyName(p.name),
554-
(m: any) => this.rubyName(m.name),
596+
(p) => this.rubyName(p.name),
597+
(m) => this.rubyName(m.name),
555598
typeSpec.fqn,
556599
);
557600
const kind = typeSpec.datatype ? 'class' : 'module';
558601
const rubyName = this.rubyModuleName(typeSpec.name);
559602

560603
const bases = typeSpec.spec.interfaces ?? [];
561-
const baseMixins = bases.map((b: any) => `::${this.rubyFullTypeName(b)}`);
604+
const baseMixins = bases.map((b) => `::${this.rubyFullTypeName(b)}`);
562605
// JSII structs may extend several parents (diamond hierarchies), but a
563606
// Ruby class has a single superclass: subclass the first parent and
564607
// record the rest via `jsii_extra_struct_bases` so is_a?/kind_of?/case
@@ -594,7 +637,7 @@ export class RubyGenerator extends Generator {
594637
const props = resolvedAllProperties;
595638

596639
const initArgs = props
597-
.map((p: any) => {
640+
.map((p) => {
598641
const name = this.rubyName(p.name);
599642
return p.optional ? `${name}: nil` : `${name}:`;
600643
})
@@ -678,14 +721,14 @@ export class RubyGenerator extends Generator {
678721

679722
for (const method of resolvedAllMethods) {
680723
const sigParams = method.parameters
681-
.map((p: any) => {
724+
.map((p) => {
682725
const rubyParam = this.rubyName(p.name);
683726
if (p.variadic) return `*${rubyParam}`;
684727
return p.optional ? `${rubyParam} = nil` : rubyParam;
685728
})
686729
.join(', ');
687730
const callParams = method.parameters
688-
.map((p: any) => {
731+
.map((p) => {
689732
const rubyParam = this.rubyName(p.name);
690733
if (p.variadic) return `*${rubyParam}`;
691734
return rubyParam;
@@ -741,21 +784,21 @@ export class RubyGenerator extends Generator {
741784
this.code.line('');
742785
}
743786

744-
private emitClassType(typeSpec: any, prefix: string): void {
787+
private emitClassType(typeSpec: reflect.ClassType, prefix: string): void {
745788
const { props: resolvedAllProperties, methods: resolvedAllMethods } =
746789
this.dedupCrossCategory(
747790
this.dedupByRubyName(
748-
typeSpec.allProperties as any[],
749-
(p: any) => this.rubyPropertyName(p),
791+
typeSpec.allProperties,
792+
(p) => this.rubyPropertyName(p),
750793
typeSpec.fqn,
751794
),
752795
this.dedupByRubyName(
753-
typeSpec.allMethods as any[],
754-
(m: any) => this.rubyMethodName(m),
796+
typeSpec.allMethods,
797+
(m) => this.rubyMethodName(m),
755798
typeSpec.fqn,
756799
),
757-
(p: any) => this.rubyPropertyName(p),
758-
(m: any) => this.rubyMethodName(m),
800+
(p) => this.rubyPropertyName(p),
801+
(m) => this.rubyMethodName(m),
759802
typeSpec.fqn,
760803
);
761804
const rubyName = this.rubyModuleName(typeSpec.name);
@@ -768,7 +811,7 @@ export class RubyGenerator extends Generator {
768811

769812
const interfaces = typeSpec.spec.interfaces ?? [];
770813
const interfaceMixins = interfaces.map(
771-
(i: any) => `::${this.rubyFullTypeName(i)}`,
814+
(i) => `::${this.rubyFullTypeName(i)}`,
772815
);
773816

774817
this.emitDocs(typeSpec);
@@ -790,7 +833,7 @@ export class RubyGenerator extends Generator {
790833
initializer.parameters.length > 0
791834
) {
792835
const initParams = initializer.parameters
793-
.map((p: any) => {
836+
.map((p) => {
794837
const rubyParam = this.rubyName(p.name);
795838
if (p.variadic) return `*${rubyParam}`;
796839
return p.optional ? `${rubyParam} = nil` : rubyParam;
@@ -804,7 +847,7 @@ export class RubyGenerator extends Generator {
804847
this.emitStructCoercion(rubyParam, p.type);
805848
}
806849
const superArgs = initializer.parameters
807-
.map((p: any) => {
850+
.map((p) => {
808851
const rubyParam = this.rubyName(p.name);
809852
if (p.variadic) return `*${rubyParam}`;
810853
return rubyParam;
@@ -856,11 +899,12 @@ export class RubyGenerator extends Generator {
856899
// call instead. A child that overrides a static still gets its own
857900
// stub, because allMethods/allProperties yield the most-derived
858901
// declaration (see the StaticHelloParent/Child fixture in jsii-calc).
859-
const isOwnStatic = (m: any) => m.definingType?.fqn === typeSpec.fqn;
902+
const isOwnStatic = (m: reflect.Property | reflect.Method) =>
903+
m.definingType.fqn === typeSpec.fqn;
860904

861-
const overridableMethods = resolvedAllMethods.filter((m: any) => !m.static);
905+
const overridableMethods = resolvedAllMethods.filter((m) => !m.static);
862906
const overridableProps = resolvedAllProperties.filter(
863-
(p: any) => !p.static,
907+
(p) => !p.static,
864908
);
865909

866910
this.code.open('def self.jsii_overridable_methods');
@@ -886,15 +930,15 @@ export class RubyGenerator extends Generator {
886930
if (!method.static || !isOwnStatic(method)) continue;
887931

888932
const sigParams = method.parameters
889-
.map((p: any) => {
933+
.map((p) => {
890934
const rubyParam = this.rubyName(p.name);
891935
if (p.variadic) return `*${rubyParam}`;
892936
return p.optional ? `${rubyParam} = nil` : rubyParam;
893937
})
894938
.join(', ');
895939

896940
const callParams = method.parameters
897-
.map((p: any) => {
941+
.map((p) => {
898942
const rubyParam = this.rubyName(p.name);
899943
if (p.variadic) return `*${rubyParam}`;
900944
return rubyParam;
@@ -980,15 +1024,15 @@ export class RubyGenerator extends Generator {
9801024
if (method.static) continue;
9811025

9821026
const sigParams = method.parameters
983-
.map((p: any) => {
1027+
.map((p) => {
9841028
const rubyParam = this.rubyName(p.name);
9851029
if (p.variadic) return `*${rubyParam}`;
9861030
return p.optional ? `${rubyParam} = nil` : rubyParam;
9871031
})
9881032
.join(', ');
9891033

9901034
const callParams = method.parameters
991-
.map((p: any) => {
1035+
.map((p) => {
9921036
const rubyParam = this.rubyName(p.name);
9931037
if (p.variadic) return `*${rubyParam}`;
9941038
return rubyParam;
@@ -1214,7 +1258,7 @@ export class RubyGenerator extends Generator {
12141258

12151259
private emitStructCoercion(
12161260
variableName: string,
1217-
type: any,
1261+
type: RubyTypeRef | undefined,
12181262
options: { variadic?: boolean; assignment?: string } = {},
12191263
): void {
12201264
const ref = this.typeRefSpec(type);
@@ -1240,7 +1284,7 @@ export class RubyGenerator extends Generator {
12401284

12411285
private emitTypeChecking(
12421286
variableName: string,
1243-
type: any,
1287+
type: RubyTypeRef | undefined,
12441288
jsiiName: string,
12451289
options: { isOptional?: boolean; isVariadic?: boolean } = {},
12461290
): void {
@@ -1313,15 +1357,18 @@ export class RubyGenerator extends Generator {
13131357
* (`def self.foo` vs `def foo`), so collisions are only checked within
13141358
* the same staticness.
13151359
*/
1316-
private dedupCrossCategory(
1317-
props: any[],
1318-
methods: any[],
1319-
propRubyName: (p: any) => string,
1320-
methodRubyName: (m: any) => string,
1360+
private dedupCrossCategory<P extends MemberLike, M extends MemberLike>(
1361+
props: P[],
1362+
methods: M[],
1363+
propRubyName: (p: P) => string,
1364+
methodRubyName: (m: M) => string,
13211365
fqn: string,
1322-
): { props: any[]; methods: any[] } {
1323-
const buckets = new Map<string, Array<{ member: any; isProp: boolean }>>();
1324-
const add = (member: any, isProp: boolean, name: string) => {
1366+
): { props: P[]; methods: M[] } {
1367+
const buckets = new Map<
1368+
string,
1369+
Array<{ member: P | M; isProp: boolean }>
1370+
>();
1371+
const add = (member: P | M, isProp: boolean, name: string) => {
13251372
const key = `${member.static ? 'static' : 'instance'}:${name}`;
13261373
const bucket = buckets.get(key) ?? [];
13271374
bucket.push({ member, isProp });
@@ -1368,9 +1415,11 @@ export class RubyGenerator extends Generator {
13681415
};
13691416
}
13701417

1371-
private dedupByRubyName<
1372-
T extends { name: string; docs?: { deprecated?: string } },
1373-
>(members: readonly T[], rubyName: (m: T) => string, fqn: string): T[] {
1418+
private dedupByRubyName<T extends MemberLike>(
1419+
members: readonly T[],
1420+
rubyName: (m: T) => string,
1421+
fqn: string,
1422+
): T[] {
13741423
const byName = new Map<string, T[]>();
13751424
for (const m of members) {
13761425
const key = rubyName(m);
@@ -1430,7 +1479,9 @@ export class RubyGenerator extends Generator {
14301479
* rewrite an unrelated `RamUsage` type in the consuming assembly (or in
14311480
* a sibling dependency).
14321481
*/
1433-
private assemblyAcronyms(config: any): string[] {
1482+
private assemblyAcronyms(
1483+
config: { targets?: spec.AssemblyTargets } | undefined,
1484+
): string[] {
14341485
return (config?.targets?.ruby?.acronyms ?? []).filter(
14351486
(a: unknown): a is string => typeof a === 'string' && a.length > 0,
14361487
);

0 commit comments

Comments
 (0)