From 7e2043a78d69b1e54b9ce26ce430da3fd16d5013 Mon Sep 17 00:00:00 2001 From: dcb6 Date: Wed, 2 Oct 2024 13:06:00 -0400 Subject: [PATCH] address comments --- generators/php/codegen/src/ast/Class.ts | 16 +++++------ generators/php/codegen/src/ast/DataClass.ts | 8 +++--- generators/php/codegen/src/ast/Trait.ts | 12 ++++---- .../context/AbstractPhpGeneratorContext.ts | 18 ++++++------ generators/php/model/src/generateTraits.ts | 28 ++++++++----------- .../php/model/src/object/ObjectGenerator.ts | 6 ++-- .../php/model/src/trait/TraitGenerator.ts | 2 +- .../WrappedEndpointRequestGenerator.ts | 6 ++-- 8 files changed, 44 insertions(+), 52 deletions(-) diff --git a/generators/php/codegen/src/ast/Class.ts b/generators/php/codegen/src/ast/Class.ts index cbf05527c40..7b26e5a8813 100644 --- a/generators/php/codegen/src/ast/Class.ts +++ b/generators/php/codegen/src/ast/Class.ts @@ -23,7 +23,7 @@ export declare namespace Class { /* The class to inherit from if any */ parentClassReference?: AstNode; /* The traits that this class uses, if any */ - usedTraits?: ClassReference[]; + traits?: ClassReference[]; } interface Constructor { @@ -42,20 +42,20 @@ export class Class extends AstNode { public readonly abstract: boolean; public readonly docs: string | undefined; public readonly parentClassReference: AstNode | undefined; - public readonly usedTraits: ClassReference[]; + public readonly traits: ClassReference[]; public readonly fields: Field[] = []; public readonly methods: Method[] = []; private constructor_: Class.Constructor | undefined; - constructor({ name, namespace, abstract, docs, parentClassReference, usedTraits }: Class.Args) { + constructor({ name, namespace, abstract, docs, parentClassReference, traits }: Class.Args) { super(); this.name = name; this.namespace = namespace; this.abstract = abstract ?? false; this.docs = docs; this.parentClassReference = parentClassReference; - this.usedTraits = usedTraits ?? []; + this.traits = traits ?? []; } public addConstructor(constructor: Class.Constructor): void { @@ -70,8 +70,8 @@ export class Class extends AstNode { this.methods.push(method); } - public addUsedTrait(traitClassReference: ClassReference): void { - this.usedTraits.push(traitClassReference); + public addTrait(traitClassReference: ClassReference): void { + this.traits.push(traitClassReference); } public write(writer: Writer): void { @@ -87,9 +87,9 @@ export class Class extends AstNode { writer.newLine(); writer.writeLine("{"); writer.indent(); - if (this.usedTraits.length > 0) { + if (this.traits.length > 0) { writer.write("use "); - this.usedTraits.forEach((trait, index) => { + this.traits.forEach((trait, index) => { if (index > 0) { writer.write(","); } diff --git a/generators/php/codegen/src/ast/DataClass.ts b/generators/php/codegen/src/ast/DataClass.ts index 31072e67f8c..f2e4b3a6f8c 100644 --- a/generators/php/codegen/src/ast/DataClass.ts +++ b/generators/php/codegen/src/ast/DataClass.ts @@ -23,11 +23,11 @@ export class DataClass extends AstNode { public readonly namespace: string; private class_: Class; - constructor({ name, namespace, abstract, docs, parentClassReference, usedTraits }: DataClass.Args) { + constructor({ name, namespace, abstract, docs, parentClassReference, traits }: DataClass.Args) { super(); this.name = name; this.namespace = namespace; - this.class_ = new Class({ name, namespace, abstract, docs, parentClassReference, usedTraits }); + this.class_ = new Class({ name, namespace, abstract, docs, parentClassReference, traits }); } public addField(field: Field): void { @@ -37,8 +37,8 @@ export class DataClass extends AstNode { public addMethod(method: Method): void { this.class_.addMethod(method); } - public addUsedTrait(traitClassRefeference: ClassReference): void { - this.class_.addUsedTrait(traitClassRefeference); + public addTrait(traitClassRefeference: ClassReference): void { + this.class_.addTrait(traitClassRefeference); } public write(writer: Writer): void { diff --git a/generators/php/codegen/src/ast/Trait.ts b/generators/php/codegen/src/ast/Trait.ts index d8e25c326ee..a3a10a2264b 100644 --- a/generators/php/codegen/src/ast/Trait.ts +++ b/generators/php/codegen/src/ast/Trait.ts @@ -15,7 +15,7 @@ export declare namespace Trait { /* Docs associated with the trait */ docs?: string; /* The traits that this trait uses, if any */ - usedTraits?: ClassReference[]; + traits?: ClassReference[]; } } @@ -23,17 +23,17 @@ export class Trait extends AstNode { public readonly name: string; public readonly namespace: string; public readonly docs: string | undefined; - public readonly usedTraits: ClassReference[]; + public readonly traits: ClassReference[]; public readonly fields: Field[] = []; public readonly methods: Method[] = []; - constructor({ name, namespace, docs, usedTraits }: Trait.Args) { + constructor({ name, namespace, docs, traits }: Trait.Args) { super(); this.name = name; this.namespace = namespace; this.docs = docs; - this.usedTraits = usedTraits ?? []; + this.traits = traits ?? []; } public addField(field: Field): void { @@ -51,9 +51,9 @@ export class Trait extends AstNode { writer.writeLine("{"); writer.indent(); - if (this.usedTraits.length > 0) { + if (this.traits.length > 0) { writer.write("use "); - this.usedTraits.forEach((trait, index) => { + this.traits.forEach((trait, index) => { if (index > 0) { writer.write(","); } diff --git a/generators/php/codegen/src/context/AbstractPhpGeneratorContext.ts b/generators/php/codegen/src/context/AbstractPhpGeneratorContext.ts index 72d4e2c2a75..7ad80f37e12 100644 --- a/generators/php/codegen/src/context/AbstractPhpGeneratorContext.ts +++ b/generators/php/codegen/src/context/AbstractPhpGeneratorContext.ts @@ -285,12 +285,8 @@ export abstract class AbstractPhpGeneratorContext< } } - public getUnderlyingObjectTypeDeclaration(typeReference: TypeReference): ObjectTypeDeclaration | undefined { + public getUnderlyingObjectTypeDeclaration(typeReference: TypeReference): ObjectTypeDeclaration { switch (typeReference.type) { - case "primitive": - case "unknown": - case "container": - return undefined; case "named": { const declaration = this.getTypeDeclarationOrThrow(typeReference.typeId); if (declaration.shape.type === "alias") { @@ -299,21 +295,23 @@ export abstract class AbstractPhpGeneratorContext< if (declaration.shape.type === "object") { return declaration.shape; } - return undefined; + throw new Error("Type is not an object type"); } + case "primitive": + case "unknown": + case "container": } + throw new Error("Type is not an object type"); } - public getUnderlyingObjectTypeDeclarationFromTypeDeclaration( - typeDeclaration: TypeDeclaration - ): ObjectTypeDeclaration | undefined { + public getUnderlyingObjectTypeDeclarationOrThrow(typeDeclaration: TypeDeclaration): ObjectTypeDeclaration { if (typeDeclaration.shape.type === "alias") { return this.getUnderlyingObjectTypeDeclaration(typeDeclaration.shape.aliasOf); } if (typeDeclaration.shape.type === "object") { return typeDeclaration.shape; } - return undefined; + throw new Error("Type is not an object type"); } public maybeLiteral(typeReference: TypeReference): Literal | undefined { diff --git a/generators/php/model/src/generateTraits.ts b/generators/php/model/src/generateTraits.ts index 5bb547c2ec0..a06f26bfc68 100644 --- a/generators/php/model/src/generateTraits.ts +++ b/generators/php/model/src/generateTraits.ts @@ -2,18 +2,15 @@ import { ModelGeneratorContext } from "./ModelGeneratorContext"; import { TraitGenerator } from "./trait/TraitGenerator"; export function generateTraits(context: ModelGeneratorContext): void { - const extendedTypeIdsFromTypes: Set = new Set( - Object.values(context.ir.types).flatMap((typeDeclaration) => - typeDeclaration.shape._visit({ - alias: () => [], - enum: () => [], - object: (objectDeclaration) => - objectDeclaration.extends.map((declaredTypeName) => declaredTypeName.typeId), - undiscriminatedUnion: () => [], - union: () => [], - _other: () => [] - }) - ) + const extendedTypeIdsFromTypes = Object.values(context.ir.types).flatMap((typeDeclaration) => + typeDeclaration.shape._visit({ + alias: () => [], + enum: () => [], + object: (objectDeclaration) => objectDeclaration.extends.map((declaredTypeName) => declaredTypeName.typeId), + undiscriminatedUnion: () => [], + union: () => [], + _other: () => [] + }) ); const extendedTypeIdsFromInlinedRequests = Object.values(context.ir.services) .flatMap((service) => service.endpoints) @@ -22,12 +19,9 @@ export function generateTraits(context: ModelGeneratorContext): void { ? endpoint.requestBody.extends.map((declaredTypeName) => declaredTypeName.typeId) : []; }); - for (const typeId of [...extendedTypeIdsFromTypes, ...extendedTypeIdsFromInlinedRequests]) { + for (const typeId of new Set([...extendedTypeIdsFromTypes, ...extendedTypeIdsFromInlinedRequests])) { const typeDeclaration = context.getTypeDeclarationOrThrow(typeId); - const objectTypeDeclaration = context.getUnderlyingObjectTypeDeclarationFromTypeDeclaration(typeDeclaration); - if (objectTypeDeclaration == null) { - throw new Error("Unexpected no object type declaration"); - } + const objectTypeDeclaration = context.getUnderlyingObjectTypeDeclarationOrThrow(typeDeclaration); const file = new TraitGenerator(context, typeDeclaration, objectTypeDeclaration).generate(); context.project.addSourceFiles(file); } diff --git a/generators/php/model/src/object/ObjectGenerator.ts b/generators/php/model/src/object/ObjectGenerator.ts index 60412ccd618..719b6257f8a 100644 --- a/generators/php/model/src/object/ObjectGenerator.ts +++ b/generators/php/model/src/object/ObjectGenerator.ts @@ -24,13 +24,13 @@ export class ObjectGenerator extends FileGenerator + traits: this.objectDeclaration.extends.map((declaredTypeName) => this.context.phpTypeMapper.convertToTraitClassReference(declaredTypeName) ) }); for (const property of this.objectDeclaration.properties) { - clazz.addField(this.toField({ property, inherited: false })); + clazz.addField(this.toField({ property })); } for (const property of this.objectDeclaration.extendedProperties ?? []) { clazz.addField(this.toField({ property, inherited: true })); @@ -44,7 +44,7 @@ export class ObjectGenerator extends FileGenerator + traits: this.objectDeclaration.extends.map((declaredTypeName) => this.context.phpTypeMapper.convertToTraitClassReference(declaredTypeName) ) }); diff --git a/generators/php/sdk/src/endpoint/request/WrappedEndpointRequestGenerator.ts b/generators/php/sdk/src/endpoint/request/WrappedEndpointRequestGenerator.ts index afd7252bcab..787fdf30b0c 100644 --- a/generators/php/sdk/src/endpoint/request/WrappedEndpointRequestGenerator.ts +++ b/generators/php/sdk/src/endpoint/request/WrappedEndpointRequestGenerator.ts @@ -84,13 +84,13 @@ export class WrappedEndpointRequestGenerator extends FileGenerator< }, inlinedRequestBody: (request) => { for (const property of request.properties) { - clazz.addField(this.toField({ property, inherited: false })); + clazz.addField(this.toField({ property })); } for (const property of request.extendedProperties ?? []) { clazz.addField(this.toField({ property, inherited: true })); } for (const declaredTypeName of request.extends) { - clazz.addUsedTrait(this.context.phpTypeMapper.convertToTraitClassReference(declaredTypeName)); + clazz.addTrait(this.context.phpTypeMapper.convertToTraitClassReference(declaredTypeName)); } }, fileUpload: () => undefined, @@ -106,7 +106,7 @@ export class WrappedEndpointRequestGenerator extends FileGenerator< }); } - private toField({ property, inherited }: { property: InlinedRequestBodyProperty; inherited: boolean }): php.Field { + private toField({ property, inherited }: { property: InlinedRequestBodyProperty; inherited?: boolean }): php.Field { const convertedType = this.context.phpTypeMapper.convert({ reference: property.valueType }); return php.field({ type: convertedType,