From 6937e1eadbc517b9bddaed5c98fadb91980f0607 Mon Sep 17 00:00:00 2001 From: Vishal Kumar Date: Fri, 22 Nov 2024 18:43:44 -0800 Subject: [PATCH] Fix migration issue when multiple resource access code found on single line. --- .../file/generation/ReportGenerator.kt | 2 +- .../manager/MigrationManager.kt | 164 +++++++++--------- .../manager/MigrationManagerTest.kt | 18 ++ .../mock/main_activity_kt_after_migration.txt | 5 + .../main_activity_kt_before_migration.txt | 4 + 5 files changed, 108 insertions(+), 85 deletions(-) diff --git a/resourcemanager/src/main/kotlin/dev/randos/resourcemanager/file/generation/ReportGenerator.kt b/resourcemanager/src/main/kotlin/dev/randos/resourcemanager/file/generation/ReportGenerator.kt index 57b6102..1f2480e 100644 --- a/resourcemanager/src/main/kotlin/dev/randos/resourcemanager/file/generation/ReportGenerator.kt +++ b/resourcemanager/src/main/kotlin/dev/randos/resourcemanager/file/generation/ReportGenerator.kt @@ -97,7 +97,7 @@ internal object ReportGenerator { appendLine(5, "Updated Code") appendLine(4, "") - for (change in file.changes) { + for (change in file.changes.sortedBy { it.lineNumber }) { appendLine(4, "") appendLine(5, "${change.lineNumber}") appendLine(5, "${change.current.escapeHtml()}") diff --git a/resourcemanager/src/main/kotlin/dev/randos/resourcemanager/manager/MigrationManager.kt b/resourcemanager/src/main/kotlin/dev/randos/resourcemanager/manager/MigrationManager.kt index d2b4a00..8af0228 100644 --- a/resourcemanager/src/main/kotlin/dev/randos/resourcemanager/manager/MigrationManager.kt +++ b/resourcemanager/src/main/kotlin/dev/randos/resourcemanager/manager/MigrationManager.kt @@ -57,7 +57,9 @@ internal class MigrationManager( getOneParamRegex("getDrawable", "drawable") to "Drawables", getOneParamRegex("getQuantityString", "plurals") to "Plurals", getOneParamRegex("getString", "string") to "Strings", - getTwoParamRegex("getFraction", "fraction") to "Fractions" + getTwoParamRegex("getFraction", "fraction") to "Fractions", + getContextCompatRegex("getDrawable", "drawable") to "Drawables", + getContextCompatRegex("getColor", "color") to "Colors" ) } @@ -78,6 +80,19 @@ internal class MigrationManager( "Fractions" to "fraction" ) + /** + * Constructs a regex pattern to match resource retrieval methods which uses ContextCompact. + * + * Example match: + * - `ContextCompat.getDrawable(baseContext, R.drawable.ic_repeat)` + */ + private fun getContextCompatRegex( + methodName: String, + resourceType: String + ): Regex { + return Regex("(?:ContextCompat|AppCompatResources).${methodName}\\(\\s*(\\w+)\\s*,\\s*R\\.${resourceType}\\.(\\w+)\\s*\\)") + } + /** * Constructs a regex pattern to match resource retrieval methods with no additional parameter. * @@ -88,7 +103,7 @@ internal class MigrationManager( methodName: String, resourceType: String ): Regex { - return Regex("${common}${methodName}\\(\\s*(.+)?R\\.${resourceType}\\.(\\w+)\\s*\\)") + return Regex("${common}${methodName}\\(\\s*([\\w.]+)?R\\.${resourceType}\\.(\\w+)\\s*\\)") } /** @@ -101,7 +116,7 @@ internal class MigrationManager( methodName: String, resourceType: String ): Regex { - return Regex("${common}${methodName}\\(\\s*(.+)?R\\.${resourceType}\\.(\\w+),\\s*(.+)\\s*\\)") + return Regex("${common}${methodName}\\(\\s*([\\w.]+)?R\\.${resourceType}\\.(\\w+),\\s*(.+)\\s*\\)") } /** @@ -114,7 +129,7 @@ internal class MigrationManager( methodName: String, resourceType: String ): Regex { - return Regex("${common}${methodName}\\(\\s*(.+)?R\\.${resourceType}\\.(\\w+),\\s*(.+)\\s*,\\s*(.+)\\s*\\)") + return Regex("${common}${methodName}\\(\\s*([\\w.]+)?R\\.${resourceType}\\.(\\w+),\\s*(.+)\\s*,\\s*(.+)\\s*\\)") } fun migrate() { @@ -133,6 +148,7 @@ internal class MigrationManager( sourceFiles.forEach { sourceFile -> var currentResourceImportStatement: String? = null + var currentResourceImportStatementIndex = 0 val changes = mutableListOf() @@ -145,109 +161,89 @@ internal class MigrationManager( val matchResultImport = importStatementRegex.find(line) if (matchResultImport?.groups?.size == 2) { currentResourceImportStatement = matchResultImport.groups[0]?.value.orEmpty() + currentResourceImportStatementIndex = index } - var matchResultResource: MatchResult? = null + var matchResultResources: Sequence + var lineToAppend = line // Check if it is resource access, iterate through replacement regex for ((regex, resourcePath) in replacements) { - matchResultResource = regex.find(line) - if (matchResultResource == null) continue - - val moduleNamespace = - matchResultResource.groups[1]?.value.orEmpty() // 'com.example.mylibrary1.' in case of -> getString(com.example.mylibrary1.R.string.fun_title) - val resourceId = - matchResultResource.groups[2]?.value.orEmpty() // 'fun_title' in case of -> getString(com.example.mylibrary1.R.string.fun_title)/getString(R.string.fun_title) - - // If we don't have this resourceId in our set, simply skip this. - if (!resourceIds.contains(resourceId)) { - matchResultResource = null - log(sourceFile, index, false, "ResourceId not found in generated ResourceManager class.") - break - } - - val completeResourceId = "R.${androidResMap[resourcePath]}.$resourceId" - val functionNames = resourceIdFunctionNameMap[completeResourceId] - if (functionNames == null) { - matchResultResource = null - log(sourceFile, index, false, "ResourceId not found in generated ResourceManager class.") - break - } + matchResultResources = regex.findAll(lineToAppend) - // TODO improve this so it handles situation when there are more than one function names for a given resourceId. - if (functionNames.size > 1) { - matchResultResource = null - log(sourceFile, index, false, "More than one function names found for this ResourceId.") - break - } + for (matchResultResource in matchResultResources) { + val moduleNamespace = + matchResultResource.groups[1]?.value.orEmpty() // 'com.example.mylibrary1.' in case of -> getString(com.example.mylibrary1.R.string.fun_title) + val resourceId = + matchResultResource.groups[2]?.value.orEmpty() // 'fun_title' in case of -> getString(com.example.mylibrary1.R.string.fun_title)/getString(R.string.fun_title) - val resourceCallReplacementString = "ResourceManager.$resourcePath.${resourceIdFunctionNameMap[completeResourceId]?.first()}" - - // Apply changes to fileContent based on group size. - when (matchResultResource.groups.size) { - // Group size will be 3 when there is no params passed to resource access code. - 3 -> { - val currentResourceAccessCode = matchResultResource.value - val newResourceAccessCode = "$resourceCallReplacementString()" - - val newLine = line.replace(currentResourceAccessCode, newResourceAccessCode) - fileContent.appendLine(newLine) - changes.add( - Change(index, currentResourceAccessCode, newResourceAccessCode) - ) - log(sourceFile, index) + // If we don't have this resourceId in our set, simply skip this. + if (!resourceIds.contains(resourceId)) { + log(sourceFile, index, false, "ResourceId not found in generated ResourceManager class.") break } - // Group size will be 4 when there is one params passed to resource access code. - 4 -> { - val param = matchResultResource.groups[3]?.value.orEmpty() - val currentResourceAccessCode = matchResultResource.value - val newResourceAccessCode = "$resourceCallReplacementString($param)" - - val newLine = line.replace(currentResourceAccessCode, newResourceAccessCode) - fileContent.appendLine(newLine) - changes.add( - Change(index, currentResourceAccessCode, newResourceAccessCode) - ) - log(sourceFile, index) + val completeResourceId = "R.${androidResMap[resourcePath]}.$resourceId" + val functionNames = resourceIdFunctionNameMap[completeResourceId] + if (functionNames == null) { + log(sourceFile, index, false, "ResourceId not found in generated ResourceManager class.") break } - // Group size will be 5 when there is two params passed to resource access code. - 5 -> { - val param1 = matchResultResource.groups[3]?.value.orEmpty() - val param2 = matchResultResource.groups[4]?.value.orEmpty() - val currentResourceAccessCode = matchResultResource.value - val newResourceAccessCode = "$resourceCallReplacementString($param1, $param2)" - - val newLine = - line.replace(currentResourceAccessCode, newResourceAccessCode) - fileContent.appendLine(newLine) - changes.add( - Change(index, currentResourceAccessCode, newResourceAccessCode) - ) - log(sourceFile, index) + // TODO improve this so it handles situation when there are more than one function names for a given resourceId. + if (functionNames.size > 1) { + log(sourceFile, index, false, "More than one function names found for this ResourceId.") break } - else -> { - matchResultResource = null + val resourceCallReplacementString = "ResourceManager.$resourcePath.${resourceIdFunctionNameMap[completeResourceId]?.first()}" + + var currentResourceAccessCode: String + var newResourceAccessCode: String + // Apply changes to fileContent based on group size. + when (matchResultResource.groups.size) { + // Group size will be 2 when ContextCompat is used to access resource. + 2 -> { + currentResourceAccessCode = matchResultResource.value + newResourceAccessCode = "$resourceCallReplacementString()" + } + + // Group size will be 3 when there is no params passed to resource access code. + 3 -> { + currentResourceAccessCode = matchResultResource.value + newResourceAccessCode = "$resourceCallReplacementString()" + } + + // Group size will be 4 when there is one param passed to resource access code. + 4 -> { + val param = matchResultResource.groups[3]?.value.orEmpty() + currentResourceAccessCode = matchResultResource.value + newResourceAccessCode = "$resourceCallReplacementString($param)" + } + + // Group size will be 5 when there is two params passed to resource access code. + 5 -> { + val param1 = matchResultResource.groups[3]?.value.orEmpty() + val param2 = matchResultResource.groups[4]?.value.orEmpty() + currentResourceAccessCode = matchResultResource.value + newResourceAccessCode = "$resourceCallReplacementString($param1, $param2)" + } + + else -> { + continue + } } + lineToAppend = lineToAppend.replace(currentResourceAccessCode, newResourceAccessCode) + changes.add(Change(index, currentResourceAccessCode, newResourceAccessCode)) + log(sourceFile, index) } } - - // Skip this line if already processed by matchResultResource. - if (matchResultResource != null) { - continue - } - - fileContent.appendLine(line) + fileContent.appendLine(lineToAppend) } // If some changes are done to the file content rewrite the file content. if (changes.isNotEmpty()) { - val contentToWrite = getContentToWrite(fileContent, sourceFile, namespace, currentResourceImportStatement, changes, index) + val contentToWrite = getContentToWrite(fileContent, sourceFile, namespace, currentResourceImportStatement, changes, currentResourceImportStatementIndex) sourceFile.writeText(contentToWrite) updatedSourceFiles.add( SourceFileDetails(sourceFile.name, sourceFile.absolutePath, changes) diff --git a/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/manager/MigrationManagerTest.kt b/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/manager/MigrationManagerTest.kt index fce99c1..6555813 100644 --- a/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/manager/MigrationManagerTest.kt +++ b/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/manager/MigrationManagerTest.kt @@ -86,6 +86,24 @@ class MigrationManagerTest { assertEquals(expectedResult, sourceFile.readText()) } + @Test + fun run_whenMigrationTaskRunMultipleTimes_shouldUpdateGivenFileAccordinglyAndResultShouldNotChange() { + // Given + val sourceDirs = File(app, "src/main/kotlin/com/example/app") + sourceDirs.mkdirs() + val sourceFile = File(sourceDirs, "MainActivity.kt") + sourceFile.writeText(MockFileReader.read("main_activity_kt_before_migration.txt")) + + // When + migrationManager.migrate() + migrationManager.migrate() + migrationManager.migrate() + + // Then + val expectedResult = MockFileReader.read("main_activity_kt_after_migration.txt") + assertEquals(expectedResult, sourceFile.readText()) + } + @Test fun run_whenSourceFileHaveResourceImportStatementFromModule_shouldUpdateFileAccordingly() { // Given diff --git a/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/mock/main_activity_kt_after_migration.txt b/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/mock/main_activity_kt_after_migration.txt index 86db7c5..15cfe5a 100644 --- a/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/mock/main_activity_kt_after_migration.txt +++ b/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/mock/main_activity_kt_after_migration.txt @@ -6,6 +6,8 @@ import android.view.View import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.content.res.AppCompatResources import androidx.core.content.ContextCompat +import com.example.app.R +import com.example.app.ResourceManager class MainActivity : AppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) { @@ -55,5 +57,8 @@ class MainActivity : AppCompatActivity() { // We have same id (integer_res) in both app and module package (Skipped). val integer = resources.getInteger(R.integer.integer_res) val quantity = ResourceManager.Plurals.pluralsRes(0) + val name1 = if(true) ResourceManager.Strings.stringParameterizedRes("Kumar") else ResourceManager.Strings.stringRes() + val name2 = if(true) ResourceManager.Strings.stringParameterizedRes() else ResourceManager.Strings.stringRes() + val name3 = if(true) ResourceManager.Strings.stringParameterizedResMod_module() else ResourceManager.Strings.stringRes() } } diff --git a/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/mock/main_activity_kt_before_migration.txt b/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/mock/main_activity_kt_before_migration.txt index abed372..f6c62d4 100644 --- a/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/mock/main_activity_kt_before_migration.txt +++ b/resourcemanager/src/test/kotlin/dev/randos/resourcemanager/mock/main_activity_kt_before_migration.txt @@ -6,6 +6,7 @@ import android.view.View import androidx.appcompat.app.AppCompatActivity import androidx.appcompat.content.res.AppCompatResources import androidx.core.content.ContextCompat +import com.example.app.R class MainActivity : AppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) { @@ -55,5 +56,8 @@ class MainActivity : AppCompatActivity() { // We have same id (integer_res) in both app and module package (Skipped). val integer = resources.getInteger(R.integer.integer_res) val quantity = resources.getQuantityString(R.plurals.plurals_res, 0) + val name1 = if(true) jkl().getString(R.string.string_parameterized_res, "Kumar") else baseContext.resources.getString(R.string.string_res) + val name2 = if(true) getString(R.string.string_parameterized_res) else baseContext.resources.getString(R.string.string_res) + val name3 = if(true) jkl().aa.getString(com.example.mylibrary1.R.string.string_parameterized_res_mod) else baseContext.resources.getString(R.string.string_res) } }