Kotlin Review

Kotlin Code Review

Review Kotlin source files, Gradle build configs, and Flyway migrations against team conventions and official Kotlin best practices. This skill should be loaded both when explicitly asked to review Kotlin code and proactively after writing significant Kotlin code.

Bootstrap

No setup required. The agent must have read access to the Kotlin source files under review. Before starting:

1. Identify which service or module the code belongs to.
2. Read the project's AGENTS.md and .editorconfig if you have not already.
3. Check the project's build.gradle.kts to understand toolchain version, linting, and coverage settings.

Workflow

Follow these steps for every review. Do NOT skip any step.

1. Identify changed files

Determine which files are under review -- from a PR diff, staged changes, or files the agent just wrote. Categorize each file:

2. Run the formatting and lint checklist

For each production and test Kotlin file, verify:

• No star imports -- every import is explicit
• Trailing commas on all multi-line parameter lists, argument lists, and collection literals
• Max line length -- respect the project's .editorconfig setting (typically 140 chars for production, unlimited for tests)
• No tabs -- indentation uses 4 spaces

If in doubt, suggest running:

./gradlew formatKotlin && ./gradlew lintKotlinMain

3. Review naming conventions

Flag any deviation from these patterns.

4. Review language idioms

Check for these required patterns:

• val over var -- mutable state must be justified
• Data classes for entities, DTOs, value objects, config properties
• @Embeddable data class for composite JPA keys with @EmbeddedId
• Named arguments on all multi-parameter function calls
• when expression over if/else chains with 3+ branches
• Extension functions for mappers (not Mapper objects or standalone functions)
• requireNotNull() / require() for precondition validation -- never silent fallbacks
• copy() for immutable updates on data classes
• Nested enums inside entities when they logically belong there, with companion object factory methods using entries.find {} (not valueOf())
• inline fun <reified T> to avoid type erasure when deserializing generics

Check for these anti-patterns and flag them:

5. Review error handling

Message consumer error classification

Every message consumer must classify errors into retryable vs. non-retryable:

Verify that all consumer code paths handle errors according to this classification. If a shared pipeline function handles this centrally, verify that manual consumers replicate the same classification.

Custom exceptions

• Must extend RuntimeException (Kotlin has no checked exceptions)
• HTTP clients should distinguish retryable vs. non-retryable subtypes
• Use @Retryable / @Recover for external HTTP calls with appropriate backoff configuration

REST API errors

• Global exception handling via @RestControllerAdvice
• Consistent error response body with message, error, status, path, timestamp

6. Review Spring Boot patterns

• Constructor injection only -- no @Autowired on fields, no setter injection
• Controllers contain no business logic -- delegate entirely to services
• Controllers implement generated OpenAPI interface when code generation is used
• @Transactional on service methods only -- not on controllers or consumers
• Profile-split security -- separate @Configuration classes for @Profile("!local") and @Profile("local")
• Typed @ConfigurationProperties data classes for configuration
• Observability annotations (@Timed, @WithSpan) on consumer handler methods
• Structured logging context via withMDC() helper or equivalent -- never raw MDC.put() without cleanup

Consumer-to-service boundary

Services must accept primitives and domain value objects, not generated event DTO types. The consumer is responsible for:

1. Deserializing the event
2. Extracting fields
3. Passing primitives/domain objects to the service

// CORRECT -- consumer extracts, service receives domain types
orderService.processOrder(
    orderId = OrderId(type = event.orderType, code = event.orderCode),
    customerName = event.customer.name,
    quantity = event.quantity,
)

// WRONG -- service coupled to generated DTO
orderService.processOrder(event)

7. Review test code

Unit tests

• Uses @ExtendWith(MockKExtension::class)
• Test subject is sut with @InjectMockKs
• Dependencies are @MockK with lateinit var
• Test method names use backtick GIVEN/WHEN/THEN format
• Assertions use AssertJ assertThat() -- not Kotlin assert()
• verify(exactly = N) for call count verification
• slot<T>() for argument capture when needed
• assertThrows<ExceptionType> {} for exception assertions
• Parameterized tests use @ParameterizedTest + @MethodSource with @JvmStatic companion method
• No @Disabled, @Ignore, .skip(), or .only()

Integration tests

• Extends a shared AbstractIntegrationTest base class
• Uses Testcontainers for real infrastructure (database, message broker)
• External services stubbed with WireMock (@AutoConfigureWireMock)
• Reference data pre-seeded via JPA repositories
• Asynchronous assertions wrapped in Awaitility (await().atMost(10.seconds).untilAsserted {})
• JSON fixtures loaded from src/test/resources/
• Tests exercise the full consumer path -- not just the service layer in isolation

8. Review Gradle build configuration

When build.gradle.kts or settings.gradle.kts is modified:

• Java 21 toolchain configured
• Kotlinter configured with ignoreLintFailures = false and ignoreFormatFailures = false
• JaCoCo minimum threshold is at least 0.8 (80%) -- flag any reduction
• Generated sources excluded from linting
• Generated sources excluded from JaCoCo coverage measurement
• compileKotlin depends on code generation tasks (OpenAPI, AsyncAPI)
• Test JVM args include --add-opens for MockK on Java 21+
• check task depends on installKotlinterPrePushHook
• Integration test source set properly configured with classpath extending testImplementation

9. Review Flyway migrations

When SQL migration files are added or modified:

• Versioned migrations follow V{N}__description.sql naming
• Undo migrations provided: U{N}__description.sql matching each versioned migration
• Version number {N} is the next sequential number (check existing migrations)
• No destructive operations without an undo migration
• Repeatable migrations use R__description.sql
• No secrets or hardcoded credentials in SQL files

10. Summarize findings

Produce a review summary grouped by severity:

For each finding, include:

• File path and line number
• What the issue is
• Why it matters
• Suggested fix (with code if applicable)

Error Handling