False Positive: IterableIterator.ql reports classes whose `hasNext()` still reliably disables iteration.

[Carlson-JLQ]

False Positive: IterableIterator.ql reports classes whose hasNext() still reliably disables iteration.

Version
codeql 2.24.3

Checker

• Checker id: Language Abuse/IterableIterator.ql
• Checker description: This checker detects classes that implement Iterable by returning themselves as the Iterator but lack a guard to prevent multiple concurrent iterations.

Description of the false positive

These classes do return this from iterator(), but hasNext() still deterministically returns false, which is exactly the built-in guard that keeps iteration from proceeding. The refactoring only changes how that false result is computed.

Affected test cases

NegCase6_Var2.java

hasNext() still disables reuse of the iterator instance in practice, so this should not be reported as an unsafe self-iterable.

// A concrete class that implements Iterable, returns "this" in iterator(), and has hasNext() returning false should not be flagged.
package scensct.var.neg;

import java.util.Iterator;

public class NegCase6_Var2 implements Iterable<Double>, Iterator<Double> { // [REPORTED LINE]
    
    public Iterator<Double> iterator() {
        return this;
    }
    
    private boolean neverHasNext() {
        return false;
    }
    
    public boolean hasNext() {
        return neverHasNext();
    }
    
    public Double next() {
        return 0.0;
    }
}

NegCase6_Var4.java

The iteration guard is still present even though the control flow is slightly different.

// A concrete class that implements Iterable, returns "this" in iterator(), and has hasNext() returning false should not be flagged.
package scensct.var.neg;

import java.util.Iterator;

public class NegCase6_Var4 implements Iterable<Double>, Iterator<Double> { // [REPORTED LINE]
    private final boolean NO_MORE = false;
    
    public Iterator<Double> iterator() {
        return this;
    }
    
    public boolean hasNext() {
        for (int i = 0; i < 1; i++) {
            // loop does nothing
        }
        return NO_MORE;
    }
    
    public Double next() {
        return 0.0;
    }
}

Cause analysis

The query appears too literal about what counts as a valid guard. Once hasNext() returns false via a helper or a constant field instead of a bare literal, the class is still reported.

That is overly rigid. The safety property here is semantic: iteration is disabled, regardless of whether false is returned directly or indirectly.

References

None known.

[redsun82]

👋 @Carlson-JLQ thanks for this. As I already mentioned, we don't currently prioritize fixing false positives. Moreover, this seems something that wouldn't really occur often in real code.