Skip to content

fix: eliminate variable shadowing in acceptParams function - #6981

Closed
SAURABHHAM44 wants to merge 1 commit into
expressjs:masterfrom
SAURABHHAM44:patch-1
Closed

SAURABHHAM44 wants to merge 1 commit into
expressjs:masterfrom
SAURABHHAM44:patch-1

Conversation

@SAURABHHAM44

Copy link
Copy Markdown

What does this PR do?

This PR fixes a variable shadowing issue in the acceptParams function in utils.js. The inner loop was redeclaring the 'colonIndex' variable, which shadowed the outer scope's variable with the same name. This has been fixed by renaming the inner variable to 'nextColonIndex'.

Why is this change needed?

Variable shadowing can lead to confusing code and potential bugs. By using distinct variable names for different purposes (colonIndex for the first semicolon search, nextColonIndex for subsequent searches within the loop), the code becomes clearer and easier to maintain. This follows best practices for code quality and readability.

How was this tested?

This is a code quality fix with no functional changes. The renaming maintains the exact same behavior - variable shadowing did not affect the logic, it was purely a code clarity issue.

### What does this PR do?
This PR fixes a variable shadowing issue in the acceptParams function in utils.js. The inner loop was redeclaring the 'colonIndex' variable, which shadowed the outer scope's variable with the same name. This has been fixed by renaming the inner variable to 'nextColonIndex'.

### Why is this change needed?
Variable shadowing can lead to confusing code and potential bugs. By using distinct variable names for different purposes (colonIndex for the first semicolon search, nextColonIndex for subsequent searches within the loop), the code becomes clearer and easier to maintain. This follows best practices for code quality and readability.

### How was this tested?
This is a code quality fix with no functional changes. The renaming maintains the exact same behavior - variable shadowing did not affect the logic, it was purely a code clarity issue.

@jonchurch jonchurch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This change introduces a bug w/ an infinite loop. The code works fine as it is, and refactoring for the sake of it risks introducing the exact kind of bug this PR introduces.

@jonchurch jonchurch closed this Jan 11, 2026
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