[sergej-koscejev]

Summary

New Kotlin guidelines:

• Keep mutable state owned by one class.
• Prefer a nested or top-level class to an inner class. An inner class needs a comment above it that says why it is inner.
• Keep each decision in one place.
• Make the role of a class clear from its name.

A new detekt rule, UnexplainedInnerClass, enforces the inner class bullet.

 settings.gradle.kts
+  include("detekt-rules")
 detekt-rules/                                # new module
+  UnexplainedInnerClass.kt                   # the rule
+  MopsRuleSetProvider.kt                     # rule set "mops"
 build-logic/mops.kotlin-jvm-conventions
+  detektPlugins(project(":detekt-rules"))    # every Kotlin module, detekt-rules too
 config/detekt/detekt.yml
+  mops: UnexplainedInnerClass: active
 daemon/.../JetBrainsMpsAccess.kt
+  // Inner: ...                               # reason comments on the 3 existing inner classes

What the rule reports:

on class declaration
  if it is declared `inner`
    and no comment (// , /* */, or KDoc) is directly above it
      report

The rule only checks that a comment is there. Reviewers check that the comment says why the class is inner. The rule needs no type resolution.

Evidence

• Rule tests (:detekt-rules:test, 3 tests, all pass, each compares the full list of findings):

  • inner class without a comment → 1 finding, on its line
  • inner classes with a // comment and with a KDoc above them → no findings
  • nested non-inner class → no findings

• Real build: with the comment above JetBrainsMpsWrite removed, :daemon:detektMain fails:

  e: JetBrainsMpsAccess.kt:452:25 Inner class JetBrainsMpsWrite has no comment that says why it is inner. Make it a nested or top-level class, or add the comment. [UnexplainedInnerClass]
  

• ./gradlew detektMain :detekt-rules:test :architecture-tests:test cpdCheck passes.

Known future hit: once #217 lands, the rule reports Pass in daemon/src/main/kotlin/com/specificlanguages/mops/daemon/WatchedFolderRefresh.kt. Pass is an inner class with no comment above it, and it changes the outer refreshedFolders.

Merge Danger

Blast Radius: build

Every Kotlin module's detekt run now loads the detekt-rules jar. The rule is compiled against the detekt 2.0 alpha API, so a detekt upgrade may need changes to the rule.