Skip to content

Commit

Permalink
fix: Fix some issues in DuplicateResourceFilesDetector (#3)
Browse files Browse the repository at this point in the history 
* Fix some test functions name

* Remove tools namespace attributes from the root node

* Clear the last document

* Flush the string writer

* Remove tools namespace declaration

* Add a test to validate a file with tools namespace and another without

* Remove attributes node instead of replacing the string

* Rename some files in the tests
  • Loading branch information
@hfeky
hfeky authored Oct 27, 2022
1 parent cc957c4 commit 225957b
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 31 deletions.
44 changes: 23 additions & 21 deletions linta-android/src/main/java/com/swvl/lint/checks/DuplicateResourceFilesDetector.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.swvl.lint.checks
import com.android.resources.ResourceFolderType
import com.android.tools.lint.detector.api.*
import org.w3c.dom.Document
import org.w3c.dom.Element
import org.w3c.dom.Node
import java.io.StringWriter
import javax.xml.transform.TransformerFactory
Expand All @@ -32,8 +33,6 @@ class DuplicateResourceFilesDetector : ResourceXmlDetector() {

private val resources = HashMap<String, MutableList<ResourceDeclaration>>()

private lateinit var currentDocument: String

override fun appliesTo(folderType: ResourceFolderType): Boolean {
return folderType in setOf(
ResourceFolderType.LAYOUT,
Expand All @@ -49,42 +48,45 @@ class DuplicateResourceFilesDetector : ResourceXmlDetector() {
}

override fun visitDocument(context: XmlContext, document: Document) {
removeToolsNamespaceAttributes(document.firstChild ?: return)

val stringWriter = StringWriter()

// The transformer will auto-order the elements attributes.
TransformerFactory.newInstance().newTransformer()
.transform(DOMSource(document), StreamResult(stringWriter))

currentDocument = stringWriter.toString()

removeToolsNamespaceAttributes(document.firstChild ?: return)
// Remove whitespaces.
val currentDocument = stringWriter.buffer.replace("\\s+".toRegex(), "")

currentDocument = currentDocument.replace("\\s+".toRegex(), "")
stringWriter.flush()

// Cache the document.
resources[currentDocument] =
resources.getOrDefault(currentDocument, ArrayList()).apply {
add(ResourceDeclaration(context.file.name, context.createLocationHandle(document)))
}
}

private fun removeToolsNamespaceAttributes(child: Node?) {
if (child?.childNodes?.length == 0) {
return
private fun removeToolsNamespaceAttributes(node: Node) {
// Remove tools namespace and all attributes under it.
if (node.nodeType == Element.ELEMENT_NODE) {
var i = 0
while (i < node.attributes.length) {
val attr = node.attributes.item(i)
if (attr.namespaceURI == TOOLS_NAMESPACE_URI || attr.nodeValue == TOOLS_NAMESPACE_URI) {
node.attributes.removeNamedItem(attr.nodeName)
continue
}
i++
}
}

val childrenCount = child?.childNodes?.length ?: 0
// Do the same with all children.
val childrenCount = node.childNodes.length
for (i in 0 until childrenCount) {
val subChild = child?.childNodes?.item(i)
val attributesCount = subChild?.attributes?.length ?: 0
for (j in 0 until attributesCount) {
val attr = subChild?.attributes?.item(j)
if (attr?.namespaceURI == TOOLS_NAMESPACE_URI)
currentDocument = currentDocument.replace(
attr.toString(),
""
)
}
removeToolsNamespaceAttributes(subChild)
val child = node.childNodes.item(i)
removeToolsNamespaceAttributes(child)
}
}

Expand Down
60 changes: 50 additions & 10 deletions linta-android/src/test/java/com/swvl/lint/checks/DuplicateResourceFilesDetectorTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ import org.junit.runners.JUnit4
class DuplicateResourceFilesDetectorTest {

@Test
fun `Given duplicate shape resources with different white-spacing, an error should be reported`() {
fun `Given duplicate shape resources with different white-spacing, a warning should be reported`() {
TestLintTask.lint()
.files(
xml(
"res/drawable/shape.xml",
"res/drawable/shape1.xml",
"""
<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:shape="rectangle">
Expand Down Expand Up @@ -69,11 +69,11 @@ class DuplicateResourceFilesDetectorTest {
}

@Test
fun `Given different shape resources, no error should be reported`() {
fun `Given different shape resources, no warning should be reported`() {
TestLintTask.lint()
.files(
xml(
"res/drawable/shape.xml",
"res/drawable/shape1.xml",
"""
<shape xmlns:android="http://schemas.android.com/apk/res/android"
android:shape="rectangle">
Expand Down Expand Up @@ -106,7 +106,48 @@ class DuplicateResourceFilesDetectorTest {
}

@Test
fun `Given 2 identical resources with different attributes that are under the tools namespace, an error should be reported`() {
fun `Given 2 duplicate resources, one with attributes that are under the tools namespace and another without, a warning should be reported`() {
TestLintTask.lint()
.files(
xml(
"res/layout/item_title.xml",
"""
<TextView xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:id="@+id/tv_id"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:background="?selectableItemBackground"
android:paddingHorizontal="32dp"
android:paddingVertical="16dp"
android:textColor="#000000"
android:textSize="20sp"
tools:text="Title" />
"""
).indented(),
xml(
"res/layout/item_option.xml",
"""
<TextView xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/tv_id"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:background="?selectableItemBackground"
android:paddingHorizontal="32dp"
android:paddingVertical="16dp"
android:textColor="#000000"
android:textSize="20sp" />
"""
).indented()
)
.issues(DuplicateResourceFilesDetector.ISSUE)
.allowMissingSdk()
.run()
.expectCount(1, Severity.WARNING)
}

@Test
fun `Given 2 duplicate resources with different attributes that are under the tools namespace, a warning should be reported`() {
TestLintTask.lint()
.files(
xml(
Expand Down Expand Up @@ -173,11 +214,11 @@ class DuplicateResourceFilesDetectorTest {
}

@Test
fun `Given duplicate resources with different attributes order, an error should be reported`() {
fun `Given duplicate resources with different attributes order, a warning should be reported`() {
TestLintTask.lint()
.files(
xml(
"res/drawable/resource.xml",
"res/drawable/resource1.xml",
"""
<inset xmlns:android="http://schemas.android.com/apk/res/android"
android:insetTop="-1dp">
Expand All @@ -191,7 +232,6 @@ class DuplicateResourceFilesDetectorTest {
android:color="@color/black_10" />
</shape>
</inset>
"""
).indented(),
xml(
Expand Down Expand Up @@ -219,11 +259,11 @@ class DuplicateResourceFilesDetectorTest {
}

@Test
fun `Given duplicate shape resources, where one doesn't have the xml declaration, an error should be reported`() {
fun `Given duplicate shape resources, where one doesn't have the xml declaration, a warning should be reported`() {
TestLintTask.lint()
.files(
xml(
"res/drawable/shape.xml",
"res/drawable/shape1.xml",
"""
<?xml version="1.0" encoding="utf-8"?>
<shape xmlns:android="http://schemas.android.com/apk/res/android"
Expand Down

0 comments on commit 225957b

Please sign in to comment.