Skip to content

Commit

Permalink
chore(toolkit): enforce codes at compile time (#32893)
Browse files Browse the repository at this point in the history 
### Reason for this change

Instead of running regexp at runtime, we validate message codes at
compile time

### Checklist
- [x] My code adheres to the [CONTRIBUTING
GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and
[DESIGN
GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license*
  • Loading branch information
@HBobertz
HBobertz authored Jan 13, 2025
2 parents c9bc8ad + 0f2fb63 commit e104e60
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 135 deletions.
36 changes: 16 additions & 20 deletions packages/aws-cdk/lib/logging.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as util from 'util';
import * as chalk from 'chalk';
import { IoMessageLevel, IoMessage, CliIoHost, validateMessageCode } from './toolkit/cli-io-host';
import { IoMessageLevel, IoMessage, CliIoHost, IoMessageSpecificCode, IoMessageCode, IoMessageCodeCategory, IoCodeLevel } from './toolkit/cli-io-host';

// Corking mechanism
let CORK_COUNTER = 0;
Expand Down Expand Up @@ -74,7 +74,7 @@ interface LogOptions {
* @pattern [A-Z]+_[0-2][0-9]{3}
* @default TOOLKIT_[0/1/2]000
*/
readonly code: string;
readonly code: IoMessageCode;
}

/**
Expand All @@ -86,8 +86,6 @@ function log(options: LogOptions) {
return;
}

validateMessageCode(options.code, options.level);

const ioMessage: IoMessage = {
level: options.level,
message: options.message,
Expand All @@ -113,7 +111,7 @@ function log(options: LogOptions) {
function formatMessageAndLog(
level: IoMessageLevel,
forceStdout: boolean,
input: LogInput,
input: LogInput<IoCodeLevel>,
style?: (str: string) => string,
...args: unknown[]
): void {
Expand All @@ -128,8 +126,6 @@ function formatMessageAndLog(
// Apply style if provided
const finalMessage = style ? style(formattedMessage) : formattedMessage;

validateMessageCode(code, level);

log({
level,
message: finalMessage,
Expand All @@ -138,27 +134,27 @@ function formatMessageAndLog(
});
}

function getDefaultCode(level: IoMessageLevel, category: string = 'TOOLKIT'): string {
function getDefaultCode(level: IoMessageLevel, category: IoMessageCodeCategory = 'TOOLKIT'): IoMessageCode {
const levelIndicator = level === 'error' ? 'E' :
level === 'warn' ? 'W' :
'I';
return `CDK_${category}_${levelIndicator}000`;
return `CDK_${category}_${levelIndicator}0000`;
}

// Type for the object parameter style
interface LogParams {
interface LogParams<L extends IoCodeLevel> {
/**
* @see {@link IoMessage.code}
*/
readonly code?: string;
readonly code?: IoMessageSpecificCode<L>;
/**
* @see {@link IoMessage.message}
*/
readonly message: string;
}

// Type for the exported log function arguments
type LogInput = string | LogParams;
type LogInput<L extends IoCodeLevel> = string | LogParams<L>;

// Exported logging functions. If any additional logging functionality is required, it should be added as
// a new logging function here.
Expand All @@ -174,7 +170,7 @@ type LogInput = string | LogParams;
* error({ message: 'operation failed: %s', code: 'CDK_SDK_E001' }, e) // specifies error code `CDK_SDK_E001`
* ```
*/
export const error = (input: LogInput, ...args: unknown[]) => {
export const error = (input: LogInput<'E'>, ...args: unknown[]) => {
return formatMessageAndLog('error', false, input, undefined, ...args);
};

Expand All @@ -189,7 +185,7 @@ export const error = (input: LogInput, ...args: unknown[]) => {
* warning({ message: 'deprected feature: %s', code: 'CDK_SDK_W001' }, message) // specifies warning code `CDK_SDK_W001`
* ```
*/
export const warning = (input: LogInput, ...args: unknown[]) => {
export const warning = (input: LogInput<'W'>, ...args: unknown[]) => {
return formatMessageAndLog('warn', false, input, undefined, ...args);
};

Expand All @@ -204,7 +200,7 @@ export const warning = (input: LogInput, ...args: unknown[]) => {
* info({ message: 'processing: %s', code: 'CDK_TOOLKIT_I001' }, message) // specifies info code `CDK_TOOLKIT_I001`
* ```
*/
export const info = (input: LogInput, ...args: unknown[]) => {
export const info = (input: LogInput<'I'>, ...args: unknown[]) => {
return formatMessageAndLog('info', false, input, undefined, ...args);
};

Expand All @@ -219,7 +215,7 @@ export const info = (input: LogInput, ...args: unknown[]) => {
* data({ message: 'stats: %j', code: 'CDK_DATA_I001' }, stats) // specifies info code `CDK_DATA_I001`
* ```
*/
export const data = (input: LogInput, ...args: unknown[]) => {
export const data = (input: LogInput<'I'>, ...args: unknown[]) => {
return formatMessageAndLog('info', true, input, undefined, ...args);
};

Expand All @@ -234,7 +230,7 @@ export const data = (input: LogInput, ...args: unknown[]) => {
* debug({ message: 'ratio: %d%%', code: 'CDK_TOOLKIT_I001' }, ratio) // specifies info code `CDK_TOOLKIT_I001`
* ```
*/
export const debug = (input: LogInput, ...args: unknown[]) => {
export const debug = (input: LogInput<'I'>, ...args: unknown[]) => {
return formatMessageAndLog('debug', false, input, undefined, ...args);
};

Expand All @@ -249,7 +245,7 @@ export const debug = (input: LogInput, ...args: unknown[]) => {
* trace({ message: 'method: %s', code: 'CDK_TOOLKIT_I001' }, name) // specifies info code `CDK_TOOLKIT_I001`
* ```
*/
export const trace = (input: LogInput, ...args: unknown[]) => {
export const trace = (input: LogInput<'I'>, ...args: unknown[]) => {
return formatMessageAndLog('trace', false, input, undefined, ...args);
};

Expand All @@ -264,7 +260,7 @@ export const trace = (input: LogInput, ...args: unknown[]) => {
* success({ message: 'items: %d', code: 'CDK_TOOLKIT_I001' }, count) // specifies info code `CDK_TOOLKIT_I001`
* ```
*/
export const success = (input: LogInput, ...args: unknown[]) => {
export const success = (input: LogInput<'I'>, ...args: unknown[]) => {
return formatMessageAndLog('info', false, input, chalk.green, ...args);
};

Expand All @@ -279,6 +275,6 @@ export const success = (input: LogInput, ...args: unknown[]) => {
* highlight({ message: 'notice: %s', code: 'CDK_TOOLKIT_I001' }, msg) // specifies info code `CDK_TOOLKIT_I001`
* ```
*/
export const highlight = (input: LogInput, ...args: unknown[]) => {
export const highlight = (input: LogInput<'I'>, ...args: unknown[]) => {
return formatMessageAndLog('info', false, input, chalk.bold, ...args);
};
56 changes: 12 additions & 44 deletions packages/aws-cdk/lib/toolkit/cli-io-host.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import * as chalk from 'chalk';

export type IoMessageCodeCategory = 'TOOLKIT' | 'SDK' | 'ASSETS';
export type IoCodeLevel = 'E' | 'W' | 'I';
export type IoMessageSpecificCode<L extends IoCodeLevel> = `CDK_${IoMessageCodeCategory}_${L}${number}${number}${number}${number}`;
export type IoMessageCode = IoMessageSpecificCode<IoCodeLevel>;

/**
* Basic message structure for toolkit notifications.
* Messages are emitted by the toolkit and handled by the IoHost.
Expand Down Expand Up @@ -36,7 +41,7 @@ export interface IoMessage {
* 'CDK_SDK_W023' // valid: specific sdk warning message
* ```
*/
readonly code: string;
readonly code: IoMessageCode;

/**
* The message text.
Expand Down Expand Up @@ -73,10 +78,15 @@ export class CliIoHost {
return CliIoHost.instance;
}

/**
* Singleton instance of the CliIoHost
*/
private static instance: CliIoHost | undefined;

/**
* Determines which output stream to use based on log level and configuration.
*/
public static getStream(level: IoMessageLevel, forceStdout: boolean) {
private static getStream(level: IoMessageLevel, forceStdout: boolean) {
// For legacy purposes all log streams are written to stderr by default, unless
// specified otherwise, by passing `forceStdout`, which is used by the `data()` logging function, or
// if the CDK is running in a CI environment. This is because some CI environments will immediately
Expand All @@ -89,11 +99,6 @@ export class CliIoHost {
return this.ci ? process.stdout : process.stderr;
}

/**
* Singleton instance of the CliIoHost
*/
private static instance: CliIoHost | undefined;

/**
* Whether the host should apply chalk styles to messages. Defaults to false if the host is not running in a TTY.
*
Expand Down Expand Up @@ -186,43 +191,6 @@ export class CliIoHost {
}
}

/**
* Validates that a message code follows the required format:
* CDK_[CATEGORY]_[E/W/I][000-999]
*
* Examples:
* - CDK_ASSETS_E005 (specific asset error)
* - CDK_SDK_W001 (specific SDK warning)
* - CDK_TOOLKIT_I000 (generic toolkit info)
*
* @param code The message code to validate
* @param level The message level (used to validate level indicator matches)
* @throws Error if the code format is invalid
*/
export function validateMessageCode(code: string, level: IoMessageLevel): void {
const MESSAGE_CODE_PATTERN = /^CDK_[A-Z]+_[EWI][0-9]{3}$/;
if (!MESSAGE_CODE_PATTERN.test(code)) {
throw new Error(
`Invalid message code format: "${code}". ` +
'Code must match pattern: CDK_[CATEGORY]_[E/W/I][000-999]',
);
}

// Extract level indicator from code (E/W/I after second underscore)
const levelIndicator = code.split('_')[2][0];

// Validate level indicator matches message level
const expectedIndicator = level === 'error' ? 'E' :
level === 'warn' ? 'W' : 'I';

if (levelIndicator !== expectedIndicator) {
throw new Error(
`Message code level indicator '${levelIndicator}' does not match message level '${level}'. ` +
`Expected '${expectedIndicator}'`,
);
}
}

export const styleMap: Record<IoMessageLevel, (str: string) => string> = {
error: chalk.red,
warn: chalk.yellow,
Expand Down
11 changes: 3 additions & 8 deletions packages/aws-cdk/test/api/logs/logging.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,14 +235,9 @@ describe('logging', () => {
describe('message codes', () => {
test('validates message codes correctly', () => {
// Valid codes
expect(() => error({ message: 'test', code: 'CDK_TOOLKIT_E001' })).not.toThrow();
expect(() => warning({ message: 'test', code: 'CDK_ASSETS_W499' })).not.toThrow();
expect(() => info({ message: 'test', code: 'CDK_SDK_I000' })).not.toThrow();

// Invalid codes
expect(() => error({ message: 'test', code: 'ERROR_E001' })).toThrow();
expect(() => warning({ message: 'test', code: 'CDK_WARN_300' })).toThrow();
expect(() => info({ message: 'test', code: 'CDK_001' })).toThrow();
expect(() => error({ message: 'test', code: 'CDK_TOOLKIT_E0001' })).not.toThrow();
expect(() => warning({ message: 'test', code: 'CDK_ASSETS_W4999' })).not.toThrow();
expect(() => info({ message: 'test', code: 'CDK_SDK_I0000' })).not.toThrow();
});

test('uses default codes when none provided', () => {
Expand Down
Loading

0 comments on commit e104e60

Please sign in to comment.