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

Bắt đầu với Likely Bugs/Likely Typos/ContainerSizeCmpZero.ql và kiểm tra cách nó nhận diện các phép so sánh trực tiếp kích thước container. Tái hiện vấn đề bằng các ví dụ có tên từ PosCase1_Var1.java đến PosCase4_Var5.java. Hoàn thành khi checker đánh dấu các phép so sánh luôn đúng và luôn sai được liệt kê, ngay cả khi size hoặc length được sao chép, dùng làm bí danh hoặc bọc như minh họa.

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.

// 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.

// 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<Integer> 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.

// 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<Double> original = new LinkedList<>();
        List<Double> 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.

// 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.

// 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<Integer, String> 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.

// 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<Integer, String> 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.