-
-
Notifications
You must be signed in to change notification settings - Fork 133
feat: detect and report all Android resource references in Manifest files #1450
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,6 @@ package com.autonomousapps.model.internal | |
import com.autonomousapps.internal.parse.AndroidResParser | ||
import com.autonomousapps.internal.utils.LexicographicIterableComparator | ||
import com.autonomousapps.internal.utils.efficient | ||
import com.autonomousapps.model.internal.AndroidResSource.AttrRef.Companion.type | ||
import com.autonomousapps.model.internal.intermediates.consumer.LdcConstant | ||
import com.squareup.moshi.JsonClass | ||
import dev.zacsweers.moshix.sealed.annotations.TypeLabel | ||
|
@@ -174,7 +173,7 @@ internal data class AndroidResSource( | |
* | ||
* @see <a href="https://developer.android.com/guide/topics/resources/providing-resources#ResourcesFromXml">Accessing resources from XML</a> | ||
*/ | ||
private val TYPE_REGEX = Regex("""@(\w+:)?(?<type>\w+)/(\w+)""") | ||
private val TYPE_REGEX = Regex("""@(\w+:)?(?<type>\w+)/([\w.]+)""") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also added a unit test for this. |
||
|
||
/** | ||
* TODO(tsr): this regex is too permissive. I only want `@+id/...`, but I lazily just copied the above with a | ||
|
@@ -201,10 +200,6 @@ internal data class AndroidResSource( | |
*/ | ||
private val ATTR_REGEX = Regex("""\?(\w+:)?(\w+/)?(?<attr>\w+)""") | ||
|
||
fun style(name: String): AttrRef? { | ||
return if (name.isBlank()) null else AttrRef("style", name.toCanonicalResString()) | ||
} | ||
|
||
/** | ||
* Push [AttrRef]s into the [container], either as external references or as internal "new IDs" (`@+id`). The | ||
* purpose of this approach is to avoid parsing the XML file twice. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,9 @@ | |
package com.autonomousapps.internal | ||
|
||
import com.autonomousapps.internal.ManifestParser.ManifestParseException | ||
import com.autonomousapps.model.internal.AndroidResSource.AttrRef | ||
import com.google.common.truth.Truth.assertThat | ||
import org.intellij.lang.annotations.Language | ||
import org.junit.jupiter.api.Assertions.assertThrows | ||
import org.junit.jupiter.api.Test | ||
import org.junit.jupiter.api.io.TempDir | ||
|
@@ -150,26 +152,60 @@ class ManifestParserTest { | |
assertThat(manifest.applicationName).isEqualTo("mutual.aid.explode") | ||
} | ||
|
||
@Test fun `parse themes`() { | ||
@Test fun `parse resource references`() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this test, I added common references that we are using in our own app. This is definitely not an exhaustive list of all places where references are allowed. |
||
val manifest = parse( | ||
manifest = """ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<manifest | ||
xmlns:android="http://schemas.android.com/apk/res/android" | ||
xmlns:tools="http://schemas.android.com/tools" > | ||
<manifest xmlns:android="http://schemas.android.com/apk/res/android" | ||
xmlns:tools="http://schemas.android.com/tools"> | ||
|
||
<application android:theme="@style/TheEternalVoid"> | ||
<activity android:name=".MainActivity" android:theme="@style/TheTwistedLand"/> | ||
</application> | ||
<application android:icon="@mipmap/ic_launcher" android:label="@string/app_name" | ||
android:networkSecurityConfig="@xml/network_security_config" | ||
android:theme="@style/TheEternalVoid"> | ||
<activity android:name=".MainActivity" android:theme="@style/TheTwistedLand"> | ||
<intent-filter android:autoVerify="true"> | ||
<action android:name="android.intent.action.VIEW" /> | ||
<category android:name="android.intent.category.DEFAULT" /> | ||
<category android:name="android.intent.category.BROWSABLE" /> | ||
<data android:scheme="https" /> | ||
<data android:host="@string/deeplink_host" /> | ||
<data android:path="@string/deeplink_path" /> | ||
</intent-filter> | ||
</activity> | ||
<provider android:name="androidx.startup.InitializationProvider" | ||
android:authorities="${'$'}{applicationId}.androidx-startup" | ||
android:exported="false" tools:node="merge"> | ||
<meta-data android:name="com.app.MyInitializer" | ||
android:value="@string/androidx_startup" /> | ||
</provider> | ||
<meta-data android:name="com.google.android.geo.API_KEY" | ||
android:value="@string/google_maps_api_key" /> | ||
<meta-data android:name="com.google.android.gms.version" | ||
android:value="@integer/google_play_services_version" /> | ||
<meta-data android:name="google_analytics_default_allow_analytics_storage" | ||
android:value="@bool/google_analytics_enabled" /> | ||
</application> | ||
</manifest> | ||
""".trimIndent(), | ||
dslNamespace = "com.app" | ||
) | ||
|
||
assertThat(manifest.themes).isEqualTo(setOf("TheEternalVoid", "TheTwistedLand")) | ||
assertThat(manifest.attrRefs).containsExactly( | ||
AttrRef(type = "mipmap", id = "ic_launcher"), | ||
AttrRef(type = "string", id = "app_name"), | ||
AttrRef(type = "xml", id = "network_security_config"), | ||
AttrRef(type = "style", id = "TheEternalVoid"), | ||
AttrRef(type = "style", id = "TheTwistedLand"), | ||
AttrRef(type = "string", id = "deeplink_host"), | ||
AttrRef(type = "string", id = "deeplink_path"), | ||
AttrRef(type = "string", id = "androidx_startup"), | ||
AttrRef(type = "string", id = "google_maps_api_key"), | ||
AttrRef(type = "integer", id = "google_play_services_version"), | ||
AttrRef(type = "bool", id = "google_analytics_enabled"), | ||
) | ||
} | ||
|
||
private fun parse(manifest: String, dslNamespace: String = ""): ManifestParser.ParseResult { | ||
private fun parse(@Language("XML") manifest: String, dslNamespace: String = ""): ManifestParser.ParseResult { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding this for syntax highlighting in the IDE |
||
val file = tempFolder.resolve("AndroidManifest.xml").toFile() | ||
file.writeText(manifest) | ||
return ManifestParser(dslNamespace).parse(file) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merge the
themes
inside a genericattrRefs
.Also, I parsed it directly as
AndroidResSource.AttrRef
to make tests more expressive (instead of inexplodeAndroidResSource.kt
).