Create new codegen module and refactor annotation processor to use it #1382

lauzadis · 2024-08-12T15:06:45Z

Issue #

Description of changes

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

github-actions · 2024-08-12T15:15:51Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

github-actions · 2024-08-12T16:07:01Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/core/CodeGenerator.kt

...rocessor/jvm/src/aws/sdk/kotlin/hll/dynamodbmapper/processor/rendering/AnnotationRenderer.kt

github-actions · 2024-08-12T16:17:46Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

github-actions · 2024-08-12T16:32:09Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

ianbotsf

Looks like a great start! In general I like the stuff you pulled out into a common module but I think we should be more zealous about using #T and type references rather than #L and names that we assume are in scope.

settings.gradle.kts

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/core/CodeGenerator.kt

hll/codegen/build.gradle.kts

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/core/ImportDirectives.kt

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/model/Type.kt

...rocessor/jvm/src/aws/sdk/kotlin/hll/dynamodbmapper/processor/rendering/AnnotationRenderer.kt

...-annotation-processor/jvm/src/aws/sdk/kotlin/hll/dynamodbmapper/processor/MapperProcessor.kt

...processor/jvm/src/aws/sdk/kotlin/hll/dynamodbmapper/processor/rendering/HighLevelRenderer.kt

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/model/Type.kt

0marperez · 2024-08-13T03:57:30Z

...-annotation-processor/jvm/src/aws/sdk/kotlin/hll/dynamodbmapper/processor/MapperProcessor.kt

        val annotated = resolver.getSymbolsWithAnnotation(annotationName)
        val invalid = annotated.filterNot { it.validate() }.toList()
-        env.logger.info("Found invalid classes $invalid")
+        logger.info("Found invalid classes $invalid")


question: will we always have invalid classes?

I'm not sure, this validation is something done by KSP

0marperez · 2024-08-13T03:57:57Z

...-annotation-processor/jvm/src/aws/sdk/kotlin/hll/dynamodbmapper/processor/MapperProcessor.kt

-                val ddbName = ksProperty.getAnnotationsByType(DynamoDbAttribute::class).singleOrNull()?.name ?: name
-                Property(name, ddbName, typeName, isPk)
-            }
+        return invalid


question: I think I'm missing some context here, why are we returning invalid?

The behavior of returning unprocessed symbols is defined by KSP

https://github.com/google/ksp/blob/main/api/src/main/kotlin/com/google/devtools/ksp/processing/SymbolProcessor.kt#L32-L37

github-actions · 2024-08-14T17:02:13Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

ianbotsf

Nice improvements! No blocking concerns left for me so leaving an approval conditional on addressing the remaining nits and questions. Style and suggestion comments optional.

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/core/TemplateProcessor.kt

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/model/Types.kt

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/rendering/BuilderRenderer.kt

ianbotsf · 2024-08-15T20:51:32Z

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/rendering/BuilderRenderer.kt

+import com.google.devtools.ksp.symbol.*
+
+/**
+ * A DSL-style builder renderer.
+ * @param renderer The base renderer in which the builder will be written
+ * @param classDeclaration The [KSClassDeclaration] for which a builder will be generated
+ */
+class BuilderRenderer(
+    private val renderer: RendererBase,
+    private val classDeclaration: KSClassDeclaration,
+) {


Comment: A lot of KSP types have bled into this renderer. I can't definitively say that's wrong but I wonder if coupling too tightly in deep places will be an issue at some point. 🤔

I agree, let me try to stop the bleeding by just passing a TypeRef

I think I've got a way cleaner signature now:

/** * A DSL-style builder renderer. * @param renderer The base renderer in which the builder will be written * @param classType The [TypeRef] representing the class for which a builder will be generated * @param members The [Set] of members of [classType] which will be included in the builder */ class BuilderRenderer( private val renderer: RendererBase, private val classType: TypeRef, private val members: Set<Member> ) { ... }

hll/codegen/src/main/kotlin/aws/sdk/kotlin/hll/codegen/rendering/RendererBase.kt

...ain/kotlin/aws/sdk/kotlin/hll/dynamodbmapper/codegen/annotations/rendering/SchemaRenderer.kt

...degen/src/main/kotlin/aws/sdk/kotlin/hll/dynamodbmapper/codegen/model/DynamoDbMapperTypes.kt

ianbotsf · 2024-08-15T21:53:14Z

.../main/resources/META-INF/services/com.google.devtools.ksp.processing.SymbolProcessorProvider

+aws.sdk.kotlin.hll.dynamodbmapper.codegen.operations.HighLevelOpsProcessorProvider
+aws.sdk.kotlin.hll.dynamodbmapper.codegen.annotations.AnnotationsProcessorProvider
+


Comment: It is weird that our cross-mapper codegen module has multiple KSP processors and that we have to disable them by excludeProcessor. It'd be much simpler if we could includeProcessor or specify which we want by KSP arguments in Gradle somehow. 😖 I don't have a better suggestion, just saying it's weird.

We spoke offline and both agreed it's weird, we're going to live with it and see if a solution becomes more necessary / clear.

Some possible solutions include using ksp DSL's arg to pass a mode (enabled / disabled) to the KSP processors. Another option is to replace excludeProcessor with a custom DSL function like singleProcessor(...) or includeProcessor(...)

hll/dynamodb-mapper/dynamodb-mapper/build.gradle.kts

github-actions · 2024-08-16T15:32:58Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

sonarqubecloud · 2024-08-16T16:32:34Z

Quality Gate passed

Issues
13 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions · 2024-08-16T16:36:05Z

A new generated diff is ready to view.

No codegen difference in the AWS SDK

…lin (#1451) * initial poc commit of DynamoDB Mapper (#1232) * add support for Mapper initialization (#1237) * implement mapper pipeline (#1266) * initial implementation of codegen for low-level operations/types (#1357) * initial implementation of secondary index support (#1375) * Create new codegen module and refactor annotation processor to use it (#1382) * feat: add Schema generator Gradle plugin (#1385) * Fix plugin test package * add attribute converters for "standard" values (#1381) * fix: schema generator plugin test module (#1394) * feat: annotation processor codegen configuration (#1392) * feat: add `@DynamoDbIgnore` annotation (#1402) * DDB Mapper filter expressions (runtime components) (#1401) * feat: basic annotation processing (#1399) * add DSL overloads, paginators, and better builder integration for DDB Mapper ops codegen (#1409) * chore: split dynamodb-mapper-codegen into two modules (#1414) * emit DDB_MAPPER business metric (#1426) * feat: setup DynamoDbMapper publication (#1419) * DDB Mapper filter expressions (codegen components) (#1424) * correct docs * mark every HLL/DDBM API experimental (#1428) * fix accidental inclusion of expression attribute members in high-level DynamoDB Mapper requests (#1432) * Upgrade to latest build plugin version * fix: various issues found during testing (#1450) * chore: update Athena changelog notes for 1.3.57 (2024-10-18) release (#1449) * feat: update AWS API models * feat: update AWS service endpoints metadata * chore: release 1.3.60 * chore: bump snapshot version to 1.3.61-SNAPSHOT * feat: initial release of Developer Preview of DynamoDB Mapper for Kotlin * Fix Kotlin gradle-plugin version * fix: ddb mapper tests (#1453) * Bump build plugin version --------- Co-authored-by: Matas <lauzmata@amazon.com> Co-authored-by: aws-sdk-kotlin-ci <aws-kotlin-sdk-automation@amazon.com>

lauzadis added 9 commits August 9, 2024 09:47

Create codegen module

a51de4b

Create codegen module

0a779a1

Add tests and .api

011f367

Bootstrap

75e208a

tests

d8c65c9

Fix package name in StringsTest

5edef54

Latest

df80d66

Rendering correctly now

3d5818e

Small style changes

73514a5

lauzadis added 6 commits August 12, 2024 11:23

ktlint

ba04d3e

ktlint

dbd737e

ktlint

1da4704

Remove unused types

48455a9

StringMap -> stringMap

77f0792

Remove old-form codegen

725a391

lauzadis added 2 commits August 12, 2024 12:09

cleanup

77c3264

KDocs

a0134e0

lauzadis commented Aug 12, 2024

View reviewed changes

lauzadis marked this pull request as ready for review August 12, 2024 16:12

lauzadis requested a review from a team as a code owner August 12, 2024 16:12

apiDump

165e59b

ianbotsf requested changes Aug 12, 2024

View reviewed changes

lauzadis added 2 commits August 12, 2024 17:06

Add extra metadata

3513113

Remove unused function

e995954

0marperez reviewed Aug 13, 2024

View reviewed changes

Pass around a codeGeneratorName

a1cfc8e

lauzadis added 3 commits August 14, 2024 12:48

Don't explicitly import types from the kotlin namespace

5a39f82

Remove explicit import on user class

e63b9e1

ktlint

b68fa5e

ianbotsf approved these changes Aug 15, 2024

View reviewed changes

lauzadis added 12 commits August 16, 2024 09:51

Use kotlin function

5d698ca

Add two new Type.from methods

5c2915e

Use #T in builder docs

98b59fb

classType = Type.from(classDeclaration)

c971520

apiDump

881a7a8

Move rendererName to renderCtx

f4f399a

Add single-line docs(...) function and fix API validation settings

9c63c47

Further nesting of types and rename to MapperTypes

d3e3aeb

Remove unused types

1da7f9a

exclude annotation processor in ops-codegen

345fd8e

rename module codegen to hll-codegen

fbd7ba1

rename module codegen to hll-codegen

930fb53

lauzadis added 4 commits August 16, 2024 12:15

Add type.baseName

3513aa6

Move [Member] to shared codegen module and clean up BuilderRenderer

1ca559c

ktlint

a1b54b6

Remove unnecessary visibility modifiers

a5d6c9d

0marperez approved these changes Aug 16, 2024

View reviewed changes

ianbotsf approved these changes Aug 19, 2024

View reviewed changes

lauzadis merged commit 592be7b into feat-ddb-mapper Aug 19, 2024

lauzadis deleted the feat-ddb-codegen branch August 19, 2024 20:15

lauzadis mentioned this pull request Aug 27, 2024

feat: annotation processor codegen configuration #1392

Merged

		aws.sdk.kotlin.hll.dynamodbmapper.codegen.operations.HighLevelOpsProcessorProvider
		aws.sdk.kotlin.hll.dynamodbmapper.codegen.annotations.AnnotationsProcessorProvider

Create new codegen module and refactor annotation processor to use it #1382

Create new codegen module and refactor annotation processor to use it #1382

Uh oh!

Conversation

lauzadis commented Aug 12, 2024

Issue #

Description of changes

Uh oh!

github-actions bot commented Aug 12, 2024

Uh oh!

github-actions bot commented Aug 12, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 12, 2024

Uh oh!

github-actions bot commented Aug 12, 2024

Uh oh!

ianbotsf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 14, 2024

Uh oh!

ianbotsf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 16, 2024

Uh oh!

sonarqubecloud bot commented Aug 16, 2024

Quality Gate passed

Uh oh!

github-actions bot commented Aug 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants