Run detekt with type resolution analysis on CI (#3015)

* Fix unsafe call issues or use cleaner preconditions

* Run detekt with type resolution analysis on CI

* Update detekt-test/src/main/kotlin/io/gitlab/arturbosch/detekt/test/FindingsAssertions.kt

* Run detekt with type resolution analysis on CI

* Let detekt run only in separate CI job

* Revert change to open; instead suppress issue

.github/workflows/detekt-with-type-resolution.yaml

@@ -0,0 +1,37 @@
+name: detekt with type resolution
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - '*'
+
+jobs:
+  gradle:
+    runs-on: ubuntu-latest
+    if: ${{ !contains(github.event.head_commit.message, 'ci skip') }}
+    env:
+      GRADLE_OPTS: -Dorg.gradle.daemon=false
+    steps:
+    - name: Checkout Repo
+      uses: actions/checkout@v2
+
+    - name: Cache Gradle Folders
+      uses: actions/cache@v2
+      with:
+        path: |
+          ~/.gradle/caches/
+          ~/.gradle/wrapper/
+        key: cache-gradle-${{ hashFiles('detekt-bom/build.gradle.kts') }}
+        restore-keys: |
+          cache-gradle-
+
+    - name: Setup Java
+      uses: actions/setup-java@v1
+      with:
+        java-version: 11
+
+    - name: Run analysis
+      run: ./gradlew detektMain detektTest --build-cache --parallel

.github/workflows/pre-merge.yaml

@@ -53,7 +53,7 @@ jobs:
 
 
     - name: Build detekt
-      run: ./gradlew build :detekt-cli:shadowJarExecutable --parallel
+      run: ./gradlew build :detekt-cli:shadowJarExecutable --parallel -x detekt
     - name: Run detekt-cli --help
       run: java -jar ./detekt-cli/build/run/detekt --help
     - name: Run detekt-cli with argsfile

detekt-api/src/main/kotlin/io/gitlab/arturbosch/detekt/api/internal/ValidatableConfiguration.kt

@@ -54,16 +54,16 @@ fun validateConfig(
 
             val propertyPath = "${if (parentPath == null) "" else "$parentPath>"}$prop"
 
-            val matchedDeprecation = DEPRECATED_PROPERTIES
+            val deprecationWarning = DEPRECATED_PROPERTIES
                 .find { (regex, _) -> regex.matches(propertyPath) }
-            val isDeprecated = matchedDeprecation != null
+                ?.second
             val isExcluded = excludePatterns.any { it.matches(propertyPath) }
 
-            if (isDeprecated) {
-                notifications.add(propertyIsDeprecated(propertyPath, matchedDeprecation!!.second))
+            if (deprecationWarning != null) {
+                notifications.add(propertyIsDeprecated(propertyPath, deprecationWarning))
             }
 
-            if (isDeprecated || isExcluded) {
+            if (deprecationWarning != null || isExcluded) {
                 continue
             }
 

detekt-core/src/main/kotlin/io/gitlab/arturbosch/detekt/core/Analyzer.kt

@@ -37,6 +37,7 @@ internal class Analyzer(
         bindingContext: BindingContext = BindingContext.EMPTY
     ): Map<RuleSetId, List<Finding>> {
         val languageVersionSettings = settings.environment.configuration.languageVersionSettings
+
         @Suppress("DEPRECATION")
         val dataFlowValueFactory = DataFlowValueFactoryImpl(languageVersionSettings)
         val compilerResources = CompilerResources(languageVersionSettings, dataFlowValueFactory)
@@ -113,9 +114,9 @@ internal class Analyzer(
             for (rule in rules) {
                 rule.visitFile(file, bindingContext, compilerResources)
                 for (finding in rule.findings) {
-                    val mappedRuleSet = idMapping[finding.id] ?: error("Mapping for '${finding.id}' expected.")
-                    result.putIfAbsent(mappedRuleSet, mutableListOf())
-                    result[mappedRuleSet]!!.add(finding)
+                    val mappedRuleSet = checkNotNull(idMapping[finding.id]) { "Mapping for '${finding.id}' expected." }
+                    result.computeIfAbsent(mappedRuleSet) { mutableListOf() }
+                        .add(finding)
                 }
             }
         }

detekt-core/src/test/kotlin/io/gitlab/arturbosch/detekt/core/rules/MultiRuleSpec.kt

@@ -83,6 +83,7 @@ private class TestMultiRule(config: Config) : MultiRule() {
     }
 }
 
+@Suppress("detekt.UnnecessaryAbstractClass") // uses inherited members
 private abstract class AbstractRule(config: Config) : Rule(config) {
     override val issue: Issue = Issue(javaClass.simpleName, Severity.Minor, "", Debt.TWENTY_MINS)
     override fun visitKtFile(file: KtFile) = report(CodeSmell(issue, Entity.from(file), message = ""))

detekt-metrics/src/main/kotlin/io/github/detekt/metrics/ComplexityReportGenerator.kt

@@ -43,8 +43,8 @@ class ComplexityReportGenerator(private val complexityMetric: ComplexityMetric)
             else -> {
                 numberOfSmells = complexityMetric.findings.sumBy { it.value.size }
                 smellPerThousandLines = numberOfSmells * 1000 / complexityMetric.lloc
-                mccPerThousandLines = complexityMetric.mcc!! * 1000 / complexityMetric.lloc
-                commentSourceRatio = complexityMetric.cloc!! * 100 / complexityMetric.sloc
+                mccPerThousandLines = requireNotNull(complexityMetric.mcc) * 1000 / complexityMetric.lloc
+                commentSourceRatio = requireNotNull(complexityMetric.cloc) * 100 / complexityMetric.sloc
                 false
             }
         }

detekt-rules-complexity/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/complexity/TooManyFunctions.kt

@@ -79,7 +79,7 @@ class TooManyFunctions(config: Config = Config.empty) : Rule(config) {
             klass.isInterface() -> {
                 if (amount >= thresholdInInterfaces) {
                     report(ThresholdedCodeSmell(issue,
-                        Entity.from(klass.nameIdentifier!!),
+                        Entity.atName(klass),
                         Metric("SIZE", amount, thresholdInInterfaces),
                         "Interface '${klass.name}' with '$amount' functions detected. " +
                                 "Defined threshold inside interfaces is set to " +
@@ -89,7 +89,7 @@ class TooManyFunctions(config: Config = Config.empty) : Rule(config) {
             klass.isEnum() -> {
                 if (amount >= thresholdInEnums) {
                     report(ThresholdedCodeSmell(issue,
-                        Entity.from(klass.nameIdentifier!!),
+                        Entity.atName(klass),
                         Metric("SIZE", amount, thresholdInEnums),
                         "Enum class '${klass.name}' with '$amount' functions detected. " +
                                 "Defined threshold inside enum classes is set to " +
@@ -99,7 +99,7 @@ class TooManyFunctions(config: Config = Config.empty) : Rule(config) {
             else -> {
                 if (amount >= thresholdInClasses) {
                     report(ThresholdedCodeSmell(issue,
-                            Entity.from(klass.nameIdentifier!!),
+                            Entity.atName(klass),
                             Metric("SIZE", amount, thresholdInClasses),
                             "Class '${klass.name}' with '$amount' functions detected. " +
                                     "Defined threshold inside classes is set to '$thresholdInClasses'"))

detekt-rules-empty/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/empty/EmptyRule.kt

@@ -14,6 +14,7 @@ import org.jetbrains.kotlin.psi.KtExpression
 /**
  * Rule to detect empty blocks of code.
  */
+@Suppress("detekt.UnnecessaryAbstractClass") // we really do not want instances of this class
 abstract class EmptyRule(
     config: Config,
     description: String = "Empty block of code detected. As they serve no purpose they should be removed.",

detekt-test/src/main/kotlin/io/gitlab/arturbosch/detekt/test/FindingsAssertions.kt

@@ -25,8 +25,6 @@ class FindingsAssert(actual: List<Finding>) :
             FindingAssert(value).`as`(description)
 
     fun hasSourceLocations(vararg expected: SourceLocation) = apply {
-        isNotNull
-
         val actualSources = actual.asSequence()
                 .map { it.location.source }
                 .sortedWith(compareBy({ it.line }, { it.column }))
@@ -46,8 +44,6 @@ class FindingsAssert(actual: List<Finding>) :
     }
 
     fun hasTextLocations(vararg expected: Pair<Int, Int>) = apply {
-        isNotNull
-
         val actualSources = actual.asSequence()
                 .map { it.location.text }
                 .sortedWith(compareBy({ it.start }, { it.end }))
@@ -64,8 +60,6 @@ class FindingsAssert(actual: List<Finding>) :
     }
 
     fun hasTextLocations(vararg expected: String): FindingsAssert {
-        isNotNull
-
         val finding = actual.firstOrNull()
         if (finding == null) {
             if (expected.isEmpty()) {
@@ -74,13 +68,12 @@ class FindingsAssert(actual: List<Finding>) :
                 failWithMessage("Expected ${expected.size} findings but was 0")
             }
         }
-        val code = finding!!.entity.ktElement?.containingKtFile?.text
-        if (code == null) {
-            failWithMessage("Expected ${expected.size} findings but was 0")
+        val code = requireNotNull(finding?.entity?.ktElement?.containingKtFile?.text) {
+            "Finding expected to provide a KtElement."
         }
 
         val textLocations = expected.map { snippet ->
-            val index = code!!.indexOf(snippet)
+            val index = code.indexOf(snippet)
             if (index < 0) {
                 failWithMessage("The snippet \"$snippet\" doesn't exist in the code")
             } else {