When using a property initializer in a primary constructor, the annotations don't have the correct use-site target

[fejesjoco]

Sample code

fun main() {
  val prop1 =
    PropertySpec.builder("prop1", String::class)
      //.initializer("prop1")
      .addModifiers(KModifier.PRIVATE)
      .addAnnotation(AnnotationSpec.builder(Suppress::class).addMember("%S", "ann1_prop").build())
      .build()
  val param1 = ParameterSpec.builder("prop1", String::class).build()
  val prop2 =
    PropertySpec.builder("prop2", String::class)
      //.initializer("prop2")
      .addModifiers(KModifier.PRIVATE)
      .build()
  val param2 =
    ParameterSpec.builder("prop2", String::class)
      .addAnnotation(AnnotationSpec.builder(Suppress::class).addMember("%S", "ann2_param").build())
      .build()

  FileSpec.builder("com.test", "HelloWorld")
    .addType(
      TypeSpec.classBuilder("HelloWorld")
        .addProperty(prop1)
        .addProperty(prop2)
        .primaryConstructor(
          FunSpec.constructorBuilder().addParameter(param1).addParameter(param2).build()
        )
        .build()
    )
    .build()
    .writeTo(System.out)

Expected behavior

If you run the above, the following is printed, which correctly shows that prop1 is annotated as a field but not as a constructor parameter, while prop2 is the opposite:

package com.test

import kotlin.String
import kotlin.Suppress

public class HelloWorld(
  prop1: String,
  @Suppress("ann2_param")
  prop2: String,
) {
  @Suppress("ann1_prop")
  private val prop1: String

  private val prop2: String
}

Unexpected behavior

If you uncomment the two lines in the original code to get the collapsed parameter+property syntax, both prop1 and prop2 have the same annotation, so as part of the collapsing process, the use-site target is lost:

package com.test

import kotlin.String
import kotlin.Suppress

public class HelloWorld(
  @Suppress("ann1_prop")
  private val prop1: String,
  @Suppress("ann2_param")
  private val prop2: String,
)

Additional info

To match the built type fully, we would expect prop1 to have @get:Suppress and prop2 to have @param:Suppress. Not having a use-site target is not too wrong, it just includes more scopes than necessary, while fixing this issue would remove some scopes. It raises some interesting questions though, like what if the user specifies a use-site target on the AnnotationSpec itself, who should win, what if they specify an incorrect use-site target (e.g. annotate the ParameterSpec as UseSiteTarget.FIELD which is wrong), what if someone likes the current behavior and only want to annotate either the PropertySpec or the ParameterSpec but not both, etc. I think most of these problems stem from the fact that the user is expected to build two elements which are generated as one, so of course some coercion will have to take place. If I could build a primary constructor with only PropertySpec's but not ParameterSpec's, it would be easier to figure out what the user really wanted, and the builder code would be less repetitive as well.

[Egorand]

Unexpected behavior

If you uncomment the two lines in the original code to get the collapsed parameter+property syntax, both prop1 and prop2 have the same annotation, so as part of the collapsing process, the use-site target is lost

It doesn't look though that the use-site target got lost in these examples, as it has never been specified explicitly? I don't think that's a correct expectation.

To match the built type fully, we would expect prop1 to have @get:Suppress and prop2 to have @param:Suppress.

Not unless these use-site targets were set explicitly, the default is no use-site targets.

And to answer the questions:

what if the user specifies a use-site target on the AnnotationSpec itself, who should win

I'd say the user should be responsible for making sure there are no clashes, the library simply merges the annotations from the property and the parameter.

what if they specify an incorrect use-site target

That's user error, I don't think the library should be correcting that error, as the library doesn't know what the user's real intent was. Also, please note that it's perfectly possible to generate incorrect, uncompilable code with KotlinPoet. KotlinPoet very deliberately doesn't act as a Kotlin compiler (even though it packages some basic correctness checks), this is not its purpose.

If I could build a primary constructor with only PropertySpec's but not ParameterSpec's, it would be easier to figure out what the user really wanted, and the builder code would be less repetitive as well.

But this is not how Kotlin models these concepts. Primary constructor syntax allows you to simultaneously declare a property and a parameter with the same name, but they're still two different things, which KotlinPoet reflects.

All in all, it's not really clear what the issue here is. Is the ask to have the library auto-generate use-site targets based on which specs the annotations are attached to? Or is the ask to have more complex rules around merging annotations from properties and parameters?

[Egorand]

I'm gonna close this issue as it's been inactive for over two weeks and it's not clear what the request is, but please feel free to reopen and clarify your expectations. Thank you!

[fejesjoco]

I missed the notification.

All I'm saying is: if I annotate a property, it has a default use-site target of @get, and if I annotate a parameter, it has a default use-site target of @param.

You wrote:

Primary constructor syntax allows you to simultaneously declare a property and a parameter with the same name, but they're still two different things, which KotlinPoet reflects.

So I guess we're on the same page so far?

Then, when I build a constructor where the property and parameter are merged, and KotlinPoet omits the use-site target, then the annotation which was originally meant for the parameter, it also gets copied to the property, or vice versa.

What KotlinPoet does: if there was no use-site target on the originally built elements, then there's no use-site target on the final code output either. That is reasonable as well. The issue is that the lack of a use-site target has different meanings, because different contexts have different defaults. From that point of view, omitting the use-site target means adding a use-site target to an element where it was not specified.

[Egorand]

Got it, I see what you're saying, thanks for clarifying! I think it would make sense to generate use-site targets to preserve behaviour.