github / github/codeql

False Negative: ContainerSizeCmpZero.ql misses impossible size checks once `length()` or `size()` is copied into locals.

未关闭
#21,538 1 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
主要语言
CodeQL
星标
10.1k
派生
2.1k
平均合并
2 天 15 小时
30 天内合并 PR
141

描述

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.

贡献指南

打开贡献指南

调研方向

Start with Likely Bugs/Likely Typos/ContainerSizeCmpZero.ql and inspect how it recognizes direct container-size comparisons. Reproduce the issue with the named PosCase1_Var1.java through PosCase4_Var5.java examples. Done means the checker flags the listed always-true and always-false comparisons even when size or length is copied, aliased, or wrapped as shown.

由索引模型根据 Issue 内容生成。

评估

技术栈
java
领域
devtools, security
Issue 类型
缺陷
难度
3/5
预计耗时
1-2 天
活跃度
冷清
描述清晰度
基本清楚
新手友好度
48/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。