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

fix: EXPOSED-280 Comparison operators show incorrect compiler warning with datetime columns #1984

Merged
merged 1 commit into from
Feb 5, 2024

Conversation

bog-walk
Copy link
Member

@bog-walk bog-walk commented Feb 2, 2024

I came across this warning while scanning the datetime modules for PR #1983 .

A strange compiler warning is flagged for the following code in exposed-jodatime and exposed-java-time modules:

object TestTable : Table() {
    val created = date("created")
    val deleted = date("deleted")
}
TestTable
    .selectAll()
    .where { TestTable.created eq TestTable.deleted }

Type argument for a type parameter E can't be inferred because it has incompatible upper bounds: LocalDate, EntityID (multiple incompatible classes). This will become an error in Kotlin 2.0

The same warning doesn't appear in the exposed-kotlin-datetime module.
But a user has already reported that they are seeing the warning with regular (non-entity id) UUID columns, suggesting a pattern only involving objects that aren't part of the Kotlin standard library.

This occurs because this convenience overload (introduced by PR #1961 ) is being incorrectly chosen:

infix fun <T : Comparable<T>, V : T?, E : EntityID<T>?> Expression<V>.eq(
    other: ExpressionWithColumnType<in E>
): Op<Boolean> = other eq this

These overloads incorrectly make the ExpressionWithColumnType argument's type parameter contravariant, when this annotation should instead be set on the calling Expression object's type parameter.

Note: No tests have been added but there are multiple examples of the highlighted warning in JodaTimeTests/testlocalDateComparison and testLocalDateTimeComparison. Switching between branches confirms the warning is resolved by this fix.

… with datetime columns

An incorrect compiler warning is flagged if a comparison operator, like eq(),
is used with 2 columns that store certain types. The warning treats the right-side
column as an EntityID column. The pattern so far seems to be stored types that are
not part of the Kotlin standard library, namely javatime, jodatime, and UUID types.

This occurs because of recently introduced convenience overrides that are being
inappropriately chosen. These overrides incorrectly make the ExpressionWithColumnType
argument's type parameter contravariant, when this annotation should instead be
set on the calling Expression object's type parameter.
@bog-walk bog-walk requested review from e5l and joc-a February 2, 2024 02:03
Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

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

lgtm

@bog-walk bog-walk merged commit 6732924 into main Feb 5, 2024
5 checks passed
@bog-walk bog-walk deleted the bog-walk/fix-eq-datetime-warning branch February 5, 2024 18:12
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