Skip to content

Commit e828112

Browse files
authored
fix: Fix non local return in getDescriptionFromAnnotation (#269)
1 parent befcafa commit e828112

File tree

10 files changed

+240
-23
lines changed

10 files changed

+240
-23
lines changed

Makefile

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ scan:
2424
@./gradlew clean kotlinWasmUpgradePackageLock kotlinUpgradePackageLock build --scan --rerun-tasks
2525
@echo "✅ Build with scan is complete!"
2626

27+
.PHONY: apidump
28+
apidump:
29+
@echo "🪏 API dump..."
30+
@./gradlew updateLegacyAbi
31+
2732
.PHONY: apidocs
2833
apidocs:
2934
@echo "📚 Generating API documentation..."

README.md

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ Quick Links:
8989

9090
## Key Features
9191

92-
**Dual Generation Modes:**
92+
**Generation Modes:**
9393

9494
- **Compile-time (KSP)**: Zero runtime overhead, multiplatform, for your annotated classes
9595
- **Runtime (Reflection)**: JVM-only, for any class including third-party libraries
@@ -104,22 +104,22 @@ Quick Links:
104104
**Flexible Annotation Support:**
105105

106106
- Recognizes `@Description`, `@LLMDescription`, `@JsonPropertyDescription`, `@P`, and more
107-
- Recognizes KDoc
107+
- Recognizes KDoc (KSP compile-time only)
108108
- Works with annotations from Jackson, LangChain4j, Koog without code changes
109109

110110
**Comprehensive Type Support:**
111111

112112
- **Enums, collections, maps, nested objects, nullability, generics** (with star-projection)
113113
- **Sealed class hierarchies** with automatic `oneOf` generation and discriminator field
114114
- **Union types** for nullable parameters (`["string", "null"]`)
115-
- **Type constraints** (min/max, patterns, formats)
115+
- **Type constraints** (min/max, patterns, formats) via the JSON Schema DSL
116116
- **Default values** (compile-time: tracked but not extracted; runtime: fully extracted)
117117
- **`$ref`/`$defs` deduplication**: named types appear once in `$defs` and are referenced everywhere via `$ref`
118118
- **`kotlin.Any`**: maps to the empty schema `{}` (accepts any JSON value)
119119

120120
**Developer Experience:**
121121

122-
- Gradle plugin for one-line setup
122+
- Gradle plugin for one-line setup (experimental)
123123
- Type-safe Kotlin DSL for programmatic schema construction
124124
- Works everywhere: JVM, JS, iOS, macOS, Wasm
125125

@@ -157,7 +157,7 @@ This library solves three key challenges:
157157
| **Class must be `@Serializable`** | No | Yes | No |
158158
| **Annotate class with `@Schema`** | Required | Not required | Not required |
159159
| **KDoc extracted to description** ||||
160-
| **Extract data class defaults** ||||
160+
| **Extract default values** ||||
161161
| **Third-party classes** || ✅ (only `@Serializable`) | ✅ any JVM class |
162162

163163
- **[Pick KSP](docs/ksp.md)** when you own the classes, want zero runtime overhead, and target Multiplatform or need KDoc
@@ -1297,13 +1297,12 @@ data class Product(
12971297

12981298
### Precedence Rules
12991299

1300-
If multiple description annotations are present on the same element, the library uses this precedence order:
1300+
If multiple description annotations are present on the same element, the **first matching annotation in source-code
1301+
order** wins. There is no special priority for `@Description` over other recognized annotations — whichever recognized
1302+
annotation appears first on the declaration is used.
13011303

1302-
1. `@Description` (kotlinx-schema's own annotation)
1303-
2. Other annotations in alphabetical order by simple name
1304-
1305-
**Tip**: For best compatibility, prefer `@Description` from kotlinx-schema when writing new code, but existing
1306-
annotations from other libraries work seamlessly.
1304+
**Tip**: For predictability, place only one description annotation per element. If you use `@Description` from
1305+
kotlinx-schema alongside another recognized annotation, put `@Description` first in source order to ensure it wins.
13071306

13081307
## JSON Schema DSL
13091308

docs/build.gradle.kts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ dependencies {
1414
implementation(project(":kotlinx-schema-annotations"))
1515
implementation(project(":kotlinx-schema-generator-json"))
1616
implementation(libs.kotlinx.serialization.json)
17+
implementation(libs.kotest.assertions.json)
1718

1819
dokka(project(":kotlinx-schema-annotations"))
1920
dokka(project(":kotlinx-schema-generator-core"))

kotlinx-schema-generator-core/src/commonMain/kotlin/kotlinx/schema/generator/core/ir/Introspections.kt

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,16 @@ public object Introspections {
9393
* // description == "User's email address"
9494
* ```
9595
*
96+
* ## Attribute ordering
97+
*
98+
* When multiple parameters of the annotation match [descriptionValueAttributes] (e.g., an
99+
* annotation with both `value` and `description` fields), the **first non-empty string value**
100+
* in the provided `annotationArguments` list is returned. The ordering of
101+
* `annotationArguments` is determined by the caller (typically the order in which
102+
* `annotationClass.members` enumerates properties via Kotlin reflection, which is not
103+
* specified by the Kotlin language and may vary across JVM implementations). In practice this
104+
* is only relevant when both fields are non-empty simultaneously, which is an unusual usage.
105+
*
96106
* @param annotationName The simple name of the annotation to inspect (e.g., "Description", "P")
97107
* @param annotationArguments List of key-value pairs representing the annotation's parameters
98108
* @return The description text if found, or null if the annotation is not recognized or
@@ -106,10 +116,7 @@ public object Introspections {
106116
if (annotationName.lowercase() in descriptionAnnotationNames) {
107117
annotationArguments
108118
.filter { it.first.lowercase() in descriptionValueAttributes }
109-
.firstNotNullOfOrNull {
110-
val value = it.second
111-
return (value as? String)
112-
}
119+
.firstNotNullOfOrNull { (_, value) -> (value as? String)?.takeIf { it.isNotEmpty() } }
113120
} else {
114121
null
115122
}

kotlinx-schema-generator-core/src/jvmMain/kotlin/kotlinx/schema/generator/reflect/ReflectionUtils.kt

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import kotlin.reflect.KProperty
88
import kotlin.reflect.KProperty1
99
import kotlin.reflect.KType
1010
import kotlin.reflect.KVisibility
11+
import kotlin.reflect.full.IllegalCallableAccessException
1112
import kotlin.reflect.full.allSuperclasses
1213
import kotlin.reflect.full.primaryConstructor
1314
import kotlin.reflect.jvm.javaMethod
@@ -16,13 +17,14 @@ import kotlin.reflect.jvm.kotlinFunction
1617
/**
1718
* Gets the [KType.classifier], ensuring it is a [KClass].
1819
*/
19-
internal val KType.klass: KClass<*> get() {
20-
return requireNotNull(classifier as? KClass<*>) {
21-
"Unsupported classifier: $classifier. " +
22-
"Only KClass classifiers are supported. Type parameters and other classifiers " +
23-
"cannot be introspected using reflection."
20+
internal val KType.klass: KClass<*>
21+
get() {
22+
return requireNotNull(classifier as? KClass<*>) {
23+
"Unsupported classifier: $classifier. " +
24+
"Only KClass classifiers are supported. Type parameters and other classifiers " +
25+
"cannot be introspected using reflection."
26+
}
2427
}
25-
}
2628

2729
/**
2830
* Finds a property in a class by name.
@@ -53,12 +55,43 @@ internal fun extractDescription(annotations: List<Annotation>): String? =
5355
annotation.annotationClass.members
5456
.filterIsInstance<KProperty1<Annotation, *>>()
5557
.forEach { property ->
56-
add(property.name to property.get(annotation))
58+
val value =
59+
try {
60+
property.get(annotation)
61+
} catch (_: IllegalCallableAccessException) {
62+
fallbackViaJavaReflection(annotation, property) ?: return@forEach
63+
} catch (_: IllegalAccessException) {
64+
fallbackViaJavaReflection(annotation, property) ?: return@forEach
65+
}
66+
add(property.name to value)
5767
}
5868
}
5969
Introspections.getDescriptionFromAnnotation(annotationName, annotationArguments)
6070
}
6171

72+
/**
73+
* KProperty1.get() wraps java.lang.IllegalAccessException in
74+
* kotlin.reflect.full.IllegalCallableAccessException when the annotation
75+
* interface is non-public (e.g. package-private for Kotlin `internal`
76+
* annotations from external modules). Fall back to Java reflection on
77+
* the proxy class, which is always accessible.
78+
* Returns null when the fallback itself fails; null is safe here because
79+
* Java annotation elements cannot return null values.
80+
*/
81+
private fun fallbackViaJavaReflection(
82+
annotation: Annotation,
83+
property: KProperty1<Annotation, *>,
84+
): Any? =
85+
try {
86+
val javaMethod = annotation.javaClass.getDeclaredMethod(property.name)
87+
javaMethod.isAccessible = true
88+
javaMethod.invoke(annotation)
89+
} catch (_: ReflectiveOperationException) {
90+
null
91+
} catch (_: SecurityException) {
92+
null
93+
}
94+
6295
/**
6396
* Returns a new [TypeRef] with the specified nullable flag.
6497
*/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package kotlinx.schema.generator.reflect.testfixtures;
2+
3+
import java.lang.annotation.ElementType;
4+
import java.lang.annotation.Retention;
5+
import java.lang.annotation.RetentionPolicy;
6+
import java.lang.annotation.Target;
7+
8+
/**
9+
* Test fixtures for the package-private annotation reflection fallback in {@code extractDescription}.
10+
*
11+
* <p>The nested {@code LLMDescription} annotation has no {@code public} modifier — it is
12+
* package-private. This simulates third-party Kotlin {@code internal} annotations (which compile
13+
* to package-private interfaces) that are accessed from outside their package. From a different
14+
* package, {@code KProperty1.get()} may throw {@link IllegalAccessException}; the fallback in
15+
* {@code extractDescription} recovers by calling {@code getDeclaredMethod()} on the proxy class
16+
* with {@code isAccessible = true}.
17+
*/
18+
public final class PackagePrivateAnnotationFixtures {
19+
20+
/**
21+
* Package-private annotation with two elements: {@code value} (shorthand) and
22+
* {@code description} (verbose). Matched by simple name "LLMDescription" via
23+
* {@code Introspections}.
24+
*/
25+
@Target({ElementType.METHOD, ElementType.TYPE})
26+
@Retention(RetentionPolicy.RUNTIME)
27+
@interface LLMDescription {
28+
String value() default "";
29+
30+
String description() default "";
31+
}
32+
33+
/** Holder with a method annotated with the package-private {@code @LLMDescription}. */
34+
public static final class AnnotatedHolder {
35+
@LLMDescription(description = "product search tool")
36+
public static void searchProducts() {}
37+
38+
@LLMDescription("find user by id")
39+
public static void findUser() {}
40+
}
41+
42+
private PackagePrivateAnnotationFixtures() {}
43+
}
Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,20 @@
11
package kotlinx.schema.generator.core.ir
22

33
import io.kotest.matchers.shouldBe
4+
import io.kotest.matchers.shouldNotBe
5+
import org.junit.jupiter.api.Test
46
import org.junit.jupiter.params.ParameterizedTest
57
import org.junit.jupiter.params.provider.CsvSource
68

79
class IntrospectionsTest {
10+
//region Single-element annotation
811
@ParameterizedTest
912
@CsvSource(
1013
"Description, value",
1114
"LLMDescription, value",
1215
"P, description",
1316
)
14-
fun getDescriptionFromAnnotation(
17+
fun `extracts description from single-element annotation`(
1518
name: String,
1619
attribute: String,
1720
) {
@@ -20,4 +23,31 @@ class IntrospectionsTest {
2023
listOf(attribute to "My Description"),
2124
) shouldBe "My Description"
2225
}
26+
//endregion
27+
28+
//region Multi-element LLMDescription regression
29+
@Test
30+
fun `extracts description from LLMDescription with description= style when value is empty`() {
31+
// Regression: @LLMDescription has both value and description fields;
32+
// when used as @LLMDescription(description = "text"), value defaults to ""
33+
// and should not shadow the non-empty description field.
34+
val result =
35+
Introspections.getDescriptionFromAnnotation(
36+
annotationName = "LLMDescription",
37+
annotationArguments = listOf("value" to "", "description" to "Product identifier"),
38+
)
39+
result shouldBe "Product identifier"
40+
}
41+
42+
@Test
43+
fun `extracts description from LLMDescription with value= shorthand style`() {
44+
// @LLMDescription("Product name") sets value="Product name", description=""
45+
val result =
46+
Introspections.getDescriptionFromAnnotation(
47+
annotationName = "LLMDescription",
48+
annotationArguments = listOf("value" to "Product name", "description" to ""),
49+
)
50+
result shouldBe "Product name"
51+
}
52+
//endregion
2353
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package kotlinx.schema.generator.reflect
2+
3+
import io.kotest.matchers.shouldBe
4+
import kotlinx.schema.generator.reflect.testfixtures.PackagePrivateAnnotationFixtures
5+
import kotlin.test.Test
6+
7+
class ExtractDescriptionTest {
8+
//region Package-private annotation fallback
9+
/**
10+
* [PackagePrivateAnnotationFixtures.LLMDescription] is package-private in
11+
* `kotlinx.schema.generator.reflect.testfixtures`. When [extractDescription] (in
12+
* `kotlinx.schema.generator.reflect`) calls `KProperty1.get()` on its elements, the JVM
13+
* denies access across packages for a non-public interface and throws
14+
* [IllegalAccessException]. The fallback recovers via `getDeclaredMethod()` on the proxy
15+
* class with `isAccessible = true`.
16+
*/
17+
@Test
18+
fun `extracts description from package-private annotation description= style via Java reflection fallback`() {
19+
val method =
20+
PackagePrivateAnnotationFixtures.AnnotatedHolder::class.java
21+
.getDeclaredMethod("searchProducts")
22+
23+
extractDescription(method.annotations.toList()) shouldBe "product search tool"
24+
}
25+
26+
@Test
27+
fun `extracts description from package-private annotation value= shorthand style via Java reflection fallback`() {
28+
val method =
29+
PackagePrivateAnnotationFixtures.AnnotatedHolder::class.java
30+
.getDeclaredMethod("findUser")
31+
32+
extractDescription(method.annotations.toList()) shouldBe "find user by id"
33+
}
34+
//endregion
35+
}

kotlinx-schema-generator-json/src/jvmTest/kotlin/kotlinx/schema/generator/json/FunctionCallingSchemaGeneratorTest.kt

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,19 @@ import kotlinx.serialization.json.Json
1111
import kotlin.reflect.KCallable
1212
import kotlin.test.Test
1313

14+
/**
15+
* Local test annotation mirroring the shape of [ai.koog.agents.core.tools.annotations.LLMDescription]:
16+
* two String elements — [value] (shorthand) and [description] (verbose).
17+
*
18+
* Matched by simple name "LLMDescription" via [kotlinx.schema.generator.core.ir.Introspections].
19+
*/
20+
@Target(AnnotationTarget.FUNCTION, AnnotationTarget.VALUE_PARAMETER, AnnotationTarget.CLASS)
21+
@Retention(AnnotationRetention.RUNTIME)
22+
internal annotation class LLMDescription(
23+
val value: String = "",
24+
val description: String = "",
25+
)
26+
1427
class FunctionCallingSchemaGeneratorTest {
1528
private val generator =
1629
ReflectionFunctionCallingSchemaGenerator(
@@ -677,4 +690,52 @@ class FunctionCallingSchemaGeneratorTest {
677690
}
678691
""".trimIndent()
679692
}
693+
694+
//region LLMDescription regression tests
695+
object LLMDescriptionTool {
696+
// Regression: @LLMDescription has two elements (value, description).
697+
// Using the verbose description= style leaves value="" — the extractor must not return ""
698+
// and instead find the non-empty description field.
699+
@LLMDescription(description = "Search for products in the catalog")
700+
fun search(
701+
@LLMDescription(description = "Search query string")
702+
query: String,
703+
@LLMDescription("Maximum number of results")
704+
limit: Int = 10,
705+
): String = ""
706+
}
707+
708+
@Test
709+
fun `extracts descriptions from LLMDescription verbose style on function and parameters`() {
710+
// Regression: @LLMDescription(description = "text") must not produce empty description.
711+
// Previously getDescriptionFromAnnotation returned "" because value="" was the first match.
712+
val schemaString = generator.generateSchemaString(LLMDescriptionTool::search)
713+
714+
schemaString shouldEqualJson
715+
// language=json
716+
"""
717+
{
718+
"type": "function",
719+
"name": "search",
720+
"description": "Search for products in the catalog",
721+
"strict": true,
722+
"parameters": {
723+
"type": "object",
724+
"properties": {
725+
"query": {
726+
"type": "string",
727+
"description": "Search query string"
728+
},
729+
"limit": {
730+
"type": "integer",
731+
"description": "Maximum number of results"
732+
}
733+
},
734+
"required": ["query", "limit"],
735+
"additionalProperties": false
736+
}
737+
}
738+
""".trimIndent()
739+
}
740+
//endregion
680741
}

0 commit comments

Comments
 (0)