Skip to content

Commit d29aed3

Browse files
daogradydaogrady
authored
Safely override .kind and Buffer improvements (#367)
1 parent 56dbe64 commit d29aed3

File tree

12 files changed

+132
-40
lines changed

12 files changed

+132
-40
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
1212

1313
### Fixed
1414
- Entity elements of named structured types are flattened when using the option `--inlineDeclarations flat`
15+
- `override` modifier on `.kind` property is now only generated if the property is actually inherited, satisfying strict `tsconfig.json`s
1516
- Properly support mandatory (`not null`) action parameters with `array of` types
1617

1718
## Version 0.27.0 - 2024-10-02

‎lib/components/class.js

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* Create a class member with given modifiers in the right order.
3+
* @param {object} options - options
4+
* @param {string} options.name - the name of the member
5+
* @param {string} [options.type] - the type of the member
6+
* @param {string} [options.initialiser] - the initialiser for the member
7+
* @param {string} [options.statementEnd] - the closing character for the member
8+
* @param {boolean} [options.isDeclare] - whether the member is declared
9+
* @param {boolean} [options.isStatic] - whether the member is static
10+
* @param {boolean} [options.isReadonly] - whether the member is readonly
11+
* @param {boolean} [options.isOverride] - whether the member is an override
12+
*/
13+
function createMember ({name, type = undefined, initialiser = undefined, statementEnd = ';', isDeclare = false, isStatic = false, isReadonly = false, isOverride = false}) {
14+
if (isDeclare && isOverride) throw new Error('member cannot have both declare and override modifiers')
15+
16+
const parts = []
17+
18+
if (isDeclare) parts.push('declare')
19+
if (isStatic) parts.push('static')
20+
if (isOverride) parts.push('override')
21+
if (isReadonly) parts.push('readonly')
22+
23+
parts.push(type ? `${name}: ${type}` : name)
24+
25+
if (initialiser) parts.push(`= ${initialiser}`)
26+
27+
const member = parts.join(' ')
28+
return statementEnd
29+
? `${member}${statementEnd}`
30+
: member
31+
}
32+
33+
module.exports = {
34+
createMember
35+
}

‎lib/components/enum.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,12 @@ const enumVal = (key, value, enumType) => enumType === 'cds.String' ? JSON.strin
5555
function printEnum(buffer, name, kvs, options = {}, doc=[]) {
5656
const opts = {...{export: true}, ...options}
5757
buffer.add('// enum')
58-
if (opts.export) doc.forEach(d => { buffer.add(d) })
58+
if (opts.export) buffer.add(doc)
5959
buffer.addIndentedBlock(`${opts.export ? 'export ' : ''}const ${name} = {`, () =>
6060
kvs.forEach(([k, v]) => { buffer.add(`${normalise(k)}: ${v},`) })
6161
, '} as const;')
6262
buffer.add(`${opts.export ? 'export ' : ''}type ${name} = ${stringifyEnumType(kvs)}`)
63-
buffer.add('')
63+
buffer.blankLine()
6464
}
6565

6666
/**

‎lib/components/inline.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ class StructuredInlineDeclarationResolver extends InlineDeclarationResolver {
276276
printInlineType({fq, type, buffer, modifiers, statementEnd}) {
277277
const sub = new Buffer()
278278
this.flatten({fq, type, buffer: sub, modifiers, statementEnd})
279-
sub.parts.forEach(p => { buffer.add(p) })
279+
buffer.add(sub.parts)
280280
}
281281

282282
/**

‎lib/components/wrappers.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ const docify = doc => {
105105
return ['/**'].concat(lines.map(line => `* ${line}`)).concat(['*/'])
106106
}
107107

108+
/**
109+
* Wraps a string in single quotes. No escaping is done, so use with caution.
110+
* @param {string} s - the string to wrap.
111+
* @returns {string}
112+
*/
113+
const stringIdent = s => `'${s}'`
114+
108115
module.exports = {
109116
createArrayOf,
110117
createKey,
@@ -118,5 +125,6 @@ module.exports = {
118125
createCompositionOfOne,
119126
createCompositionOfMany,
120127
deepRequire,
121-
docify
128+
docify,
129+
stringIdent
122130
}

‎lib/file.js

+20-3
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ class SourceFile extends File {
400400
buffer.add(`import * as ${imp.asIdentifier()} from '${imp.asDirectory({relative: this.path.asDirectory()})}';`)
401401
}
402402
}
403-
buffer.add('') // empty line after imports
403+
buffer.blankLine()
404404
return buffer
405405
}
406406

@@ -590,10 +590,27 @@ class Buffer {
590590

591591
/**
592592
* Adds an element to the buffer with the current indentation level.
593-
* @param {string} part - what to attach to the buffer
593+
* @param {string | (() => string) | ((() => string) | string)[]} part - what to attach to the buffer
594594
*/
595595
add(part) {
596-
this.parts.push(this.currentIndent + part)
596+
if (typeof part === 'string') {
597+
this.parts.push(this.currentIndent + part)
598+
} else if (Array.isArray(part)) {
599+
for (const p of part) {
600+
this.add(p) // recurse to have proper indentation
601+
}
602+
} else if (typeof part === 'function') {
603+
this.parts.push(part())
604+
} else {
605+
throw new Error(`trying to add something of type ${typeof part} to a Buffer`)
606+
}
607+
}
608+
609+
/**
610+
* Adds a blank line to the buffer.
611+
*/
612+
blankLine() {
613+
this.add('')
597614
}
598615

599616
/**

‎lib/resolution/entity.js

+1
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class EntityInfo {
6161
/** @type {import('../typedefs').resolver.EntityCSN | undefined} */
6262
#csn
6363

64+
/** @returns the **inferred** csn for this entity. */
6465
get csn () {
6566
return this.#csn ??= this.#resolver.csn.definitions[this.fullyQualifiedName]
6667
}

‎lib/visitor.js

+42-12
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const { SourceFile, FileRepository, Buffer, Path } = require('./file')
88
const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline')
99
const { Resolver } = require('./resolution/resolver')
1010
const { LOG } = require('./logging')
11-
const { docify, createPromiseOf, createUnionOf, createKeysOf, createElementsOf } = require('./components/wrappers')
11+
const { docify, createPromiseOf, createUnionOf, createKeysOf, createElementsOf, stringIdent } = require('./components/wrappers')
1212
const { csnToEnumPairs, propertyToInlineEnumName, isInlineEnumType, stringifyEnumType } = require('./components/enum')
1313
const { isReferenceType } = require('./components/reference')
1414
const { empty } = require('./components/typescript')
@@ -17,6 +17,7 @@ const { EntityRepository, asIdentifier } = require('./resolution/entity')
1717
const { last } = require('./components/identifier')
1818
const { getPropertyModifiers } = require('./components/property')
1919
const { configuration } = require('./config')
20+
const { createMember } = require('./components/class')
2021

2122
/** @typedef {import('./file').File} File */
2223
/** @typedef {import('./typedefs').visitor.Context} Context */
@@ -155,7 +156,12 @@ class Visitor {
155156
}, '}'
156157
) // end of actions
157158
} else {
158-
buffer.add(`declare static readonly actions: ${inherited}${empty}`)
159+
buffer.add(createMember({
160+
name: 'actions',
161+
type: `${inherited}${empty}`,
162+
isStatic: true,
163+
isReadonly: true
164+
}))
159165
}
160166
}
161167

@@ -164,15 +170,27 @@ class Visitor {
164170
* @param {string} clean - the clean name of the entity
165171
*/
166172
#printStaticKeys(buffer, clean) {
167-
buffer.add(`declare static readonly keys: ${createKeysOf(clean)}`)
173+
buffer.add(createMember({
174+
name: 'keys',
175+
type: createKeysOf(clean),
176+
isDeclare: true,
177+
isStatic: true,
178+
isReadonly: true,
179+
}))
168180
}
169181

170182
/**
171183
* @param {Buffer} buffer - the buffer to write the elements into
172184
* @param {string} clean - the clean name of the entity
173185
*/
174186
#printStaticElements(buffer, clean) {
175-
buffer.add(`declare static readonly elements: ${createElementsOf(clean)}`)
187+
buffer.add(createMember({
188+
name: 'elements',
189+
type: createElementsOf(clean),
190+
isDeclare: true,
191+
isStatic: true,
192+
isReadonly: true
193+
}))
176194
}
177195

178196
/**
@@ -280,13 +298,25 @@ class Visitor {
280298

281299
for (const e of enums) {
282300
const eDoc = docify(e.doc)
283-
eDoc.forEach(d => { buffer.add(d) })
284-
buffer.add(`static ${e.name} = ${propertyToInlineEnumName(clean, e.name)}`)
301+
buffer.add(eDoc)
302+
buffer.add(createMember({
303+
name: e.name,
304+
initialiser: propertyToInlineEnumName(clean, e.name),
305+
isStatic: true,
306+
}))
285307
file.addInlineEnum(clean, fq, e.name, csnToEnumPairs(e, {unwrapVals: true}), eDoc)
286308
}
287309

288310
if ('kind' in entity) {
289-
buffer.add(`static readonly kind: 'entity' | 'type' | 'aspect' = '${entity.kind}';`)
311+
buffer.add(createMember({
312+
name: 'kind',
313+
type: '"entity" | "type" | "aspect"',
314+
isStatic: true,
315+
isReadonly: true,
316+
isDeclare: false,
317+
isOverride: ancestorInfos.some(ancestor => ancestor.csn.kind),
318+
initialiser: stringIdent(entity.kind)
319+
}))
290320
}
291321
this.#printStaticKeys(buffer, clean)
292322
this.#printStaticElements(buffer, clean)
@@ -296,7 +326,7 @@ class Visitor {
296326

297327
// CLASS WITH ADDED ASPECTS
298328
file.addImport(baseDefinitions.path)
299-
docify(entity.doc).forEach(d => { buffer.add(d) })
329+
buffer.add(docify(entity.doc))
300330
buffer.add(`export class ${identSingular(clean)} extends ${identAspect(toLocalIdent({clean, fq}))}(${baseDefinitions.path.asIdentifier()}.Entity) {${this.#staticClassContents(clean, entity).join('\n')}}`)
301331
this.contexts.pop()
302332
}
@@ -382,11 +412,11 @@ class Visitor {
382412
// so it can get passed as value to CQL functions.
383413
const additionalProperties = this.#staticClassContents(singular, entity)
384414
additionalProperties.push('$count?: number')
385-
docify(entity.doc).forEach(d => { buffer.add(d) })
415+
buffer.add(docify(entity.doc))
386416
buffer.add(`export class ${plural} extends Array<${singular}> {${additionalProperties.join('\n')}}`)
387417
buffer.add(overrideNameProperty(plural, entity.name))
388418
}
389-
buffer.add('')
419+
buffer.blankLine()
390420
}
391421

392422
/**
@@ -531,12 +561,12 @@ class Visitor {
531561
// file.addImport(new Path(['cds'], '')) TODO make sap/cds import work
532562
buffer.addIndentedBlock(`export class ${serviceNameSimple} extends cds.Service {`, () => {
533563
Object.entries(service.operations ?? {}).forEach(([name, {doc}]) => {
534-
docify(doc).forEach(d => { buffer.add(d) })
564+
buffer.add(docify(doc))
535565
buffer.add(`declare ${name}: typeof ${name}`)
536566
})
537567
}, '}')
538568
buffer.add(`export default ${serviceNameSimple}`)
539-
buffer.add('')
569+
buffer.blankLine()
540570
file.addService(service.name)
541571
}
542572

‎library/cds.hana.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export class VARCHAR extends String {};
99
export class CLOB extends String {};
1010
export class BINARY extends String {}
1111
export class ST_POINT {
12-
public x: number;
13-
public y: number;
12+
declare public x: number;
13+
declare public y: number;
1414
}
1515
export class ST_GEOMETRY { /* FIXME */ }

‎test/tscheck.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ function checkProgram (program) {
2929
* @param {import('typescript').CompilerOptions} opts - the options to pass to the TS compiler
3030
*/
3131
async function checkTranspilation (apiFiles, opts = {}) {
32-
const options = {...{ noEmit: true, esModuleInterop: true }, ...opts}
32+
const options = {...{ noEmit: true, esModuleInterop: true, strict: true }, ...opts}
3333
const program = ts.createProgram({ rootNames: apiFiles, options })
3434
checkProgram(program)
3535
}

‎test/unit/files/actions/model.ts

+17-17
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,15 @@ export class S extends cds.ApplicationService { async init(){
3838

3939
this.on(getOneExternalType, req => { return {extType:1} satisfies ExternalType})
4040
this.on(getManyExternalTypes, req => { return [{extType:1}] satisfies ExternalType[] })
41-
this.on(fSingleParamSingleReturn, req => { const {val} = req.data; return {e1:val.e1} satisfies E })
42-
this.on(fSingleParamManyReturn, req => { const {val} = req.data; return [{e1:val.e1}] satisfies E[] })
43-
this.on(fManyParamManyReturn, req => { const {val} = req.data; return val satisfies E[] })
44-
this.on(fManyParamSingleReturn, req => { const {val} = req.data; return {e1:val[0].e1} satisfies E })
41+
this.on(fSingleParamSingleReturn, req => { const {val} = req.data; return {e1:val?.e1} satisfies E })
42+
this.on(fSingleParamManyReturn, req => { const {val} = req.data; return [{e1:val?.e1}] satisfies E[] })
43+
this.on(fManyParamManyReturn, req => { const {val} = req.data; return val! satisfies E[] })
44+
this.on(fManyParamSingleReturn, req => { const {val} = req.data; return {e1:val?.at(0)?.e1} satisfies E })
4545

46-
this.on(aSingleParamSingleReturn, req => { const {val} = req.data; return {e1:val.e1} satisfies E })
47-
this.on(aSingleParamManyReturn, req => { const {val} = req.data; return [{e1:val.e1}] satisfies E[] })
48-
this.on(aManyParamManyReturn, req => { const {val} = req.data; return val satisfies E[] })
49-
this.on(aManyParamSingleReturn, req => { const {val} = req.data; return {e1:val[0].e1} satisfies E })
46+
this.on(aSingleParamSingleReturn, req => { const {val} = req.data; return {e1:val?.e1} satisfies E })
47+
this.on(aSingleParamManyReturn, req => { const {val} = req.data; return [{e1:val?.e1}] satisfies E[] })
48+
this.on(aManyParamManyReturn, req => { const {val} = req.data; return val! satisfies E[] })
49+
this.on(aManyParamSingleReturn, req => { const {val} = req.data; return {e1:val?.at(0)?.e1} satisfies E })
5050

5151
this.on(aMandatoryParam, req => {
5252
false satisfies IsKeyOptional<typeof req.data, 'val1'>
@@ -59,27 +59,27 @@ export class S extends cds.ApplicationService { async init(){
5959
this.on(free2, req => { return {extType:1} satisfies ExternalType })
6060
this.on(free3, req => { return {extRoot:1} satisfies ExternalInRoot})
6161
this.on(freevoid, req => { return undefined satisfies void })
62-
this.on(freetypeof, req => { req.data.p satisfies number })
62+
this.on(freetypeof, req => { req.data.p! satisfies number })
6363
this.on(free4, req => { return { extType2:1 } satisfies ExternalType2 })
6464

6565
// calling actions
66-
const s2 = await cds.connect.to(S2)
67-
await s2.a1({p1: '', p2: [ { extType2: 1 } ]}) satisfies ExternalType
68-
await s2.a1('', [ { extType2: 1 } ]) satisfies ExternalType
69-
await s2.a2({p1: '', p3: 1}) satisfies ExternalType
70-
await s2.a2('', [], 1) satisfies ExternalType
66+
const s2:S2 = await cds.connect.to(S2)
67+
await s2.a1({p1: '', p2: [ { extType2: 1 } ]}) satisfies ExternalType | null
68+
await s2.a1('', [ { extType2: 1 } ]) satisfies ExternalType | null
69+
await s2.a2({p1: '', p3: 1}) satisfies ExternalType | null
70+
await s2.a2('', [], 1) satisfies ExternalType | null
7171

72-
await s_.aRfcStyle({INPUT: ''}) satisfies ExternalType
72+
await s_.aRfcStyle({INPUT: ''}) satisfies ExternalType | null
7373
//@ts-expect-error
7474
await s_.aRfcStyle('') // no positional call style allowed for RFC actions
7575

7676
// docs -> hover over actions
7777
// provider side
78-
this.on(aDocOneLine, req => { const {val1, val2} = req.data; return {e1:val1[0].e1} satisfies E })
78+
this.on(aDocOneLine, req => { const {val1, val2} = req.data; return {e1:val1?.e1} satisfies E | null})
7979
// caller side
8080
s_.aDocOneLine({val1: {}, val2: {}})
8181
// provider side
82-
this.on(aDocMoreLinesWithBadChar, req => { const {val} = req.data; return {e1:val[0].e1} satisfies E })
82+
this.on(aDocMoreLinesWithBadChar, req => { const {val} = req.data; return {e1:val?.e1} satisfies E | null})
8383
// caller side
8484
s_.aDocMoreLinesWithBadChar({val: {}})
8585

‎test/unit/files/keys/model.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ C.keys.e2 // inherited from E2
88
C.keys.c // own key
99
// @ts-expect-error - own non-key property not contained in .keys
1010
C.keys.d
11-
new C().c.toLowerCase() // still a regular string
11+
new C().c?.toLowerCase() // still a regular string
1212
// @ts-expect-error - Symbol marker not accessible
1313
new C().c.key

0 commit comments

Comments
 (0)