Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kotlin codegen generates invalid code when a parameterized mapped superclass is used #891

Closed
1 task done
jpmsilva opened this issue Feb 11, 2025 · 7 comments
Closed
1 task done
Labels

Comments

@jpmsilva
Copy link
Contributor

Important Notice

Thank you for opening an issue! Please note that, as outlined in the README, I currently only work on feature requests or bug fixes when sponsored. Balancing this project with professional and personal priorities means I have a very limited amount of effort I can divert to this project.

You must put in the work to address this issue, or it won't be addressed.

  • I am willing to put in the work and submit a PR to resolve this issue.

Describe the bug

When using a parameterized mapped superclass the generated Q-classes are invalid with respect to the generic parameters.

To Reproduce

A test example can be seen at https://github.com/jpmsilva/openfeign-querydsl/tree/kotlin-codegen-generic-supertype (branched from the 6.10.1 release).

Running clean package -P quickbuild from the folder querydsl-examples/querydsl-example-kotlin-codegen shows the issue:

[INFO] --- kotlin:2.1.0:compile (compile) @ querydsl-example-kotlin-codegen ---
[INFO] Applied plugin: 'jpa'
[WARNING] Duplicate source root: querydsl/querydsl-examples/querydsl-example-kotlin-codegen/target/generated-sources/kapt/compile
[WARNING] Duplicate source root: querydsl/querydsl-examples/querydsl-example-kotlin-codegen/target/generated-sources/kaptKotlin/compile
[ERROR] querydsl/querydsl-examples/querydsl-example-kotlin-codegen/target/generated-sources/kaptKotlin/compile/com/querydsl/examples/kotlin/entity/QExampleBaseEntity.kt: (23, 42) None of the following candidates is applicable:
constructor<T : Any!>(p0: Class<out T!>!, p1: String!): EntityPathBase<T>
constructor<T : Any!>(p0: Class<out T!>!, p1: PathMetadata!): EntityPathBase<T>
[ERROR] querydsl/querydsl-examples/querydsl-example-kotlin-codegen/target/generated-sources/kaptKotlin/compile/com/querydsl/examples/kotlin/entity/QExampleBaseEntity.kt: (27, 48) None of the following candidates is applicable:
constructor<T : Any!>(p0: Class<out T!>!, p1: String!): EntityPathBase<T>
constructor<T : Any!>(p0: Class<out T!>!, p1: PathMetadata!): EntityPathBase<T>
[ERROR] querydsl/querydsl-examples/querydsl-example-kotlin-codegen/target/generated-sources/kaptKotlin/compile/com/querydsl/examples/kotlin/entity/QExampleEntity.kt: (27, 5) None of the following candidates is applicable:
constructor(variable: String): QExampleBaseEntity
constructor(path: Path<out ExampleBaseEntity<Serializable>>): QExampleBaseEntity
constructor(metadata: PathMetadata): QExampleBaseEntity

Expected behavior
Build should complete without errors.

Screenshots
N/A

Desktop (please complete the following information):
N/A

Smartphone (please complete the following information):
N/A

Additional context
I might be able to submit a PR to properly set the out keyword in the superclass parameters of the generated Q-classes. This aligns the generated Kotlin Q-classes with the Java Q-classes with regards to entity parameters, when they are used.

When using the Java codegen variant these generic mapped superclass types are properly handled. The issue only happens in the Kotlin codegen module.

@jpmsilva
Copy link
Contributor Author

Please note that I also tested the same scenario using the KSP codegen, but it results in a different error. Unfortunately I am not able to properly diagnose that issue, as KSP is not something I know how to work with:

e: [ksp] java.lang.IllegalStateException: Error processing com.querydsl.example.ksp.BaseEntity.id: Type was not recognised, This may be an entity that has not been annotated with @Entity, or maybe you are using javax instead of jakarta.

Do let me know if you'd like a separate issue filed for the KSP codegen issue, but please take note that I won't be able to assist in that case.

jpmsilva added a commit to jpmsilva/openfeign-querydsl that referenced this issue Feb 11, 2025
jpmsilva added a commit to jpmsilva/openfeign-querydsl that referenced this issue Feb 11, 2025
jpmsilva added a commit to jpmsilva/openfeign-querydsl that referenced this issue Feb 11, 2025
@IceBlizz6
Copy link
Collaborator

Looking at the ExampleBaseEntity class you provided.
KSP will try to generate a QExampleBaseEntity with an id field.
It identifies that Serializable is the boundary for the id field so it should use that.
But that type is not covered in the code.

I tried to add a fallback where it will just use SimplePath for any types not yet covered.
Seems to work, it generated this:

class QExampleBaseEntity : EntityPathBase<ExampleBaseEntity<*>> {
    public val id: SimplePath<Serializable> = createSimple("id", java.io.Serializable::class.java)
    
    // constructors ...
    
    companion object {
        @JvmField
        val exampleBaseEntity: QExampleBaseEntity =
                com.querydsl.example.ksp.QExampleBaseEntity("exampleBaseEntity")
    }
}

If this works for you then i can make a pull request for it.
I haven't checked yet how java handles that case, do you know if it uses SimplePath aswell?

@jpmsilva
Copy link
Contributor Author

Switching to the java codegen I get this generated Q-class for the example I provided above:

/**
 * QExampleBaseEntity is a Querydsl query type for ExampleBaseEntity
 */
@SuppressWarnings("this-escape")
@Generated("com.querydsl.codegen.DefaultSupertypeSerializer")
public class QExampleBaseEntity extends EntityPathBase<ExampleBaseEntity<? extends java.io.Serializable>> {

    private static final long serialVersionUID = 668380935L;

    public static final QExampleBaseEntity exampleBaseEntity = new QExampleBaseEntity("exampleBaseEntity");

    public final SimplePath<java.io.Serializable> id = createSimple("id", java.io.Serializable.class);

    @SuppressWarnings({"all", "rawtypes", "unchecked"})
    public QExampleBaseEntity(String variable) {
        super((Class) ExampleBaseEntity.class, forVariable(variable));
    }

    @SuppressWarnings({"all", "rawtypes", "unchecked"})
    public QExampleBaseEntity(Path<? extends ExampleBaseEntity> path) {
        super((Class) path.getType(), path.getMetadata());
    }

    @SuppressWarnings({"all", "rawtypes", "unchecked"})
    public QExampleBaseEntity(PathMetadata metadata) {
        super((Class) ExampleBaseEntity.class, metadata);
    }

}

So I guess it does use SimplePath: public final SimplePath<java.io.Serializable> id = createSimple("id", java.io.Serializable.class). Hope that helps!

I couldn't help but notice that KSP uses a star type parameter EntityPathBase<ExampleBaseEntity<*>>, which is different from how the Java and Kotlin APT processors generate their code EntityPathBase<ExampleBaseEntity<? extends java.io.Serializable>>. Perhaps it may be beneficial to align the strategy on how code is generated, so that API users don't get different results depending on which codegen they end up using?

I myself would prefer to use the KSP plugin, but our project is using Maven as a build tool, which leaves us without options to run KSP plugins 😢

@IceBlizz6
Copy link
Collaborator

That helps, thank you.
Since you posted earlier about trying KSP i was under the impression that i could help you start using KSP if we fixed the issue.

jpmsilva added a commit to jpmsilva/openfeign-querydsl that referenced this issue Feb 17, 2025
jpmsilva added a commit to jpmsilva/openfeign-querydsl that referenced this issue Feb 17, 2025
@jpmsilva
Copy link
Contributor Author

Hi @IceBlizz6
Thank you for the KSP module!
I would absolutely like to go with the KSP route, since KAPT appears to be somewhat deprecated (even though a 2.0 release now appears to be in the works).
It is sad that a supported maven plugin does not exist, though.
Migrating the whole project to Gradle is not an option for us, so for the time being, we must keep using KAPT.

IceBlizz6 added a commit that referenced this issue Feb 18, 2025
Refer to issue #891 for the
complete report.

This PR handles the issue by adding out keywords to the generated
Q-classes when they are needed.
Copy link
Contributor

This issue has been automatically marked as stale due to inactivity. If no further action is taken, it will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 20, 2025
Copy link
Contributor

This issue has been automatically closed due to prolonged inactivity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants