Skip to content

Commit

Permalink
Reuse cds.log instead of custom logging logic (#265)
Browse files Browse the repository at this point in the history
  • Loading branch information
daogrady committed Aug 26, 2024
1 parent d360de7 commit e1e102e
Show file tree
Hide file tree
Showing 8 changed files with 62 additions and 112 deletions.
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 { normalize } = require('path')
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 @@ const { Visitor } = require('./visitor')
* 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 @@ const writeJsConfig = (path, logger) => {
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 @@ -46,14 +45,13 @@ const writeJsConfig = (path, logger) => {
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 @@ -257,13 +255,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 @@ -352,7 +350,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 @@ -367,7 +365,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 @@ -383,7 +381,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 @@ -395,7 +393,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 @@ -413,7 +411,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 @@ -453,7 +451,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

0 comments on commit e1e102e

Please sign in to comment.