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

Create Rule Set For Libraries #5360

Merged
merged 7 commits into from Oct 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions build-logic/src/main/kotlin/releasing.gradle.kts
Expand Up @@ -40,6 +40,8 @@ project.afterEvaluate {
cliBuildDir.resolve("distributions/detekt-cli-${project.version}.zip"),
project(":detekt-formatting").buildDir.resolve("libs/detekt-formatting-${project.version}.jar"),
project(":detekt-generator").buildDir.resolve("libs/detekt-generator-${project.version}-all.jar"),
project(":detekt-rules-libraries").buildDir
.resolve("libs/detekt-rules-libraries-${project.version}.jar"),
project(":detekt-rules-ruleauthors").buildDir
.resolve("libs/detekt-rules-ruleauthors-${project.version}.jar")
)
Expand Down
1 change: 1 addition & 0 deletions build.gradle.kts
Expand Up @@ -27,6 +27,7 @@ allprojects {
dependencies {
detekt(project(":detekt-cli"))
detektPlugins(project(":detekt-formatting"))
detektPlugins(project(":detekt-rules-libraries"))
detektPlugins(project(":detekt-rules-ruleauthors"))
}

Expand Down
14 changes: 10 additions & 4 deletions config/detekt/detekt.yml
Expand Up @@ -190,10 +190,6 @@ style:
- 'org.jetbrains.kotlin.diagnostics.DiagnosticUtils.getLineAndColumnInPsiFile'
ForbiddenVoid:
active: true
LibraryCodeMustSpecifyReturnType:
active: true
excludes: ['**/*.kt']
includes: ['**/detekt-api/src/main/**/api/*.kt']
VitalyVPinchuk marked this conversation as resolved.
Show resolved Hide resolved
MagicNumber:
excludes: ['**/test/**', '**/*Test.kt', '**/*Spec.kt']
ignorePropertyDeclaration: true
Expand Down Expand Up @@ -271,3 +267,13 @@ style:
WildcardImport:
active: true
excludeImports: []

libraries:
ForbiddenPublicDataClass:
active: false
LibraryEntitiesShouldNotBePublic:
active: false
LibraryCodeMustSpecifyReturnType:
active: true
excludes: ['**/*.kt']
includes: ['**/detekt-api/src/main/**/api/*.kt']
10 changes: 6 additions & 4 deletions detekt-cli/build.gradle.kts
Expand Up @@ -8,7 +8,7 @@ application {
mainClass.set("io.gitlab.arturbosch.detekt.cli.Main")
}

val formattingJar by configurations.creating {
val pluginsJar: Configuration by configurations.creating {
isTransitive = false
}

Expand All @@ -27,7 +27,9 @@ dependencies {
testImplementation(projects.detektTestUtils)
testImplementation(libs.assertj)

formattingJar(projects.detektFormatting)
pluginsJar(projects.detektFormatting)
pluginsJar(projects.detektRulesLibraries)
pluginsJar(projects.detektRulesRuleauthors)
}

val javaComponent = components["java"] as AdhocComponentWithVariants
Expand All @@ -54,11 +56,11 @@ tasks {

val runWithArgsFile by registering(JavaExec::class) {
// The task generating these jar files run first.
inputs.files(formattingJar)
inputs.files(pluginsJar)
doNotTrackState("The entire root directory is read as the input source.")
classpath = files(shadowJar)
workingDir = rootDir
args = listOf("@./config/detekt/argsfile", "-p", formattingJar.singleFile.path)
args = listOf("@./config/detekt/argsfile", "-p", pluginsJar.files.joinToString(",") { it.path })
}

check {
Expand Down
12 changes: 0 additions & 12 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -544,12 +544,6 @@ style:
value: 'kotlin.io.print'
- reason: 'println does not allow you to configure the output stream. Use a logger instead.'
value: 'kotlin.io.println'
ForbiddenPublicDataClass:
active: true
excludes: ['**']
ignorePackages:
- '*.internal'
- '*.internal.*'
ForbiddenSuppress:
active: false
rules: []
Expand All @@ -562,12 +556,6 @@ style:
ignoreOverridableFunction: true
ignoreActualFunction: true
excludedFunctions: []
LibraryCodeMustSpecifyReturnType:
active: true
excludes: ['**']
LibraryEntitiesShouldNotBePublic:
active: true
excludes: ['**']
LoopWithTooManyJumpStatements:
active: true
maxJumpCount: 1
Expand Down
3 changes: 3 additions & 0 deletions detekt-core/src/main/resources/deprecation.properties
Expand Up @@ -13,3 +13,6 @@ style>UnnecessaryAbstractClass>excludeAnnotatedClasses=Use `ignoreAnnotated` ins
style>UseDataClass>excludeAnnotatedClasses=Use `ignoreAnnotated` instead
formatting>Indentation>continuationIndentSize=`continuationIndentSize` is ignored by KtLint and will have no effect
formatting>ParameterListWrapping>indentSize=`indentSize` is ignored by KtLint and will have no effect
style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin
style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin
style>LibraryEntitiesShouldNotBePublic=Rule migrated to `libraries` ruleset plugin
12 changes: 11 additions & 1 deletion detekt-generator/build.gradle.kts
Expand Up @@ -26,6 +26,7 @@ val cliOptionsFile = "${rootProject.rootDir}/website/docs/gettingstarted/_cli-op
val defaultConfigFile = "$configDir/default-detekt-config.yml"
val deprecationFile = "$configDir/deprecation.properties"
val formattingConfigFile = "${rootProject.rootDir}/detekt-formatting/src/main/resources/config/config.yml"
val librariesConfigFile = "${rootProject.rootDir}/detekt-rules-libraries/src/main/resources/config/config.yml"
val ruleauthorsConfigFile = "${rootProject.rootDir}/detekt-rules-ruleauthors/src/main/resources/config/config.yml"

val ruleModules = rootProject.subprojects
Expand All @@ -35,12 +36,19 @@ val ruleModules = rootProject.subprojects
.map { "${rootProject.rootDir}/$it/src/main/kotlin" }

val generateDocumentation by tasks.registering(JavaExec::class) {
dependsOn(tasks.assemble, ":detekt-api:dokkaHtml", tasks.shadowJar, ":detekt-rules-ruleauthors:sourcesJar")
dependsOn(
tasks.assemble,
tasks.shadowJar,
":detekt-api:dokkaHtml",
":detekt-rules-libraries:sourcesJar",
":detekt-rules-ruleauthors:sourcesJar",
)
description = "Generates detekt documentation and the default config.yml based on Rule KDoc"
group = "documentation"

inputs.files(
ruleModules.map { fileTree(it) },
fileTree("${rootProject.rootDir}/detekt-rules-libraries/src/main/kotlin"),
fileTree("${rootProject.rootDir}/detekt-rules-ruleauthors/src/main/kotlin"),
fileTree("${rootProject.rootDir}/detekt-formatting/src/main/kotlin"),
file("${rootProject.rootDir}/detekt-generator/build/libs/detekt-generator-${Versions.DETEKT}-all.jar"),
Expand All @@ -50,6 +58,7 @@ val generateDocumentation by tasks.registering(JavaExec::class) {
fileTree(documentationDir),
file(defaultConfigFile),
file(formattingConfigFile),
file(librariesConfigFile),
file(ruleauthorsConfigFile),
file(deprecationFile),
file(cliOptionsFile),
Expand All @@ -64,6 +73,7 @@ val generateDocumentation by tasks.registering(JavaExec::class) {
args = listOf(
"--input",
ruleModules
.plus("${rootProject.rootDir}/detekt-rules-libraries/src/main/kotlin")
.plus("${rootProject.rootDir}/detekt-rules-ruleauthors/src/main/kotlin")
.plus("${rootProject.rootDir}/detekt-formatting/src/main/kotlin")
.joinToString(","),
Expand Down
Expand Up @@ -24,7 +24,9 @@ class DetektPrinter(private val arguments: GeneratorArgs) {
}
}
yamlWriter.write(arguments.configPath, "default-detekt-config") {
ConfigPrinter.print(pages.filterNot { it.ruleSet.name == "formatting" || it.ruleSet.name == "ruleauthors" })
ConfigPrinter.print(
pages.filterNot { it.ruleSet.name in listOf("formatting", "libraries", "ruleauthors") }
)
}
propertiesWriter.write(arguments.configPath, "deprecation") {
// We intentionally not filter for "formatting" as we want to be able to deprecate
Expand All @@ -36,6 +38,11 @@ class DetektPrinter(private val arguments: GeneratorArgs) {
printRuleSetPage(pages.first { it.ruleSet.name == "formatting" })
}
}
yamlWriter.write(Paths.get("../detekt-rules-libraries/src/main/resources/config"), "config") {
yaml {
printRuleSetPage(pages.first { it.ruleSet.name == "libraries" })
}
}
yamlWriter.write(Paths.get("../detekt-rules-ruleauthors/src/main/resources/config"), "config") {
yaml {
printRuleSetPage(pages.first { it.ruleSet.name == "ruleauthors" })
Expand Down
Expand Up @@ -11,7 +11,7 @@ object DeprecatedPrinter : DocumentationPrinter<List<RuleSetPage>> {
item.forEach { ruleSet ->
ruleSet.rules.forEach { rule ->
if (rule.isDeprecated()) {
builder.appendLine(writeRuleProperty(ruleSet, rule))
builder.appendLine(writeRule(ruleSet, rule))
}
rule.configuration.forEach { configuration ->
if (configuration.isDeprecated()) {
Expand All @@ -20,16 +20,25 @@ object DeprecatedPrinter : DocumentationPrinter<List<RuleSetPage>> {
}
}
}
builder.appendLine(writeMigratedRules())
return builder.toString()
}
}

private fun writeRule(ruleSet: RuleSetPage, rule: Rule): String {
@Suppress("UnsafeCallOnNullableType")
return "${ruleSet.ruleSet.name}>${rule.name}=${rule.deprecationMessage!!}"
}

private fun writeProperty(ruleSet: RuleSetPage, rule: Rule, configuration: Configuration): String {
@Suppress("UnsafeCallOnNullableType")
return "${ruleSet.ruleSet.name}>${rule.name}>${configuration.name}=${configuration.deprecated!!}"
}

private fun writeRuleProperty(ruleSet: RuleSetPage, rule: Rule): String {
@Suppress("UnsafeCallOnNullableType")
return "${ruleSet.ruleSet.name}>${rule.name}=${rule.deprecationMessage!!}"
internal fun writeMigratedRules(): String {
return """
style>ForbiddenPublicDataClass=Rule migrated to `libraries` ruleset plugin
style>LibraryCodeMustSpecifyReturnType=Rule migrated to `libraries` ruleset plugin
style>LibraryEntitiesShouldNotBePublic=Rule migrated to `libraries` ruleset plugin
""".trimIndent()
}
Expand Up @@ -5,7 +5,7 @@ import io.gitlab.arturbosch.detekt.generator.collection.Rule
/**
* Holds a list of extra exclusions for rules and rule sets.
*/
val exclusions = arrayOf(TestExclusions, KotlinScriptExclusions, KotlinScriptAndTestExclusions, LibraryExclusions)
val exclusions = arrayOf(TestExclusions, KotlinScriptExclusions, KotlinScriptAndTestExclusions)

/**
* Tracks rules and rule sets which needs an extra `excludes: $pattern` property
Expand Down Expand Up @@ -56,13 +56,3 @@ private object KotlinScriptAndTestExclusions : Exclusions() {
"'**/*.kts']"
override val rules = setOf("MagicNumber")
}

private object LibraryExclusions : Exclusions() {

override val pattern = "['**']"
override val rules = setOf(
"ForbiddenPublicDataClass",
"LibraryCodeMustSpecifyReturnType",
"LibraryEntitiesShouldNotBePublic",
)
}
Expand Up @@ -13,8 +13,7 @@ class DeprecatedPrinterSpec {
style>MagicNumber>conf2=use conf1 instead
style>MagicNumber>conf4=use conf3 instead
style>DuplicateCaseInWhenExpression=is deprecated

""".trimIndent()
""".trimIndent() + "\n${writeMigratedRules()}\n"
assertThat(markdownString).isEqualTo(expectedMarkdownString)
}
}
9 changes: 9 additions & 0 deletions detekt-rules-libraries/build.gradle.kts
@@ -0,0 +1,9 @@
plugins {
id("module")
}

dependencies {
compileOnly(projects.detektApi)
testImplementation(projects.detektTest)
testImplementation(libs.assertj)
}
@@ -1,4 +1,4 @@
package io.gitlab.arturbosch.detekt.rules.style
package io.gitlab.arturbosch.detekt.libraries

import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
Expand Down
@@ -1,4 +1,4 @@
package io.gitlab.arturbosch.detekt.rules.style
package io.gitlab.arturbosch.detekt.libraries

import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
Expand Down
@@ -1,4 +1,4 @@
package io.gitlab.arturbosch.detekt.rules.style
package io.gitlab.arturbosch.detekt.libraries

import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
Expand Down
@@ -0,0 +1,24 @@
package io.gitlab.arturbosch.detekt.libraries

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.RuleSet
import io.gitlab.arturbosch.detekt.api.RuleSetProvider
import io.gitlab.arturbosch.detekt.api.internal.ActiveByDefault

/**
* Rules in this rule set report issues related to libraries API exposure.
*/
@ActiveByDefault("1.16.0")
class RuleLibrariesProvider : RuleSetProvider {

override val ruleSetId: String = "libraries"

override fun instance(config: Config) = RuleSet(
ruleSetId,
listOf(
ForbiddenPublicDataClass(config),
LibraryEntitiesShouldNotBePublic(config),
LibraryCodeMustSpecifyReturnType(config),
)
)
}
@@ -0,0 +1 @@
io.gitlab.arturbosch.detekt.libraries.RuleLibrariesProvider
11 changes: 11 additions & 0 deletions detekt-rules-libraries/src/main/resources/config/config.yml
@@ -0,0 +1,11 @@
libraries:
Copy link
Member

Choose a reason for hiding this comment

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

@BraisGabin is this an internal file to Detekt, or ruleset plugin authors can also provide this file?

Copy link
Member

Choose a reason for hiding this comment

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

This one targets library authors. I don't know if this one could be useful for plugin authors. I don't recall the name now but on this version we are going to add other rule set only for rule authors.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I meant exactly this file, the config yml. Because src/main/resources will get packaged into the jar by default, and can be opened using a Class. If this is only used for generating something internally, it might be better to move it out of this folder so it doesn't get delivered. So my question is if it's being used by some mechanism outside of docs generation.

Copy link
Member

Choose a reason for hiding this comment

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

Looking forward to rules for rules! That's a great plugin we really need.

Copy link
Member

Choose a reason for hiding this comment

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

If this is only used for generating something internally, it might be better to move it out of this folder so it doesn't get delivered. So my question is if it's being used by some mechanism outside of docs generation.

This file is used when generating the default config file for the users as well as during validation, so it's definitelly needed and should be shipped

Copy link
Member

Choose a reason for hiding this comment

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

Thank you :)

active: true
ForbiddenPublicDataClass:
active: true
ignorePackages:
- '*.internal'
- '*.internal.*'
LibraryCodeMustSpecifyReturnType:
active: true
LibraryEntitiesShouldNotBePublic:
active: true
@@ -1,4 +1,4 @@
package io.gitlab.arturbosch.detekt.rules.style
package io.gitlab.arturbosch.detekt.libraries

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.test.TestConfig
Expand Down
@@ -1,4 +1,4 @@
package io.gitlab.arturbosch.detekt.rules.style
package io.gitlab.arturbosch.detekt.libraries

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.rules.KotlinCoreEnvironmentTest
Expand Down
@@ -1,4 +1,4 @@
package io.gitlab.arturbosch.detekt.rules.style
package io.gitlab.arturbosch.detekt.libraries

import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.test.TestConfig
Expand Down
Expand Up @@ -41,7 +41,6 @@ class StyleGuideProvider : DefaultRuleSetProvider {
ForbiddenComment(config),
ForbiddenImport(config),
ForbiddenMethodCall(config),
ForbiddenPublicDataClass(config),
ForbiddenSuppress(config),
FunctionOnlyReturningConstant(config),
SpacingBetweenPackageAndImports(config),
Expand Down Expand Up @@ -89,8 +88,6 @@ class StyleGuideProvider : DefaultRuleSetProvider {
UseCheckOrError(config),
UseIfInsteadOfWhen(config),
RedundantExplicitType(config),
LibraryEntitiesShouldNotBePublic(config),
LibraryCodeMustSpecifyReturnType(config),
UseArrayLiteralsInAnnotations(config),
UseEmptyCounterpart(config),
UseCheckNotNull(config),
Expand Down
1 change: 1 addition & 0 deletions settings.gradle.kts
Expand Up @@ -28,6 +28,7 @@ include("detekt-rules-documentation")
include("detekt-rules-empty")
include("detekt-rules-errorprone")
include("detekt-rules-exceptions")
include("detekt-rules-libraries")
include("detekt-rules-naming")
include("detekt-rules-performance")
include("detekt-rules-ruleauthors")
Expand Down