diff --git a/build.gradle b/build.gradle index 663ee5f1f..6bf1fd9c4 100644 --- a/build.gradle +++ b/build.gradle @@ -70,6 +70,8 @@ configurations { dependencies { 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") { diff --git a/buildSrc/src/main/java/Dependencies.kt b/buildSrc/src/main/java/Dependencies.kt index 0f2826ac5..86b65fa09 100644 --- a/buildSrc/src/main/java/Dependencies.kt +++ b/buildSrc/src/main/java/Dependencies.kt @@ -11,6 +11,7 @@ object Versions { const val leanplum = "5.2.3" const val osslicenses_plugin = "0.9.5" const val osslicenses_library = "17.0.0" + const val detekt = "1.0.0-RC16" const val androidx_appcompat = "1.1.0" const val androidx_biometric = "1.0.1" @@ -45,7 +46,8 @@ object Versions { const val adjust = "4.18.3" 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 mockk = "1.9.kotlin12" 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 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}" } diff --git a/config/detekt.yml b/config/detekt.yml index dcd2a6a8c..7bb1fe487 100644 --- a/config/detekt.yml +++ b/config/detekt.yml @@ -106,6 +106,15 @@ complexity: thresholdInObjects: 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: active: true EmptyCatchBlock: diff --git a/mozilla-detekt-rules/.gitignore b/mozilla-detekt-rules/.gitignore new file mode 100644 index 000000000..796b96d1c --- /dev/null +++ b/mozilla-detekt-rules/.gitignore @@ -0,0 +1 @@ +/build diff --git a/mozilla-detekt-rules/build.gradle b/mozilla-detekt-rules/build.gradle new file mode 100644 index 000000000..7ddd37cce --- /dev/null +++ b/mozilla-detekt-rules/build.gradle @@ -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 +} + diff --git a/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetConsoleReport.kt b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetConsoleReport.kt new file mode 100644 index 000000000..de83c3d5c --- /dev/null +++ b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetConsoleReport.kt @@ -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() + } + } +} \ No newline at end of file diff --git a/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetProvider.kt b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetProvider.kt new file mode 100644 index 000000000..887170f7b --- /dev/null +++ b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/CustomRulesetProvider.kt @@ -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) + ) + ) + + +} diff --git a/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/MozillaBannedPropertyAccess.kt b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/MozillaBannedPropertyAccess.kt new file mode 100644 index 000000000..f9cc9ba62 --- /dev/null +++ b/mozilla-detekt-rules/src/main/java/org/mozilla/fenix/detektrules/MozillaBannedPropertyAccess.kt @@ -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() + } 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" diff --git a/mozilla-detekt-rules/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConsoleReport b/mozilla-detekt-rules/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConsoleReport new file mode 100644 index 000000000..fb899601e --- /dev/null +++ b/mozilla-detekt-rules/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.ConsoleReport @@ -0,0 +1 @@ +org.mozilla.fenix.detektrules.CustomRulesetConsoleReport diff --git a/mozilla-detekt-rules/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider b/mozilla-detekt-rules/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider new file mode 100644 index 000000000..81e411197 --- /dev/null +++ b/mozilla-detekt-rules/src/main/resources/META-INF/services/io.gitlab.arturbosch.detekt.api.RuleSetProvider @@ -0,0 +1 @@ +org.mozilla.fenix.detektrules.CustomRulesetProvider diff --git a/mozilla-detekt-rules/src/main/resources/config.yml b/mozilla-detekt-rules/src/main/resources/config.yml new file mode 100644 index 000000000..21944490b --- /dev/null +++ b/mozilla-detekt-rules/src/main/resources/config.yml @@ -0,0 +1,3 @@ + MozillaBannedPropertyAccess: + active: true + bannedProperties: "Sample.Banned" \ No newline at end of file diff --git a/mozilla-detekt-rules/src/test/java/org/mozilla/fenix/detektrules/MozillaBannedPropertyAccessTest.kt b/mozilla-detekt-rules/src/test/java/org/mozilla/fenix/detektrules/MozillaBannedPropertyAccessTest.kt new file mode 100644 index 000000000..0c12eaf1e --- /dev/null +++ b/mozilla-detekt-rules/src/test/java/org/mozilla/fenix/detektrules/MozillaBannedPropertyAccessTest.kt @@ -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 = + 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" + } + } + } + """ \ No newline at end of file diff --git a/settings.gradle b/settings.gradle index d2fc7cf20..9579227e2 100644 --- a/settings.gradle +++ b/settings.gradle @@ -1,5 +1,6 @@ include ':app' +include ':mozilla-detekt-rules' def log(message) { logger.lifecycle("[settings] ${message}") }