From 690d6517d752e4db728fa77a62e36b7e498d908a Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Tue, 25 Oct 2022 10:16:37 +0200 Subject: [PATCH 1/2] Kotlin: Add abstract to concrete type cast guarded by `when` --- .../AbstractToConcreteCollection.expected | 1 + .../AbstractToConcreteCollection.qlref | 1 + .../kotlin/query-tests/AbstractToConcreteCollection/Test.kt | 5 +++++ 3 files changed, 7 insertions(+) create mode 100644 java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.expected create mode 100644 java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.qlref create mode 100644 java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/Test.kt diff --git a/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.expected b/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.expected new file mode 100644 index 00000000000..6160059acfd --- /dev/null +++ b/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.expected @@ -0,0 +1 @@ +| Test.kt:3:9:3:9 | | $@ is cast to the concrete type $@, losing abstraction. | file:///modules/java.base/java/util/List.class:0:0:0:0 | List | List | file:///modules/java.base/java/util/ArrayList.class:0:0:0:0 | ArrayList | ArrayList | diff --git a/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.qlref b/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.qlref new file mode 100644 index 00000000000..ddc5d95d9d1 --- /dev/null +++ b/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.qlref @@ -0,0 +1 @@ +Violations of Best Practice/Implementation Hiding/AbstractToConcreteCollection.ql \ No newline at end of file diff --git a/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/Test.kt b/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/Test.kt new file mode 100644 index 00000000000..c05317462c4 --- /dev/null +++ b/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/Test.kt @@ -0,0 +1,5 @@ +fun fn(m: MutableList) { + if (m is ArrayList) { + m.ensureCapacity(5) + } +} From a0490f454bbd3f3add28e9f449b6c5ec019a685f Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Tue, 25 Oct 2022 10:17:43 +0200 Subject: [PATCH 2/2] Kotlin: Improve `java/abstract-to-concrete-cast` to handle `when` branches --- .../AbstractToConcreteCollection.ql | 13 ++++++++++--- .../AbstractToConcreteCollection.expected | 1 - 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/java/ql/src/Violations of Best Practice/Implementation Hiding/AbstractToConcreteCollection.ql b/java/ql/src/Violations of Best Practice/Implementation Hiding/AbstractToConcreteCollection.ql index 9faf348cf97..670156cfccb 100644 --- a/java/ql/src/Violations of Best Practice/Implementation Hiding/AbstractToConcreteCollection.ql +++ b/java/ql/src/Violations of Best Practice/Implementation Hiding/AbstractToConcreteCollection.ql @@ -16,8 +16,11 @@ import java import semmle.code.java.Collections predicate guardedByInstanceOf(VarAccess e, RefType t) { - exists(IfStmt s, InstanceOfExpr instanceCheck, RefType checkType | - s.getCondition() = instanceCheck and + exists(Stmt s, InstanceOfExpr instanceCheck, RefType checkType | + ( + s.(IfStmt).getCondition() = instanceCheck or + s.(WhenBranch).getCondition() = instanceCheck + ) and instanceCheck.getCheckedType() = checkType and // The same variable appears as the subject of the `instanceof`. instanceCheck.getExpr() = e.getVariable().getAnAccess() and @@ -27,7 +30,11 @@ predicate guardedByInstanceOf(VarAccess e, RefType t) { (checkType = t or checkType = t.getSourceDeclaration().(GenericType).getRawType()) and // The expression appears in one of the branches. // (We do not verify here whether the guard is correctly implemented.) - exists(Stmt branch | branch = s.getThen() or branch = s.getElse() | + exists(Stmt branch | + branch = s.(IfStmt).getThen() or + branch = s.(IfStmt).getElse() or + branch = s.(WhenBranch).getRhs() + | branch = e.getEnclosingStmt().getEnclosingStmt+() ) ) diff --git a/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.expected b/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.expected index 6160059acfd..e69de29bb2d 100644 --- a/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.expected +++ b/java/ql/test/kotlin/query-tests/AbstractToConcreteCollection/AbstractToConcreteCollection.expected @@ -1 +0,0 @@ -| Test.kt:3:9:3:9 | | $@ is cast to the concrete type $@, losing abstraction. | file:///modules/java.base/java/util/List.class:0:0:0:0 | List | List | file:///modules/java.base/java/util/ArrayList.class:0:0:0:0 | ArrayList | ArrayList |