Skip to content

Commit

Permalink
Fix Kotlin complexity bug, other updates.
Browse files Browse the repository at this point in the history
  • Loading branch information
gchallen committed Oct 13, 2021
1 parent 4ce187b commit 349dca3
Show file tree
Hide file tree
Showing 17 changed files with 95 additions and 46 deletions.
1 change: 1 addition & 0 deletions .idea/inspectionProfiles/ktlint.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ allprojects {
}
subprojects {
group = "com.github.cs125-illinois.jeed"
version = "2021.10.0"
version = "2021.10.1"
tasks.withType<Test> {
useJUnitPlatform()
enableAssertions = true
Expand Down
4 changes: 2 additions & 2 deletions containerrunner/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import java.io.File
plugins {
kotlin("jvm")
application
id("com.github.johnrengelman.shadow") version "7.0.0"
id("com.github.johnrengelman.shadow") version "7.1.0"
id("com.palantir.docker") version "0.30.0"
id("org.jmailen.kotlinter")
id("io.gitlab.arturbosch.detekt")
Expand All @@ -14,7 +14,7 @@ dependencies {
implementation("ch.qos.logback:logback-classic:1.2.6")
implementation("io.github.microutils:kotlin-logging:2.0.11")
implementation("com.github.ajalt:clikt:2.8.0")
implementation("io.github.classgraph:classgraph:4.8.120")
implementation("io.github.classgraph:classgraph:4.8.126")
}
application {
@Suppress("DEPRECATION")
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
version=2021.10.0
version=2021.10.1
4 changes: 2 additions & 2 deletions core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ dependencies {
implementation("com.puppycrawl.tools:checkstyle:9.0.1")
implementation("com.pinterest.ktlint:ktlint-core:0.42.1")
implementation("com.pinterest.ktlint:ktlint-ruleset-standard:0.42.1")
implementation("com.github.jknack:handlebars:4.2.1")
implementation("com.github.jknack:handlebars:4.3.0")
implementation("com.squareup.moshi:moshi:1.12.0")
implementation("org.ow2.asm:asm:9.2")
implementation("org.ow2.asm:asm-util:9.2")
implementation("org.slf4j:slf4j-api:1.7.32")
implementation("ch.qos.logback:logback-classic:1.2.6")
implementation("io.github.microutils:kotlin-logging:2.0.11")
implementation("io.github.classgraph:classgraph:4.8.120")
implementation("io.github.classgraph:classgraph:4.8.126")
implementation("net.java.dev.jna:jna:5.9.0")
implementation("io.github.java-diff-utils:java-diff-utils:4.11")
implementation("com.google.googlejavaformat:google-java-format:1.11.0")
Expand Down
10 changes: 6 additions & 4 deletions core/src/main/kotlin/Cache.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import kotlinx.coroutines.runBlocking
import java.time.Instant

const val JEED_DEFAULT_COMPILATION_CACHE_SIZE_MB = 256L

@Suppress("TooGenericExceptionCaught")
val compilationCacheSizeMB = try {
System.getenv("JEED_COMPILATION_CACHE_SIZE")?.toLong() ?: JEED_DEFAULT_COMPILATION_CACHE_SIZE_MB
Expand All @@ -20,6 +21,7 @@ val compilationCacheSizeMB = try {
}

const val JEED_DEFAULT_USE_CACHE = false

@Suppress("TooGenericExceptionCaught")
val useCompilationCache = try {
System.getenv("JEED_USE_CACHE")?.toBoolean() ?: JEED_DEFAULT_USE_CACHE
Expand Down Expand Up @@ -69,9 +71,9 @@ fun Source.tryCache(
}
MoreCacheStats.hits++

val cachedCompilationArguments = cachedResult.compilationArguments ?: require {
val cachedCompilationArguments = cachedResult.compilationArguments ?: error(
"Cached compilation result missing arguments"
}
)
if (cachedResult.compilerName != compilerName) {
return null
}
Expand Down Expand Up @@ -131,9 +133,9 @@ fun Source.tryCache(
return null
}
MoreCacheStats.hits++
val cachedKompilationArguments = cachedResult.kompilationArguments ?: require {
val cachedKompilationArguments = cachedResult.kompilationArguments ?: error(
"Cached kompilation result missing arguments"
}
)
if (cachedResult.compilerName != compilerName) {
return null
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/kotlin/Kompile.kt
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ const val KOTLIN_COROUTINE_MIN_EXTRA_THREADS = 4
fun CompiledSource.usesCoroutines(): Boolean = source.sources.keys
.map { source.getParsed(it).tree }
.any { tree ->
tree as? KotlinParser.KotlinFileContext ?: check { "Parse tree is not from a Kotlin file" }
tree as? KotlinParser.KotlinFileContext ?: error("Parse tree is not from a Kotlin file")
tree.preamble().importList().importHeader().any { importName ->
KOTLIN_COROUTINE_IMPORTS.any { importName.identifier().text.startsWith(it) }
}
Expand Down
60 changes: 50 additions & 10 deletions core/src/main/kotlin/KotlinComplexity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ class KotlinComplexityListener(val source: Source, entry: Map.Entry<String, Stri
private val fileName = entry.key
private var currentClass = ""

private var anonymousClassDepth = 0
private var objectLiteralCounter = 0

private val currentComplexity: ComplexityValue
get() = complexityStack[0]

Expand Down Expand Up @@ -136,7 +139,13 @@ class KotlinComplexityListener(val source: Source, entry: Map.Entry<String, Stri
it.last().text
}
}
val longName = "$name($parameters)${returnType?.let { ":$returnType" } ?: ""}"
val longName = ("$name($parameters)${returnType?.let { ":$returnType" } ?: ""}").let {
if (anonymousClassDepth > 0) {
"${it}${"$"}$objectLiteralCounter"
} else {
it
}
}

enterMethodOrConstructor(
longName,
Expand Down Expand Up @@ -168,24 +177,36 @@ class KotlinComplexityListener(val source: Source, entry: Map.Entry<String, Stri

// ||
override fun enterDisjunction(ctx: KotlinParser.DisjunctionContext) {
if (!ctx.text.contains("||")) return
if (ctx.text.contains("(") || ctx.text.contains(")")) return
if (!ctx.text.contains("||")) {
return
}
if (ctx.text.contains("(") || ctx.text.contains(")")) {
return
}
require(complexityStack.isNotEmpty())
currentComplexity.complexity++
}

// &&
override fun enterConjunction(ctx: KotlinParser.ConjunctionContext) {
if (!ctx.text.contains("&&")) return
if (ctx.text.contains("(") || ctx.text.contains(")")) return
if (!ctx.text.contains("&&")) {
return
}
if (ctx.text.contains("(") || ctx.text.contains(")")) {
return
}
require(complexityStack.isNotEmpty())
currentComplexity.complexity++
}

// ?:
override fun enterElvisExpression(ctx: KotlinParser.ElvisExpressionContext) {
if (!ctx.text.contains("?:")) return
if (ctx.text.contains("(") || ctx.text.contains(")")) return
if (!ctx.text.contains("?:")) {
return
}
if (ctx.text.contains("(") || ctx.text.contains(")")) {
return
}
require(complexityStack.isNotEmpty())
currentComplexity.complexity++
}
Expand All @@ -199,7 +220,9 @@ class KotlinComplexityListener(val source: Source, entry: Map.Entry<String, Stri

// if & else if
override fun enterIfExpression(ctx: KotlinParser.IfExpressionContext) {
if (!ctx.text.contains("if")) return
if (!ctx.text.contains("if")) {
return
}
require(complexityStack.isNotEmpty())
currentComplexity.complexity++
}
Expand All @@ -224,18 +247,35 @@ class KotlinComplexityListener(val source: Source, entry: Map.Entry<String, Stri

// ?.
override fun enterMemberAccessOperator(ctx: KotlinParser.MemberAccessOperatorContext) {
if (ctx.text != "?.") return
if (ctx.text != "?.") {
return
}
require(complexityStack.isNotEmpty())
currentComplexity.complexity++
}

// !!.
override fun enterPostfixUnaryOperation(ctx: KotlinParser.PostfixUnaryOperationContext) {
if (ctx.text != "!!") return
if (ctx.text != "!!") {
return
}
require(complexityStack.isNotEmpty())
currentComplexity.complexity++
}

override fun enterObjectLiteral(ctx: KotlinParser.ObjectLiteralContext) {
if (ctx.classBody() != null) {
anonymousClassDepth++
objectLiteralCounter++
}
}

override fun exitObjectLiteral(ctx: KotlinParser.ObjectLiteralContext) {
if (ctx.classBody() != null) {
anonymousClassDepth--
}
}

init {
ParseTreeWalker.DEFAULT.walk(this, source.getParsed(fileName).tree)
}
Expand Down
7 changes: 4 additions & 3 deletions core/src/main/kotlin/Sandbox.kt
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ object Sandbox {
confinedTask.truncatedLines,
executionArguments
)
@Suppress("ThrowingExceptionsWithoutMessageOrCause")
runBlocking { resultChannel.send(ExecutorResult(executionResult, Exception())) }
} catch (e: Throwable) {
runBlocking { resultChannel.send(ExecutorResult(null, e)) }
Expand Down Expand Up @@ -1304,7 +1305,7 @@ object Sandbox {

@JvmStatic
fun redirectOutput(block: () -> Any?): JeedOutputCapture {
val confinedTask = confinedTaskByThreadGroup() ?: check { "should only be used from a confined task" }
val confinedTask = confinedTaskByThreadGroup() ?: error("should only be used from a confined task")
check(!confinedTask.redirectingOutput) { "can't nest calls to redirectOutput" }

confinedTask.redirectingOutput = true
Expand Down Expand Up @@ -1361,7 +1362,7 @@ object Sandbox {
* But the result is that you have to implement the entire PrintStream public API. Leaving anything out means that
* stuff doesn't get forwarded, and we have to make sure that nothing is shared.
*
* Hence this sadness.
* Hence, this sadness.
*
* PS: also fuck you Java for leaving me no choice but to resort to this. There are a half-dozen different terrible
* design decisions that led us to this place. Please do better next time.
Expand Down Expand Up @@ -1550,7 +1551,7 @@ object Sandbox {

System.setOut(RedirectingPrintStream(TaskResults.OutputLine.Console.STDOUT))
System.setErr(RedirectingPrintStream(TaskResults.OutputLine.Console.STDERR))
// Try to silence ThreadDeath error messages. Not sure this works but it can't hurt.
// Try to silence ThreadDeath error messages. Not sure if this works, but it can't hurt.
// Thread.setDefaultUncaughtExceptionHandler { _, _ -> }

threadPool = Executors.newFixedThreadPool(size)
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/kotlin/Snippet.kt
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ ${" ".repeat(snippetArguments.indent * 2)}@JvmStatic fun main() {""".lines().let
SnippetTransformationError(
SourceLocation(
SNIPPET_SOURCE,
looseCodeMapping[it.line] ?: require { "Missing loose code mapping " },
looseCodeMapping[it.line] ?: error("Missing loose code mapping"),
it.charPositionInLine
),
"return statements not allowed at top level in snippets"
Expand Down
17 changes: 2 additions & 15 deletions core/src/main/kotlin/Source.kt
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ open class Source(
)
}?.joinToString(separator = "") {
String.format(Locale.US, "%02x", it)
} ?: require { "Problem computing hash" }
} ?: error("Problem computing hash")
}

override fun equals(other: Any?): Boolean {
Expand All @@ -136,7 +136,7 @@ open class Source(
return when (val extension = filename.split("/").last().split(".").last()) {
"java" -> FileType.JAVA
"kt" -> FileType.KOTLIN
else -> require { "invalid extension: $extension" }
else -> error("invalid extension: $extension")
}
}

Expand Down Expand Up @@ -293,19 +293,6 @@ fun Method.getQualifiedName(): String {
return "$name(${parameters.joinToString(separator = ", ")})"
}

// Overloads of built-in functions that can be used to the right of Elvis operators
fun assert(block: () -> String): Nothing {
throw AssertionError(block())
}

fun check(block: () -> String): Nothing {
throw IllegalStateException(block())
}

fun require(block: () -> String): Nothing {
throw IllegalArgumentException(block())
}

class SourceMappingException(message: String) : Exception(message)

fun Source.FileType.extension() = when {
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/kotlin/Template.kt
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ fun Source.Companion.fromTemplates(sourceMap: Map<String, String>, templateMap:
} catch (e: HandlebarsException) {
templatingErrors.add(TemplatingError(templateName, e.error.line, e.error.column, e.error.message))
return@mapValues ""
} ?: assert { "should have created a template" }
} ?: error("should have created a template")

try {
val indentedSource = source.lines().mapIndexed { i, line ->
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
version=2021.10.0
version=2021.10.1
8 changes: 6 additions & 2 deletions core/src/test/kotlin/TestJavaComplexity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,17 @@ interface Simple {
interface Test {
void test();
}
Test test = new Test() {
Test first = new Test() {
@Override
public void test() { }
};
Test second = new Test() {
@Override
public void test() { }
};
""".trim()
).complexity().also {
it.lookup(".", "").complexity shouldBe 2
it.lookup(".", "").complexity shouldBe 3
}
}
"should not fail on generic methods" {
Expand Down
14 changes: 14 additions & 0 deletions core/src/test/kotlin/TestKotlinComplexity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,20 @@ fun main2(first: Int, second: String, third: Blah?): Int {
it.lookupFile("Main.kt") shouldBe 2
}
}
"should handle multiple anonymous classes" {
Source.fromKotlin(
"""
interface Adder {
fun addTo(value: Int): Int
}
val addOne = object : Adder {
override fun addTo(value: Int) = value + 1
}
val addEight = object : Adder {
override fun addTo(value: Int) = value + 8
}""".trim()
).complexity()
}
"should not overflow on deep nesting" {
shouldThrow<ComplexityFailed> {
Source.fromKotlin(
Expand Down
2 changes: 1 addition & 1 deletion server/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ plugins {
kotlin("kapt")
application
`maven-publish`
id("com.github.johnrengelman.shadow") version "7.0.0"
id("com.github.johnrengelman.shadow") version "7.1.0"
id("com.palantir.docker") version "0.30.0"
id("org.jmailen.kotlinter")
id("io.gitlab.arturbosch.detekt")
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1 @@
version=2021.10.0
version=2021.10.1

0 comments on commit 349dca3

Please sign in to comment.