From 07dbaf8f09c317eb48e089d561a09fcfccc321ae Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sat, 5 Oct 2024 17:35:27 +0200 Subject: [PATCH 01/24] refactor: overhaul/fix creation of `drafts` properties in entities --- eslint.config.js | 3 + lib/csn.js | 250 ++++++++---------- lib/visitor.js | 18 +- test/unit/draft.test.js | 69 ++--- .../files/draft/catalog-service-error.cds | 10 + test/unit/files/draft/catalog-service.cds | 3 +- test/unit/files/draft/data-model.cds | 7 +- test/unit/files/draft/model.cds | 51 ---- test/util.js | 3 +- 9 files changed, 152 insertions(+), 262 deletions(-) create mode 100644 test/unit/files/draft/catalog-service-error.cds delete mode 100644 test/unit/files/draft/model.cds diff --git a/eslint.config.js b/eslint.config.js index 0cca5c24..0f74f434 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -13,6 +13,9 @@ module.exports = [ ...require('globals').node } }, + env: { + jest: true + }, files: ['**/*.js'], plugins: { '@stylistic/js': require('@stylistic/eslint-plugin-js') diff --git a/lib/csn.js b/lib/csn.js index 837951fe..93c0f714 100644 --- a/lib/csn.js +++ b/lib/csn.js @@ -1,4 +1,8 @@ -const annotation = '@odata.draft.enabled' +const { LOG } = require('./logging') + +const DRAFT_ENABLED_ANNO = '@odata.draft.enabled' +/** @type {string[]} */ +const draftEnabledEntities = [] /** @typedef {import('./typedefs').resolver.CSN} CSN */ /** @typedef {import('./typedefs').resolver.EntityCSN} EntityCSN */ @@ -40,9 +44,9 @@ const isUnresolved = entity => entity._unresolved === true const isCsnAny = entity => entity?.constructor?.name === 'any' /** - * @param {EntityCSN} entity - the entity + * @param {string} fq - the fqn of an entity */ -const isDraftEnabled = entity => entity['@odata.draft.enabled'] === true +const isDraftEnabled = fq => draftEnabledEntities.includes(fq) /** * @param {EntityCSN} entity - the entity @@ -87,183 +91,144 @@ const getProjectionTarget = entity => isProjection(entity) ? entity.projection?.from?.ref?.[0] : undefined -class DraftUnroller { - /** @type {Set} */ - #positives = new Set() - /** @type {{[key: string]: boolean}} */ - #draftable = {} - /** @type {{[key: string]: string}} */ - #projections = {} +class DraftEnabledEntityCollector { /** @type {EntityCSN[]} */ - #entities = [] + #draftRoots = [] + /** @type {string[]} */ + #serviceNames = [] /** @type {CSN | undefined} */ #csn - set csn(c) { - this.#csn = c - if (c === undefined) return - this.#entities = Object.values(c.definitions) - this.#projections = this.#entities.reduce((pjs, entity) => { - if (isProjection(entity)) { - // @ts-ignore - we know that entity is a projection here - pjs[entity.name] = getProjectionTarget(entity) - } - return pjs - }, {}) - } - get csn() { return this.#csn } + #compileError = false /** - * @param {EntityCSN | string} entityOrFq - entity to set draftable annotation for. - * @param {boolean} value - whether the entity is draftable. + * @returns {string[]} */ - #setDraftable(entityOrFq, value) { - const entity = typeof entityOrFq === 'string' - ? this.#getDefinition(entityOrFq) - : entityOrFq - if (!entity) return // inline definition -- not found in definitions - entity[annotation] = value - this.#draftable[entity.name] = value - if (value) { - this.#positives.add(entity.name) - } else { - this.#positives.delete(entity.name) - } + #getServiceNames() { + return Object.values(this.#csn?.definitions ?? {}).filter(d => d.kind === 'service').map(d => d.name) } /** - * @param {EntityCSN | string} entityOrFq - entity to look draftability up for. - * @returns {boolean} + * @returns {EntityCSN[]} */ - #getDraftable(entityOrFq) { - const entity = typeof entityOrFq === 'string' - ? this.#getDefinition(entityOrFq) - : entityOrFq - // assert(typeof entity !== 'string') - const name = entity?.name ?? entityOrFq - // @ts-expect-error - .name not being present means entityOrFq is a string, so name is always a string and therefore a valid index - return this.#draftable[name] ??= this.#propagateInheritance(entity) + #collectDraftRoots() { + return Object.values(this.#csn?.definitions ?? {}).filter( + d => isEntity(d) && this.#isDraftEnabled(d) && this.#isPartOfAnyService(d.name) + ) } /** - * FIXME: could use EntityRepository here - * @param {string} name - name of the entity. - * @returns {EntityCSN} + * @param {string} entityName - entity to check + * @returns {boolean} `true` if entity is part an service */ - // @ts-expect-error - poor man's #getDefinitionOrThrow. We are always sure name is a valid key - #getDefinition(name) { return this.csn?.definitions[name] } - - /** - * Propagate draft annotations through inheritance (includes). - * The latest annotation through the inheritance chain "wins". - * Annotations on the entity itself are always queued last, so they will always be decisive over ancestors. - * @param {EntityCSN | undefined} entity - entity to pull draftability from its parents. - */ - #propagateInheritance(entity) { - if (!entity) return - /** @type {(boolean | undefined)[]} */ - const annotations = (entity.includes ?? []).map(parent => this.#getDraftable(parent)) - annotations.push(entity[annotation]) - this.#setDraftable(entity, annotations.filter(a => a !== undefined).at(-1) ?? false) + #isPartOfAnyService(entityName) { + return this.#serviceNames.some(s => entityName.startsWith(s)) } /** - * Propagate draft-enablement through projections. + * Collect all entities that are transitively reachable via compositions from `entity` into `draftNodes`. + * Check that no entity other than the root node has `@odata.draft.enabled` + * @param {EntityCSN} entity - + * @param {string} entityName - + * @param {EntityCSN} rootEntity - root entity where composition traversal started. + * @param {Record} draftEntities - Dictionary of entitys */ - #propagateProjections() { - /** - * @param {string} from - entity to propagate draftability from. - * @param {string} to - entity to propagate draftability to. - */ - const propagate = (from, to) => { - do { - this.#setDraftable(to, this.#getDraftable(to) || this.#getDraftable(from)) - from = to - to = this.#projections[to] - } while (to) - } + #collectDraftNodesInto(entity, entityName, rootEntity, draftEntities) { + draftEntities[entityName] = entity - for (let [projection, target] of Object.entries(this.#projections)) { - propagate(projection, target) - propagate(target, projection) + for (const elem of Object.values(entity.elements ?? {})) { + if (!elem.target || elem.type !== 'cds.Composition') continue + + const draftNode = this.#csn?.definitions[elem.target] + const draftNodeName = elem.target + + if (!draftNode) { + throw new Error(`Expecting target to be resolved: ${JSON.stringify(elem, null, 2)}`) + } + + if (!this.#isPartOfAnyService(draftNodeName)) { + LOG.warn(`Ignoring draft node for composition target ${draftNodeName} because it is not part of a service`) + continue + } + + if (draftNode !== rootEntity && this.#isDraftEnabled(draftNode)) { + this.#compileError = true + LOG.error(`Composition in draft-enabled entity can't lead to another entity with "@odata.draft.enabled" (in entity: "${entityName}"/element: ${elem.name})!`) + delete draftEntities[draftNodeName] + continue + } + + if (!this.#isDraftEnabled(draftNode) && !draftEntities[draftNodeName]) { + this.#collectDraftNodesInto(draftNode, draftNodeName, rootEntity, draftEntities) + } } } /** - * If an entity E is draftable and contains any composition of entities, - * then those entities also become draftable. Recursively. - * @param {EntityCSN} entity - entity to propagate all compositions from. + * @param {EntityCSN} entity - entity to check + * @returns {boolean} */ - #propagateCompositions(entity) { - if (!this.#getDraftable(entity)) return - - for (const comp of Object.values(entity.compositions ?? {})) { - const target = this.#getDefinition(comp.target) - const current = this.#getDraftable(target) - if (!current) { - this.#setDraftable(target, true) - this.#propagateCompositions(target) - } - } + #isDraftEnabled(entity) { + return entity[DRAFT_ENABLED_ANNO] === true } /** @param {CSN} csn - the full csn */ - unroll(csn) { - this.csn = csn + run(csn) { + if (!csn) return - // inheritance - for (const entity of this.#entities) { - this.#propagateInheritance(entity) - } + this.#csn = csn + this.#serviceNames = this.#getServiceNames() + this.#draftRoots = this.#collectDraftRoots() - // transitivity through compositions - // we have to do this in a second pass, as we only now know which entities are draft-enables themselves - for (const entity of this.#entities) { - this.#propagateCompositions(entity) - } + for (const draftRoot of this.#draftRoots) { + /** @type {Record} */ + const draftEntities = {} + this.#collectDraftNodesInto(draftRoot, draftRoot.name, draftRoot, draftEntities) - this.#propagateProjections() + for (const draftNode of Object.values(draftEntities)) { + draftEnabledEntities.push(draftNode.name) + } + } + if (this.#compileError) throw new Error('Compilation of model failed') } } // note to self: following doc uses @ homoglyph instead of @, as the latter apparently has special semantics in code listings /** - * We are unrolling the @odata.draft.enabled annotations into related entities manually. - * This includes three scenarios: + * We collect all entities that are draft enabled. + * (@see `@sap/cds-compiler/lib/transform/draft/db.js#generateDraft`) * - * (a) aspects via `A: B`, where `B` is draft enabled. - * Note that when an entity extends two other entities of which one has drafts enabled and - * one has not, then the one that is later in the list of mixins "wins": + * This includes thwo scenarios: + * - (a) Entities that are part of a service and have the annotation @odata.draft.enabled + * - (b) Entities that are draft enabled propagate this property down through compositions. + * NOTE: The compositions themselves must not be draft enabled, otherwise no draft entity will be generated for them * @param {any} csn - the entity * @example - * ```ts - * @odata.draft.enabled true - * entity T {} - * @odata.draft.enabled false - * entity F {} - * entity A: T,F {} // draft not enabled - * entity B: F,T {} // draft enabled - * ``` + * (a) + * ```cds + * // service.cds + * service MyService { + * @odata.draft.enabled true + * entity A {} * - * (b) Draft enabled projections make the entity we project on draft enabled. - * @example - * ```ts - * @odata.draft.enabled: true - * entity A as projection on B {} - * entity B {} // draft enabled + * @odata.draft.enabled true + * entity B {} + * } * ``` - * - * (c) Entities that are draft enabled propagate this property down through compositions: - * - * ```ts - * @odata.draft.enabled: true - * entity A { - * b: Composition of B + * @example + * (b) + * ```cds + * // service.cds + * service MyService { + * @odata.draft.enabled: true + * entity A { + * b: Composition of B + * } + * entity B {} // draft enabled * } - * entity B {} // draft enabled * ``` */ -function unrollDraftability(csn) { - new DraftUnroller().unroll(csn) +function collectDraftEnabledEntities(csn) { + new DraftEnabledEntityCollector().run(csn) } /** @@ -320,15 +285,6 @@ function propagateForeignKeys(csn) { } } -/** - * - * @param {any} csn - complete csn - */ -function amendCSN(csn) { - unrollDraftability(csn) - propagateForeignKeys(csn) -} - /** * @param {EntityCSN} entity - the entity */ @@ -349,7 +305,7 @@ const getProjectionAliases = entity => { } module.exports = { - amendCSN, + collectDraftEnabledEntities, isView, isProjection, isViewOrProjection, diff --git a/lib/visitor.js b/lib/visitor.js index 3ffde294..5b172af6 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -2,7 +2,7 @@ const util = require('./util') -const { amendCSN, isView, isUnresolved, propagateForeignKeys, isDraftEnabled, isType, isProjection, getMaxCardinality, isViewOrProjection, isEnum } = require('./csn') +const { isView, isUnresolved, propagateForeignKeys, collectDraftEnabledEntities, isDraftEnabled, isType, isProjection, getMaxCardinality, isViewOrProjection, isEnum } = require('./csn') // eslint-disable-next-line no-unused-vars const { SourceFile, FileRepository, Buffer, Path } = require('./file') const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline') @@ -40,8 +40,10 @@ class Visitor { * @param {{xtended: CSN, inferred: CSN}} csn - root CSN */ constructor(csn) { - amendCSN(csn.xtended) + propagateForeignKeys(csn.xtended) propagateForeignKeys(csn.inferred) + // has to be executed on the inferred model as autoexposed entities are not included in the xtended csn + collectDraftEnabledEntities(csn.inferred) this.csn = csn /** @type {Context[]} **/ @@ -283,16 +285,17 @@ class Visitor { // CLASS WITH ADDED ASPECTS file.addImport(baseDefinitions.path) docify(entity.doc).forEach(d => { buffer.add(d) }) - buffer.add(`export class ${identSingular(clean)} extends ${identAspect(toLocalIdent({clean, fq}))}(${baseDefinitions.path.asIdentifier()}.Entity) {${this.#staticClassContents(clean, entity).join('\n')}}`) + buffer.add(`export class ${identSingular(clean)} extends ${identAspect(toLocalIdent({clean, fq}))}(${baseDefinitions.path.asIdentifier()}.Entity) {${this.#staticClassContents(fq, clean, entity).join('\n')}}`) this.contexts.pop() } /** + * @param {string} fq - fully qualified name of the entity * @param {string} clean - the clean name of the entity * @param {EntityCSN} entity - the entity to generate the static contents for */ - #staticClassContents(clean, entity) { - return isDraftEnabled(entity) ? [`static drafts: typeof ${clean}`] : [] + #staticClassContents(fq, clean, entity) { + return isDraftEnabled(fq) ? [`static drafts: typeof ${clean}`] : [] } /** @@ -348,9 +351,6 @@ class Visitor { ? this.csn.inferred.definitions[fq] : entity - // draft enablement is stored in csn.xtended. Iff we took the entity from csn.inferred, we have to carry the draft-enablement over at this point - target['@odata.draft.enabled'] = isDraftEnabled(entity) - this.#aspectify(fq, target, buffer, { cleanName: singular }) buffer.add(overrideNameProperty(singular, entity.name)) @@ -366,7 +366,7 @@ class Visitor { } // plural can not be a type alias to $singular[] but needs to be a proper class instead, // so it can get passed as value to CQL functions. - const additionalProperties = this.#staticClassContents(singular, entity) + const additionalProperties = this.#staticClassContents(fq, singular, entity) additionalProperties.push('$count?: number') docify(entity.doc).forEach(d => { buffer.add(d) }) buffer.add(`export class ${plural} extends Array<${singular}> {${additionalProperties.join('\n')}}`) diff --git a/test/unit/draft.test.js b/test/unit/draft.test.js index 74c63216..ebe97651 100644 --- a/test/unit/draft.test.js +++ b/test/unit/draft.test.js @@ -1,7 +1,6 @@ 'use strict' const path = require('path') -const { describe, beforeAll, test, expect } = require('@jest/globals') const { ASTWrapper } = require('../ast') const { locations, prepareUnitTest } = require('../util') @@ -9,65 +8,31 @@ const draftable_ = (entity, ast) => ast.find(n => n.name === entity && n.members const draftable = (entity, ast, plural = e => `${e}_`) => draftable_(entity, ast) && draftable_(plural(entity), ast) describe('bookshop', () => { - test('Projections Up and Down', async () => { + test('Draft via root and compositions', async () => { const paths = (await prepareUnitTest('draft/catalog-service.cds', locations.testOutput('bookshop_projection'))).paths const service = new ASTWrapper(path.join(paths[1], 'index.ts')).tree const model = new ASTWrapper(path.join(paths[2], 'index.ts')).tree + // root and composition become draft enabled expect(draftable('Book', service, () => 'Books')).toBeTruthy() expect(draftable('Publisher', service, () => 'Publishers')).toBeTruthy() - expect(draftable('Book', model, () => 'Books')).toBeTruthy() - expect(draftable('Publisher', model, () => 'Publishers')).toBeTruthy() + + // associated entity will not become draft enabled + expect(draftable('Author', service, () => 'Authors')).toBeFalsy() + + // non-service entities will not be draft enabled + expect(draftable('Book', model, () => 'Books')).toBeFalsy() + expect(draftable('Publisher', model, () => 'Publishers')).toBeFalsy() + expect(draftable('Author', model, () => 'Authors')).toBeFalsy() }) -}) -describe('@odata.draft.enabled', () => { - let ast + test('Draft-enabled composition produces compiler error', async () => { + const spyOnConsole = jest.spyOn(console, 'error') + await prepareUnitTest('draft/catalog-service-error.cds', locations.testOutput('bookshop_projection'), {typerOptions: {logLevel: 'ERROR'}}) - beforeAll(async () => ast = (await prepareUnitTest('draft/model.cds', locations.testOutput('draft_test'))).astw.tree) - - test('Direct Annotation', async () => expect(draftable('A', ast)).toBeTruthy()) - - test('First Level Inheritance', async () => expect(draftable('B', ast)).toBeTruthy()) - - test('Explicit Override via Inheritance', async () => expect(draftable('C', ast)).not.toBeTruthy()) - - test('Inheritance of Explicit Override', async () => expect(draftable('D', ast)).not.toBeTruthy()) - - test('Declaration With true', async () => expect(draftable('E', ast)).toBeTruthy()) - - test('Multiple Inheritance With Most Significant true', async () => expect(draftable('F', ast)).toBeTruthy()) - - test('Multiple Inheritance With Most Significant false', async () => expect(draftable('G', ast)).not.toBeTruthy()) - - test('Draftable by Association/ Composition', async () => { - expect(draftable('H', ast)).not.toBeTruthy() - expect(draftable('I', ast)).not.toBeTruthy() - expect(draftable('J', ast)).not.toBeTruthy() - expect(draftable('K', ast)).not.toBeTruthy() - }) - - test('Unchanged by Association/ Composition', async () => { - expect(draftable('L', ast)).not.toBeTruthy() - expect(draftable('M', ast)).not.toBeTruthy() - }) - - test('Precedence Over Explicit Annotation', async () => { - expect(draftable('P', ast)).toBeTruthy() - expect(draftable('Q', ast)).toBeTruthy() - }) - - test('Via Projection', async () => expect(draftable('PA', ast)).toBeTruthy()) - - test('Transitive Via Projection and Composition', async () => { - expect(draftable('ProjectedReferrer', ast)).toBeTruthy() - expect(draftable('Referrer', ast)).toBeTruthy() - expect(draftable('Referenced', ast)).toBeTruthy() - }) - - test('Transitive Via Multiple Levels of Projection', async () => { - expect(draftable('Foo', ast)).toBeTruthy() - expect(draftable('ProjectedFoo', ast)).toBeTruthy() - expect(draftable('ProjectedProjectedFoo', ast)).toBeTruthy() + expect(spyOnConsole).toHaveBeenCalledWith( + '[cds-typer] -', + 'Composition in draft-enabled entity can\'t lead to another entity with "@odata.draft.enabled" (in entity: "bookshop.service.CatalogService.Books"/element: publishers)!' + ) }) }) \ No newline at end of file diff --git a/test/unit/files/draft/catalog-service-error.cds b/test/unit/files/draft/catalog-service-error.cds new file mode 100644 index 00000000..2be300e6 --- /dev/null +++ b/test/unit/files/draft/catalog-service-error.cds @@ -0,0 +1,10 @@ +namespace bookshop.service; + +using bookshop as my from './data-model'; + +service CatalogService { + @odata.draft.enabled + entity Books as projection on my.Books; + @odata.draft.enabled + entity Publishers as projection on my.Publishers; +} \ No newline at end of file diff --git a/test/unit/files/draft/catalog-service.cds b/test/unit/files/draft/catalog-service.cds index 75e3d7d2..59ab2157 100644 --- a/test/unit/files/draft/catalog-service.cds +++ b/test/unit/files/draft/catalog-service.cds @@ -5,5 +5,6 @@ using bookshop as my from './data-model'; service CatalogService { @odata.draft.enabled entity Books as projection on my.Books; - entity Publishers as projection on my.Publishers; + + entity Authors as projection on my.Authors; } \ No newline at end of file diff --git a/test/unit/files/draft/data-model.cds b/test/unit/files/draft/data-model.cds index d885452a..c2a9bd66 100644 --- a/test/unit/files/draft/data-model.cds +++ b/test/unit/files/draft/data-model.cds @@ -11,8 +11,13 @@ aspect managed { entity Books : managed, cuid { title : String; + author : Association to Authors; publishers : Composition of many Publishers - on publishers.book = $self; + on publishers.book = $self; +} + +entity Authors : managed, cuid { + name : String; } entity Publishers : managed, cuid { diff --git a/test/unit/files/draft/model.cds b/test/unit/files/draft/model.cds deleted file mode 100644 index b8768299..00000000 --- a/test/unit/files/draft/model.cds +++ /dev/null @@ -1,51 +0,0 @@ -namespace draft_test; - -@odata.draft.enabled -entity A {} -entity B: A {} -@odata.draft.enabled: false -entity C: B {} -entity D: C {} -@odata.draft.enabled: true -entity E: D {} -entity F: C,E {} -entity G: E,C {} - -// don't become draftable -entity H { ref: Association to A } -entity I { ref: Association to many A } -// should not become draftable -entity J { ref: Composition of A } -entity K { ref: Composition of many A } - -// should not -entity L { ref: Composition of C } -entity M { ref: Composition of many C } - -@odata.draft.enabled: true -entity N { ref: Composition of many O } -// ! should be enabled -entity O {} - -@odata.draft.enabled: true -entity P { ref: Association to C } -@odata.draft.enabled: false -entity Q { } -@odata.draft.enabled: true -entity R { ref: Composition of Q } - -entity PA as projection on A {} - -// propagate from projection to referenced entity -entity Referenced {} -entity Referrer { - ref: Composition of Referenced -} -@odata.draft.enabled: true -entity ProjectedReferrer as projection on Referrer {} - -// propagate over two levels of projection -entity Foo {} -entity ProjectedFoo as projection on Foo {} -@odata.draft.enabled: true -entity ProjectedProjectedFoo as projection on ProjectedFoo {} \ No newline at end of file diff --git a/test/util.js b/test/util.js index d0c863d0..adc66b91 100644 --- a/test/util.js +++ b/test/util.js @@ -183,7 +183,8 @@ async function prepareUnitTest(model, outputDirectory, parameters = {}) { await checkTranspilation(tsFiles) } configuration.setFrom(configurationBefore) - return { astw: new ASTWrapper(path.join(parameters.fileSelector(paths), 'index.ts')), paths } + // return undefined for astw in case type-generation failed + return { astw: paths?.length > 0 ? new ASTWrapper(path.join(parameters.fileSelector(paths), 'index.ts')) : undefined, paths } } From 8260c32b7ca94f8d1018ba614ecaf366d13858a1 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sat, 5 Oct 2024 17:45:05 +0200 Subject: [PATCH 02/24] refactor: rename cds file that causes transpilation errors --- test/unit/draft.test.js | 2 +- .../{catalog-service-error.cds => error-catalog-service.cds} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/unit/files/draft/{catalog-service-error.cds => error-catalog-service.cds} (100%) diff --git a/test/unit/draft.test.js b/test/unit/draft.test.js index ebe97651..e2565b41 100644 --- a/test/unit/draft.test.js +++ b/test/unit/draft.test.js @@ -28,7 +28,7 @@ describe('bookshop', () => { test('Draft-enabled composition produces compiler error', async () => { const spyOnConsole = jest.spyOn(console, 'error') - await prepareUnitTest('draft/catalog-service-error.cds', locations.testOutput('bookshop_projection'), {typerOptions: {logLevel: 'ERROR'}}) + await prepareUnitTest('draft/error-catalog-service.cds', locations.testOutput('bookshop_projection'), {typerOptions: {logLevel: 'ERROR'}}) expect(spyOnConsole).toHaveBeenCalledWith( '[cds-typer] -', diff --git a/test/unit/files/draft/catalog-service-error.cds b/test/unit/files/draft/error-catalog-service.cds similarity index 100% rename from test/unit/files/draft/catalog-service-error.cds rename to test/unit/files/draft/error-catalog-service.cds From 8d2b6976e63fb30e7f9d9d8e1cacb2466af78ce3 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sat, 5 Oct 2024 18:07:26 +0200 Subject: [PATCH 03/24] fix: fixes lint issues --- eslint.config.js | 6 ++---- lib/csn.js | 2 +- lib/visitor.js | 4 ++-- test/unit/draft.test.js | 3 ++- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/eslint.config.js b/eslint.config.js index 0f74f434..11428c5c 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -10,12 +10,10 @@ module.exports = [ ecmaVersion: 'latest', sourceType: 'commonjs', globals: { - ...require('globals').node + ...require('globals').node, + jest: true } }, - env: { - jest: true - }, files: ['**/*.js'], plugins: { '@stylistic/js': require('@stylistic/eslint-plugin-js') diff --git a/lib/csn.js b/lib/csn.js index 93c0f714..749b3dbe 100644 --- a/lib/csn.js +++ b/lib/csn.js @@ -194,7 +194,7 @@ class DraftEnabledEntityCollector { // note to self: following doc uses @ homoglyph instead of @, as the latter apparently has special semantics in code listings /** - * We collect all entities that are draft enabled. + * We collect all entities that are draft enabled. * (@see `@sap/cds-compiler/lib/transform/draft/db.js#generateDraft`) * * This includes thwo scenarios: diff --git a/lib/visitor.js b/lib/visitor.js index 5b172af6..e3ae3154 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -292,9 +292,9 @@ class Visitor { /** * @param {string} fq - fully qualified name of the entity * @param {string} clean - the clean name of the entity - * @param {EntityCSN} entity - the entity to generate the static contents for + * @param {EntityCSN} _entity - the entity to generate the static contents for */ - #staticClassContents(fq, clean, entity) { + #staticClassContents(fq, clean, _entity) { return isDraftEnabled(fq) ? [`static drafts: typeof ${clean}`] : [] } diff --git a/test/unit/draft.test.js b/test/unit/draft.test.js index e2565b41..bda6ebab 100644 --- a/test/unit/draft.test.js +++ b/test/unit/draft.test.js @@ -2,6 +2,7 @@ const path = require('path') const { ASTWrapper } = require('../ast') +const { describe, test, expect } = require('@jest/globals') const { locations, prepareUnitTest } = require('../util') const draftable_ = (entity, ast) => ast.find(n => n.name === entity && n.members.find(({name}) => name === 'drafts')) @@ -16,7 +17,7 @@ describe('bookshop', () => { // root and composition become draft enabled expect(draftable('Book', service, () => 'Books')).toBeTruthy() expect(draftable('Publisher', service, () => 'Publishers')).toBeTruthy() - + // associated entity will not become draft enabled expect(draftable('Author', service, () => 'Authors')).toBeFalsy() From 6050b063bfeea43b148a8d7629fa80c58599fbd0 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Thu, 10 Oct 2024 13:49:00 +0200 Subject: [PATCH 04/24] refactor: remove xtended csn --- lib/compile.js | 5 +-- lib/resolution/resolver.js | 7 +-- lib/typedefs.d.ts | 2 +- lib/visitor.js | 88 +++++++++++++------------------------- 4 files changed, 37 insertions(+), 65 deletions(-) diff --git a/lib/compile.js b/lib/compile.js index 8d45d992..c8bbc228 100644 --- a/lib/compile.js +++ b/lib/compile.js @@ -36,7 +36,7 @@ const writeJsConfig = path => { /** * Compiles a CSN object to Typescript types. - * @param {{xtended: import('./typedefs').resolver.CSN, inferred: import('./typedefs').resolver.CSN}} csn - csn tuple + * @param {import('./typedefs').resolver.CSN} csn - csn tuple */ const compileFromCSN = async csn => { @@ -56,9 +56,8 @@ const compileFromCSN = async csn => { */ const compileFromFile = async inputFile => { const paths = typeof inputFile === 'string' ? normalize(inputFile) : inputFile.map(f => normalize(f)) - const xtended = await cds.linked(await cds.load(paths, { docs: true, flavor: 'xtended' })) const inferred = await cds.linked(await cds.load(paths, { docs: true })) - return compileFromCSN({xtended, inferred}) + return compileFromCSN(inferred) } module.exports = { diff --git a/lib/resolution/resolver.js b/lib/resolution/resolver.js index 6584d86f..23407acd 100644 --- a/lib/resolution/resolver.js +++ b/lib/resolution/resolver.js @@ -23,7 +23,7 @@ const { configuration } = require('../config') /** @typedef {{typeName: string, typeInfo: TypeResolveInfo & { inflection: Inflection }}} ResolveAndRequireInfo */ class Resolver { - get csn() { return this.visitor.csn.inferred } + get csn() { return this.visitor.csn } /** @param {Visitor} visitor - the visitor */ constructor(visitor) { @@ -164,7 +164,7 @@ class Resolver { */ const isPropertyOf = (property, entity) => property && Object.hasOwn(entity?.elements ?? {}, property) - const defs = this.visitor.csn.inferred.definitions + const defs = this.visitor.csn.definitions // assume parts to contain [Namespace, Service, Entity1, Entity2, Entity3, property1, property2] /** @type {string} */ // @ts-expect-error - nope, we know there is at least one element @@ -449,6 +449,7 @@ class Resolver { const cardinality = getMaxCardinality(element) + /** @type {TypeResolveInfo} */ const result = { isBuiltin: false, // will be rectified in the corresponding handlers, if needed isInlineDeclaration: false, @@ -564,7 +565,7 @@ class Resolver { * @returns @see resolveType */ resolveTypeName(t, into) { - const result = into ?? {} + const result = into ?? /** @type {TypeResolveInfo} */({}) const path = t.split('.') const builtin = this.builtinResolver.resolveBuiltin(path) if (builtin === undefined) { diff --git a/lib/typedefs.d.ts b/lib/typedefs.d.ts index cbab34d6..d2a58ed5 100644 --- a/lib/typedefs.d.ts +++ b/lib/typedefs.d.ts @@ -13,7 +13,7 @@ export module resolver { compositions?: { target: string }[] doc?: string, elements?: { [key: string]: EntityCSN } - key?: string // custom!! + key?: boolean // custom!! keys?: { [key:string]: any } kind: string, includes?: string[] diff --git a/lib/visitor.js b/lib/visitor.js index e3ae3154..cd7fd4af 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -37,13 +37,11 @@ class Visitor { } /** - * @param {{xtended: CSN, inferred: CSN}} csn - root CSN + * @param {CSN} csn - root CSN */ constructor(csn) { - propagateForeignKeys(csn.xtended) - propagateForeignKeys(csn.inferred) - // has to be executed on the inferred model as autoexposed entities are not included in the xtended csn - collectDraftEnabledEntities(csn.inferred) + propagateForeignKeys(csn) + collectDraftEnabledEntities(csn) this.csn = csn /** @type {Context[]} **/ @@ -70,41 +68,9 @@ class Visitor { * Visits all definitions within the CSN definitions. */ visitDefinitions() { - for (const [name, entity] of Object.entries(this.csn.xtended.definitions)) { - if (isView(entity)) { - this.visitEntity(name, this.csn.inferred.definitions[name]) - } else if (isProjection(entity) || !isUnresolved(entity)) { - this.visitEntity(name, entity) - } else { - LOG.warn(`Skipping unresolved entity: ${name}`) - } - } - // FIXME: optimise - // We are currently working with two flavours of CSN: - // xtended, as it is as close as possible to an OOP class hierarchy - // inferred, as it contains information missing in xtended - // This is less than optimal and has to be revisited at some point! - const handledKeys = new Set(Object.keys(this.csn.xtended.definitions)) - // we are looking for autoexposed entities in services - const missing = Object.entries(this.csn.inferred.definitions).filter(([key]) => !key.endsWith('.texts') &&!handledKeys.has(key)) - for (const [name, entity] of missing) { - // instead of using the definition from inferred CSN, we refer to the projected entity from xtended CSN instead. - // The latter contains the CSN fixes (propagated foreign keys, etc) and none of the localised fields we don't handle yet. - if (entity.projection) { - const targetName = entity.projection.from.ref[0] - // FIXME: references to types of entity properties may be missing from xtendend flavour (see #103) - // this should be revisted once we settle on a single flavour. - const target = this.csn.xtended.definitions[targetName] ?? this.csn.inferred.definitions[targetName] - if (target.kind !== 'type') { - // skip if the target is a property, like in: - // books: Association to many Author.books ... - // as this would result in a type definition that - // name-clashes with the actual declaration of Author - this.visitEntity(name, target) - } - } else { - LOG.error(`Expecting an autoexposed projection within a service. Skipping ${name}`) - } + for (const [name, entity] of Object.entries(this.csn.definitions)) { + if (name.endsWith('.texts')) continue + this.visitEntity(name, entity) } } @@ -115,15 +81,8 @@ class Visitor { * @returns {[string, object][]} array of key name and key element pairs */ #keys(fq) { - // FIXME: this is actually pretty bad, as not only have to propagate keys through - // both flavours of CSN (see constructor), but we are now also collecting them from - // both flavours and deduplicating them. - // xtended contains keys that have been inherited from parents - // inferred contains keys from queried entities (thing `entity Foo as select from Bar`, where Bar has keys) - // So we currently need them both. return Object.entries({ - ...this.csn.inferred.definitions[fq]?.keys ?? {}, - ...this.csn.xtended.definitions[fq]?.keys ?? {} + ...this.csn.definitions[fq]?.keys ?? {} }) } @@ -194,7 +153,7 @@ class Visitor { // FIXME: replace with resolution/entity::asIdentifier const toLocalIdent = ({ns, clean, fq}) => { // types are not inflected, so don't change those to singular - const csn = this.csn.inferred.definitions[fq] + const csn = this.csn.definitions[fq] const ident = isType(csn) ? clean : this.resolver.inflect({csn, plainName: clean}).singular @@ -231,7 +190,9 @@ class Visitor { buffer.addIndentedBlock(`return class ${clean} extends ${ancestorsAspects} {`, () => { /** @type {import('./typedefs').resolver.EnumCSN[]} */ const enums = [] + const inheritedElements = this.#getInheritedElements(entity) for (let [ename, element] of Object.entries(entity.elements ?? {})) { + if (inheritedElements.includes(ename)) continue if (element.target && /\.texts?/.test(element.target)) { LOG.warn(`referring to .texts property in ${fq}. This is currently not supported and will be ignored.`) continue @@ -259,7 +220,7 @@ class Visitor { } // store inline enums for later handling, as they have to go into one common "static elements" wrapper - if (isInlineEnumType(element, this.csn.xtended)) { + if (isInlineEnumType(element, this.csn)) { enums.push(element) } } @@ -289,6 +250,22 @@ class Visitor { this.contexts.pop() } + /** + * @param {EntityCSN} entity - + * @returns {string[]} + */ + #getInheritedElements(entity) { + /** @type {string[]} */ + const inheritedElements = [] + for (const parent of entity.includes ?? []) { + for (const element of Object.keys(this.csn.definitions[parent]?.elements ?? {})) { + inheritedElements.push(element) + } + } + + return inheritedElements + } + /** * @param {string} fq - fully qualified name of the entity * @param {string} clean - the clean name of the entity @@ -325,7 +302,7 @@ class Visitor { // as types are not inflected, their singular will always clash and there is also no plural for them anyway -> skip // if the user defined their entities in singular form we would also have a false positive here -> skip const namespacedSingular = `${ns.asNamespace()}.${singular}` - if (!isType(entity) && namespacedSingular !== fq && namespacedSingular in this.csn.xtended.definitions) { + if (!isType(entity) && namespacedSingular !== fq && namespacedSingular in this.csn.definitions) { LOG.error( `Derived singular '${singular}' for your entity '${fq}', already exists. The resulting types will be erronous. Consider using '@singular:'/ '@plural:' annotations in your model or move the offending declarations into different namespaces to resolve this collision.` ) @@ -346,12 +323,7 @@ class Visitor { // edge case: @singular annotation present. singular4 will take care of that. file.addInflection(util.singular4(entity, true), plural, clean) - // in case of projections `entity` is empty -> retrieve from inferred csn where the actual properties are rolled out - const target = isProjection(entity) || isView(entity) - ? this.csn.inferred.definitions[fq] - : entity - - this.#aspectify(fq, target, buffer, { cleanName: singular }) + this.#aspectify(fq, entity, buffer, { cleanName: singular }) buffer.add(overrideNameProperty(singular, entity.name)) buffer.add(`Object.defineProperty(${singular}, 'is_singular', { value: true })`) @@ -457,7 +429,7 @@ class Visitor { if (isEnum(type) && !isReferenceType(type) && this.resolver.builtinResolver.resolveBuiltin(type.type)) { file.addEnum(fq, entityName, csnToEnumPairs(type), docify(type.doc)) } else { - const isEnumReference = typeof type.type === 'string' && isEnum(this.csn.inferred.definitions[type?.type]) + const isEnumReference = typeof type.type === 'string' && isEnum(this.csn.definitions[type?.type]) // alias file.addType(fq, entityName, this.resolver.resolveAndRequire(type, file).typeName, isEnumReference) } From ac1552f2c5d6b227b8dbeab7ba50c54bfd8636c0 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Thu, 10 Oct 2024 13:50:00 +0200 Subject: [PATCH 05/24] fix: transfer `key` value of associations to foreign key element --- lib/visitor.js | 3 ++- test/unit/references.test.js | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/visitor.js b/lib/visitor.js index cd7fd4af..8d24a302 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -211,7 +211,8 @@ class Visitor { LOG.error(`Attempting to generate a foreign key reference called '${foreignKey}' in type definition for entity ${fq}. But a property of that name is already defined explicitly. Consider renaming that property.`) } else { const kelement = Object.assign(Object.create(originalKeyElement), { - isRefNotNull: !!element.notNull || !!element.key + isRefNotNull: !!element.notNull || !!element.key, + key: element.key }) this.visitElement(foreignKey, kelement, file, buffer) } diff --git a/test/unit/references.test.js b/test/unit/references.test.js index 52c693de..d94da5a2 100644 --- a/test/unit/references.test.js +++ b/test/unit/references.test.js @@ -24,9 +24,9 @@ describe('References', () => { m.type.name === 'many' && m.type.args[0].name === 'Foo_' )).toBeTruthy() - expect(astw.exists('_BarAspect', 'assoc_one_first_key', m => check.isNullable(m.type, [st => check.isKeyOf(st, check.isString)]))).toBeTruthy() - expect(astw.exists('_BarAspect', 'assoc_one_second_key', m => check.isNullable(m.type, [st => check.isKeyOf(st, check.isString)]))).toBeTruthy() - expect(astw.exists('_BarAspect', 'assoc_one_ID', m => check.isNullable(m.type, [st => check.isKeyOf(st, check.isString)]))).toBeTruthy() + expect(astw.exists('_BarAspect', 'assoc_one_first_key', m => check.isNullable(m.type, [st => check.isString(st)]))).toBeTruthy() + expect(astw.exists('_BarAspect', 'assoc_one_second_key', m => check.isNullable(m.type, [st => check.isString(st)]))).toBeTruthy() + expect(astw.exists('_BarAspect', 'assoc_one_ID', m => check.isNullable(m.type, [st => check.isString(st)]))).toBeTruthy() }) test('Inline', async () => { From 1c5844cd761e4906d20051e55b9dcf4f69ebf419 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Thu, 10 Oct 2024 14:30:59 +0200 Subject: [PATCH 06/24] fix: do not skip inherited elements for views/projections --- lib/visitor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/visitor.js b/lib/visitor.js index 8d24a302..0d268a6e 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -190,7 +190,7 @@ class Visitor { buffer.addIndentedBlock(`return class ${clean} extends ${ancestorsAspects} {`, () => { /** @type {import('./typedefs').resolver.EnumCSN[]} */ const enums = [] - const inheritedElements = this.#getInheritedElements(entity) + const inheritedElements = !isViewOrProjection(entity) ? this.#getInheritedElements(entity) : [] for (let [ename, element] of Object.entries(entity.elements ?? {})) { if (inheritedElements.includes(ename)) continue if (element.target && /\.texts?/.test(element.target)) { From 383b541182339533b7579aa94f4f55daaf56f821 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Thu, 10 Oct 2024 21:04:35 +0200 Subject: [PATCH 07/24] fix: fixes inline composition resolution --- lib/resolution/resolver.js | 37 +++++++++++++++++++++++++++---------- lib/typedefs.d.ts | 1 + 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/lib/resolution/resolver.js b/lib/resolution/resolver.js index 23407acd..7bf31a53 100644 --- a/lib/resolution/resolver.js +++ b/lib/resolution/resolver.js @@ -165,10 +165,12 @@ class Resolver { const isPropertyOf = (property, entity) => property && Object.hasOwn(entity?.elements ?? {}, property) const defs = this.visitor.csn.definitions + + // check if name is already an entity, then we do not have a property access, but a nested entity + if (defs[p]?.kind === 'entity') return [] + // assume parts to contain [Namespace, Service, Entity1, Entity2, Entity3, property1, property2] - /** @type {string} */ - // @ts-expect-error - nope, we know there is at least one element - let qualifier = parts.shift() + let qualifier = /** @type {string} */ (parts.shift()) // find first entity from left (Entity1) while ((!defs[qualifier] || !isEntity(defs[qualifier])) && parts.length) { qualifier += `.${parts.shift()}` @@ -239,6 +241,8 @@ class Resolver { } else { // TODO: make sure the resolution still works. Currently, we only cut off the namespace! plural = util.getPluralAnnotation(typeInfo.csn) ?? typeInfo.plainName + // remove leading entity name + if (plural.indexOf('.') !== -1) plural = plural.split('.').slice(-1)[0] singular = util.getSingularAnnotation(typeInfo.csn) ?? util.singular4(typeInfo.csn, true) // util.singular4(typeInfo.csn, true) // can not use `plural` to honor possible @singular annotation // don't slice off namespace if it isn't part of the inflected name. @@ -306,6 +310,8 @@ class Resolver { const targetTypeInfo = this.resolveAndRequire(target, file) if (targetTypeInfo.typeInfo.isDeepRequire === true) { typeName = cardinality > 1 ? toMany(targetTypeInfo.typeName) : toOne(targetTypeInfo.typeName) + } else if (targetTypeInfo.typeInfo.isInlineEntity === true) { + typeName = cardinality > 1 ? toMany(targetTypeInfo.typeInfo.inflection.plural) : toOne(targetTypeInfo.typeInfo.inflection.singular) } else { let { singular, plural } = targetTypeInfo.typeInfo.inflection @@ -368,13 +374,24 @@ class Resolver { // handle typeof (unless it has already been handled above) const target = element.target?.name ?? element.type?.ref?.join('.') ?? element.type if (target && !typeInfo.isDeepRequire) { - const { propertyAccess } = this.visitor.entityRepository.getByFq(target) ?? {} - if (propertyAccess?.length) { - const element = target.slice(0, -propertyAccess.join('.').length - 1) - const access = this.visitor.inlineDeclarationResolver.getTypeLookup(propertyAccess) - // singular, as we have to access the property of the entity - typeName = deepRequire(util.singular4(element)) + access - typeInfo.isDeepRequire = true + if (typeInfo.plainName?.indexOf('.') > -1) { + // target is obviously an inline entity + // update inflections with proper prefix + const nameParts = typeInfo.plainName.split('.').slice(0, -1) + typeInfo.inflection = { + singular: [...nameParts, typeInfo.inflection?.singular].join('.'), + plural: [...nameParts, typeInfo.inflection?.plural].join('.') + } + typeInfo.isInlineEntity = true + } else { + const { propertyAccess } = this.visitor.entityRepository.getByFq(target) ?? {} + if (propertyAccess?.length) { + const element = target.slice(0, -propertyAccess.join('.').length - 1) + const access = this.visitor.inlineDeclarationResolver.getTypeLookup(propertyAccess) + // singular, as we have to access the property of the entity + typeName = deepRequire(util.singular4(element)) + access + typeInfo.isDeepRequire = true + } } } diff --git a/lib/typedefs.d.ts b/lib/typedefs.d.ts index d2a58ed5..6df6a230 100644 --- a/lib/typedefs.d.ts +++ b/lib/typedefs.d.ts @@ -67,6 +67,7 @@ export module resolver { export type TypeResolveInfo = { isBuiltin?: boolean, isDeepRequire?: boolean, + isInlineEntity?: boolean, isNotNull?: boolean, isInlineDeclaration?: boolean, isForeignKeyReference?: boolean, From cf955ffccb86fd3025a5bd0bd04e8d5e249c5d64 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Fri, 11 Oct 2024 12:11:01 +0200 Subject: [PATCH 08/24] fix: fixes scoped entities --- lib/resolution/resolver.js | 43 +++++++++++--------------------------- lib/typedefs.d.ts | 1 - lib/visitor.js | 16 +++++++------- 3 files changed, 20 insertions(+), 40 deletions(-) diff --git a/lib/resolution/resolver.js b/lib/resolution/resolver.js index 7bf31a53..f27aea8a 100644 --- a/lib/resolution/resolver.js +++ b/lib/resolution/resolver.js @@ -242,7 +242,7 @@ class Resolver { // TODO: make sure the resolution still works. Currently, we only cut off the namespace! plural = util.getPluralAnnotation(typeInfo.csn) ?? typeInfo.plainName // remove leading entity name - if (plural.indexOf('.') !== -1) plural = plural.split('.').slice(-1)[0] + if (plural.indexOf('.') !== -1) plural = last(plural) singular = util.getSingularAnnotation(typeInfo.csn) ?? util.singular4(typeInfo.csn, true) // util.singular4(typeInfo.csn, true) // can not use `plural` to honor possible @singular annotation // don't slice off namespace if it isn't part of the inflected name. @@ -310,23 +310,9 @@ class Resolver { const targetTypeInfo = this.resolveAndRequire(target, file) if (targetTypeInfo.typeInfo.isDeepRequire === true) { typeName = cardinality > 1 ? toMany(targetTypeInfo.typeName) : toOne(targetTypeInfo.typeName) - } else if (targetTypeInfo.typeInfo.isInlineEntity === true) { - typeName = cardinality > 1 ? toMany(targetTypeInfo.typeInfo.inflection.plural) : toOne(targetTypeInfo.typeInfo.inflection.singular) } else { let { singular, plural } = targetTypeInfo.typeInfo.inflection - // FIXME: super hack!! - // Inflection currently does not retain the scope of the entity. - // But we can't just fix it in inflection(...), as that would break several other things - // So we bandaid-fix it back here, as it is the least intrusive place -- but this should get fixed asap! - if (target.type) { - const untangled = this.visitor.entityRepository.getByFqOrThrow(target.type) - const scope = untangled.scope.join('.') - if (scope && !singular.startsWith(scope)) { - singular = `${scope}.${singular}` - } - } - typeName = cardinality > 1 ? toMany(plural) : toOne(this.visitor.isSelfReference(target) ? 'this' : singular) @@ -374,24 +360,19 @@ class Resolver { // handle typeof (unless it has already been handled above) const target = element.target?.name ?? element.type?.ref?.join('.') ?? element.type if (target && !typeInfo.isDeepRequire) { - if (typeInfo.plainName?.indexOf('.') > -1) { - // target is obviously an inline entity - // update inflections with proper prefix - const nameParts = typeInfo.plainName.split('.').slice(0, -1) + const { propertyAccess, scope } = this.visitor.entityRepository.getByFq(target) ?? {} + if (scope?.length) { + // update inflections with proper prefix, e.g. Books.text, Books.texts typeInfo.inflection = { - singular: [...nameParts, typeInfo.inflection?.singular].join('.'), - plural: [...nameParts, typeInfo.inflection?.plural].join('.') - } - typeInfo.isInlineEntity = true - } else { - const { propertyAccess } = this.visitor.entityRepository.getByFq(target) ?? {} - if (propertyAccess?.length) { - const element = target.slice(0, -propertyAccess.join('.').length - 1) - const access = this.visitor.inlineDeclarationResolver.getTypeLookup(propertyAccess) - // singular, as we have to access the property of the entity - typeName = deepRequire(util.singular4(element)) + access - typeInfo.isDeepRequire = true + singular: [...scope, typeInfo.inflection?.singular].join('.'), + plural: [...scope, typeInfo.inflection?.plural].join('.') } + } else if (propertyAccess?.length) { + const element = target.slice(0, -propertyAccess.join('.').length - 1) + const access = this.visitor.inlineDeclarationResolver.getTypeLookup(propertyAccess) + // singular, as we have to access the property of the entity + typeName = deepRequire(util.singular4(element)) + access + typeInfo.isDeepRequire = true } } diff --git a/lib/typedefs.d.ts b/lib/typedefs.d.ts index 6df6a230..d2a58ed5 100644 --- a/lib/typedefs.d.ts +++ b/lib/typedefs.d.ts @@ -67,7 +67,6 @@ export module resolver { export type TypeResolveInfo = { isBuiltin?: boolean, isDeepRequire?: boolean, - isInlineEntity?: boolean, isNotNull?: boolean, isInlineDeclaration?: boolean, isForeignKeyReference?: boolean, diff --git a/lib/visitor.js b/lib/visitor.js index 0d268a6e..aacbd233 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -287,7 +287,7 @@ class Visitor { * @param {string} content - the content to set the name property to */ const overrideNameProperty = (clazz, content) => `Object.defineProperty(${clazz}, 'name', { value: '${content}' })` - const { namespace: ns, entityName: clean, inflection } = this.entityRepository.getByFqOrThrow(fq) + const { namespace: ns, entityName: clean, inflection, scope } = this.entityRepository.getByFqOrThrow(fq) const file = this.fileRepository.getNamespaceFile(ns) let { singular, plural } = inflection @@ -316,13 +316,13 @@ class Visitor { ? file.getSubNamespace(this.resolver.trimNamespace(parent.name)) : file.classes - // we can't just use "singular" here, as it may have the subnamespace removed: - // "Books.text" is just "text" in "singular". Within the inflected exports we need - // to have Books.texts = Books.text, so we derive the singular once more without cutting off the ns. - // Directly deriving it from the plural makes sure we retain any parent namespaces of kind "entity", - // which would not be possible while already in singular form, as "Book.text" could not be resolved in CSN. - // edge case: @singular annotation present. singular4 will take care of that. - file.addInflection(util.singular4(entity, true), plural, clean) + if (scope?.length > 0) { + /** @param {string} n - name of entity */ + const scoped = n => [...scope, n].join('.') + file.addInflection(scoped(singular), scoped(plural), scoped(clean)) + } else { + file.addInflection(singular, plural, clean) + } this.#aspectify(fq, entity, buffer, { cleanName: singular }) From 9e48f81c675467e895ab587893a871218324de51 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Fri, 11 Oct 2024 12:11:49 +0200 Subject: [PATCH 09/24] feat: include texts entities --- lib/visitor.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/visitor.js b/lib/visitor.js index aacbd233..85ec2745 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -69,7 +69,6 @@ class Visitor { */ visitDefinitions() { for (const [name, entity] of Object.entries(this.csn.definitions)) { - if (name.endsWith('.texts')) continue this.visitEntity(name, entity) } } @@ -193,10 +192,6 @@ class Visitor { const inheritedElements = !isViewOrProjection(entity) ? this.#getInheritedElements(entity) : [] for (let [ename, element] of Object.entries(entity.elements ?? {})) { if (inheritedElements.includes(ename)) continue - if (element.target && /\.texts?/.test(element.target)) { - LOG.warn(`referring to .texts property in ${fq}. This is currently not supported and will be ignored.`) - continue - } this.visitElement(ename, element, file, buffer) // make foreign keys explicit From 61a429f08e6a5a5442aa60aa6aeea26777f8567b Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Fri, 11 Oct 2024 15:30:43 +0200 Subject: [PATCH 10/24] test: adjust tests to handle entities in subnamespacees --- test/ast.js | 48 ++++++++++++++++++++++++++---------- test/unit/references.test.js | 17 +++++++------ test/unit/typeof.test.js | 7 +++--- 3 files changed, 49 insertions(+), 23 deletions(-) diff --git a/test/ast.js b/test/ast.js index 1f67c245..08467c81 100644 --- a/test/ast.js +++ b/test/ast.js @@ -103,10 +103,17 @@ function visitCallExpression(node) { * @returns {ModuleDeclaration} */ function visitModuleDeclaration(node) { + const nodeNames = [node.name] + let body = node.body + // consider nested modules + while (!body.statements) { + nodeNames.push(body.name) + body = body.body + } return { nodeType: kinds.ModuleDeclaration, - name: visit(node.name), - body: node.body.statements.map(visit) + name: nodeNames.map(n => visit(n)).join('.'), + body: body.statements.map(visit) } } @@ -326,9 +333,12 @@ class ASTWrapper { return this.tree.filter(n => n.nodeType === kinds.ImportDeclaration) } - /** @returns {FunctionDeclaration[]} */ - getAspectFunctions() { - return this.tree.filter(n => n.nodeType === kinds.FunctionDeclaration + /** + * @param {object[]} [tree] - tree to be used + * @returns {FunctionDeclaration[]} + */ + getAspectFunctions(tree) { + return (Array.isArray(tree) ? tree : this.tree).filter(n => n.nodeType === kinds.FunctionDeclaration && n.body.length === 1 && n.body[0].nodeType === kinds.ClassExpression) } @@ -371,17 +381,23 @@ class ASTWrapper { // .filter(n => n.heritage?.at(0)?.subtypes?.at(0)?.keyword === keywords.ExpressionWithTypeArguments) // } - /** @returns {ClassDeclaration[]} */ - getInlineClassDeclarations() { + /** + * @param {object[]} [tree] - tree to be used + * @returns {ClassDeclaration[]} + */ + getInlineClassDeclarations(tree) { // this is total bogus, as its the same as getAspects... - return this.tree + return (Array.isArray(tree) ? tree : this.tree) .filter(n => n.nodeType === kinds.FunctionDeclaration) .map(fn => ({...fn.body[0], name: fn.name })) } - /** @returns {ClassExpression[]} */ - getAspects() { - return this.getAspectFunctions().map(({name, body}) => ({...body[0], name})) + /** + * @param {object[]} [tree] - tree to be used + * @returns {ClassExpression[]} + */ + getAspects(tree) { + return this.getAspectFunctions(tree).map(({name, body}) => ({...body[0], name})) } getAspect(name) { @@ -398,8 +414,14 @@ class ASTWrapper { } exists(clazz, property, type, typeArg) { - const entities = this.getInlineClassDeclarations().concat(this.getAspects()) - const clz = entities.find(c => c.name === clazz) + const getEntities = clazz => { + let tree = this.tree + const module = clazz.split('.').slice(0, -1).join('.') + if (module) tree = this.getModuleDeclaration(module).body + return this.getInlineClassDeclarations(tree).concat(this.getAspects(tree)) + } + const entities = getEntities(clazz) + const clz = entities.find(c => c.name === clazz.split('.').at(-1)) if (!clz) throw Error(`no class with name ${clazz}`) if (!property) return true diff --git a/test/unit/references.test.js b/test/unit/references.test.js index d94da5a2..9fde582f 100644 --- a/test/unit/references.test.js +++ b/test/unit/references.test.js @@ -5,6 +5,7 @@ const { check } = require('../ast') const { locations, prepareUnitTest } = require('../util') describe('References', () => { + /** @type {import('../ast').ASTWrapper} */ let astw beforeAll(async () => astw = (await prepareUnitTest('references/model.cds', locations.testOutput('references_test'))).astw) @@ -32,19 +33,21 @@ describe('References', () => { test('Inline', async () => { expect(astw.exists('_BarAspect', 'inl_comp_one', m => { const comp = m.type.subtypes[0] - const [a] = comp.args[0].members + const type = comp.args[0] return check.isNullable(m.type) && comp.name === 'of' - && a.name === 'a' - && check.isNullable(a.type, [check.isString]) + && type.full === 'Bar.inl_comp_one' })).toBeTruthy() + expect(astw.exists('Bar._inl_comp_oneAspect', 'a', m => check.isNullable(m.type))).toBeTruthy() + expect(astw.exists('Bar._inl_comp_oneAspect', 'ID', m => check.isKeyOf(m.type, check.isString))).toBeTruthy() + expect(astw.exists('Bar._inl_comp_oneAspect', 'up__id', m => check.isKeyOf(m.type, check.isNumber))).toBeTruthy() expect(astw.exists('_BarAspect', 'inl_comp_many', m => { const [arr] = m.type.args - return m.type.name === 'many' - && arr.name === 'Array' - && arr.args[0].members[0].name === 'a' - && check.isNullable(arr.args[0].members[0].type, [check.isString]) + return m.type.name === 'many' && arr.full === 'Bar.inl_comp_many_' })).toBeTruthy() + expect(astw.exists('Bar._inl_comp_manyAspect', 'a', m => check.isNullable(m.type))).toBeTruthy() + expect(astw.exists('Bar._inl_comp_manyAspect', 'up__id', m => check.isKeyOf(m.type, check.isNumber))).toBeTruthy() + // inline ID is not propagated into the parent entity expect(() => astw.exists('_BarAspect', 'inl_comp_one_ID')).toThrow(Error) }) diff --git a/test/unit/typeof.test.js b/test/unit/typeof.test.js index 49641fd6..ba2f1050 100644 --- a/test/unit/typeof.test.js +++ b/test/unit/typeof.test.js @@ -14,12 +14,13 @@ describe('Typeof Syntax', () => { test('Deep Required', async () => { const astw = (await prepareUnitTest('typeof/deep.cds', locations.testOutput('typeof_deep'))).astw expect(astw.exists('_UserRoleAspect', 'users', - m => check.isTypeReference(m.type) && check.isIndexedAccessType(m.type.args.at(0)) && check.isLiteral(m.type.args.at(0).indexType, 'roles') + m => check.isTypeReference(m.type) && check.isTypeReference(m.type.args.at(0), 'Users.roles') )).toBeTruthy() expect(astw.exists('_UserRoleGroupAspect', 'users', m => check.isNullable(m.type, [ - st => check.isTypeReference(st) && check.isIndexedAccessType(st.args[0]) && check.isLiteral(st.args[0].indexType, 'roleGroups') - ]))).toBeTruthy() + st => check.isTypeReference(st.args.at(0), 'Users.roleGroup') + ]) + )).toBeTruthy() }) test('Structured', async () => { From 935995e352b39b9a339878d14b459f344a5926fd Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sat, 12 Oct 2024 14:19:20 +0200 Subject: [PATCH 11/24] refactor: remove unused imports --- lib/visitor.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/visitor.js b/lib/visitor.js index 85ec2745..128585a4 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -1,8 +1,6 @@ 'use strict' -const util = require('./util') - -const { isView, isUnresolved, propagateForeignKeys, collectDraftEnabledEntities, isDraftEnabled, isType, isProjection, getMaxCardinality, isViewOrProjection, isEnum } = require('./csn') +const { propagateForeignKeys, collectDraftEnabledEntities, isDraftEnabled, isType, getMaxCardinality, isViewOrProjection, isEnum } = require('./csn') // eslint-disable-next-line no-unused-vars const { SourceFile, FileRepository, Buffer, Path } = require('./file') const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline') From 3c8c5b43fb3ed1beaef09457ab6ddc7d943ea56b Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sat, 23 Nov 2024 20:14:45 +0100 Subject: [PATCH 12/24] fix: imports/exports for esm mode --- lib/file.js | 5 +++-- lib/printers/javascript.js | 8 ++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/file.js b/lib/file.js index 6309929c..881e23d6 100644 --- a/lib/file.js +++ b/lib/file.js @@ -402,7 +402,8 @@ class SourceFile extends File { } for (const imp of Object.values(this.imports)) { if (!imp.isCwd(this.path.asDirectory())) { - buffer.add(`import * as ${imp.asIdentifier()} from '${imp.asDirectory({relative: this.path.asDirectory()})}';`) + const importSuffix = configuration.targetModuleType === 'esm' ? '/index.js' : '' + buffer.add(`import * as ${imp.asIdentifier()} from '${imp.asDirectory({relative: this.path.asDirectory()})}${importSuffix}';`) } } buffer.blankLine() @@ -481,7 +482,7 @@ class SourceFile extends File { return { singularRhs: `createEntityProxy(['${namespace}', '${original}'], { target: { is_singular: true }${customPropsStr} })`, - pluralRhs: `createEntityProxy(['${namespace}', '${original}'])`, + pluralRhs: `createEntityProxy(['${namespace}', '${original}'], { target: { is_singular: false }})`, } } else { return { diff --git a/lib/printers/javascript.js b/lib/printers/javascript.js index fe3b36e0..2f0bb2c7 100644 --- a/lib/printers/javascript.js +++ b/lib/printers/javascript.js @@ -76,12 +76,16 @@ class ESMPrinter extends JavaScriptPrinter { /** @type {JavaScriptPrinter['printDeconstructedImport']} */ printDeconstructedImport (imports, from) { - return `import { ${imports.join(', ')} } from '${from}'` + return `import { ${imports.join(', ')} } from '${from}/index.js'` } /** @type {JavaScriptPrinter['printExport']} */ printExport (name, value) { - return `export const ${name} = ${value}` + if (name.indexOf('.') !== -1) { + return `${name} = ${value}` + } else { + return `export const ${name} = ${value}` + } } /** @type {JavaScriptPrinter['printDefaultExport']} */ From dc565727c29c6228dbdf49923b80a72d469db461 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sun, 1 Dec 2024 21:59:57 +0100 Subject: [PATCH 13/24] feat: propagate inflection annotations from xtended --- lib/compile.js | 5 +++-- lib/csn.js | 25 +++++++++++++++++++++++++ lib/typedefs.d.ts | 2 ++ lib/visitor.js | 13 ++++++++----- test/unit/files/inflection/model.cds | 4 ++++ 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/lib/compile.js b/lib/compile.js index c8bbc228..8d45d992 100644 --- a/lib/compile.js +++ b/lib/compile.js @@ -36,7 +36,7 @@ const writeJsConfig = path => { /** * Compiles a CSN object to Typescript types. - * @param {import('./typedefs').resolver.CSN} csn - csn tuple + * @param {{xtended: import('./typedefs').resolver.CSN, inferred: import('./typedefs').resolver.CSN}} csn - csn tuple */ const compileFromCSN = async csn => { @@ -56,8 +56,9 @@ const compileFromCSN = async csn => { */ const compileFromFile = async inputFile => { const paths = typeof inputFile === 'string' ? normalize(inputFile) : inputFile.map(f => normalize(f)) + const xtended = await cds.linked(await cds.load(paths, { docs: true, flavor: 'xtended' })) const inferred = await cds.linked(await cds.load(paths, { docs: true })) - return compileFromCSN(inferred) + return compileFromCSN({xtended, inferred}) } module.exports = { diff --git a/lib/csn.js b/lib/csn.js index 9993b712..e70f5cea 100644 --- a/lib/csn.js +++ b/lib/csn.js @@ -292,6 +292,30 @@ function propagateForeignKeys(csn) { } } +/** + * Clears "correct" singular/plural annotations from inferred model + * copies the ones from the xtended model. + * + * This is done to prevent potential duplicate class names because of annotation propagation. + * @param {{inferred: CSN, xtended: CSN}} csn - CSN models + */ +function propagateInflectionAnnotations(csn) { + const singularAnno = '@singular' + const pluralAnno = '@plural' + for (const [name, def] of Object.entries(csn.inferred.definitions)) { + // clear annotations from inferred definition + if (singularAnno in def) delete def[singularAnno] + if (pluralAnno in def) delete def[pluralAnno] + + const xtendedDef = csn.xtended.definitions[name] + if (xtendedDef) { + // transfer annotation from xtended if existing + if (singularAnno in xtendedDef) def[singularAnno] = xtendedDef[singularAnno] + if (pluralAnno in xtendedDef) def[pluralAnno] = xtendedDef[pluralAnno] + } + } +} + /** * @param {EntityCSN} entity - the entity */ @@ -326,5 +350,6 @@ module.exports = { getProjectionAliases, getViewTarget, propagateForeignKeys, + propagateInflectionAnnotations, isCsnAny } diff --git a/lib/typedefs.d.ts b/lib/typedefs.d.ts index 55970be9..37f27dd3 100644 --- a/lib/typedefs.d.ts +++ b/lib/typedefs.d.ts @@ -25,6 +25,8 @@ export module resolver { target?: string, type: string | ref, name: string, + '@singular'?: string, + '@plural'?: string, '@odata.draft.enabled'?: boolean // custom! _unresolved?: boolean isRefNotNull?: boolean // custom! diff --git a/lib/visitor.js b/lib/visitor.js index ffe5c1ca..d69d2149 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -1,6 +1,6 @@ 'use strict' -const { propagateForeignKeys, collectDraftEnabledEntities, isDraftEnabled, isType, getMaxCardinality, isViewOrProjection, isEnum, isEntity } = require('./csn') +const { propagateForeignKeys, propagateInflectionAnnotations, collectDraftEnabledEntities, isDraftEnabled, isType, getMaxCardinality, isViewOrProjection, isEnum, isEntity } = require('./csn') // eslint-disable-next-line no-unused-vars const { SourceFile, FileRepository, Buffer, Path } = require('./file') const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline') @@ -37,12 +37,15 @@ class Visitor { } /** - * @param {CSN} csn - root CSN + * @param {{xtended: CSN, inferred: CSN}} csn - root CSN */ constructor(csn) { - propagateForeignKeys(csn) - collectDraftEnabledEntities(csn) - this.csn = csn + propagateForeignKeys(csn.inferred) + propagateInflectionAnnotations(csn) + collectDraftEnabledEntities(csn.inferred) + + // xtendend csn not required after this point -> continue with inferred + this.csn = csn.inferred /** @type {Context[]} **/ this.contexts = [] diff --git a/test/unit/files/inflection/model.cds b/test/unit/files/inflection/model.cds index 2758078a..0be8a9aa 100644 --- a/test/unit/files/inflection/model.cds +++ b/test/unit/files/inflection/model.cds @@ -32,6 +32,8 @@ entity C { } +entity CSub : C {} + @UI.HeaderInfo.TypeName: 'OneD' @UI.HeaderInfo.TypeNamePlural: 'ManyDs' @singular: 'OneSingleD' @@ -39,6 +41,8 @@ entity D { } +entity DSub : D {} + entity Referer { // annotated a: Association to Bazes; From 2d00a011f6218c6de91aab7904aac89a3abd5aca Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sun, 1 Dec 2024 22:12:03 +0100 Subject: [PATCH 14/24] fix: fix output test --- test/unit/output.test.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/unit/output.test.js b/test/unit/output.test.js index b5118214..e9b3c58a 100644 --- a/test/unit/output.test.js +++ b/test/unit/output.test.js @@ -184,11 +184,15 @@ describe('Compilation', () => { ['A_', 'A'], ['A', 'A'], ['C', 'C'], + ['CSub', 'CSub'], + ['CSub_', 'CSub'], ['LotsOfCs', 'C'], ['OneSingleD', 'D'], ['D', 'D'], + ['DSub', 'DSub'], + ['DSub_', 'DSub'], ['Referer', 'Referer'], - ['Referer_', 'Referer'] + ['Referer_', 'Referer'], ]) // ...are currently exceptions where both singular _and_ plural // are annotated and the original name is used as an export on top of that. @@ -207,6 +211,8 @@ describe('Compilation', () => { '_CAspect', '_OneSingleDAspect', '_RefererAspect', + '_CSubAspect', + '_DSubAspect' ] expect(aspects.length).toBe(expected.length) expect(aspects.map(({name}) => name)).toEqual(expect.arrayContaining(expected)) @@ -230,7 +236,11 @@ describe('Compilation', () => { 'D', 'D', 'Referer', - 'Referer_' + 'Referer_', + 'DSub', + 'DSub_', + 'CSub', + 'CSub_' ] expect(fns.map(({name}) => name)).toEqual(expect.arrayContaining(expected)) expect(fns.length).toBe(expected.length) From f2d462a33781ca9602fefab1bf8b795fdb977efe Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Sun, 1 Dec 2024 22:12:37 +0100 Subject: [PATCH 15/24] docs: add preliminary changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7602a87f..c46e941e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ This project adheres to [Semantic Versioning](http://semver.org/). The format is based on [Keep a Changelog](http://keepachangelog.com/). ## Version 0.30.0 - TBD +### Added +- adds classes for inline compositions +- adds support for `localized` which generates dedicated `text` classes ## Version 0.29.1 - TBD ### Fixed From 6d17ed50a84a370ea51c730289b1fc651e304355 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Tue, 3 Dec 2024 22:55:00 +0100 Subject: [PATCH 16/24] feat: print inline enum to subnamespace buffer if necessary --- lib/file.js | 34 +++++++++++++++++++++++++++++----- lib/visitor.js | 2 +- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/lib/file.js b/lib/file.js index 2c29ea33..5c8c71aa 100644 --- a/lib/file.js +++ b/lib/file.js @@ -257,6 +257,7 @@ class SourceFile extends File { if (!(name in this.namespaces)) { const buffer = new Buffer() buffer.closed = false + buffer.namespace = name buffer.add(`export namespace ${name} {`) buffer.indent() this.namespaces[name] = buffer @@ -286,6 +287,8 @@ class SourceFile extends File { * @param {string} entityFqName - name of the entity the enum is attached to with namespace * @param {string} propertyName - property to which the enum is attached. * @param {[string, string][]} kvs - list of key-value pairs + * @param {Buffer} [buffer] - if buffer is of subnamespace the enum will be added there, + * otherwise to the inline enums of the file * @param {string[]} doc - the enum docs * If given, the enum is considered to be an inline definition of an enum. * If not, it is considered to be regular, named enum. @@ -310,16 +313,32 @@ class SourceFile extends File { * } * ``` */ - addInlineEnum(entityCleanName, entityFqName, propertyName, kvs, doc=[]) { + addInlineEnum(entityCleanName, entityFqName, propertyName, kvs, buffer, doc=[]) { + const namespacedEntity = `${buffer?.namespace ? buffer.namespace + '.' : ''}${entityCleanName}` this.enums.data.push({ - name: `${entityCleanName}.${propertyName}`, + name: `${namespacedEntity}.${propertyName}`, property: propertyName, kvs, - fq: `${entityCleanName}.${propertyName}` + fq: `${namespacedEntity}.${propertyName}` }) - const entityProxy = this.entityProxies[entityCleanName] ?? (this.entityProxies[entityCleanName] = []) + const entityProxy = this.entityProxies[namespacedEntity] ?? (this.entityProxies[namespacedEntity] = []) entityProxy.push(propertyName) - printEnum(this.inlineEnums.buffer, propertyToInlineEnumName(entityCleanName, propertyName), kvs, {export: false}, doc) + + // REVISIT: find a better way to do this??? + const printEnumToBuffer = (/** @type {Buffer} */buffer) => printEnum(buffer, propertyToInlineEnumName(entityCleanName, propertyName), kvs, {export: false}, doc) + + if (buffer?.namespace) { + const tempBuffer = new Buffer() + // we want to put the enums on class level + tempBuffer.indent() + printEnumToBuffer(tempBuffer) + + // we want to write the enums at the beginning of the namespace + const [first,...rest] = buffer.parts + buffer.parts = [first, ...tempBuffer.parts, ...rest] + } else { + printEnumToBuffer(this.inlineEnums.buffer) + } } /** @@ -586,6 +605,11 @@ class Buffer { * @type {boolean} */ this.closed = false + /** + * Required for inline enums of inline compositions or text entities + * @type {string | undefined} + */ + this.namespace = undefined } /** diff --git a/lib/visitor.js b/lib/visitor.js index d69d2149..1c84ca66 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -270,7 +270,7 @@ class Visitor { initialiser: propertyToInlineEnumName(clean, e.name), isStatic: true, })) - file.addInlineEnum(clean, fq, e.name, csnToEnumPairs(e, {unwrapVals: true}), eDoc) + file.addInlineEnum(clean, fq, e.name, csnToEnumPairs(e, {unwrapVals: true}), buffer, eDoc) } if ('kind' in entity) { From 9524c6a6c4885cc598d9e853f5337b59f47f10e6 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Thu, 5 Dec 2024 00:14:12 +0100 Subject: [PATCH 17/24] fix: use index access for inline entities in js exports --- lib/file.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/file.js b/lib/file.js index 5c8c71aa..a2f49daf 100644 --- a/lib/file.js +++ b/lib/file.js @@ -509,9 +509,12 @@ class SourceFile extends File { pluralRhs: `createEntityProxy(['${namespace}', '${original}'], { target: { is_singular: false }})`, } } else { + // standard entity: csn.Books + // inline entity: csn['Books.texts'] + const csnAccess = original.indexOf('.') !== -1 ? `csn['${original}']` : `csn.${original}` return { - singularRhs: `{ is_singular: true, __proto__: csn.${original} }`, - pluralRhs: `csn.${original}` + singularRhs: `{ is_singular: true, __proto__: ${csnAccess} }`, + pluralRhs: csnAccess } } } From b274d0537ed0a0c8356b1791bacd911461a17c51 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Thu, 5 Dec 2024 00:15:43 +0100 Subject: [PATCH 18/24] fix: do not discard of inflection annotations exclusive to inferred csn --- lib/csn.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/csn.js b/lib/csn.js index e70f5cea..dea1c8cb 100644 --- a/lib/csn.js +++ b/lib/csn.js @@ -303,16 +303,16 @@ function propagateInflectionAnnotations(csn) { const singularAnno = '@singular' const pluralAnno = '@plural' for (const [name, def] of Object.entries(csn.inferred.definitions)) { - // clear annotations from inferred definition - if (singularAnno in def) delete def[singularAnno] - if (pluralAnno in def) delete def[pluralAnno] - const xtendedDef = csn.xtended.definitions[name] - if (xtendedDef) { - // transfer annotation from xtended if existing - if (singularAnno in xtendedDef) def[singularAnno] = xtendedDef[singularAnno] - if (pluralAnno in xtendedDef) def[pluralAnno] = xtendedDef[pluralAnno] - } + // we keep the annotations from definition specific to the inferred model (e.g. inline compositions) + if (!xtendedDef) continue + + // clear annotations from inferred definition + if (Object.hasOwn(def, singularAnno)) delete def[singularAnno] + if (Object.hasOwn(def, pluralAnno)) delete def[pluralAnno] + // transfer annotation from xtended if existing + if (Object.hasOwn(xtendedDef, singularAnno)) def[singularAnno] = xtendedDef[singularAnno] + if (Object.hasOwn(xtendedDef, pluralAnno)) def[pluralAnno] = xtendedDef[pluralAnno] } } From 05b26c1257fee110ba76c6974e27d60c437bd91c Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Thu, 5 Dec 2024 00:16:00 +0100 Subject: [PATCH 19/24] test: add test cases --- test/unit/files/inlinecompositions/model.cds | 59 ++++++++++++++++++++ test/unit/files/inlinecompositions/model.ts | 51 +++++++++++++++++ test/unit/files/localized/model.cds | 8 +++ test/unit/files/localized/model.ts | 19 +++++++ 4 files changed, 137 insertions(+) create mode 100644 test/unit/files/inlinecompositions/model.cds create mode 100644 test/unit/files/inlinecompositions/model.ts create mode 100644 test/unit/files/localized/model.cds create mode 100644 test/unit/files/localized/model.ts diff --git a/test/unit/files/inlinecompositions/model.cds b/test/unit/files/inlinecompositions/model.cds new file mode 100644 index 00000000..bbe4bd29 --- /dev/null +++ b/test/unit/files/inlinecompositions/model.cds @@ -0,0 +1,59 @@ +using { + cuid, + sap.common.CodeList +} from '@sap/cds/common'; + + +namespace inl_comp; + +entity Genres : CodeList { + key code : String enum { + Fiction; + } +} + +@singular: 'PEditor' +@plural : 'PEditors' +aspect Editors : cuid { + name : String; +} + + +@singular: 'Bestseller' +@plural : 'Bestsellers' +entity Books : cuid { + title : String; + genre : Association to Genres; + publishers : Composition of many { + key ID : UUID; + name : String; + type : String enum { + self; + independent; + }; + // will get inflections from aspect + intEditors : Composition of many Editors; + // will get inflections from aspect + extEditors : Composition of many Editors; + offices : Composition of many { + key ID : UUID; + city : String; + zipCode : String; + size : String enum { + small; + medium; + large; + } + } + } +} + +// overwrite annotations from aspect to avoid name duplicates +annotate Books.publishers.extEditors with @singular: 'EEditor' + @plural : 'EEditors'; + + +service CatService { + // autoexposed inline compositions will have the same names as in the schema + entity Books as projection on inl_comp.Books; +} diff --git a/test/unit/files/inlinecompositions/model.ts b/test/unit/files/inlinecompositions/model.ts new file mode 100644 index 00000000..329c0a9d --- /dev/null +++ b/test/unit/files/inlinecompositions/model.ts @@ -0,0 +1,51 @@ +import cds from '@sap/cds' +import * as CatSrv from '#cds-models/inl_comp/CatService' +import { Genre, Books, Bestsellers } from '#cds-models/inl_comp' + +export class InlineCompService extends cds.ApplicationService { + override async init() { + // Checks on model entities + const office: Books.publishers.office = { + zipCode: '4934', + up__ID: '', + up__up__ID: '', + up_: { + name: '', + }, + size: Books.publishers.office.size.large, + } + const intEditor: Books.publishers.PEditor = { name: 'editor 1' } + const extEditors: Books.publishers.EEditors = [{ name: 'editor 2' }] + const publisher: Books.publisher = { + ID: '134', + name: 'Publisher1', + intEditors: [{name: 'Int. Editor 1'}, {name: 'Int. Editor 2'}], + extEditors: [{name: 'Ext. Editor 1'}], + offices: [{ + city: 'A', + zipCode: '4934', + size: Books.publishers.office.size.large + }] + } + const bestseller: Bestsellers = [{ genre_code: Genre.code.Fiction }] + + // Checks on Service entities + const book: CatSrv.Book = { + ID: '493493', + title: 'Book 1', + genre_code: CatSrv.Genre.code.Fiction, + publishers: [{ + ID: '134', + name: 'Publisher1', + intEditors: [{name: 'Int. Editor 1'}, {name: 'Int. Editor 2'}], + extEditors: [{name: 'Ext. Editor 1'}], + offices: [{ + city: 'A', + zipCode: '4934', + size: Books.publishers.office.size.large + }] + }] + } + return super.init() + } +} diff --git a/test/unit/files/localized/model.cds b/test/unit/files/localized/model.cds new file mode 100644 index 00000000..bc21c51c --- /dev/null +++ b/test/unit/files/localized/model.cds @@ -0,0 +1,8 @@ +using {cuid} from '@sap/cds/common.cds'; + +namespace localized_model; + +entity Books : cuid { + title : localized String; + authorName : String; +} diff --git a/test/unit/files/localized/model.ts b/test/unit/files/localized/model.ts new file mode 100644 index 00000000..cb2637df --- /dev/null +++ b/test/unit/files/localized/model.ts @@ -0,0 +1,19 @@ +import cds from '@sap/cds' +import { Books } from '#cds-models/localized_model' + +const bookText: Books.text = { + title: 'Book A', + locale: 'de' +} + +const book: Books = [{ + title: 'Book A', + authorName: 'John', + localized: { + title: 'Book A - default', + }, + texts: [ + { title: 'Book A - english', locale: 'en' }, + { title: 'Book A - deutsch', locale: 'de' }, + ], +}] From f4080c3acb3b40d710f7d8ac205aa9622066166d Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Fri, 6 Dec 2024 21:13:05 +0100 Subject: [PATCH 20/24] refactor: cache inherited elements of entity inside repository --- lib/resolution/entity.js | 16 ++++++++++++++++ lib/visitor.js | 20 ++------------------ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/resolution/entity.js b/lib/resolution/entity.js index 81e0c0e1..ec1ce777 100644 --- a/lib/resolution/entity.js +++ b/lib/resolution/entity.js @@ -61,6 +61,22 @@ class EntityInfo { /** @type {import('../typedefs').resolver.EntityCSN | undefined} */ #csn + /** @type {Set | null} */ + #inheritedElements = null + + /** @returns set of inherited elements (e.g. ID of aspect cuid) */ + get inheritedElements() { + if (this.#inheritedElements) return this.#inheritedElements + this.#inheritedElements = new Set() + for (const parentName of this.csn.includes ?? []) { + const parent = this.#repository.getByFq(parentName) + for (const element of Object.keys(parent?.csn?.elements ?? {})) { + this.#inheritedElements.add(element) + } + } + return this.#inheritedElements + } + /** @returns the **inferred** csn for this entity. */ get csn () { return this.#csn ??= this.#resolver.csn.definitions[this.fullyQualifiedName] diff --git a/lib/visitor.js b/lib/visitor.js index 1c84ca66..ef16ab28 100644 --- a/lib/visitor.js +++ b/lib/visitor.js @@ -220,6 +220,7 @@ class Visitor { .reverse() // reverse so that own aspect A is applied before extensions B,C: B(C(A(Entity))) .reduce((wrapped, ancestor) => `${asIdentifier({info: ancestor, wrapper: name => `_${name}Aspect`, relative: file.path})}(${wrapped})`, 'Base') + const inheritedElements = !isViewOrProjection(entity) ? info.inheritedElements : null this.contexts.push({ entity: fq }) // CLASS ASPECT @@ -227,12 +228,11 @@ class Visitor { buffer.addIndentedBlock(`return class ${clean} extends ${ancestorsAspects} {`, () => { /** @type {import('./typedefs').resolver.EnumCSN[]} */ const enums = [] - const inheritedElements = !isViewOrProjection(entity) ? this.#getInheritedElements(entity) : [] /** @type {TypeResolveOptions} */ const resolverOptions = { forceInlineStructs: isEntity(entity) && configuration.inlineDeclarations === 'flat'} for (let [ename, element] of Object.entries(entity.elements ?? [])) { - if (inheritedElements.includes(ename)) continue + if (inheritedElements?.has(ename)) continue this.visitElement({name: ename, element, file, buffer, resolverOptions}) // make foreign keys explicit @@ -297,22 +297,6 @@ class Visitor { this.contexts.pop() } - /** - * @param {EntityCSN} entity - - * @returns {string[]} - */ - #getInheritedElements(entity) { - /** @type {string[]} */ - const inheritedElements = [] - for (const parent of entity.includes ?? []) { - for (const element of Object.keys(this.csn.definitions[parent]?.elements ?? {})) { - inheritedElements.push(element) - } - } - - return inheritedElements - } - /** * @param {string} fq - fully qualified name of the entity * @param {string} clean - the clean name of the entity From e1957ed283e92dbf65e1aff1811204cf16139170 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Fri, 6 Dec 2024 21:13:42 +0100 Subject: [PATCH 21/24] test: add/enhance test for inline compositions --- test/ast.js | 80 ++++++++++++++++---- test/unit/entitiesproxy.test.js | 2 + test/unit/inlinecompositions.test.js | 109 +++++++++++++++++++++++++++ test/unit/output.test.js | 2 + 4 files changed, 178 insertions(+), 15 deletions(-) create mode 100644 test/unit/inlinecompositions.test.js diff --git a/test/ast.js b/test/ast.js index 28a6e939..0947788b 100644 --- a/test/ast.js +++ b/test/ast.js @@ -449,8 +449,13 @@ class JSASTWrapper { this.program = acorn.parse(code, { ecmaVersion: 'latest'}) } - exportsAre(expected) { - if (expected.length < this.getExports().length) throw new Error(`there are more actual than expected exports. Expected ${expected.length}, found ${this.getExports().length}`) + /** + * @param {[string,string][]} expected - expected export as tuples of `[, ]` + * @param {'enum' | 'entity'} [exportType] - the type of export + */ + exportsAre(expected, exportType = 'entity') { + const exports = this.getExports()?.filter(e => e.type === exportType) + if (expected.length < exports.length) throw new Error(`there are more actual than expected exports. Expected ${expected.length}, found ${exports.length}`) for (const [lhs, rhs] of expected) { if (!this.hasExport(lhs, rhs)) throw new Error(`missing export module.exports.${lhs} = ${rhs}`) } @@ -477,30 +482,75 @@ class JSASTWrapper { if (!customProps.every(c => propKeys.includes(c))) throw new Error('not all expected custom props found in argument') } + getExport(name) { + return this.getExports().find(e => e.lhs === name) + } + hasExport(lhs, rhs) { return this.getExports().find(exp => exp.lhs === lhs && (exp.rhs === rhs || exp.rhs.name === rhs)) } /** - * @returns {{ lhs: string, rhs: string | { singular: boolean, name: string }}} + * Collects modules export in the following formats + * - `module.exports.A = createEntityProxy(['namespace', 'A'], { target: { is_singular: true }})` + * - `module.exports.A.sub = createEntityProxy(['namespace', 'A.sub'], { target: { is_singular: true }})` + * - `module.exports.A = { is_singular: true, __proto__: csn.A }` + * - `module.exports.A.sub = csn['A.sub']` + * - `module.exports.A.sub = { is_singular: true, __proto__: csn['A.sub']}` + * - `module.exports.A.type = { a: 'a', b: 'b', c: 'c' } + * @returns {{ lhs: string, rhs: string | { singular: boolean, name: string } | Record, type: 'entity' | 'enum', proxyArgs?: any[]}[]} */ getExports() { - const processObjectLiteral = ({properties}) => ({ - singular: properties.find(p => p.key.name === 'is_singular' && p.value.value), - name: properties.find(p => p.key.name === '__proto__')?.value.property.name - }) + const processRhsObjLiteral = ({ properties }) => { + const proto = properties.find(p => p.key.name === '__proto__') + return { + singular: !!properties.find(p => p.key.name === 'is_singular' && p.value.value), + name: proto?.value.property.name ?? proto?.value.property.value, + } + } + const isLhsCsnExport = obj => { + if (!obj || !('object' in obj) || !('property' in obj)) return false + return (obj.object?.name === 'module' && obj.property?.name === 'exports') || isLhsCsnExport(obj.object) + } + const getLhsExportId = (obj, expPath = []) => { + if (obj?.property?.name && obj.property.name !== 'exports') expPath.push(obj.property.name) + if (obj?.object?.name && obj.object.name !== 'module') expPath.push(obj.object.name) + if (obj.object?.object) return getLhsExportId(obj.object, expPath) + return expPath.reverse().join('.') + } return this.exports ??= this.program.body.filter(node => { if (node.type !== 'ExpressionStatement') return false if (node.expression.left.type !== 'MemberExpression') return false - const { object, property } = node.expression.left.object - return object.name === 'module' && property.name === 'exports' + if ( + !node.expression.right.property?.name && + !node.expression.right.property?.value && + !node.expression.right.arguments && + !node.expression.right.properties + ) return false + return isLhsCsnExport(node.expression.left.object) }).map(node => { - return { - lhs: node.expression.left.property.name, - rhs: this.proxyExports - ? node.expression.right.arguments?.[0].elements[1].value - : node.expression.right.property?.name ?? processObjectLiteral(node.expression.right), - proxyArgs: node.expression.right.arguments + const { left, right } = node.expression + const exportId = getLhsExportId(left) + if (node.expression.operator === '??=') { + return { + lhs: exportId, + type: 'enum', + rhs: right.properties?.reduce((a, c) => { + a[c.key.name] = c.value.value + return a + }, {}), + } + } else { + return { + lhs: exportId, + type: 'entity', + rhs: this.proxyExports + ? right.arguments?.[0].elements[1].value // proxy function arg -> 'A.sub' + : right.property?.name ?? // csn.A + right.property?.value ?? // csn['A.sub'] + processRhsObjLiteral(right), // {__proto__: csn.A} | {__proto__: csn['A.sub']} + proxyArgs: right.arguments, + } } }) } diff --git a/test/unit/entitiesproxy.test.js b/test/unit/entitiesproxy.test.js index b383f6a5..e7762672 100644 --- a/test/unit/entitiesproxy.test.js +++ b/test/unit/entitiesproxy.test.js @@ -27,6 +27,8 @@ describe('Compilation/Runtime - with Entities Proxies', () => { jsw.exportsAre([ ['Book', 'Books'], ['Books', 'Books'], + ['Books.text', 'Books.texts'], + ['Books.texts', 'Books.texts'], ['Author', 'Authors'], ['Authors', 'Authors'], ['Genre', 'Genres'], diff --git a/test/unit/inlinecompositions.test.js b/test/unit/inlinecompositions.test.js new file mode 100644 index 00000000..8ccf62a2 --- /dev/null +++ b/test/unit/inlinecompositions.test.js @@ -0,0 +1,109 @@ +'use strict' + +const path = require('path') +const { describe, test, expect } = require('@jest/globals') +const { JSASTWrapper } = require('../ast') +const { locations, prepareUnitTest } = require('../util') + +/** + * @typedef {import('../ast').JSASTWrapper} JSASTWrapper + */ + +describe('Inline compositions', () => { + + test.each([ + ['with entities proxy', { useEntitiesProxy: true }], + ['with direct csn export', {}], + ])('Test exports > %s', async (_, typerOptions) => { + const paths = ( + await prepareUnitTest('inlinecompositions/model.cds', locations.testOutput('inlinecompositions_test'), { + typerOptions, + }) + ).paths + const jsw = await JSASTWrapper.initialise( + path.join(paths[1], 'index.js'), + typerOptions?.useEntitiesProxy ?? false + ) + jsw.exportsAre([ + ['Genre', 'Genres'], + ['Genres', 'Genres'], + ['Bestseller', 'Books'], + ['Bestsellers', 'Books'], + ['Books', 'Books'], + ['Genres.text', 'Genres.texts'], + ['Genres.texts', 'Genres.texts'], + ['Books.publisher', 'Books.publishers'], + ['Books.publishers', 'Books.publishers'], + ['Books.publishers.PEditor', 'Books.publishers.intEditors'], + ['Books.publishers.PEditors', 'Books.publishers.intEditors'], + ['Books.publishers.intEditors', 'Books.publishers.intEditors'], + ['Books.publishers.EEditor', 'Books.publishers.extEditors'], + ['Books.publishers.EEditors', 'Books.publishers.extEditors'], + ['Books.publishers.extEditors', 'Books.publishers.extEditors'], + ['Books.publishers.office', 'Books.publishers.offices'], + ['Books.publishers.offices', 'Books.publishers.offices'], + ]) + expect(jsw.getExport('Genre.code').rhs).toEqual({ Fiction: 'Fiction' }) + expect(jsw.getExport('Genres.text.code')?.rhs).toEqual({ Fiction: 'Fiction' }) + expect(jsw.getExport('Books.publisher.type')?.rhs).toEqual({ self: 'self', independent: 'independent' }) + expect(jsw.getExport('Books.publishers.office.size')?.rhs).toEqual({ + small: 'small', + medium: 'medium', + large: 'large', + }) + + if (typerOptions?.useEntitiesProxy) { + jsw.hasProxyExport('Genres.text', ['code']) + jsw.hasProxyExport('Books.publisher', ['type']) + jsw.hasProxyExport('Books.publishers.office', ['size']) + } + }) + + test.each([ + ['with entities proxy', { useEntitiesProxy: true }], + ['with direct csn export', {}], + ])('Test service exports > %s', async (_, typerOptions) => { + const paths = ( + await prepareUnitTest('inlinecompositions/model.cds', locations.testOutput('inlinecompositions_test'), { + typerOptions, + }) + ).paths + const jsw = await JSASTWrapper.initialise( + path.join(paths[2], 'index.js'), + typerOptions?.useEntitiesProxy ?? false + ) + jsw.exportsAre([ + ['Genre', 'Genres'], + ['Genres', 'Genres'], + ['Book', 'Books'], + ['Books', 'Books'], + ['Books', 'Books'], + ['Genres.text', 'Genres.texts'], + ['Genres.texts', 'Genres.texts'], + ['Books.publisher', 'Books.publishers'], + ['Books.publishers', 'Books.publishers'], + ['Books.publishers.PEditor', 'Books.publishers.intEditors'], + ['Books.publishers.PEditors', 'Books.publishers.intEditors'], + ['Books.publishers.intEditors', 'Books.publishers.intEditors'], + ['Books.publishers.EEditor', 'Books.publishers.extEditors'], + ['Books.publishers.EEditors', 'Books.publishers.extEditors'], + ['Books.publishers.extEditors', 'Books.publishers.extEditors'], + ['Books.publishers.office', 'Books.publishers.offices'], + ['Books.publishers.offices', 'Books.publishers.offices'], + ]) + expect(jsw.getExport('Genre.code').rhs).toEqual({ Fiction: 'Fiction' }) + expect(jsw.getExport('Genres.text.code')?.rhs).toEqual({ Fiction: 'Fiction' }) + expect(jsw.getExport('Books.publisher.type')?.rhs).toEqual({ self: 'self', independent: 'independent' }) + expect(jsw.getExport('Books.publishers.office.size')?.rhs).toEqual({ + small: 'small', + medium: 'medium', + large: 'large', + }) + + if (typerOptions?.useEntitiesProxy) { + jsw.hasProxyExport('Genres.text', ['code']) + jsw.hasProxyExport('Books.publisher', ['type']) + jsw.hasProxyExport('Books.publishers.office', ['size']) + } + }) +}) \ No newline at end of file diff --git a/test/unit/output.test.js b/test/unit/output.test.js index e9b3c58a..e5829e5d 100644 --- a/test/unit/output.test.js +++ b/test/unit/output.test.js @@ -18,6 +18,8 @@ describe('Compilation', () => { jsw.exportsAre([ ['Book', 'Books'], ['Books', 'Books'], + ['Books.text', 'Books.texts'], + ['Books.texts', 'Books.texts'], ['Author', 'Authors'], ['Authors', 'Authors'], ['Genre', 'Genres'], From 7e8d370e1cc2417d45de938197e1e4a8f60fe5b8 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Tue, 17 Dec 2024 23:31:54 +0100 Subject: [PATCH 22/24] chore: update changelog --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e34dd9e9..34caf6d5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -### Added +### Added +- dedicated classes for inline compositions +- dedicated text-classes for entities with `localized` elements + ### Changed ### Deprecated ### Removed @@ -19,8 +22,6 @@ All notable changes to this project will be documented in this file. ### Added - cds aspects now generate a synthetic plural type too, to be used in `composition of many` -- dedicated classes for inline compositions -- support for `localized` which generates dedicated `text` classes ## [0.30.0] - 2024-12-02 From 8ac088ce9a6473d673d1dfdf4813ba52c2fe4546 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Wed, 8 Jan 2025 10:33:26 +0100 Subject: [PATCH 23/24] refactor: resolve review remarks --- lib/csn.js | 5 +++-- lib/file.js | 4 ++-- lib/printers/javascript.js | 8 +++----- lib/resolution/resolver.js | 2 +- lib/util.js | 4 ++-- 5 files changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/csn.js b/lib/csn.js index dea1c8cb..66f9f037 100644 --- a/lib/csn.js +++ b/lib/csn.js @@ -1,4 +1,5 @@ const { LOG } = require('./logging') +const { annotations } = require('./util') const DRAFT_ENABLED_ANNO = '@odata.draft.enabled' /** @type {string[]} */ @@ -300,8 +301,8 @@ function propagateForeignKeys(csn) { * @param {{inferred: CSN, xtended: CSN}} csn - CSN models */ function propagateInflectionAnnotations(csn) { - const singularAnno = '@singular' - const pluralAnno = '@plural' + const singularAnno = annotations.singular[0] + const pluralAnno = annotations.plural[0] for (const [name, def] of Object.entries(csn.inferred.definitions)) { const xtendedDef = csn.xtended.definitions[name] // we keep the annotations from definition specific to the inferred model (e.g. inline compositions) diff --git a/lib/file.js b/lib/file.js index 33b81671..ce35733e 100644 --- a/lib/file.js +++ b/lib/file.js @@ -314,7 +314,7 @@ class SourceFile extends File { * ``` */ addInlineEnum(entityCleanName, entityFqName, propertyName, kvs, buffer, doc=[]) { - const namespacedEntity = `${buffer?.namespace ? buffer.namespace + '.' : ''}${entityCleanName}` + const namespacedEntity = [buffer?.namespace, entityCleanName].filter(Boolean).join('.') this.enums.data.push({ name: `${namespacedEntity}.${propertyName}`, property: propertyName, @@ -514,7 +514,7 @@ class SourceFile extends File { } else { // standard entity: csn.Books // inline entity: csn['Books.texts'] - const csnAccess = original.indexOf('.') !== -1 ? `csn['${original}']` : `csn.${original}` + const csnAccess = original.includes('.') ? `csn['${original}']` : `csn.${original}` return { singularRhs: `{ is_singular: true, __proto__: ${csnAccess} }`, pluralRhs: csnAccess diff --git a/lib/printers/javascript.js b/lib/printers/javascript.js index ceb511f3..1d22016d 100644 --- a/lib/printers/javascript.js +++ b/lib/printers/javascript.js @@ -89,11 +89,9 @@ class ESMPrinter extends JavaScriptPrinter { /** @type {JavaScriptPrinter['printExport']} */ printExport (name, value) { - if (name.indexOf('.') !== -1) { - return `${name} = ${value}` - } else { - return `export const ${name} = ${value}` - } + return name.includes('.') + ? `${name} = ${value}` + : `export const ${name} = ${value}` } /** @type {JavaScriptPrinter['printDefaultExport']} */ diff --git a/lib/resolution/resolver.js b/lib/resolution/resolver.js index 8451e304..68d622be 100644 --- a/lib/resolution/resolver.js +++ b/lib/resolution/resolver.js @@ -243,7 +243,7 @@ class Resolver { // TODO: make sure the resolution still works. Currently, we only cut off the namespace! plural = util.getPluralAnnotation(typeInfo.csn) ?? typeInfo.plainName // remove leading entity name - if (plural.indexOf('.') !== -1) plural = last(plural) + if (plural.includes('.')) plural = last(plural) singular = util.getSingularAnnotation(typeInfo.csn) ?? util.singular4(typeInfo.csn, true) // util.singular4(typeInfo.csn, true) // can not use `plural` to honor possible @singular annotation // don't slice off namespace if it isn't part of the inflected name. diff --git a/lib/util.js b/lib/util.js index decdcef3..f4a4ce66 100644 --- a/lib/util.js +++ b/lib/util.js @@ -15,10 +15,10 @@ if (process.version.startsWith('v14')) { const last = /\w+$/ -const annotations = { +const annotations = /** @type {const} */ ({ singular: ['@singular'], plural: ['@plural'], -} +}) /** * Converts a camelCase string to snake_case. From c6ea598e74ca0ff845cfeb3580e029c3b716e2a7 Mon Sep 17 00:00:00 2001 From: Ludwig Stockbauer-Muhr Date: Wed, 8 Jan 2025 11:17:18 +0100 Subject: [PATCH 24/24] refactor: resolve review remarks --- lib/resolution/entity.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/resolution/entity.js b/lib/resolution/entity.js index 3a0cb846..0b5b30b6 100644 --- a/lib/resolution/entity.js +++ b/lib/resolution/entity.js @@ -61,8 +61,8 @@ class EntityInfo { /** @type {import('../typedefs').resolver.EntityCSN | undefined} */ #csn - /** @type {Set | null} */ - #inheritedElements = null + /** @type {Set | undefined} */ + #inheritedElements /** @returns set of inherited elements (e.g. ID of aspect cuid) */ get inheritedElements() {