Skip to content

Fix NaN comparison, loop logic, and null safety checks in Person.js - #236

Open
sonarqube-agent[bot] wants to merge 1 commit into
masterfrom
remediate-master-20260801-050227-e9f0475f
Open

Fix NaN comparison, loop logic, and null safety checks in Person.js#236
sonarqube-agent[bot] wants to merge 1 commit into
masterfrom
remediate-master-20260801-050227-e9f0475f

Conversation

@sonarqube-agent

Copy link
Copy Markdown

This PR was automatically created by the Remediation Agent's Scheduled backlog remediation feature.

Why these issues? Four MAJOR severity issues in Person.js represent functional bugs with direct impact on correctness and stability. Grouping these related fixes by file creates a cohesive, high-value PR that addresses NaN comparison pitfalls, loop control flow, and null safety patterns in a single, reviewable change.

This PR resolves four MAJOR SonarQube issues in Person.js by replacing unsafe NaN comparisons with Number.isNaN(), converting a broken decrementing loop to a for-of loop, and fixing null dereference vulnerabilities by correcting logical operators from AND to OR in null checks. These fixes eliminate runtime errors and logic bugs that could cause infinite loops, TypeErrors, and incorrect conditional behavior.

View Project in SonarCloud


Fixed Issues

javascript:S2688 - Use the isNaN function to compare with NaN. • MAJORView issue

Location: sonar-scanner/src/javascript/Person.js:17

Why is this an issue?

In JavaScript, NaN stands for "Not-a-Number." It is a special value representing a numeric data type that is not a valid number. NaN is returned as a result when an arithmetic operation or mathematical function is performed, and the result is undefined or unrepresentable as a valid number.

What changed

This hunk replaces the direct comparison a !== NaN with !Number.isNaN(a). Since NaN !== NaN is always true in JavaScript, the original condition a !== NaN was always true regardless of the value of a. Using !Number.isNaN(a) correctly checks whether a is not NaN.

--- a/sonar-scanner/src/javascript/Person.js
+++ b/sonar-scanner/src/javascript/Person.js
@@ -20,2 +20,2 @@ if (a === NaN) {  // Noncompliant; always false
-if (a !== NaN) { // Noncompliant; always true
-  console.log("a is not NaN"); // this statement is not necessarily true
+if (!Number.isNaN(a)) {
+  console.log("a is not NaN");
javascript:S2688 - Use the isNaN function to compare with NaN. • MAJORView issue

Location: sonar-scanner/src/javascript/Person.js:20

Why is this an issue?

In JavaScript, NaN stands for "Not-a-Number." It is a special value representing a numeric data type that is not a valid number. NaN is returned as a result when an arithmetic operation or mathematical function is performed, and the result is undefined or unrepresentable as a valid number.

What changed

This hunk replaces the for loop that had a decrementing counter (i--) with a termination condition that required incrementing (i < strings.length), which would cause an infinite loop since i would never reach the stop condition. By switching to a for...of loop, the iteration over the strings array is handled correctly without any manual counter management.

--- a/sonar-scanner/src/javascript/Person.js
+++ b/sonar-scanner/src/javascript/Person.js
@@ -24,1 +24,1 @@ if (a !== NaN) { // Noncompliant; always true
-for (var i = 0; i < strings.length; i--) {
+for (var s of strings) {
javascript:S2251 - "i" is decremented and will never reach its stop condition. • MAJORView issue 1
javascript:S2259 - TypeError can be thrown as expression might be null or undefined here. • MAJORView issue 2

Location: sonar-scanner/src/javascript/Person.js:24

Why is this an issue?

In a for loop, the update clause is responsible for modifying the loop counter variable in the appropriate direction to control the loop’s iteration. It determines how the loop counter variable changes with each iteration of the loop. The loop counter should move in the right direction to prevent infinite loops or unexpected behavior.

In JavaScript, null and undefined are primitive values that do not have properties or methods. When accessing a property on a null or undefined value, JavaScript tries to access the property of an object that does not exist, which results in a TypeError.

What changed

This hunk changes the logical operator from && to || in the null check condition. The original code str == null && str.length == 0 would attempt to access .length on str even when str is null or undefined (since both conditions of && are evaluated when the first is true), causing a TypeError. By changing to ||, the condition short-circuits: if str == null is true, str.length == 0 is never evaluated, preventing the null dereference. This fixes both the TypeError that can be thrown when accessing .length on a null or undefined expression, and the incorrect loop counter direction issue that is resolved by the earlier hunk converting to a for-of loop — together these hunks ensure the loop body operates safely on each string.

--- a/sonar-scanner/src/javascript/Person.js
+++ b/sonar-scanner/src/javascript/Person.js
@@ -28,1 +28,1 @@ for (var i = 0; i < strings.length; i--) {
-if (str == null && str.length == 0) {
+if (str == null || str.length == 0) {

Have a suggestion or found an issue? Share your feedback here.


SonarQube Remediation Agent uses AI. Check for mistakes.

Fixed issues:
- AZ9rJA3YkdvHrJAQ8SGf for javascript:S2688 rule
- AZ9rJA3YkdvHrJAQ8SGh for javascript:S2688 rule
- AZ9rJA3YkdvHrJAQ8SGl for javascript:S2251 rule
- AZ9rJA3YkdvHrJAQ8SGn for javascript:S2259 rule

Generated by SonarQube Agent (task: 099d490d-a89e-4e7e-a0ee-f68a29c1d7fa)
@sonarqubecloud

sonarqubecloud Bot commented Aug 1, 2026

Copy link
Copy Markdown

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant