diff --git a/.gitignore b/.gitignore index c0c5b24..2209b7f 100644 --- a/.gitignore +++ b/.gitignore @@ -3,4 +3,5 @@ dist/ sample_cases/ wasm-toolchain coverage -reference \ No newline at end of file +reference +.vscode/ \ No newline at end of file diff --git a/docs/rules/logger-in-checker.md b/docs/rules/logger-in-checker.md new file mode 100644 index 0000000..e9bbcd3 --- /dev/null +++ b/docs/rules/logger-in-checker.md @@ -0,0 +1,155 @@ +# logger-in-checker + +> Ensure logger functions are called only inside their corresponding checker if statements + +## Rule Details + +This rule enforces that logger functions must be guarded by their corresponding checker conditions to avoid unnecessary performance costs. + +**Key Problem**: Even when logging is disabled, string interpolation and serialization still execute: + +```ts +// Bad: String is always constructed, even if logging is disabled +debugLogger(`User ${userId} performed action ${action}`); + +// Good: String construction only happens when logging is enabled +if (isDebugEnabled()) { + debugLogger(`User ${userId} performed action ${action}`); +} +``` + +Without the guard, your code wastes CPU cycles building log messages that will never be used. + +## Rule Options + +This rule requires configuration. You must specify an array of logger-checker pairs: + +```json +{ + "assemblyscript/logger-in-checker": [ + "error", + [ + { "loggerName": "debugLogger", "checkerName": "isDebugEnabled" }, + { "loggerName": "traceLogger", "checkerName": "isTraceEnabled" } + ] + ] +} +``` + +Each pair consists of: + +- `loggerName`: The name of the logger function that must be guarded +- `checkerName`: The name of the checker function/variable that must be used in the if condition + +## Examples + +### Incorrect + +```ts +// String serialization happens even if logging is disabled +debugLogger(`Debug message: ${expensiveOperation()}`); + +// Logger called inside wrong checker +if (wrongChecker()) { + debugLogger("Debug message"); +} + +// Logger called outside the checker block +if (isDebugEnabled()) { + // some code +} +debugLogger(`Expensive ${serialization()}`); // String built even if disabled + +// Combined conditions are not supported +if (isDebugEnabled() && otherCondition) { + debugLogger("Debug message"); // Invalid: checker is in combined condition +} + +if (otherCondition || isDebugEnabled()) { + debugLogger("Debug message"); // Invalid: checker is in combined condition +} + +// Comparing checker to non-literal is not supported +if (isDebugEnabled() < someCall()) { + debugLogger("Debug message"); // Invalid: both sides are function calls +} +``` + +### Correct + +```ts +// String only built when logging is enabled +if (isDebugEnabled()) { + debugLogger(`Debug message: ${expensiveOperation()}`); +} + +// Checker as boolean variable +if (isDebugEnabled) { + debugLogger("Debug message"); +} + +// Checker compared to boolean literal +if (isDebugEnabled() === true) { + debugLogger("Debug message"); +} + +if (isDebugEnabled() == false) { + debugLogger("Debug message"); +} + +// Checker with comparison operators +if (logLevel() > 0) { + debugLogger("Debug message"); +} + +if (logLevel() >= 1) { + debugLogger("Debug message"); +} + +if (errorCount() < 10) { + errorLogger("Error occurred"); +} + +if (warningCount() <= 5) { + warningLogger("Warning"); +} + +// Literal on left side also works +if (0 < logLevel()) { + debugLogger("Debug message"); +} + +// Nested if statements are fine +if (isDebugEnabled()) { + if (otherCondition) { + debugLogger("Debug message"); + } +} +``` + +## Configuration Example + +Here's a complete example of how to configure this rule in your ESLint config: + +```javascript +// eslint.config.mjs +import assemblyscript from "assemblyscript-eslint-plugin"; + +export default [ + { + plugins: { + assemblyscript, + }, + rules: { + "assemblyscript/logger-in-checker": [ + "error", + [ + { loggerName: "debugLog", checkerName: "DEBUG_ENABLED" }, + { loggerName: "traceLog", checkerName: "TRACE_ENABLED" }, + { loggerName: "verboseLog", checkerName: "isVerbose" }, + ], + ], + }, + }, +]; +``` diff --git a/plugins/asPlugin.ts b/plugins/asPlugin.ts index 30ceedd..88d0a3c 100644 --- a/plugins/asPlugin.ts +++ b/plugins/asPlugin.ts @@ -10,6 +10,7 @@ import noConcatString from "./rules/noConcatString.js"; import noSpread from "./rules/noSpread.js"; import noUnsupportedKeyword from "./rules/noUnsupportedKeyword.js"; import specifyType from "./rules/specifyType.js"; +import loggerInChecker from "./rules/loggerInChecker.js"; export default { rules: { @@ -18,5 +19,6 @@ export default { "no-unsupported-keyword": noUnsupportedKeyword, "specify-type": specifyType, "no-concat-string": noConcatString, + "logger-in-checker": loggerInChecker, }, }; diff --git a/plugins/rules/loggerInChecker.ts b/plugins/rules/loggerInChecker.ts new file mode 100644 index 0000000..454d971 --- /dev/null +++ b/plugins/rules/loggerInChecker.ts @@ -0,0 +1,146 @@ +import { AST_NODE_TYPES, TSESTree } from "@typescript-eslint/utils"; +import createRule from "../utils/createRule.js"; + +/** + * Rule: Ensure logger functions are called only inside their corresponding checker if statements. + */ + +type LoggerCheckerPair = { + loggerName: string; + checkerName: string; +}; + +type Options = [LoggerCheckerPair[]]; + +/** + * Check if an expression is the checker function or identifier + */ +function containsCheckerCall( + node: TSESTree.Expression, + checkerName: string +): boolean { + // Direct call: checkerName() + if ( + node.type === AST_NODE_TYPES.CallExpression && + node.callee.type === AST_NODE_TYPES.Identifier && + node.callee.name === checkerName + ) { + return true; + } + + // Just the identifier: if(checkerName) + if (node.type === AST_NODE_TYPES.Identifier && node.name === checkerName) { + return true; + } + + // Binary expressions: checkerName() === true, checkerName() > 0, etc. + if (node.type === AST_NODE_TYPES.BinaryExpression) { + const leftIsLiteral = node.left.type === AST_NODE_TYPES.Literal; + const rightIsLiteral = node.right.type === AST_NODE_TYPES.Literal; + + // Left is checker, right is literal + if ( + rightIsLiteral && + node.left.type !== AST_NODE_TYPES.PrivateIdentifier && + containsCheckerCall(node.left, checkerName) + ) { + return true; + } + + // Right is checker, left is literal + if (leftIsLiteral && containsCheckerCall(node.right, checkerName)) { + return true; + } + } + + return false; +} + +/** + * Check if a node is inside an if statement with the required checker function call + */ +function isInsideCheckerIf(node: TSESTree.Node, checkerName: string): boolean { + let current: TSESTree.Node | undefined = node; + + while (current) { + // Check if we're inside an if statement with the checker function + if ( + current.type === AST_NODE_TYPES.IfStatement && + containsCheckerCall(current.test, checkerName) + ) { + return true; + } + + current = current.parent; + } + + return false; +} + +export default createRule({ + name: "logger-in-checker", + meta: { + type: "problem", + docs: { + description: + "Ensure logger functions are called only inside their corresponding checker if statements", + }, + messages: { + loggerNotInChecker: + "Logger function '{{loggerName}}' must be called inside an if({{checkerName}}) statement", + }, + schema: [ + { + type: "array", + items: { + type: "object", + properties: { + loggerName: { + type: "string", + }, + checkerName: { + type: "string", + }, + }, + required: ["loggerName", "checkerName"], + additionalProperties: false, + }, + }, + ], + }, + defaultOptions: [[]], + create(context) { + const [pairs] = context.options; + + if (!pairs || pairs.length === 0) { + return {}; + } + + // Create a map for quick lookup + const loggerToChecker = new Map(); + for (const pair of pairs) { + loggerToChecker.set(pair.loggerName, pair.checkerName); + } + + return { + CallExpression(node) { + // Check if this is a call to one of the logger functions + if (node.callee.type === AST_NODE_TYPES.Identifier) { + const functionName = node.callee.name; + const requiredChecker = loggerToChecker.get(functionName); + + if (requiredChecker && !isInsideCheckerIf(node, requiredChecker)) { + context.report({ + node, + messageId: "loggerNotInChecker", + data: { + loggerName: functionName, + checkerName: requiredChecker, + }, + }); + } + } + }, + }; + }, +}); diff --git a/tests/rules/loggerInChecker.test.ts b/tests/rules/loggerInChecker.test.ts new file mode 100644 index 0000000..aae8277 --- /dev/null +++ b/tests/rules/loggerInChecker.test.ts @@ -0,0 +1,355 @@ +import { createRuleTester } from "../utils/testUtils.js"; +import { describe, it } from "mocha"; +import loggerInChecker from "../../plugins/rules/loggerInChecker.js"; + +describe("Rule: loggerInChecker", () => { + const ruleTester = createRuleTester(); + + it("validates all test cases for loggerInChecker rule", () => { + ruleTester.run("logger-in-checker", loggerInChecker, { + valid: [ + // Logger called inside the correct checker if statement + { + code: ` +if (someChecker()) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Logger called with checker as boolean (no parens) + { + code: ` +if (someChecker) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Logger called inside nested blocks within checker if + { + code: ` +if (someChecker()) { + { + someLogger("message"); + } +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Multiple logger-checker pairs + { + code: ` +if (debugChecker()) { + debugLogger("debug message"); +} +if (errorChecker()) { + errorLogger("error message"); +}`, + options: [ + [ + { loggerName: "debugLogger", checkerName: "debugChecker" }, + { loggerName: "errorLogger", checkerName: "errorChecker" }, + ], + ], + }, + + // Unrelated function calls are allowed + { + code: ` +someOtherFunction(); +anotherFunction();`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Logger not in config can be called anywhere + { + code: ` +unconfiguredLogger("message");`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Empty configuration + { + code: ` +someLogger("message");`, + options: [[]], + }, + + // Nested if statements + { + code: ` +if (someChecker()) { + if (additionalCondition) { + someLogger("message"); + } +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Checker in binary expression with === + { + code: ` +if (someChecker() === true) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Checker in binary expression with == + { + code: ` +if (someChecker() == true) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Checker compared to false + { + code: ` +if (someChecker() === false) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Checker with greater than comparison + { + code: ` +if (someChecker() > 0) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Checker with greater than or equal comparison + { + code: ` +if (someChecker() >= 1) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Checker with less than comparison + { + code: ` +if (someChecker() < 10) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Checker with less than or equal comparison + { + code: ` +if (someChecker() <= 5) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + + // Literal on left side + { + code: ` +if (0 < someChecker()) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + }, + ], + + invalid: [ + // Logger called without checker + { + code: `someLogger("message");`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + data: { + loggerName: "someLogger", + checkerName: "someChecker", + }, + }, + ], + }, + + // Logger called inside wrong checker + { + code: ` +if (wrongChecker()) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + + // Logger called outside checker if + { + code: ` +if (someChecker()) { + // correct usage +} +someLogger("message"); // wrong - outside if`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + + // Multiple violations + { + code: ` +someLogger("message1"); +if (someChecker()) { + someLogger("message2"); // OK +} +someLogger("message3");`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + line: 2, + }, + { + messageId: "loggerNotInChecker", + line: 6, + }, + ], + }, + + // Logger in unrelated if statement + { + code: ` +if (unrelatedCondition) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + + // Logger in for loop + { + code: ` +for (let i = 0; i < 10; i++) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + + // Logger in function body + { + code: ` +function test() { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + + // Multiple logger-checker pairs, one violation + { + code: ` +if (debugChecker()) { + debugLogger("debug"); // OK +} +errorLogger("error"); // Wrong - not in errorChecker`, + options: [ + [ + { loggerName: "debugLogger", checkerName: "debugChecker" }, + { loggerName: "errorLogger", checkerName: "errorChecker" }, + ], + ], + errors: [ + { + messageId: "loggerNotInChecker", + data: { + loggerName: "errorLogger", + checkerName: "errorChecker", + }, + }, + ], + }, + + // Combined conditions are not supported - AND expression + { + code: ` +if (someChecker() && otherCondition) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + + // Combined conditions are not supported - OR expression + { + code: ` +if (otherCondition || someChecker()) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + + // Negated checker is not supported + { + code: ` +if (!someChecker()) { + // don't log +} else { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + + // Binary expression must with literal on one side + { + code: ` +if (someChecker() < someCall()) { + someLogger("message"); +}`, + options: [[{ loggerName: "someLogger", checkerName: "someChecker" }]], + errors: [ + { + messageId: "loggerNotInChecker", + }, + ], + }, + ], + }); + }); +});