Feat/nullable cds fields (#111)

* Add "null" to compiled TS definitions * Improve getPropertyDatatype * Implement "not null" logic for CDS Asso/Compo * Refactor getPropertyDatatype * Fix nullable parameters/returns for CDS functions/actions * Fix repeated `| not null` for CDS Asso/Compo * Reset tests and adjust to not-null * Add test suite for not null * Add changelog entry --------- Co-authored-by: Siarhei Murkou <[email protected]> Co-authored-by: Siarhei Murkou <[email protected]>
cap-js · Nov 23, 2023 · 382a806 · 382a806
1 parent 776ace5
commit 382a806
Show file tree

Hide file tree

Showing 19 changed files with 271 additions and 138 deletions.
diff --git a/.gitignore b/.gitignore
@@ -5,6 +5,7 @@ coverage
 test/**/output
 test/**/cloud-cap-samples
 .DS_Store
+.idea/
 
 # Logs
 logs

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
 - Generate `cds.LargeBinary` as string, buffer, _or readable_ in the case of media content
 
 ### Added
+- Added support for the `not null` modifier
+
 ### Fixed
 - Now using names of enum values in generated _index.js_ files if no explicit value is present
 

diff --git a/lib/components/inline.js b/lib/components/inline.js
@@ -85,6 +85,17 @@ class InlineDeclarationResolver {
         return this.visitor.options.propertiesOptional ? '?:' : ':'
     }
 
+    /**
+     * It returns TypeScript datatype for provided TS property
+     * @param {{typeName: string, typeInfo: TypeResolveInfo & { inflection: Inflection } }} type
+     * @param {string} typeName name of the TypeScript property
+     * @return {string} the datatype to be presented on TypeScript layer
+     * @public
+     */
+    getPropertyDatatype(type, typeName = type.typeName) {
+        return type.typeInfo.isNotNull ? typeName : `${typeName} | null`
+    }
+
     /** @param {import('../visitor').Visitor} visitor */
     constructor(visitor) {
         this.visitor = visitor
@@ -144,7 +155,7 @@ class FlatInlineDeclarationResolver extends InlineDeclarationResolver {
     flatten(prefix, type) {
         return type.typeInfo.structuredType
             ? Object.entries(type.typeInfo.structuredType).map(([k,v]) => this.flatten(`${this.prefix(prefix)}${k}`, v))
-            : [`${prefix}${this.getPropertyTypeSeparator()} ${type.typeName}`]
+            : [`${prefix}${this.getPropertyTypeSeparator()} ${this.getPropertyDatatype(type)}`]
     }
 
     printInlineType(name, type, buffer) {
@@ -194,9 +205,9 @@ class StructuredInlineDeclarationResolver extends InlineDeclarationResolver {
                 this.flatten(n, t, buffer)
             }
             buffer.outdent()
-            buffer.add(`}${lineEnding}`)
+            buffer.add(`}${this.getPropertyDatatype(type, '')}${lineEnding}`)
         } else {
-            buffer.add(`${name}${this.getPropertyTypeSeparator()} ${type.typeName}${lineEnding}`)
+            buffer.add(`${name}${this.getPropertyTypeSeparator()} ${this.getPropertyDatatype(type)}${lineEnding}`)
         }
         this.printDepth--
         return buffer

diff --git a/lib/components/resolver.js b/lib/components/resolver.js
@@ -23,6 +23,7 @@ const { isInlineEnumType, propertyToAnonymousEnumName } = require('./enum')
  * ```
  * @typedef {{
  *    isBuiltin: boolean,
+ *    isNotNull: boolean,
  *    isInlineDeclaration: boolean,
  *    isForeignKeyReference: boolean,
  *    isArray: boolean,
@@ -232,6 +233,8 @@ class Resolver {
 
             if (toOne && toMany) {
                 const target = element.items ?? (typeof element.target === 'string' ? { type: element.target } : element.target)
+                /** set `notNull = true` to avoid repeated `| not null` TS construction */
+                target.notNull = true
                 const { singular, plural } = this.resolveAndRequire(target, file).typeInfo.inflection
                 typeName =
                     cardinality > 1 ? toMany(plural) : toOne(this.visitor.isSelfReference(target) ? 'this' : singular)
@@ -349,11 +352,16 @@ class Resolver {
             return element
         }
 
+        const cardinality = this.getMaxCardinality(element)
+
         const result = {
             isBuiltin: false, // will be rectified in the corresponding handlers, if needed
             isInlineDeclaration: false,
             isForeignKeyReference: false,
             isArray: false,
+            isNotNull: element?.isRefNotNull !== undefined 
+                ? element?.isRefNotNull 
+                : element?.key || element?.notNull || cardinality > 1,
         }
 
         if (element?.type === undefined) {
@@ -378,6 +386,8 @@ class Resolver {
         // objects and arrays
         if (element?.items) {
             result.isArray = true
+            // TODO: re-implement this line once {element.notNull} will be provided for array-like elements
+            result.isNotNull = true
             result.isBuiltin = true
             this.resolveType(element.items, file)
             //delete element.items

diff --git a/lib/visitor.js b/lib/visitor.js
@@ -165,7 +165,10 @@ class Visitor {
                 // We don't really have to care for this case, as keys from such structs are _not_ propagated to
                 // the containing entity.
                 for (const [kname, kelement] of Object.entries(this.csn.xtended.definitions[element.target]?.keys ?? {})) {
-                    this.visitElement(`${ename}_${kname}`, kelement, file, buffer)
+                    if (this.resolver.getMaxCardinality(element) === 1) {
+                        kelement.isRefNotNull = !!element.notNull || !!element.key
+                        this.visitElement(`${ename}_${kname}`, kelement, file, buffer)
+                    }
                 }
             }
 
@@ -304,7 +307,7 @@ class Visitor {
                 .filter(([, type]) => type?.type !== '$self' && !(type.items?.type === '$self'))
                 .map(([name, type]) => [
                     name,
-                    this.resolver.resolveAndRequire(type, file).typeName,
+                    this.resolver.visitor.inlineDeclarationResolver.getPropertyDatatype(this.resolver.resolveAndRequire(type, file)),
                 ])
             : []
     }
@@ -315,7 +318,9 @@ class Visitor {
         const ns = this.resolver.resolveNamespace(name.split('.'))
         const file = this.getNamespaceFile(ns)
         const params = this.#stringifyFunctionParams(func.params, file)
-        const returns = this.resolver.resolveAndRequire(func.returns, file).typeName
+        const returns = this.resolver.visitor.inlineDeclarationResolver.getPropertyDatatype(
+            this.resolver.resolveAndRequire(func.returns, file)
+        )
         file.addFunction(name.split('.').at(-1), params, returns)
     }
 
@@ -324,7 +329,9 @@ class Visitor {
         const ns = this.resolver.resolveNamespace(name.split('.'))
         const file = this.getNamespaceFile(ns)
         const params = this.#stringifyFunctionParams(action.params, file)
-        const returns = this.resolver.resolveAndRequire(action.returns, file).typeName
+        const returns = this.resolver.visitor.inlineDeclarationResolver.getPropertyDatatype(
+            this.resolver.resolveAndRequire(action.returns, file)
+        )
         file.addAction(name.split('.').at(-1), params, returns)
     }
 

diff --git a/test/ast.js b/test/ast.js
@@ -407,7 +407,12 @@ const check = {
     isString: node => checkKeyword(node, 'string'),
     isNumber: node => checkKeyword(node, 'number'),
     isAny: node => checkKeyword(node, 'any'),
-    isStatic: node => checkKeyword(node, 'static')
+    isStatic: node => checkKeyword(node, 'static'),
+    isIndexedAccessType: node => checkKeyword(node, 'indexedaccesstype'),
+    isNull: node => checkKeyword(node, 'literaltype') && checkKeyword(node.literal, 'null'),
+    isUnionType: (node, of = []) => checkKeyword(node, 'uniontype') 
+        && of.reduce((acc, predicate) => acc && node.subtypes.some(st => predicate(st)), true),
+    isNullable: (node, of = []) => check.isUnionType(node, of.concat([check.isNull])),
 }
 
 

diff --git a/test/unit/actions.test.js b/test/unit/actions.test.js
@@ -16,13 +16,15 @@ describe('Actions', () => {
         const actions = astw.getAspectProperty('_EAspect', 'actions')
         expect(actions.modifiers.some(check.isStatic)).toBeTruthy()
         checkFunction(actions.type.members.find(fn => fn.name === 'f'), {
-            parameterCheck: ({members: [fst]}) => fst.name === 'x' && check.isString(fst.type)
+            parameterCheck: ({members: [fst]}) => fst.name === 'x' && check.isNullable(fst.type, [check.isString])
         })
         checkFunction(actions.type.members.find(fn => fn.name === 'g'), {
             parameterCheck: ({members: [fst, snd]}) => {
-                const fstCorrect = fst.name === 'a' && fst.type.members[0].name === 'x' && check.isNumber(fst.type.members[0].type)
-                    && fst.type.members[1].name === 'y' && check.isNumber(fst.type.members[1].type)
-                const sndCorrect = snd.name === 'b' && check.isNumber(snd.type)
+                const fstCorrect = fst.name === 'a' 
+                    && fst.type.subtypes[0].members[0].name === 'x' 
+                    && check.isNullable(fst.type.subtypes[0].members[0].type, [check.isNumber])
+                    && fst.type.subtypes[0].members[1].name === 'y' && check.isNullable(fst.type.subtypes[0].members[1].type, [check.isNumber])
+                const sndCorrect = snd.name === 'b' && check.isNullable(snd.type, [check.isNumber])
                 return fstCorrect && sndCorrect
             }
         })
@@ -33,11 +35,17 @@ describe('Actions', () => {
         const ast = new ASTWrapper(path.join(paths[2], 'index.ts')).tree
         checkFunction(ast.find(node => node.name === 'free'), {
             modifiersCheck: (modifiers = []) => !modifiers.some(check.isStatic),
-            callCheck: ({members: [fst, snd]}) => fst.name === 'a' && check.isNumber(fst.type)
-                && snd.name === 'b' && check.isString(snd.type),
-            parameterCheck: ({members: [fst]}) => fst.name === 'param' && check.isString(fst.type),
-            returnTypeCheck: ({members: [fst, snd]}) => fst.name === 'a' && check.isNumber(fst.type)
-                && snd.name === 'b' && check.isString(snd.type)
+            callCheck: type => check.isNullable(type) 
+                && type.subtypes[0].members[0].name === 'a'
+                && check.isNullable(type.subtypes[0].members[0].type, [check.isNumber])
+                && type.subtypes[0].members[1].name === 'b'
+                && check.isNullable(type.subtypes[0].members[1].type, [check.isString]),
+            parameterCheck: ({members: [fst]}) => fst.name === 'param' && check.isNullable(fst.type, [check.isString]),
+            returnTypeCheck: type => check.isNullable(type) 
+                && type.subtypes[0].members[0].name === 'a'
+                && check.isNullable(type.subtypes[0].members[0].type, [check.isNumber])
+                && type.subtypes[0].members[1].name === 'b'
+                && check.isNullable(type.subtypes[0].members[1].type, [check.isString]),
         })
     })
 
@@ -48,7 +56,7 @@ describe('Actions', () => {
         expect(actions.modifiers.some(check.isStatic)).toBeTruthy()
         checkFunction(actions.type.members.find(fn => fn.name === 'f'), {
             callCheck: signature => check.isAny(signature),
-            parameterCheck: ({members: [fst]}) => fst.name === 'x' && check.isString(fst.type),
+            parameterCheck: ({members: [fst]}) => fst.name === 'x' && check.isNullable(fst.type, [check.isString]),
             returnTypeCheck: returns => check.isAny(returns)
         })
 
@@ -69,14 +77,14 @@ describe('Actions', () => {
 
         checkFunction(ast.find(node => node.name === 'free2'), {
             modifiersCheck: (modifiers = []) => !modifiers.some(check.isStatic),
-            callCheck: ({full}) => full === '_elsewhere.ExternalType',
-            returnTypeCheck: ({full}) => full === '_elsewhere.ExternalType'
+            callCheck: type => check.isNullable(type, [t => t?.full === '_elsewhere.ExternalType']),
+            returnTypeCheck: type => check.isNullable(type, [t => t?.full === '_elsewhere.ExternalType'])
         })
 
         checkFunction(ast.find(node => node.name === 'free3'), {
             modifiersCheck: (modifiers = []) => !modifiers.some(check.isStatic),
-            callCheck: ({full}) => full === '_.ExternalInRoot',
-            returnTypeCheck: ({full}) => full === '_.ExternalInRoot'
+            callCheck: type => check.isNullable(type, [t => t?.full === '_.ExternalInRoot']),
+            returnTypeCheck: type => check.isNullable(type, [t => t?.full === '_.ExternalInRoot'])
         })
     })
 
@@ -101,7 +109,7 @@ describe('Actions', () => {
         checkFunction(actions.type.members.find(fn => fn.name === 'sx'), {
             callCheck: signature => check.isAny(signature),
             returnTypeCheck: returns => check.isAny(returns),
-            parameterCheck: ({members: [fst]}) => check.isNumber(fst.type)
+            parameterCheck: ({members: [fst]}) => check.isNullable(fst.type, [check.isNumber])
         })
     })
 

diff --git a/test/unit/enum.test.js b/test/unit/enum.test.js
@@ -3,7 +3,7 @@
 const fs = require('fs').promises
 const path = require('path')
 const cds2ts = require('../../lib/compile')
-const { ASTWrapper } = require('../ast')
+const { ASTWrapper, check } = require('../ast')
 const { locations } = require('../util')
 
 const dir = locations.testOutput('enums_test')
@@ -31,7 +31,7 @@ describe('Enum Types', () => {
 
             test('Referring Property', async () =>
                 expect(ast.getAspects().find(({name, members}) => name === '_InlineEnumAspect'
-                && members?.find(member => member.name === 'gender' && member.type?.full === 'InlineEnum_gender')))
+                && members?.find(member => member.name === 'gender' && check.isNullable(member.type, [t => t?.full === 'InlineEnum_gender']))))
                 .toBeTruthy())
 
         })
@@ -47,7 +47,7 @@ describe('Enum Types', () => {
 
             test('Referring Property', async () =>
                 expect(ast.getAspects().find(({name, members}) => name === '_InlineEnumAspect'
-                && members?.find(member => member.name === 'status' && member.type?.full === 'InlineEnum_status')))
+                && members?.find(member => member.name === 'status' && check.isNullable(member.type, [t => t?.full === 'InlineEnum_status']))))
                 .toBeTruthy())
         })
 
@@ -62,7 +62,7 @@ describe('Enum Types', () => {
 
             test('Referring Property', async () =>
                 expect(ast.getAspects().find(({name, members}) => name === '_InlineEnumAspect'
-                && members?.find(member => member.name === 'yesno' && member.type?.full === 'InlineEnum_yesno')))
+                && members?.find(member => member.name === 'yesno' &&  check.isNullable(member.type, [t => t?.full === 'InlineEnum_yesno']))))
                 .toBeTruthy())
         })
     })

diff --git a/test/unit/events.test.js b/test/unit/events.test.js
@@ -3,7 +3,7 @@
 const fs = require('fs').promises
 const path = require('path')
 const cds2ts = require('../../lib/compile')
-const { ASTWrapper } = require('../ast')
+const { ASTWrapper, check } = require('../ast')
 const { locations } = require('../util')
 
 const dir = locations.testOutput('events_test')
@@ -25,8 +25,8 @@ describe('events', () => {
         test('Top Level Event', async () => {
             expect(ast.tree.find(cls => cls.name === 'Bar' 
                 && cls.members.length === 2
-                && cls.members[0].name === 'id' && cls.members[0].type.keyword === 'number'
-                && cls.members[1].name === 'name' && cls.members[1].type.keyword === 'indexedaccesstype'
+                && cls.members[0].name === 'id' && check.isNullable(cls.members[0].type, [check.isNumber])
+                && cls.members[1].name === 'name' && check.isNullable(cls.members[1].type, [check.isIndexedAccessType])
             )).toBeTruthy()
         })
     })

diff --git a/test/unit/files/arrayof/model.cds b/test/unit/files/arrayof/model.cds
@@ -11,4 +11,4 @@ entity E {
     inlinez: array of { a: String; b: Integer; }
 }
 
-function fn (xs: array of Integer) returns array of String;
+function fn (xs: array of Integer) returns array of String;
diff --git a/test/unit/files/enums/model.cds b/test/unit/files/enums/model.cds
@@ -32,4 +32,4 @@ entity ExternalEnums {
     status : Status;
     gender: Gender;
     yesno: Truthy;
-}
+}
diff --git a/test/unit/files/notnull/model.cds b/test/unit/files/notnull/model.cds
@@ -0,0 +1,19 @@
+namespace not_null;
+
+entity Foo {}
+
+entity E {
+    x: Integer not null;
+    foo_assoc: Association to Foo not null;
+    foos_assoc: Association to many Foo not null;
+    foo_comp: Composition of Foo not null;
+    foos_comp: Composition of many Foo not null;
+    inline: {
+        a: Integer not null
+    } not null
+}
+    actions {
+        action f (x: String not null);
+    }
+
+action free (param: String not null) returns Integer not null;
diff --git a/test/unit/files/typeof/model.cds b/test/unit/files/typeof/model.cds
@@ -13,4 +13,4 @@ entity Bar {
     ref_a: Foo:a;
     ref_b: Foo:b;
     ref_c: Foo:c.x;
-};
+};
diff --git a/test/unit/hana.test.js b/test/unit/hana.test.js
@@ -3,7 +3,7 @@
 const path = require('path')
 const cds2ts = require('../../lib/compile')
 const { locations } = require('../util')
-const { ASTWrapper } = require('../ast')
+const { ASTWrapper, check } = require('../ast')
 
 const dir = locations.testOutput('hana_files')
 
@@ -33,18 +33,18 @@ describe('Builtin HANA Datatypes', () => {
         })
 
         test('Types Correct', () => {
-            ast.exists('_EverythingAspect', 'bar', 'string')
-            ast.exists('_EverythingAspect', 'smallint', 'SMALLINT')
-            ast.exists('_EverythingAspect', 'tinyint', 'TINYINT')
-            ast.exists('_EverythingAspect', 'smalldecimal', 'SMALLDECIMAL')
-            ast.exists('_EverythingAspect', 'real', 'REAL')
-            ast.exists('_EverythingAspect', 'char', 'CHAR')
-            ast.exists('_EverythingAspect', 'nchar', 'NCHAR')
-            ast.exists('_EverythingAspect', 'varchar', 'VARCHAR')
-            ast.exists('_EverythingAspect', 'clob', 'CLOB')
-            ast.exists('_EverythingAspect', 'binary', 'BINARY')
-            ast.exists('_EverythingAspect', 'point', 'ST_POINT')
-            ast.exists('_EverythingAspect', 'geometry', 'ST_GEOMETRY')
-            ast.exists('_EverythingAspect', 'shorthand', 'REAL')
+            ast.exists('_EverythingAspect', 'bar', ({type}) => check.isNullable(type, [check.isString]))
+            ast.exists('_EverythingAspect', 'smallint', ({type}) => check.isNullable(type, [t => t.name === 'SMALLINT']))
+            ast.exists('_EverythingAspect', 'tinyint', ({type}) => check.isNullable(type, [t => t.name === 'TINYINT']))
+            ast.exists('_EverythingAspect', 'smalldecimal', ({type}) => check.isNullable(type, [t => t.name === 'SMALLDECIMAL']))
+            ast.exists('_EverythingAspect', 'real', ({type}) => check.isNullable(type, [t => t.name === 'REAL']))
+            ast.exists('_EverythingAspect', 'char', ({type}) => check.isNullable(type, [t => t.name === 'CHAR']))
+            ast.exists('_EverythingAspect', 'nchar', ({type}) => check.isNullable(type, [t => t.name === 'NCHAR']))
+            ast.exists('_EverythingAspect', 'varchar', ({type}) => check.isNullable(type, [t => t.name === 'VARCHAR']))
+            ast.exists('_EverythingAspect', 'clob', ({type}) => check.isNullable(type, [t => t.name === 'CLOB']))
+            ast.exists('_EverythingAspect', 'binary', ({type}) => check.isNullable(type, [t => t.name === 'BINARY']))
+            ast.exists('_EverythingAspect', 'point', ({type}) => check.isNullable(type, [t => t.name === 'ST_POINT']))
+            ast.exists('_EverythingAspect', 'geometry', ({type}) => check.isNullable(type, [t => t.name === 'ST_GEOMETRY']))
+            ast.exists('_EverythingAspect', 'shorthand', ({type}) => check.isNullable(type, [t => t.name === 'REAL']))
         })
 })
-Original file line number
+Diff line change
@@ Expand Up / @@ -5,6 +5,7 @@ coverage @@
     test/**/output
     test/**/cloud-cap-samples
     .DS_Store
+    .idea/
     # Logs
     logs
@@ Expand Down @@