Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detekt #6038

Merged
merged 22 commits into from
May 13, 2022
Merged

Detekt #6038

Show file tree
Hide file tree
Changes from 19 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,4 @@ jobs:
path: |
vector/build/outputs/apk/*/release/*.apk

# TODO: add exodus checks
# TODO add exodus checks
20 changes: 20 additions & 0 deletions .github/workflows/quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
16 changes: 14 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -140,6 +143,15 @@ allprojects {
"experimental:kdoc-wrapping",
]
}

detekt {
// preconfigure defaults
buildUponDefaultConfig = true
// activate all available (even unstable) rules.
allRules = false
Copy link
Contributor

@ouchadam ouchadam May 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can generate a baseline if we want to enable more rules without having to fix them straight away https://detekt.dev/docs/introduction/baseline/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, thanks, I have seen that. Good to prevent new issues. I would prefer to enable the disabled rules one by one and fix the existing issues though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have set this value to true and the output is the same.

// point to your custom config defining rules to run, overwriting default behavior
config = files("$rootDir/tools/detekt/detekt.yml")
}
}

task clean(type: Delete) {
Expand Down
2 changes: 2 additions & 0 deletions dependencies_groups.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -195,6 +196,7 @@ ext.groups = [
'org.testng',
'org.threeten',
'org.webjars',
'org.yaml',
'ru.noties',
'xerces',
'xml-apis',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>() }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ internal class RoomDecryptorProvider @Inject constructor(
newSessionListeners.toList().forEach {
try {
it.onNewSession(roomId, senderKey, sessionId)
} catch (e: Throwable) {
} catch (ignore: Throwable) {
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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())
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
)
Original file line number Diff line number Diff line change
Expand Up @@ -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;
)
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?,
Expand All @@ -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,
Expand Down
Loading