github / github/codeql

Java: `RefType.getAnAncestor()` is error-prone when supertype is parameterized type with Object as type argument

Open
#5,595 2 comments 0 reactions 0 assignees View on GitHub
Java question
Dominant language
CodeQL
Stars
10.1k
Forks
2.1k
Avg merge
2d 15h
Merged PRs (30d)
141

Description

The predicate [`RefType.getAnAcestor()`](https://codeql.github.com/codeql-standard-libraries/java/semmle/code/java/Type.qll/predicate.Type$RefType$getAnAncestor.0.html) is error-prone when one of the supertypes is a generic type parameterized with `Object` as type argument. In that case `getASupertype()` and `getAnAcestor()` return all types which have a lower bound (but are not actually relevant), e.g.:
```ql
import java

from RefType t
where t.hasName("ObjectToStringComparator")
select t, t.getAnAncestor()
```
[Query Console link](https://lgtm.com/query/7884599397805363842/)

Result hierarchy (click to expand)

- `ObjectToStringComparator`
- `Serializable`
- `Object`
- `Comparator`
- `Comparator<>`
- `Comparator`
- `Comparator>`
- `Comparator`
- `Comparator>`
- `Comparator>`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator>`
- `Comparator>`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator>`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator>>`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Comparator`
- `Object`

This is most likely not the desired behavior. Therefore it might be good to deprecate `getAnAncestor()` and instead add a predicate `getASourceAncestor()` (which also deliberately does not have `this` as result):
```ql
/**
* Gets a source ancestor of this type, that is, a direct supertype or a direct supertype of the
* source declaration of a supertype, recursively.
*/
RefType getASourceAncestor() {
// Checking for source declaration is necessary, otherwise class `Generic` would have `Generic<>` (raw),
// `Generic` and `Generic` as supertypes
result = getASupertype() and result.getSourceDeclaration() != getSourceDeclaration()
or result = getASourceAncestor(getASupertype().getSourceDeclaration())
}
```

As seen here, it is necessary to check `result.getSourceDeclaration() != getSourceDeclaration()` since `RefType` is currently missing a predicate for getting a supertype declared in source (or the implicit `Object`); this is slightly related to #3818.
Ideally such a predicate would have the name `getASourceSupertype()`, however that name is already [taken](https://codeql.github.com/codeql-standard-libraries/java/semmle/code/java/Type.qll/predicate.Type$RefType$getASourceSupertype.0.html). A better fitting name for the existing predicate would probably be `getASupertypeSource()` / `getASupertypeSourceDeclaration()`, since that is what it actually does: "Gets the source declaration of a direct supertype of this type"

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.