Skip to content

Commit 957947b

Browse files
Tnation87hfeky
Tnation87
and
hfeky
authored
feat: Ignore comment nodes in DuplicateResourceFilesDetector.kt (#22)
* feat: ignore comment nodes in DuplicateResourceFilesDetector.kt * update documentation in README.md * rename function * Update linta-android/src/test/java/com/swvl/lint/checks/DuplicateResourceFilesDetectorTest.kt Co-authored-by: Hussein El Feky <[email protected]> * insert comments inside if branches instead of outside * fix indentation of test files in DuplicateResourceFilesDetectorTest.kt * add braces to if statement * add comma before or in README.md * fix merge conflict issues * Update linta-android/src/main/java/com/swvl/lint/checks/DuplicateResourceFilesDetector.kt Co-authored-by: Hussein El Feky <[email protected]> * apply suggested comment Co-authored-by: Hussein El Feky <[email protected]>
1 parent 3f745bf commit 957947b

File tree

3 files changed

+96
-13
lines changed

3 files changed

+96
-13
lines changed

‎README.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,13 @@ lintChecks "com.swvl.lint:linta-android:x.y.z"
2727

2828
The following are the lint checks currently implemented by Linta [please add the documentation to any recent addition and/or any missing ones]:
2929

30-
| Lint Issue ID | Severity | Description |
31-
|:-----------------------:|:-------------:|--------------------------------------------------------------------------------------------------------|
32-
| `DuplicateColors` | warning | When a duplicate color is defined in a resource file |
33-
| `DuplicateResourceFiles`| warning | When there are two or more duplicate resource files containing the same exact attributes, regardless of differences in whitespaces, attributes order, or used tools namespace if any |
34-
| `HardcodedColorSrcCode` | error | When a hardcoded color is used in a source code file (Java or Kotlin) |
35-
| `HardcodedColorXml` | error | When a hardcoded color is used in a resource file (drawables, layouts, etc.) |
36-
| `RedundantStyles` | warning | When a style is created without adding any new attributes |
30+
| Lint Issue ID | Severity | Description |
31+
|:-----------------------:|:-------------:|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
32+
| `DuplicateColors` | warning | When a duplicate color is defined in a resource file |
33+
| `DuplicateResourceFiles`| warning | When there are two or more duplicate resource files containing the same exact attributes, regardless of differences in whitespaces, attributes order, comments, or used tools namespace if any |
34+
| `HardcodedColorSrcCode` | error | When a hardcoded color is used in a source code file (Java or Kotlin) |
35+
| `HardcodedColorXml` | error | When a hardcoded color is used in a resource file (drawables, layouts, etc.) |
36+
| `RedundantStyles` | warning | When a style is created without adding any new attributes |
3737

3838
To see how these checks work in action, check the [generated lint report](https://github.com/swvl/linta/tree/main/sample/build/reports/lint-results-release.html) in our [sample app](https://github.com/swvl/linta/tree/main/sample).
3939

‎linta-android/src/main/java/com/swvl/lint/checks/DuplicateResourceFilesDetector.kt

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ class DuplicateResourceFilesDetector : ResourceXmlDetector() {
5252
// instance is used in all other lint detectors.
5353
val documentClone = document.cloneNode(true)
5454

55-
removeToolsNamespaceAttributes(documentClone.firstChild ?: return)
55+
removeToolsNamespaceAttributesAndComments(documentClone.firstChild ?: return)
5656

5757
val stringWriter = StringWriter()
5858

@@ -72,9 +72,9 @@ class DuplicateResourceFilesDetector : ResourceXmlDetector() {
7272
}
7373
}
7474

75-
private fun removeToolsNamespaceAttributes(node: Node) {
76-
// Remove tools namespace and all attributes under it.
75+
private fun removeToolsNamespaceAttributesAndComments(node: Node) {
7776
if (node.nodeType == Element.ELEMENT_NODE) {
77+
// Remove tools namespace and all attributes under it.
7878
var i = 0
7979
while (i < node.attributes.length) {
8080
val attr = node.attributes.item(i)
@@ -84,13 +84,22 @@ class DuplicateResourceFilesDetector : ResourceXmlDetector() {
8484
}
8585
i++
8686
}
87+
} else if (node.nodeType == Element.COMMENT_NODE) {
88+
// Remove comment nodes.
89+
node.parentNode.removeChild(node)
8790
}
8891

8992
// Do the same with all children.
90-
val childrenCount = node.childNodes.length
91-
for (i in 0 until childrenCount) {
93+
var i = 0
94+
while (i < node.childNodes.length) {
9295
val child = node.childNodes.item(i)
93-
removeToolsNamespaceAttributes(child)
96+
removeToolsNamespaceAttributesAndComments(child)
97+
98+
// Only increase the counter if the node isn't a comment.
99+
if (child.nodeType == Element.COMMENT_NODE) {
100+
continue
101+
}
102+
i++
94103
}
95104
}
96105

‎linta-android/src/test/java/com/swvl/lint/checks/DuplicateResourceFilesDetectorTest.kt

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,80 @@ class DuplicateResourceFilesDetectorTest {
296296
.expectCount(1, Severity.WARNING)
297297
}
298298

299+
@Test
300+
fun `Given 2 duplicate layouts where one has comments and the other doesn't, a warning should be reported`() {
301+
TestLintTask.lint()
302+
.files(
303+
xml(
304+
"res/layout/layout1.xml",
305+
"""
306+
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
307+
android:layout_width="match_parent"
308+
android:layout_height="wrap_content"
309+
android:gravity="center|start"
310+
android:orientation="horizontal"
311+
android:paddingBottom="16dp">
312+
<!-- This is a button -->
313+
<Button
314+
android:id="@+id/addPassenger_button"
315+
style="@style/Widget.App.Button.Outlined.Primary"
316+
android:layout_width="wrap_content"
317+
android:layout_height="wrap_content"
318+
android:paddingStart="20dp"
319+
android:paddingEnd="20dp"
320+
android:text="@string/travel_bookingConfirmation_addPassenger_button_title" />
321+
322+
<!-- End of layout -->
323+
324+
<LinearLayout
325+
android:layout_width="match_parent"
326+
android:layout_height="wrap_content"
327+
android:gravity="center|start"
328+
android:orientation="horizontal">
329+
330+
<!-- Empty layout -->
331+
332+
</LinearLayout>
333+
334+
</LinearLayout>
335+
"""
336+
).indented(),
337+
xml(
338+
"res/layout/layout2.xml",
339+
"""
340+
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
341+
android:layout_width="match_parent"
342+
android:layout_height="wrap_content"
343+
android:gravity="center|start"
344+
android:orientation="horizontal"
345+
android:paddingBottom="16dp">
346+
347+
<Button
348+
android:id="@+id/addPassenger_button"
349+
style="@style/Widget.App.Button.Outlined.Primary"
350+
android:layout_width="wrap_content"
351+
android:layout_height="wrap_content"
352+
android:paddingStart="20dp"
353+
android:paddingEnd="20dp"
354+
android:text="@string/travel_bookingConfirmation_addPassenger_button_title" />
355+
356+
<LinearLayout
357+
android:layout_width="match_parent"
358+
android:layout_height="wrap_content"
359+
android:gravity="center|start"
360+
android:orientation="horizontal">
361+
</LinearLayout>
362+
363+
</LinearLayout>
364+
"""
365+
).indented()
366+
)
367+
.issues(DuplicateResourceFilesDetector.ISSUE)
368+
.allowMissingSdk()
369+
.run()
370+
.expectCount(1, Severity.WARNING)
371+
}
372+
299373
@Test
300374
fun `Given a resource with hardcoded colors but with the lint issue suppressed, when the duplicate resource files detector is run before the hardcoded color detector, no issue should be reported`() {
301375
TestLintTask.lint()

0 commit comments

Comments
 (0)