Accumulate ancestors in aspects, not in exported class (#267)

CHANGELOG.md

@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
 ## Version 0.23.0 - TBD
 ### Fixed
 - Plurals no longer have `is_singular` attached in the resulting .js files
+- Properties are properly propagated beyond just one level of inheritance
 
 ## Version 0.22.0 - 2024-06-20
 ### Fixed

lib/visitor.js

@@ -142,12 +142,31 @@ class Visitor {
         const file = this.fileRepository.getNamespaceFile(namespace)
         const identSingular = name => name
         const identAspect = name => `_${name}Aspect`
+        const toAspectIdent = (wrapped, [ns, n, fq]) => {
+            // types are not inflected, so don't change those to singular
+            const refersToType = isType(this.csn.inferred.definitions[fq])
+            const ident = identAspect(refersToType 
+                ? n 
+                : this.resolver.inflect({csn: this.csn.inferred.definitions[fq], plainName: n}).singular
+            )
+            return !ns || ns.isCwd(file.path.asDirectory())
+                ? `${ident}(${wrapped})`
+                : `${ns.asIdentifier()}.${ident}(${wrapped})` 
+        }
+        const ancestors = (entity.includes ?? [])
+            .map(parent => {
+                const { namespace, entityName } = this.entityRepository.getByFq(parent)
+                file.addImport(namespace)
+                return [namespace, entityName, parent]
+            })
+            .reverse() // reverse so that own aspect A is applied before extensions B,C: B(C(A(Entity)))
+            .reduce(toAspectIdent, 'Base')
 
         this.contexts.push({ entity: fq })
 
         // CLASS ASPECT
         buffer.addIndentedBlock(`export function ${identAspect(clean)}<TBase extends new (...args: any[]) => object>(Base: TBase) {`, () => {
-            buffer.addIndentedBlock(`return class ${clean} extends Base {`, () => {
+            buffer.addIndentedBlock(`return class extends ${ancestors} {`, () => {
                 const enums = []
                 for (let [ename, element] of Object.entries(entity.elements ?? {})) {
                     if (element.target && /\.texts?/.test(element.target)) {
@@ -206,31 +225,7 @@ class Visitor {
 
         // CLASS WITH ADDED ASPECTS
         file.addImport(baseDefinitions.path)
-        const ancestors = (entity.includes ?? [])
-            .map(parent => {
-                const { namespace, entityName } = this.entityRepository.getByFq(parent)
-                file.addImport(namespace)
-                return [namespace, entityName, parent]
-            })
-            .concat([[undefined, clean, fq]]) // add own aspect without namespace AFTER imports were created
-            //.concat([[undefined, clean, [namespace, clean].filter(Boolean).join('.')]]) // add own aspect without namespace AFTER imports were created
-            .reverse() // reverse so that own aspect A is applied before extensions B,C: B(C(A(Entity)))
-            .reduce((wrapped, [ns, n, fq]) => {
-                // types are not inflected, so don't change those to singular
-                const refersToType = isType(this.csn.inferred.definitions[fq])
-                const ident = identAspect(refersToType
-                    ? n
-                    : this.resolver.inflect({csn: this.csn.inferred.definitions[fq], plainName: n}).singular
-                )
-                return !ns || ns.isCwd(file.path.asDirectory())
-                    ? `${ident}(${wrapped})`
-                    : `${ns.asIdentifier()}.${ident}(${wrapped})`
-            },
-            `${baseDefinitions.path.asIdentifier()}.Entity`
-            )
-
-        buffer.add(`export class ${identSingular(clean)} extends ${ancestors} {${this.#staticClassContents(clean, entity).join('\n')}}`)
-        //buffer.add(`export type ${clean} = InstanceType<typeof ${identSingular(clean)}>`)
+        buffer.add(`export class ${identSingular(clean)} extends ${toAspectIdent(`${baseDefinitions.path.asIdentifier()}.Entity`, [undefined, clean, fq])} {${this.#staticClassContents(clean, entity).join('\n')}}`)
         this.contexts.pop()
     }
 

test/ast.js

@@ -239,7 +239,8 @@ function visitPropertyDeclaration(node) {
 function visitClassExpression(node) {
     const name = visit(node.name)
     const members = node.members.map(visit)
-    return { name, members, nodeType: kinds.ClassExpression }
+    const heritage = node.heritageClauses?.map(visit) ?? []
+    return { name, members, nodeType: kinds.ClassExpression, heritage }
 }
 
 /** @param {ts.Statement} node - the node to visit */

test/unit/files/inheritance/multilevel.cds

@@ -0,0 +1,13 @@
+namespace multilevel_inheritance;
+
+aspect A1 {
+  property1 : String(10) not null;
+}
+
+aspect A2: A1 {
+  property2 : String(20) not null;
+}
+
+entity E : A2 {
+    key ID : UUID not null;
+}

test/unit/inheritance.test.js

@@ -10,19 +10,25 @@ describe('Inheritance', () => {
     beforeAll(async () => ast = (await prepareUnitTest('inheritance/model.cds', locations.testOutput('inheritance_test'))).astw.tree)
 
     test('Entity, Type <- Entity', async () => {
-        const leaf = ast.find(n => n.name === 'LeafEntity')
+        const [leafAspect] = ast.find(n => n.name === '_LeafEntityAspect').body
+        const leaf = ast.find(n => n.name === 'LeafEntity')//'LeafEntity')
         // inherit from singular aspects
-        expect(checkInheritance(leaf, ['_AAspect', '_BAspect', '_TAspect', '_._ExtEAspect', '_._ExtTAspect', '_LeafEntityAspect'])).toBe(true)
+        expect(checkInheritance(leafAspect, ['_AAspect', '_BAspect', '_TAspect', '_._ExtEAspect', '_._ExtTAspect'])).toBe(true)
         // not from plural
-        expect(checkInheritance(leaf, ['_AAspects'])).toBe(false)
+        expect(checkInheritance(leafAspect, ['_AAspects'])).toBe(false)
+        // class only extends the aspect
+        expect(checkInheritance(leaf, ['_LeafEntityAspect'])).toBe(true)
     })
 
     test('Entity, Type <- Type', async () => {
+        const [leafAspect] = ast.find(n => n.name === '_LeafTypeAspect').body
         const leaf = ast.find(n => n.name === 'LeafType')
         // inherit from singular aspects
-        expect(checkInheritance(leaf, ['_AAspect', '_BAspect', '_TAspect', '_._ExtEAspect', '_._ExtTAspect', '_LeafTypeAspect'])).toBe(true)
+        expect(checkInheritance(leafAspect, ['_AAspect', '_BAspect', '_TAspect', '_._ExtEAspect', '_._ExtTAspect'])).toBe(true)
         // not from plural
-        expect(checkInheritance(leaf, ['_AAspects'])).toBe(false)
+        expect(checkInheritance(leafAspect, ['_AAspects'])).toBe(false)
+        // class only extends the aspect
+        expect(checkInheritance(leaf, ['_LeafTypeAspect'])).toBe(true)
     })
 
 
@@ -33,4 +39,11 @@ describe('Inheritance', () => {
     test('Extends Own Aspect (-s, No Annotation)', async () =>
         expect(checkInheritance(ast.find(n => n.name === 'Abys'), ['_AbysAspect'])).toBe(true)
     )
+
+    test('Multilevel Inheritance', async () => {
+        const ast = (await prepareUnitTest('inheritance/multilevel.cds', locations.testOutput('inheritance_test'))).astw.tree
+        // we can't really check the transitive inheritance relationship here, which would manifest in instances of E owning the properties from A1...
+        expect(checkInheritance(ast.find(n => n.name === '_EAspect').body[0], ['_A2Aspect'])).toBe(true)
+        expect(checkInheritance(ast.find(n => n.name === '_A2Aspect').body[0], ['_A1Aspect'])).toBe(true)
+    })
 })