Merge pull request #494 from vector-im/feature/bma/checkCode

Check code

.github/workflows/quality.yml

@@ -12,6 +12,14 @@ env:
   CI_GRADLE_ARG_PROPERTIES: --stacktrace -PpreDexEnable=false --max-workers 2 --no-daemon --warn
 
 jobs:
+  checkScript:
+    name: Search for forbidden patterns
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Run code quality check suite
+        run: ./tools/check/check_code_quality.sh
+
   check:
     name: Project Check Suite
     runs-on: ubuntu-latest

libraries/matrix/impl/build.gradle.kts

@@ -32,6 +32,7 @@ dependencies {
     // api(projects.libraries.rustsdk)
     implementation(libs.matrix.sdk)
     implementation(projects.libraries.di)
+    implementation(projects.services.toolbox.api)
     api(projects.libraries.matrix.api)
     implementation(libs.dagger)
     implementation(projects.libraries.core)

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt

@@ -44,6 +44,7 @@ import io.element.android.libraries.matrix.impl.usersearch.UserProfileMapper
 import io.element.android.libraries.matrix.impl.usersearch.UserSearchResultMapper
 import io.element.android.libraries.matrix.impl.verification.RustSessionVerificationService
 import io.element.android.libraries.sessionstorage.api.SessionStore
+import io.element.android.services.toolbox.api.systemclock.SystemClock
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -77,6 +78,7 @@ class RustMatrixClient constructor(
     private val coroutineScope: CoroutineScope,
     private val dispatchers: CoroutineDispatchers,
     private val baseDirectory: File,
+    private val clock: SystemClock,
 ) : MatrixClient {
 
     override val sessionId: UserId = UserId(client.userId())
@@ -215,6 +217,7 @@ class RustMatrixClient constructor(
             innerRoom = fullRoom,
             coroutineScope = coroutineScope,
             coroutineDispatchers = dispatchers,
+            clock = clock,
         )
     }
 

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt

@@ -29,6 +29,7 @@ import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.matrix.impl.RustMatrixClient
 import io.element.android.libraries.sessionstorage.api.SessionData
 import io.element.android.libraries.sessionstorage.api.SessionStore
+import io.element.android.services.toolbox.api.systemclock.SystemClock
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -51,6 +52,7 @@ class RustMatrixAuthenticationService @Inject constructor(
     private val coroutineScope: CoroutineScope,
     private val coroutineDispatchers: CoroutineDispatchers,
     private val sessionStore: SessionStore,
+    private val clock: SystemClock,
 ) : MatrixAuthenticationService {
 
     private val authService: RustAuthenticationService = RustAuthenticationService(
@@ -177,6 +179,7 @@ class RustMatrixAuthenticationService @Inject constructor(
             coroutineScope = coroutineScope,
             dispatchers = coroutineDispatchers,
             baseDirectory = baseDirectory,
+            clock = clock,
         )
     }
 }

libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt

@@ -31,6 +31,7 @@ import io.element.android.libraries.matrix.api.room.roomMembers
 import io.element.android.libraries.matrix.api.timeline.MatrixTimeline
 import io.element.android.libraries.matrix.impl.media.map
 import io.element.android.libraries.matrix.impl.timeline.RustMatrixTimeline
+import io.element.android.services.toolbox.api.systemclock.SystemClock
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
@@ -54,6 +55,7 @@ class RustMatrixRoom(
     private val innerRoom: Room,
     private val coroutineScope: CoroutineScope,
     private val coroutineDispatchers: CoroutineDispatchers,
+    private val clock: SystemClock,
 ) : MatrixRoom {
 
     override val membersStateFlow: StateFlow<MatrixRoomMembersState>
@@ -77,9 +79,9 @@ class RustMatrixRoom(
                 it.rooms.contains(roomId.value)
             }
             .map {
-                System.currentTimeMillis()
+                clock.epochMillis()
             }
-            .onStart { emit(System.currentTimeMillis()) }
+            .onStart { emit(clock.epochMillis()) }
     }
 
     override fun timeline(): MatrixTimeline {

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/PushersManager.kt

@@ -115,8 +115,7 @@ class PushersManager @Inject constructor(
             appDisplayName = appName,
             deviceDisplayName = currentSession.sessionParams.deviceId ?: "MOBILE"
         )
-
-         */
+        */
     }
 
     fun getPusherForCurrentSession() {}/*: Pusher? {

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt

@@ -71,32 +71,32 @@ class NotifiableEventResolver @Inject constructor(
 
         return notificationData.asNotifiableEvent(sessionId)
     }
-}
 
-private fun NotificationData.asNotifiableEvent(userId: SessionId): NotifiableEvent {
-    return NotifiableMessageEvent(
-        sessionId = userId,
-        roomId = roomId,
-        eventId = eventId,
-        editedEventId = null,
-        canBeReplaced = true,
-        noisy = isNoisy,
-        timestamp = System.currentTimeMillis(),
-        senderName = senderDisplayName,
-        senderId = senderId.value,
-        body = "Message ${eventId.value.take(8)}… in room ${roomId.value.take(8)}…",
-        imageUriString = null,
-        threadId = null,
-        roomName = null,
-        roomIsDirect = false,
-        roomAvatarPath = roomAvatarUrl,
-        senderAvatarPath = senderAvatarUrl,
-        soundName = null,
-        outGoingMessage = false,
-        outGoingMessageFailed = false,
-        isRedacted = false,
-        isUpdated = false
-    )
+    private fun NotificationData.asNotifiableEvent(userId: SessionId): NotifiableEvent {
+        return NotifiableMessageEvent(
+            sessionId = userId,
+            roomId = roomId,
+            eventId = eventId,
+            editedEventId = null,
+            canBeReplaced = true,
+            noisy = isNoisy,
+            timestamp = clock.epochMillis(),
+            senderName = senderDisplayName,
+            senderId = senderId.value,
+            body = "Message ${eventId.value.take(8)}… in room ${roomId.value.take(8)}…",
+            imageUriString = null,
+            threadId = null,
+            roomName = null,
+            roomIsDirect = false,
+            roomAvatarPath = roomAvatarUrl,
+            senderAvatarPath = senderAvatarUrl,
+            soundName = null,
+            outGoingMessage = false,
+            outGoingMessageFailed = false,
+            isRedacted = false,
+            isUpdated = false
+        )
+    }
 }
 
 /**

libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/di/SessionStorageModule.kt

@@ -34,10 +34,10 @@ object SessionStorageModule {
     @SingleIn(AppScope::class)
     fun provideMatrixDatabase(@ApplicationContext context: Context): SessionDatabase {
         val name = "session_database"
-        val secretFile = context.getDatabasePath("$name.key")
+        val secretFile = context.getDatabasePath("${name}.key")
         val passphraseProvider = RandomSecretPassphraseProvider(context, secretFile)
         val driver = SqlCipherDriverFactory(passphraseProvider)
-            .create(SessionDatabase.Schema, "$name.db", context)
+            .create(SessionDatabase.Schema, "${name}.db", context)
         return SessionDatabase(driver)
     }
 }

samples/minimal/src/main/kotlin/io/element/android/samples/minimal/MainActivity.kt

@@ -30,6 +30,7 @@ import io.element.android.libraries.designsystem.theme.ElementTheme
 import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService
 import io.element.android.libraries.matrix.impl.auth.RustMatrixAuthenticationService
 import io.element.android.libraries.sessionstorage.impl.memory.InMemorySessionStore
+import io.element.android.services.toolbox.impl.systemclock.DefaultSystemClock
 import kotlinx.coroutines.runBlocking
 import java.io.File
 
@@ -43,6 +44,7 @@ class MainActivity : ComponentActivity() {
             coroutineScope = Singleton.appScope,
             coroutineDispatchers = Singleton.coroutineDispatchers,
             sessionStore = InMemorySessionStore(),
+            clock = DefaultSystemClock()
         )
     }
 

tools/check/check_code_quality.sh

@@ -0,0 +1,55 @@
+#!/usr/bin/env bash
+
+#
+# Copyright 2023 New Vector Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+#######################################################################################################################
+# Search forbidden pattern
+#######################################################################################################################
+
+searchForbiddenStringsScript=./tmp/search_forbidden_strings.pl
+
+if [[ -f ${searchForbiddenStringsScript} ]]; then
+  echo "${searchForbiddenStringsScript} already there"
+else
+  mkdir tmp
+  echo "Get the script"
+  wget https://raw.githubusercontent.com/matrix-org/matrix-dev-tools/develop/bin/search_forbidden_strings.pl -O ${searchForbiddenStringsScript}
+fi
+
+if [[ -x ${searchForbiddenStringsScript} ]]; then
+  echo "${searchForbiddenStringsScript} is already executable"
+else
+  echo "Make the script executable"
+  chmod u+x ${searchForbiddenStringsScript}
+fi
+
+echo
+echo "Search for forbidden patterns in code..."
+
+# list all Kotlin folders of the project.
+allKotlinDirs=`find . -type d |grep -v build |grep -v \.git |grep -v \.gradle |grep kotlin$`
+
+${searchForbiddenStringsScript} ./tools/check/forbidden_strings_in_code.txt $allKotlinDirs
+
+resultForbiddenStringInCode=$?
+
+if [[ ${resultForbiddenStringInCode} -eq 0 ]]; then
+   echo "MAIN OK"
+else
+   echo "❌ MAIN ERROR"
+   exit 1
+fi

tools/check/forbidden_strings_in_code.txt

@@ -0,0 +1,131 @@
+#
+# Copyright 2023 New Vector Ltd
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# This file list String which are not allowed in source code.
+# Use Perl regex to write forbidden strings
+# Note: line cannot start with a space. Use \s instead.
+# It is possible to specify an authorized number of occurrence with === suffix. Default is 0
+# Example:
+# AuthorizedStringThreeTimes===3
+
+# Extension:kt
+
+### No import static: use full class name
+import static
+
+### Rubbish from merge. Please delete those lines (sometimes in comment)
+<<<<<<<
+>>>>>>>
+
+### carry return before "}". Please remove empty lines.
+\n\s*\n\s*\}
+
+### typo detected.
+formated
+abtract
+Succes[^s]
+succes[^s]
+
+### Use int instead of Integer
+protected Integer
+
+### Use the interface declaration. Example: use type "Map" instead of type "HashMap" to declare variable or parameter. For Kotlin, use mapOf, setOf, ...
+(private|public|protected|    ) (static )?(final )?(HashMap|HashSet|ArrayList)<
+
+### Use int instead of short
+Short\.parseShort
+\(short\)
+private short
+final short
+
+### Line length is limited to 160 chars. Please split long lines
+#[^─]{161}
+
+### "DO NOT COMMIT" has been committed
+DO NOT COMMIT
+
+### invalid formatting
+\s{8}/\*\n     \*
+# Now checked by ktlint
+# [^\w]if\(
+# while\(
+# for\(
+
+# Add space after //
+# DISABLED To re-enable when code will be formatted globally
+#^\s*//[^\s]
+
+### invalid formatting (too many space char)
+^      /\*
+
+### unnecessary parenthesis around numbers, example: " (0)"
+ \(\d+\)
+
+### import the package, do not use long class name with package
+android\.os\.Build\.
+
+### Tab char is forbidden. Use only spaces
+\t
+
+# Empty lines and trailing space
+# DISABLED To re-enable when code will be formatted globally
+#[ ]$
+
+### Deprecated, use retrofit2.HttpException
+import retrofit2\.adapter\.rxjava\.HttpException
+
+### This is generally not necessary, no need to reset the padding if there is no drawable
+setCompoundDrawablePadding\(0\)
+
+# Change thread with Rx
+# DISABLED
+#runOnUiThread
+
+### Bad formatting of chain (missing new line)
+\w\.flatMap\(
+
+### In Kotlin, Void has to be null safe, i.e. use 'Void?' instead of 'Void'
+\: Void\)
+
+### Kotlin conversion tools introduce this, but is can be replace by trim()
+trim \{ it \<\= \' \' \}
+
+### Put the operator at the beginning of next line
+ ==$
+
+### Use JsonUtils.getBasicGson()
+new Gson\(\)
+
+### Use TextUtils.formatFileSize
+Formatter\.formatFileSize===1
+
+### Use TextUtils.formatFileSize with short format param to true
+Formatter\.formatShortFileSize===1
+
+### Use `Context#getSystemService` extension function provided by `core-ktx`
+getSystemService\(Context
+
+### Use DefaultSharedPreferences.getInstance() instead for better performance
+PreferenceManager\.getDefaultSharedPreferences==2
+
+### Use the Clock interface, or use `measureTimeMillis`
+System\.currentTimeMillis\(\)===1
+
+### Remove extra space between the name and the description
+\* @\w+ \w+  +
+
+### 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_]