From 7f087be3688a63331d4933b83566839b189cbba1 Mon Sep 17 00:00:00 2001 From: John Ed Quinn Date: Fri, 18 Aug 2023 13:49:51 -0700 Subject: [PATCH 1/2] Converts generation of Sprout product types to be abstract classes Adds default and methods to each product Previous generation resulted in interfaces --- CHANGELOG.md | 5 + .../target/kotlin/KotlinGenerator.kt | 61 +++++++-- .../target/kotlin/poems/KotlinBuilderPoem.kt | 4 +- .../target/kotlin/spec/KotlinNodeSpec.kt | 2 +- settings.gradle.kts | 1 + test/sprout-tests/build.gradle.kts | 42 ++++++ .../src/main/resources/example.ion | 60 +++++++++ .../sprout/tests/ArgumentsProviderBase.kt | 40 ++++++ .../sprout/tests/example/EqualityTests.kt | 127 ++++++++++++++++++ 9 files changed, 330 insertions(+), 12 deletions(-) create mode 100644 test/sprout-tests/build.gradle.kts create mode 100644 test/sprout-tests/src/main/resources/example.ion create mode 100644 test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/ArgumentsProviderBase.kt create mode 100644 test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/example/EqualityTests.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 777483efd5..420ff3ef4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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`. diff --git a/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/KotlinGenerator.kt b/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/KotlinGenerator.kt index 13d0868f9f..267887cbc3 100644 --- a/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/KotlinGenerator.kt +++ b/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/KotlinGenerator.kt @@ -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 @@ -130,7 +132,7 @@ class KotlinGenerator(private val options: KotlinOptions) : Generator + 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) + } + 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() + ) + } } diff --git a/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/poems/KotlinBuilderPoem.kt b/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/poems/KotlinBuilderPoem.kt index fd63eb4b18..dd60461c8e 100644 --- a/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/poems/KotlinBuilderPoem.kt +++ b/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/poems/KotlinBuilderPoem.kt @@ -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) @@ -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) diff --git a/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/spec/KotlinNodeSpec.kt b/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/spec/KotlinNodeSpec.kt index 254d589a4b..def9c3055b 100644 --- a/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/spec/KotlinNodeSpec.kt +++ b/lib/sprout/src/main/kotlin/org/partiql/sprout/generator/target/kotlin/spec/KotlinNodeSpec.kt @@ -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, ) { diff --git a/settings.gradle.kts b/settings.gradle.kts index 82eaeb2b54..ebcb52e25c 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -27,5 +27,6 @@ include( "lib:sprout", "test:partiql-tests-runner", "test:partiql-randomized-tests", + "test:sprout-tests", "examples", ) diff --git a/test/sprout-tests/build.gradle.kts b/test/sprout-tests/build.gradle.kts new file mode 100644 index 0000000000..aa748f374a --- /dev/null +++ b/test/sprout-tests/build.gradle.kts @@ -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("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) +} diff --git a/test/sprout-tests/src/main/resources/example.ion b/test/sprout-tests/src/main/resources/example.ion new file mode 100644 index 0000000000..40332581ec --- /dev/null +++ b/test/sprout-tests/src/main/resources/example.ion @@ -0,0 +1,60 @@ +imports::{ + kotlin: [ + ion::'com.amazon.ionelement.api.IonElement' + ], +} + +statement::[ + + // PartiQL Expressions + query::{ + expr: expr, + }, +] + +expr::[ + + // Ion Literal Value, ie `` + ion::{ + value: '.ion', + }, + + // Variable Reference + var::{ + identifier: identifier, + scope: [ + DEFAULT, + // x.y.z + LOCAL, + // @x.y.z + ], + }, + empty::{} +] +// Identifiers and Qualified Identifiers +//---------------------------------------------- +// ::= | +// +// ::= // case-insensitive +// | "" // case-sensitive +// +// ::= ('.' )+; +// +identifier::[ + symbol::{ + symbol: string, + case_sensitivity: case_sensitivity, + }, + qualified::{ + root: symbol, + steps: list::[ + symbol + ], + }, + _::[ + case_sensitivity::[ + SENSITIVE, + INSENSITIVE, + ], + ], +] diff --git a/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/ArgumentsProviderBase.kt b/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/ArgumentsProviderBase.kt new file mode 100644 index 0000000000..e7fdd32fdd --- /dev/null +++ b/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/ArgumentsProviderBase.kt @@ -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 + + @Throws(Exception::class) + override fun provideArguments(extensionContext: ExtensionContext): Stream? { + return getParameters().map { Arguments.of(it) }.stream() + } +} diff --git a/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/example/EqualityTests.kt b/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/example/EqualityTests.kt new file mode 100644 index 0000000000..d3874502e9 --- /dev/null +++ b/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/example/EqualityTests.kt @@ -0,0 +1,127 @@ +/* + * 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.example + +import com.amazon.ionelement.api.ionInt +import org.junit.jupiter.params.ParameterizedTest +import org.junit.jupiter.params.provider.ArgumentsSource +import org.partiql.sprout.tests.ArgumentsProviderBase +import org.partiql.sprout.tests.example.builder.ExampleFactoryImpl +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals + +/** + * Tests the generated equals and hashCode methods + */ +class EqualityTests { + + @ParameterizedTest + @ArgumentsSource(EqualArgumentsProvider::class) + fun testEquality(tc: EqualArgumentsProvider.TestCase) { + assertEquals(tc.first, tc.second) + assertEquals(tc.first.hashCode(), tc.second.hashCode()) + } + + /** + * We do not check that hashCode produces unequal results due to the `hashCode()` JavaDocs: + * > It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then + * > calling the hashCode method on each of the two objects must produce distinct integer results. + */ + @ParameterizedTest + @ArgumentsSource(NotEqualArgumentsProvider::class) + fun testNotEquality(tc: NotEqualArgumentsProvider.TestCase) { + assertNotEquals(tc.first, tc.second) + } + + class EqualArgumentsProvider : ArgumentsProviderBase() { + private val factory = ExampleFactoryImpl() + override fun getParameters(): List = listOf( + TestCase(factory.exprEmpty(), factory.exprEmpty()), + TestCase(factory.exprIon(ionInt(1)), factory.exprIon(ionInt(1))), + TestCase( + factory.identifierQualified( + factory.identifierSymbol("hello", Identifier.CaseSensitivity.INSENSITIVE), + emptyList() + ), + factory.identifierQualified( + factory.identifierSymbol("hello", Identifier.CaseSensitivity.INSENSITIVE), + emptyList() + ) + ), + TestCase( + factory.identifierQualified( + factory.identifierSymbol("hello", Identifier.CaseSensitivity.INSENSITIVE), + listOf( + factory.identifierSymbol("world", Identifier.CaseSensitivity.SENSITIVE), + factory.identifierSymbol("yeah", Identifier.CaseSensitivity.INSENSITIVE), + factory.identifierSymbol("foliage", Identifier.CaseSensitivity.INSENSITIVE), + ) + ), + factory.identifierQualified( + factory.identifierSymbol("hello", Identifier.CaseSensitivity.INSENSITIVE), + listOf( + factory.identifierSymbol("world", Identifier.CaseSensitivity.SENSITIVE), + factory.identifierSymbol("yeah", Identifier.CaseSensitivity.INSENSITIVE), + factory.identifierSymbol("foliage", Identifier.CaseSensitivity.INSENSITIVE), + ) + ) + ), + TestCase( + factory.statementQuery(factory.exprEmpty()), + factory.statementQuery(factory.exprEmpty()) + ) + ) + + class TestCase( + val first: ExampleNode, + val second: ExampleNode + ) + } + + class NotEqualArgumentsProvider : ArgumentsProviderBase() { + private val factory = ExampleFactoryImpl() + override fun getParameters(): List = listOf( + TestCase(factory.exprEmpty(), factory.exprIon(ionInt(1))), + TestCase(factory.exprIon(ionInt(1)), factory.exprIon(ionInt(2))), + TestCase( + factory.identifierSymbol("hello", Identifier.CaseSensitivity.INSENSITIVE), + factory.identifierSymbol("hello", Identifier.CaseSensitivity.SENSITIVE) + ), + TestCase( + factory.identifierQualified( + factory.identifierSymbol("hello", Identifier.CaseSensitivity.INSENSITIVE), + listOf( + factory.identifierSymbol("world", Identifier.CaseSensitivity.SENSITIVE), + factory.identifierSymbol("yeah", Identifier.CaseSensitivity.INSENSITIVE), + factory.identifierSymbol("foliage", Identifier.CaseSensitivity.INSENSITIVE), + ) + ), + factory.identifierQualified( + factory.identifierSymbol("hello", Identifier.CaseSensitivity.INSENSITIVE), + listOf( + factory.identifierSymbol("NOT_WORLD", Identifier.CaseSensitivity.SENSITIVE), + factory.identifierSymbol("yeah", Identifier.CaseSensitivity.INSENSITIVE), + factory.identifierSymbol("foliage", Identifier.CaseSensitivity.INSENSITIVE), + ) + ) + ) + ) + + class TestCase( + val first: ExampleNode, + val second: ExampleNode + ) + } +} From 702aacac517bd5e893f628d1acaadb8de8534cbd Mon Sep 17 00:00:00 2001 From: John Ed Quinn Date: Tue, 22 Aug 2023 09:20:17 -0700 Subject: [PATCH 2/2] Adds deep equality tests --- .../src/main/resources/example.ion | 9 +- .../sprout/tests/example/EqualityTests.kt | 158 ++++++++++++++++++ 2 files changed, 165 insertions(+), 2 deletions(-) diff --git a/test/sprout-tests/src/main/resources/example.ion b/test/sprout-tests/src/main/resources/example.ion index 40332581ec..dc7ac62858 100644 --- a/test/sprout-tests/src/main/resources/example.ion +++ b/test/sprout-tests/src/main/resources/example.ion @@ -24,11 +24,16 @@ expr::[ identifier: identifier, scope: [ DEFAULT, - // x.y.z LOCAL, - // @x.y.z ], }, + + nested::{ + itemsList: list::[list::[expr]], + itemsSet: set::[set::[expr]], + itemsMap: map::[string, map::[string, expr]] + }, + empty::{} ] // Identifiers and Qualified Identifiers diff --git a/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/example/EqualityTests.kt b/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/example/EqualityTests.kt index d3874502e9..f8624cf5df 100644 --- a/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/example/EqualityTests.kt +++ b/test/sprout-tests/src/test/kotlin/org/partiql/sprout/tests/example/EqualityTests.kt @@ -81,6 +81,85 @@ class EqualityTests { TestCase( factory.statementQuery(factory.exprEmpty()), factory.statementQuery(factory.exprEmpty()) + ), + // Tests deep equality of LISTS + TestCase( + factory.exprNested( + itemsList = listOf( + listOf( + factory.exprEmpty(), + factory.exprIon(ionInt(1)) + ), + listOf( + factory.exprIon(ionInt(3)) + ) + ), + itemsSet = emptySet(), + itemsMap = emptyMap() + ), + factory.exprNested( + itemsList = listOf( + listOf( + factory.exprEmpty(), + factory.exprIon(ionInt(1)) + ), + listOf( + factory.exprIon(ionInt(3)) + ) + ), + itemsSet = emptySet(), + itemsMap = emptyMap() + ), + ), + // Tests deep equality of SETS + TestCase( + first = factory.exprNested( + itemsList = emptyList(), + itemsSet = setOf( + setOf(), + setOf(factory.exprEmpty()), + setOf(factory.exprIon(ionInt(1)), factory.exprIon(ionInt(2))) + ), + itemsMap = emptyMap() + ), + second = factory.exprNested( + itemsList = emptyList(), + itemsSet = setOf( + setOf(), + setOf(factory.exprEmpty()), + setOf(factory.exprIon(ionInt(1)), factory.exprIon(ionInt(2))) + ), + itemsMap = emptyMap() + ), + ), + // Tests deep equality of MAPS + TestCase( + first = factory.exprNested( + itemsList = emptyList(), + itemsSet = emptySet(), + itemsMap = mapOf( + "hello" to mapOf( + "world" to factory.exprEmpty(), + "!" to factory.exprIon(ionInt(1)) + ), + "goodbye" to mapOf( + "friend" to factory.exprIon(ionInt(2)) + ) + ) + ), + second = factory.exprNested( + itemsList = emptyList(), + itemsSet = emptySet(), + itemsMap = mapOf( + "hello" to mapOf( + "world" to factory.exprEmpty(), + "!" to factory.exprIon(ionInt(1)) + ), + "goodbye" to mapOf( + "friend" to factory.exprIon(ionInt(2)) + ) + ) + ), ) ) @@ -116,6 +195,85 @@ class EqualityTests { factory.identifierSymbol("foliage", Identifier.CaseSensitivity.INSENSITIVE), ) ) + ), + // Tests deep equality of LISTS + TestCase( + factory.exprNested( + itemsList = listOf( + listOf( + factory.exprEmpty(), + factory.exprIon(ionInt(1)) + ), + listOf( + factory.exprIon(ionInt(3)) + ) + ), + itemsSet = emptySet(), + itemsMap = emptyMap() + ), + factory.exprNested( + itemsList = listOf( + listOf( + factory.exprEmpty(), + factory.exprIon(ionInt(2)) + ), + listOf( + factory.exprIon(ionInt(3)) + ) + ), + itemsSet = emptySet(), + itemsMap = emptyMap() + ), + ), + // Tests deep equality of SETS + TestCase( + first = factory.exprNested( + itemsList = emptyList(), + itemsSet = setOf( + setOf(), + setOf(factory.exprEmpty()), + setOf(factory.exprIon(ionInt(1)), factory.exprIon(ionInt(2))) + ), + itemsMap = emptyMap() + ), + second = factory.exprNested( + itemsList = emptyList(), + itemsSet = setOf( + setOf(), + setOf(factory.exprEmpty()), + setOf(factory.exprIon(ionInt(1)), factory.exprIon(ionInt(3))) + ), + itemsMap = emptyMap() + ), + ), + // Tests deep equality of MAPS + TestCase( + first = factory.exprNested( + itemsList = emptyList(), + itemsSet = emptySet(), + itemsMap = mapOf( + "hello" to mapOf( + "world" to factory.exprEmpty(), + "!" to factory.exprIon(ionInt(1)) + ), + "goodbye" to mapOf( + "friend" to factory.exprIon(ionInt(2)) + ) + ) + ), + second = factory.exprNested( + itemsList = emptyList(), + itemsSet = emptySet(), + itemsMap = mapOf( + "hello" to mapOf( + "world" to factory.exprEmpty(), + "!" to factory.exprIon(ionInt(1)) + ), + "goodbye" to mapOf( + "friend" to factory.exprIon(ionInt(3)) + ) + ) + ), ) )