No Issue: Add custom detekt rule to blacklist certain properties
Add a custom detekt rule to blacklist certain properties. This is immediately useful for making sure that developers do not configure runtime behavior using the `BuildConfig.DEBUG` property but it is useful in a wider context.master
parent
509fa112d0
commit
f69009aa9e
|
@ -70,6 +70,8 @@ configurations {
|
||||||
|
|
||||||
dependencies {
|
dependencies {
|
||||||
ktlint "com.pinterest:ktlint:0.34.2"
|
ktlint "com.pinterest:ktlint:0.34.2"
|
||||||
|
detekt project(":mozilla-detekt-rules")
|
||||||
|
detekt "io.gitlab.arturbosch.detekt:detekt-cli:${Versions.detekt}"
|
||||||
}
|
}
|
||||||
|
|
||||||
task ktlint(type: JavaExec, group: "verification") {
|
task ktlint(type: JavaExec, group: "verification") {
|
||||||
|
|
|
@ -11,6 +11,7 @@ object Versions {
|
||||||
const val leanplum = "5.2.3"
|
const val leanplum = "5.2.3"
|
||||||
const val osslicenses_plugin = "0.9.5"
|
const val osslicenses_plugin = "0.9.5"
|
||||||
const val osslicenses_library = "17.0.0"
|
const val osslicenses_library = "17.0.0"
|
||||||
|
const val detekt = "1.0.0-RC16"
|
||||||
|
|
||||||
const val androidx_appcompat = "1.1.0"
|
const val androidx_appcompat = "1.1.0"
|
||||||
const val androidx_biometric = "1.0.1"
|
const val androidx_biometric = "1.0.1"
|
||||||
|
@ -45,7 +46,8 @@ object Versions {
|
||||||
const val adjust = "4.18.3"
|
const val adjust = "4.18.3"
|
||||||
const val installreferrer = "1.0"
|
const val installreferrer = "1.0"
|
||||||
|
|
||||||
const val junit = "4.12"
|
const val junit = "5.5.2"
|
||||||
|
const val assertJ = "3.13.2"
|
||||||
const val mockito = "2.24.5"
|
const val mockito = "2.24.5"
|
||||||
const val mockk = "1.9.kotlin12"
|
const val mockk = "1.9.kotlin12"
|
||||||
const val assertk = "0.19"
|
const val assertk = "0.19"
|
||||||
|
@ -204,4 +206,11 @@ object Deps {
|
||||||
const val google_ads_id = "com.google.android.gms:play-services-ads-identifier:${Versions.google_ads_id_version}"
|
const val google_ads_id = "com.google.android.gms:play-services-ads-identifier:${Versions.google_ads_id_version}"
|
||||||
|
|
||||||
const val lottie = "com.airbnb.android:lottie:${Versions.airbnb_lottie}"
|
const val lottie = "com.airbnb.android:lottie:${Versions.airbnb_lottie}"
|
||||||
|
|
||||||
|
const val detektApi = "io.gitlab.arturbosch.detekt:detekt-api:${Versions.detekt}"
|
||||||
|
const val detektTest = "io.gitlab.arturbosch.detekt:detekt-test:${Versions.detekt}"
|
||||||
|
const val junitApi = "org.junit.jupiter:junit-jupiter-api:${Versions.junit}"
|
||||||
|
const val junitParams = "org.junit.jupiter:junit-jupiter-params:${Versions.junit}"
|
||||||
|
const val assertJ = "org.assertj:assertj-core:${Versions.assertJ}"
|
||||||
|
const val junitEngine = "org.junit.jupiter:junit-jupiter-engine:${Versions.junit}"
|
||||||
}
|
}
|
||||||
|
|
|
@ -106,6 +106,15 @@ complexity:
|
||||||
thresholdInObjects: 11
|
thresholdInObjects: 11
|
||||||
thresholdInEnums: 11
|
thresholdInEnums: 11
|
||||||
|
|
||||||
|
mozilla-detekt-rules:
|
||||||
|
active: true
|
||||||
|
MozillaBannedPropertyAccess:
|
||||||
|
active: true
|
||||||
|
# BuildConfig.Debug: This property tests whether the application was built
|
||||||
|
# with the debuggable flag or not. Use a check for different build variants,
|
||||||
|
# instead.
|
||||||
|
bannedProperties: "BuildConfig.DEBUG"
|
||||||
|
|
||||||
empty-blocks:
|
empty-blocks:
|
||||||
active: true
|
active: true
|
||||||
EmptyCatchBlock:
|
EmptyCatchBlock:
|
||||||
|
|
|
@ -0,0 +1 @@
|
||||||
|
/build
|
|
@ -0,0 +1,14 @@
|
||||||
|
apply plugin: 'kotlin'
|
||||||
|
|
||||||
|
dependencies {
|
||||||
|
compileOnly Deps.detektApi
|
||||||
|
|
||||||
|
testImplementation Deps.detektApi
|
||||||
|
testImplementation Deps.detektTest
|
||||||
|
implementation Deps.kotlin_stdlib
|
||||||
|
testImplementation Deps.junitApi
|
||||||
|
testImplementation Deps.junitParams
|
||||||
|
testImplementation Deps.assertJ
|
||||||
|
testRuntimeOnly Deps.junitEngine
|
||||||
|
}
|
||||||
|
|
|
@ -0,0 +1,12 @@
|
||||||
|
package org.mozilla.fenix.detektrules
|
||||||
|
|
||||||
|
import io.gitlab.arturbosch.detekt.api.ConsoleReport
|
||||||
|
import io.gitlab.arturbosch.detekt.api.Detektion
|
||||||
|
|
||||||
|
class CustomRulesetConsoleReport: ConsoleReport() {
|
||||||
|
override fun render(detektion: Detektion): String? {
|
||||||
|
return detektion.findings["mozilla-detekt-rules"]?.fold("") { output, finding ->
|
||||||
|
output + finding.locationAsString + ": " + finding.messageOrDescription()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
|
@ -0,0 +1,22 @@
|
||||||
|
/* This Source Code Form is subject to the terms of the Mozilla Public
|
||||||
|
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||||
|
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
|
||||||
|
|
||||||
|
package org.mozilla.fenix.detektrules
|
||||||
|
|
||||||
|
import io.gitlab.arturbosch.detekt.api.Config
|
||||||
|
import io.gitlab.arturbosch.detekt.api.RuleSet
|
||||||
|
import io.gitlab.arturbosch.detekt.api.RuleSetProvider
|
||||||
|
|
||||||
|
class CustomRulesetProvider: RuleSetProvider {
|
||||||
|
override val ruleSetId: String = "mozilla-detekt-rules"
|
||||||
|
|
||||||
|
override fun instance(config: Config): RuleSet = RuleSet(
|
||||||
|
ruleSetId,
|
||||||
|
listOf(
|
||||||
|
MozillaBannedPropertyAccess(config)
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
}
|
|
@ -0,0 +1,53 @@
|
||||||
|
/* This Source Code Form is subject to the terms of the Mozilla Public
|
||||||
|
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||||
|
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
|
||||||
|
|
||||||
|
package org.mozilla.fenix.detektrules
|
||||||
|
|
||||||
|
import io.gitlab.arturbosch.detekt.api.CodeSmell
|
||||||
|
import io.gitlab.arturbosch.detekt.api.Config
|
||||||
|
import io.gitlab.arturbosch.detekt.api.Debt
|
||||||
|
import io.gitlab.arturbosch.detekt.api.Entity
|
||||||
|
import io.gitlab.arturbosch.detekt.api.Issue
|
||||||
|
import io.gitlab.arturbosch.detekt.api.Rule
|
||||||
|
import io.gitlab.arturbosch.detekt.api.Severity
|
||||||
|
import org.jetbrains.kotlin.psi.*
|
||||||
|
|
||||||
|
class MozillaBannedPropertyAccess(config: Config = Config.empty): Rule(config) {
|
||||||
|
override val issue = Issue(
|
||||||
|
"MozillaBannedPropertyAccess",
|
||||||
|
Severity.Defect,
|
||||||
|
DESCR,
|
||||||
|
Debt.FIVE_MINS
|
||||||
|
)
|
||||||
|
|
||||||
|
private val banned by lazy {
|
||||||
|
val bannedPropertiesList = valueOrDefault("bannedProperties", "")
|
||||||
|
if (bannedPropertiesList.contentEquals("")) {
|
||||||
|
listOf<String>()
|
||||||
|
} else {
|
||||||
|
bannedPropertiesList.split(",")
|
||||||
|
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
override fun visitDotQualifiedExpression(expression: KtDotQualifiedExpression) {
|
||||||
|
super.visitDotQualifiedExpression(expression)
|
||||||
|
val possiblyBannedPropertyAccess = expression.node.chars
|
||||||
|
|
||||||
|
// Does the current property access, a.b.c.d.possibly.Banned, end with
|
||||||
|
// it. Use endsWith() so that package qualification does not interfere
|
||||||
|
// with the check. In other words, if the configuration tells this rule
|
||||||
|
// to ban d.definitely.Banned, flag a use of the property via
|
||||||
|
// x.y.z.d.definitely.Banned.
|
||||||
|
banned.filter { possiblyBannedPropertyAccess.endsWith(it) }.map {
|
||||||
|
CodeSmell(
|
||||||
|
issue,
|
||||||
|
Entity.from(expression),
|
||||||
|
"Using ${possiblyBannedPropertyAccess} is not allowed because accessing property ${it} is against Mozilla policy. See 'mozilla-detekt-rules' stanza in 'config/detekt.yml' for more information.\n"
|
||||||
|
)
|
||||||
|
}.forEach { report(it) }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
internal const val DESCR = "Using banned property"
|
|
@ -0,0 +1 @@
|
||||||
|
org.mozilla.fenix.detektrules.CustomRulesetConsoleReport
|
|
@ -0,0 +1 @@
|
||||||
|
org.mozilla.fenix.detektrules.CustomRulesetProvider
|
|
@ -0,0 +1,3 @@
|
||||||
|
MozillaBannedPropertyAccess:
|
||||||
|
active: true
|
||||||
|
bannedProperties: "Sample.Banned"
|
|
@ -0,0 +1,81 @@
|
||||||
|
/* This Source Code Form is subject to the terms of the Mozilla Public
|
||||||
|
* License, v. 2.0. If a copy of the MPL was not distributed with this
|
||||||
|
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
|
||||||
|
|
||||||
|
package org.mozilla.fenix.detektrules
|
||||||
|
|
||||||
|
import io.gitlab.arturbosch.detekt.test.lint
|
||||||
|
import io.gitlab.arturbosch.detekt.api.YamlConfig
|
||||||
|
import org.assertj.core.api.Assertions.assertThat
|
||||||
|
import org.junit.jupiter.api.DisplayName
|
||||||
|
import org.junit.jupiter.api.Test
|
||||||
|
import org.junit.jupiter.params.ParameterizedTest
|
||||||
|
import org.junit.jupiter.params.provider.Arguments
|
||||||
|
import org.junit.jupiter.params.provider.Arguments.arguments
|
||||||
|
import org.junit.jupiter.params.provider.MethodSource
|
||||||
|
import java.util.stream.Stream
|
||||||
|
|
||||||
|
internal class MozillaBannedPropertyAccessTest {
|
||||||
|
@Test
|
||||||
|
internal fun `non compliant property access should warn`() {
|
||||||
|
val findings =
|
||||||
|
MozillaBannedPropertyAccess(YamlConfig.loadResource(this.javaClass.getResource("/config.yml"))).lint(
|
||||||
|
NONCOMPLIANT_ACCESS.trimIndent()
|
||||||
|
)
|
||||||
|
assertThat(findings).hasSize(1)
|
||||||
|
assertThat(findings[0].issue.description).isEqualTo(DESCR)
|
||||||
|
}
|
||||||
|
|
||||||
|
@DisplayName("compliant ")
|
||||||
|
@MethodSource("compliantProvider")
|
||||||
|
@ParameterizedTest(name = "{1} should not warn")
|
||||||
|
internal fun testCompliantWhen(source: String) {
|
||||||
|
val findings =
|
||||||
|
MozillaBannedPropertyAccess(YamlConfig.loadResource(this.javaClass.getResource("/config.yml"))).lint(
|
||||||
|
source
|
||||||
|
)
|
||||||
|
assertThat(findings).isEmpty()
|
||||||
|
}
|
||||||
|
|
||||||
|
companion object {
|
||||||
|
@JvmStatic
|
||||||
|
fun compliantProvider(): Stream<Arguments> =
|
||||||
|
Stream.of(
|
||||||
|
arguments(COMPLIANT_ACCESS, "Safe property access")
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const val NONCOMPLIANT_ACCESS = """
|
||||||
|
class Test {
|
||||||
|
lateinit var x: String
|
||||||
|
class Sample {
|
||||||
|
companion object {
|
||||||
|
public var Banned = "true".toBoolean()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
init {
|
||||||
|
if (Sample.Banned) {
|
||||||
|
x = "true"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
"""
|
||||||
|
|
||||||
|
const val COMPLIANT_ACCESS = """
|
||||||
|
class Test {
|
||||||
|
lateinit var x: String
|
||||||
|
class Sample {
|
||||||
|
companion object {
|
||||||
|
public var Banned = "true".toBoolean()
|
||||||
|
public var Allowed = "false".toBoolean()
|
||||||
|
|
||||||
|
}
|
||||||
|
}
|
||||||
|
init {
|
||||||
|
if (Sample.Allowed) {
|
||||||
|
x = "true"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
"""
|
|
@ -1,5 +1,6 @@
|
||||||
include ':app'
|
include ':app'
|
||||||
|
|
||||||
|
include ':mozilla-detekt-rules'
|
||||||
def log(message) {
|
def log(message) {
|
||||||
logger.lifecycle("[settings] ${message}")
|
logger.lifecycle("[settings] ${message}")
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue