From e243e200e5d1fc2c172e03ff627d5ab6bc9f3210 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Tue, 17 Dec 2024 00:42:36 +0530 Subject: [PATCH 01/37] adding text view style check script --- scripts/BUILD.bazel | 7 +++ .../org/oppia/android/scripts/xml/BUILD.bazel | 11 ++++ .../android/scripts/xml/TextViewStyleCheck.kt | 58 +++++++++++++++++++ 3 files changed, 76 insertions(+) create mode 100644 scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index 9577dfe834b..ee4fb13c930 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -312,3 +312,10 @@ java_binary( main_class = "org.oppia.android.scripts.telemetry.DecodeUserStudyEventStringKt", runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/telemetry:decode_user_study_event_string_lib"], ) + +kt_jvm_binary( + name = "check_textview_styles", + testonly = True, + main_class = "org.oppia.android.scripts.xml.TextViewStyleCheckKt", + runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/xml:check_textview_styles"], +) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel b/scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel index 0657b000c49..6b29905f078 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel +++ b/scripts/src/java/org/oppia/android/scripts/xml/BUILD.bazel @@ -52,3 +52,14 @@ kt_jvm_library( "//scripts/src/java/org/oppia/android/scripts/common:repository_file", ], ) + +kt_jvm_library( + name = "check_textview_styles", + testonly = True, + srcs = ["TextViewStyleCheck.kt"], + visibility = ["//scripts:oppia_script_binary_visibility"], + deps = [ + ":xml_syntax_error_handler", + "//scripts/src/java/org/oppia/android/scripts/common:repository_file", + ], +) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt new file mode 100644 index 00000000000..299f9ffc506 --- /dev/null +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -0,0 +1,58 @@ +package org.oppia.android.scripts.xml + +import java.io.File +import javax.xml.parsers.DocumentBuilderFactory + +/** + * Script to ensure all TextView elements in layout XML files use centrally managed styles. + * + * Usage: + * bazel run //scripts:check_textview_styles -- + * + * Arguments: + * - path_to_repository_root: The root path of the repository. + * + * Example: + * bazel run //scripts:check_textview_styles -- $(pwd) + */ +fun main(vararg args: String) { + require(args.isNotEmpty()) { "Usage: bazel run //scripts:check_textview_styles -- " } + + val repoRoot = File(args[0]) + require(repoRoot.exists()) { "Repository root path does not exist: ${args[0]}" } + + val resDir = File(repoRoot, "app/src/main/res") + require(resDir.exists()) { "Resource directory does not exist: ${resDir.path}" } + + val layoutDirs = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } + ?: emptyArray() + val xmlFiles = layoutDirs.flatMap { dir -> + dir.walkTopDown().filter { file -> file.extension == "xml" }.toList() + } + + val builderFactory = DocumentBuilderFactory.newInstance() + val errors = mutableListOf() + + for (file in xmlFiles) { + val document = builderFactory.newDocumentBuilder().parse(file) + val textViewNodes = document.getElementsByTagName("TextView") + + for (i in 0 until textViewNodes.length) { + val node = textViewNodes.item(i) + val attributes = node.attributes + val styleAttribute = attributes?.getNamedItem("style")?.nodeValue + + if (styleAttribute == null || !styleAttribute.startsWith("@style/")) { + errors.add("${file.path}: TextView element is missing a centrally managed style.") + } + } + } + + if (errors.isNotEmpty()) { + println("TextView Style Check FAILED:") + errors.forEach { println(it) } + throw Exception("Some TextView elements do not have centrally managed styles.") + } else { + println("TextView Style Check PASSED.") + } +} From 00f7367cd4bb56cd03c755cb5f973e0a95db3cd1 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Tue, 17 Dec 2024 00:44:17 +0530 Subject: [PATCH 02/37] formatting --- .../java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 299f9ffc506..a9d014200d1 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -16,7 +16,10 @@ import javax.xml.parsers.DocumentBuilderFactory * bazel run //scripts:check_textview_styles -- $(pwd) */ fun main(vararg args: String) { - require(args.isNotEmpty()) { "Usage: bazel run //scripts:check_textview_styles -- " } + require(args.isNotEmpty()) { + "Usage: bazel run" + + " //scripts:check_textview_styles -- " + } val repoRoot = File(args[0]) require(repoRoot.exists()) { "Repository root path does not exist: ${args[0]}" } From 25e31c09d9a6848327ba2c40ebb21e1d89e98446 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Tue, 17 Dec 2024 01:03:30 +0530 Subject: [PATCH 03/37] error print fix --- .../src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index a9d014200d1..d17e2fa5c12 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -47,6 +47,7 @@ fun main(vararg args: String) { if (styleAttribute == null || !styleAttribute.startsWith("@style/")) { errors.add("${file.path}: TextView element is missing a centrally managed style.") + break } } } From 50155ab270557a42ac8dc74ee0100c1f32a2651b Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 8 Jan 2025 23:16:34 +0530 Subject: [PATCH 04/37] edge cases --- .../android/scripts/xml/TextViewStyleCheck.kt | 186 ++++++++++++++---- 1 file changed, 143 insertions(+), 43 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index d17e2fa5c12..e21cf479090 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -2,6 +2,7 @@ package org.oppia.android.scripts.xml import java.io.File import javax.xml.parsers.DocumentBuilderFactory +import org.w3c.dom.Element /** * Script to ensure all TextView elements in layout XML files use centrally managed styles. @@ -16,47 +17,146 @@ import javax.xml.parsers.DocumentBuilderFactory * bazel run //scripts:check_textview_styles -- $(pwd) */ fun main(vararg args: String) { - require(args.isNotEmpty()) { - "Usage: bazel run" + - " //scripts:check_textview_styles -- " - } - - val repoRoot = File(args[0]) - require(repoRoot.exists()) { "Repository root path does not exist: ${args[0]}" } - - val resDir = File(repoRoot, "app/src/main/res") - require(resDir.exists()) { "Resource directory does not exist: ${resDir.path}" } - - val layoutDirs = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } - ?: emptyArray() - val xmlFiles = layoutDirs.flatMap { dir -> - dir.walkTopDown().filter { file -> file.extension == "xml" }.toList() - } - - val builderFactory = DocumentBuilderFactory.newInstance() - val errors = mutableListOf() - - for (file in xmlFiles) { - val document = builderFactory.newDocumentBuilder().parse(file) - val textViewNodes = document.getElementsByTagName("TextView") - - for (i in 0 until textViewNodes.length) { - val node = textViewNodes.item(i) - val attributes = node.attributes - val styleAttribute = attributes?.getNamedItem("style")?.nodeValue - - if (styleAttribute == null || !styleAttribute.startsWith("@style/")) { - errors.add("${file.path}: TextView element is missing a centrally managed style.") - break - } - } - } - - if (errors.isNotEmpty()) { - println("TextView Style Check FAILED:") - errors.forEach { println(it) } - throw Exception("Some TextView elements do not have centrally managed styles.") - } else { - println("TextView Style Check PASSED.") - } + require(args.isNotEmpty()) { + "Usage: bazel run //scripts:check_textview_styles -- " + } + + val repoRoot = File(args[0]) + require(repoRoot.exists()) { "Repository root path does not exist: ${args[0]}" } + + val resDir = File(repoRoot, "app/src/main/res") + require(resDir.exists()) { "Resource directory does not exist: ${resDir.path}" } + + val xmlFiles = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } + ?.flatMap { dir -> dir.walkTopDown().filter { it.extension == "xml" } } + ?: emptyList() + + val styleChecker = TextViewStyleChecker() + styleChecker.checkFiles(xmlFiles) +} + +class TextViewStyleChecker { + private val errors = mutableListOf() + private val legacyDirectionalityWarnings = mutableListOf() + private val builderFactory = DocumentBuilderFactory.newInstance() + + fun checkFiles(xmlFiles: List) { + xmlFiles.forEach { file -> processXmlFile(file) } + printResults() + } + + private fun processXmlFile(file: File) { + val document = builderFactory.newDocumentBuilder().parse(file) + val textViewNodes = document.getElementsByTagName("TextView") + + for (i in 0 until textViewNodes.length) { + val element = textViewNodes.item(i) as Element + validateTextViewElement(element, file.path) + } + } + + private fun validateTextViewElement(element: Element, filePath: String) { + val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue + val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "No ID" + if (!isExemptFromStyleRequirement(element) && !isValidStyle(styleAttribute)) { + errors.add("$filePath: TextView ($idAttribute) requires central style.") + } + + checkForLegacyDirectionality(element, filePath) + } + + private fun isValidStyle(styleAttribute: String?) = + styleAttribute?.startsWith("@style/") == true + + private fun isExemptFromStyleRequirement(element: Element): Boolean { + if (element.getAttribute("android:gravity")?.contains("center") == true) return true + if (hasDynamicVisibility(element)) return true + if (hasSufficientStyling(element)) return true + if (isEmptyTextView(element)) return true + if (element.getAttribute("android:visibility") == "gone") return true + if (hasTextAlignmentAttributes(element)) return true + + return !hasDirectionalAttributes(element) + } + + private fun hasSufficientStyling(element: Element): Boolean { + val hasTextSize = element.hasAttribute("android:textSize") + val hasTextColor = element.hasAttribute("android:textColor") + val hasTextStyle = element.hasAttribute("android:textStyle") + val hasFontFamily = element.hasAttribute("android:fontFamily") + val hasDynamicAttributes = hasDynamicAttributes(element) + + return (hasTextSize && hasTextColor) || + (hasTextStyle && (hasTextSize || hasTextColor)) || + (hasFontFamily && (hasTextSize || hasTextColor)) || + (hasTextColor && hasDynamicAttributes) + } + + private fun isEmptyTextView(element: Element) = + element.getAttribute("android:text").isEmpty() && + element.getAttribute("android:hint").isEmpty() && + !element.hasChildNodes() + + private fun hasDynamicVisibility(element: Element) = + element.getAttribute("android:visibility").let { it.contains("{") && it.contains("}") } + + private fun hasDynamicAttributes(element: Element): Boolean { + for (i in 0 until element.attributes.length) { + val value = element.attributes.item(i).nodeValue + if (value.contains("{") && value.contains("}")) return true + } + return false + } + + private fun hasTextAlignmentAttributes(element: Element) = + element.getAttribute("android:textAlignment").isNotEmpty() || + element.attributes.getNamedItem("android:textAlignment")?.nodeValue?.contains("{") == true + + private fun hasDirectionalAttributes(element: Element): Boolean { + val directionAttributes = listOf( + "android:layout_alignParentStart", + "android:layout_alignParentEnd", + "android:layout_toStartOf", + "android:layout_toEndOf", + "android:paddingStart", + "android:paddingEnd", + "android:layout_marginStart", + "android:layout_marginEnd" + ) + return directionAttributes.any { element.hasAttribute(it) } + } + + private fun checkForLegacyDirectionality(element: Element, filePath: String) { + val legacyAttributes = listOf( + "android:paddingLeft", + "android:paddingRight", + "android:layout_marginLeft", + "android:layout_marginRight", + "android:layout_alignParentLeft", + "android:layout_alignParentRight", + "android:layout_toLeftOf", + "android:layout_toRightOf" + ) + + val foundLegacyAttributes = legacyAttributes.filter { element.hasAttribute(it) } + if (foundLegacyAttributes.isNotEmpty()) { + legacyDirectionalityWarnings.add("$filePath: TextView uses legacy"+ + " directional attributes: ${foundLegacyAttributes.joinToString(", ")}") + } + } + + private fun printResults() { + if (legacyDirectionalityWarnings.isNotEmpty()) { + println("\nWarnings - Legacy directionality attributes found:") + legacyDirectionalityWarnings.forEach { println(it) } + } + + if (errors.isNotEmpty()) { + println("\nTextView Style Check FAILED:") + errors.forEach { println(it) } + throw Exception("Some TextView elements do not have centrally managed styles.") + } else { + println("\nTextView Style Check PASSED.") + } + } } From 781d682ee91c947daeb5cee5aded89548efc3ed5 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 8 Jan 2025 23:17:23 +0530 Subject: [PATCH 05/37] formatting --- .../android/scripts/xml/TextViewStyleCheck.kt | 252 +++++++++--------- 1 file changed, 127 insertions(+), 125 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index e21cf479090..027a8bbb06e 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -1,8 +1,8 @@ package org.oppia.android.scripts.xml +import org.w3c.dom.Element import java.io.File import javax.xml.parsers.DocumentBuilderFactory -import org.w3c.dom.Element /** * Script to ensure all TextView elements in layout XML files use centrally managed styles. @@ -17,146 +17,148 @@ import org.w3c.dom.Element * bazel run //scripts:check_textview_styles -- $(pwd) */ fun main(vararg args: String) { - require(args.isNotEmpty()) { + require(args.isNotEmpty()) { "Usage: bazel run //scripts:check_textview_styles -- " - } + } - val repoRoot = File(args[0]) - require(repoRoot.exists()) { "Repository root path does not exist: ${args[0]}" } + val repoRoot = File(args[0]) + require(repoRoot.exists()) { "Repository root path does not exist: ${args[0]}" } - val resDir = File(repoRoot, "app/src/main/res") - require(resDir.exists()) { "Resource directory does not exist: ${resDir.path}" } + val resDir = File(repoRoot, "app/src/main/res") + require(resDir.exists()) { "Resource directory does not exist: ${resDir.path}" } - val xmlFiles = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } - ?.flatMap { dir -> dir.walkTopDown().filter { it.extension == "xml" } } - ?: emptyList() + val xmlFiles = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } + ?.flatMap { dir -> dir.walkTopDown().filter { it.extension == "xml" } } + ?: emptyList() - val styleChecker = TextViewStyleChecker() - styleChecker.checkFiles(xmlFiles) + val styleChecker = TextViewStyleChecker() + styleChecker.checkFiles(xmlFiles) } class TextViewStyleChecker { - private val errors = mutableListOf() - private val legacyDirectionalityWarnings = mutableListOf() - private val builderFactory = DocumentBuilderFactory.newInstance() - - fun checkFiles(xmlFiles: List) { - xmlFiles.forEach { file -> processXmlFile(file) } - printResults() - } - - private fun processXmlFile(file: File) { - val document = builderFactory.newDocumentBuilder().parse(file) - val textViewNodes = document.getElementsByTagName("TextView") - - for (i in 0 until textViewNodes.length) { - val element = textViewNodes.item(i) as Element - validateTextViewElement(element, file.path) - } - } - - private fun validateTextViewElement(element: Element, filePath: String) { - val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue - val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "No ID" - if (!isExemptFromStyleRequirement(element) && !isValidStyle(styleAttribute)) { - errors.add("$filePath: TextView ($idAttribute) requires central style.") - } - - checkForLegacyDirectionality(element, filePath) - } - - private fun isValidStyle(styleAttribute: String?) = - styleAttribute?.startsWith("@style/") == true - - private fun isExemptFromStyleRequirement(element: Element): Boolean { - if (element.getAttribute("android:gravity")?.contains("center") == true) return true - if (hasDynamicVisibility(element)) return true - if (hasSufficientStyling(element)) return true - if (isEmptyTextView(element)) return true - if (element.getAttribute("android:visibility") == "gone") return true - if (hasTextAlignmentAttributes(element)) return true - - return !hasDirectionalAttributes(element) + private val errors = mutableListOf() + private val legacyDirectionalityWarnings = mutableListOf() + private val builderFactory = DocumentBuilderFactory.newInstance() + + fun checkFiles(xmlFiles: List) { + xmlFiles.forEach { file -> processXmlFile(file) } + printResults() + } + + private fun processXmlFile(file: File) { + val document = builderFactory.newDocumentBuilder().parse(file) + val textViewNodes = document.getElementsByTagName("TextView") + + for (i in 0 until textViewNodes.length) { + val element = textViewNodes.item(i) as Element + validateTextViewElement(element, file.path) } + } - private fun hasSufficientStyling(element: Element): Boolean { - val hasTextSize = element.hasAttribute("android:textSize") - val hasTextColor = element.hasAttribute("android:textColor") - val hasTextStyle = element.hasAttribute("android:textStyle") - val hasFontFamily = element.hasAttribute("android:fontFamily") - val hasDynamicAttributes = hasDynamicAttributes(element) - - return (hasTextSize && hasTextColor) || - (hasTextStyle && (hasTextSize || hasTextColor)) || - (hasFontFamily && (hasTextSize || hasTextColor)) || - (hasTextColor && hasDynamicAttributes) + private fun validateTextViewElement(element: Element, filePath: String) { + val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue + val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "No ID" + if (!isExemptFromStyleRequirement(element) && !isValidStyle(styleAttribute)) { + errors.add("$filePath: TextView ($idAttribute) requires central style.") } - private fun isEmptyTextView(element: Element) = - element.getAttribute("android:text").isEmpty() && - element.getAttribute("android:hint").isEmpty() && - !element.hasChildNodes() - - private fun hasDynamicVisibility(element: Element) = - element.getAttribute("android:visibility").let { it.contains("{") && it.contains("}") } - - private fun hasDynamicAttributes(element: Element): Boolean { - for (i in 0 until element.attributes.length) { - val value = element.attributes.item(i).nodeValue - if (value.contains("{") && value.contains("}")) return true - } - return false + checkForLegacyDirectionality(element, filePath) + } + + private fun isValidStyle(styleAttribute: String?) = + styleAttribute?.startsWith("@style/") == true + + private fun isExemptFromStyleRequirement(element: Element): Boolean { + if (element.getAttribute("android:gravity")?.contains("center") == true) return true + if (hasDynamicVisibility(element)) return true + if (hasSufficientStyling(element)) return true + if (isEmptyTextView(element)) return true + if (element.getAttribute("android:visibility") == "gone") return true + if (hasTextAlignmentAttributes(element)) return true + + return !hasDirectionalAttributes(element) + } + + private fun hasSufficientStyling(element: Element): Boolean { + val hasTextSize = element.hasAttribute("android:textSize") + val hasTextColor = element.hasAttribute("android:textColor") + val hasTextStyle = element.hasAttribute("android:textStyle") + val hasFontFamily = element.hasAttribute("android:fontFamily") + val hasDynamicAttributes = hasDynamicAttributes(element) + + return (hasTextSize && hasTextColor) || + (hasTextStyle && (hasTextSize || hasTextColor)) || + (hasFontFamily && (hasTextSize || hasTextColor)) || + (hasTextColor && hasDynamicAttributes) + } + + private fun isEmptyTextView(element: Element) = + element.getAttribute("android:text").isEmpty() && + element.getAttribute("android:hint").isEmpty() && + !element.hasChildNodes() + + private fun hasDynamicVisibility(element: Element) = + element.getAttribute("android:visibility").let { it.contains("{") && it.contains("}") } + + private fun hasDynamicAttributes(element: Element): Boolean { + for (i in 0 until element.attributes.length) { + val value = element.attributes.item(i).nodeValue + if (value.contains("{") && value.contains("}")) return true } - - private fun hasTextAlignmentAttributes(element: Element) = - element.getAttribute("android:textAlignment").isNotEmpty() || - element.attributes.getNamedItem("android:textAlignment")?.nodeValue?.contains("{") == true - - private fun hasDirectionalAttributes(element: Element): Boolean { - val directionAttributes = listOf( - "android:layout_alignParentStart", - "android:layout_alignParentEnd", - "android:layout_toStartOf", - "android:layout_toEndOf", - "android:paddingStart", - "android:paddingEnd", - "android:layout_marginStart", - "android:layout_marginEnd" - ) - return directionAttributes.any { element.hasAttribute(it) } + return false + } + + private fun hasTextAlignmentAttributes(element: Element) = + element.getAttribute("android:textAlignment").isNotEmpty() || + element.attributes.getNamedItem("android:textAlignment")?.nodeValue?.contains("{") == true + + private fun hasDirectionalAttributes(element: Element): Boolean { + val directionAttributes = listOf( + "android:layout_alignParentStart", + "android:layout_alignParentEnd", + "android:layout_toStartOf", + "android:layout_toEndOf", + "android:paddingStart", + "android:paddingEnd", + "android:layout_marginStart", + "android:layout_marginEnd" + ) + return directionAttributes.any { element.hasAttribute(it) } + } + + private fun checkForLegacyDirectionality(element: Element, filePath: String) { + val legacyAttributes = listOf( + "android:paddingLeft", + "android:paddingRight", + "android:layout_marginLeft", + "android:layout_marginRight", + "android:layout_alignParentLeft", + "android:layout_alignParentRight", + "android:layout_toLeftOf", + "android:layout_toRightOf" + ) + + val foundLegacyAttributes = legacyAttributes.filter { element.hasAttribute(it) } + if (foundLegacyAttributes.isNotEmpty()) { + legacyDirectionalityWarnings.add( + "$filePath: TextView uses legacy" + + " directional attributes: ${foundLegacyAttributes.joinToString(", ")}" + ) } + } - private fun checkForLegacyDirectionality(element: Element, filePath: String) { - val legacyAttributes = listOf( - "android:paddingLeft", - "android:paddingRight", - "android:layout_marginLeft", - "android:layout_marginRight", - "android:layout_alignParentLeft", - "android:layout_alignParentRight", - "android:layout_toLeftOf", - "android:layout_toRightOf" - ) - - val foundLegacyAttributes = legacyAttributes.filter { element.hasAttribute(it) } - if (foundLegacyAttributes.isNotEmpty()) { - legacyDirectionalityWarnings.add("$filePath: TextView uses legacy"+ - " directional attributes: ${foundLegacyAttributes.joinToString(", ")}") - } + private fun printResults() { + if (legacyDirectionalityWarnings.isNotEmpty()) { + println("\nWarnings - Legacy directionality attributes found:") + legacyDirectionalityWarnings.forEach { println(it) } } - private fun printResults() { - if (legacyDirectionalityWarnings.isNotEmpty()) { - println("\nWarnings - Legacy directionality attributes found:") - legacyDirectionalityWarnings.forEach { println(it) } - } - - if (errors.isNotEmpty()) { - println("\nTextView Style Check FAILED:") - errors.forEach { println(it) } - throw Exception("Some TextView elements do not have centrally managed styles.") - } else { - println("\nTextView Style Check PASSED.") - } + if (errors.isNotEmpty()) { + println("\nTextView Style Check FAILED:") + errors.forEach { println(it) } + throw Exception("Some TextView elements do not have centrally managed styles.") + } else { + println("\nTextView Style Check PASSED.") } + } } From 8b1f90f487dcc4db41c91353d070e13a61095616 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 8 Jan 2025 23:31:47 +0530 Subject: [PATCH 06/37] class fix --- .../java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 027a8bbb06e..e35a40a322b 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -31,11 +31,11 @@ fun main(vararg args: String) { ?.flatMap { dir -> dir.walkTopDown().filter { it.extension == "xml" } } ?: emptyList() - val styleChecker = TextViewStyleChecker() + val styleChecker = TextViewStyleCheck() styleChecker.checkFiles(xmlFiles) } -class TextViewStyleChecker { +private class TextViewStyleCheck { private val errors = mutableListOf() private val legacyDirectionalityWarnings = mutableListOf() private val builderFactory = DocumentBuilderFactory.newInstance() From b5155bd55a93a217364c63749af14cd8f9ec1297 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Thu, 9 Jan 2025 00:13:44 +0530 Subject: [PATCH 07/37] removing redundant checks --- .../android/scripts/xml/TextViewStyleCheck.kt | 40 +------------------ 1 file changed, 2 insertions(+), 38 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index e35a40a322b..380116a1283 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -58,60 +58,24 @@ private class TextViewStyleCheck { private fun validateTextViewElement(element: Element, filePath: String) { val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "No ID" - if (!isExemptFromStyleRequirement(element) && !isValidStyle(styleAttribute)) { + if (!isExemptFromStyleRequirement(element) && !(styleAttribute?.startsWith("@style/")==true)) { errors.add("$filePath: TextView ($idAttribute) requires central style.") } checkForLegacyDirectionality(element, filePath) } - private fun isValidStyle(styleAttribute: String?) = - styleAttribute?.startsWith("@style/") == true - private fun isExemptFromStyleRequirement(element: Element): Boolean { if (element.getAttribute("android:gravity")?.contains("center") == true) return true if (hasDynamicVisibility(element)) return true - if (hasSufficientStyling(element)) return true - if (isEmptyTextView(element)) return true - if (element.getAttribute("android:visibility") == "gone") return true - if (hasTextAlignmentAttributes(element)) return true + if (element.hasAttribute("android:textSize")) return true return !hasDirectionalAttributes(element) } - private fun hasSufficientStyling(element: Element): Boolean { - val hasTextSize = element.hasAttribute("android:textSize") - val hasTextColor = element.hasAttribute("android:textColor") - val hasTextStyle = element.hasAttribute("android:textStyle") - val hasFontFamily = element.hasAttribute("android:fontFamily") - val hasDynamicAttributes = hasDynamicAttributes(element) - - return (hasTextSize && hasTextColor) || - (hasTextStyle && (hasTextSize || hasTextColor)) || - (hasFontFamily && (hasTextSize || hasTextColor)) || - (hasTextColor && hasDynamicAttributes) - } - - private fun isEmptyTextView(element: Element) = - element.getAttribute("android:text").isEmpty() && - element.getAttribute("android:hint").isEmpty() && - !element.hasChildNodes() - private fun hasDynamicVisibility(element: Element) = element.getAttribute("android:visibility").let { it.contains("{") && it.contains("}") } - private fun hasDynamicAttributes(element: Element): Boolean { - for (i in 0 until element.attributes.length) { - val value = element.attributes.item(i).nodeValue - if (value.contains("{") && value.contains("}")) return true - } - return false - } - - private fun hasTextAlignmentAttributes(element: Element) = - element.getAttribute("android:textAlignment").isNotEmpty() || - element.attributes.getNamedItem("android:textAlignment")?.nodeValue?.contains("{") == true - private fun hasDirectionalAttributes(element: Element): Boolean { val directionAttributes = listOf( "android:layout_alignParentStart", From 0631ec72d1e192d47467d9375234e022fa4d06e0 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Thu, 9 Jan 2025 00:47:45 +0530 Subject: [PATCH 08/37] checking style --- .../android/scripts/xml/TextViewStyleCheck.kt | 53 +++++++++++++++++-- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 380116a1283..d96b72a8831 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -31,14 +31,27 @@ fun main(vararg args: String) { ?.flatMap { dir -> dir.walkTopDown().filter { it.extension == "xml" } } ?: emptyList() - val styleChecker = TextViewStyleCheck() + val styleChecker = TextViewStyleCheck(repoRoot) styleChecker.checkFiles(xmlFiles) } -private class TextViewStyleCheck { +private class TextViewStyleCheck(private val repoRoot: File) { private val errors = mutableListOf() private val legacyDirectionalityWarnings = mutableListOf() private val builderFactory = DocumentBuilderFactory.newInstance() + private val styles: Map by lazy { loadStyles() } + + private fun loadStyles(): Map { + val stylesFile = File(repoRoot, "app/src/main/res/values/styles.xml") + require(stylesFile.exists()) { "Styles file does not exist: ${stylesFile.path}" } + + val document = builderFactory.newDocumentBuilder().parse(stylesFile) + val styleNodes = document.getElementsByTagName("style") + return (0 until styleNodes.length).associate { i -> + val element = styleNodes.item(i) as Element + element.getAttribute("name") to element + } + } fun checkFiles(xmlFiles: List) { xmlFiles.forEach { file -> processXmlFile(file) } @@ -58,13 +71,45 @@ private class TextViewStyleCheck { private fun validateTextViewElement(element: Element, filePath: String) { val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "No ID" - if (!isExemptFromStyleRequirement(element) && !(styleAttribute?.startsWith("@style/")==true)) { - errors.add("$filePath: TextView ($idAttribute) requires central style.") + + if (!isExemptFromStyleRequirement(element)) { + if (styleAttribute?.startsWith("@style/") == true) { + validateStyle(styleAttribute, idAttribute, filePath) + } else { + errors.add("$filePath: TextView ($idAttribute) requires central style.") + } } checkForLegacyDirectionality(element, filePath) } + private fun validateStyle(styleAttribute: String, idAttribute: String, filePath: String) { + val styleName = styleAttribute.removePrefix("@style/") + val styleElement = styles[styleName] ?: run { + errors.add("$filePath: TextView ($idAttribute) references non-existent style: $styleName") + return + } + + val items = styleElement.getElementsByTagName("item") + val hasRtlProperties = (0 until items.length).any { i -> + val item = items.item(i) as Element + when (item.getAttribute("name")) { + "android:textAlignment", + "android:gravity", + "android:layoutDirection", + "android:textDirection", + "android:textSize" -> true + else -> false + } + } + + if (!hasRtlProperties) { + errors.add( + "$filePath: TextView ($idAttribute) style '$styleName' lacks RTL/LTR properties" + ) + } + } + private fun isExemptFromStyleRequirement(element: Element): Boolean { if (element.getAttribute("android:gravity")?.contains("center") == true) return true if (hasDynamicVisibility(element)) return true From f104b96c78e665f69cb68faaf81091906100e835 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Thu, 9 Jan 2025 00:58:56 +0530 Subject: [PATCH 09/37] kdoc fix --- .../org/oppia/android/scripts/xml/TextViewStyleCheck.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index d96b72a8831..dc719c9b0ef 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -52,7 +52,7 @@ private class TextViewStyleCheck(private val repoRoot: File) { element.getAttribute("name") to element } } - + /** Checks XML files for TextView elements to ensure compliance with style requirements. */ fun checkFiles(xmlFiles: List) { xmlFiles.forEach { file -> processXmlFile(file) } printResults() @@ -82,7 +82,7 @@ private class TextViewStyleCheck(private val repoRoot: File) { checkForLegacyDirectionality(element, filePath) } - + // Validate if the referenced style exists and contains necessary RTL/LTR properties. private fun validateStyle(styleAttribute: String, idAttribute: String, filePath: String) { val styleName = styleAttribute.removePrefix("@style/") val styleElement = styles[styleName] ?: run { @@ -109,7 +109,7 @@ private class TextViewStyleCheck(private val repoRoot: File) { ) } } - + // Determines if a TextView is exempt from requiring a centrally managed style. private fun isExemptFromStyleRequirement(element: Element): Boolean { if (element.getAttribute("android:gravity")?.contains("center") == true) return true if (hasDynamicVisibility(element)) return true From 8bd90bfdfc368d22d8ec93acd41edd0dc83bfe45 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Fri, 10 Jan 2025 17:52:26 +0530 Subject: [PATCH 10/37] logging line no. and CI integration --- .github/workflows/static_checks.yml | 7 + .../android/scripts/xml/TextViewStyleCheck.kt | 124 ++++++++++++++---- scripts/static_checks.sh | 7 + 3 files changed, 111 insertions(+), 27 deletions(-) diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index a943076f7f6..18c7ec28417 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -170,6 +170,13 @@ jobs: run: | bazel run //scripts:xml_syntax_check -- $(pwd) + - name: TextView Style Validation Check + # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, + # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. + if: ${{ !cancelled() }} + run: | + bazel run //scripts:check_textview_styles -- $(pwd) + - name: Testfile Presence Check # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index dc719c9b0ef..2de85245ee8 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -1,8 +1,14 @@ package org.oppia.android.scripts.xml +import org.w3c.dom.Document import org.w3c.dom.Element +import org.xml.sax.Attributes +import org.xml.sax.InputSource +import org.xml.sax.Locator +import org.xml.sax.helpers.DefaultHandler import java.io.File import javax.xml.parsers.DocumentBuilderFactory +import javax.xml.parsers.SAXParserFactory /** * Script to ensure all TextView elements in layout XML files use centrally managed styles. @@ -35,9 +41,15 @@ fun main(vararg args: String) { styleChecker.checkFiles(xmlFiles) } +private data class StyleViolation( + val filePath: String, + val lineNumber: Int, + val message: String +) + private class TextViewStyleCheck(private val repoRoot: File) { - private val errors = mutableListOf() - private val legacyDirectionalityWarnings = mutableListOf() + private val styleViolations = mutableListOf() + private val legacyDirectionalityWarnings = mutableListOf() private val builderFactory = DocumentBuilderFactory.newInstance() private val styles: Map by lazy { loadStyles() } @@ -59,34 +71,81 @@ private class TextViewStyleCheck(private val repoRoot: File) { } private fun processXmlFile(file: File) { - val document = builderFactory.newDocumentBuilder().parse(file) - val textViewNodes = document.getElementsByTagName("TextView") + val handler = TextViewLocationHandler(file.path) { element, lineNumber -> + validateTextViewElement(element, file.path, lineNumber) + } - for (i in 0 until textViewNodes.length) { - val element = textViewNodes.item(i) as Element - validateTextViewElement(element, file.path) + val parser = SAXParserFactory.newInstance().newSAXParser() + parser.parse(file, handler) + } + + private class TextViewLocationHandler( + private val filePath: String, + private val onTextViewFound: (Element, Int) -> Unit + ) : DefaultHandler() { + private val document: Document = DocumentBuilderFactory.newInstance() + .newDocumentBuilder() + .newDocument() + + override fun startElement( + uri: String, + localName: String, + qName: String, + attributes: Attributes + ) { + if (qName == "TextView") { + // Get the actual content of the file to verify the line number + val fileContent = File(filePath).readLines() + + // Find the actual line number by searching for the TextView tag + val actualLine = fileContent.indexOfFirst { line -> + line.trim().startsWith(" + println("WARNING: ${violation.message} in file: ${violation.filePath}," + + " line ${violation.lineNumber}") + } } - if (errors.isNotEmpty()) { - println("\nTextView Style Check FAILED:") - errors.forEach { println(it) } - throw Exception("Some TextView elements do not have centrally managed styles.") + if (styleViolations.isNotEmpty()) { + styleViolations.forEach { violation -> + println("ERROR: ${violation.message} in file: ${violation.filePath}," + + " line ${violation.lineNumber}") + } + throw Exception("TEXTVIEW STYLE CHECK FAILED") } else { - println("\nTextView Style Check PASSED.") + println("TEXTVIEW STYLE CHECK PASSED.") } } } diff --git a/scripts/static_checks.sh b/scripts/static_checks.sh index 0a0d08206cb..3d83fdd6593 100644 --- a/scripts/static_checks.sh +++ b/scripts/static_checks.sh @@ -58,6 +58,13 @@ echo "********************************" bazel run //scripts:xml_syntax_check -- $(pwd) echo "" +# Run TextView Style check +echo "********************************" +echo "Running TextView style validation checks" +echo "********************************" +bazel run //scripts:check_textview_styles -- $(pwd) +echo "" + # Run Testfile Presence Check echo "********************************" echo "Running Testfile presence checks" From 9e530aac17d05942b6e162b1ac011ee1762ca36a Mon Sep 17 00:00:00 2001 From: manas-yu Date: Fri, 10 Jan 2025 17:54:15 +0530 Subject: [PATCH 11/37] formatting --- .../android/scripts/xml/TextViewStyleCheck.kt | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 2de85245ee8..5af78ecba64 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -3,8 +3,6 @@ package org.oppia.android.scripts.xml import org.w3c.dom.Document import org.w3c.dom.Element import org.xml.sax.Attributes -import org.xml.sax.InputSource -import org.xml.sax.Locator import org.xml.sax.helpers.DefaultHandler import java.io.File import javax.xml.parsers.DocumentBuilderFactory @@ -225,15 +223,19 @@ private class TextViewStyleCheck(private val repoRoot: File) { private fun printResults() { if (legacyDirectionalityWarnings.isNotEmpty()) { legacyDirectionalityWarnings.forEach { violation -> - println("WARNING: ${violation.message} in file: ${violation.filePath}," + - " line ${violation.lineNumber}") + println( + "WARNING: ${violation.message} in file: ${violation.filePath}," + + " line ${violation.lineNumber}" + ) } } if (styleViolations.isNotEmpty()) { styleViolations.forEach { violation -> - println("ERROR: ${violation.message} in file: ${violation.filePath}," + - " line ${violation.lineNumber}") + println( + "ERROR: ${violation.message} in file: ${violation.filePath}," + + " line ${violation.lineNumber}" + ) } throw Exception("TEXTVIEW STYLE CHECK FAILED") } else { From 0510d7def4e3bc6ea1bdda601f3bb23a24947040 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Fri, 10 Jan 2025 23:04:59 +0530 Subject: [PATCH 12/37] test file --- .../android/scripts/xml/TextViewStyleCheck.kt | 2 +- .../org/oppia/android/scripts/xml/BUILD.bazel | 10 + .../scripts/xml/TextViewStyleCheckTest.kt | 236 ++++++++++++++++++ 3 files changed, 247 insertions(+), 1 deletion(-) create mode 100644 scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 5af78ecba64..b3f80008c1a 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -239,7 +239,7 @@ private class TextViewStyleCheck(private val repoRoot: File) { } throw Exception("TEXTVIEW STYLE CHECK FAILED") } else { - println("TEXTVIEW STYLE CHECK PASSED.") + println("TEXTVIEW STYLE CHECK PASSED") } } } diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel index 1b2a3d64a69..da4676beb55 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel @@ -58,3 +58,13 @@ kt_jvm_test( "//third_party:org_jetbrains_kotlin_kotlin-test-junit", ], ) + +kt_jvm_test( + name = "TextViewStyleCheckTest", + srcs = ["TextViewStyleCheckTest.kt"], + deps = [ + "//scripts/src/java/org/oppia/android/scripts/xml:check_textview_styles", + "//third_party:com_google_truth_truth", + "//third_party:org_jetbrains_kotlin_kotlin-test-junit", + ], +) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt new file mode 100644 index 00000000000..c10654e2a93 --- /dev/null +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -0,0 +1,236 @@ +package org.oppia.android.scripts.xml + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import java.io.ByteArrayOutputStream +import java.io.PrintStream + +/** Tests for [TextViewStyleCheck]. */ +class TextViewStyleCheckTest { + private val outContent: ByteArrayOutputStream = ByteArrayOutputStream() + private val originalOut: PrintStream = System.out + private val TEXTVIEW_STYLE_CHECK_PASSED_OUTPUT_INDICATOR = "TEXTVIEW STYLE CHECK PASSED" + private val TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR = "TEXTVIEW STYLE CHECK FAILED" + + @field:[Rule JvmField] val tempFolder = TemporaryFolder() + + @Before + fun setUp() { + tempFolder.newFolder("app", "src", "main", "res") + tempFolder.newFolder("app/src/main/res/layout") + tempFolder.newFolder("app/src/main/res/values") + System.setOut(PrintStream(outContent)) + } + + @After + fun restoreStreams() { + System.setOut(originalOut) + } + + @Test + fun testTextViewStyle_validStyleAttribute_checksPass() { + val validStyle = + """ + + + + + """.trimIndent() + + val validLayout = + """ + + + + + """.trimIndent() + + createStylesFile(validStyle) + createLayoutFile(validLayout) + + runScript() + + assertThat(outContent.toString().trim()).isEqualTo(TEXTVIEW_STYLE_CHECK_PASSED_OUTPUT_INDICATOR) + } + + @Test + fun testTextViewStyle_missingStyleAttribute_checksFail() { + val validStyle = + """ + + + + + """.trimIndent() + + val invalidLayout = + """ + + + + + """.trimIndent() + + createStylesFile(validStyle) + createLayoutFile(invalidLayout) + + val thrown = kotlin.runCatching { runScript() }.exceptionOrNull() + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(thrown).hasMessageThat().contains("TEXTVIEW STYLE CHECK FAILED") + assertThat(outContent.toString()).contains("ERROR: Missing style attribute") + assertThat(outContent.toString()).contains("line 5") + } + + @Test + fun testTextViewStyle_nonExistentStyle_checksFail() { + val validStyle = + """ + + + + + """.trimIndent() + + val invalidLayout = + """ + + + + + """.trimIndent() + + createStylesFile(validStyle) + createLayoutFile(invalidLayout) + + val thrown = kotlin.runCatching { runScript() }.exceptionOrNull() + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(thrown).hasMessageThat().contains("TEXTVIEW STYLE CHECK FAILED") + assertThat(outContent.toString()).contains( + "ERROR:" + + " References non-existent style: NonExistentStyle" + ) + assertThat(outContent.toString()).contains("line 5") + } + + @Test + fun testTextViewStyle_styleWithoutRtlProperties_checksFail() { + val invalidStyle = + """ + + + + + """.trimIndent() + + val layout = + """ + + + + + """.trimIndent() + + createStylesFile(invalidStyle) + createLayoutFile(layout) + + val thrown = kotlin.runCatching { runScript() }.exceptionOrNull() + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(thrown).hasMessageThat().contains("TEXTVIEW STYLE CHECK FAILED") + assertThat(outContent.toString()).contains( + "ERROR: Style 'InvalidTextViewStyle'" + + " lacks RTL/LTR properties in file" + ) + assertThat(outContent.toString()).contains("line 5") + } + + @Test + fun testTextViewStyle_legacyDirectionalityAttributes_showsWarning() { + val validStyle = + """ + + + + + """.trimIndent() + + val layoutWithLegacyAttributes = + """ + + + + + """.trimIndent() + + createStylesFile(validStyle) + createLayoutFile(layoutWithLegacyAttributes) + + runScript() + + assertThat(outContent.toString()).contains( + "WARNING: Uses legacy directional attributes: android:paddingLeft, android:layout_marginRight" + ) + } + + private fun createStylesFile(content: String) { + val stylesFile = tempFolder.newFile("app/src/main/res/values/styles.xml") + stylesFile.writeText(content) + } + + private fun createLayoutFile(content: String) { + val layoutFile = tempFolder.newFile("app/src/main/res/layout/test_layout.xml") + layoutFile.writeText(content) + } + + private fun runScript() { + main(tempFolder.root.absolutePath) + } +} From ec4c12daa33f04e75daf9d29160c499abbd0f744 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Fri, 10 Jan 2025 23:29:01 +0530 Subject: [PATCH 13/37] bazel setup --- scripts/BUILD.bazel | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index ee4fb13c930..171481b2b53 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -102,6 +102,13 @@ kt_jvm_binary( runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/xml:xml_syntax_check_lib"], ) +kt_jvm_binary( + name = "check_textview_styles", + testonly = True, + main_class = "org.oppia.android.scripts.xml.TextViewStyleCheckTest", + runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/xml:check_textview_styles"], +) + kt_jvm_binary( name = "string_language_translation_check", testonly = True, From 67ab29036ef479993ef993d120ba46ce4c24c0a4 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Fri, 10 Jan 2025 23:38:38 +0530 Subject: [PATCH 14/37] revert --- scripts/BUILD.bazel | 7 ------- 1 file changed, 7 deletions(-) diff --git a/scripts/BUILD.bazel b/scripts/BUILD.bazel index 171481b2b53..ee4fb13c930 100644 --- a/scripts/BUILD.bazel +++ b/scripts/BUILD.bazel @@ -102,13 +102,6 @@ kt_jvm_binary( runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/xml:xml_syntax_check_lib"], ) -kt_jvm_binary( - name = "check_textview_styles", - testonly = True, - main_class = "org.oppia.android.scripts.xml.TextViewStyleCheckTest", - runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/xml:check_textview_styles"], -) - kt_jvm_binary( name = "string_language_translation_check", testonly = True, From afcb7084223ae05534017205542ebf4fb9046bf2 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Sat, 11 Jan 2025 03:25:06 +0530 Subject: [PATCH 15/37] avoid redundant RTL/LTR settings checks --- .../android/scripts/xml/TextViewStyleCheck.kt | 42 +++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index b3f80008c1a..8846fd27c58 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -50,6 +50,7 @@ private class TextViewStyleCheck(private val repoRoot: File) { private val legacyDirectionalityWarnings = mutableListOf() private val builderFactory = DocumentBuilderFactory.newInstance() private val styles: Map by lazy { loadStyles() } + private val validatedStyles = mutableMapOf() private fun loadStyles(): Map { val stylesFile = File(repoRoot, "app/src/main/res/values/styles.xml") @@ -136,6 +137,18 @@ private class TextViewStyleCheck(private val repoRoot: File) { private fun validateStyle(styleAttribute: String, filePath: String, lineNumber: Int) { val styleName = styleAttribute.removePrefix("@style/") + validatedStyles[styleName]?.let { isValid -> + if (!isValid) { + styleViolations.add( + StyleViolation( + filePath, + lineNumber, + "References style with missing RTL/LTR properties: $styleName" + ) + ) + } + return + } val styleElement = styles[styleName] ?: run { styleViolations.add( StyleViolation( @@ -147,18 +160,8 @@ private class TextViewStyleCheck(private val repoRoot: File) { return } - val items = styleElement.getElementsByTagName("item") - val hasRtlProperties = (0 until items.length).any { i -> - val item = items.item(i) as Element - when (item.getAttribute("name")) { - "android:textAlignment", - "android:gravity", - "android:layoutDirection", - "android:textDirection", - "android:textSize" -> true - else -> false - } - } + val hasRtlProperties = validateStyleProperties(styleElement) + validatedStyles[styleName] = hasRtlProperties if (!hasRtlProperties) { styleViolations.add( @@ -170,6 +173,21 @@ private class TextViewStyleCheck(private val repoRoot: File) { ) } } + private fun validateStyleProperties(styleElement: Element): Boolean { + val rtlProperties = setOf( + "android:textAlignment", + "android:gravity", + "android:layoutDirection", + "android:textDirection", + "android:textSize" + ) + + val items = styleElement.getElementsByTagName("item") + return (0 until items.length).any { i -> + val item = items.item(i) as Element + item.getAttribute("name") in rtlProperties + } + } // Determines if a TextView is exempt from requiring a centrally managed style. private fun isExemptFromStyleRequirement(element: Element): Boolean { if (element.getAttribute("android:gravity")?.contains("center") == true) return true From 992e64480aca71ea9fb4bab28da1a4296104a9ce Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 15 Jan 2025 18:02:56 +0530 Subject: [PATCH 16/37] revert --- .../android/scripts/xml/TextViewStyleCheck.kt | 249 ++---------------- 1 file changed, 24 insertions(+), 225 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 8846fd27c58..d17e2fa5c12 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -1,12 +1,7 @@ package org.oppia.android.scripts.xml -import org.w3c.dom.Document -import org.w3c.dom.Element -import org.xml.sax.Attributes -import org.xml.sax.helpers.DefaultHandler import java.io.File import javax.xml.parsers.DocumentBuilderFactory -import javax.xml.parsers.SAXParserFactory /** * Script to ensure all TextView elements in layout XML files use centrally managed styles. @@ -22,7 +17,8 @@ import javax.xml.parsers.SAXParserFactory */ fun main(vararg args: String) { require(args.isNotEmpty()) { - "Usage: bazel run //scripts:check_textview_styles -- " + "Usage: bazel run" + + " //scripts:check_textview_styles -- " } val repoRoot = File(args[0]) @@ -31,233 +27,36 @@ fun main(vararg args: String) { val resDir = File(repoRoot, "app/src/main/res") require(resDir.exists()) { "Resource directory does not exist: ${resDir.path}" } - val xmlFiles = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } - ?.flatMap { dir -> dir.walkTopDown().filter { it.extension == "xml" } } - ?: emptyList() - - val styleChecker = TextViewStyleCheck(repoRoot) - styleChecker.checkFiles(xmlFiles) -} - -private data class StyleViolation( - val filePath: String, - val lineNumber: Int, - val message: String -) - -private class TextViewStyleCheck(private val repoRoot: File) { - private val styleViolations = mutableListOf() - private val legacyDirectionalityWarnings = mutableListOf() - private val builderFactory = DocumentBuilderFactory.newInstance() - private val styles: Map by lazy { loadStyles() } - private val validatedStyles = mutableMapOf() - - private fun loadStyles(): Map { - val stylesFile = File(repoRoot, "app/src/main/res/values/styles.xml") - require(stylesFile.exists()) { "Styles file does not exist: ${stylesFile.path}" } - - val document = builderFactory.newDocumentBuilder().parse(stylesFile) - val styleNodes = document.getElementsByTagName("style") - return (0 until styleNodes.length).associate { i -> - val element = styleNodes.item(i) as Element - element.getAttribute("name") to element - } - } - /** Checks XML files for TextView elements to ensure compliance with style requirements. */ - fun checkFiles(xmlFiles: List) { - xmlFiles.forEach { file -> processXmlFile(file) } - printResults() - } - - private fun processXmlFile(file: File) { - val handler = TextViewLocationHandler(file.path) { element, lineNumber -> - validateTextViewElement(element, file.path, lineNumber) - } - - val parser = SAXParserFactory.newInstance().newSAXParser() - parser.parse(file, handler) + val layoutDirs = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } + ?: emptyArray() + val xmlFiles = layoutDirs.flatMap { dir -> + dir.walkTopDown().filter { file -> file.extension == "xml" }.toList() } - private class TextViewLocationHandler( - private val filePath: String, - private val onTextViewFound: (Element, Int) -> Unit - ) : DefaultHandler() { - private val document: Document = DocumentBuilderFactory.newInstance() - .newDocumentBuilder() - .newDocument() + val builderFactory = DocumentBuilderFactory.newInstance() + val errors = mutableListOf() - override fun startElement( - uri: String, - localName: String, - qName: String, - attributes: Attributes - ) { - if (qName == "TextView") { - // Get the actual content of the file to verify the line number - val fileContent = File(filePath).readLines() + for (file in xmlFiles) { + val document = builderFactory.newDocumentBuilder().parse(file) + val textViewNodes = document.getElementsByTagName("TextView") - // Find the actual line number by searching for the TextView tag - val actualLine = fileContent.indexOfFirst { line -> - line.trim().startsWith(" - if (!isValid) { - styleViolations.add( - StyleViolation( - filePath, - lineNumber, - "References style with missing RTL/LTR properties: $styleName" - ) - ) - } - return - } - val styleElement = styles[styleName] ?: run { - styleViolations.add( - StyleViolation( - filePath, - lineNumber, - "References non-existent style: $styleName" - ) - ) - return - } - - val hasRtlProperties = validateStyleProperties(styleElement) - validatedStyles[styleName] = hasRtlProperties - - if (!hasRtlProperties) { - styleViolations.add( - StyleViolation( - filePath, - lineNumber, - "Style '$styleName' lacks RTL/LTR properties" - ) - ) - } - } - private fun validateStyleProperties(styleElement: Element): Boolean { - val rtlProperties = setOf( - "android:textAlignment", - "android:gravity", - "android:layoutDirection", - "android:textDirection", - "android:textSize" - ) - - val items = styleElement.getElementsByTagName("item") - return (0 until items.length).any { i -> - val item = items.item(i) as Element - item.getAttribute("name") in rtlProperties - } - } - // Determines if a TextView is exempt from requiring a centrally managed style. - private fun isExemptFromStyleRequirement(element: Element): Boolean { - if (element.getAttribute("android:gravity")?.contains("center") == true) return true - if (hasDynamicVisibility(element)) return true - if (element.hasAttribute("android:textSize")) return true - - return !hasDirectionalAttributes(element) - } - - private fun hasDynamicVisibility(element: Element) = - element.getAttribute("android:visibility").let { it.contains("{") && it.contains("}") } - - private fun hasDirectionalAttributes(element: Element): Boolean { - val directionAttributes = listOf( - "android:layout_alignParentStart", - "android:layout_alignParentEnd", - "android:layout_toStartOf", - "android:layout_toEndOf", - "android:paddingStart", - "android:paddingEnd", - "android:layout_marginStart", - "android:layout_marginEnd" - ) - return directionAttributes.any { element.hasAttribute(it) } - } - - private fun checkForLegacyDirectionality(element: Element, filePath: String, lineNumber: Int) { - val legacyAttributes = listOf( - "android:paddingLeft", - "android:paddingRight", - "android:layout_marginLeft", - "android:layout_marginRight", - "android:layout_alignParentLeft", - "android:layout_alignParentRight", - "android:layout_toLeftOf", - "android:layout_toRightOf" - ) - - val foundLegacyAttributes = legacyAttributes.filter { element.hasAttribute(it) } - if (foundLegacyAttributes.isNotEmpty()) { - legacyDirectionalityWarnings.add( - StyleViolation( - filePath, - lineNumber, - "Uses legacy directional attributes: ${foundLegacyAttributes.joinToString(", ")}" - ) - ) - } - } - - private fun printResults() { - if (legacyDirectionalityWarnings.isNotEmpty()) { - legacyDirectionalityWarnings.forEach { violation -> - println( - "WARNING: ${violation.message} in file: ${violation.filePath}," + - " line ${violation.lineNumber}" - ) - } - } - - if (styleViolations.isNotEmpty()) { - styleViolations.forEach { violation -> - println( - "ERROR: ${violation.message} in file: ${violation.filePath}," + - " line ${violation.lineNumber}" - ) - } - throw Exception("TEXTVIEW STYLE CHECK FAILED") - } else { - println("TEXTVIEW STYLE CHECK PASSED") - } + if (errors.isNotEmpty()) { + println("TextView Style Check FAILED:") + errors.forEach { println(it) } + throw Exception("Some TextView elements do not have centrally managed styles.") + } else { + println("TextView Style Check PASSED.") } } From 73c90aacc35b6d8e2962856ab2dbc09046adbacc Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 15 Jan 2025 18:35:08 +0530 Subject: [PATCH 17/37] adding ids --- .../walkthrough_final_fragment.xml | 2 + app/src/main/res/layout/drawer_fragment.xml | 2 + .../res/layout/walkthrough_final_fragment.xml | 2 + .../android/scripts/xml/TextViewStyleCheck.kt | 98 +++++++---- .../scripts/xml/TextViewStyleCheckTest.kt | 154 ------------------ 5 files changed, 75 insertions(+), 183 deletions(-) diff --git a/app/src/main/res/layout-land/walkthrough_final_fragment.xml b/app/src/main/res/layout-land/walkthrough_final_fragment.xml index 4cc89c8fc26..cd6e06ee4c5 100644 --- a/app/src/main/res/layout-land/walkthrough_final_fragment.xml +++ b/app/src/main/res/layout-land/walkthrough_final_fragment.xml @@ -52,6 +52,7 @@ app:contentPadding="@dimen/walkthrough_final_fragment_card_content_padding"> - * - * Arguments: - * - path_to_repository_root: The root path of the repository. - * - * Example: - * bazel run //scripts:check_textview_styles -- $(pwd) */ fun main(vararg args: String) { require(args.isNotEmpty()) { - "Usage: bazel run" + - " //scripts:check_textview_styles -- " + "Usage: bazel run //scripts:check_textview_styles -- " } val repoRoot = File(args[0]) @@ -27,36 +21,82 @@ fun main(vararg args: String) { val resDir = File(repoRoot, "app/src/main/res") require(resDir.exists()) { "Resource directory does not exist: ${resDir.path}" } - val layoutDirs = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } - ?: emptyArray() - val xmlFiles = layoutDirs.flatMap { dir -> - dir.walkTopDown().filter { file -> file.extension == "xml" }.toList() - } + val xmlFiles = resDir.listFiles { file -> file.isDirectory && file.name.startsWith("layout") } + ?.flatMap { dir -> dir.walkTopDown().filter { it.extension == "xml" } } + ?: emptyList() + + val styleChecker = TextViewStyleCheck() + styleChecker.checkFiles(xmlFiles) +} - val builderFactory = DocumentBuilderFactory.newInstance() - val errors = mutableListOf() +private class TextViewStyleCheck { + private val styleValidationIssues = mutableListOf() + private val directionalityWarnings = mutableListOf() + private val builderFactory = DocumentBuilderFactory.newInstance() - for (file in xmlFiles) { + fun checkFiles(xmlFiles: List) { + xmlFiles.forEach { file -> processXmlFile(file) } + printResults() + } + + private fun processXmlFile(file: File) { val document = builderFactory.newDocumentBuilder().parse(file) val textViewNodes = document.getElementsByTagName("TextView") + val relativePath = file.path.substringAfter("main/res/") for (i in 0 until textViewNodes.length) { - val node = textViewNodes.item(i) - val attributes = node.attributes - val styleAttribute = attributes?.getNamedItem("style")?.nodeValue + val element = textViewNodes.item(i) as Element + validateTextViewElement(element, relativePath) + } + } + + private fun validateTextViewElement(element: Element, filePath: String) { + val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue + val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "No ID" - if (styleAttribute == null || !styleAttribute.startsWith("@style/")) { - errors.add("${file.path}: TextView element is missing a centrally managed style.") - break + if (styleAttribute == null && (idAttribute=="No ID")) { + styleValidationIssues.add("ERROR: Missing style attribute in file: $filePath for TextView ($idAttribute)") + } + + checkForLegacyDirectionality(element, filePath, idAttribute) + } + + private fun checkForLegacyDirectionality(element: Element, filePath: String, idAttribute: String) { + val legacyAttributes = mapOf( + "android:paddingLeft" to "paddingStart", + "android:paddingRight" to "paddingEnd", + "android:layout_marginLeft" to "layout_marginStart", + "android:layout_marginRight" to "layout_marginEnd", + "android:layout_alignParentLeft" to "layout_alignParentStart", + "android:layout_alignParentRight" to "layout_alignParentEnd", + "android:layout_toLeftOf" to "layout_toStartOf", + "android:layout_toRightOf" to "layout_toEndOf" + ) + + val foundLegacyAttributes = legacyAttributes.filter { element.hasAttribute(it.key) } + if (foundLegacyAttributes.isNotEmpty()) { + foundLegacyAttributes.forEach { (legacyAttr, modernAttr) -> + directionalityWarnings.add( + "WARNING: Hardcoded left/right attribute '$legacyAttr' in file: $filePath " + + "for TextView ($idAttribute). Consider using '$modernAttr' instead." + ) } } } - if (errors.isNotEmpty()) { - println("TextView Style Check FAILED:") - errors.forEach { println(it) } - throw Exception("Some TextView elements do not have centrally managed styles.") - } else { - println("TextView Style Check PASSED.") + private fun printResults() { + if (styleValidationIssues.isNotEmpty()) { + styleValidationIssues.forEach { println(it) } + } + + if (directionalityWarnings.isNotEmpty()) { + directionalityWarnings.forEach { println(it) } + } + + if (styleValidationIssues.isEmpty()) { + println("TEXTVIEW STYLE CHECK PASSED") + } else { + throw Exception("TEXTVIEW STYLE CHECK FAILED") + } } } diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index c10654e2a93..a1f57d52982 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -66,160 +66,6 @@ class TextViewStyleCheckTest { assertThat(outContent.toString().trim()).isEqualTo(TEXTVIEW_STYLE_CHECK_PASSED_OUTPUT_INDICATOR) } - @Test - fun testTextViewStyle_missingStyleAttribute_checksFail() { - val validStyle = - """ - - - - - """.trimIndent() - - val invalidLayout = - """ - - - - - """.trimIndent() - - createStylesFile(validStyle) - createLayoutFile(invalidLayout) - - val thrown = kotlin.runCatching { runScript() }.exceptionOrNull() - assertThat(thrown).isInstanceOf(Exception::class.java) - assertThat(thrown).hasMessageThat().contains("TEXTVIEW STYLE CHECK FAILED") - assertThat(outContent.toString()).contains("ERROR: Missing style attribute") - assertThat(outContent.toString()).contains("line 5") - } - - @Test - fun testTextViewStyle_nonExistentStyle_checksFail() { - val validStyle = - """ - - - - - """.trimIndent() - - val invalidLayout = - """ - - - - - """.trimIndent() - - createStylesFile(validStyle) - createLayoutFile(invalidLayout) - - val thrown = kotlin.runCatching { runScript() }.exceptionOrNull() - assertThat(thrown).isInstanceOf(Exception::class.java) - assertThat(thrown).hasMessageThat().contains("TEXTVIEW STYLE CHECK FAILED") - assertThat(outContent.toString()).contains( - "ERROR:" + - " References non-existent style: NonExistentStyle" - ) - assertThat(outContent.toString()).contains("line 5") - } - - @Test - fun testTextViewStyle_styleWithoutRtlProperties_checksFail() { - val invalidStyle = - """ - - - - - """.trimIndent() - - val layout = - """ - - - - - """.trimIndent() - - createStylesFile(invalidStyle) - createLayoutFile(layout) - - val thrown = kotlin.runCatching { runScript() }.exceptionOrNull() - assertThat(thrown).isInstanceOf(Exception::class.java) - assertThat(thrown).hasMessageThat().contains("TEXTVIEW STYLE CHECK FAILED") - assertThat(outContent.toString()).contains( - "ERROR: Style 'InvalidTextViewStyle'" + - " lacks RTL/LTR properties in file" - ) - assertThat(outContent.toString()).contains("line 5") - } - - @Test - fun testTextViewStyle_legacyDirectionalityAttributes_showsWarning() { - val validStyle = - """ - - - - - """.trimIndent() - - val layoutWithLegacyAttributes = - """ - - - - - """.trimIndent() - - createStylesFile(validStyle) - createLayoutFile(layoutWithLegacyAttributes) - - runScript() - - assertThat(outContent.toString()).contains( - "WARNING: Uses legacy directional attributes: android:paddingLeft, android:layout_marginRight" - ) - } - private fun createStylesFile(content: String) { val stylesFile = tempFolder.newFile("app/src/main/res/values/styles.xml") stylesFile.writeText(content) From 7670404ff6debf5fe49aa30eb9bd476a0bed4abc Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 15 Jan 2025 19:54:26 +0530 Subject: [PATCH 18/37] updating tests --- .../android/scripts/xml/TextViewStyleCheck.kt | 150 ++++++++++++++++-- .../scripts/xml/TextViewStyleCheckTest.kt | 145 ++++++++++++++++- 2 files changed, 276 insertions(+), 19 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index dcd5f7f77fc..2b2c70b8184 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -5,10 +5,16 @@ import java.io.File import javax.xml.parsers.DocumentBuilderFactory /** - * Script to ensure all TextView elements in layout XML files use styles and proper RTL attributes. + * Script to ensure all TextView elements in layout XML files use centrally managed styles. * * Usage: * bazel run //scripts:check_textview_styles -- + * + * Arguments: + * - path_to_repository_root: The root path of the repository. + * + * Example: + * bazel run //scripts:check_textview_styles -- $(pwd) */ fun main(vararg args: String) { require(args.isNotEmpty()) { @@ -52,16 +58,25 @@ private class TextViewStyleCheck { private fun validateTextViewElement(element: Element, filePath: String) { val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue - val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "No ID" + val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "NO ID" + + if(idAttribute in attributeIds) return - if (styleAttribute == null && (idAttribute=="No ID")) { - styleValidationIssues.add("ERROR: Missing style attribute in file: $filePath for TextView ($idAttribute)") + if (styleAttribute.isNullOrBlank()) { + styleValidationIssues.add( + "ERROR: Missing style attribute in file:" + + " $filePath for TextView ($idAttribute)" + ) } checkForLegacyDirectionality(element, filePath, idAttribute) } - private fun checkForLegacyDirectionality(element: Element, filePath: String, idAttribute: String) { + private fun checkForLegacyDirectionality( + element: Element, + filePath: String, + idAttribute: String + ) { val legacyAttributes = mapOf( "android:paddingLeft" to "paddingStart", "android:paddingRight" to "paddingEnd", @@ -73,9 +88,8 @@ private class TextViewStyleCheck { "android:layout_toRightOf" to "layout_toEndOf" ) - val foundLegacyAttributes = legacyAttributes.filter { element.hasAttribute(it.key) } - if (foundLegacyAttributes.isNotEmpty()) { - foundLegacyAttributes.forEach { (legacyAttr, modernAttr) -> + for ((legacyAttr, modernAttr) in legacyAttributes) { + if (element.hasAttribute(legacyAttr)) { directionalityWarnings.add( "WARNING: Hardcoded left/right attribute '$legacyAttr' in file: $filePath " + "for TextView ($idAttribute). Consider using '$modernAttr' instead." @@ -85,18 +99,120 @@ private class TextViewStyleCheck { } private fun printResults() { + // Print warnings first + directionalityWarnings.forEach { println(it) } + + // Then print any validation issues and determine if the check passed if (styleValidationIssues.isNotEmpty()) { styleValidationIssues.forEach { println(it) } - } - - if (directionalityWarnings.isNotEmpty()) { - directionalityWarnings.forEach { println(it) } - } - - if (styleValidationIssues.isEmpty()) { - println("TEXTVIEW STYLE CHECK PASSED") - } else { throw Exception("TEXTVIEW STYLE CHECK FAILED") + } else if (directionalityWarnings.isEmpty()) { + // Only print success if there are no warnings or errors + println("TEXTVIEW STYLE CHECK PASSED") } } } + +private val attributeIds = listOf( + "@+id/developer_options_text_view", + "@+id/administrator_controls_text_view", + "@+id/onboarding_language_text_view", + "@+id/walkthrough_final_no_text_view", + "@+id/walkthrough_final_yes_text_view", + "@+id/walkthrough_final_title_text_view", + "@+id/chapter_index", + "@+id/chapter_index", + "@+id/test_text_view", + "@+id/feedback_text_view", + "@+id/item_selection_contents_text_view", + "@+id/learner_analytics_sync_status_text_view", + "@+id/text_view_for_int_no_data_binding", + "@+id/walkthrough_topic_name_text_view", + "@+id/walkthrough_lesson_count_text_view", + "@+id/hint_bar_title", + "@+id/coming_soon_text_view", + "@+id/topic_name_text_view", + "@+id/lesson_count_text_view", + "@+id/multiple_choice_content_text_view", + "@+id/language_text_view", + "@+id/welcome_text_view", + "@+id/app_version_text_view", + "@+id/app_last_update_date_text_view", + "@+id/test_margin_text_view", + "@+id/content_text_view", + "@+id/action_options", + "@+id/action_help", + "@+id/action_close", + "@+id/continue_studying_text_view", + "@+id/language_unavailable_notice", + "@+id/story_progress_chapter_completed_text", + "@+id/profile_id_view_profile_name", + "@+id/profile_id_view_learner_id", + "@+id/learner_events_waiting_upload_label", + "@+id/learner_events_waiting_upload_count", + "@+id/learner_events_uploaded_label", + "@+id/learner_events_uploaded_count", + "@+id/uncategorized_events_waiting_upload_label", + "@+id/uncategorized_events_waiting_upload_count", + "@+id/uncategorized_events_uploaded_label", + "@+id/uncategorized_events_uploaded_count", + "@+id/text_view_for_live_data_no_data_binding", + "@+id/selection_interaction_textview", + "@+id/onboarding_steps_count", + "@+id/profile_picture_edit_dialog_view_picture", + "@+id/profile_picture_edit_dialog_change_picture", + "@+id/chapter_title", + "@+id/walkthrough_welcome_title_text_view", + "@+id/story_name_text_view", + "@+id/topic_name_text_view", + "@+id/chapter_index", + "@+id/copyright_license_text_view", + "@+id/multiple_choice_content_text_view", + "@+id/ga_update_notice_dialog_message", + "@+id/create_profile_picture_prompt", + "@+id/profile_reset_pin_main", + "@+id/submitted_answer_text_view", + "@+id/language_text_view", + "@+id/end_session_header_text_view", + "@+id/end_session_body_text_view", + "@+id/question_progress_text", + "@+id/congratulations_text_view", + "@+id/beta_notice_dialog_message", + "@+id/chapter_index", + "@+id/onboarding_language_explanation", + "@+id/onboarding_steps_count", + "@+id/create_profile_picture_prompt", + "@+id/create_profile_title", + "@+id/end_session_header_text_view", + "@+id/end_session_body_text_view", + "@+id/question_progress_text", + "@+id/congratulations_text_view", + "@+id/walkthrough_final_no_text_view", + "@+id/walkthrough_final_yes_text_view", + "@+id/walkthrough_final_title_text_view", + "@+id/profile_name_text_view", + "@+id/create_profile_picture_prompt", + "@+id/create_profile_title", + "@+id/end_session_header_text_view", + "@+id/end_session_body_text_view", + "@+id/question_progress_text", + "@+id/congratulations_text_view", + "@+id/resume_lesson_chapter_title_text_view", + "@+id/topic_name_text_view", + "@+id/story_count_text_view", + "@+id/download_size_text_view", + "@+id/onboarding_language_explanation", + "@+id/onboarding_steps_count", + "@+id/create_profile_picture_prompt", + "@+id/create_profile_title", + "@+id/end_session_header_text_view", + "@+id/end_session_body_text_view", + "@+id/question_progress_text", + "@+id/options_activity_selected_options_title", + "@+id/profile_select_text", + "@+id/continue_studying_text_view", + "@+id/extra_controls_title", + "@+id/chapter_title", + "@+id/options_activity_selected_options_title", + "@+id/view_all_text_view" + ) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index a1f57d52982..a32dd7cee61 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -66,17 +66,158 @@ class TextViewStyleCheckTest { assertThat(outContent.toString().trim()).isEqualTo(TEXTVIEW_STYLE_CHECK_PASSED_OUTPUT_INDICATOR) } + @Test + fun testTextViewStyle_missingStyleAttribute_checksFail() { + val validLayout = + """ + + + + + """.trimIndent() + + createLayoutFile(validLayout) + + val thrown = catchThrowable { runScript() } + + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + assertThat(outContent.toString()).contains("ERROR: Missing style attribute") + } + + @Test + fun testTextViewStyle_legacyDirectionalityAttributes_showsWarnings() { + val validStyle = + """ + + + + + """.trimIndent() + + val layoutWithLegacyAttributes = + """ + + + + + """.trimIndent() + + createStylesFile(validStyle) + createLayoutFile(layoutWithLegacyAttributes) + + runScript() + + val output = outContent.toString() + assertThat(output).contains("WARNING: Hardcoded left/right attribute 'android:paddingLeft'") + assertThat(output).contains("WARNING: Hardcoded left/right attribute 'android:layout_marginRight'") + assertThat(output).doesNotContain(TEXTVIEW_STYLE_CHECK_PASSED_OUTPUT_INDICATOR) + } + + @Test + fun testTextViewStyle_multipleTextViews_checksAllTextViews() { + val layoutWithMultipleTextViews = + """ + + + + + + """.trimIndent() + + createLayoutFile(layoutWithMultipleTextViews) + + val thrown = catchThrowable { runScript() } + + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + assertThat(outContent.toString()).contains("ERROR: Missing style attribute") + } + + @Test + fun testTextViewStyle_multipleLayoutFiles_checksAllFiles() { + val validLayout1 = + """ + + + + + """.trimIndent() + + val invalidLayout2 = + """ + + + + + """.trimIndent() + + createLayoutFile(validLayout1, "test_layout1.xml") + createLayoutFile(invalidLayout2, "test_layout2.xml") + + val thrown = catchThrowable { runScript() } + + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + assertThat(outContent.toString()).contains("ERROR: Missing style attribute") + } + private fun createStylesFile(content: String) { val stylesFile = tempFolder.newFile("app/src/main/res/values/styles.xml") stylesFile.writeText(content) } - private fun createLayoutFile(content: String) { - val layoutFile = tempFolder.newFile("app/src/main/res/layout/test_layout.xml") + private fun createLayoutFile(content: String, fileName: String = "test_layout.xml") { + val layoutFile = tempFolder.newFile("app/src/main/res/layout/$fileName") layoutFile.writeText(content) } private fun runScript() { main(tempFolder.root.absolutePath) } + + private fun catchThrowable(executable: () -> Unit): Throwable { + try { + executable() + throw AssertionError("Expected to throw, but did not") + } catch (e: Throwable) { + return e + } + } } From 5f7c85df7e7f33cacce96e6e6fe815c8c2257b08 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 15 Jan 2025 20:01:40 +0530 Subject: [PATCH 19/37] formatting --- .../oppia/android/scripts/xml/TextViewStyleCheck.kt | 12 ++++++------ .../android/scripts/xml/TextViewStyleCheckTest.kt | 9 ++++++--- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 2b2c70b8184..f4957418fd8 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -60,13 +60,13 @@ private class TextViewStyleCheck { val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "NO ID" - if(idAttribute in attributeIds) return + if (idAttribute in attributeIds) return if (styleAttribute.isNullOrBlank()) { - styleValidationIssues.add( - "ERROR: Missing style attribute in file:" + - " $filePath for TextView ($idAttribute)" - ) + styleValidationIssues.add( + "ERROR: Missing style attribute in file:" + + " $filePath for TextView ($idAttribute)" + ) } checkForLegacyDirectionality(element, filePath, idAttribute) @@ -215,4 +215,4 @@ private val attributeIds = listOf( "@+id/chapter_title", "@+id/options_activity_selected_options_title", "@+id/view_all_text_view" - ) +) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index a32dd7cee61..a9bef10ea9f 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -102,7 +102,7 @@ class TextViewStyleCheckTest { 16sp - """.trimIndent() + """.trimIndent() val layoutWithLegacyAttributes = """ @@ -118,7 +118,7 @@ class TextViewStyleCheckTest { android:paddingLeft="16dp" android:layout_marginRight="8dp"/> - """.trimIndent() + """.trimIndent() createStylesFile(validStyle) createLayoutFile(layoutWithLegacyAttributes) @@ -127,7 +127,10 @@ class TextViewStyleCheckTest { val output = outContent.toString() assertThat(output).contains("WARNING: Hardcoded left/right attribute 'android:paddingLeft'") - assertThat(output).contains("WARNING: Hardcoded left/right attribute 'android:layout_marginRight'") + assertThat(output).contains( + "WARNING: Hardcoded left/right attribute" + + " 'android:layout_marginRight'" + ) assertThat(output).doesNotContain(TEXTVIEW_STYLE_CHECK_PASSED_OUTPUT_INDICATOR) } From ca14146fcd711b378f6f4ba0eac0fe98ee6494bd Mon Sep 17 00:00:00 2001 From: manas-yu Date: Thu, 16 Jan 2025 22:05:11 +0530 Subject: [PATCH 20/37] adding style --- app/src/main/res/layout/drawer_fragment.xml | 9 ++------- app/src/main/res/values/styles.xml | 9 +++++++++ .../org/oppia/android/scripts/xml/TextViewStyleCheck.kt | 5 +---- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/src/main/res/layout/drawer_fragment.xml b/app/src/main/res/layout/drawer_fragment.xml index eeffd6e3704..59ee16aa952 100644 --- a/app/src/main/res/layout/drawer_fragment.xml +++ b/app/src/main/res/layout/drawer_fragment.xml @@ -105,14 +105,9 @@ + android:textColor="@{footerViewModel.isAdministratorControlsSelected ? @color/component_color_drawer_fragment_admin_controls_selected_text_color : @color/component_color_shared_primary_dark_text_color}" /> diff --git a/app/src/main/res/values/styles.xml b/app/src/main/res/values/styles.xml index 32fcd43fce6..1cc8f1ea810 100644 --- a/app/src/main/res/values/styles.xml +++ b/app/src/main/res/values/styles.xml @@ -805,6 +805,15 @@ @dimen/onboarding_shared_text_size_large + + - - """.trimIndent() - val validLayout = """ @@ -58,9 +46,7 @@ class TextViewStyleCheckTest { """.trimIndent() - createStylesFile(validStyle) createLayoutFile(validLayout) - runScript() assertThat(outContent.toString().trim()).isEqualTo(TEXTVIEW_STYLE_CHECK_PASSED_OUTPUT_INDICATOR) @@ -68,7 +54,7 @@ class TextViewStyleCheckTest { @Test fun testTextViewStyle_missingStyleAttribute_checksFail() { - val validLayout = + val invalidLayout = """ """.trimIndent() - createLayoutFile(validLayout) + createLayoutFile(invalidLayout) val thrown = catchThrowable { runScript() } @@ -91,52 +77,31 @@ class TextViewStyleCheckTest { } @Test - fun testTextViewStyle_legacyDirectionalityAttributes_showsWarnings() { - val validStyle = + fun testTextViewStyle_onlyDirectionalityWarnings_checksPass() { + val layoutWithDirectionalityWarnings = """ - - - - - """.trimIndent() - - val layoutWithLegacyAttributes = - """ - - - - + + + + """.trimIndent() - createStylesFile(validStyle) - createLayoutFile(layoutWithLegacyAttributes) - + createLayoutFile(layoutWithDirectionalityWarnings) runScript() val output = outContent.toString() - assertThat(output).contains("WARNING: Hardcoded left/right attribute 'android:paddingLeft'") - assertThat(output).contains( - "WARNING: Hardcoded left/right attribute" + - " 'android:layout_marginRight'" - ) - assertThat(output).doesNotContain(TEXTVIEW_STYLE_CHECK_PASSED_OUTPUT_INDICATOR) + assertThat(output).contains("WARNING: Hardcoded left/right attribute") } @Test - fun testTextViewStyle_multipleTextViews_checksAllTextViews() { - val layoutWithMultipleTextViews = + fun testTextViewStyle_multipleTextViews_someWithoutStyle_checksFail() { + val layoutWithMixedTextViews = """ + """.trimIndent() - createLayoutFile(layoutWithMultipleTextViews) + createLayoutFile(layoutWithMixedTextViews) val thrown = catchThrowable { runScript() } @@ -164,8 +134,8 @@ class TextViewStyleCheckTest { } @Test - fun testTextViewStyle_multipleLayoutFiles_checksAllFiles() { - val validLayout1 = + fun testTextViewStyle_multipleLayoutFiles_mixedValidation_checksFail() { + val validLayout = """ """.trimIndent() - val invalidLayout2 = + val warningLayout = + """ + + + + + """.trimIndent() + + val invalidLayout = """ """.trimIndent() - createLayoutFile(validLayout1, "test_layout1.xml") - createLayoutFile(invalidLayout2, "test_layout2.xml") + createLayoutFile(validLayout, "valid_layout.xml") + createLayoutFile(warningLayout, "warning_layout.xml") + createLayoutFile(invalidLayout, "invalid_layout.xml") val thrown = catchThrowable { runScript() } assertThat(thrown).isInstanceOf(Exception::class.java) assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) - assertThat(outContent.toString()).contains("ERROR: Missing style attribute") + val output = outContent.toString() + assertThat(output).contains("WARNING: Hardcoded left/right attribute") + assertThat(output).contains("ERROR: Missing style attribute") + } + @Test + fun testTextViewStyle_missingStyle_logsCorrectLineNumber() { + val layoutWithLineSpacing = + """ + + + + + + + """.trimIndent() + + createLayoutFile(layoutWithLineSpacing) + + val thrown = catchThrowable { runScript() } + + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(outContent.toString()).contains("line 9") } + @Test + fun testTextViewStyle_directionalityWarning_logsCorrectLineNumber() { + val layoutWithLineSpacing = + """ + + + + + + + + + """.trimIndent() + + createLayoutFile(layoutWithLineSpacing) + runScript() + + assertThat(outContent.toString()).contains("line 12") + } + @Test + fun testTextViewStyle_multipleTextViews_logsCorrectLineNumbers() { + val layoutWithMultipleTextViews = + """ + + + + + + + + + + """.trimIndent() + + createLayoutFile(layoutWithMultipleTextViews) + + val thrown = catchThrowable { runScript() } + + val output = outContent.toString() + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(output).contains("line 8") // First TextView error + assertThat(output).contains("line 17") // Second TextView warning + } + @Test + fun testTextViewStyle_nestedTextViews_logsCorrectLineNumbers() { + val nestedLayout = + """ + + + + + + + + + + + + """.trimIndent() - private fun createStylesFile(content: String) { - val stylesFile = tempFolder.newFile("app/src/main/res/values/styles.xml") - stylesFile.writeText(content) + createLayoutFile(nestedLayout) + + val thrown = catchThrowable { runScript() } + + val output = outContent.toString() + assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(output).contains("line 11") // First TextView error + assertThat(output).contains("line 19") // Second TextView warning } private fun createLayoutFile(content: String, fileName: String = "test_layout.xml") { From 2fb3effb560e7fefd73e492b24e63f6d518f964a Mon Sep 17 00:00:00 2001 From: manas-yu Date: Fri, 17 Jan 2025 18:12:46 +0530 Subject: [PATCH 23/37] formatting --- WORKSPACE | 1 - .../android/scripts/xml/TextViewStyleCheck.kt | 110 +++++++++--------- 2 files changed, 54 insertions(+), 57 deletions(-) diff --git a/WORKSPACE b/WORKSPACE index 8fb60b0ee50..e877ad78248 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -14,7 +14,6 @@ android_sdk_repository( name = "androidsdk", api_level = BUILD_SDK_VERSION, build_tools_version = BUILD_TOOLS_VERSION, - path="/home/manas-yu/Android/Sdk" ) # Oppia's backend proto API definitions. diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 506e826f849..1416c6498d6 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -2,15 +2,12 @@ package org.oppia.android.scripts.xml import org.w3c.dom.Document import org.w3c.dom.Element -import org.w3c.dom.Node import org.xml.sax.Attributes import org.xml.sax.Locator -import org.xml.sax.SAXException import org.xml.sax.helpers.DefaultHandler import java.io.File import java.io.FileInputStream import java.util.Stack -import javax.xml.parsers.DocumentBuilder import javax.xml.parsers.DocumentBuilderFactory import javax.xml.parsers.SAXParser import javax.xml.parsers.SAXParserFactory @@ -106,69 +103,70 @@ private class TextViewStyleCheck { } } - private fun readXMLWithLineNumbers(inputStream: FileInputStream, lineNumAttribName: String): Document { - val doc: Document - val parser: SAXParser - try { - val factory = SAXParserFactory.newInstance() - parser = factory.newSAXParser() - val docBuilderFactory = DocumentBuilderFactory.newInstance() - val docBuilder = docBuilderFactory.newDocumentBuilder() - doc = docBuilder.newDocument() - } catch (e: Exception) { - throw RuntimeException("Can't create SAX parser / DOM builder.", e) - } + private fun readXMLWithLineNumbers(inputStream: FileInputStream, lineNumAttribName: String): + Document { + val doc: Document + val parser: SAXParser + try { + val factory = SAXParserFactory.newInstance() + parser = factory.newSAXParser() + val docBuilderFactory = DocumentBuilderFactory.newInstance() + val docBuilder = docBuilderFactory.newDocumentBuilder() + doc = docBuilder.newDocument() + } catch (e: Exception) { + throw RuntimeException("Can't create SAX parser / DOM builder.", e) + } - val elementStack = Stack() - val textBuffer = StringBuilder() + val elementStack = Stack() + val textBuffer = StringBuilder() - val handler = object : DefaultHandler() { - private lateinit var locator: Locator + val handler = object : DefaultHandler() { + private lateinit var locator: Locator - override fun setDocumentLocator(locator: Locator) { - this.locator = locator - } + override fun setDocumentLocator(locator: Locator) { + this.locator = locator + } - override fun startElement( - uri: String, - localName: String, - qName: String, - attributes: Attributes - ) { - addTextIfNeeded() - val el = doc.createElement(qName) - for (i in 0 until attributes.length) { - el.setAttribute(attributes.getQName(i), attributes.getValue(i)) + override fun startElement( + uri: String, + localName: String, + qName: String, + attributes: Attributes + ) { + addTextIfNeeded() + val el = doc.createElement(qName) + for (i in 0 until attributes.length) { + el.setAttribute(attributes.getQName(i), attributes.getValue(i)) + } + el.setAttribute(lineNumAttribName, locator.lineNumber.toString()) + elementStack.push(el) } - el.setAttribute(lineNumAttribName, locator.lineNumber.toString()) - elementStack.push(el) - } - override fun endElement(uri: String, localName: String, qName: String) { - addTextIfNeeded() - val closedEl = elementStack.pop() - if (elementStack.isEmpty()) { - doc.appendChild(closedEl) - } else { - elementStack.peek().appendChild(closedEl) + override fun endElement(uri: String, localName: String, qName: String) { + addTextIfNeeded() + val closedEl = elementStack.pop() + if (elementStack.isEmpty()) { + doc.appendChild(closedEl) + } else { + elementStack.peek().appendChild(closedEl) + } } - } - override fun characters(ch: CharArray, start: Int, length: Int) { - textBuffer.append(ch, start, length) - } + override fun characters(ch: CharArray, start: Int, length: Int) { + textBuffer.append(ch, start, length) + } - private fun addTextIfNeeded() { - if (textBuffer.isNotEmpty()) { - val el = elementStack.peek() - val textNode = doc.createTextNode(textBuffer.toString()) - el.appendChild(textNode) - textBuffer.clear() + private fun addTextIfNeeded() { + if (textBuffer.isNotEmpty()) { + val el = elementStack.peek() + val textNode = doc.createTextNode(textBuffer.toString()) + el.appendChild(textNode) + textBuffer.clear() + } } } - } - parser.parse(inputStream, handler) - return doc - } + parser.parse(inputStream, handler) + return doc + } } From 6e7630399d55b00f7b9f5f200d42f9c2baae0c2d Mon Sep 17 00:00:00 2001 From: manas-yu Date: Fri, 17 Jan 2025 18:18:50 +0530 Subject: [PATCH 24/37] attri list --- .../android/scripts/xml/TextViewStyleCheck.kt | 121 ++++++++++++++++-- .../scripts/xml/TextViewStyleCheckTest.kt | 8 +- 2 files changed, 117 insertions(+), 12 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 1416c6498d6..45b6d86a2b9 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -36,6 +36,7 @@ private class TextViewStyleCheck { private val directionalityWarnings = mutableListOf() private val LINE_NUMBER_ATTRIBUTE = "lineNumber" + /** Checks XML files for TextView elements to ensure compliance with style requirements. */ fun checkFiles(xmlFiles: List) { xmlFiles.forEach { file -> processXmlFile(file) } printResults() @@ -55,6 +56,9 @@ private class TextViewStyleCheck { private fun validateTextViewElement(element: Element, filePath: String) { val lineNumber = element.getAttribute(LINE_NUMBER_ATTRIBUTE).toInt().minus(1).toString() val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue + val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "NO ID" + + if (idAttribute in attributeIds) return if (styleAttribute.isNullOrBlank()) { styleValidationIssues.add( @@ -81,14 +85,11 @@ private class TextViewStyleCheck { "android:layout_toRightOf" ) - for (legacyAttr in legacyAttributes) { - if (element.hasAttribute(legacyAttr)) { - directionalityWarnings.add( - "WARNING: Hardcoded left/right attribute in file: $filePath, line $lineNumber. " + - "Consider using start/end." - ) - break - } + if (legacyAttributes.any { element.hasAttribute(it) }) { + directionalityWarnings.add( + "WARNING: Hardcoded left/right attribute in file: $filePath, line $lineNumber. " + + "Consider using start/end." + ) } } @@ -170,3 +171,107 @@ private class TextViewStyleCheck { return doc } } + +// Known exceptions that currently lack a style and are being tracked for fixes. +private val attributeIds = listOf( + "@+id/developer_options_text_view", + "@+id/onboarding_language_text_view", + "@+id/walkthrough_final_no_text_view", + "@+id/walkthrough_final_yes_text_view", + "@+id/walkthrough_final_title_text_view", + "@+id/chapter_index", + "@+id/chapter_index", + "@+id/test_text_view", + "@+id/feedback_text_view", + "@+id/item_selection_contents_text_view", + "@+id/learner_analytics_sync_status_text_view", + "@+id/text_view_for_int_no_data_binding", + "@+id/walkthrough_topic_name_text_view", + "@+id/walkthrough_lesson_count_text_view", + "@+id/hint_bar_title", + "@+id/coming_soon_text_view", + "@+id/topic_name_text_view", + "@+id/lesson_count_text_view", + "@+id/multiple_choice_content_text_view", + "@+id/language_text_view", + "@+id/welcome_text_view", + "@+id/app_version_text_view", + "@+id/app_last_update_date_text_view", + "@+id/test_margin_text_view", + "@+id/content_text_view", + "@+id/action_options", + "@+id/action_help", + "@+id/action_close", + "@+id/continue_studying_text_view", + "@+id/language_unavailable_notice", + "@+id/story_progress_chapter_completed_text", + "@+id/profile_id_view_profile_name", + "@+id/profile_id_view_learner_id", + "@+id/learner_events_waiting_upload_label", + "@+id/learner_events_waiting_upload_count", + "@+id/learner_events_uploaded_label", + "@+id/learner_events_uploaded_count", + "@+id/uncategorized_events_waiting_upload_label", + "@+id/uncategorized_events_waiting_upload_count", + "@+id/uncategorized_events_uploaded_label", + "@+id/uncategorized_events_uploaded_count", + "@+id/text_view_for_live_data_no_data_binding", + "@+id/selection_interaction_textview", + "@+id/onboarding_steps_count", + "@+id/profile_picture_edit_dialog_view_picture", + "@+id/profile_picture_edit_dialog_change_picture", + "@+id/chapter_title", + "@+id/walkthrough_welcome_title_text_view", + "@+id/story_name_text_view", + "@+id/topic_name_text_view", + "@+id/chapter_index", + "@+id/copyright_license_text_view", + "@+id/multiple_choice_content_text_view", + "@+id/ga_update_notice_dialog_message", + "@+id/create_profile_picture_prompt", + "@+id/profile_reset_pin_main", + "@+id/submitted_answer_text_view", + "@+id/language_text_view", + "@+id/end_session_header_text_view", + "@+id/end_session_body_text_view", + "@+id/question_progress_text", + "@+id/congratulations_text_view", + "@+id/beta_notice_dialog_message", + "@+id/chapter_index", + "@+id/onboarding_language_explanation", + "@+id/onboarding_steps_count", + "@+id/create_profile_picture_prompt", + "@+id/create_profile_title", + "@+id/end_session_header_text_view", + "@+id/end_session_body_text_view", + "@+id/question_progress_text", + "@+id/congratulations_text_view", + "@+id/walkthrough_final_no_text_view", + "@+id/walkthrough_final_yes_text_view", + "@+id/walkthrough_final_title_text_view", + "@+id/profile_name_text_view", + "@+id/create_profile_picture_prompt", + "@+id/create_profile_title", + "@+id/end_session_header_text_view", + "@+id/end_session_body_text_view", + "@+id/question_progress_text", + "@+id/congratulations_text_view", + "@+id/resume_lesson_chapter_title_text_view", + "@+id/topic_name_text_view", + "@+id/story_count_text_view", + "@+id/download_size_text_view", + "@+id/onboarding_language_explanation", + "@+id/onboarding_steps_count", + "@+id/create_profile_picture_prompt", + "@+id/create_profile_title", + "@+id/end_session_header_text_view", + "@+id/end_session_body_text_view", + "@+id/question_progress_text", + "@+id/options_activity_selected_options_title", + "@+id/profile_select_text", + "@+id/continue_studying_text_view", + "@+id/extra_controls_title", + "@+id/chapter_title", + "@+id/options_activity_selected_options_title", + "@+id/view_all_text_view" +) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index 69e8b03b55e..899fc987f52 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -266,8 +266,8 @@ class TextViewStyleCheckTest { val output = outContent.toString() assertThat(thrown).isInstanceOf(Exception::class.java) - assertThat(output).contains("line 8") // First TextView error - assertThat(output).contains("line 17") // Second TextView warning + assertThat(output).contains("line 8") + assertThat(output).contains("line 17") } @Test fun testTextViewStyle_nestedTextViews_logsCorrectLineNumbers() { @@ -302,8 +302,8 @@ class TextViewStyleCheckTest { val output = outContent.toString() assertThat(thrown).isInstanceOf(Exception::class.java) - assertThat(output).contains("line 11") // First TextView error - assertThat(output).contains("line 19") // Second TextView warning + assertThat(output).contains("line 11") + assertThat(output).contains("line 19") } private fun createLayoutFile(content: String, fileName: String = "test_layout.xml") { From c80cfe923bf2fc434ee5151bcbf94846d9101811 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Fri, 17 Jan 2025 18:24:38 +0530 Subject: [PATCH 25/37] kdoc --- .../oppia/android/scripts/xml/TextViewStyleCheck.kt | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 45b6d86a2b9..08444b20442 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -12,6 +12,18 @@ import javax.xml.parsers.DocumentBuilderFactory import javax.xml.parsers.SAXParser import javax.xml.parsers.SAXParserFactory +/** + * Script to ensure all TextView elements in layout XML files use centrally managed styles. + * + * Usage: + * bazel run //scripts:check_textview_styles -- + * + * Arguments: + * - path_to_repository_root: The root path of the repository. + * + * Example: + * bazel run //scripts:check_textview_styles -- $(pwd) + */ fun main(vararg args: String) { require(args.isNotEmpty()) { "Usage: bazel run //scripts:check_textview_styles -- " From 4bdbe27684ff12b19dea36b015db50788ca80b60 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Sun, 19 Jan 2025 14:49:06 +0530 Subject: [PATCH 26/37] Deque and line no --- .../android/scripts/xml/TextViewStyleCheck.kt | 112 +++++++++--------- .../scripts/xml/TextViewStyleCheckTest.kt | 12 +- 2 files changed, 63 insertions(+), 61 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 08444b20442..6b2e6bcafad 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -7,7 +7,7 @@ import org.xml.sax.Locator import org.xml.sax.helpers.DefaultHandler import java.io.File import java.io.FileInputStream -import java.util.Stack +import java.util.ArrayDeque import javax.xml.parsers.DocumentBuilderFactory import javax.xml.parsers.SAXParser import javax.xml.parsers.SAXParserFactory @@ -66,7 +66,7 @@ private class TextViewStyleCheck { } private fun validateTextViewElement(element: Element, filePath: String) { - val lineNumber = element.getAttribute(LINE_NUMBER_ATTRIBUTE).toInt().minus(1).toString() + val lineNumber = element.getAttribute(LINE_NUMBER_ATTRIBUTE).toString() val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "NO ID" @@ -111,77 +111,79 @@ private class TextViewStyleCheck { if (styleValidationIssues.isNotEmpty()) { styleValidationIssues.forEach { println(it) } throw Exception("TEXTVIEW STYLE CHECK FAILED") - } else if (directionalityWarnings.isEmpty()) { + } else { println("TEXTVIEW STYLE CHECK PASSED") } } private fun readXMLWithLineNumbers(inputStream: FileInputStream, lineNumAttribName: String): Document { - val doc: Document - val parser: SAXParser - try { - val factory = SAXParserFactory.newInstance() - parser = factory.newSAXParser() - val docBuilderFactory = DocumentBuilderFactory.newInstance() - val docBuilder = docBuilderFactory.newDocumentBuilder() - doc = docBuilder.newDocument() - } catch (e: Exception) { - throw RuntimeException("Can't create SAX parser / DOM builder.", e) - } + val document: Document + val parser: SAXParser + try { + val factory = SAXParserFactory.newInstance() + parser = factory.newSAXParser() + val docBuilderFactory = DocumentBuilderFactory.newInstance() + val docBuilder = docBuilderFactory.newDocumentBuilder() + document = docBuilder.newDocument() + } catch (e: Exception) { + error("Can't create SAX parser / DOM builder.") + } - val elementStack = Stack() - val textBuffer = StringBuilder() + val elementStack = ArrayDeque() + val textBuffer = StringBuilder() - val handler = object : DefaultHandler() { - private lateinit var locator: Locator + val handler = object : DefaultHandler() { + private lateinit var locator: Locator - override fun setDocumentLocator(locator: Locator) { - this.locator = locator - } + override fun setDocumentLocator(locator: Locator) { + this.locator = locator + } - override fun startElement( - uri: String, - localName: String, - qName: String, - attributes: Attributes - ) { - addTextIfNeeded() - val el = doc.createElement(qName) - for (i in 0 until attributes.length) { - el.setAttribute(attributes.getQName(i), attributes.getValue(i)) - } - el.setAttribute(lineNumAttribName, locator.lineNumber.toString()) - elementStack.push(el) - } + override fun startElement( + uri: String, + localName: String, + qName: String, + attributes: Attributes + ) { + addTextIfNeeded() + val element = document.createElement(qName) + val openingTagLine = locator.lineNumber - attributes.length - override fun endElement(uri: String, localName: String, qName: String) { - addTextIfNeeded() - val closedEl = elementStack.pop() - if (elementStack.isEmpty()) { - doc.appendChild(closedEl) - } else { - elementStack.peek().appendChild(closedEl) - } + for (i in 0 until attributes.length) { + element.setAttribute(attributes.getQName(i), attributes.getValue(i)) } + element.setAttribute(lineNumAttribName, openingTagLine.toString()) + elementStack.addLast(element) + } - override fun characters(ch: CharArray, start: Int, length: Int) { - textBuffer.append(ch, start, length) + override fun endElement(uri: String, localName: String, qName: String) { + addTextIfNeeded() + val closedElement = elementStack.removeLast() + if (elementStack.isEmpty()) { + document.appendChild(closedElement) + } else { + elementStack.last().appendChild(closedElement) } + } - private fun addTextIfNeeded() { - if (textBuffer.isNotEmpty()) { - val el = elementStack.peek() - val textNode = doc.createTextNode(textBuffer.toString()) - el.appendChild(textNode) - textBuffer.clear() - } - } + override fun characters(ch: CharArray, start: Int, length: Int) { + textBuffer.append(ch, start, length) } - parser.parse(inputStream, handler) - return doc + private fun addTextIfNeeded() { + if (textBuffer.isNotEmpty()) { + val element = elementStack.last() + val textNode = document.createTextNode(textBuffer.toString()) + element.appendChild(textNode) + textBuffer.clear() + } + } } + + parser.parse(inputStream, handler) + return document + } } // Known exceptions that currently lack a style and are being tracked for fixes. diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index 899fc987f52..7b84008e13b 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -208,7 +208,7 @@ class TextViewStyleCheckTest { val thrown = catchThrowable { runScript() } assertThat(thrown).isInstanceOf(Exception::class.java) - assertThat(outContent.toString()).contains("line 9") + assertThat(outContent.toString()).contains("line 7") } @Test fun testTextViewStyle_directionalityWarning_logsCorrectLineNumber() { @@ -233,7 +233,7 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithLineSpacing) runScript() - assertThat(outContent.toString()).contains("line 12") + assertThat(outContent.toString()).contains("line 9") } @Test fun testTextViewStyle_multipleTextViews_logsCorrectLineNumbers() { @@ -266,8 +266,8 @@ class TextViewStyleCheckTest { val output = outContent.toString() assertThat(thrown).isInstanceOf(Exception::class.java) - assertThat(output).contains("line 8") - assertThat(output).contains("line 17") + assertThat(output).contains("line 6") + assertThat(output).contains("line 13") } @Test fun testTextViewStyle_nestedTextViews_logsCorrectLineNumbers() { @@ -302,8 +302,8 @@ class TextViewStyleCheckTest { val output = outContent.toString() assertThat(thrown).isInstanceOf(Exception::class.java) - assertThat(output).contains("line 11") - assertThat(output).contains("line 19") + assertThat(output).contains("line 10") + assertThat(output).contains("line 16") } private fun createLayoutFile(content: String, fileName: String = "test_layout.xml") { From 72e30eb46523f92ea91810db0e90386bf93398bd Mon Sep 17 00:00:00 2001 From: manas-yu Date: Sun, 19 Jan 2025 14:50:02 +0530 Subject: [PATCH 27/37] formatting --- .../android/scripts/xml/TextViewStyleCheck.kt | 106 +++++++++--------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 6b2e6bcafad..350f7b9d6bb 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -118,72 +118,72 @@ private class TextViewStyleCheck { private fun readXMLWithLineNumbers(inputStream: FileInputStream, lineNumAttribName: String): Document { - val document: Document - val parser: SAXParser - try { - val factory = SAXParserFactory.newInstance() - parser = factory.newSAXParser() - val docBuilderFactory = DocumentBuilderFactory.newInstance() - val docBuilder = docBuilderFactory.newDocumentBuilder() - document = docBuilder.newDocument() - } catch (e: Exception) { - error("Can't create SAX parser / DOM builder.") - } + val document: Document + val parser: SAXParser + try { + val factory = SAXParserFactory.newInstance() + parser = factory.newSAXParser() + val docBuilderFactory = DocumentBuilderFactory.newInstance() + val docBuilder = docBuilderFactory.newDocumentBuilder() + document = docBuilder.newDocument() + } catch (e: Exception) { + error("Can't create SAX parser / DOM builder.") + } - val elementStack = ArrayDeque() - val textBuffer = StringBuilder() + val elementStack = ArrayDeque() + val textBuffer = StringBuilder() - val handler = object : DefaultHandler() { - private lateinit var locator: Locator + val handler = object : DefaultHandler() { + private lateinit var locator: Locator - override fun setDocumentLocator(locator: Locator) { - this.locator = locator - } + override fun setDocumentLocator(locator: Locator) { + this.locator = locator + } - override fun startElement( - uri: String, - localName: String, - qName: String, - attributes: Attributes - ) { - addTextIfNeeded() - val element = document.createElement(qName) - val openingTagLine = locator.lineNumber - attributes.length + override fun startElement( + uri: String, + localName: String, + qName: String, + attributes: Attributes + ) { + addTextIfNeeded() + val element = document.createElement(qName) + val openingTagLine = locator.lineNumber - attributes.length - for (i in 0 until attributes.length) { - element.setAttribute(attributes.getQName(i), attributes.getValue(i)) + for (i in 0 until attributes.length) { + element.setAttribute(attributes.getQName(i), attributes.getValue(i)) + } + element.setAttribute(lineNumAttribName, openingTagLine.toString()) + elementStack.addLast(element) } - element.setAttribute(lineNumAttribName, openingTagLine.toString()) - elementStack.addLast(element) - } - override fun endElement(uri: String, localName: String, qName: String) { - addTextIfNeeded() - val closedElement = elementStack.removeLast() - if (elementStack.isEmpty()) { - document.appendChild(closedElement) - } else { - elementStack.last().appendChild(closedElement) + override fun endElement(uri: String, localName: String, qName: String) { + addTextIfNeeded() + val closedElement = elementStack.removeLast() + if (elementStack.isEmpty()) { + document.appendChild(closedElement) + } else { + elementStack.last().appendChild(closedElement) + } } - } - override fun characters(ch: CharArray, start: Int, length: Int) { - textBuffer.append(ch, start, length) - } + override fun characters(ch: CharArray, start: Int, length: Int) { + textBuffer.append(ch, start, length) + } - private fun addTextIfNeeded() { - if (textBuffer.isNotEmpty()) { - val element = elementStack.last() - val textNode = document.createTextNode(textBuffer.toString()) - element.appendChild(textNode) - textBuffer.clear() + private fun addTextIfNeeded() { + if (textBuffer.isNotEmpty()) { + val element = elementStack.last() + val textNode = document.createTextNode(textBuffer.toString()) + element.appendChild(textNode) + textBuffer.clear() + } } } - } - parser.parse(inputStream, handler) - return document - } + parser.parse(inputStream, handler) + return document + } } // Known exceptions that currently lack a style and are being tracked for fixes. From c26d301655ed2164406636e8ae4af8fa16aed121 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 22 Jan 2025 18:09:57 +0530 Subject: [PATCH 28/37] TODO and idInformation --- .../android/scripts/xml/TextViewStyleCheck.kt | 9 +++-- .../org/oppia/android/scripts/xml/BUILD.bazel | 1 + .../scripts/xml/TextViewStyleCheckTest.kt | 37 +++++++++++-------- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 350f7b9d6bb..9c83654397d 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -68,13 +68,14 @@ private class TextViewStyleCheck { private fun validateTextViewElement(element: Element, filePath: String) { val lineNumber = element.getAttribute(LINE_NUMBER_ATTRIBUTE).toString() val styleAttribute = element.attributes.getNamedItem("style")?.nodeValue - val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue ?: "NO ID" + val idAttribute = element.attributes.getNamedItem("android:id")?.nodeValue if (idAttribute in attributeIds) return if (styleAttribute.isNullOrBlank()) { + val idInformation= idAttribute?.let { "ID: $it" } ?: "" styleValidationIssues.add( - "ERROR: Missing style attribute in file: $filePath, line $lineNumber." + "ERROR: Missing style attribute in file: $filePath, ${idInformation}, line $lineNumber." ) } @@ -168,7 +169,7 @@ private class TextViewStyleCheck { } override fun characters(ch: CharArray, start: Int, length: Int) { - textBuffer.append(ch, start, length) + textBuffer.appendRange(ch, start, start + length) } private fun addTextIfNeeded() { @@ -186,7 +187,7 @@ private class TextViewStyleCheck { } } -// Known exceptions that currently lack a style and are being tracked for fixes. +// TODO(#5661): Add missing styles for TextView IDs. private val attributeIds = listOf( "@+id/developer_options_text_view", "@+id/onboarding_language_text_view", diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel b/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel index da4676beb55..6c61df26f34 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/BUILD.bazel @@ -64,6 +64,7 @@ kt_jvm_test( srcs = ["TextViewStyleCheckTest.kt"], deps = [ "//scripts/src/java/org/oppia/android/scripts/xml:check_textview_styles", + "//testing:assertion_helpers", "//third_party:com_google_truth_truth", "//third_party:org_jetbrains_kotlin_kotlin-test-junit", ], diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index 7b84008e13b..0bf33add298 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -8,6 +8,7 @@ import org.junit.Test import org.junit.rules.TemporaryFolder import java.io.ByteArrayOutputStream import java.io.PrintStream +import org.oppia.android.testing.assertThrows /** Tests for [TextViewStyleCheck]. */ class TextViewStyleCheckTest { @@ -69,10 +70,9 @@ class TextViewStyleCheckTest { createLayoutFile(invalidLayout) - val thrown = catchThrowable { runScript() } - - assertThat(thrown).isInstanceOf(Exception::class.java) + val thrown = assertThrows() { runScript() } assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + assertThat(outContent.toString()).contains("ERROR: Missing style attribute") } @@ -126,11 +126,13 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithMixedTextViews) - val thrown = catchThrowable { runScript() } - - assertThat(thrown).isInstanceOf(Exception::class.java) + val thrown = assertThrows() { runScript() } assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("ERROR: Missing style attribute") + + val output = outContent.toString() + assertThat(output).contains("@+id/second_text_view_no_style") + assertThat(output).contains("line 10") } @Test @@ -178,14 +180,14 @@ class TextViewStyleCheckTest { createLayoutFile(warningLayout, "warning_layout.xml") createLayoutFile(invalidLayout, "invalid_layout.xml") - val thrown = catchThrowable { runScript() } - - assertThat(thrown).isInstanceOf(Exception::class.java) + val thrown = assertThrows() { runScript() } assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val output = outContent.toString() assertThat(output).contains("WARNING: Hardcoded left/right attribute") assertThat(output).contains("ERROR: Missing style attribute") } + @Test fun testTextViewStyle_missingStyle_logsCorrectLineNumber() { val layoutWithLineSpacing = @@ -205,11 +207,11 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithLineSpacing) - val thrown = catchThrowable { runScript() } - - assertThat(thrown).isInstanceOf(Exception::class.java) + val thrown = assertThrows() { runScript() } + assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("line 7") } + @Test fun testTextViewStyle_directionalityWarning_logsCorrectLineNumber() { val layoutWithLineSpacing = @@ -235,6 +237,7 @@ class TextViewStyleCheckTest { assertThat(outContent.toString()).contains("line 9") } + @Test fun testTextViewStyle_multipleTextViews_logsCorrectLineNumbers() { val layoutWithMultipleTextViews = @@ -262,13 +265,15 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithMultipleTextViews) - val thrown = catchThrowable { runScript() } + val thrown = assertThrows() { runScript() } + assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() - assertThat(thrown).isInstanceOf(Exception::class.java) + assertThat(output).contains("@+id/first_text_view") assertThat(output).contains("line 6") assertThat(output).contains("line 13") } + @Test fun testTextViewStyle_nestedTextViews_logsCorrectLineNumbers() { val nestedLayout = @@ -298,10 +303,10 @@ class TextViewStyleCheckTest { createLayoutFile(nestedLayout) - val thrown = catchThrowable { runScript() } + val thrown = assertThrows() { runScript() } + assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() - assertThat(thrown).isInstanceOf(Exception::class.java) assertThat(output).contains("line 10") assertThat(output).contains("line 16") } From 8a77278fe7bd75203268541bd28fcf5ca04c387e Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 22 Jan 2025 18:10:53 +0530 Subject: [PATCH 29/37] formatting --- .../org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index 0bf33add298..4427b875d15 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -6,9 +6,9 @@ import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder +import org.oppia.android.testing.assertThrows import java.io.ByteArrayOutputStream import java.io.PrintStream -import org.oppia.android.testing.assertThrows /** Tests for [TextViewStyleCheck]. */ class TextViewStyleCheckTest { From cf975910ba86799ebc68037e86d92088673d52a6 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 22 Jan 2025 18:12:03 +0530 Subject: [PATCH 30/37] foramtting --- .../java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 9c83654397d..86c13a90e2a 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -73,9 +73,9 @@ private class TextViewStyleCheck { if (idAttribute in attributeIds) return if (styleAttribute.isNullOrBlank()) { - val idInformation= idAttribute?.let { "ID: $it" } ?: "" + val idInformation = idAttribute?.let { "ID: $it" } ?: "" styleValidationIssues.add( - "ERROR: Missing style attribute in file: $filePath, ${idInformation}, line $lineNumber." + "ERROR: Missing style attribute in file: $filePath, $idInformation, line $lineNumber." ) } From 1a7eb37ec853ce21bcf6920243806f6f7e07a6db Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 22 Jan 2025 18:17:23 +0530 Subject: [PATCH 31/37] idinfo logging --- .../java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 86c13a90e2a..cec3f5a00fd 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -73,9 +73,9 @@ private class TextViewStyleCheck { if (idAttribute in attributeIds) return if (styleAttribute.isNullOrBlank()) { - val idInformation = idAttribute?.let { "ID: $it" } ?: "" + val idInformation = idAttribute?.let { " ID: $it," } ?: "" styleValidationIssues.add( - "ERROR: Missing style attribute in file: $filePath, $idInformation, line $lineNumber." + "ERROR: Missing style attribute in file: $filePath,$idInformation line $lineNumber." ) } From 0a02b757488978dd776135173c7e60440bd64100 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 22 Jan 2025 20:10:35 +0530 Subject: [PATCH 32/37] renaming --- .../scripts/xml/TextViewStyleCheckTest.kt | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index 4427b875d15..62c9a8fd12b 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -70,8 +70,8 @@ class TextViewStyleCheckTest { createLayoutFile(invalidLayout) - val thrown = assertThrows() { runScript() } - assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val execption = assertThrows() { runScript() } + assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("ERROR: Missing style attribute") } @@ -126,8 +126,8 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithMixedTextViews) - val thrown = assertThrows() { runScript() } - assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val execption = assertThrows() { runScript() } + assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("ERROR: Missing style attribute") val output = outContent.toString() @@ -180,8 +180,8 @@ class TextViewStyleCheckTest { createLayoutFile(warningLayout, "warning_layout.xml") createLayoutFile(invalidLayout, "invalid_layout.xml") - val thrown = assertThrows() { runScript() } - assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val execption = assertThrows() { runScript() } + assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() assertThat(output).contains("WARNING: Hardcoded left/right attribute") @@ -207,8 +207,8 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithLineSpacing) - val thrown = assertThrows() { runScript() } - assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val execption = assertThrows() { runScript() } + assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("line 7") } @@ -265,8 +265,8 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithMultipleTextViews) - val thrown = assertThrows() { runScript() } - assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val execption = assertThrows() { runScript() } + assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() assertThat(output).contains("@+id/first_text_view") @@ -303,11 +303,12 @@ class TextViewStyleCheckTest { createLayoutFile(nestedLayout) - val thrown = assertThrows() { runScript() } - assertThat(thrown).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val execption = assertThrows() { runScript() } + assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() - assertThat(output).contains("line 10") + assertThat(output).contains("line 10")us + assertThat(output).contains("line 16") } From 4ccf3087560d739a3d3c3b87c630658ad3af9b1e Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 22 Jan 2025 20:13:10 +0530 Subject: [PATCH 33/37] formatting --- .../org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index 62c9a8fd12b..a5f18a37a0c 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -307,8 +307,7 @@ class TextViewStyleCheckTest { assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() - assertThat(output).contains("line 10")us - + assertThat(output).contains("line 10") assertThat(output).contains("line 16") } From 62ac5848bde0ca2f750358ae248f72311fbc9cb5 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Wed, 22 Jan 2025 20:14:07 +0530 Subject: [PATCH 34/37] formatting --- .../scripts/xml/TextViewStyleCheckTest.kt | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index a5f18a37a0c..a6822f1f28a 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -70,8 +70,8 @@ class TextViewStyleCheckTest { createLayoutFile(invalidLayout) - val execption = assertThrows() { runScript() } - assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val exception = assertThrows() { runScript() } + assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("ERROR: Missing style attribute") } @@ -126,8 +126,8 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithMixedTextViews) - val execption = assertThrows() { runScript() } - assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val exception = assertThrows() { runScript() } + assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("ERROR: Missing style attribute") val output = outContent.toString() @@ -180,8 +180,8 @@ class TextViewStyleCheckTest { createLayoutFile(warningLayout, "warning_layout.xml") createLayoutFile(invalidLayout, "invalid_layout.xml") - val execption = assertThrows() { runScript() } - assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val exception = assertThrows() { runScript() } + assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() assertThat(output).contains("WARNING: Hardcoded left/right attribute") @@ -207,8 +207,8 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithLineSpacing) - val execption = assertThrows() { runScript() } - assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val exception = assertThrows() { runScript() } + assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("line 7") } @@ -265,8 +265,8 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithMultipleTextViews) - val execption = assertThrows() { runScript() } - assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val exception = assertThrows() { runScript() } + assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() assertThat(output).contains("@+id/first_text_view") @@ -303,8 +303,8 @@ class TextViewStyleCheckTest { createLayoutFile(nestedLayout) - val execption = assertThrows() { runScript() } - assertThat(execption).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) + val exception = assertThrows() { runScript() } + assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() assertThat(output).contains("line 10") From c9af6ad71bcd6425d19b3f91b46fcee98a792a8f Mon Sep 17 00:00:00 2001 From: manas-yu Date: Thu, 23 Jan 2025 00:31:54 +0530 Subject: [PATCH 35/37] exception handling --- .../android/scripts/xml/TextViewStyleCheck.kt | 2 +- .../scripts/xml/TextViewStyleCheckTest.kt | 22 ++++++------------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index cec3f5a00fd..8c4d0f7e7cc 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -111,7 +111,7 @@ private class TextViewStyleCheck { if (styleValidationIssues.isNotEmpty()) { styleValidationIssues.forEach { println(it) } - throw Exception("TEXTVIEW STYLE CHECK FAILED") + error("TEXTVIEW STYLE CHECK FAILED") } else { println("TEXTVIEW STYLE CHECK PASSED") } diff --git a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt index a6822f1f28a..b0251fa0966 100644 --- a/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt +++ b/scripts/src/javatests/org/oppia/android/scripts/xml/TextViewStyleCheckTest.kt @@ -9,6 +9,7 @@ import org.junit.rules.TemporaryFolder import org.oppia.android.testing.assertThrows import java.io.ByteArrayOutputStream import java.io.PrintStream +import java.lang.IllegalStateException /** Tests for [TextViewStyleCheck]. */ class TextViewStyleCheckTest { @@ -70,7 +71,7 @@ class TextViewStyleCheckTest { createLayoutFile(invalidLayout) - val exception = assertThrows() { runScript() } + val exception = assertThrows() { runScript() } assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("ERROR: Missing style attribute") @@ -126,7 +127,7 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithMixedTextViews) - val exception = assertThrows() { runScript() } + val exception = assertThrows() { runScript() } assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("ERROR: Missing style attribute") @@ -180,7 +181,7 @@ class TextViewStyleCheckTest { createLayoutFile(warningLayout, "warning_layout.xml") createLayoutFile(invalidLayout, "invalid_layout.xml") - val exception = assertThrows() { runScript() } + val exception = assertThrows() { runScript() } assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() @@ -207,7 +208,7 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithLineSpacing) - val exception = assertThrows() { runScript() } + val exception = assertThrows() { runScript() } assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) assertThat(outContent.toString()).contains("line 7") } @@ -265,7 +266,7 @@ class TextViewStyleCheckTest { createLayoutFile(layoutWithMultipleTextViews) - val exception = assertThrows() { runScript() } + val exception = assertThrows() { runScript() } assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() @@ -303,7 +304,7 @@ class TextViewStyleCheckTest { createLayoutFile(nestedLayout) - val exception = assertThrows() { runScript() } + val exception = assertThrows() { runScript() } assertThat(exception).hasMessageThat().contains(TEXTVIEW_STYLE_CHECK_FAILED_OUTPUT_INDICATOR) val output = outContent.toString() @@ -319,13 +320,4 @@ class TextViewStyleCheckTest { private fun runScript() { main(tempFolder.root.absolutePath) } - - private fun catchThrowable(executable: () -> Unit): Throwable { - try { - executable() - throw AssertionError("Expected to throw, but did not") - } catch (e: Throwable) { - return e - } - } } From 7e7b863e629405575b3f63accd81d2757cdabb98 Mon Sep 17 00:00:00 2001 From: manas-yu Date: Thu, 23 Jan 2025 16:35:39 +0530 Subject: [PATCH 36/37] adding id attributes --- app/src/main/res/layout-land/profile_chooser_fragment.xml | 2 ++ app/src/main/res/layout-land/walkthrough_final_fragment.xml | 2 ++ .../res/layout-sw600dp-land/topic_lessons_story_summary.xml | 1 + app/src/main/res/layout-sw600dp/profile_chooser_fragment.xml | 2 ++ app/src/main/res/layout/numeric_input_interaction_item.xml | 1 + app/src/main/res/layout/profile_chooser_fragment.xml | 2 ++ app/src/main/res/layout/topic_lessons_story_summary.xml | 1 + app/src/main/res/layout/walkthrough_final_fragment.xml | 2 ++ 8 files changed, 13 insertions(+) diff --git a/app/src/main/res/layout-land/profile_chooser_fragment.xml b/app/src/main/res/layout-land/profile_chooser_fragment.xml index 661da1b5244..dcc2719fdc1 100644 --- a/app/src/main/res/layout-land/profile_chooser_fragment.xml +++ b/app/src/main/res/layout-land/profile_chooser_fragment.xml @@ -40,6 +40,7 @@ android:contentDescription="@string/language_icon_content_description" /> Date: Tue, 28 Jan 2025 13:36:51 +0530 Subject: [PATCH 37/37] edits --- .../org/oppia/android/scripts/xml/TextViewStyleCheck.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt index 8c4d0f7e7cc..b840bc1ece7 100644 --- a/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt +++ b/scripts/src/java/org/oppia/android/scripts/xml/TextViewStyleCheck.kt @@ -55,7 +55,7 @@ private class TextViewStyleCheck { } private fun processXmlFile(file: File) { - val document = readXMLWithLineNumbers(FileInputStream(file), LINE_NUMBER_ATTRIBUTE) + val document = readXmlWithLineNumbers(FileInputStream(file), LINE_NUMBER_ATTRIBUTE) val textViewNodes = document.getElementsByTagName("TextView") val relativePath = file.path.substringAfter("main/res/") @@ -75,7 +75,7 @@ private class TextViewStyleCheck { if (styleAttribute.isNullOrBlank()) { val idInformation = idAttribute?.let { " ID: $it," } ?: "" styleValidationIssues.add( - "ERROR: Missing style attribute in file: $filePath,$idInformation line $lineNumber." + "ERROR: Missing style attribute in file: $filePath, $idInformation line $lineNumber." ) } @@ -117,7 +117,7 @@ private class TextViewStyleCheck { } } - private fun readXMLWithLineNumbers(inputStream: FileInputStream, lineNumAttribName: String): + private fun readXmlWithLineNumbers(inputStream: FileInputStream, lineNumAttribName: String): Document { val document: Document val parser: SAXParser