Skip to content

Fix nullable strings in kotlin#50

Merged
vshcryabets merged 2 commits intomainfrom
fix/kotlin-nullable-string
Apr 7, 2026
Merged

Fix nullable strings in kotlin#50
vshcryabets merged 2 commits intomainfrom
fix/kotlin-nullable-string

Conversation

@vshcryabets
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown
Contributor

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 updates the Kotlin type-name generator to correctly support nullable strings (DataType.stringNullable) and adds a regression test, while also enabling the Foojay toolchain resolver in Gradle settings.

Changes:

  • Add Foojay toolchain resolver convention plugin to settings.gradle.kts.
  • Update GetTypeNameUseCase to map stringNullable to Kotlin String (with ? appended via canBeNull).
  • Add a unit test to validate DataType.stringNullable renders as String?.

Reviewed changes

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

File Description
settings.gradle.kts Adds Foojay toolchain resolver plugin configuration.
cgen-lib/src/main/kotlin/generators/kotlin/GetTypeNameUseCase.kt Extends the Kotlin type mapping to include stringNullable.
cgen-lib/src/test/java/generators/kotlin/GetTypeNameUseCaseTest.kt Adds a test asserting nullable string maps to String?.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

DataType.float64, DataType.float64Nullable -> "Double"
DataType.bool, DataType.boolNullable -> "Boolean"
is DataType.string -> "String"
is DataType.string, DataType.stringNullable -> "String"
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

The when branch mixes a type-check (is DataType.string) with a value match (DataType.stringNullable) in the same condition list, which won’t compile. Since string/stringNullable are singleton objects, match them consistently (e.g., DataType.string, DataType.stringNullable -> "String"), or use is for both.

Suggested change
is DataType.string, DataType.stringNullable -> "String"
DataType.string, DataType.stringNullable -> "String"

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +10
val getArrayDataTypeUseCase = GetArrayDataTypeUseCase()
val useCase = GetTypeNameUseCase(getArrayDataTypeUseCase)
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

These helper fields are declared as public vals; in the existing tests in this package they’re typically private val to avoid leaking test internals and to align with JUnit test style in this codebase.

Suggested change
val getArrayDataTypeUseCase = GetArrayDataTypeUseCase()
val useCase = GetTypeNameUseCase(getArrayDataTypeUseCase)
private val getArrayDataTypeUseCase = GetArrayDataTypeUseCase()
private val useCase = GetTypeNameUseCase(getArrayDataTypeUseCase)

Copilot uses AI. Check for mistakes.
Comment on lines +4 to +6
import generators.obj.syntaxParseTree.FileDataImpl
import org.junit.jupiter.api.Assertions
import org.junit.jupiter.api.Test
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

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

To match the surrounding Kotlin tests in this module, consider using a static import (Assertions.assertEquals -> import org.junit.jupiter.api.Assertions.assertEquals) and calling assertEquals(...) directly for consistency.

Copilot uses AI. Check for mistakes.
@vshcryabets vshcryabets merged commit c37073f into main Apr 7, 2026
2 checks passed
@vshcryabets vshcryabets deleted the fix/kotlin-nullable-string branch April 7, 2026 05:35
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