Merge pull request #6038 from vector-im/feature/bma/detekt

Detekt

.github/workflows/build.yml

@@ -67,4 +67,4 @@ jobs:
           path: |
             vector/build/outputs/apk/*/release/*.apk
 
-# TODO: add exodus checks
+# TODO add exodus checks

.github/workflows/quality.yml

@@ -147,3 +147,23 @@ jobs:
           name: release-lint-report-${{ matrix.target }}
           path: |
             vector/build/reports/*.*
+
+  detekt:
+    name: Detekt Analysis
+    runs-on: ubuntu-latest
+    # Allow all jobs on main and develop. Just one per PR.
+    concurrency:
+      group: ${{ github.ref == 'refs/heads/main' && format('detekt-main-{0}', github.sha) || github.ref == 'refs/heads/develop' && format('detekt-develop-{0}', github.sha) || format('detekt-{0}', github.ref) }}
+      cancel-in-progress: true
+    steps:
+      - uses: actions/checkout@v3
+      - name: Run detekt
+        run: |
+          ./gradlew detekt
+      - name: Upload reports
+        if: always()
+        uses: actions/upload-artifact@v3
+        with:
+          name: detekt-report
+          path: |
+            */build/reports/detekt/detekt.html

build.gradle

@@ -35,9 +35,11 @@ buildscript {
     }
 }
 
-// ktlint Plugin
 plugins {
+    // ktlint Plugin
     id "org.jlleitschuh.gradle.ktlint" version "10.3.0"
+    // Detekt
+    id "io.gitlab.arturbosch.detekt" version "1.20.0"
 }
 
 // https://github.com/jeremylong/DependencyCheck
@@ -52,6 +54,7 @@ dependencyCheck {
 
 allprojects {
     apply plugin: "org.jlleitschuh.gradle.ktlint"
+    apply plugin: "io.gitlab.arturbosch.detekt"
 
     repositories {
         // Do not use `mavenCentral()`, it prevents Dependabot from working properly
@@ -119,7 +122,7 @@ allprojects {
         // display the corresponding rule
         verbose = true
         disabledRules = [
-                // TODO: Re-enable these 4 rules after reformatting project
+                // TODO Re-enable these 4 rules after reformatting project
                 "indent",
                 "experimental:argument-list-wrapping",
                 "max-line-length",
@@ -140,6 +143,15 @@ allprojects {
                 "experimental:kdoc-wrapping",
         ]
     }
+
+    detekt {
+        // preconfigure defaults
+        buildUponDefaultConfig = true
+        // activate all available (even unstable) rules.
+        allRules = true
+        // point to your custom config defining rules to run, overwriting default behavior
+        config = files("$rootDir/tools/detekt/detekt.yml")
+    }
 }
 
 task clean(type: Delete) {

changelog.d/6038.misc

@@ -0,0 +1 @@
+Setup detekt

dependencies_groups.gradle

@@ -123,6 +123,7 @@ ext.groups = [
                         'io.github.detekt.sarif4k',
                         'io.github.microutils',
                         'io.github.reactivecircus.flowbinding',
+                        'io.gitlab.arturbosch.detekt',
                         'io.grpc',
                         'io.jsonwebtoken',
                         'io.kindedj',
@@ -195,6 +196,7 @@ ext.groups = [
                         'org.testng',
                         'org.threeten',
                         'org.webjars',
+                        'org.yaml',
                         'ru.noties',
                         'xerces',
                         'xml-apis',

library/multipicker/src/main/java/im/vector/lib/multipicker/MultiPicker.kt

@@ -16,7 +16,7 @@
 
 package im.vector.lib.multipicker
 
-class MultiPicker<T> {
+class MultiPicker<T> private constructor() {
 
     companion object Type {
         val IMAGE by lazy { MultiPicker<ImagePicker>() }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/logger/LoggerTag.kt

@@ -22,15 +22,15 @@ package org.matrix.android.sdk.api.logger
  * val loggerTag = LoggerTag("MyTag", LoggerTag.VOIP)
  * Timber.tag(loggerTag.value).v("My log message")
  */
-open class LoggerTag(_value: String, parentTag: LoggerTag? = null) {
+open class LoggerTag(name: String, parentTag: LoggerTag? = null) {
 
     object SYNC : LoggerTag("SYNC")
     object VOIP : LoggerTag("VOIP")
     object CRYPTO : LoggerTag("CRYPTO")
 
     val value: String = if (parentTag == null) {
-        _value
+        name
     } else {
-        "${parentTag.value}/$_value"
+        "${parentTag.value}/$name"
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomGuestAccessContent.kt

@@ -27,13 +27,13 @@ import timber.log.Timber
 @JsonClass(generateAdapter = true)
 data class RoomGuestAccessContent(
         // Required. Whether guests can join the room. One of: ["can_join", "forbidden"]
-        @Json(name = "guest_access") val _guestAccess: String? = null
+        @Json(name = "guest_access") val guestAccessStr: String? = null
 ) {
-    val guestAccess: GuestAccess? = when (_guestAccess) {
+    val guestAccess: GuestAccess? = when (guestAccessStr) {
         "can_join"  -> GuestAccess.CanJoin
         "forbidden" -> GuestAccess.Forbidden
         else        -> {
-            Timber.w("Invalid value for GuestAccess: `$_guestAccess`")
+            Timber.w("Invalid value for GuestAccess: `$guestAccessStr`")
             null
         }
     }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomHistoryVisibilityContent.kt

@@ -22,15 +22,15 @@ import timber.log.Timber
 
 @JsonClass(generateAdapter = true)
 data class RoomHistoryVisibilityContent(
-        @Json(name = "history_visibility") val _historyVisibility: String? = null
+        @Json(name = "history_visibility") val historyVisibilityStr: String? = null
 ) {
-    val historyVisibility: RoomHistoryVisibility? = when (_historyVisibility) {
+    val historyVisibility: RoomHistoryVisibility? = when (historyVisibilityStr) {
         "world_readable" -> RoomHistoryVisibility.WORLD_READABLE
         "shared"         -> RoomHistoryVisibility.SHARED
         "invited"        -> RoomHistoryVisibility.INVITED
         "joined"         -> RoomHistoryVisibility.JOINED
         else             -> {
-            Timber.w("Invalid value for RoomHistoryVisibility: `$_historyVisibility`")
+            Timber.w("Invalid value for RoomHistoryVisibility: `$historyVisibilityStr`")
             null
         }
     }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/RoomJoinRulesContent.kt

@@ -27,22 +27,22 @@ import timber.log.Timber
  */
 @JsonClass(generateAdapter = true)
 data class RoomJoinRulesContent(
-        @Json(name = "join_rule") val _joinRules: String? = null,
+        @Json(name = "join_rule") val joinRulesStr: String? = null,
         /**
          * If the allow key is an empty list (or not a list at all),
          * then no users are allowed to join without an invite.
          * Each entry is expected to be an object with the following keys:
          */
         @Json(name = "allow") val allowList: List<RoomJoinRulesAllowEntry>? = null
 ) {
-    val joinRules: RoomJoinRules? = when (_joinRules) {
+    val joinRules: RoomJoinRules? = when (joinRulesStr) {
         "public"     -> RoomJoinRules.PUBLIC
         "invite"     -> RoomJoinRules.INVITE
         "knock"      -> RoomJoinRules.KNOCK
         "private"    -> RoomJoinRules.PRIVATE
         "restricted" -> RoomJoinRules.RESTRICTED
         else         -> {
-            Timber.w("Invalid value for RoomJoinRules: `$_joinRules`")
+            Timber.w("Invalid value for RoomJoinRules: `$joinRulesStr`")
             null
         }
     }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/room/model/create/RoomFeaturePreset.kt

@@ -47,7 +47,7 @@ class RestrictedRoomPreset(val homeServerCapabilities: HomeServerCapabilities, v
                         type = EventType.STATE_ROOM_JOIN_RULES,
                         stateKey = "",
                         content = RoomJoinRulesContent(
-                                _joinRules = RoomJoinRules.RESTRICTED.value,
+                                joinRulesStr = RoomJoinRules.RESTRICTED.value,
                                 allowList = restrictedList
                         ).toContent()
                 )

matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/sync/model/LazyRoomSyncEphemeral.kt

@@ -20,6 +20,6 @@ import com.squareup.moshi.JsonClass
 
 @JsonClass(generateAdapter = false)
 sealed class LazyRoomSyncEphemeral {
-    data class Parsed(val _roomSyncEphemeral: RoomSyncEphemeral) : LazyRoomSyncEphemeral()
+    data class Parsed(val roomSyncEphemeral: RoomSyncEphemeral) : LazyRoomSyncEphemeral()
     object Stored : LazyRoomSyncEphemeral()
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/auth/version/Versions.kt

@@ -74,7 +74,7 @@ internal fun Versions.isLoginAndRegistrationSupportedBySdk(): Boolean {
  * Indicate if the homeserver support MSC3440 for threads
  */
 internal fun Versions.doesServerSupportThreads(): Boolean {
-    // TODO: Check for v1.3 or whichever spec version formally specifies MSC3440.
+    // TODO Check for v1.3 or whichever spec version formally specifies MSC3440.
     return unstableFeatures?.get(FEATURE_THREADS_MSC3440_STABLE) ?: false
 }
 

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

@@ -46,7 +46,7 @@ internal class PerSessionBackupQueryRateLimiter @Inject constructor(
 ) {
 
     companion object {
-        val MIN_TRY_BACKUP_PERIOD_MILLIS = 60 * 60_000 // 1 hour
+        const val MIN_TRY_BACKUP_PERIOD_MILLIS = 60 * 60_000 // 1 hour
     }
 
     data class Info(

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

@@ -79,7 +79,7 @@ internal class RoomDecryptorProvider @Inject constructor(
                             newSessionListeners.toList().forEach {
                                 try {
                                     it.onNewSession(roomId, senderKey, sessionId)
-                                } catch (e: Throwable) {
+                                } catch (ignore: Throwable) {
                                 }
                             }
                         }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/megolm/MXMegolmEncryption.kt

@@ -79,7 +79,7 @@ internal class MXMegolmEncryption(
     }
 
     // Default rotation periods
-    // TODO: Make it configurable via parameters
+    // TODO Make it configurable via parameters
     // Session rotation periods
     private var sessionRotationPeriodMsgs: Int = 100
     private var sessionRotationPeriodMs: Int = 7 * 24 * 3600 * 1000

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/algorithms/olm/MXOlmEncryption.kt

@@ -38,7 +38,7 @@ internal class MXOlmEncryption(
     override suspend fun encryptEventContent(eventContent: Content, eventType: String, userIds: List<String>): Content {
         // pick the list of recipients based on the membership list.
         //
-        // TODO: there is a race condition here! What if a new user turns up
+        // TODO there is a race condition here! What if a new user turns up
         ensureSession(userIds)
         val deviceInfos = ArrayList<CryptoDeviceInfo>()
         for (userId in userIds) {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/repository/WarnOnUnknownDeviceRepository.kt

@@ -22,7 +22,7 @@ import javax.inject.Inject
 @SessionScope
 internal class WarnOnUnknownDeviceRepository @Inject constructor() {
 
-    // TODO: set it back to true by default. Need UI
+    // TODO set it back to true by default. Need UI
     // Warn the user if some new devices are detected while encrypting a message.
     private var warnOnUnknownDevices = false
 

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/store/db/migration/MigrateCryptoTo007.kt

@@ -42,7 +42,7 @@ internal class MigrateCryptoTo007(realm: DynamicRealm) : RealmMigrator(realm, 7)
                 val jsonSignatures = crossSigningKeysMapper.serializeSignatures(objectSignatures)
                 it.setString(KeyInfoEntityFields.SIGNATURES, jsonSignatures)
             }
-        } catch (failure: Throwable) {
+        } catch (ignore: Throwable) {
         }
 
         // Migrate frozen classes

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/SendEventTask.kt

@@ -69,6 +69,7 @@ internal class DefaultSendEventTask @Inject constructor(
             }
         } catch (e: Throwable) {
 //            localEchoRepository.updateSendState(params.event.eventId!!, SendState.UNDELIVERED)
+            Timber.w(e, "Unable to send the Event")
             throw e
         }
     }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/crypto/tasks/UploadSignaturesTask.kt

@@ -15,7 +15,6 @@
  */
 package org.matrix.android.sdk.internal.crypto.tasks
 
-import org.matrix.android.sdk.api.failure.Failure
 import org.matrix.android.sdk.internal.crypto.api.CryptoApi
 import org.matrix.android.sdk.internal.network.GlobalErrorReceiver
 import org.matrix.android.sdk.internal.network.executeRequest
@@ -34,20 +33,15 @@ internal class DefaultUploadSignaturesTask @Inject constructor(
 ) : UploadSignaturesTask {
 
     override suspend fun execute(params: UploadSignaturesTask.Params) {
-        try {
-            val response = executeRequest(
-                    globalErrorReceiver,
-                    canRetry = true,
-                    maxRetriesCount = 10
-            ) {
-                cryptoApi.uploadSignatures(params.signatures)
-            }
-            if (response.failures?.isNotEmpty() == true) {
-                throw Throwable(response.failures.toString())
-            }
-            return
-        } catch (f: Failure) {
-            throw f
+        val response = executeRequest(
+                globalErrorReceiver,
+                canRetry = true,
+                maxRetriesCount = 10
+        ) {
+            cryptoApi.uploadSignatures(params.signatures)
+        }
+        if (response.failures?.isNotEmpty() == true) {
+            throw Throwable(response.failures.toString())
         }
     }
 }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/network/ssl/CertUtil.kt

@@ -250,7 +250,7 @@ internal object CertUtil {
         builder.supportsTlsExtensions(hsConfig.shouldAcceptTlsExtensions)
         val list = ArrayList<ConnectionSpec>()
         list.add(builder.build())
-        // TODO: we should display a warning if user enter an http url
+        // TODO we should display a warning if user enter an http url
         if (hsConfig.allowHttpExtension || hsConfig.homeServerUriBase.toString().startsWith("http://")) {
             list.add(ConnectionSpec.CLEARTEXT)
         }

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultFileService.kt

@@ -88,7 +88,7 @@ internal class DefaultFileService @Inject constructor(
 
         Timber.v("## FileService downloadFile $url")
 
-        // TODO: Remove use of `synchronized` in suspend function.
+        // TODO Remove use of `synchronized` in suspend function.
         val existingDownload = synchronized(ongoing) {
             val existing = ongoing[url]
             if (existing != null) {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/group/model/GroupSummaryRoomsSection.kt

@@ -29,6 +29,6 @@ internal data class GroupSummaryRoomsSection(
 
         @Json(name = "rooms") val rooms: List<String> = emptyList()
 
-        // @TODO: Check the meaning and the usage of these categories. This dictionary is empty FTM.
+        // TODO Check the meaning and the usage of these categories. This dictionary is empty FTM.
         // public Map<Object, Object> categories;
 )

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/group/model/GroupSummaryUsersSection.kt

@@ -30,6 +30,6 @@ internal data class GroupSummaryUsersSection(
 
         @Json(name = "users") val users: List<String> = emptyList()
 
-        // @TODO: Check the meaning and the usage of these roles. This dictionary is empty FTM.
+        // TODO Check the meaning and the usage of these roles. This dictionary is empty FTM.
         // public Map<Object, Object> roles;
 )

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/identity/model/SignInvitationBody.kt

@@ -16,14 +16,16 @@
 
 package org.matrix.android.sdk.internal.session.identity.model
 
+import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
 
 @JsonClass(generateAdapter = true)
 internal data class SignInvitationBody(
-        /**The Matrix user ID of the user accepting the invitation.*/
+        /** The Matrix user ID of the user accepting the invitation.*/
         val mxid: String,
-        /**The token from the call to store- invite..*/
+        /** The token from the call to store- invite..*/
         val token: String,
         /** The private key, encoded as Unpadded base64. */
-        val private_key: String
+        @Json(name = "private_key")
+        val privateKey: String
 )

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/state/DefaultStateService.kt

@@ -136,7 +136,7 @@ internal class DefaultStateService @AssistedInject constructor(@Assisted private
         if (joinRules != null) {
             val body = if (joinRules == RoomJoinRules.RESTRICTED) {
                 RoomJoinRulesContent(
-                        _joinRules = RoomJoinRules.RESTRICTED.value,
+                        joinRulesStr = RoomJoinRules.RESTRICTED.value,
                         allowList = allowList
                 ).toContent()
             } else {

matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/room/state/SafePowerLevelContent.kt

@@ -23,8 +23,11 @@ import org.matrix.android.sdk.api.session.events.model.toModel
 import org.matrix.android.sdk.api.session.room.model.PowerLevelsContent
 import org.matrix.android.sdk.api.util.JsonDict
 
+/**
+ * Serializable object
+ */
 @JsonClass(generateAdapter = true)
-internal data class SerializablePowerLevelsContent(
+internal data class SafePowerLevelContent(
         @Json(name = "ban") val ban: Int?,
         @Json(name = "kick") val kick: Int?,
         @Json(name = "invite") val invite: Int?,
@@ -41,7 +44,7 @@ internal data class SerializablePowerLevelsContent(
 internal fun JsonDict.toSafePowerLevelsContentDict(): JsonDict {
     return toModel<PowerLevelsContent>()
             ?.let { content ->
-                SerializablePowerLevelsContent(
+                SafePowerLevelContent(
                         ban = content.ban,
                         kick = content.kick,
                         invite = content.invite,