Skip to content

Commit

Permalink
c#, fix: class names + namespace conflicts (#4229)
Browse files Browse the repository at this point in the history
  • Loading branch information
dcb6 authored Aug 8, 2024
1 parent 66dff51 commit ab1ea1a
Show file tree
Hide file tree
Showing 565 changed files with 2,362 additions and 736 deletions.
4 changes: 3 additions & 1 deletion generators/csharp/codegen/src/TestFileGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ export class TestFileGenerator extends FileGenerator<
});
return new CSharpFile({
clazz: testClass,
directory: RelativeFilePath.of("")
directory: RelativeFilePath.of(""),
allNamespaceSegments: this.context.getAllNamespaceSegments(),
namespace: this.context.getNamespace()
});
}
}
4 changes: 4 additions & 0 deletions generators/csharp/codegen/src/ast/Class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ export class Class extends AstNode {
this.annotations.push(annotation);
}

public getNamespace(): string {
return this.namespace;
}

public write(writer: Writer): void {
if (!this.isNestedClass) {
writer.writeLine(`namespace ${this.namespace};`);
Expand Down
60 changes: 58 additions & 2 deletions generators/csharp/codegen/src/ast/ClassReference.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ export class ClassReference extends AstNode {
}

public write(writer: Writer): void {
writer.addReference(this);
writer.write(`${this.name}`);
if (this.qualifiedTypeNameRequired(writer)) {
const typeQualification = this.getTypeQualification(this.namespace, writer.getNamespace());
writer.write(`${typeQualification}${this.name}`);
} else {
writer.addReference(this);
writer.write(`${this.name}`);
}
if (this.generics != null && this.generics.length > 0) {
writer.write("<");
this.generics.forEach((generic, idx) => {
Expand All @@ -39,6 +44,57 @@ export class ClassReference extends AstNode {
writer.write(">");
}
}

/**
* Computes the type qualification starting at the point where the namespace of the type
* differs from the namespace being written to.
*
* Example:
* - classReferenceNamespace: Company.Employee.Engineer.Backend
* - namespaceToBeWrittenTo: Company.Employee.Janitor
*
* Result: Engineer.Backend.
*/
private getTypeQualification(classReferenceNamespace: string, namespaceToBeWrittenTo: string): string {
let i = 0;
// Find the length of the longest matching prefix
while (
i < classReferenceNamespace.length &&
i < namespaceToBeWrittenTo.length &&
classReferenceNamespace[i] === namespaceToBeWrittenTo[i]
) {
i++;
}
// Return the part of 'a' after the matching prefix
const typeQualification = classReferenceNamespace.slice(i);
return `${typeQualification}${typeQualification ? "." : ""}`;
}

/**
* This method addresses an edge case involving namespace and type conflicts.
* When a class name matches any segment of a namespace within the project, the .NET compiler
* might require references to that class to be qualified to avoid conflicts.
* The rules governing this behavior are complex, so this method errs on the side of caution
* by performing a simple check.
*
* -- Exploration supporting this --
*
* LEGEND: <Class Name> -- <Namespace of Class>
* SETUP: Company.Net is the root namespace (i.e., the project name).
*
* Full qualification required:
* - Guarantor -- Company.Net.Guarantor.V1
* - ImportInvoice -- Company.Net.ImportInvoice.V1
* - ImportInvoice -- Company.Net.Guarantor.V1 (if Candid.Net.ImportInvoice.V1 also exists)
*
* Qualification not required:
* - V1 -- Company.Net.Guarantor.V1
* - V1 -- Company.Net.Guarantor.V1.Types
* - Net -- Company.Net
*/
private qualifiedTypeNameRequired(writer: Writer): boolean {
return writer.getAllNamespaceSegments().has(this.name);
}
}

export const OneOfClassReference = new ClassReference({
Expand Down
20 changes: 0 additions & 20 deletions generators/csharp/codegen/src/ast/CodeBlock.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { execSync } from "child_process";
import { AstNode } from "./core/AstNode";
import { Writer } from "./core/Writer";

Expand All @@ -22,23 +21,4 @@ export class CodeBlock extends AstNode {
this.value(writer);
}
}

/**
* function for formatting snippets, useful in testing
*/
public toFormattedSnippet(addSemiColon = true): string {
let codeString = this.toString();
if (addSemiColon) {
codeString += ";";
}
try {
return CodeBlock.formatCSharpCode(codeString);
} catch (e: unknown) {
return codeString;
}
}

private static formatCSharpCode(code: string): string {
return execSync("dotnet csharpier", { input: code, encoding: "utf-8" });
}
}
4 changes: 4 additions & 0 deletions generators/csharp/codegen/src/ast/Enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ export class Enum extends AstNode {
});
}

public getNamespace(): string {
return this.namespace;
}

public addMember(field: Enum.Member): void {
this.fields.push({
name: field.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ describe("class reference", () => {
)
]
});
expect(clazz.toString()).toContain("OneOf<string, bool, List<string>>");
expect(clazz.toString("", new Set<string>(), "")).toContain("OneOf<string, bool, List<string>>");
});
});
4 changes: 2 additions & 2 deletions generators/csharp/codegen/src/ast/core/AstNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ export abstract class AstNode {
/**
* Writes the node to a string.
*/
public toString(): string {
const writer = new Writer({});
public toString(namespace: string, allBaseNamespaces: Set<string>, rootNamespace: string): string {
const writer = new Writer({ namespace, allBaseNamespaces, rootNamespace });
this.write(writer);
return writer.toString();
}
Expand Down
28 changes: 25 additions & 3 deletions generators/csharp/codegen/src/ast/core/Writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ const TAB_SIZE = 4;
export declare namespace Writer {
interface Args {
/* The namespace that is being written to */
namespace?: string;
namespace: string;
/* All base namespaces in the project */
allBaseNamespaces: Set<string>;
/* The root namespace of the project */
rootNamespace: string;
}
}

Expand All @@ -25,10 +29,16 @@ export class Writer {
/* The current line number */
private references: Record<Namespace, ClassReference[]> = {};
/* The namespace that is being written to */
private namespace: string | undefined;
private namespace: string;
/* All base namespaces in the project */
private allBaseNamespaces: Set<string>;
/* The root namespace of the project */
private rootNamespace: string;

constructor({ namespace }: Writer.Args) {
constructor({ namespace, allBaseNamespaces, rootNamespace }: Writer.Args) {
this.namespace = namespace;
this.allBaseNamespaces = allBaseNamespaces;
this.rootNamespace = rootNamespace;
}

public write(text: string): void {
Expand Down Expand Up @@ -132,6 +142,18 @@ export class Writer {
}
}

public getAllNamespaceSegments(): Set<string> {
return this.allBaseNamespaces;
}

public getRootNamespace(): string {
return this.rootNamespace;
}

public getNamespace(): string {
return this.namespace;
}

public toString(): string {
const imports = this.stringifyImports();
if (imports.length > 0) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { RelativeFilePath } from "@fern-api/fs-utils";
import { AbstractGeneratorContext, FernGeneratorExec, GeneratorNotificationService } from "@fern-api/generator-commons";
import {
FernFilepath,
IntermediateRepresentation,
Name,
TypeDeclaration,
Expand Down Expand Up @@ -30,6 +31,7 @@ export abstract class AbstractCsharpGeneratorContext<
public readonly project: CsharpProject;
public readonly csharpTypeMapper: CsharpTypeMapper;
public publishConfig: FernGeneratorExec.NugetGithubPublishInfo | undefined;
private allNamespaceSegmentsCache?: Set<string>;

public constructor(
public readonly ir: IntermediateRepresentation,
Expand Down Expand Up @@ -74,6 +76,30 @@ export abstract class AbstractCsharpGeneratorContext<
});
}

public getAllNamespaceSegments(): Set<string> {
if (this.allNamespaceSegmentsCache == null) {
const namespaces: string[] = [];
for (const [_, subpackage] of Object.entries(this.ir.subpackages)) {
const namespaceSegments = this.getFullNamespaceSegments(subpackage.fernFilepath);
if (namespaceSegments != null) {
namespaces.push(...namespaceSegments);
}
}
this.allNamespaceSegmentsCache = new Set(namespaces);
}
return this.allNamespaceSegmentsCache;
}

public getNamespaceFromFernFilepath(fernFilepath: FernFilepath): string {
return this.getFullNamespaceSegments(fernFilepath).join(".");
}

abstract getChildNamespaceSegments(fernFilepath: FernFilepath): string[];

public getFullNamespaceSegments(fernFilepath: FernFilepath): string[] {
return [this.getNamespace(), ...this.getChildNamespaceSegments(fernFilepath)];
}

public getStringEnumSerializerClassReference(): csharp.ClassReference {
return csharp.classReference({
namespace: this.getCoreNamespace(),
Expand Down
8 changes: 6 additions & 2 deletions generators/csharp/codegen/src/project/CSharpFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,16 @@ export declare namespace CSharpFile {
clazz: Class | Enum;
/* Directory of the filepath */
directory: RelativeFilePath;
/* All base namespaces. Can be pulled directly from context. */
allNamespaceSegments: Set<string>;
/* The root namespace of the project. Can be pulled directly from context. */
namespace: string;
}
}

export class CSharpFile extends File {
constructor({ clazz, directory }: CSharpFile.Args) {
super(`${clazz.name}.cs`, directory, clazz.toString());
constructor({ clazz, directory, allNamespaceSegments, namespace }: CSharpFile.Args) {
super(`${clazz.name}.cs`, directory, clazz.toString(clazz.getNamespace(), allNamespaceSegments, namespace));
}

public async tryWrite(directoryPrefix: AbsoluteFilePath): Promise<void> {
Expand Down
6 changes: 5 additions & 1 deletion generators/csharp/model/src/ModelGeneratorContext.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AbstractCsharpGeneratorContext, AsIsFiles } from "@fern-api/csharp-codegen";
import { RelativeFilePath } from "@fern-api/fs-utils";
import { TypeId } from "@fern-fern/ir-sdk/api";
import { FernFilepath, TypeId } from "@fern-fern/ir-sdk/api";
import { ModelCustomConfigSchema } from "./ModelCustomConfig";

export class ModelGeneratorContext extends AbstractCsharpGeneratorContext<ModelCustomConfigSchema> {
Expand Down Expand Up @@ -33,4 +33,8 @@ export class ModelGeneratorContext extends AbstractCsharpGeneratorContext<ModelC
public getExtraDependencies(): Record<string, string> {
return {};
}

override getChildNamespaceSegments(fernFilepath: FernFilepath): string[] {
return fernFilepath.packagePath.map((segmentName) => segmentName.pascalCase.safeName);
}
}
4 changes: 3 additions & 1 deletion generators/csharp/model/src/enum/EnumGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ export class EnumGenerator extends FileGenerator<CSharpFile, ModelCustomConfigSc

return new CSharpFile({
clazz: enum_,
directory: this.context.getDirectoryForTypeId(this.typeDeclaration.name.typeId)
directory: this.context.getDirectoryForTypeId(this.typeDeclaration.name.typeId),
allNamespaceSegments: this.context.getAllNamespaceSegments(),
namespace: this.context.getNamespace()
});
}

Expand Down
4 changes: 3 additions & 1 deletion generators/csharp/model/src/object/ObjectGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ export class ObjectGenerator extends FileGenerator<CSharpFile, ModelCustomConfig

return new CSharpFile({
clazz: class_,
directory: this.context.getDirectoryForTypeId(this.typeDeclaration.name.typeId)
directory: this.context.getDirectoryForTypeId(this.typeDeclaration.name.typeId),
allNamespaceSegments: this.context.getAllNamespaceSegments(),
namespace: this.context.getNamespace()
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ export class ObjectSerializationTestGenerator extends FileGenerator<
});
return new CSharpFile({
clazz: testClass.getClass(),
directory: SERIALIZATION_TEST_FOLDER
directory: SERIALIZATION_TEST_FOLDER,
allNamespaceSegments: this.context.getAllNamespaceSegments(),
namespace: this.context.getNamespace()
});
}

Expand Down
4 changes: 4 additions & 0 deletions generators/csharp/sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.10.0 - 2024-08-07]

- Fix: Fix a bug where conflicting class names and namespaces cause compile to fail.

## [0.9.0 - 2024-08-01]

- Feature: Add the `base-api-exception-class-name` and `base-exception-class-name` generator configuration.
Expand Down
2 changes: 1 addition & 1 deletion generators/csharp/sdk/VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
0.9.0
0.10.0
6 changes: 3 additions & 3 deletions generators/csharp/sdk/src/SdkGeneratorContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,10 @@ export class SdkGeneratorContext extends AbstractCsharpGeneratorContext<SdkCusto
return this.customConfig["extra-dependencies"] ?? {};
}

private getNamespaceFromFernFilepath(fernFilepath: FernFilepath): string {
const parts =
override getChildNamespaceSegments(fernFilepath: FernFilepath): string[] {
const segmentNames =
this.customConfig["explicit-namespaces"] === true ? fernFilepath.allParts : fernFilepath.packagePath;
return [this.getNamespace(), ...parts.map((path) => path.pascalCase.safeName)].join(".");
return segmentNames.map((segmentName) => segmentName.pascalCase.safeName);
}

public isOptional(typeReference: TypeReference): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ export class MultiUrlEnvironmentGenerator extends FileGenerator<

return new CSharpFile({
clazz: class_,
directory: RelativeFilePath.of(this.context.getCoreDirectory())
directory: RelativeFilePath.of(this.context.getCoreDirectory()),
allNamespaceSegments: this.context.getAllNamespaceSegments(),
namespace: this.context.getNamespace()
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ export class SingleUrlEnvironmentGenerator extends FileGenerator<

return new CSharpFile({
clazz: class_,
directory: RelativeFilePath.of(this.context.getCoreDirectory())
directory: RelativeFilePath.of(this.context.getCoreDirectory()),
allNamespaceSegments: this.context.getAllNamespaceSegments(),
namespace: this.context.getNamespace()
});
}

Expand Down
4 changes: 3 additions & 1 deletion generators/csharp/sdk/src/error/BaseApiExceptionGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ export class BaseApiExceptionGenerator extends FileGenerator<CSharpFile, SdkCust
);
return new CSharpFile({
clazz: class_,
directory: this.context.getCoreDirectory()
directory: this.context.getCoreDirectory(),
allNamespaceSegments: this.context.getAllNamespaceSegments(),
namespace: this.context.getNamespace()
});
}
protected getFilepath(): RelativeFilePath {
Expand Down
4 changes: 3 additions & 1 deletion generators/csharp/sdk/src/error/BaseExceptionGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ export class BaseExceptionGenerator extends FileGenerator<CSharpFile, SdkCustomC
});
return new CSharpFile({
clazz: class_,
directory: this.context.getCoreDirectory()
directory: this.context.getCoreDirectory(),
allNamespaceSegments: this.context.getAllNamespaceSegments(),
namespace: this.context.getNamespace()
});
}
protected getFilepath(): RelativeFilePath {
Expand Down
Loading

0 comments on commit ab1ea1a

Please sign in to comment.