Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #4689: Add regex check to prohibit post() and postDelayed() #4900

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions scripts/assets/file_content_validation_checks.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -454,3 +454,14 @@ file_content_checks {
exempted_file_name: "app/src/main/java/org/oppia/android/app/application/AbstractOppiaApplication.kt"
exempted_file_patterns: ".+?Test\\.kt"
}
file_content_checks {
file_path_regex: ".+?.kt"
prohibited_content_regex: ".+?\\.(post|postDelayed)[\\s]*(\\(|\\{).*?"
failure_message: "Prefer avoiding post() and postDelayed() methods as they can can lead to subtle and difficult-to-debug crashes. Prefer using LifecycleSafeTimerFactory for most cases when an operation needs to run at a future time. For cases when state needs to be synchronized with a view, use doOnPreDraw or doOnLayout instead. For more context on the underlying issue, see: https://betterprogramming.pub/stop-using-post-postdelayed-in-your-android-views-9d1c8eeaadf2."
exempted_file_name: "app/src/main/java/org/oppia/android/app/drawer/NavigationDrawerFragmentPresenter.kt"
exempted_file_name: "app/src/main/java/org/oppia/android/app/player/state/StateFragmentPresenter.kt"
exempted_file_name: "app/src/main/java/org/oppia/android/app/topic/info/TopicInfoFragmentPresenter.kt"
exempted_file_name: "app/src/main/java/org/oppia/android/app/utility/ClickableAreasImage.kt"
exempted_file_name: "scripts/src/javatests/org/oppia/android/scripts/regex/RegexPatternValidationCheckTest.kt"
exempted_file_name: "utility/src/main/java/org/oppia/android/util/parser/image/UrlImageParser.kt"
}
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,13 @@ class RegexPatternValidationCheckTest {
"Use AnalyticsStartupListener to retrieve an instance of WorkManager rather than fetching one" +
" using getInstance (as the latter may create a WorkManager if one isn't already present, " +
"and the application may intend to disable it)."
private val doesNotUsePostOrPostDelayed =
"Prefer avoiding post() and postDelayed() methods as they can can lead to subtle and " +
"difficult-to-debug crashes. Prefer using LifecycleSafeTimerFactory for most cases when " +
"an operation needs to run at a future time. For cases when state needs to be synchronized " +
"with a view, use doOnPreDraw or doOnLayout instead. For more context on the underlying " +
"issue, see: https://betterprogramming.pub/stop-using-post-postdelayed-in-your" +
"-android-views-9d1c8eeaadf2."
private val wikiReferenceNote =
"Refer to https://github.com/oppia/oppia-android/wiki/Static-Analysis-Checks" +
"#regexpatternvalidation-check for more details on how to fix this."
Expand Down Expand Up @@ -2409,6 +2416,81 @@ class RegexPatternValidationCheckTest {
)
}

@Test
fun testFileContent_postDelayedUsed_fileContentIsNotCorrect() {
val prohibitedContent =
"""
binding.view.postDelayed({ binding.view.visibility = View.GONE }, 1000)
""".trimIndent()
tempFolder.newFolder("testfiles", "app", "src", "main", "java", "org", "oppia", "android")
val stringFilePath = "app/src/main/java/org/oppia/android/TestPresenter.kt"
tempFolder.newFile("testfiles/$stringFilePath").writeText(prohibitedContent)

val exception = assertThrows(Exception::class) {
runScript()
}

assertThat(exception).hasMessageThat().contains(REGEX_CHECK_FAILED_OUTPUT_INDICATOR)

assertThat(outContent.toString().trim())
.isEqualTo(
"""
$stringFilePath:1: $doesNotUsePostOrPostDelayed
$wikiReferenceNote
""".trimIndent()
)
}

@Test
fun testFileContent_postUsed_withParenthesis_fileContentIsNotCorrect() {
val prohibitedContent =
"""
binding.view.post({ binding.view.visibility = View.GONE })
""".trimIndent()
tempFolder.newFolder("testfiles", "app", "src", "main", "java", "org", "oppia", "android")
val stringFilePath = "app/src/main/java/org/oppia/android/TestPresenter.kt"
tempFolder.newFile("testfiles/$stringFilePath").writeText(prohibitedContent)

val exception = assertThrows(Exception::class) {
runScript()
}

assertThat(exception).hasMessageThat().contains(REGEX_CHECK_FAILED_OUTPUT_INDICATOR)
BenHenning marked this conversation as resolved.
Show resolved Hide resolved

assertThat(outContent.toString().trim())
.isEqualTo(
"""
$stringFilePath:1: $doesNotUsePostOrPostDelayed
$wikiReferenceNote
""".trimIndent()
)
}

@Test
fun testFileContent_postUsed_withCurlyBraces_fileContentIsNotCorrect() {
val prohibitedContent =
"""
binding.view.post { binding.view.visibility = View.GONE }
""".trimIndent()
tempFolder.newFolder("testfiles", "app", "src", "main", "java", "org", "oppia", "android")
val stringFilePath = "app/src/main/java/org/oppia/android/TestPresenter.kt"
tempFolder.newFile("testfiles/$stringFilePath").writeText(prohibitedContent)

val exception = assertThrows(Exception::class) {
runScript()
}

assertThat(exception).hasMessageThat().contains(REGEX_CHECK_FAILED_OUTPUT_INDICATOR)

assertThat(outContent.toString().trim())
.isEqualTo(
"""
$stringFilePath:1: $doesNotUsePostOrPostDelayed
$wikiReferenceNote
""".trimIndent()
)
}

/** Runs the regex_pattern_validation_check. */
private fun runScript() {
main(File(tempFolder.root, "testfiles").absolutePath)
Expand Down