Why is this a problem? Straight forward 'for' loop

SonarQube Cloud
1

Template for a good new topic, formatted with Markdown:

  • ALM used:
    • GitHub cloned project from BitBucket (used the mirror technique)
  • CI system used (Bitbucket Cloud, Azure DevOps, Travis CI, Circle CI
    • Not using (internally we use Jenkins on a VM)
  • Scanner command used when applicable (private details masked)
    • Uploaded to SonarCloud (14 day trial, but I’ve ALREADY PAID FOR THE MONTH via BitBucket. Apparently that isn’t transferable)
  • Languages of the repository
    • Java
    • Python
  • Only if the SonarCloud project is public, the URL
    • Private
  • Error observed
    • Seems like good code?
  • Steps to reproduce
    • Scan a project
  • Potential workaround
    • Ignore “Blockers”

I have some JavaScript code for a Lambda:

getannualTaxSheildAmount: function(financedAmount, emi, rate, year, pFee, quoteData) {
		// ... 
		var tenureInMonth = year * 12;
		for (i = 1; i <= tenureInMonth; i++) {
			var tempInterest = balanceAmount[i - 1] * monthlyrate;
			// ... code snipped
		}

I get a severity: Blocker error:

Intentionality Not logical

‘tenureInMonth’ is not modified in this loop.

Loops should not be infinite

Software qualities impacted:

  • Reliability

How in the world is this an infinite loop? I mean, maybe it’s worried about ‘year’ being passed in (yes, I realize the code looks weird) but modifying the end condition inside a straight forward for loop seems weird.

2

Hey @John_Gwinner_nact

Thanks for the report. We’re aware that this rule is pretty noisy, and have a few open tickets to improve it. Specifically, I think that
JS-132 relates to this issue. Can you check?

3

I looked at that Jira ticket, but the use case is pretty different. This is a loop where the variable IS used in the loop statement itself, not in the body. That’s even worse. I haven’t checked it w/eslint though.

5

Hello @John_Gwinner_nact,

I was able to reproduce the FP. This rule, S2189 is extending the ESLint core rule no-unmodified-loop-condition.js, and it is also raising there. After investigating a bit it seems that adding var into the i counter fixes the FP:

for (var i = 1; i <= tenureInMonth; i++) {
   var tempInterest = balanceAmount[i - 1] * monthlyrate;
}

The rule is looking for references of the variables in the condition and without the variable declaration it does not seem to behave as expected.

Added the sample to the previously mentioned ticket.

7

Hi Victor!

Good catch. It’s still a false positive, but that may be a good coding style anyway.

== John ==