From 7054224451b6eab6229887af3e39b6af2efbbf5f Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 31 May 2023 15:18:42 +0200 Subject: [PATCH 1/3] Search for forbidden patterns in Kotlin files. --- .github/workflows/quality.yml | 8 ++ tools/check/check_code_quality.sh | 55 +++++++++ tools/check/forbidden_strings_in_code.txt | 131 ++++++++++++++++++++++ 3 files changed, 194 insertions(+) create mode 100755 tools/check/check_code_quality.sh create mode 100755 tools/check/forbidden_strings_in_code.txt diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 86e823f335..c945559c8d 100644 --- a/.github/workflows/quality.yml +++ b/.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 diff --git a/tools/check/check_code_quality.sh b/tools/check/check_code_quality.sh new file mode 100755 index 0000000000..9e8c964499 --- /dev/null +++ b/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 diff --git a/tools/check/forbidden_strings_in_code.txt b/tools/check/forbidden_strings_in_code.txt new file mode 100755 index 0000000000..ae346c170d --- /dev/null +++ b/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_] From 8cf61eaa5592fce699116d4ebc08ce677fc57579 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 31 May 2023 15:31:05 +0200 Subject: [PATCH 2/3] Fix detected forbidden pattern. --- libraries/matrix/impl/build.gradle.kts | 1 + .../libraries/matrix/impl/RustMatrixClient.kt | 3 ++ .../auth/RustMatrixAuthenticationService.kt | 4 +- .../matrix/impl/room/RustMatrixRoom.kt | 6 ++- .../libraries/push/impl/PushersManager.kt | 3 +- .../notifications/NotifiableEventResolver.kt | 50 +++++++++---------- .../impl/di/SessionStorageModule.kt | 4 +- 7 files changed, 39 insertions(+), 32 deletions(-) diff --git a/libraries/matrix/impl/build.gradle.kts b/libraries/matrix/impl/build.gradle.kts index 6b76a17985..1ac5b4b507 100644 --- a/libraries/matrix/impl/build.gradle.kts +++ b/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) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index bac98dd852..ce76980a6f 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/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, ) } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt index aff83c3b5a..f03ba4562c 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/auth/RustMatrixAuthenticationService.kt @@ -25,10 +25,10 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.auth.MatrixAuthenticationService import io.element.android.libraries.matrix.api.auth.MatrixHomeServerDetails import io.element.android.libraries.matrix.api.core.SessionId -import io.element.android.libraries.matrix.api.core.UserId 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 @@ -49,6 +49,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(baseDirectory.absolutePath, null, null) @@ -115,6 +116,7 @@ class RustMatrixAuthenticationService @Inject constructor( coroutineScope = coroutineScope, dispatchers = coroutineDispatchers, baseDirectory = baseDirectory, + clock = clock, ) } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt index 55576c7b96..c21ceb5076 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt +++ b/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 @@ -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 { diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/PushersManager.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/PushersManager.kt index 04d2875328..8bb1ddf20c 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/PushersManager.kt +++ b/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? { diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt index 82e3e43d50..de168090f4 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotifiableEventResolver.kt +++ b/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 + ) + } } /** diff --git a/libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/di/SessionStorageModule.kt b/libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/di/SessionStorageModule.kt index 052943388e..323aa93bb7 100644 --- a/libraries/session-storage/impl/src/main/kotlin/io/element/android/libraries/sessionstorage/impl/di/SessionStorageModule.kt +++ b/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) } } From 6681ef268015c8e0053b647ff74ba8f28d90cd41 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Wed, 31 May 2023 15:47:24 +0200 Subject: [PATCH 3/3] Fix compilation issue. --- .../kotlin/io/element/android/samples/minimal/MainActivity.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/MainActivity.kt b/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/MainActivity.kt index ef776c613c..12481b81e1 100644 --- a/samples/minimal/src/main/kotlin/io/element/android/samples/minimal/MainActivity.kt +++ b/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() ) }