Skip to content

Commit

Permalink
Fix/inline enums in types (#150)
Browse files Browse the repository at this point in the history
* Handle enums and other type definitios differently

* Rewrite

* Treat structured types differently instead, fix test

* Fix test

* Add changelog entry
  • Loading branch information
daogrady authored Feb 1, 2024
1 parent 1abce09 commit d981f76
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
### Changed
- Changed default log level from `NONE` to `ERROR`. See the doc to manually pass in another log level for cds-typer runs
- Name collisions between automatically generated foreign key fields (`.…_ID`, `.…_code`, etc.) with explicitly named fields will now raise an error
- Generate CDS types that are actually structured types as if they were entities. This allows the correct representation of mixing aspects and types in CDS inheritance, and also fixes issues with inline enums in such types

### Fixed
- Externally defined enums can now be used as parameter types in actions
Expand Down
10 changes: 9 additions & 1 deletion lib/components/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,15 @@ class Resolver {
if (isInlineEnumType(element, this.csn)) {
// element.parent is only set if the enum is attached to an entity's property.
// If it is missing then we are dealing with an inline parameter type of an action.
if (element.parent) {
// Edge case: element.parent is set, but no .name property is attached. This happens
// for inline enums inside types:
// ```cds
// type T {
// x : String enum { ... }; // no element.name for x
// }
// ```
// In that case, we currently resolve to the more general type (cds.String, here)
if (element.parent?.name) {
result.isInlineDeclaration = true
// we use the singular as the initial declaration of these enums takes place
// while defining the singular class. Which therefore uses the singular over the plural name.
Expand Down
10 changes: 7 additions & 3 deletions lib/visitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -446,12 +446,16 @@ class Visitor {
case 'function':
this.#printAction(name, entity)
break
case 'type':
this.#printType(name, entity)
break
case 'aspect':
this.#printAspect(name, entity)
break
case 'type': {
// types like inline definitions can be used very similarly to entities.
// They can be extended, contain inline enums, etc., so we treat them as entities.
const handler = entity.elements ? this.#printEntity : this.#printType
handler.bind(this)(name, entity)
break
}
case 'event':
this.#printEvent(name, entity)
break
Expand Down
File renamed without changes.
10 changes: 10 additions & 0 deletions test/unit/enum.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ describe('Enum Types', () => {
})

describe('Anonymous', () => {
describe('Within type Definition', () => {
test('Property References Artificially Named Enum', () => {
astw.exists(
'_TypeWithInlineEnumAspect',
'inlineEnumProperty',
p => check.isNullable(p.type, [t => check.isTypeReference(t, 'TypeWithInlineEnum_inlineEnumProperty')])
)
})
})

describe('String Enum', () => {
test('Definition Present', async () =>
expect(astw.tree.find(n => n.name === 'InlineEnum_gender'
Expand Down
5 changes: 5 additions & 0 deletions test/unit/files/enums/model.cds
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ type Status : Integer enum {
unknown = 0;
cancelled = -1;
}
type TypeWithInlineEnum {
inlineEnumProperty: String enum {
foo; bar
}
}

entity InlineEnums {
gender: String enum {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/type.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('type Definitions', () => {

test('All Definitions Present', async () => {
expect(ast.tree.find(({name, nodeType}) => name === 'IntAlias' && nodeType === 'typeAliasDeclaration')).toBeTruthy()
expect(ast.tree.find(({name, nodeType}) => name === 'Points' && nodeType === 'typeAliasDeclaration')).toBeTruthy()
expect(ast.tree.find(({name, nodeType}) => name === 'Points' && nodeType === 'classDeclaration')).toBeTruthy()
expect(ast.tree.find(({name, nodeType}) => name === 'Lines' && nodeType === 'typeAliasDeclaration')).toBeTruthy()
})

Expand Down

0 comments on commit d981f76

Please sign in to comment.