Fix #6365: Resolve intermittent Android Lint errors in app, testing, and utility - #6423
Fix #6365: Resolve intermittent Android Lint errors in app, testing, and utility#6423Kishan8548 wants to merge 5 commits into
Conversation
…ing, and utility - Replace java.util.Base64 (API 26+) with android.util.Base64 (API 1+) in ControlButtonsViewModel to fix NEW_API lint errors across all API levels. - Replace getDeclaredAnnotation() (API 24+) with getAnnotation() (API 1+) in OppiaParameterizedTestRunner to fix NEW_API lint errors. - Replace getDeclaredAnnotationsByType() (API 24+) with a custom Method.fetchIterations() helper using Java reflection for repeatable @iteration annotations, removing all @RequiresApi(N) annotations. - Add @Suppress("WrongConstant") to getOpacity() in MathTagHandler and SvgPictureDrawable where PixelFormat constants are correctly used but trigger false-positive lint warnings due to UAST annotation bugs.
Coverage ReportResultsNumber of files assessed: 4 Passing coverageFiles with passing code coverage
Exempted coverageFiles exempted from coverage
|
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Resolves intermittent Android Lint failures (especially in --mode=full) by removing NewApi usages below minSdk and suppressing known false-positive WrongConstant warnings for Drawable#getOpacity().
Changes:
- Replaced Java
Base64usage withandroid.util.Base64to avoid API 26NewApilint errors. - Updated parameterized test runner reflection logic to avoid API 24 annotation APIs.
- Suppressed false-positive
WrongConstantlint warnings for validPixelFormatopacity values.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| utility/src/main/java/org/oppia/android/util/parser/svg/SvgPictureDrawable.kt | Suppresses WrongConstant for PixelFormat.TRANSLUCENT in getOpacity(). |
| utility/src/main/java/org/oppia/android/util/parser/html/MathTagHandler.kt | Suppresses WrongConstant for PixelFormat.TRANSPARENT in getOpacity(). |
| testing/src/main/java/org/oppia/android/testing/junit/OppiaParameterizedTestRunner.kt | Removes API 24-only annotation reflection and replaces it with a custom iteration fetcher. |
| app/src/main/java/org/oppia/android/app/administratorcontrols/learneranalytics/ControlButtonsViewModel.kt | Uses android.util.Base64 to avoid API 26-only java.util.Base64. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| try { | ||
| val valueMethod = annotation.annotationClass.java.getMethod("value") | ||
| val result = valueMethod.invoke(annotation) | ||
| val containerIterations = (result as? Array<*>)?.filterIsInstance<Iteration>() | ||
| if (containerIterations != null) { | ||
| iterations.addAll(containerIterations) | ||
| } | ||
| } catch (ignored: Exception) { |
There was a problem hiding this comment.
I agree with this concern. Could you please update this logic so that unexpected reflection errors are not silently ignored? @Kishan8548
Neer-rn
left a comment
There was a problem hiding this comment.
Hi @Kishan8548 please look at the co-pilot's comment and after you fix it please assign to manas or sandesh.
| try { | ||
| val valueMethod = annotation.annotationClass.java.getMethod("value") | ||
| val result = valueMethod.invoke(annotation) | ||
| val containerIterations = (result as? Array<*>)?.filterIsInstance<Iteration>() | ||
| if (containerIterations != null) { | ||
| iterations.addAll(containerIterations) | ||
| } | ||
| } catch (ignored: Exception) { |
There was a problem hiding this comment.
I agree with this concern. Could you please update this logic so that unexpected reflection errors are not silently ignored? @Kishan8548
Replace try-catch with NoSuchMethodException (control flow) with a firstOrNull check that explicitly verifies the method name, parameter count, return type array, and component type equals Iteration. Only invoke() is now wrapped in a try-catch, catching the specific exceptions it can throw (IllegalAccessException, InvocationTargetException) rather than silently ignoring all exceptions.
…tchIterations() The cast is safe because returnType.componentType is verified to equal Iteration::class.java before invoke() is called. The @Suppress is needed to prevent the compiler from treating the unchecked cast warning as an error.
…pty() (API 1+) Method.getParameterCount() requires API 26 which is above minSdk 21. Replace with method.parameterTypes.isEmpty() using getParameterTypes() which is available from API 1+.
Coverage ReportResultsNumber of files assessed: 4 Passing coverageFiles with passing code coverage
Exempted coverageFiles exempted from coverage
|
|
PTAL @manas-yu |
Explanation
Fixes #6365
Android Lint was intermittently failing (and consistently failing in
--mode=full) due toNEW_APIandWRONG_CONSTANTerrors in three files.Root Causes & Fixes
1.
ControlButtonsViewModel.kt—NEW_API(API 26)java.util.Base64.getEncoder().encodeToString()requires API 26, but minSdk is 21. Replaced withandroid.util.Base64.encodeToString(..., Base64.NO_WRAP)which works on API 1+.NO_WRAPproduces identical unwrapped output, preserving correct behaviour with the downstreamchunked(80)logic.2.
OppiaParameterizedTestRunner.kt—NEW_API(API 24)Three calls required Java 8 / API 24:
field.getDeclaredAnnotation(...)→ replaced withfield.getAnnotation(...)(API 1+)testClass.getDeclaredAnnotation(...)→ replaced withtestClass.getAnnotation(...)(API 1+)method.getDeclaredAnnotationsByType(Iteration::class.java)→ replaced with a customMethod.fetchIterations()extension function using standard Java reflection to handle both direct and repeatable container annotations without any API 24+ calls.All
@RequiresApi(Build.VERSION_CODES.N)annotations and unusedBuild/RequiresApiimports were removed.3.
MathTagHandler.kt&SvgPictureDrawable.kt—WRONG_CONSTANT(false positive)PixelFormat.TRANSPARENTandPixelFormat.TRANSLUCENTare valid return values forgetOpacity()per the AndroidDrawablecontract. Android Lint'sWrongConstantDetectorflags these as false positives due to a UAST annotation propagation bug. Added@Suppress("WrongConstant")with an explanatory comment in each case to silence the false-positive without masking any real error.Verification
bazel run //scripts:android_lint_check -- $(pwd) --mode=full→Total Issues: 0 — ANDROID LINT CHECK PASSEDbazel run //scripts:android_lint_check -- $(pwd) --checks=NewApi,WrongConstant→Total Issues: 0 — ANDROID LINT CHECK PASSEDMathTagHandlerTest→ PASSED (23 tests)ProfileAndDeviceIdFragmentTest→ PASSED (51 tests)ProfileNameValidatorTest→ PASSED (17 tests)Before & After Screenshots (for
WRONG_CONSTANTusage trace)As requested, here are before/after screenshots confirming that suppressing the
WRONG_CONSTANTwarnings inMathTagHandler.kt(LaTeX/math rendering) andSvgPictureDrawable.kt(SVG illustrations) causes zero visual regression:LaTeX / Math rendering (
MathTagHandler.kt)SVG / Vector illustration rendering (
SvgPictureDrawable.kt)Essential Checklist
Disclosure of LLM Usage