Skip to content

Commit 6788929

Browse files
committed
add more test cases
1 parent bb862e4 commit 6788929

File tree

2 files changed

+54
-8
lines changed

2 files changed

+54
-8
lines changed

plugins/eslint-plugin-aws-toolkits/lib/rules/no-json-stringify-in-log.ts

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ import { Rule } from 'eslint'
88

99
export const errMsg = 'Avoid using JSON.stringify within logging and error messages, prefer %O.'
1010

11-
function isJsonStringifyCall(node: any) {
11+
/**
12+
* Check if a given expression is a JSON.stringify call.
13+
*/
14+
15+
function isJsonStringifyCall(node: any): boolean {
1216
return (
1317
node.type === AST_NODE_TYPES.CallExpression &&
1418
node.callee.type === AST_NODE_TYPES.MemberExpression &&
@@ -19,6 +23,39 @@ function isJsonStringifyCall(node: any) {
1923
)
2024
}
2125

26+
/**
27+
* Check if node is representing syntax of the form getLogger().f(msg) for some f and msg.
28+
*
29+
*/
30+
function isLoggerCall(node: any): boolean {
31+
return (
32+
node.callee.type === AST_NODE_TYPES.MemberExpression &&
33+
node.callee.object.type === AST_NODE_TYPES.CallExpression &&
34+
node.callee.object.callee.type === AST_NODE_TYPES.Identifier &&
35+
node.callee.object.callee.name === 'getLogger'
36+
)
37+
}
38+
39+
/**
40+
* Use two simple heuristics to detect `disguised` logger calls. This is when we log without getLogger.
41+
* Ex.
42+
* const logger = getLogger()
43+
* logger.debug(m)
44+
* To catch these we try two checks:
45+
* 1) If the left side is an identifier including the word logger
46+
* 2) If the left side is a property of some object, including the word logger.
47+
*/
48+
function isDisguisedLoggerCall(node: any): boolean {
49+
return (
50+
(node.callee.type === AST_NODE_TYPES.MemberExpression &&
51+
node.callee.object.type === AST_NODE_TYPES.Identifier &&
52+
node.callee.object.name.toLowerCase().includes('logger')) ||
53+
(node.callee.type === AST_NODE_TYPES.MemberExpression &&
54+
node.callee.object.type === AST_NODE_TYPES.MemberExpression &&
55+
node.callee.object.property.name.toLowerCase().includes('logger'))
56+
)
57+
}
58+
2259
export default ESLintUtils.RuleCreator.withoutDocs({
2360
meta: {
2461
docs: {
@@ -36,13 +73,8 @@ export default ESLintUtils.RuleCreator.withoutDocs({
3673
create(context) {
3774
return {
3875
CallExpression(node) {
39-
// Look for a getLogger().f() call.
40-
if (
41-
node.callee.type === AST_NODE_TYPES.MemberExpression &&
42-
node.callee.object.type === AST_NODE_TYPES.CallExpression &&
43-
node.callee.object.callee.type === AST_NODE_TYPES.Identifier &&
44-
node.callee.object.callee.name === 'getLogger'
45-
) {
76+
// Look for a getLogger().f() call or a disguised one, see isDisguisedLoggerCall for more info.
77+
if (isLoggerCall(node) || isDisguisedLoggerCall(node)) {
4678
// For each argument to the call above, check if it contains a JSON.stringify
4779
node.arguments.forEach((arg) => {
4880
// Check if arg itself if a JSON.stringify call

plugins/eslint-plugin-aws-toolkits/test/rules/no-json-stringify-in-log.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ getRuleTester().run('no-json-stringify-in-log', rules['no-json-stringify-in-log'
1414
'getLogger().debug(`this does not include anything`)',
1515
'getLogger().debug(`another example ${JSON.notString(something)}`)',
1616
'getLogger().fakeFunction(`another example ${JSON.notString(something)}`)',
17+
'this.deps.devLogger?.debug("crashMonitoring: CLEAR_STATE: Succeeded")',
18+
'getLogger().debug(`called startBuilderIdSetup()`)',
1719
],
1820

1921
invalid: [
@@ -37,5 +39,17 @@ getRuleTester().run('no-json-stringify-in-log', rules['no-json-stringify-in-log'
3739
code: 'getLogger().verbose(`provideDebugConfigurations: debugconfigs: ${JSON.stringify(configs)}`)',
3840
errors: [errMsg],
3941
},
42+
{
43+
code: 'devLogger?.debug(`crashMonitoring: CHECKED: Result of cleaning up crashed instances\nBEFORE: ${JSON.stringify(before)}\nAFTER: ${JSON.stringify(after)}\nACTUAL: ${JSON.stringify(afterActual)}`)',
44+
errors: [errMsg, errMsg, errMsg],
45+
},
46+
{
47+
code: 'getLogger().warn(`skipping invalid item in telemetry cache: ${JSON.stringify(item)}\n`)',
48+
errors: [errMsg],
49+
},
50+
{
51+
code: 'this.deps.devLogger?.debug(`crashMonitoring: CLEAR_STATE: Succeeded ${JSON.stringify(item)}`)',
52+
errors: [errMsg],
53+
},
4054
],
4155
})

0 commit comments

Comments
 (0)