False Negative: ContainerSizeCmpZero.ql misses impossible size checks once `length()` or `size()` is copied into locals.
- Langage dominant
- CodeQL
- Étoiles
- 10.1k
- Forks
- 2.1k
- Merge moyen
- 2 j 15 h
- PR mergées (30 j)
- 141
Description
Version
codeql 2.24.3
## Checker
- Checker id: `Likely Bugs/Likely Typos/ContainerSizeCmpZero.ql`
- Checker description: This checker detects comparisons of container size (array length, string length, collection size, or map size) to zero that are always true or always false because container sizes cannot be negative.
## Description of the false negative
All of these cases are still impossible or tautological comparisons on container sizes. The only change is that the result of `length()` or `size()` is first copied into a local variable, or zero is produced by a tiny helper before the comparison happens.
That does not change the fact that container sizes are non-negative.
## Affected test cases
### `PosCase1_Var1.java`
`len < 0` is still always false. Pulling `arr.length` into a local should not hide that.
```java
// Comparing array length to 0 with less-than should be flagged as always false.
package scensct.var.pos;
import java.util.*;
public class PosCase1_Var1 {
public static void main(String[] args) {
int[] arr = new int[5];
// Use a temporary variable for length
int len = arr.length;
if (len < 0) { // Always false
System.out.println("Unreachable");
}
}
}
```
### `PosCase2_Var1.java`
`count >= 0` is always true for a collection size. The extra local does not make it meaningful.
```java
// Comparing collection size to 0 with greater-than-or-equal should be flagged as always true.
package scensct.var.pos;
import java.util.*;
public class PosCase2_Var1 {
public static void main(String[] args) {
// Variant 1: Use a temporary variable and rename collection
Collection items = new HashSet<>();
int count = items.size();
if (count >= 0) { // Always true
System.out.println("Always true");
}
}
}
```
### `PosCase2_Var5.java`
The alias and ternary operator add noise, but `size >= 0` is still a tautology.
```java
// Comparing collection size to 0 with greater-than-or-equal should be flagged as always true.
package scensct.var.pos;
import java.util.*;
public class PosCase2_Var5 {
public static void main(String[] args) {
// Variant 5: Introduce aliasing and ternary operator
List original = new LinkedList<>();
List alias = original;
int size = alias.size();
String result = size >= 0 ? "Always true" : "Never printed";
System.out.println(result);
}
}
```
### `PosCase3_Var1.java`
`0 > length` is just another spelling of an always-false negative-size check.
```java
// Comparing integer literal 0 to string length with greater-than should be flagged as always false.
package scensct.var.pos;
import java.util.*;
public class PosCase3_Var1 {
public static void main(String[] args) {
String text = "test";
int length = text.length();
// Using a temporary variable for the comparison
if (0 > length) {
System.out.println("Unreachable");
}
}
}
```
### `PosCase4_Var1.java`
`zero <= dataMap.size()` is always true. The boolean temporary only hides the same impossible check.
```java
// Comparing integer literal 0 to map size with less-than-or-equal should be flagged as always true.
package scensct.var.pos;
import java.util.*;
public class PosCase4_Var1 {
public static void main(String[] args) {
// Variant 1: Lexical refactoring - rename map and use explicit type
HashMap dataMap = new HashMap<>();
int zero = 0;
boolean condition = zero <= dataMap.size();
if (condition) {
System.out.println("Always true");
}
}
}
```
### `PosCase4_Var5.java`
The helper `getZero()` does not change the comparison. `0 <= map.size()` remains always true.
```java
// Comparing integer literal 0 to map size with less-than-or-equal should be flagged as always true.
package scensct.var.pos;
import java.util.*;
public class PosCase4_Var5 {
// Variant 5: Complex expression with method call
private static int getZero() {
return 0;
}
public static void main(String[] args) {
Map map = new LinkedHashMap<>();
// Use method call for zero and nested comparison
if (getZero() <= map.size() && true) {
System.out.println("Always true");
}
}
}
```
## Cause analysis
The issue here is not ambiguity; it is loss of simple numeric reasoning once `size()` or `length()` is one step removed from the comparison. `Likely Bugs/Likely Typos/ContainerSizeCmpZero.ql` appears to require a very direct AST shape and stops recognizing the same always-true or always-false condition when a local or helper is introduced.
That is narrower than developers expect. These are still straightforward container-size sanity bugs.
## References
None known.
Guide de contribution
Ouvrir le guide de contribution
Piste de recherche
Commencez par Likely Bugs/Likely Typos/ContainerSizeCmpZero.ql et examinez comment il reconnaît les comparaisons directes de la taille des conteneurs. Reproduisez le problème avec les exemples nommés PosCase1_Var1.java à PosCase4_Var5.java. La tâche est terminée lorsque le checker signale les comparaisons listées qui sont toujours vraies et toujours fausses, même lorsque size ou length est copié, utilisé comme alias ou enveloppé comme indiqué.
Rédigé par le modèle d'indexation à partir du texte de l'issue.
Évaluation
- Stack technique
- java
- Domaine
- devtools, security
- Type d'issue
- Bug
- Difficulté
- 3/5
- Temps estimé
- 1-2 jours
- Activité
- Calme
- Clarté
- Plutôt claire
- Accessibilité débutants
- 48/100