Merge pull request #6843 from vector-im/feature/bma/detect_object_log

Fix some string template

build.gradle

@@ -151,6 +151,8 @@ allprojects {
                 "experimental:comment-wrapping",
                 // - A KDoc comment after any other element on the same line must be separated by a new line
                 "experimental:kdoc-wrapping",
+                // Ignore error "Redundant curly braces", since we use it to fix false positives, for instance in "elementLogs.${i}.txt"
+                "string-template",
         ]
     }
 

changelog.d/6843.misc

@@ -0,0 +1 @@
+Fix some string template

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/verification/VerificationTransportToDevice.kt

@@ -76,12 +76,12 @@ internal class VerificationTransportToDevice(
                 .configureWith(SendToDeviceTask.Params(MessageType.MSGTYPE_VERIFICATION_REQUEST, contentMap)) {
                     this.callback = object : MatrixCallback<Unit> {
                         override fun onSuccess(data: Unit) {
-                            Timber.v("## verification [$tx.transactionId] send toDevice request success")
+                            Timber.v("## verification [${tx?.transactionId}] send toDevice request success")
                             callback.invoke(localId, validKeyReq)
                         }
 
                         override fun onFailure(failure: Throwable) {
-                            Timber.e("## verification [$tx.transactionId] failed to send toDevice request")
+                            Timber.e("## verification [${tx?.transactionId}] failed to send toDevice request")
                         }
                     }
                 }
@@ -103,12 +103,12 @@ internal class VerificationTransportToDevice(
                 .configureWith(SendToDeviceTask.Params(EventType.KEY_VERIFICATION_READY, contentMap)) {
                     this.callback = object : MatrixCallback<Unit> {
                         override fun onSuccess(data: Unit) {
-                            Timber.v("## verification [$tx.transactionId] send toDevice request success")
+                            Timber.v("## verification [${tx?.transactionId}] send toDevice request success")
                             callback?.invoke()
                         }
 
                         override fun onFailure(failure: Throwable) {
-                            Timber.e("## verification [$tx.transactionId] failed to send toDevice request")
+                            Timber.e("## verification [${tx?.transactionId}] failed to send toDevice request")
                         }
                     }
                 }
@@ -136,7 +136,7 @@ internal class VerificationTransportToDevice(
                 .configureWith(SendToDeviceTask.Params(type, contentMap)) {
                     this.callback = object : MatrixCallback<Unit> {
                         override fun onSuccess(data: Unit) {
-                            Timber.v("## SAS verification [$tx.transactionId] toDevice type '$type' success.")
+                            Timber.v("## SAS verification [${tx.transactionId}] toDevice type '$type' success.")
                             if (onDone != null) {
                                 onDone()
                             } else {
@@ -149,7 +149,7 @@ internal class VerificationTransportToDevice(
                         }
 
                         override fun onFailure(failure: Throwable) {
-                            Timber.e("## SAS verification [$tx.transactionId] failed to send toDevice in state : $tx.state")
+                            Timber.e("## SAS verification [${tx.transactionId}] failed to send toDevice in state : ${tx.state}")
                             tx.cancel(onErrorReason)
                         }
                     }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/database/SessionRealmConfigurationFactory.kt

@@ -93,7 +93,7 @@ internal class SessionRealmConfigurationFactory @Inject constructor(
             return
         }
 
-        listOf(REALM_NAME, "$REALM_NAME.lock", "$REALM_NAME.note", "$REALM_NAME.management").forEach { file ->
+        listOf(REALM_NAME, "${REALM_NAME}.lock", "${REALM_NAME}.note", "${REALM_NAME}.management").forEach { file ->
             try {
                 File(directory, file).deleteRecursively()
             } catch (e: Exception) {

tools/check/forbidden_strings_in_code.txt

@@ -185,3 +185,6 @@ System\.currentTimeMillis\(\)===2
 onCreateOptionsMenu
 onOptionsItemSelected
 onPrepareOptionsMenu
+
+### Suspicious String template. Please check that the string template will behave as expected, i.e. the class field and not the whole object will be used. For instance `Timber.d("$event.type")` is not correct, you should write `Timber.d("${event.type}")`. In the former the whole event content will be logged, since it's a data class. If this is expected (i.e. to fix false positive), please add explicit curly braces (`{` and `}`) around the variable, for instance `"elementLogs.${i}.txt"`
+\$[a-zA-Z_]\w*\??\.[a-zA-Z_]

vector/src/androidTest/java/im/vector/app/espresso/tools/ScreenshotFailureRule.kt

@@ -83,7 +83,7 @@ private fun useMediaStoreScreenshotStorage(
         screenshotLocation: String,
         bitmap: Bitmap
 ) {
-    contentValues.put(MediaStore.MediaColumns.DISPLAY_NAME, "$screenshotName.jpeg")
+    contentValues.put(MediaStore.MediaColumns.DISPLAY_NAME, "${screenshotName}.jpeg")
     contentValues.put(MediaStore.Images.Media.RELATIVE_PATH, screenshotLocation)
     val uri: Uri? = contentResolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues)
     if (uri != null) {
@@ -104,7 +104,7 @@ private fun usePublicExternalScreenshotStorage(
     if (!directory.exists()) {
         directory.mkdirs()
     }
-    val file = File(directory, "$screenshotName.jpeg")
+    val file = File(directory, "${screenshotName}.jpeg")
     saveScreenshotToStream(bitmap, FileOutputStream(file))
     contentResolver.insert(MediaStore.Images.Media.EXTERNAL_CONTENT_URI, contentValues)
 }

vector/src/main/java/im/vector/app/core/extensions/Fragment.kt

@@ -175,7 +175,7 @@ fun Fragment.queryExportKeys(userId: String, activityResultLauncher: ActivityRes
     selectTxtFileToWrite(
             activity = requireActivity(),
             activityResultLauncher = activityResultLauncher,
-            defaultFileName = "$appName-megolm-export-$userId-$timestamp.txt",
+            defaultFileName = "$appName-megolm-export-$userId-${timestamp}.txt",
             chooserHint = getString(R.string.keys_backup_setup_step1_manual_export)
     )
 }
@@ -187,7 +187,7 @@ fun Activity.queryExportKeys(userId: String, activityResultLauncher: ActivityRes
     selectTxtFileToWrite(
             activity = this,
             activityResultLauncher = activityResultLauncher,
-            defaultFileName = "$appName-megolm-export-$userId-$timestamp.txt",
+            defaultFileName = "$appName-megolm-export-$userId-${timestamp}.txt",
             chooserHint = getString(R.string.keys_backup_setup_step1_manual_export)
     )
 }

vector/src/main/java/im/vector/app/features/crypto/keysbackup/setup/KeysBackupSetupStep3Fragment.kt

@@ -134,7 +134,7 @@ class KeysBackupSetupStep3Fragment @Inject constructor() : VectorBaseFragment<Fr
             selectTxtFileToWrite(
                     activity = requireActivity(),
                     activityResultLauncher = saveRecoveryActivityResultLauncher,
-                    defaultFileName = "recovery-key-$userId-$timestamp.txt",
+                    defaultFileName = "recovery-key-$userId-${timestamp}.txt",
                     chooserHint = getString(R.string.save_recovery_key_chooser_hint)
             )
             dialog.dismiss()

vector/src/main/java/im/vector/app/features/rageshake/VectorFileLogger.kt

@@ -74,7 +74,7 @@ class VectorFileLogger @Inject constructor(
         }
 
         for (i in 0..15) {
-            val file = File(cacheDirectory, "elementLogs.$i.txt")
+            val file = File(cacheDirectory, "elementLogs.${i}.txt")
             tryOrNull { file.delete() }
         }
 
@@ -121,7 +121,7 @@ class VectorFileLogger @Inject constructor(
                     ?.flush()
                     ?.let { 0 until logRotationCount }
                     ?.mapNotNull { index ->
-                        File(cacheDirectory, "$fileNamePrefix.$index.txt")
+                        File(cacheDirectory, "$fileNamePrefix.${index}.txt")
                                 .takeIf { it.exists() }
                     }
         }