Skip to content

Commit

Permalink
fixes: 💎 Ruby: Fix typos, imports and several other papercuts within …
Browse files Browse the repository at this point in the history
…SDK generation (#2868)

* several sdk fixes

* add back the ruby playground and rerun with the right seed

* several fixes for SDK

* several fixes for SDK
  • Loading branch information
armandobelardo authored Feb 5, 2024
1 parent 20d3a18 commit a0122fd
Show file tree
Hide file tree
Showing 775 changed files with 8,979 additions and 7,487 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ generators/**/tsconfig*tsbuildinfo
generators/**/out

# don't ignore the ruby lib within the playground
!packages/**/ruby/playground/lib
!generators/ruby/playground/lib

# next
.next
Expand Down
2 changes: 1 addition & 1 deletion generators/ruby/codegen/src/ast/Argument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export class Argument extends AstNode {
});
}

public fromJson(): FunctionInvocation | undefined {
public fromJson(): AstNode | undefined {
return;
}
}
11 changes: 11 additions & 0 deletions generators/ruby/codegen/src/ast/Parameter.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Argument } from "./Argument";
import { ClassReference, NilValue } from "./classes/ClassReference";
import { AstNode } from "./core/AstNode";
import { Import } from "./Import";
Expand Down Expand Up @@ -58,4 +59,14 @@ export class Parameter extends AstNode {
this.type.forEach((cr) => (imports = new Set([...imports, ...cr.getImports()])));
return imports;
}

public toArgument(value: Variable | string): Argument {
return new Argument({
name: this.name,
type: this.type,
value,
isNamed: this.isNamed,
documentation: this.documentation
});
}
}
5 changes: 2 additions & 3 deletions generators/ruby/codegen/src/ast/Variable.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { ClassReference } from "./classes/ClassReference";
import { AstNode } from "./core/AstNode";
import { FunctionInvocation } from "./functions/FunctionInvocation";

export enum VariableType {
CLASS,
Expand Down Expand Up @@ -45,10 +44,10 @@ export class Variable extends AstNode {
this.addText({ stringContent: this.name, templateString, startingTabSpaces });
}

public fromJson(): FunctionInvocation | undefined {
public fromJson(): AstNode | undefined {
const classReferences = this.type;
// Do not support calling fromJson when there are multiple class references
if (classReferences.length === 0) {
if (classReferences.length <= 1) {
const classReference = classReferences.at(0);
return classReference !== undefined ? classReference.fromJson(this) : undefined;
}
Expand Down
51 changes: 38 additions & 13 deletions generators/ruby/codegen/src/ast/Yardoc.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ObjectProperty, TypeId } from "@fern-fern/ir-sdk/api";
import { ClassReference, ClassReferenceFactory } from "./classes/ClassReference";
import { ArrayReference, ClassReference, ClassReferenceFactory } from "./classes/ClassReference";
import { AstNode } from "./core/AstNode";
import { Parameter } from "./Parameter";
import { Property } from "./Property";
Expand Down Expand Up @@ -69,20 +69,45 @@ export class Yardoc extends AstNode {
}
}

private writeMultilineYardocComment(documentation?: string[], defaultComment?: string): void {
documentation?.forEach((doc, index) => {
const trimmedDoc = doc.trim();
this.addText({
stringContent: trimmedDoc.length > 0 ? trimmedDoc : undefined,
templateString: index > 0 ? "# " : undefined,
appendToLastString: index === 0
});
}) ?? this.addText({ stringContent: defaultComment, appendToLastString: true });
}

private writeParameterAsHash(parameter: Parameter, startingTabSpaces: number): void {
const typeId = parameter.type[0]?.resolvedTypeId;
const properties: ObjectProperty[] | undefined = this.flattenedProperties?.get(typeId ?? "");
// TODO: handle multitype better with hashes
if (parameter.type.length > 1) {
return this.writeParameterAsClass(parameter, startingTabSpaces);
}

let typeId = parameter.type[0]?.resolvedTypeId;
let isArray = false;
const classFactory = this.crf;
// TODO: handle nested hashes better
if (parameter.type[0] instanceof ArrayReference && parameter.type[0].innerType instanceof ClassReference) {
isArray = true;
typeId = parameter.type[0].innerType.resolvedTypeId;
}
const properties: ObjectProperty[] | undefined = this.flattenedProperties?.get(typeId ?? "");

if (typeId === undefined || properties === undefined || classFactory === undefined) {
this.writeParameterAsClass(parameter, startingTabSpaces);
} else {
this.addText({ stringContent: parameter.name, templateString: "# @param %s [Hash] ", startingTabSpaces });
this.addText({
stringContent:
parameter.documentation ??
`Request of type ${parameter.type.map((param) => param.typeHint).join(", ")}, as a Hash`,
appendToLastString: true
stringContent: parameter.name,
templateString: isArray ? "# @param %s [Array<Hash>] " : "# @param %s [Hash] ",
startingTabSpaces
});
this.writeMultilineYardocComment(
parameter.documentation,
`Request of type ${parameter.type.map((param) => param.typeHint).join(", ")}, as a Hash`
);
properties.forEach((prop) => {
const classReference = classFactory.fromTypeReference(prop.valueType);
this.writeHashContents(
Expand All @@ -99,15 +124,15 @@ export class Yardoc extends AstNode {
private writeParameterAsClass(parameter: Parameter, startingTabSpaces: number): void {
if (parameter.isBlock) {
this.addText({ stringContent: parameter.name, templateString: "# @yield", startingTabSpaces });
this.addText({ stringContent: parameter.documentation, appendToLastString: true });
this.writeMultilineYardocComment(parameter.documentation);
} else {
this.addText({ stringContent: parameter.name, templateString: "# @param %s", startingTabSpaces });
this.addText({
stringContent: parameter.type.map((param) => param.typeHint).join(", "),
templateString: " [%s] ",
appendToLastString: true
});
this.addText({ stringContent: parameter.documentation, appendToLastString: true });
this.writeMultilineYardocComment(parameter.documentation);
}
}

Expand All @@ -120,10 +145,10 @@ export class Yardoc extends AstNode {
: this.reference.type instanceof ClassReference
? this.reference.type.typeHint
: this.reference.type;
const documentation =
this.reference.type instanceof Property ? this.reference.type.documentation : undefined;
this.addText({ stringContent: typeName, templateString: "# @type [%s] ", startingTabSpaces });
this.addText({ stringContent: documentation, appendToLastString: true });
this.writeMultilineYardocComment(
this.reference.type instanceof Property ? this.reference.type.documentation : []
);
} else {
this.reference.parameters.forEach((parameter) => {
if (parameter.describeAsHashInYardoc) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { BLOCK_END } from "../../utils/RubyConstants";
import { Class_ } from "../classes/Class_";
import { AstNode } from "../core/AstNode";
import { Import } from "../Import";

Expand Down Expand Up @@ -28,23 +29,24 @@ export class ConditionalStatement extends AstNode {
}

private writeCondition(startingTabSpaces: number, condition: Condition, type: "if" | "elsif" | "else"): void {
if (condition.operation === "!" && condition.leftSide === undefined) {
this.addText({
stringContent: "unless ",
startingTabSpaces
});
} else {
const updatedType =
condition.operation === "!" && (condition.leftSide === undefined || condition.rightSide === undefined)
? "unless"
: type;
const leftString = condition.leftSide instanceof AstNode ? condition.leftSide.write({}) : condition.leftSide;
const rightString =
condition.rightSide instanceof AstNode ? condition.rightSide.write({}) : condition.rightSide;
this.addText({
stringContent: leftString ?? rightString,
templateString: `${updatedType} %s`,
startingTabSpaces
});
if (condition.leftSide !== undefined && condition.rightSide !== undefined) {
this.addText({
stringContent:
condition.leftSide instanceof AstNode ? condition.leftSide.write({}) : condition.leftSide,
templateString: `${type} %s`,
stringContent: rightString,
startingTabSpaces
});
}
this.addText({
stringContent: condition.rightSide instanceof AstNode ? condition.rightSide.write({}) : condition.rightSide,
appendToLastString: true
});
condition.expressions.forEach((exp) =>
this.addText({ stringContent: exp.write({}), startingTabSpaces: this.tabSizeSpaces + startingTabSpaces })
);
Expand Down Expand Up @@ -73,11 +75,19 @@ export class ConditionalStatement extends AstNode {

public getImports(): Set<Import> {
let imports = new Set<Import>();
[
...this.if_.expressions,
...Array.from(this.elseIf?.values() ?? []).flatMap((condition) => condition.expressions),
...(this.else_ ?? [])
].forEach((exp) => (imports = new Set([...imports, ...exp.getImports()])));
(
[
this.if_.leftSide,
this.if_.rightSide,
...this.if_.expressions,
...Array.from(this.elseIf?.values() ?? []).flatMap((condition) => [
condition.leftSide,
condition.rightSide,
...condition.expressions
]),
...(this.else_ ?? [])
].filter((node) => node instanceof AstNode && !(node instanceof Class_)) as AstNode[]
).forEach((exp) => (imports = new Set([...imports, ...exp.getImports()])));
return imports;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
VoidClassReference
} from "../classes/ClassReference";
import { Class_ } from "../classes/Class_";
import { AstNode } from "../core/AstNode";
import { Expression } from "../expressions/Expression";
import { FunctionInvocation } from "../functions/FunctionInvocation";
import { Function_ } from "../functions/Function_";
Expand Down Expand Up @@ -78,7 +79,7 @@ export class DiscriminatedUnion extends Class_ {
memberProperty: Property,
unionSubclass: DiscriminatedUnion.Subclass
): Expression {
const rightSide = unionSubclass.unionPropertiesType._visit<FunctionInvocation | string>({
const rightSide = unionSubclass.unionPropertiesType._visit<AstNode | string>({
samePropertiesAsObject: () => unionSubclass.classReference?.fromJson(jsonParameter) ?? jsonParameter,
singleProperty: (sutp: SingleUnionTypeProperty) =>
unionSubclass.classReference?.fromJson(`${jsonParameter}.${sutp.name.wireValue}`) ??
Expand Down Expand Up @@ -175,10 +176,12 @@ export class DiscriminatedUnion extends Class_ {
contents: new Map([[discriminantField, discriminantProperty.toVariable()]]),
// Take the member property and spread it into this hash that includes the discriminant
additionalHashes: [
new FunctionInvocation({
baseFunction: new Function_({ name: "to_json", functionBody: [] }),
onObject: memberProperty.toVariable()
})
{
value: new FunctionInvocation({
baseFunction: new Function_({ name: "to_json", functionBody: [] }),
onObject: memberProperty.toVariable()
})
}
]
}),
singleProperty: (sutp: SingleUnionTypeProperty) =>
Expand Down
15 changes: 11 additions & 4 deletions generators/ruby/codegen/src/ast/abstractions/SerializableObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
ArrayReference,
ClassReference,
DateReference,
EnumReference,
GenericClassReference,
HashInstance,
HashReference,
Expand Down Expand Up @@ -77,9 +78,9 @@ export class SerializableObject extends Class_ {

const hasFromJson =
variable.fromJson() !== undefined &&
!(prop.type instanceof ArrayReference) &&
!(prop.type instanceof HashReference) &&
!(prop.type instanceof DateReference);
!(prop.type[0] instanceof ArrayReference) &&
!(prop.type[0] instanceof HashReference) &&
!(prop.type[0] instanceof DateReference);
if (hasFromJson) {
// If there's a special fromJson, then cast the object back to JSON before proceeding
const variable = new Variable({
Expand Down Expand Up @@ -141,7 +142,13 @@ export class SerializableObject extends Class_ {
baseFunction: new Function_({ name: "to_json", functionBody: [] }),
onObject: new HashInstance({
contents: new Map(
properties?.map((prop) => [`"${prop.wireValue ?? prop.name}"`, prop.toVariable()])
properties?.map((prop) => {
if (prop.type[0] instanceof EnumReference) {
return [`"${prop.wireValue ?? prop.name}"`, prop.type[0].toJson(prop.toVariable())];
}
// TODO: confirm enums are the only special case here
return [`"${prop.wireValue ?? prop.name}"`, prop.toVariable()];
})
)
})
})
Expand Down
Loading

0 comments on commit a0122fd

Please sign in to comment.