Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 17 additions & 9 deletions java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1507,14 +1507,15 @@ open class KotlinFileExtractor(
}
is IrFunction -> {
if (s.isLocalFunction()) {
val classId = extractGeneratedClass(s, listOf(pluginContext.irBuiltIns.anyType))
val compilerGeneratedKindOverride = if (s.origin == IrDeclarationOrigin.ADAPTER_FOR_CALLABLE_REFERENCE) {
CompilerGeneratedKinds.DECLARING_CLASSES_OF_ADAPTER_FUNCTIONS
} else {
null
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels unintuitive for me that we pass null, but in the end it becomes CompilerGeneratedKinds.CALLABLE_CLASS. Should we instead explicitly specify the latter here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered that, but then we'd have to repeat CALLABLE_CLASS in 3 places. I think of the null here as "no special value; just use the default". Perhaps I should have called it compilerGeneratedKindOverride instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Another option would be specifying a default parameter value. But compilerGeneratedKindOverride also sounds reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem with a default parameter value is I think I'd need to specify it for both overloads of the function, as well as passing it as a parameter in the call that makes use of the "last argument is a block" syntax. So I've gone with the ...Override name to avoid repeating it.

}
val classId = extractGeneratedClass(s, listOf(pluginContext.irBuiltIns.anyType), compilerGeneratedKindOverride = compilerGeneratedKindOverride)
extractLocalTypeDeclStmt(classId, s, callable, parent, idx)
val ids = getLocallyVisibleFunctionLabels(s)
tw.writeKtLocalFunction(ids.function)

if (s.origin == IrDeclarationOrigin.ADAPTER_FOR_CALLABLE_REFERENCE) {
tw.writeCompiler_generated(classId, CompilerGeneratedKinds.DECLARING_CLASSES_OF_ADAPTER_FUNCTIONS.kind)
}
} else {
logger.errorElement("Expected to find local function", s)
}
Expand Down Expand Up @@ -4685,7 +4686,7 @@ open class KotlinFileExtractor(
val baseClass = pluginContext.referenceClass(FqName("kotlin.jvm.internal.FunctionReference"))?.owner?.typeWith()
?: pluginContext.irBuiltIns.anyType

val classId = extractGeneratedClass(ids, listOf(baseClass, fnInterfaceType), locId, functionReferenceExpr, declarationParent, { it.valueParameters.size == 1 }) {
val classId = extractGeneratedClass(ids, listOf(baseClass, fnInterfaceType), locId, functionReferenceExpr, declarationParent, null, { it.valueParameters.size == 1 }) {
// The argument to FunctionReference's constructor is the function arity.
extractConstantInteger(type.arguments.size - 1, locId, it, 0, ids.constructor, it)
}
Expand Down Expand Up @@ -5389,13 +5390,15 @@ open class KotlinFileExtractor(
locId: Label<DbLocation>,
elementToReportOn: IrElement,
declarationParent: IrDeclarationParent,
compilerGeneratedKindOverride: CompilerGeneratedKinds? = null,
superConstructorSelector: (IrFunction) -> Boolean = { it.valueParameters.isEmpty() },
extractSuperconstructorArgs: (Label<DbSuperconstructorinvocationstmt>) -> Unit = {}
extractSuperconstructorArgs: (Label<DbSuperconstructorinvocationstmt>) -> Unit = {},
): Label<out DbClass> {
// Write class
val id = ids.type.javaResult.id.cast<DbClass>()
val pkgId = extractPackage("")
tw.writeClasses(id, "", pkgId, id)
tw.writeCompiler_generated(id, (compilerGeneratedKindOverride ?: CompilerGeneratedKinds.CALLABLE_CLASS).kind)
tw.writeHasLocation(id, locId)

// Extract constructor
Expand Down Expand Up @@ -5442,11 +5445,15 @@ open class KotlinFileExtractor(
/**
* Extracts the class around a local function or a lambda. The superclass must have a no-arg constructor.
*/
private fun extractGeneratedClass(localFunction: IrFunction, superTypes: List<IrType>) : Label<out DbClass> {
private fun extractGeneratedClass(
localFunction: IrFunction,
superTypes: List<IrType>,
compilerGeneratedKindOverride: CompilerGeneratedKinds? = null
) : Label<out DbClass> {
with("generated class", localFunction) {
val ids = getLocallyVisibleFunctionLabels(localFunction)

val id = extractGeneratedClass(ids, superTypes, tw.getLocation(localFunction), localFunction, localFunction.parent)
val id = extractGeneratedClass(ids, superTypes, tw.getLocation(localFunction), localFunction, localFunction.parent, compilerGeneratedKindOverride = compilerGeneratedKindOverride)

// Extract local function as a member
extractFunction(localFunction, id, extractBody = true, extractMethodAndParameterTypeAccesses = true, null, listOf())
Expand Down Expand Up @@ -5520,5 +5527,6 @@ open class KotlinFileExtractor(
DEFAULT_ARGUMENTS_METHOD(10),
INTERFACE_FORWARDER(11),
ENUM_CONSTRUCTOR_ARGUMENT(12),
CALLABLE_CLASS(13),
}
}
2 changes: 2 additions & 0 deletions java/ql/lib/semmle/code/java/Element.qll
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ class Element extends @element, Top {
i = 11 and result = "Forwarder for a Kotlin class inheriting an interface default method"
or
i = 12 and result = "Argument for enum constructor call"
or
i = 13 and result = "The class around a local function, a lambda, or a function reference"
)
}
}
Expand Down
3 changes: 2 additions & 1 deletion java/ql/src/Advisory/Naming/NamingConventionsRefTypes.ql
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import java

from RefType t
where
t.getFile().isJavaSourceFile() and
t.fromSource() and
not t instanceof AnonymousClass and
not t.isCompilerGenerated() and
not t.getName().substring(0, 1).toUpperCase() = t.getName().substring(0, 1)
select t, "Class and interface names should start in uppercase."
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The query `java/misnamed-type` is now enabled for Kotlin.
Original file line number Diff line number Diff line change
Expand Up @@ -282,23 +282,58 @@ compGenerated
| file://<external>/LongRange.class:0:0:0:0 | forEach | Forwarder for a Kotlin class inheriting an interface default method |
| file://<external>/LongRange.class:0:0:0:0 | spliterator | Forwarder for a Kotlin class inheriting an interface default method |
| file://<external>/String.class:0:0:0:0 | isEmpty | Forwarder for a Kotlin class inheriting an interface default method |
| reflection.kt:7:49:7:54 | new Function2<Ccc,Integer,Double>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:10:38:10:42 | new KProperty1<C,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:14:38:14:44 | new Function1<C,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:15:35:15:41 | new KProperty0<Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:17:45:17:49 | new KMutableProperty1<C,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:21:44:21:50 | new Function2<C,Integer,Unit>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:22:42:22:48 | new KMutableProperty0<Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:24:46:24:64 | new Function1<KCallable<?>,Boolean>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:33:9:33:23 | getP0 | Default property accessor |
| reflection.kt:34:9:34:23 | getP1 | Default property accessor |
| reflection.kt:34:9:34:23 | setP1 | Default property accessor |
| reflection.kt:50:13:50:28 | new KProperty1<String,Character>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:51:13:51:28 | new KProperty0<Character>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:60:17:60:32 | new Function2<Generic<Integer>,Integer,String>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:61:17:61:34 | new Function1<Integer,String>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:62:17:62:34 | new Function1<Generic<Integer>,String>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:63:17:63:36 | new Function0<String>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:64:17:64:34 | new Function1<Generic<Integer>,String>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:65:17:65:36 | new Function0<String>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:67:17:67:32 | new KMutableProperty1<Generic<Integer>,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:68:17:68:34 | new KMutableProperty0<Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:70:17:70:30 | new KProperty0<Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:71:17:71:34 | new KProperty0<Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:72:17:72:35 | new KMutableProperty0<Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:83:17:83:28 | getValue | Default property accessor |
| reflection.kt:90:18:90:24 | new Function1<String,Inner<String>>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:97:14:97:21 | new Function1<String,Class2<String>>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:98:14:98:17 | new Function1<String,Unit>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:99:14:99:29 | new Function1<String,Inner<String>>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:105:18:105:31 | getProp1 | Default property accessor |
| reflection.kt:105:18:105:31 | setProp1 | Default property accessor |
| reflection.kt:109:17:109:27 | new KMutableProperty0<Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:115:9:115:27 | | The class around a local function, a lambda, or a function reference |
| reflection.kt:116:40:116:44 | new Function1<Integer,Unit>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:126:9:126:13 | | Declaring classes of adapter functions in Kotlin |
| reflection.kt:126:9:126:13 | new Function0<Unit>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:131:1:131:50 | takesOptionalParam$default | Forwarder for Kotlin calls that need default arguments filling in |
| reflection.kt:134:21:134:40 | | Declaring classes of adapter functions in Kotlin |
| reflection.kt:134:21:134:40 | new Function1<Integer,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:140:5:140:54 | takesOptionalParam$default | Forwarder for Kotlin calls that need default arguments filling in |
| reflection.kt:144:21:144:41 | | Declaring classes of adapter functions in Kotlin |
| reflection.kt:144:21:144:41 | new Function1<Integer,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:145:32:145:70 | | Declaring classes of adapter functions in Kotlin |
| reflection.kt:145:32:145:70 | new Function2<MemberOptionalsTest,Integer,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:150:1:150:60 | extTakesOptionalParam$default | Forwarder for Kotlin calls that need default arguments filling in |
| reflection.kt:153:21:153:44 | | Declaring classes of adapter functions in Kotlin |
| reflection.kt:153:21:153:44 | new Function1<Integer,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:154:33:154:61 | | Declaring classes of adapter functions in Kotlin |
| reflection.kt:154:33:154:61 | new Function2<String,Integer,Integer>(...) { ... } | The class around a local function, a lambda, or a function reference |
| reflection.kt:157:1:157:49 | ConstructorOptional | Forwarder for Kotlin calls that need default arguments filling in |
| reflection.kt:162:25:162:45 | | Declaring classes of adapter functions in Kotlin |
| reflection.kt:162:25:162:45 | new Function1<Integer,ConstructorOptional>(...) { ... } | The class around a local function, a lambda, or a function reference |
propertyReferenceOverrides
| reflection.kt:10:38:10:42 | ...::... | reflection.kt:10:38:10:42 | get | kotlin.reflect.KProperty1<C,Integer>.get(Reflection.C) |
| reflection.kt:10:38:10:42 | ...::... | reflection.kt:10:38:10:42 | invoke | kotlin.jvm.functions.Function1<C,Integer>.invoke(Reflection.C) |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
| Test.kt:12:1:12:13 | aaaa | Class and interface names should start in uppercase. |