Skip to content

chore: add detekt with basic ruleset#622

Open
wilmveel wants to merge 7 commits into
masterfrom
chore/add-detekt
Open

chore: add detekt with basic ruleset#622
wilmveel wants to merge 7 commits into
masterfrom
chore/add-detekt

Conversation

@wilmveel

Copy link
Copy Markdown
Contributor

Summary

Adds detekt static analysis to the project, following the same convention-plugin pattern used for spotless.

  • gradle/plugins/detekt/ — convention plugin module.detekt (detekt 1.23.8, buildUponDefaultConfig = true)
  • config/detekt/detekt.yml — basic ruleset overlaying detekt defaults (magic-number off, 160-char line limit, slightly relaxed complexity thresholds)
  • id("module.detekt") applied to the same 22 Kotlin modules that already use module.spotless

How to run

./gradlew detekt

Produces HTML and XML reports per module under build/reports/detekt/.

Not included (requires workflow scope)

A CI job is not added in this PR — the workflow file requires a token with workflow scope to modify. Suggested job to add manually to .github/workflows/build.yml:

  detekt:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-java@v4
        with:
          java-version: '21'
          distribution: 'temurin'
          cache: gradle
      - name: Run detekt
        run: ./gradlew detekt
      - name: Upload detekt reports
        if: always()
        uses: actions/upload-artifact@v4
        with:
          name: detekt-reports
          path: '**/build/reports/detekt/'

…and add detekt to the success job's needs: list.

Test plan

  • ./gradlew detekt runs to completion
  • Reports are generated under build/reports/detekt/
  • Baseline or suppressions added for any findings that surface

- Add a module.detekt convention plugin mirroring module.spotless
- Enable detekt on all Kotlin modules under src/ via the convention plugin
- Provide a basic ruleset at config/detekt/detekt.yml that builds on
  detekt's defaults with a handful of project-friendly overrides
- Add a dedicated detekt CI job uploading HTML/XML reports as artifacts
- Generate per-module detekt-baseline.xml so existing findings don't break
  the build; detekt now enforces "no new findings"
- Pin Kotlin to 2.0.21 on detekt configurations so the analyzer keeps its
  own embedded stdlib
- Skip detekt on :src:integration:avro and :src:integration:spring; their
  Spring Boot dependencies pull Kotlin 2.3.x onto the detekt classpath and
  hit detekt 1.23.8's "compiled with Kotlin 2.0.21" compatibility check
Address the 10 findings baselined for :src:compiler:core:

- Extract parseEndpointDefinition helper methods to drop complexity/length
- Move TokenizeOptions to its own file to satisfy MatchingDeclarationName
- Rename SpaceEmitter.kt to Spacer.kt and rename SPACER const to INDENT
- Break overlong lines in Compiler.kt, Tokenizer.kt, ParserException.kt
  and Validator.kt

Baseline file is removed; detekt now enforces the rules going forward.
Previously there were 15 per-module baseline files grandfathering 90 pre-
existing findings. This commit removes all baselines and resolves the
underlying issues so detekt can enforce the ruleset for the whole repo.

Approach:

- Disable stylistic rules that fight common idioms:
    SpreadOperator, ExplicitItLambdaParameter, InvalidPackageDeclaration
- Moderately raise complexity thresholds to match parser/emitter reality:
    CyclomaticComplexMethod 15→25, LongMethod 60→100, NestedBlockDepth 4→5,
    TooManyFunctions bumps for files/classes/objects/interfaces/enums
- Raise ThrowsCount max to 4 for task/mojo entry points
- Replace generic RuntimeException throws with Gradle/Maven-specific
  exception types in the Gradle and Maven plugins; narrow catch clauses
  to ReflectiveOperationException / LinkageError where applicable
- @file:Suppress("MaxLineLength") on emitter files that embed
  target-language codegen templates to preserve output fidelity
- @file:Suppress on the OpenAPI v2/v3 parsers for the remaining
  complexity/nesting/line-length hotspots
- Refactor intellij-plugin Lexer/SyntaxHighlighter to map-backed
  dispatch; flatten ChooseByNameContributor and Reference loops
- Rename SPACER→INDENT inside Spacer, move TokenizeOptions to its own
  file, break overlong lines in Compiler/Tokenizer/ParserException/
  Validator (pulled in from #621)
- Break long method signatures in Jackson KotlinReservedKeywordNamingStrategy
- Fix WirespecPlugin.apply() empty block with in-body comment; redundant
  empty constructor removed from TypeScriptEmitter
Exclude Claude agent state files (scheduled_tasks.lock etc.) that should never be tracked.
@sonarqubecloud

Copy link
Copy Markdown

@@ -1,3 +1,5 @@
@file:Suppress("MaxLineLength") // Embedded Kotlin codegen templates stay on one line to preserve output fidelity.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what does this mean?

@@ -1,3 +1,5 @@
@file:Suppress("MaxLineLength") // Test fixtures include opaque regex literals.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

suppress only on the opaque regex then

@@ -1,3 +1,7 @@
// OpenAPI v2 covers a large spec surface; toReference/flatten intentionally
// dispatch over many schema shapes in a single function for readability.
@file:Suppress("CyclomaticComplexMethod", "NestedBlockDepth", "LongMethod")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

But it is not readable with all the cyclomatic complexity and nestedness...

@@ -1,3 +1,5 @@
@file:Suppress("MatchingDeclarationName") // Groups Node.js process bindings

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what does this mean?

class WirespecPlugin : Plugin<Project> {
override fun apply(project: Project) {}
override fun apply(project: Project) {
// Tasks are registered directly by consumers via CompileWirespecTask and

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nice!

}

log.info("Load preprocessor: $preProcessor")
val preProcessorName = preProcessor ?: return input

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

avoid nested return statements

plugins {
id("module.publication")
id("module.spotless")
id("module.detekt")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is it possible to activate this once for all modules and then opt out where it cannot be used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants