Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new custom Android Lint detector, UnreferencedResourceDetector, designed to identify and flag unused fui_* string and plurals resources within library modules, along with its corresponding unit tests. Feedback on the implementation highlights a performance bottleneck in the XML scanning logic, where visiting every element and attribute and using element.textContent can lead to quadratic complexity. The reviewer suggests optimizing this by scanning XML files textually in beforeCheckFile and limiting element scanning to only string and plurals tags.
1457f75 to
7ed73c8
Compare
5da60c0 to
109874d
Compare
7ed73c8 to
248cae6
Compare
109874d to
2244d3d
Compare
248cae6 to
895be2b
Compare
2244d3d to
f036fe0
Compare
russellwheatley
left a comment
There was a problem hiding this comment.
Thanks for this, the detector and its tests look good to me. Two things before merge:
- #2521 was squash-merged into
pre-GA, so this branch still carries the old deletion commit (91 files listed). Could you rebase ontopre-GAso the PR is just the lint check? The effective diff is onlyLintIssueRegistry.kt,UnreferencedResourceDetector.ktand its test. - The "Known limits" KDoc is wrong for two of the cases, see the inline comment.
Small nit on SourceCodeScanner inline too. Also the KDoc and one test comment say 34 dead strings but the PR deletes 35, might be worth aligning.
f036fe0 to
d2853c6
Compare
Adds an
UnreferencedResourcelint check that fails the build on afui_*string or plurals resource:authnever reads.Changes
values/folders, and references fromR.string.andR.plurals.in Kotlin and Java and@string/and@plurals/in XML and the manifest.tools:attributes, which is what found the last of the strings deleted in chore(auth): delete 35 unreferenced fui_ string resources and their translations #2521.Why a custom check
UnusedResourcesis silent for a library module because lint cannot see its consumers, andcheckDependenciesagainst the demo app would flag nearly every resource.The policy it encodes
No
fui_*resource may be unreferenced within:authitself. Normally the wrong rule for a library; it holds because the README documents these names as ones an app overrides rather than reads.Maintainer note: Fixes internal CPRN-488