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

Simple string member is always null in 20200716 #23

Closed
ZacSweers opened this issue Jul 19, 2020 · 6 comments
Closed

Simple string member is always null in 20200716 #23

ZacSweers opened this issue Jul 19, 2020 · 6 comments
Assignees

Comments

@ZacSweers
Copy link
Contributor

This appears to be a regression from 20200626 to 20200716

Repro case in this PR: ZacSweers/MoshiX#25. Run ./gradlew :sample:compileKotlin

In KspUtil.kt, there is a helper getMember() function that gets an annotation member's value by name. That PR uses this from MoshiSealedSymbolProcessor to look up the value of a generator member. It does find the member, but now its string value is always null.

internal inline fun <reified T> KSAnnotation.getMember(name: String): T {
  val matchingArg = arguments.find { it.name?.asString() == name }
      ?: error("No member name found for '$name'. All arguments: ${arguments.map { it.name?.asString() }}")

  // NOTE fails here
  return matchingArg.value as? T ?: error("No value found for $name. Was ${matchingArg.value}")
}
e: java.lang.IllegalStateException: No value found for generator. Was null
	at dev.zacsweers.moshisealed.codegen.MoshiSealedSymbolProcessor.process(MoshiSealedSymbolProcessor.kt:179)
	at org.jetbrains.kotlin.ksp.AbstractKotlinSymbolProcessingExtension.doAnalysis(KotlinSymbolProcessingExtension.kt:76)
	at org.jetbrains.kotlin.cli.jvm.compiler.TopDownAnalyzerFacadeForJVM.analyzeFilesWithJavaIntegration(TopDownAnalyzerFacadeForJVM.kt:114)
	at org.jetbrains.kotlin.cli.jvm.compiler.TopDownAnalyzerFacadeForJVM.analyzeFilesWithJavaIntegration$default(TopDownAnalyzerFacadeForJVM.kt:91)
@neetopia
Copy link
Contributor

I pulled your z/ksp branch to my local and ran ./gradlew :sample:compileKotlin, build is passing.

@neetopia
Copy link
Contributor

I am able to reproduce with :sample:build though.

@ZacSweers
Copy link
Contributor Author

Strange... I was running from IntelliJ, but that should be similar. I wonder if maybe there's some sort of stale state issue with local projects or incremental reruns?

@neetopia
Copy link
Contributor

neetopia commented Jul 19, 2020

Actually now I am no longer able to reproduce the issue again even with --no-build-cache clean...

:sample:build now fails at test assertions.

@ZacSweers
Copy link
Contributor Author

Away from my computer now, but what if you try an incremental build? That would be consistent with trying compileKotlin first followed by build, and similar to my workflow when I encountered it

@neetopia
Copy link
Contributor

neetopia commented Jul 19, 2020

I was able to reproduce after configured IDE to run Gradle build, AND disable incremental compilation in gradle properties. Even if I added one line in MoshiSealedSymbolProcessor incremental compilation is still not invalidated and I have to disable it in properties. Somehow command line still not work for me.

This is actually not a complete regression. There is a major issue (see android/kotlin-for-ksp#46), so in 20200626 build you should be getting No member name found for 'generator'. All arguments: [generateAdapter] for your repro repo, since KSP wasn't able to see the default annotation value at all.

There are some major efforts to address this issue, unfortunately in latest release we didn't include a fix that will address the use case of inspecting default annotation argument from Java library android/kotlin#67 . I tried the locally built KSP and it is producing the following result.

// Code generated by moshi-sealed. Do not edit.
package dev.zacsweers.moshisealed.sample

import com.squareup.moshi.JsonAdapter
import com.squareup.moshi.JsonReader
import com.squareup.moshi.JsonWriter
import com.squareup.moshi.Moshi
import com.squareup.moshi.adapters.PolymorphicJsonAdapterFactory
import kotlin.Suppress
import kotlin.collections.emptySet

class MessageJsonAdapter(
  moshi: Moshi
) : JsonAdapter<Message>() {
  @Suppress("UNCHECKED_CAST")
  private val runtimeAdapter: JsonAdapter<Message> =
      PolymorphicJsonAdapterFactory.of(Message::class.java, "type")
          .withSubtype(Message.Success::class.java, "success")
          .withSubtype(Message.Error::class.java, "error")
          .withDefaultValue(Message.Unknown)
          .create(Message::class.java, emptySet(), moshi) as JsonAdapter<Message>


  override fun fromJson(reader: JsonReader): Message? = runtimeAdapter.fromJson(reader)

  override fun toJson(writer: JsonWriter, value: Message?) {
    runtimeAdapter.toJson(writer, value)
  }
}

Let me know if you need help on how to build KSP locally, it should also work if you just treat null value from annotation argument since you know what the default value is (for now).

@neetopia neetopia self-assigned this Jul 20, 2020
@davidjwiner davidjwiner transferred this issue from android/kotlin Sep 25, 2020
ting-yuan pushed a commit to ting-yuan/ksp that referenced this issue Feb 18, 2022
* Add pluginClasspaths to the configuration
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

No branches or pull requests

2 participants