Skip to content

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