Skip to content

Conversation

wax911
Copy link
Member

@wax911 wax911 commented Feb 24, 2025

Pull Request Template

Thank you for contributing! Please take a moment to review our contributing guidelines
to make the process easy and effective for everyone involved.

Please open an issue before embarking on any significant pull request, especially those that
add a new library or change existing tests, otherwise you risk spending a lot of time working
on something that might not end up being merged into the project.

Before opening a pull request, please ensure:

  • You have followed our contributing guidelines
  • Double checked that your branch is based on develop and targets develop (where applicable)
  • Pull request has tests (if applicable)
  • Documentation is updated (if necessary)
  • Description explains the issue/use-case resolved

Description

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Enhancement (improves existing functionality)

IMPORTANT: By submitting a patch, you agree to allow the project
owners to license your work under the terms of the Apache License.

@wax911 wax911 linked an issue Feb 24, 2025 that may be closed by this pull request
@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch from e87d1b9 to 1e0f062 Compare March 6, 2025 20:44
@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch from 1e0f062 to e5eb429 Compare March 14, 2025 20:34
@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch from e5eb429 to f847241 Compare April 6, 2025 11:59
Copy link

github-actions bot commented May 7, 2025

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch from f847241 to 87e6f47 Compare May 11, 2025 17:55
@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch from 87e6f47 to a5e6cc6 Compare May 31, 2025 15:17
Copy link

github-actions bot commented Jul 1, 2025

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch from a5e6cc6 to 0bd02bd Compare August 3, 2025 11:57
@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch 4 times, most recently from e13837b to 829b5ab Compare August 15, 2025 13:14
@wax911 wax911 marked this pull request as ready for review August 15, 2025 13:17
@Copilot Copilot AI review requested due to automatic review settings August 15, 2025 13:17
Copilot

This comment was marked as outdated.

@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch 2 times, most recently from e8f0f46 to 17eb0cc Compare August 15, 2025 19:54
@wax911 wax911 force-pushed the feature/34-ksp-migration-from-kapt branch from 17eb0cc to f310733 Compare August 23, 2025 12:48
@wax911 wax911 requested a review from Copilot August 28, 2025 20:18
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the annotation processor from Java annotation processing tool (APT/kapt) to Kotlin Symbol Processing (KSP). This represents a significant modernization effort, moving from the legacy annotation processing model to KSP's more efficient and Kotlin-native approach.

  • Replaces the entire annotation processor infrastructure with KSP implementation
  • Updates build configurations across modules to use KSP instead of kapt
  • Adds comprehensive test suite using kotlin-compile-testing

Reviewed Changes

Copilot reviewed 27 out of 29 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
sample/src/main/kotlin/co/anitrend/support/query/builder/sample/data/entity/pet/PetEntity.kt Replaces wildcard import with explicit Room annotation imports
sample/build.gradle.kts Migrates from kapt to KSP, adds explicit project dependencies
processor/src/test/kotlin/**/*.kt Adds new test infrastructure for KSP processor testing
processor/src/main/kotlin/**/Candidate.kt Rewrites candidate model to work with KSP symbols instead of javax elements
processor/src/main/kotlin/**/ClassFactory.kt Updates code generation to use KSP's CodeGenerator
processor/src/main/kotlin/**/Processor.kt New KSP-based processor replacing the old APT processor
processor/build.gradle.kts Updates dependencies from kapt to KSP toolchain
gradle/libs.versions.toml Updates dependency versions and adds KSP-related libraries
buildSrc/**/*.kt Removes kapt plugin application and updates lifecycle dependency references

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +32 to +37
logger.info("[KSCandidate] Column name for $classDeclaration as `${columnInfo.shortName.asString()}`")

val columnName = columnInfo.arguments.find { argument ->
argument.name?.getShortName() == ColumnInfo::name.name
}?.value as String

Copy link
Preview

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

The log message uses columnInfo.shortName.asString() which would print the annotation class name, but it should print the actual column name from the annotation argument.

Suggested change
logger.info("[KSCandidate] Column name for $classDeclaration as `${columnInfo.shortName.asString()}`")
val columnName = columnInfo.arguments.find { argument ->
argument.name?.getShortName() == ColumnInfo::name.name
}?.value as String
val columnName = columnInfo.arguments.find { argument ->
argument.name?.getShortName() == ColumnInfo::name.name
}?.value as String
logger.info("[KSCandidate] Column name for $classDeclaration as `$columnName`")

Copilot uses AI. Check for mistakes.

logger.warn("[KSCandidate] Embedded property `${propertyDeclaration.simpleName.getShortName()}` does not have a prefix argument")
} else {
logger.info(
"[KSCandidate] Embedded prefix for `${argument.name}` as `${prefix}`}",
Copy link
Preview

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

Extra closing brace in the log message - should be ${prefix}" instead of ${prefix}}`

Suggested change
"[KSCandidate] Embedded prefix for `${argument.name}` as `${prefix}`}",
"[KSCandidate] Embedded prefix for `${argument.name}` as `${prefix}`",

Copilot uses AI. Check for mistakes.

tableName,
getColumns(element),
getEmbedded(),
name = requireNotNull(tableName) { "[KSCandidate.getTable] Table name cannot be null" },
Copy link
Preview

Copilot AI Aug 28, 2025

Choose a reason for hiding this comment

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

The tableName is already cast as String on line 94, so it cannot be null. Either remove the requireNotNull or handle the case where the annotation argument might not exist.

Suggested change
name = requireNotNull(tableName) { "[KSCandidate.getTable] Table name cannot be null" },
name = tableName,

Copilot uses AI. Check for mistakes.

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

Successfully merging this pull request may close these issues.

KSP migration in anticipation of Kotlin 2.x
1 participant