Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java: Fix WildcardTypeAccess.hasNoBound() not considering ArrayTypeAccess #5407

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Java: Fix WildcardTypeAccess.hasNoBound() not considering ArrayTypeAccess #5407

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5def44b
Java: Fix WildcardTypeAccess.hasNoBound() not considering ArrayTypeAc…
Marcono1234 Mar 13, 2021
87364b3
Address review feedback
Marcono1234 Mar 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions java/ql/src/semmle/code/java/Expr.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1742,11 +1742,27 @@ class WildcardTypeAccess extends Expr, @wildcardtypeaccess {
/** Gets the lower bound of this wildcard type access, if any. */
Expr getLowerBound() { result.isNthChildOf(this, 1) }

/**
* Gets the bound of this wildcard type access, if any, i.e. either the upper
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use abbreviations in qldoc.

Suggested change
* Gets the bound of this wildcard type access, if any, i.e. either the upper
* Gets the bound of this wildcard type access, if any, that is, either the upper

Copy link
Contributor Author

@Marcono1234 Marcono1234 Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made this a separate sentence ("... any. That is, ...") to be consistent with other comments and to not have so many commas.
(Though this might not be grammatically correct?)

* or the lower bound.
*/
Expr getBound() {
result = getUpperBound() or
result = getLowerBound()
}

/** Holds if this wildcard is not bounded by any type bounds. */
predicate hasNoBound() { not exists(TypeAccess t | t.getParent() = this) }
predicate hasNoBound() { not exists(getBound()) }

/** Gets a printable representation of this expression. */
override string toString() { result = "? ..." }
override string toString() {
if exists(getUpperBound())
then result = "? extends ..."
else
if exists(getLowerBound())
then result = "? super ..."
else result = "?"
}

override string getAPrimaryQlClass() { result = "WildcardTypeAccess" }
}
Expand Down
2 changes: 1 addition & 1 deletion java/ql/test/library-tests/dependency/PrintAst.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ dependency/A.java:
#-----| 4: (Parameters)
# 28| 0: [Parameter] t
# 28| 0: [TypeAccess] Collection<? extends Number>
# 28| 0: [WildcardTypeAccess] ? ...
# 28| 0: [WildcardTypeAccess] ? extends ...
# 28| 0: [TypeAccess] Number
# 28| 5: [BlockStmt] stmt
8 changes: 4 additions & 4 deletions java/ql/test/library-tests/generics/PrintAst.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,18 @@ generics/A.java:
# 16| 0: [TypeAccess] Number
# 17| 3: [FieldDeclaration] D<?> d1, ...;
# 17| -1: [TypeAccess] D<?>
# 17| 0: [WildcardTypeAccess] ? ...
# 17| 0: [WildcardTypeAccess] ?
# 18| 4: [FieldDeclaration] D<? extends Object> d2, ...;
# 18| -1: [TypeAccess] D<? extends Object>
# 18| 0: [WildcardTypeAccess] ? ...
# 18| 0: [WildcardTypeAccess] ? extends ...
# 18| 0: [TypeAccess] Object
# 19| 5: [FieldDeclaration] D<? extends Float> d3, ...;
# 19| -1: [TypeAccess] D<? extends Float>
# 19| 0: [WildcardTypeAccess] ? ...
# 19| 0: [WildcardTypeAccess] ? extends ...
# 19| 0: [TypeAccess] Float
# 20| 6: [FieldDeclaration] D<? super Double> d4, ...;
# 20| -1: [TypeAccess] D<? super Double>
# 20| 0: [WildcardTypeAccess] ? ...
# 20| 0: [WildcardTypeAccess] ? super ...
# 20| 1: [TypeAccess] Double
# 21| 7: [BlockStmt] stmt
# 21| 0: [ExprStmt] stmt
Expand Down
4 changes: 4 additions & 0 deletions java/ql/test/library-tests/locations/TypeLocations.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
| file://:0:0:0:0 | <nulltype> |
| file://:0:0:0:0 | ? |
| file://:0:0:0:0 | ? extends Number |
| file://:0:0:0:0 | ? extends Number[] |
| file://:0:0:0:0 | ? super Double |
| file://:0:0:0:0 | ? super Double[] |
| file://:0:0:0:0 | Double[] |
| file://:0:0:0:0 | G[] |
| file://:0:0:0:0 | Number[] |
| file://:0:0:0:0 | Object[] |
| file://:0:0:0:0 | Object[][] |
| file://:0:0:0:0 | boolean |
Expand Down
3 changes: 0 additions & 3 deletions java/ql/test/library-tests/locations/WildcardLocations.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +0,0 @@
| locations/C.java:6:7:6:7 | ? ... |
| locations/C.java:7:7:7:22 | ? ... |
| locations/C.java:8:7:8:20 | ? ... |
30 changes: 28 additions & 2 deletions java/ql/test/library-tests/locations/WildcardLocations.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,30 @@
import default
import TestUtilities.InlineExpectationsTest

from WildcardTypeAccess wta
select wta
private string getValue(WildcardTypeAccess wta) {
if wta.hasNoBound() // also makes sure that hasNoBound() is working correctly
then result = ""
else
// Also makes sure that getBound() is working correctly (and has at most
// one result)
exists(Expr bound | bound = wta.getBound() and not wta.getBound() != bound |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
exists(Expr bound | bound = wta.getBound() and not wta.getBound() != bound |
exists(Expr bound | bound = wta.getBound() |

It's better to test the not-having-multiple-values by potentially observing those values rather than observing nothing.

Copy link
Contributor Author

@Marcono1234 Marcono1234 Mar 22, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though then the test would also succeed when getBound() would have additionally completely unrelated results since here in the test its result is bound below with getUpperBound() / getLowerBound().

bound = wta.getUpperBound() and result = "u:" + bound
or
bound = wta.getLowerBound() and result = "l:" + bound
)
}

class WildcardTypeAccessTest extends InlineExpectationsTest {
WildcardTypeAccessTest() { this = "WildcardTypeAccessTest" }

override string getARelevantTag() { result = ["wildcardTypeAccess"] }
Marcono1234 marked this conversation as resolved.
Show resolved Hide resolved

override predicate hasActualResult(Location location, string element, string tag, string value) {
exists(WildcardTypeAccess wta |
location = wta.getLocation() and
element = wta.toString() and
tag = "wildcardTypeAccess" and
value = getValue(wta)
)
}
}
8 changes: 5 additions & 3 deletions java/ql/test/library-tests/locations/locations/C.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
import java.util.List;

public class C {
List<?> stuff;
List<? extends Number> numbers;
List<? super Double> more_numbers;
List<?> stuff; // $wildcardTypeAccess=
List<? extends Number> numbers; // $wildcardTypeAccess=u:Number
List<? super Double> more_numbers; // $wildcardTypeAccess=l:Double
List<? extends Number[]> numbersArr; // $wildcardTypeAccess=u:...[]
List<? super Double[]> more_numbersArr; // $wildcardTypeAccess=l:...[]
}
4 changes: 2 additions & 2 deletions java/ql/test/library-tests/reflection/PrintAst.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ reflection/ReflectiveAccess.java:
#-----| 4: (Parameters)
# 13| 0: [Parameter] classContainingAnnotation
# 13| 0: [TypeAccess] Class<?>
# 13| 0: [WildcardTypeAccess] ? ...
# 13| 0: [WildcardTypeAccess] ?
# 13| 1: [Parameter] annotationClass
# 13| 0: [TypeAccess] Class<A>
# 13| 0: [TypeAccess] A
Expand All @@ -33,7 +33,7 @@ reflection/ReflectiveAccess.java:
# 17| 5: [BlockStmt] stmt
# 18| 0: [LocalVariableDeclStmt] stmt
# 18| 0: [TypeAccess] Class<?>
# 18| 0: [WildcardTypeAccess] ? ...
# 18| 0: [WildcardTypeAccess] ?
# 18| 1: [LocalVariableDeclExpr] testClass
# 18| 0: [MethodAccess] forName(...)
# 18| -1: [TypeAccess] Class<>
Expand Down
4 changes: 4 additions & 0 deletions java/ql/test/library-tests/typeaccesses/ArrayTypeAccesses.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
| typeaccesses/Arrays.java:4:2:4:9 | ...[] |
| typeaccesses/Arrays.java:5:2:5:25 | ...[] |
| typeaccesses/TA.java:19:43:19:50 | ...[] |
| typeaccesses/TA.java:20:41:20:48 | ...[] |
| typeaccesses/TA.java:31:80:31:82 | ...[] |
| typeaccesses/TA.java:31:124:31:137 | ...[] |
88 changes: 87 additions & 1 deletion java/ql/test/library-tests/typeaccesses/PrintAst.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ typeaccesses/TA.java:
# 15| 11: [Method] method8
# 15| 3: [TypeAccess] TA.Inner<?>
# 15| -1: [TypeAccess] TA
# 15| 0: [WildcardTypeAccess] ? ...
# 15| 0: [WildcardTypeAccess] ?
# 15| 5: [BlockStmt] stmt
# 15| 0: [ReturnStmt] stmt
# 15| 0: [NullLiteral] null
Expand All @@ -102,3 +102,89 @@ typeaccesses/TA.java:
# 16| 5: [BlockStmt] stmt
# 16| 0: [ReturnStmt] stmt
# 16| 0: [NullLiteral] null
# 18| 13: [Method] method10
# 18| 3: [TypeAccess] void
#-----| 4: (Parameters)
# 18| 0: [Parameter] l
# 18| 0: [TypeAccess] ArrayList<? extends Number>
# 18| 0: [WildcardTypeAccess] ? extends ...
# 18| 0: [TypeAccess] Number
# 18| 5: [BlockStmt] stmt
# 19| 14: [Method] method11
# 19| 3: [TypeAccess] void
#-----| 4: (Parameters)
# 19| 0: [Parameter] l
# 19| 0: [TypeAccess] ArrayList<? extends Number[]>
# 19| 0: [WildcardTypeAccess] ? extends ...
# 19| 0: [ArrayTypeAccess] ...[]
# 19| 0: [TypeAccess] Number
# 19| 5: [BlockStmt] stmt
# 20| 15: [Method] method12
# 20| 3: [TypeAccess] void
#-----| 4: (Parameters)
# 20| 0: [Parameter] l
# 20| 0: [TypeAccess] ArrayList<? super Number[]>
# 20| 0: [WildcardTypeAccess] ? super ...
# 20| 1: [ArrayTypeAccess] ...[]
# 20| 0: [TypeAccess] Number
# 20| 5: [BlockStmt] stmt
# 22| 16: [Interface] I1
# 23| 17: [Interface] I2
# 24| 18: [Class] Inner2
#-----| -2: (Generic Parameters)
# 24| 0: [TypeVariable] T
# 24| 0: [TypeAccess] I1
# 24| 1: [TypeAccess] I2
# 25| 19: [Class] Inner3
#-----| -2: (Generic Parameters)
# 25| 0: [TypeVariable] T
# 25| 0: [TypeAccess] I2
# 25| 1: [TypeAccess] I1
# 26| 20: [Class] Inner4
#-----| -2: (Generic Parameters)
# 26| 0: [TypeVariable] T
# 26| 0: [TypeAccess] Object
# 26| 1: [TypeAccess] I2
# 26| 2: [TypeAccess] I1
# 27| 21: [Method] method13
#-----| 2: (Generic Parameters)
# 27| 0: [TypeVariable] T
# 27| 0: [TypeAccess] I1
# 27| 1: [TypeAccess] I2
# 27| 3: [TypeAccess] void
# 27| 5: [BlockStmt] stmt
# 28| 22: [Method] method14
#-----| 2: (Generic Parameters)
# 28| 0: [TypeVariable] T
# 28| 0: [TypeAccess] I2
# 28| 1: [TypeAccess] I1
# 28| 3: [TypeAccess] void
# 28| 5: [BlockStmt] stmt
# 29| 23: [Method] method15
#-----| 2: (Generic Parameters)
# 29| 0: [TypeVariable] T
# 29| 0: [TypeAccess] Object
# 29| 1: [TypeAccess] I2
# 29| 2: [TypeAccess] I1
# 29| 3: [TypeAccess] void
# 29| 5: [BlockStmt] stmt
# 31| 24: [Method] method16
#-----| 2: (Generic Parameters)
# 31| 0: [TypeVariable] T
# 31| 0: [TypeAccess] Number
# 31| 1: [TypeAccess] Runnable
# 31| 1: [TypeVariable] R
# 31| 0: [TypeAccess] T
# 31| 2: [TypeVariable] S
# 31| 0: [TypeAccess] ArrayList<? super R[]>
# 31| 0: [WildcardTypeAccess] ? super ...
# 31| 1: [ArrayTypeAccess] ...[]
# 31| 0: [TypeAccess] R
# 31| 3: [TypeAccess] ArrayList<? super ArrayList<? extends ArrayList<T>[]>>
# 31| 0: [WildcardTypeAccess] ? super ...
# 31| 1: [TypeAccess] ArrayList<? extends ArrayList<T>[]>
# 31| 0: [WildcardTypeAccess] ? extends ...
# 31| 0: [ArrayTypeAccess] ...[]
# 31| 0: [TypeAccess] ArrayList<T>
# 31| 0: [TypeAccess] T
# 31| 5: [BlockStmt] stmt
37 changes: 36 additions & 1 deletion java/ql/test/library-tests/typeaccesses/TypeAccesses.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,39 @@
| typeaccesses/TA.java:15:9:15:10 | TA |
| typeaccesses/TA.java:15:9:15:19 | TA.Inner<?> |
| typeaccesses/TA.java:16:9:16:26 | List<TA> |
| typeaccesses/TA.java:16:24:16:25 | TA |
| typeaccesses/TA.java:16:24:16:25 | TA |
| typeaccesses/TA.java:18:9:18:12 | void |
| typeaccesses/TA.java:18:23:18:49 | ArrayList<? extends Number> |
| typeaccesses/TA.java:18:43:18:48 | Number |
| typeaccesses/TA.java:19:9:19:12 | void |
| typeaccesses/TA.java:19:23:19:51 | ArrayList<? extends Number[]> |
| typeaccesses/TA.java:19:43:19:48 | Number |
| typeaccesses/TA.java:20:9:20:12 | void |
| typeaccesses/TA.java:20:23:20:49 | ArrayList<? super Number[]> |
| typeaccesses/TA.java:20:41:20:46 | Number |
| typeaccesses/TA.java:24:25:24:26 | I1 |
| typeaccesses/TA.java:24:30:24:31 | I2 |
| typeaccesses/TA.java:25:25:25:26 | I2 |
| typeaccesses/TA.java:25:30:25:31 | I1 |
| typeaccesses/TA.java:26:25:26:30 | Object |
| typeaccesses/TA.java:26:34:26:35 | I2 |
| typeaccesses/TA.java:26:39:26:40 | I1 |
| typeaccesses/TA.java:27:20:27:21 | I1 |
| typeaccesses/TA.java:27:25:27:26 | I2 |
| typeaccesses/TA.java:27:29:27:32 | void |
| typeaccesses/TA.java:28:20:28:21 | I2 |
| typeaccesses/TA.java:28:25:28:26 | I1 |
| typeaccesses/TA.java:28:29:28:32 | void |
| typeaccesses/TA.java:29:20:29:25 | Object |
| typeaccesses/TA.java:29:29:29:30 | I2 |
| typeaccesses/TA.java:29:34:29:35 | I1 |
| typeaccesses/TA.java:29:38:29:41 | void |
| typeaccesses/TA.java:31:20:31:25 | Number |
| typeaccesses/TA.java:31:29:31:36 | Runnable |
| typeaccesses/TA.java:31:49:31:49 | T |
| typeaccesses/TA.java:31:62:31:83 | ArrayList<? super R[]> |
| typeaccesses/TA.java:31:80:31:80 | R |
| typeaccesses/TA.java:31:86:31:139 | ArrayList<? super ArrayList<? extends ArrayList<T>[]>> |
| typeaccesses/TA.java:31:104:31:138 | ArrayList<? extends ArrayList<T>[]> |
| typeaccesses/TA.java:31:124:31:135 | ArrayList<T> |
| typeaccesses/TA.java:31:134:31:134 | T |
15 changes: 15 additions & 0 deletions java/ql/test/library-tests/typeaccesses/typeaccesses/TA.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,19 @@ public <T extends ArrayList<TA>> void method4() { }
class Inner<T extends ArrayList<TA>> { }
public TA.Inner<?> method8() { return null; }
public java.util.List<TA> method9() { return null; }

public void method10(ArrayList<? extends Number> l) { }
public void method11(ArrayList<? extends Number[]> l) { }
public void method12(ArrayList<? super Number[]> l) { }

public interface I1 { }
public interface I2 { }
class Inner2<T extends I1 & I2> { }
class Inner3<T extends I2 & I1> { }
class Inner4<T extends Object & I2 & I1> { }
public <T extends I1 & I2> void method13() { }
public <T extends I2 & I1> void method14() { }
public <T extends Object & I2 & I1> void method15() { }

public <T extends Number & Runnable, R extends T, S extends ArrayList<? super R[]>> ArrayList<? super ArrayList<? extends ArrayList<T>[]>> method16() { }
}