Kotlin: Clean up redundant constructs (2/n) #51170
Conversation
facebook-github-bot
added
the
CLA Signed
facebook-github-bot
added
the
Shared with Meta
|
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@cortinico merged this pull request in 04ab28f. |
|
This pull request was successfully merged by @mateoguzmana in 04ab28f When will my fix make it into a release? | How to file a pick request? |
|
|
||
| import android.content.Context | ||
| import android.content.res.ColorStateList | ||
| import androidx.core.content.res.use |
There was a problem hiding this comment.
Hey @mateoguzmana just a small heads up that this triggered a lot of crashes internally. Once we remove this import, we fallback to use Kotlin's use (https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.io/use.html) which is different than this one: https://developer.android.com/reference/kotlin/androidx/core/content/res/package-summary#(android.content.res.TypedArray).use(kotlin.Function1)
Specifically the app were crashing with .IncompatibleClassChangeError: Class 'android.content.res.TypedArray' does not implement interface 'java.lang.AutoCloseable'
There was a problem hiding this comment.
Thanks for the heads up and for fixing it @cortinico! Apologies if it caused some extra work on your side... It looked pretty harmless as it was showing as unused with static code analysis 🥲
Not sure if there is something we can do to ensure this regression doesn't happen again, but if there is, I'm happy to help with it
There was a problem hiding this comment.
Not sure if there is something we can do to ensure this regression doesn't happen again, but if there is, I'm happy to help with it
As a bare minimun having a comment on top of the import would help. Otherwise we need to suppress the IDE warning with a @Suppress annotation.
It's a bit tricky for me because I don't see a warning on that import with Android Studio so I'm a bit unsure how to suppress it
There was a problem hiding this comment.
I ran the code analysis again, and Android Studio suggested a way to suppress it. I've done it in #51272 and also added some context about why it is needed
Summary: Follow up from #51170, static code analysis shows androidx.core.content.res.use as unused, it looked pretty harmless to remove it as there was no context about its usage, but it caused some crashes. I'm suppressing the warning here, plus adding an explanation on why it is needed to prevent a future developer from touching this file and causing the same regression. ## Changelog: [INTERNAL] - Suppress unused androidx.core.content.res.use warning Pull Request resolved: #51272 Test Plan: Static code analysis should not show the import as unused. Reviewed By: cortinico Differential Revision: D74642726 Pulled By: rshest fbshipit-source-id: 14cec4fe92f06827636410df4b88a3b7088abe52
4 participants
Summary:
Follow up from #51061 – Static code analysis detected several redundant constructs across the codebase. Most of the ones fixed here are marked as warnings/weak warnings, likely code smells post-migration from Java.
Doing another small round to clean up some of them.
Changelog:
[INTERNAL] - Kotlin: Clean up redundant constructs
Test Plan: