Generate sealed interfaces in response based codegen #3448

BoD · 2021-10-26T08:08:27Z

No description provided.

Contributing.md

BoD · 2021-10-26T12:04:59Z

Question: I think Kotlin 1.4 is currently our minimum target Kotlin version, but not sure? Does that mean that we maybe need a flag to opt-in/out of sealed interfaces which work since Kotlin 1.5?

martinbonnin · 2021-10-26T12:49:24Z

Looks good 👍

Also yep, you're 100% right we still need to support Kotlin 1.4. Let's make this a Gradle option similar to https://kotlinlang.org/docs/gradle.html#attributes-common-to-jvm-and-js?

apollo {
  // By default, infer it from the KotlinExtention
  languageVersion = "1.4"
}

BoD · 2021-10-27T07:53:19Z

Added a languageVersion property, which is passed to the KotlinContext, and a small Version utility class to be able to inspect it / compare it to other versions. Not sure what's the best way to add tests to this! Let me know what you think 😊

martinbonnin · 2021-10-27T13:38:29Z

(I'm also about to allow "X.Y" in addition to "X.Y.Z")

👍 Does it even make sense to set languageVersion = "1.4.32"?

martinbonnin · 2021-10-27T13:43:49Z

...-gradle-plugin/src/test/kotlin/com/apollographql/apollo3/gradle/test/LanguageVersionTests.kt

+      }
+    }
+  }
+


Add a 3rd test to make sure we can generate interfaces "not-sealed" ? (kotlin: 1.4, apollo: 1.4)

Oh yes, you're totally right, with 1.4 on both sides 👍

BoD · 2021-10-27T13:48:50Z

Does it even make sense to set languageVersion = "1.4.32"?

Well the extension that returns the Kotlin plugin version does return the full version, so it's nice that this format is accepted (but we could also strip it of course)

martinbonnin · 2021-10-27T13:49:15Z

...-gradle-plugin/src/test/kotlin/com/apollographql/apollo3/gradle/test/LanguageVersionTests.kt

+        usesKotlinDsl = true,
+        plugins = listOf(TestUtils.kotlinJvmPlugin, TestUtils.apolloPlugin),
+        apolloConfiguration = configuration,
+        graphqlPath = "hero",


FWIW, I'm not using TestUtils.withProject for new tests as manually maintaining the code to generate build.gradle[.kts] files is tedious. Instead, I add projects to apollo-gradle-plugin/testProjects. The nice thing is that these are "standalone" so it's easy to test them without going through the whole GradleRunner thing:

./gradlew -p apollo-gradle-plugin/testProjects/$projectName build

(no need to change this though)

Ah I see!

Actually that was my first approach but then realized I would need several tests and saw this way to do it existed - but now that I think of it, maybe I could have one specific project and pass parameters to it for the different cases.

You could do a testProject with simple templating too but no silver bullet at the end of the day 🤷 .

martinbonnin · 2021-10-27T13:51:23Z

Well the extension that returns the Kotlin plugin version does return the full version, so it's nice that this format is accepted (but we could also strip it of course)

It's nice to accept it (be flexible about what you receive) but I think documentation and samples should use the "x.y" syntax, as in the Kotlin docs

martinbonnin · 2021-10-27T13:59:12Z

apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/Options.kt

+    /**
+     * Currently only used when [targetLanguage] is "kotlin".
+     */
+    val targetLanguageVersion: String = defaultTargetLanguageVersion,


What about merging that option with targetLanguage ?

"java" "kotlin_1_4" "kotlin_1_5"

Interesting - you mean only at the Options level, and not at the user facing (Gradle plugin) level, right?

Exactly!

In the Gradle plugin, I think it makes sense to keep languageLevel as in https://kotlinlang.org/docs/gradle.html#attributes-common-to-jvm-and-js. I'm not 100% convinced but I think it'd make sense

martinbonnin · 2021-10-27T14:03:38Z

apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/VersionNumber.kt

+    private val REGEX = Regex("(\\d)\\.(\\d)(\\.(\\d).*)?")
+
+    fun parse(s: String): VersionNumber {
+      val matchResult = REGEX.find(s) ?: error("Could not parse '$s' as a version number")


Might be worth catching this in the Gradle plugin and display a more user friendly message:

ApolloGraphQL: languageLevel "$languageLevel" is not supported.

apollo-gradle-plugin/src/main/kotlin/com/apollographql/apollo3/gradle/api/Service.kt

...lugin/src/main/kotlin/com/apollographql/apollo3/gradle/internal/ApolloGenerateSourcesTask.kt

apollo-gradle-plugin/src/main/kotlin/com/apollographql/apollo3/gradle/api/Service.kt

apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/Options.kt

martinbonnin · 2021-10-28T09:11:44Z

...-gradle-plugin/src/test/kotlin/com/apollographql/apollo3/gradle/test/LanguageVersionTests.kt

+        TestUtils.executeTask(":assemble", dir)
+        Assert.fail("Compiling with incompatible languageVersion should fail")
+      } catch (e: UnexpectedBuildFailure) {
+        Truth.assertThat(e.message).contains("languageVersion '3.14' is not supported")


🥧 ❤️

…in apollo-gradle-plugin

Co-authored-by: Martin Bonnin <[email protected]>

Simplification: get rid of VersionNumber and explicitly allow only "1.4" or "1.5" instead.

Co-authored-by: Martin Bonnin <[email protected]>

BoD commented Oct 26, 2021

View reviewed changes

Contributing.md Show resolved Hide resolved

BoD marked this pull request as ready for review October 26, 2021 11:42

BoD requested review from martinbonnin, sav007 and tasomaniac as code owners October 26, 2021 11:42

martinbonnin approved these changes Oct 26, 2021

View reviewed changes

martinbonnin reviewed Oct 27, 2021

View reviewed changes

apollo-gradle-plugin/src/main/kotlin/com/apollographql/apollo3/gradle/api/Service.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Oct 27, 2021

View reviewed changes

apollo-gradle-plugin/src/main/kotlin/com/apollographql/apollo3/gradle/api/Service.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Oct 27, 2021

View reviewed changes

...lugin/src/main/kotlin/com/apollographql/apollo3/gradle/internal/ApolloGenerateSourcesTask.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Oct 28, 2021

View reviewed changes

apollo-gradle-plugin/src/main/kotlin/com/apollographql/apollo3/gradle/api/Service.kt Outdated Show resolved Hide resolved

martinbonnin reviewed Oct 28, 2021

View reviewed changes

apollo-compiler/src/main/kotlin/com/apollographql/apollo3/compiler/Options.kt Show resolved Hide resolved

martinbonnin reviewed Oct 28, 2021

View reviewed changes

martinbonnin approved these changes Oct 28, 2021

View reviewed changes

BoD and others added 7 commits October 28, 2021 11:16

Generate sealed interfaces in response based codegen

ba67b29

Use a flag in ModelBuilder instead of the id to output sealed or not

64137ba

Add an integration test showing exhaustive when on sealed interface

2db9d74

Add a languageVersion property and use it in the codegen

c2899a0

Fix wiring between Service and DefaultApolloExtension and add a test …

bf127f5

…in apollo-gradle-plugin

Allow short form for VersionNumber and throw when unable to parse

63878a0

Apply suggestions from code review

15ba17e

Co-authored-by: Martin Bonnin <[email protected]>

BoD and others added 4 commits October 28, 2021 11:16

Fix version parsing

6d6bd40

Merge targetLanguageVersion and targetLanguage (use an enum now).

755cde4

Simplification: get rid of VersionNumber and explicitly allow only "1.4" or "1.5" instead.

Update API dump

86d74b3

Apply suggestions from code review

37906e3

Co-authored-by: Martin Bonnin <[email protected]>

BoD force-pushed the generate-sealed-interfaces branch from 0795137 to 37906e3 Compare October 28, 2021 09:18

BoD merged commit 4b9531f into dev-3.x Oct 28, 2021

BoD deleted the generate-sealed-interfaces branch October 28, 2021 12:39

BoD mentioned this pull request Oct 28, 2021

Represent Union Types as Sealed Classes when generating Kotlin models #1390

Closed

martinbonnin added the 2021-10 label Oct 29, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate sealed interfaces in response based codegen #3448

Generate sealed interfaces in response based codegen #3448

BoD commented Oct 26, 2021

BoD commented Oct 26, 2021

martinbonnin commented Oct 26, 2021

BoD commented Oct 27, 2021

martinbonnin commented Oct 27, 2021

martinbonnin Oct 27, 2021 •

edited

Loading

BoD Oct 27, 2021

BoD commented Oct 27, 2021

martinbonnin Oct 27, 2021 •

edited

Loading

BoD Oct 27, 2021

martinbonnin Oct 27, 2021

martinbonnin commented Oct 27, 2021 •

edited

Loading

martinbonnin Oct 27, 2021

BoD Oct 27, 2021

martinbonnin Oct 27, 2021

martinbonnin Oct 27, 2021

martinbonnin Oct 28, 2021

Generate sealed interfaces in response based codegen #3448

Generate sealed interfaces in response based codegen #3448

Conversation

BoD commented Oct 26, 2021

BoD commented Oct 26, 2021

martinbonnin commented Oct 26, 2021

BoD commented Oct 27, 2021

martinbonnin commented Oct 27, 2021

martinbonnin Oct 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BoD commented Oct 27, 2021

martinbonnin Oct 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinbonnin commented Oct 27, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martinbonnin Oct 27, 2021 •

edited

Loading

martinbonnin Oct 27, 2021 •

edited

Loading

martinbonnin commented Oct 27, 2021 •

edited

Loading