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

Adds support for Sprout-generated equality/hashCode #1182

Merged
merged 2 commits into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,13 @@ Thank you to all who have contributed!
- Adds support for Timestamp constructor call in Parser.
- Parsing of label patterns within node and edge graph patterns now supports
disjunction `|`, conjunction `&`, negation `!`, and grouping.
- Adds default `equals` and `hashCode` methods for each generated abstract class of Sprout. This affects the generated
classes in `:partiql-ast` and `:partiql-plan`.

### Changed
- **Breaking**: all product types defined by the internal Sprout tool no longer generate interfaces. They are now abstract
classes due to the generation of `equals` and `hashCode` methods. This change impacts many generated interfaces exposed
in `:partiql-ast` and `:partiql-plan`.
- Standardizes `org/partiql/cli/functions/QueryDDB` and other built-in functions in `org/partiql/lang/eval/builtins` by the new `ExprFunction` format.
- **Breaking**: Redefines `org/partiql/lang/eval/ExprFunctionkt.call()` method by only invoking `callWithRequired` function.
- **Breaking**: Redefines `org/partiql/lang/eval/builtins/DynamicLookupExprFunction` by merging `variadicParameter` into `requiredParameters` as a `StaticType.LIST`. `callWithVariadic` is now replaced by `callWithRequired`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package org.partiql.sprout.generator.target.kotlin

import com.squareup.kotlinpoet.AnnotationSpec
import com.squareup.kotlinpoet.ClassName
import com.squareup.kotlinpoet.CodeBlock
import com.squareup.kotlinpoet.FunSpec
import com.squareup.kotlinpoet.KModifier
import com.squareup.kotlinpoet.ParameterSpec
import com.squareup.kotlinpoet.PropertySpec
import com.squareup.kotlinpoet.TypeSpec
import com.squareup.kotlinpoet.asTypeName
import net.pearx.kasechange.toCamelCase
import org.partiql.sprout.generator.Generator
import org.partiql.sprout.generator.target.kotlin.poems.KotlinBuilderPoem
Expand Down Expand Up @@ -130,7 +132,7 @@ class KotlinGenerator(private val options: KotlinOptions) : Generator<KotlinResu
constructor.addParameter(para)
}
// impls are open
impl.addSuperinterface(clazz)
impl.superclass(clazz)
nodes.forEach { it.builder.addSuperinterface(symbols.base) }
this.addDataClassMethods()
}
Expand Down Expand Up @@ -170,14 +172,8 @@ class KotlinGenerator(private val options: KotlinOptions) : Generator<KotlinResu
// TODO generate hashCode, equals, componentN so we can have OPEN internal implementations
private fun KotlinNodeSpec.Product.addDataClassMethods() {
impl.addModifiers(KModifier.INTERNAL, KModifier.OPEN)
// builder.addFunction(
// FunSpec.builder("hashCode").addModifiers(KModifier.OVERRIDE, KModifier.ABSTRACT).returns(Int::class).build()
// )
// builder.addFunction(
// FunSpec.builder("equals").addModifiers(KModifier.OVERRIDE, KModifier.ABSTRACT).returns(Boolean::class)
// .addParameter(ParameterSpec.builder("other", Any::class.asTypeName().copy(nullable = true)).build())
// .build()
// )
addEqualsMethod()
addHashCodeMethod()
val args = listOf("_id") + props.map { it.name }
val copy = FunSpec.builder("copy").addModifiers(KModifier.ABSTRACT).returns(clazz)
val copyImpl = FunSpec.builder("copy")
Expand All @@ -192,4 +188,51 @@ class KotlinGenerator(private val options: KotlinOptions) : Generator<KotlinResu
builder.addFunction(copy.build())
impl.addFunction(copyImpl.build())
}

/**
* Adds `equals` method to the core abstract class
*/
private fun KotlinNodeSpec.Product.addEqualsMethod() {
val equalsFunctionBodyBuilder = CodeBlock.builder().let { body ->
body.addStatement("if (this === other) return true")
body.addStatement("if (other !is %T) return false", this.clazz)
this.props.forEach { prop ->
body.addStatement("if (%N != other.%N) return false", prop.name, prop.name)
}
Comment on lines +199 to +201
Copy link
Member

Choose a reason for hiding this comment

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

I think we should deeply compare collections as well

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be deeply comparing as-is. I just pushed tests to show some examples. Thanks!

body.addStatement("return true")
}
builder.addFunction(
FunSpec.builder("equals").addModifiers(KModifier.OVERRIDE).returns(Boolean::class)
.addParameter(ParameterSpec.builder("other", Any::class.asTypeName().copy(nullable = true)).build())
.addCode(equalsFunctionBodyBuilder.build())
.build()
)
}

/**
* Adds `hashCode` method to the core abstract class
*/
private fun KotlinNodeSpec.Product.addHashCodeMethod() {
val hashcodeBodyBuilder = CodeBlock.builder().let { body ->
when (this.props.size) {
0 -> body.addStatement("return 0")
1 -> body.addStatement("return %N.hashCode()", this.props.first().name)
else -> {
body.addStatement("var result = %N.hashCode()", this.props.first().name)
this.props.subList(1, this.props.size).forEach { prop ->
body.addStatement("result = 31 * result + %N.hashCode()", prop.name)
}
body.addStatement("return result")
}
}
body
}
builder.addFunction(
FunSpec.builder("hashCode")
.addModifiers(KModifier.OVERRIDE)
.returns(Int::class)
.addCode(hashcodeBodyBuilder.build())
.build()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class KotlinBuilderPoem(symbols: KotlinSymbols) : KotlinPoem(symbols) {
private val baseFactoryClass = ClassName(builderPackageName, baseFactoryName)
private val baseFactory = TypeSpec.classBuilder(baseFactoryClass)
.addSuperinterface(factoryClass)
.addModifiers(KModifier.ABSTRACT)
.addModifiers(KModifier.OPEN)
.addProperty(
idProvider.toBuilder()
.addModifiers(KModifier.OVERRIDE)
Expand Down Expand Up @@ -289,7 +289,7 @@ class KotlinBuilderPoem(symbols: KotlinSymbols) : KotlinPoem(symbols) {
private fun factoryCompanion() = TypeSpec.companionObjectBuilder()
.addProperty(
PropertySpec.builder("DEFAULT", factoryClass)
.initializer("%T", factoryDefault)
.initializer("%T()", baseFactoryClass)
.build()
)
.addFunction(factoryFunc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ sealed class KotlinNodeSpec(
) : KotlinNodeSpec(
def = product,
clazz = clazz,
builder = TypeSpec.interfaceBuilder(clazz),
builder = TypeSpec.classBuilder(clazz).addModifiers(KModifier.ABSTRACT),
companion = TypeSpec.companionObjectBuilder(),
ext = ext,
) {
Expand Down
1 change: 1 addition & 0 deletions settings.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,6 @@ include(
"lib:sprout",
"test:partiql-tests-runner",
"test:partiql-randomized-tests",
"test:sprout-tests",
"examples",
)
42 changes: 42 additions & 0 deletions test/sprout-tests/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

plugins {
id(Plugins.conventions)
}

dependencies {
api(Deps.ionElement)
api(project(":partiql-types"))
}

val generate = tasks.register<Exec>("generate") {
dependsOn(":lib:sprout:install")
workingDir(projectDir)
commandLine(
"../../lib/sprout/build/install/sprout/bin/sprout", "generate", "kotlin",
"-o", "$buildDir/generated-src",
"-p", "org.partiql.sprout.tests.example",
"-u", "Example",
"--poems", "visitor",
"--poems", "builder",
"--poems", "util",
"./src/main/resources/example.ion"
)
}

tasks.compileKotlin {
dependsOn(generate)
}
60 changes: 60 additions & 0 deletions test/sprout-tests/src/main/resources/example.ion
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
imports::{
kotlin: [
ion::'com.amazon.ionelement.api.IonElement'
],
}

statement::[

// PartiQL Expressions
query::{
expr: expr,
},
]

expr::[

// Ion Literal Value, ie `<ion>`
ion::{
value: '.ion',
},

// Variable Reference
var::{
identifier: identifier,
scope: [
DEFAULT,
// x.y.z
LOCAL,
// @x.y.z
],
},
empty::{}
]
// Identifiers and Qualified Identifiers
//----------------------------------------------
// <identifier > ::= <id symbol> | <id path>
//
// <id symbol> ::= <symbol> // case-insensitive
// | "<symbol>" // case-sensitive
//
// <id qualified> ::= <id symbol> ('.' <id symbol>)+;
//
identifier::[
symbol::{
symbol: string,
case_sensitivity: case_sensitivity,
},
qualified::{
root: symbol,
steps: list::[
symbol
],
},
_::[
case_sensitivity::[
SENSITIVE,
INSENSITIVE,
],
],
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at:
*
* http://aws.amazon.com/apache2.0/
*
* or in the "license" file accompanying this file. This file is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific
* language governing permissions and limitations under the License.
*/

package org.partiql.sprout.tests

import org.junit.jupiter.api.extension.ExtensionContext
import org.junit.jupiter.params.provider.Arguments
import org.junit.jupiter.params.provider.ArgumentsProvider
import java.util.stream.Stream

/**
* Reduces some of the boilerplate associated with the style of parameterized testing frequently
* utilized in this package.
*
* Since JUnit5 requires `@JvmStatic` on its `@MethodSource` argument factory methods, this requires all
* of the argument lists to reside in the companion object of a test class. This can be annoying since it
* forces the test to be separated from its tests cases.
*
* Classes that derive from this class can be defined near the `@ParameterizedTest` functions instead.
*/
abstract class ArgumentsProviderBase : ArgumentsProvider {

abstract fun getParameters(): List<Any>

@Throws(Exception::class)
override fun provideArguments(extensionContext: ExtensionContext): Stream<out Arguments>? {
return getParameters().map { Arguments.of(it) }.stream()
}
}
Loading
Loading