style(detekt): Forbid usage of kotlinx.coroutines.runBlocking

Forbid the usage of `kotlinx.coroutines.runBlocking` as the replacement `org.ossreviewtoolkit.utils.ort.runBlocking` should be used instead. The replacement preserves Log4j's MDC context which is important when ORT is used as a library and the consumer sets an MDC context which should also appear in ORT's log output. The rule is suppressed when `runCatching` is used in code that is not supposed to be used as a library, like the CLI commands. Signed-off-by: Martin Nonnenmacher <[email protected]>
oss-review-toolkit · Aug 20, 2024 · 17d1ff2 · 17d1ff2
1 parent ee3c33b
commit 17d1ff2
Show file tree

Hide file tree

Showing 7 changed files with 12 additions and 0 deletions.
diff --git a/.detekt.yml b/.detekt.yml
@@ -70,6 +70,11 @@ style:
     maxDestructuringEntries: 4
   ForbiddenComment:
     active: false
+  ForbiddenMethodCall:
+    active: true
+    # Use org.ossreviewtoolkit.utils.ort.runBlocking instead in all code that can be used as a library.
+    # One exception to that are the CLI commands.
+    methods: ['kotlinx.coroutines.runBlocking']
   LoopWithTooManyJumpStatements:
     active: false
   MagicNumber:

diff --git a/plugins/commands/advisor/src/main/kotlin/AdvisorCommand.kt b/plugins/commands/advisor/src/main/kotlin/AdvisorCommand.kt
@@ -125,6 +125,8 @@ class AdvisorCommand : OrtCommand(
         val advisor = Advisor(distinctProviders, ortConfig.advisor)
 
         val ortResultInput = readOrtResult(ortFile)
+
+        @Suppress("ForbiddenMethodCall")
         val ortResultOutput = runBlocking {
             advisor.advise(ortResultInput, skipExcluded || ortConfig.advisor.skipExcluded).mergeLabels(labels)
         }

diff --git a/plugins/commands/downloader/src/main/kotlin/DownloaderCommand.kt b/plugins/commands/downloader/src/main/kotlin/DownloaderCommand.kt
@@ -298,6 +298,7 @@ class DownloaderCommand : OrtCommand(
 
         val packageDownloadDirs = packages.associateWith { outputDir.resolve(it.id.toPath()) }
 
+        @Suppress("ForbiddenMethodCall")
         runBlocking { downloadAllPackages(packageDownloadDirs, failureMessages) }
 
         if (archiveMode == ArchiveMode.BUNDLE && !dryRun) {

diff --git a/plugins/commands/reporter/src/main/kotlin/ReporterCommand.kt b/plugins/commands/reporter/src/main/kotlin/ReporterCommand.kt
@@ -281,6 +281,7 @@ class ReporterCommand : OrtCommand(
         }
 
         val reportDurationMap = measureTimedValue {
+            @Suppress("ForbiddenMethodCall")
             runBlocking(Dispatchers.Default) {
                 reportFormats.map { reporter ->
                     async {

diff --git a/plugins/commands/scanner/src/main/kotlin/ScannerCommand.kt b/plugins/commands/scanner/src/main/kotlin/ScannerCommand.kt
@@ -159,6 +159,7 @@ class ScannerCommand : OrtCommand(
             .print().conclude(ortConfig.severeIssueThreshold, ORT_FAILURE_STATUS_CODE)
     }
 
+    @Suppress("ForbiddenMethodCall")
     private fun runScanners(
         scannerWrapperFactories: List<ScannerWrapperFactory<*>>,
         projectScannerWrapperFactories: List<ScannerWrapperFactory<*>>,

diff --git a/utils/ort/src/main/kotlin/Utils.kt b/utils/ort/src/main/kotlin/Utils.kt
@@ -227,4 +227,5 @@ fun normalizeVcsUrl(vcsUrl: String): String {
  * preserve any MDC context set by a consumer.
  */
 fun <T> runBlocking(context: CoroutineContext = EmptyCoroutineContext, block: suspend CoroutineScope.() -> T): T =
+    @Suppress("ForbiddenMethodCall")
     kotlinx.coroutines.runBlocking(context + CoroutineThreadContext()) { block() }
diff --git a/utils/ort/src/test/kotlin/UtilsTest.kt b/utils/ort/src/test/kotlin/UtilsTest.kt
@@ -482,6 +482,7 @@ class UtilsTest : WordSpec({
     "runBlocking" should {
         "preserve Log4j's MDC context which kotlinx.coroutines.runBlocking does not" {
             withLoggingContext(mapOf("key" to "value")) {
+                @Suppress("ForbiddenMethodCall")
                 kotlinx.coroutines.runBlocking(EmptyCoroutineContext) {
                     coroutineContext[CoroutineThreadContext.Key]?.contextData?.map?.get("key") should beNull()
                 }