Skip to content

Improve console.log detection in pre-commit hook#2064

Open
krataratha wants to merge 1 commit into
affaan-m:mainfrom
krataratha:patch-1
Open

Improve console.log detection in pre-commit hook#2064
krataratha wants to merge 1 commit into
affaan-m:mainfrom
krataratha:patch-1

Conversation

@krataratha
Copy link
Copy Markdown

@krataratha krataratha commented May 26, 2026

Enhance console log check to report line numbers.

What Changed

Why This Change

Testing Done

  • Manual testing completed
  • Automated tests pass locally (node tests/run-all.js)
  • Edge cases considered and tested

Type of Change

  • fix: Bug fix
  • feat: New feature
  • refactor: Code refactoring
  • docs: Documentation
  • test: Tests
  • chore: Maintenance/tooling
  • ci: CI/CD changes

Security & Quality Checklist

  • No secrets or API keys committed (ghp_, sk-, AKIA, xoxb, xoxp patterns checked)
  • JSON files validate cleanly
  • Shell scripts pass shellcheck (if applicable)
  • Pre-commit hooks pass locally (if configured)
  • No sensitive data exposed in logs or output
  • Follows conventional commits format

Documentation

  • Updated relevant documentation
  • Added comments for complex logic
  • README updated (if needed)

Summary by CodeRabbit

  • Chores
    • Enhanced internal code quality checks to provide more detailed diagnostics during development workflow.

No end-user visible changes in this release.

Review Change Stack


Summary by cubic

Pre-commit hook now reports exact line numbers for any console.log in staged JS/TS files, making cleanups faster. It warns with file and line numbers but does not block commits.

  • New Features
    • Lists line numbers for console.log in .js, .jsx, .ts, and .tsx files.
    • Keeps current behavior: warn only (no commit blocking).

Written for commit f2fbe65. Summary will update on new commits. Review in cubic

Enhance console log check to report line numbers.
Copilot AI review requested due to automatic review settings May 26, 2026 15:19
@krataratha krataratha requested a review from affaan-m as a code owner May 26, 2026 15:19
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 26, 2026

📝 Walkthrough

Walkthrough

The pre-commit hook check-console-log.js now performs line-by-line scanning of modified JavaScript and TypeScript files to identify and report specific line numbers where console.log statements appear, replacing the previous substring-matching approach while maintaining all error handling and output semantics.

Changes

Console Log Detection Enhancement

Layer / File(s) Summary
Per-file, per-line console.log scanning and reporting
scripts/hooks/check-console-log.js
The stdin handler now iterates through modified JS/TS files, scans each line with a regex to find console.log occurrences, collects matching line numbers, and logs a detailed warning including the filename and line references. The aggregate hasConsole flag, git-repo validation, error handling, and original data passthrough remain unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hook that once squinted, now sees with precision,
Line numbers and warnings with surgical vision.
Each console.log caught, no escape from the net,
Developers thanking this gift, you can bet!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: improving console.log detection in the pre-commit hook to report line numbers.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates the check-console-log hook to scan modified JS/TS files for console.log usage at commit time and emit warnings with file + line numbers.

Changes:

  • Added logic to enumerate modified js/jsx/ts/tsx files and scan their contents for console.log.
  • Added warning output listing matching line numbers and a follow-up reminder message.

Comment on lines +40 to 86
process.stdin.on('end', () => {
try {
if (!isGitRepo()) {
process.stdout.write(data);
process.exit(0);
}

const files = getGitModifiedFiles(['\\.tsx?$', '\\.jsx?$'])
.filter(f => fs.existsSync(f))
.filter(f => !EXCLUDED_PATTERNS.some(pattern => pattern.test(f)));

let hasConsole = false;

for (const file of files) {
const content = readFile(file);
if (content) {
const lines = content.split('\n');
const matchedLines = [];

lines.forEach((line, index) => {
if (line.includes('console.log')) {
matchedLines.push(index + 1);
}
});

if (matchedLines.length > 0) {
log(`[Hook] WARNING: console.log found in ${file} on line(s): ${matchedLines.join(', ')}`);
hasConsole = true;
}
}
}

if (hasConsole) {
log('[Hook] Remove console.log statements before committing');
}
} catch (err) {
log(`[Hook] check-console-log error: ${err.message}`);
}

// Always output the original data
process.stdout.write(data);
process.exit(0);
});
}
});

process.stdin.on('end', () => {
Comment on lines +59 to +63
lines.forEach((line, index) => {
if (line.includes('console.log')) {
matchedLines.push(index + 1);
}
});
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/hooks/check-console-log.js`:
- Around line 83-84: There is a stray/extra closing brace and a duplicated
process.stdin.on('end', ...) handler causing a SyntaxError and duplicate logic;
open scripts/hooks/check-console-log.js, remove the second/duplicated
process.stdin.on('end', ...) block (the stale handler) and fix the unmatched
closing brace(s) so the remaining process.stdin.on('end', ...) callback and
surrounding closure/parens are balanced (ensure process.stdin.on('end', ...) and
any surrounding process.stdin.resume()/setEncoding calls remain intact and only
appear once).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ee73b36d-28c4-45c0-b199-da829315edb7

📥 Commits

Reviewing files that changed from the base of the PR and between 928076c and f2fbe65.

📒 Files selected for processing (1)
  • scripts/hooks/check-console-log.js

Comment on lines +83 to +84
}
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="scripts/hooks/check-console-log.js"

echo "end-handler registrations:"
rg -n "process\\.stdin\\.on\\('end'" "$FILE"

echo
echo "context around parse-error region:"
nl -ba "$FILE" | sed -n '76,100p'

Repository: affaan-m/ECC

Length of output: 251


🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="scripts/hooks/check-console-log.js"

echo "end-handler registrations:"
rg -n "process\.stdin\.on\('end'" "$FILE"

echo
echo "node syntax check:"
node --check "$FILE"

echo
echo "context around 76-100:"
awk 'NR>=76 && NR<=100 {print NR ":" $0}' "$FILE"

Repository: affaan-m/ECC

Length of output: 426


Fix the syntax error and remove the duplicated stdin end handler in scripts/hooks/check-console-log.js.

  • scripts/hooks/check-console-log.js fails to parse: SyntaxError: Unexpected token '}' at line 83.
  • The hook registers process.stdin.on('end', ...) twice: at line 40 and line 86 (stale/duplicated logic).
🧰 Tools
🪛 Biome (2.4.15)

[error] 83-84: Expected a statement but instead found '}
})'.

(parse)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/hooks/check-console-log.js` around lines 83 - 84, There is a
stray/extra closing brace and a duplicated process.stdin.on('end', ...) handler
causing a SyntaxError and duplicate logic; open
scripts/hooks/check-console-log.js, remove the second/duplicated
process.stdin.on('end', ...) block (the stale handler) and fix the unmatched
closing brace(s) so the remaining process.stdin.on('end', ...) callback and
surrounding closure/parens are balanced (ensure process.stdin.on('end', ...) and
any surrounding process.stdin.resume()/setEncoding calls remain intact and only
appear once).

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/hooks/check-console-log.js">

<violation number="1" location="scripts/hooks/check-console-log.js:40">
P1: Duplicate `process.stdin.on('end')` handler left in file, creating unreachable dead code that will never execute and risks misleading future edits</violation>

<violation number="2" location="scripts/hooks/check-console-log.js:45">
P0: Remove the stray closing brace/paren pair; they make the hook file syntactically invalid.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

if (!isGitRepo()) {
process.stdout.write(data);
process.exit(0);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0: Remove the stray closing brace/paren pair; they make the hook file syntactically invalid.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/hooks/check-console-log.js, line 45:

<comment>Remove the stray closing brace/paren pair; they make the hook file syntactically invalid.</comment>

<file context>
@@ -37,6 +37,52 @@ process.stdin.on('data', chunk => {
+    if (!isGitRepo()) {
+      process.stdout.write(data);
+      process.exit(0);
+    }
+
+    const files = getGitModifiedFiles(['\\.tsx?$', '\\.jsx?$'])
</file context>

@@ -37,6 +37,52 @@ process.stdin.on('data', chunk => {
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Duplicate process.stdin.on('end') handler left in file, creating unreachable dead code that will never execute and risks misleading future edits

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/hooks/check-console-log.js, line 40:

<comment>Duplicate `process.stdin.on('end')` handler left in file, creating unreachable dead code that will never execute and risks misleading future edits</comment>

<file context>
@@ -37,6 +37,52 @@ process.stdin.on('data', chunk => {
   }
 });
 
+process.stdin.on('end', () => {
+  try {
+    if (!isGitRepo()) {
</file context>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants