diff --git a/java/ql/lib/semmle/code/java/Modifier.qll b/java/ql/lib/semmle/code/java/Modifier.qll index af34e069e1e..83795ca32bd 100644 --- a/java/ql/lib/semmle/code/java/Modifier.qll +++ b/java/ql/lib/semmle/code/java/Modifier.qll @@ -7,7 +7,13 @@ import Element /** A modifier such as `private`, `static` or `abstract`. */ class Modifier extends Element, @modifier { /** Gets the element to which this modifier applies. */ - Element getElement() { hasModifier(result, this) } + Element getElement() { + hasModifier(result, this) and + // Kotlin "internal" elements may also get "public" modifiers, so we want to filter those out + not exists(Modifier mod2 | + hasModifier(result, mod2) and modifiers(this, "public") and modifiers(mod2, "internal") + ) + } override string getAPrimaryQlClass() { result = "Modifier" } } @@ -25,7 +31,15 @@ abstract class Modifiable extends Element { * abstract, so `isAbstract()` will hold for them even if `hasModifier("abstract")` * does not. */ - predicate hasModifier(string m) { modifiers(this.getAModifier(), m) } + predicate hasModifier(string m) { + exists(Modifier mod | mod = this.getAModifier() | + modifiers(mod, m) and + // Kotlin "internal" elements may also get "public" modifiers, so we want to filter those out + not exists(Modifier mod2 | + hasModifier(this, mod2) and modifiers(mod, "public") and modifiers(mod2, "internal") + ) + ) + } /** Holds if this element has no modifier. */ predicate hasNoModifier() { not hasModifier(this, _) } @@ -46,11 +60,8 @@ abstract class Modifiable extends Element { // TODO: `isSealed()` conflicts with `ClassOrInterface.isSealed()`. What name do we want to use here? predicate isSealedKotlin() { this.hasModifier("sealed") } - /** - * Holds if this element has a `public` modifier or is implicitly public. - * Kotlin `internal` members, which are `public` in JVM Bytecode, are not considered `public`. - */ - predicate isPublic() { this.hasModifier("public") and not this.isInternal() } + /** Holds if this element has a `public` modifier or is implicitly public. */ + predicate isPublic() { this.hasModifier("public") } /** Holds if this element has a `protected` modifier. */ predicate isProtected() { this.hasModifier("protected") } diff --git a/java/ql/test/kotlin/library-tests/java_and_kotlin_internal/visibility.expected b/java/ql/test/kotlin/library-tests/java_and_kotlin_internal/visibility.expected index 8fada9a19fe..3edb62a6505 100644 --- a/java/ql/test/kotlin/library-tests/java_and_kotlin_internal/visibility.expected +++ b/java/ql/test/kotlin/library-tests/java_and_kotlin_internal/visibility.expected @@ -1,7 +1,9 @@ isPublic isInternal | Kotlin.kt:2:11:3:2 | kotlinFun$main | +modifiers_methods +| file://:0:0:0:0 | final | Kotlin.kt:2:11:3:2 | kotlinFun$main | +| file://:0:0:0:0 | internal | Kotlin.kt:2:11:3:2 | kotlinFun$main | #select | Kotlin.kt:2:11:3:2 | kotlinFun$main | final | | Kotlin.kt:2:11:3:2 | kotlinFun$main | internal | -| Kotlin.kt:2:11:3:2 | kotlinFun$main | public | diff --git a/java/ql/test/kotlin/library-tests/java_and_kotlin_internal/visibility.ql b/java/ql/test/kotlin/library-tests/java_and_kotlin_internal/visibility.ql index fad938b06a9..452cee9b3c6 100644 --- a/java/ql/test/kotlin/library-tests/java_and_kotlin_internal/visibility.ql +++ b/java/ql/test/kotlin/library-tests/java_and_kotlin_internal/visibility.ql @@ -7,3 +7,7 @@ select m, s query predicate isPublic(Method m) { m.fromSource() and m.isPublic() } query predicate isInternal(Method m) { m.fromSource() and m.isInternal() } + +query predicate modifiers_methods(Modifier mo, Method me) { + mo.getElement() = me and me.fromSource() +}