Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse cds.log instead of custom logging logic #265

Merged
merged 7 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/).
### Fixed
- Fixed a bug where keys would sometimes inconsistently become nullable

### Changed
- Logging now internally uses `cds.log` and pipes output into the `cds-typer` logger, which can be configured via `cds.env` in addition to explicitly passing a `--logLevel` parameter to CLI. Users now have to use the levels defined in [`cds.log.levels`](https://cap.cloud.sap/docs/node.js/cds-log#log-levels). The formerly valid levels `WARNING`, `CRITICAL`, and `NONE` are now deprecated and automatically mapped to valid levels for now.

## Version 0.21.2 - 2024-06-06
### Fixed
- The typescript build task will no longer attempt to run unless at least cds 8 is installed
Expand Down
27 changes: 19 additions & 8 deletions lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@
/* eslint-disable no-console */
'use strict'

const cds = require('@sap/cds')
const { compileFromFile } = require('./compile')
const { parseCommandlineArgs } = require('./util')
const { Levels } = require('./logging')
const { deprecated, _keyFor } = require('./logging')
const path = require('path')
const { EOL } = require('node:os')

Expand All @@ -21,9 +22,11 @@ const flags = {
desc: 'This text.',
},
logLevel: {
desc: 'Minimum log level that is printed.',
allowed: Object.keys(Levels),
default: Levels.ERROR,
desc: `Minimum log level that is printed.${EOL}The default is only used if no explicit value is passed${EOL}and there is no configuration passed via cds.env either.`,
allowed: Object.keys(cds.log.levels).concat(Object.keys(deprecated)),
allowedHint: Object.keys(cds.log.levels).join(' | '), // FIXME: remove once old levels are faded out
defaultHint: _keyFor(cds.log.levels.ERROR),
default: cds?.env?.log?.levels?.['cds-typer'] ?? _keyFor(cds.log.levels.ERROR),
},
jsConfigPath: {
desc: `Path to where the jsconfig.json should be written.${EOL}If specified, ${toolName} will create a jsconfig.json file and${EOL}set it up to restrict property usage in types entities to${EOL}existing properties only.`,
Expand Down Expand Up @@ -60,14 +63,16 @@ const help = () => `SYNOPSIS${EOL2}` +
.sort()
.map(([key, value]) => {
let s = indent(`--${key}`, ' ')
if (value.allowed) {
if (value.allowedHint) {
s += ` ${value.allowedHint}`
} else if (value.allowed) {
s += `: <${value.allowed.join(' | ')}>`
} else if (value.type) {
s += `: <${value.type}>`
}
if (value.default) {
if (value.defaultHint || value.default) {
s += EOL
s += indent(`(default: ${value.default})`, ' ')
s += indent(`(default: ${value.defaultHint ?? value.default})`, ' ')
}
s += `${EOL2}${indent(value.desc, ' ')}`
return s
Expand All @@ -92,10 +97,16 @@ const main = async args => {
if (args.named.jsConfigPath && !args.named.jsConfigPath.endsWith('jsconfig.json')) {
args.named.jsConfigPath = path.join(args.named.jsConfigPath, 'jsconfig.json')
}
const newLogLevel = deprecated[args.named.logLevel]
if (newLogLevel) {
console.warn(`deprecated log level '${args.named.logLevel}', use '${newLogLevel}' instead (changing this automatically for now).`)
args.named.logLevel = newLogLevel
}

compileFromFile(args.positional, {
// temporary fix until rootDir is faded out
outputDirectory: [args.named.outputDirectory, args.named.rootDir].find(d => d !== './') ?? './',
logLevel: Levels[args.named.logLevel] ?? args.named.logLevel,
logLevel: args.named.logLevel,
jsConfigPath: args.named.jsConfigPath,
inlineDeclarations: args.named.inlineDeclarations,
propertiesOptional: args.named.propertiesOptional === 'true',
Expand Down
14 changes: 6 additions & 8 deletions lib/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
const cds = require(require.resolve('@sap/cds', { paths: [process.cwd(), __dirname] }))
const util = require('./util')
const { writeout } = require('./file')
const { Logger } = require('./logging')
const { Visitor } = require('./visitor')
const { LOG, setLevel } = require('./logging')

/**
* @typedef {import('./visitor').CompileParameters} CompileParameters
Expand All @@ -16,10 +16,9 @@
* Writes the accompanying jsconfig.json file to the specified paths.
* Tries to merge nicely if an existing file is found.
* @param {string} path - filepath to jsconfig.json.
* @param {import('./logging').Logger} logger - logger
* @private
*/
const writeJsConfig = (path, logger) => {
const writeJsConfig = path => {
let values = {
compilerOptions: {
checkJs: true,
Expand All @@ -29,7 +28,7 @@
if (fs.existsSync(path)) {
const currentContents = JSON.parse(fs.readFileSync(path))
if (currentContents?.compilerOptions?.checkJs) {
logger.warning(`jsconfig at location ${path} already exists. Attempting to merge.`)
LOG.warn(`jsconfig at location ${path} already exists. Attempting to merge.`)
}
util.deepMerge(currentContents, values)
values = currentContents
Expand All @@ -40,20 +39,19 @@

/**
* Compiles a CSN object to Typescript types.
* @param {{xtended: CSN, inferred: CSN}} csn

Check warning on line 42 in lib/compile.js

View workflow job for this annotation

GitHub Actions / lint

Missing JSDoc @param "csn" description
* @param {CompileParameters} parameters - path to root directory for all generated files, min log level
*/
const compileFromCSN = async (csn, parameters) => {
const envSettings = cds.env?.typer ?? {}
parameters = { ...envSettings, ...parameters }
const logger = new Logger()
logger.addFrom(parameters.logLevel)
setLevel(parameters.logLevel)
if (parameters.jsConfigPath) {
writeJsConfig(parameters.jsConfigPath, logger)
writeJsConfig(parameters.jsConfigPath)
}
return writeout(
parameters.outputDirectory,
Object.values(new Visitor(csn, parameters, logger).getWriteoutFiles())
Object.values(new Visitor(csn, parameters).getWriteoutFiles())
)
}

Expand Down
5 changes: 3 additions & 2 deletions lib/components/resolver.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const { isInlineEnumType, propertyToInlineEnumName } = require('./enum')
const { isReferenceType } = require('./reference')
const { isEntity } = require('../csn')
const { baseDefinitions } = require('./basedefs')
const { LOG } = require('../logging')

/** @typedef {import('../visitor').Visitor} Visitor */
/** @typedef {import('../typedefs').resolver.CSN} CSN */
Expand Down Expand Up @@ -318,7 +319,7 @@ class Resolver {
}
}
if (!singular || !plural) {
this.visitor.logger.error(`Singular ('${singular}') or plural ('${plural}') for '${typeName}' is empty.`)
LOG.error(`Singular ('${singular}') or plural ('${plural}') for '${typeName}' is empty.`)
}

return { typeName, singular, plural }
Expand Down Expand Up @@ -574,7 +575,7 @@ class Resolver {
}

if (result.isBuiltin === false && result.isInlineDeclaration === false && !result.plainName) {
this.logger.warning(`Plain name is empty for ${element?.type ?? '<empty>'}. This will probably cause issues.`)
LOG.warn(`Plain name is empty for ${element?.type ?? '<empty>'}. This will probably cause issues.`)
}
return result
}
Expand Down
82 changes: 12 additions & 70 deletions lib/logging.js
Original file line number Diff line number Diff line change
@@ -1,74 +1,16 @@
/** @enum {number} */
const Levels = {
TRACE: 1,
DEBUG: 2,
INFO: 3,
WARNING: 4,
ERROR: 8,
CRITICAL: 16,
NONE: 32,
}

class Logger {
constructor() {
this.mask = 0
const lvls = Object.keys(Levels)
for (let i = 0; i < lvls.length - 1; i++) {
// -1 to ignore NONE
const level = lvls[i]
this[level.toLowerCase()] = function (message) { this._log(level, message) }.bind(this)
}
}

// only temporarily to disable those warnings...
//warning(s) {}; error(s) {}; info(s) {}; debug(s) {};

/**
* Add all log levels starting at level.
* @param {number} baseLevel - level to start from.
*/
addFrom(baseLevel) {
const vals = Object.values(Levels)
const highest = vals[vals.length - 1]
for (let l = Math.log2(baseLevel); Math.pow(2, l) <= highest; l++) {
this.add(Math.pow(2, l))
}
}
const cds = require('@sap/cds')

/**
* Adds a log level to react to.
* @param {number} level - the level to react to.
*/
add(level) {
this.mask = this.mask | level
}

/**
* Ignores a log level.
* @param {number} level - the level to ignore.
*/
ignore(level) {
this.mask = this.mask ^ level
}

/**
* Attempts to log a message.
* Only iff levelName is a valid log level
* and the corresponding number if part of mask,
* the message gets logged.
* @param {Levels} levelName - name of the log level.
* @param {string} message - message to log.
*/
_log(levelName, message) {
const level = Levels[levelName]
if (level && (this.mask & level) === level) {
// eslint-disable-next-line no-console
console.log(`[${levelName}]`, message)
}
}
}
const _keyFor = value => Object.entries(cds.log.levels).find(([,val]) => val === value)?.[0]

// workaround until retroactively setting log level to 0 is possible
cds.log('cds-typer', _keyFor(cds.log.levels.SILENT))
module.exports = {
Levels,
Logger,
_keyFor,
setLevel: level => { cds.log('cds-typer', level) },
deprecated: {
WARNING: 'WARN',
CRITICAL: 'ERROR',
NONE: 'SILENT'
},
get LOG () { return cds.log('cds-typer') }
}
4 changes: 2 additions & 2 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,9 @@ const parseCommandlineArgs = (argv, validFlags) => {
named[arg] = validFlags[arg].default
}

const allowed = validFlags[arg].allowed
const { allowed, allowedHint } = validFlags[arg]
if (allowed && !allowed.includes(named[arg])) {
throw new Error(`invalid value '${named[arg]}' for flag ${arg}. Must be one of ${allowed.join(', ')}`)
throw new Error(`invalid value '${named[arg]}' for flag ${arg}. Must be one of ${(allowedHint ?? allowed.join(', '))}`)
}
}
} else {
Expand Down
30 changes: 14 additions & 16 deletions lib/visitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { amendCSN, isView, isUnresolved, propagateForeignKeys, isDraftEnabled, is
const { SourceFile, FileRepository, Buffer } = require('./file')
const { FlatInlineDeclarationResolver, StructuredInlineDeclarationResolver } = require('./components/inline')
const { Resolver } = require('./components/resolver')
const { Logger } = require('./logging')
const { LOG } = require('./logging')
const { docify } = require('./components/wrappers')
const { csnToEnumPairs, propertyToInlineEnumName, isInlineEnumType, stringifyEnumType } = require('./components/enum')
const { isReferenceType } = require('./components/reference')
Expand Down Expand Up @@ -40,13 +40,11 @@ class Visitor {
/**
* @param {{xtended: CSN, inferred: CSN}} csn - root CSN
* @param {VisitorOptions} options
* @param {Logger} logger
*/
constructor(csn, options = {}, logger = new Logger()) {
constructor(csn, options = {}) {
amendCSN(csn.xtended)
propagateForeignKeys(csn.inferred)
this.options = { ...defaults, ...options }
this.logger = logger
this.csn = csn

/** @type {Context[]} **/
Expand Down Expand Up @@ -76,7 +74,7 @@ class Visitor {
} else if (isProjection(entity) || !isUnresolved(entity)) {
this.visitEntity(name, entity)
} else {
this.logger.warning(`Skipping unresolved entity: ${name}`)
LOG.warn(`Skipping unresolved entity: ${name}`)
}
}
// FIXME: optimise
Expand All @@ -97,7 +95,7 @@ class Visitor {
const target = this.csn.xtended.definitions[targetName] ?? this.csn.inferred.definitions[targetName]
this.visitEntity(name, target)
} else {
this.logger.error(`Expecting an autoexposed projection within a service. Skipping ${name}`)
LOG.error(`Expecting an autoexposed projection within a service. Skipping ${name}`)
}
}
}
Expand Down Expand Up @@ -147,7 +145,7 @@ class Visitor {
const enums = []
for (let [ename, element] of Object.entries(entity.elements ?? {})) {
if (element.target && /\.texts?/.test(element.target)) {
this.logger.warning(`referring to .texts property in ${name}. This is currently not supported and will be ignored.`)
LOG.warn(`referring to .texts property in ${name}. This is currently not supported and will be ignored.`)
continue
}
this.visitElement(ename, element, file, buffer)
Expand All @@ -161,7 +159,7 @@ class Visitor {
if (this.resolver.getMaxCardinality(element) === 1 && typeof element.on !== 'object') { // FIXME: kelement?
const foreignKey = `${ename}_${kname}`
if (Object.hasOwn(entity.elements, foreignKey)) {
this.logger.error(`Attempting to generate a foreign key reference called '${foreignKey}' in type definition for entity ${name}. But a property of that name is already defined explicitly. Consider renaming that property.`)
LOG.error(`Attempting to generate a foreign key reference called '${foreignKey}' in type definition for entity ${name}. 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
Expand Down Expand Up @@ -253,13 +251,13 @@ class Visitor {
// part of the name, so "A.B" and "A.Bs" just become "B" and "Bs" to be compared.
// FIXME: put this in a util function
if (plural.split('.').at(-1) === `${singular.split('.').at(-1)}_`) {
this.logger.warning(
LOG.warn(
`Derived singular and plural forms for '${singular}' are the same. This usually happens when your CDS entities are named following singular flexion. Consider naming your entities in plural or providing '@singular:'/ '@plural:' annotations to have a clear distinction between the two. Plural form will be renamed to '${plural}' to avoid compilation errors within the output.`
)
}
// as types are not inflected, their singular will always clash and there is also no plural for them anyway -> skip
if (!isType(entity) && `${ns.asNamespace()}.${singular}` in this.csn.xtended.definitions) {
this.logger.error(
LOG.error(
`Derived singular '${singular}' for your entity '${name}', 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.`
)
}
Expand Down Expand Up @@ -348,7 +346,7 @@ class Visitor {
* @param {'function' | 'action'} kind
*/
#printOperation(name, operation, kind) {
this.logger.debug(`Printing operation ${name}:\n${JSON.stringify(operation, null, 2)}`)
LOG.debug(`Printing operation ${name}:\n${JSON.stringify(operation, null, 2)}`)
const ns = this.resolver.resolveNamespace(name.split('.'))
const file = this.fileRepository.getNamespaceFile(ns)
const params = this.#stringifyFunctionParams(operation.params, file)
Expand All @@ -363,7 +361,7 @@ class Visitor {
}

#printType(name, type) {
this.logger.debug(`Printing type ${name}:\n${JSON.stringify(type, null, 2)}`)
LOG.debug(`Printing type ${name}:\n${JSON.stringify(type, null, 2)}`)
const { namespace: ns, name: clean } = this.resolver.untangle(name)
const file = this.fileRepository.getNamespaceFile(ns)
// skip references to enums.
Expand All @@ -379,7 +377,7 @@ class Visitor {
}

#printAspect(name, aspect) {
this.logger.debug(`Printing aspect ${name}`)
LOG.debug(`Printing aspect ${name}`)
const { namespace: ns, name: clean } = this.resolver.untangle(name)
const file = this.fileRepository.getNamespaceFile(ns)
// aspects are technically classes and can therefore be added to the list of defined classes.
Expand All @@ -391,7 +389,7 @@ class Visitor {
}

#printEvent(name, event) {
this.logger.debug(`Printing event ${name}`)
LOG.debug(`Printing event ${name}`)
const { namespace: ns, name: clean } = this.resolver.untangle(name)
const file = this.fileRepository.getNamespaceFile(ns)
file.addEvent(clean, name)
Expand All @@ -409,7 +407,7 @@ class Visitor {
}

#printService(name, service) {
this.logger.debug(`Printing service ${name}:\n${JSON.stringify(service, null, 2)}`)
LOG.debug(`Printing service ${name}:\n${JSON.stringify(service, null, 2)}`)
const ns = this.resolver.resolveNamespace(name)
const file = this.fileRepository.getNamespaceFile(ns)
// service.name is clean of namespace
Expand Down Expand Up @@ -449,7 +447,7 @@ class Visitor {
this.#printService(name, entity)
break
default:
this.logger.debug(`Unhandled entity kind '${entity.kind}'.`)
LOG.debug(`Unhandled entity kind '${entity.kind}'.`)
}
}

Expand Down
Loading
Loading