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

Address compile warnings in generated Kotlin code #205

Merged
merged 2 commits into from
Aug 21, 2020

Conversation

larryng
Copy link
Contributor

@larryng larryng commented Jun 6, 2020

Description:
Generated Kotlin code triggers two compiler warnings: UNCHECKED_CAST and REDUNDANT_PROJECTION. With allWarningsAsErrors enabled, we're unable to upgrade to recent versions of Motif. This PR addresses the two warnings by suppressing them.

Related issue(s):
None AFAICT.

@larryng
Copy link
Contributor Author

larryng commented Jun 19, 2020

Any feedback on this? (@Leland-Takamine?)

@Leland-Takamine
Copy link
Collaborator

Hi @larryng - can you please show a diff of the changes to the generated code from before and after your changes?

@larryng
Copy link
Contributor Author

larryng commented Jun 23, 2020

@Leland-Takamine I don't really have the time to offer a total example, but here's a redacted snippet from our codebase:

Added to generated ScopeImpl classes:

+@Suppress("REDUNDANT_PROJECTION")
 @ScopeImpl(

Initialize to null instead of None.NONE for cached members

   @Volatile
-  private var loggedInInteractor: Any = None.NONE
+  private var loggedInInteractor: LoggedInInteractor? = null

No need for casting, but do need !! but it should be safe

   fun loggedInInteractor(): LoggedInInteractor {
-    if (loggedInInteractor == None.NONE) {
+    if (loggedInInteractor == null) {
       synchronized (this) {
-        if (loggedInInteractor == None.NONE) {
+        if (loggedInInteractor == null) {
           loggedInInteractor = LoggedInInteractor(...
               ... REDACTED ...
               ...)}
       }
     }
-    return loggedInInteractor as LoggedInInteractor}
+    return loggedInInteractor!!}

@larryng
Copy link
Contributor Author

larryng commented Jul 27, 2020

Bump @Leland-Takamine

@Leland-Takamine
Copy link
Collaborator

Hey @larryng - The use of None.NONE allows for nullable dependencies if we choose to support that in the future. Was this change necessary for a Kotlin warning? If so, can we just suppress instead?

@larryng
Copy link
Contributor Author

larryng commented Jul 28, 2020

@Leland-Takamine Ah, I was wondering why it was like that. No, it's not necessary and we can just suppress it instead.

@larryng
Copy link
Contributor Author

larryng commented Jul 28, 2020

@Leland-Takamine Updated. New generated code simply has:

@Suppress("REDUNDANT_PROJECTION", "UNCHECKED_CAST")

added to @ScopeImpl classes.

@larryng
Copy link
Contributor Author

larryng commented Aug 21, 2020

Hate to bump again, but it's been awhile @Leland-Takamine

@Leland-Takamine
Copy link
Collaborator

Looks good - @tyvsmith Would you be able to push a release for this?

@Leland-Takamine Leland-Takamine merged commit 441ef19 into uber:master Aug 21, 2020
@Leland-Takamine
Copy link
Collaborator

@larryng The release has been pushed and should be available within the next few hours.

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