Skip to content

Commit

Permalink
fix(php): Handle cross package type names (#5395)
Browse files Browse the repository at this point in the history
* only keep relevant changes before editing the test

* run test

* add changelog entry

* chore: update changelog

---------

Co-authored-by: Alberto <[email protected]>
Co-authored-by: ajgateno <[email protected]>
  • Loading branch information
3 people authored Dec 12, 2024
1 parent 26b7ac8 commit 7c73af2
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 16 deletions.
3 changes: 3 additions & 0 deletions fern/pages/changelogs/php-sdk/2024-12-12.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## 0.2.2
**`(fix):`** Handle cross package type name deconfliction

2 changes: 2 additions & 0 deletions generators/php/codegen/src/ast/Class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export class Class extends AstNode {
}

public write(writer: Writer): void {
// required to fully de-conflict imports
writer.addReference(new ClassReference({ name: this.name, namespace: this.namespace }));
if (this.abstract) {
writer.write("abstract ");
}
Expand Down
9 changes: 8 additions & 1 deletion generators/php/codegen/src/ast/ClassReference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,22 @@ export declare namespace ClassReference {
export class ClassReference extends AstNode {
public readonly name: string;
public readonly namespace: string;
private fullyQualified: boolean;

constructor({ name, namespace }: ClassReference.Args) {
super();
this.name = name;
this.namespace = namespace;
this.fullyQualified = false;
}

public requireFullyQualified(): void {
this.fullyQualified = true;
}

public write(writer: Writer): void {
writer.addReference(this);
writer.write(`${this.name}`);
const refString = this.fullyQualified ? `\\${this.namespace}\\${this.name}` : this.name;
writer.write(`${refString}`);
}
}
42 changes: 33 additions & 9 deletions generators/php/codegen/src/ast/core/Writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,20 @@ import { BasePhpCustomConfigSchema } from "../../custom-config/BasePhpCustomConf
import { ClassReference } from "../ClassReference";
import { GLOBAL_NAMESPACE } from "./Constant";

type Namespace = string;
/* A fully qualified type name _without_ the initial backslash */
type FullyQualifiedName = string;

interface ParsedFullyQualifiedName {
namespace: string;
name: string;
}

function parseFullyQualifiedName(rawFullyQualifiedName: string): ParsedFullyQualifiedName {
return {
namespace: rawFullyQualifiedName.substring(0, rawFullyQualifiedName.lastIndexOf("\\")),
name: rawFullyQualifiedName.substring(rawFullyQualifiedName.lastIndexOf("\\") + 1)
};
}

export declare namespace Writer {
interface Args {
Expand All @@ -25,7 +38,7 @@ export class Writer extends AbstractWriter {
public customConfig: BasePhpCustomConfigSchema;

/* Import statements */
private references: Record<Namespace, ClassReference[]> = {};
private references: Record<FullyQualifiedName, ClassReference[]> = {};

constructor({ namespace, rootNamespace, customConfig }: Writer.Args) {
super();
Expand All @@ -38,9 +51,24 @@ export class Writer extends AbstractWriter {
if (reference.namespace == null) {
return;
}
const namespace =

// If there's a naming conflict, tell the reference to use its qualified name
const conflictingReferences = Object.keys(this.references)
// Filter out the current namespace.
.filter((seenRef) => {
const parsed = parseFullyQualifiedName(seenRef);
return parsed.namespace !== reference.namespace && parsed.name === reference.name;
});

if (conflictingReferences.length > 0) {
reference.requireFullyQualified();
return;
}

const fullyQualifiedName =
reference.namespace === GLOBAL_NAMESPACE ? reference.name : `${reference.namespace}\\${reference.name}`;
const references = (this.references[namespace] ??= []);
const references = (this.references[fullyQualifiedName] ??= []);

references.push(reference);
}

Expand All @@ -64,11 +92,7 @@ ${this.buffer}`;
}
let result = referenceKeys
// Filter out the current namespace.
.filter((reference) => {
// Remove the type name to get just the namespace
const referenceNamespace = reference.substring(0, reference.lastIndexOf("\\"));
return referenceNamespace !== this.namespace;
})
.filter((reference) => parseFullyQualifiedName(reference).namespace !== this.namespace)
.map((ref) => `use ${ref};`)
.join("\n");

Expand Down
6 changes: 6 additions & 0 deletions generators/php/sdk/versions.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
- version: 0.2.2
changelogEntry:
- type: fix
summary: >-
Handle cross package type name deconfliction
irVersion: 53
- version: 0.2.1
changelogEntry:
- type: fix
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions seed/php-sdk/seed.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ scripts:
allowedFailures:
# Basic auth is not supported yet.
- basic-auth
# We aren't handling multiple types used in a class with the same name (from different packages).
- cross-package-type-names
# Path parameter enums are not supported yet.
- enum
# Enums don't support the fromJson method yet.
Expand Down

0 comments on commit 7c73af2

Please sign in to comment.