Skip to content

Commit

Permalink
fix: some reserved keywords are overlooked (#369)
Browse files Browse the repository at this point in the history
  • Loading branch information
jonaslagoni authored Sep 15, 2021
1 parent 5c88f98 commit 4440334
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 22 deletions.
4 changes: 2 additions & 2 deletions src/generators/csharp/CSharpRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export abstract class CSharpRenderer extends AbstractRenderer<CSharpOptions> {
*/
nameType(name: string | undefined, model?: CommonModel): string {
return this.options?.namingConvention?.type
? this.options.namingConvention.type(name, { model: model || this.model, inputModel: this.inputModel, isReservedKeyword: isReservedCSharpKeyword(`${name}`) })
? this.options.namingConvention.type(name, { model: model || this.model, inputModel: this.inputModel, reservedKeywordCallback: isReservedCSharpKeyword })
: name || '';
}

Expand All @@ -42,7 +42,7 @@ export abstract class CSharpRenderer extends AbstractRenderer<CSharpOptions> {
*/
nameProperty(propertyName: string | undefined, property?: CommonModel): string {
return this.options?.namingConvention?.property
? this.options.namingConvention.property(propertyName, { model: this.model, inputModel: this.inputModel, property, isReservedKeyword: isReservedCSharpKeyword(`${propertyName}`) })
? this.options.namingConvention.property(propertyName, { model: this.model, inputModel: this.inputModel, property, reservedKeywordCallback: isReservedCSharpKeyword })
: propertyName || '';
}

Expand Down
4 changes: 2 additions & 2 deletions src/generators/java/JavaRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export abstract class JavaRenderer extends AbstractRenderer<JavaOptions, JavaGen
*/
nameType(name: string | undefined, model?: CommonModel): string {
return this.options?.namingConvention?.type
? this.options.namingConvention.type(name, { model: model || this.model, inputModel: this.inputModel, isReservedKeyword: isReservedJavaKeyword(`${name}`) })
? this.options.namingConvention.type(name, { model: model || this.model, inputModel: this.inputModel, reservedKeywordCallback: isReservedJavaKeyword })
: name || '';
}

Expand All @@ -42,7 +42,7 @@ export abstract class JavaRenderer extends AbstractRenderer<JavaOptions, JavaGen
*/
nameProperty(propertyName: string | undefined, property?: CommonModel): string {
return this.options?.namingConvention?.property
? this.options.namingConvention.property(propertyName, { model: this.model, inputModel: this.inputModel, property, isReservedKeyword: isReservedJavaKeyword(`${propertyName}`) })
? this.options.namingConvention.property(propertyName, { model: this.model, inputModel: this.inputModel, property, reservedKeywordCallback: isReservedJavaKeyword })
: propertyName || '';
}

Expand Down
4 changes: 2 additions & 2 deletions src/generators/javascript/JavaScriptRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export abstract class JavaScriptRenderer extends AbstractRenderer<JavaScriptOpti
*/
nameType(name: string | undefined, model?: CommonModel): string {
return this.options?.namingConvention?.type
? this.options.namingConvention.type(name, { model: model || this.model, inputModel: this.inputModel, isReservedKeyword: isReservedJavaScriptKeyword(`${name}`) })
? this.options.namingConvention.type(name, { model: model || this.model, inputModel: this.inputModel, reservedKeywordCallback: isReservedJavaScriptKeyword })
: name || '';
}

Expand All @@ -42,7 +42,7 @@ export abstract class JavaScriptRenderer extends AbstractRenderer<JavaScriptOpti
*/
nameProperty(propertyName: string | undefined, property?: CommonModel): string {
return this.options?.namingConvention?.property
? this.options.namingConvention.property(propertyName, { model: this.model, inputModel: this.inputModel, property, isReservedKeyword: isReservedJavaScriptKeyword(`${propertyName}`) })
? this.options.namingConvention.property(propertyName, { model: this.model, inputModel: this.inputModel, property, reservedKeywordCallback: isReservedJavaScriptKeyword })
: propertyName || '';
}

Expand Down
4 changes: 2 additions & 2 deletions src/generators/typescript/TypeScriptRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export abstract class TypeScriptRenderer extends AbstractRenderer<TypeScriptOpti
*/
nameType(name: string | undefined, model?: CommonModel): string {
return this.options?.namingConvention?.type
? this.options.namingConvention.type(name, { model: model || this.model, inputModel: this.inputModel, isReservedKeyword: isReservedTypeScriptKeyword(`${name}`)})
? this.options.namingConvention.type(name, { model: model || this.model, inputModel: this.inputModel, reservedKeywordCallback: isReservedTypeScriptKeyword })
: name || '';
}

Expand All @@ -44,7 +44,7 @@ export abstract class TypeScriptRenderer extends AbstractRenderer<TypeScriptOpti
*/
nameProperty(propertyName: string | undefined, property?: CommonModel): string {
return this.options?.namingConvention?.property
? this.options.namingConvention.property(propertyName, { model: this.model, inputModel: this.inputModel, property, isReservedKeyword: isReservedTypeScriptKeyword(`${propertyName}`) })
? this.options.namingConvention.property(propertyName, { model: this.model, inputModel: this.inputModel, property, reservedKeywordCallback: isReservedTypeScriptKeyword })
: propertyName || '';
}

Expand Down
22 changes: 12 additions & 10 deletions src/helpers/NameHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ export function getUniquePropertyName(rootModel: CommonModel, propertyName: stri
/**
* The common naming convention context type.
*/
export type CommonTypeNamingConventionCtx = { model: CommonModel, inputModel: CommonInputModel, isReservedKeyword?: boolean};
export type CommonPropertyNamingConventionCtx = { model: CommonModel, inputModel: CommonInputModel, property?: CommonModel, isReservedKeyword?: boolean};
export type CommonTypeNamingConventionCtx = { model: CommonModel, inputModel: CommonInputModel, reservedKeywordCallback?: (name: string) => boolean};
export type CommonPropertyNamingConventionCtx = { model: CommonModel, inputModel: CommonInputModel, property?: CommonModel, reservedKeywordCallback?: (name: string) => boolean};

/**
* The common naming convention type shared between generators for different languages.
Expand All @@ -44,22 +44,24 @@ export type CommonNamingConvention = {
export const CommonNamingConventionImplementation: CommonNamingConvention = {
type: (name, ctx) => {
if (!name) {return '';}
if (ctx.isReservedKeyword) {
name = `reserved_${name}`;
let formattedName = FormatHelpers.toPascalCase(name);
if (ctx.reservedKeywordCallback !== undefined && ctx.reservedKeywordCallback(formattedName)) {
formattedName = FormatHelpers.toPascalCase(`reserved_${formattedName}`);
}
return FormatHelpers.toPascalCase(name);
return formattedName;
},
property: (name, ctx) => {
if (!name) {return '';}
if (ctx.isReservedKeyword) {
let formattedName = FormatHelpers.toCamelCase(name);
if (ctx.reservedKeywordCallback !== undefined && ctx.reservedKeywordCallback(formattedName)) {
// If name is considered reserved, make sure we rename it appropriately
// and make sure no clashes occur.
name = FormatHelpers.toCamelCase(`reserved_${name}`);
if (Object.keys(ctx.model.properties || {}).includes(name)) {
formattedName = FormatHelpers.toCamelCase(`reserved_${formattedName}`);
if (Object.keys(ctx.model.properties || {}).includes(formattedName)) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return CommonNamingConventionImplementation.property!(name, ctx);
return CommonNamingConventionImplementation.property!(`reserved_${formattedName}`, ctx);
}
}
return FormatHelpers.toCamelCase(name);
return formattedName;
}
};
2 changes: 1 addition & 1 deletion test/generators/java/JavaRenderer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('JavaRenderer', () => {
});
test('should render reserved type keyword correctly', () => {
const name = renderer.nameType('enum');
expect(name).toEqual('ReservedEnum');
expect(name).toEqual('Enum');
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/generators/javascript/JavaScriptRenderer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('JavaScriptRenderer', () => {
});
test('should render reserved type keyword correctly', () => {
const name = renderer.nameType('enum');
expect(name).toEqual('ReservedEnum');
expect(name).toEqual('Enum');
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/generators/typescript/TypeScriptRenderer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('TypeScriptRenderer', () => {
});
test('should render reserved type keyword correctly', () => {
const name = renderer.nameType('enum');
expect(name).toEqual('ReservedEnum');
expect(name).toEqual('Enum');
});
});

Expand Down
9 changes: 8 additions & 1 deletion test/helpers/NameHelpers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ describe('NameHelpers', () => {
});

describe('CommonNamingConventionImplementation', () => {
const defaultCtx = {model: CommonModel.toCommonModel({}), inputModel: new CommonInputModel()};
const isReservedKeyword = jest.fn().mockReturnValue(false);
const defaultCtx = {model: CommonModel.toCommonModel({}), inputModel: new CommonInputModel(), reservedKeywordCallback: isReservedKeyword};
describe('type', () => {
test('should handle undefined', () => {
const name = undefined;
Expand All @@ -44,6 +45,12 @@ describe('NameHelpers', () => {
const formattedName = CommonNamingConventionImplementation!.property!(name, defaultCtx);
expect(formattedName).toEqual('someNotPascalString');
});
test('Should return accurate reserved property name', () => {
const name = '$ref';
isReservedKeyword.mockReturnValueOnce(true);
const formattedName = CommonNamingConventionImplementation!.property!(name, defaultCtx);
expect(formattedName).toEqual('reservedRef');
});
});
});
});

0 comments on commit 4440334

Please sign in to comment.