Fix #3300: Add check for accessibility label for activities

.github/workflows/static_checks.yml

@@ -118,3 +118,8 @@ jobs:
         if: always()
         run: |
           bazel run //scripts:test_file_check -- $(pwd)
+
+      - name: Accessibility label check
+        if: always()
+        run: |
+          bazel run //scripts:accessibility_label_check -- $(pwd) app/src/main/AndroidManifest.xml

scripts/BUILD.bazel

@@ -5,6 +5,7 @@ Kotlin-based scripts to help developers or perform continuous integration tasks.
 load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_binary", "kt_jvm_library")
 load(
     "//scripts:script_assets.bzl",
+    "generate_accessibility_label_assets_list_from_text_protos",
     "generate_regex_assets_list_from_text_protos",
     "generate_test_file_assets_list_from_text_protos",
 )
@@ -98,3 +99,25 @@ kt_jvm_binary(
     main_class = "org.oppia.android.scripts.testfile.TestFileCheckKt",
     runtime_deps = ["//scripts/src/java/org/oppia/android/scripts/testfile:test_file_check_lib"],
 )
+
+ACCESSIBILITY_LABEL_EXEMPTION_ASSETS = generate_accessibility_label_assets_list_from_text_protos(
+    name = "accessibility_label_exemption_assets",
+    accessibility_label_exemptions_name = ["accessibility_label_exemptions"],
+)
+
+kt_jvm_library(
+    name = "accessibility_label_check_assets",
+    testonly = True,
+    data = ACCESSIBILITY_LABEL_EXEMPTION_ASSETS,
+    visibility = [":oppia_script_test_visibility"],
+)
+
+kt_jvm_binary(
+    name = "accessibility_label_check",
+    testonly = True,
+    data = ACCESSIBILITY_LABEL_EXEMPTION_ASSETS,
+    main_class = "org.oppia.android.scripts.label.AccessibilityLabelCheckKt",
+    runtime_deps = [
+        "//scripts/src/java/org/oppia/android/scripts/label:accessibility_label_check_lib",
+    ],
+)

scripts/assets/accessibility_label_exemptions.textproto

@@ -0,0 +1,27 @@
+exempted_activity: "app/home/HomeActivity"
+exempted_activity: "app/home/recentlyplayed/RecentlyPlayedActivity"
+exempted_activity: "app/mydownloads/MyDownloadsActivity"
+exempted_activity: "app/player/state/testing/StateFragmentTestActivity"
+exempted_activity: "app/profileprogress/ProfilePictureActivity"
+exempted_activity: "app/settings/profile/ProfileEditActivity"
+exempted_activity: "app/splash/SplashActivity"
+exempted_activity: "app/testing/AudioFragmentTestActivity"
+exempted_activity: "app/testing/BindableAdapterTestActivity"
+exempted_activity: "app/testing/ConceptCardFragmentTestActivity"
+exempted_activity: "app/testing/DragDropTestActivity"
+exempted_activity: "app/testing/ExplorationInjectionActivity"
+exempted_activity: "app/testing/ExplorationTestActivity"
+exempted_activity: "app/testing/HomeFragmentTestActivity"
+exempted_activity: "app/testing/HomeTestActivity"
+exempted_activity: "app/testing/HtmlParserTestActivity"
+exempted_activity: "app/testing/ImageRegionSelectionTestActivity"
+exempted_activity: "app/testing/InputInteractionViewTestActivity"
+exempted_activity: "app/testing/LessonThumbnailImageViewTestActivity"
+exempted_activity: "app/testing/NavigationDrawerTestActivity"
+exempted_activity: "app/testing/ProfileChooserFragmentTestActivity"
+exempted_activity: "app/testing/SplashTestActivity"
+exempted_activity: "app/testing/TestFontScaleConfigurationUtilActivity"
+exempted_activity: "app/testing/TopicRevisionTestActivity"
+exempted_activity: "app/testing/TopicTestActivity"
+exempted_activity: "app/testing/TopicTestActivityForStory"
+exempted_activity: "app/walkthrough/WalkthroughActivity"

scripts/script_assets.bzl

@@ -62,3 +62,28 @@ def generate_test_file_assets_list_from_text_protos(
         proto_dep_bazel_target_prefix = "//scripts/src/java/org/oppia/android/scripts/proto",
         proto_package = "proto",
     )
+
+def generate_accessibility_label_assets_list_from_text_protos(
+        name,
+        accessibility_label_exemptions_name):
+    """
+    Converts multiple lists of text proto assets to binary.
+
+    Args:
+        name: str. The name of this generation instance. This will be a prefix for derived targets.
+        accessibility_label_exemptions_name: list of str. The list of accessibility label exemptions
+         file names.
+
+    Returns:
+        list of str. The list of new proto binary asset files that were generated.
+    """
+    return generate_proto_binary_assets(
+        name = name,
+        names = accessibility_label_exemptions_name,
+        proto_dep_name = "accessibility_label_exemptions",
+        proto_type_name = "AccessibilityLabelExemptions",
+        name_prefix = name,
+        asset_dir = "assets",
+        proto_dep_bazel_target_prefix = "//scripts/src/java/org/oppia/android/scripts/proto",
+        proto_package = "proto",
+    )

scripts/src/java/org/oppia/android/scripts/label/AccessibilityLabelCheck.kt

@@ -0,0 +1,156 @@
+package org.oppia.android.scripts.label
+
+import org.oppia.android.scripts.proto.AccessibilityLabelExemptions
+import org.w3c.dom.Node
+import org.w3c.dom.NodeList
+import java.io.File
+import java.io.FileInputStream
+import java.util.stream.Collectors
+import java.util.stream.IntStream
+import javax.xml.parsers.DocumentBuilderFactory
+
+/**
+ * Script for ensuring that all the Activities in the codebase are defined with accessibility
+ * labels.
+ *
+ * Usage:
+ *   bazel run //scripts:accessibility_label_check -- <path_to_directory_root> <path_to_app_level
+ *   _manifest_file>
+ *
+ * Arguments:
+ * - path_to_directory_root: directory path to the root of the Oppia Android repository.
+ * - path_to_app_level_manifest_file: path to the manifest file of app layer.
+ *
+ * Example:
+ *   bazel run //scripts:accessibility_label_check -- $(pwd) app/src/main/AndroidManifest.xml
+ */
+fun main(vararg args: String) {
+  val repoPath = "${args[0]}/"
+
+  val accessibilityLabelExemptiontextProto = "scripts/assets/accessibility_label_exemptions"
+
+  val accessibilityLabelExemptionList = loadAccessibilityLabelExemptionsProto(
+    accessibilityLabelExemptiontextProto
+  ).getExemptedActivityList()
+
+  val manifesFilePath = args[1]
+
+  val fullPathToManifestFile = repoPath + manifesFilePath
+
+  val builderFactory = DocumentBuilderFactory.newInstance()
+
+  val docBuilder = builderFactory.newDocumentBuilder()
+
+  val doc = docBuilder.parse(File(fullPathToManifestFile))
+  doc.getDocumentElement().normalize()
+
+  val completeActivityList = convertNodeListToListOfNode(doc.getElementsByTagName("activity"))
+
+  val activityListWithoutLabel = completeActivityList.filter { node ->
+    checkIfActivityHasMissingLabel(node, accessibilityLabelExemptionList)
+  }
+
+  logFailures(activityListWithoutLabel, repoPath, accessibilityLabelExemptiontextProto)
+
+  if (activityListWithoutLabel.isNotEmpty()) {
+    throw Exception("ACCESSIBILITY LABEL CHECK FAILED")
+  } else {
+    println("ACCESSIBILITY LABEL CHECK PASSED")
+  }
+}
+
+/**
+ * Checks whether an activity element has a missing label.
+ *
+ * @param activityNode instance of Node
+ * @param accessibilityLabelExemptionList list of all the exemptions of the label check
+ * @return whether the label is present or not
+ */
+private fun checkIfActivityHasMissingLabel(
+  activityNode: Node,
+  accessibilityLabelExemptionList: List<String>
+): Boolean {
+  val attributesList = activityNode.getAttributes()
+  val activityPath = attributesList.getNamedItem("android:name").getNodeValue()
+  if (
+    activityPath
+      .removePrefix(".")
+      .replace(".", "/") in accessibilityLabelExemptionList
+  ) {
+    return false
+  }
+  return attributesList.getNamedItem("android:label") == null
+}
+
+/**
+ * The [nodeList] is not iterable. This helper
+ * function converts it to List<Node>.
+ *
+ * @param nodeList instance of NodeList
+ * @return a list of nodes
+ */
+private fun convertNodeListToListOfNode(nodeList: NodeList): List<Node> {
+  return IntStream.range(0, nodeList.getLength())
+    .mapToObj(nodeList::item)
+    .collect(Collectors.toList())
+}
+
+/**
+ * Logs the failures for accessibility label check.
+ *
+ * @param matchedNodes a list of nodes having missing label
+ * @param repoPath path of the repo to be analyzed
+ */
+private fun logFailures(
+  matchedNodes: List<Node>,
+  repoPath: String,
+  accessibilityLabelExemptiontextProto: String
+) {
+  val pathPrefix = "${repoPath}app/src/main/java/org/oppia/android"
+  if (matchedNodes.isNotEmpty()) {
+    println("Accessiblity labels missing for Activities:")
+    matchedNodes.sortedBy {
+      it.getAttributes()
+        .getNamedItem("android:name")
+        .getNodeValue()
+    }.forEach { node ->
+      println(
+        "- $pathPrefix" +
+          "${
+          node.getAttributes()
+            .getNamedItem("android:name")
+            .getNodeValue()
+            .replace(".", "/")
+          }"
+      )
+    }
+    println("If this is correct, please update $accessibilityLabelExemptiontextProto.textproto")
+    println(
+      "Note that, in general, all Activities should have labels. If you choose to add an" +
+        " exemption, please specifically call this out in your PR description."
+    )
+    println()
+  }
+}
+
+/**
+ * Loads the test file exemptions list to proto.
+ *
+ * @param accessibilityLabelExemptiontextProto the location of the accessibility label exemption
+ *     textproto file.
+ * @return proto class from the parsed textproto file
+ */
+private fun loadAccessibilityLabelExemptionsProto(accessibilityLabelExemptiontextProto: String):
+  AccessibilityLabelExemptions {
+    val protoBinaryFile = File("$accessibilityLabelExemptiontextProto.pb")
+    val builder = AccessibilityLabelExemptions.getDefaultInstance().newBuilderForType()
+
+    // This cast is type-safe since proto guarantees type consistency from mergeFrom(),
+    // and this method is bounded by the generic type T.
+    @Suppress("UNCHECKED_CAST")
+    val protoObj: AccessibilityLabelExemptions =
+      FileInputStream(protoBinaryFile).use {
+        builder.mergeFrom(it)
+      }.build() as AccessibilityLabelExemptions
+    return protoObj
+  }

scripts/src/java/org/oppia/android/scripts/label/BUILD.bazel

@@ -0,0 +1,17 @@
+"""
+Libraries corresponding to accessibility label check that ensures all the activities in the codebase
+are defined with labels.
+"""
+
+load("@io_bazel_rules_kotlin//kotlin:kotlin.bzl", "kt_jvm_library")
+
+kt_jvm_library(
+    name = "accessibility_label_check_lib",
+    testonly = True,
+    srcs = ["AccessibilityLabelCheck.kt"],
+    visibility = ["//scripts:oppia_script_binary_visibility"],
+    deps = [
+        "//scripts/src/java/org/oppia/android/scripts/common:repository_file",
+        "//scripts/src/java/org/oppia/android/scripts/proto:accessibility_label_exemptions_java_proto_lite",
+    ],
+)

scripts/src/java/org/oppia/android/scripts/proto/BUILD.bazel

@@ -44,3 +44,15 @@ java_lite_proto_library(
     visibility = ["//scripts:oppia_script_library_visibility"],
     deps = [":test_file_exemptions_proto"],
 )
+
+proto_library(
+    name = "accessibility_label_exemptions_proto",
+    srcs = ["script_exemptions.proto"],
+    visibility = ["//scripts:oppia_script_binary_visibility"],
+)
+
+java_lite_proto_library(
+    name = "accessibility_label_exemptions_java_proto_lite",
+    visibility = ["//scripts:oppia_script_library_visibility"],
+    deps = [":accessibility_label_exemptions_proto"],
+)

scripts/src/java/org/oppia/android/scripts/proto/script_exemptions.proto

@@ -19,3 +19,19 @@ message TestFileExemptions {
   // at the correct lexicographical position in the textproto file.
   repeated string exempted_file_path = 1;
 }
+
+// Exemptions for the accessibility label check. Exemptions indicate Activities for which we should
+// not check for the presence of an accessibility label.
+message AccessibilityLabelExemptions {
+  // List of all the Activities which should be exempted for the accessibility label check.
+  // For exempting any Activity, provide its path w.r.t app in
+  // 'script/assets/accessibility_label_exemptions.textproto'
+  // Also, note that the exemptions in the textproto file are maintained in lexicographical order.
+  // While adding any new Activity, please add it only at the correct lexicographical position,
+  // so that the list remains sorted.
+  //
+  // For example if we want to add the 'RecentlyPlayedActivity' to the exemption list, add:
+  // 'exempted_activity: "app/home/recentlyplayed/RecentlyPlayedActivity"'
+  // at the correct lexicographical position in the textproto file.
+  repeated string exempted_activity = 1;
+}