Merge pull request #754 from Kotlin/ktlint-migration

Replacing the linter

Author: Jolanrensen

.editorconfig

@@ -1,16 +1,37 @@
 [*]
-charset=utf-8
-end_of_line=lf
-insert_final_newline=true
-indent_style=space
-indent_size=4
-max_line_length=120
+charset = utf-8
+end_of_line = lf
+insert_final_newline = true
+indent_style = space
+indent_size = 4
+max_line_length = 120
 
 [*.json]
-indent_size=2
+indent_size = 2
 
 [{*.yaml,*.yml}]
-indent_size=2
+indent_size = 2
 
 [*.ipynb]
-insert_final_newline=false
+insert_final_newline = false
+
+[*.{kt,kts}]
+ij_kotlin_code_style_defaults = KOTLIN_OFFICIAL
+
+#  Disable wildcard imports entirely
+ij_kotlin_name_count_to_use_star_import = 2147483647
+ij_kotlin_name_count_to_use_star_import_for_members = 2147483647
+ij_kotlin_packages_to_use_import_on_demand = unset
+
+ktlint_code_style = ktlint_official
+ktlint_experimental = enabled
+ktlint_standard_filename = disabled
+ktlint_standard_no-empty-first-line-in-class-body = disabled
+ktlint_class_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than = 4
+ktlint_function_signature_rule_force_multiline_when_parameter_count_greater_or_equal_than = 4
+ktlint_standard_chain-method-continuation = disabled
+ktlint_ignore_back_ticked_identifier = true
+ktlint_standard_multiline-expression-wrapping = disabled
+
+[{*/build/**/*,**/*keywords*/**,**/*.Generated.kt,**/*$Extensions.kt}]
+ktlint = disabled

CONTRIBUTING.md

@@ -33,7 +33,8 @@ so do familiarize yourself with the following guidelines.
     * PR should be linked with the issue,
       excluding minor documentation changes, adding unit tests, and fixing typos.
 * If you make any code changes:
-    * Follow the [Kotlin Coding Conventions](https://kotlinlang.org/docs/reference/coding-conventions.html).
+    * Follow the [Kotlin Coding Conventions](https://kotlinlang.org/docs/reference/coding-conventions.html). 
+      [Ktlint](https://pinterest.github.io/ktlint/latest/) can help here.
     * [Build the project](#building) to ensure it all works and passes the tests.
 * If you fix a bug:
     * Write the test that reproduces the bug.
@@ -54,15 +55,18 @@ so do familiarize yourself with the following guidelines.
 0. The contributor builds the library locally and runs all unit tests via the Gradle task `dataframe:test`
    (see the ["Building"](#building) chapter).
 1. The contributor submits the PR if the local build is successful and the tests are green.
-2. The reviewer put his name in the "Reviewers" section of the proposed PR at the start of the review process.
-3. The reviewer leaves the comments or marks the PR with the abbreviation "LGTM" (Looks good to me).
+2. The reviewer puts their name in the "Reviewers" section of the proposed PR at the start of the review process.
+3. The reviewer leaves comments or marks the PR with the abbreviation "LGTM" (Looks good to me).
 4. The contributor answers the comments or fixes the proposed PR.
 5. The reviewer marks the PR with the word "LGTM."
-6. The maintainer could suggest merging the master branch to the PR branch a few times due to changes in the `master` branch.
-7. The maintainer runs TC builds (unit tests and examples as integration tests).
-8. TC writes the result (passed or not passed) to the PR checks at the bottom of the proposed PR.
-9. If it is possible, maintainers share the details of the failed build with the contributor.
-10. Maintainer merges the PR if all checks are successful and there is no conflict with the master branch.
+6. The maintainer could suggest merging the `master` branch to the PR branch a few times due to changes in the `master` branch.
+7. If the PR influences generated code/samples, a bot will inform about this in the PR comments.
+8. The maintainer runs TeamCity builds (unit tests and examples as integration tests).
+9. TeamCity writes the result (passed or not passed) to the PR checks at the bottom of the proposed PR.
+10. If it is possible, maintainers share the details of the failed build with the contributor.
+11. The maintainer merges the PR if all checks are successful and there is no conflict with the `master` branch.
+12. The maintainer closes the PR and the issue linked to it.
+13. If the PR influences generated code, a bot will auto-commit the newly generated code into the `master` branch.
 
 ## How to fix an existing issue
 
@@ -81,13 +85,22 @@ so do familiarize yourself with the following guidelines.
 
 ## Environment requirements
 
-JDK >= 11 referred to by the `JAVA_HOME` environment variable.
+* JDK >= 11 referred to by the `JAVA_HOME` environment variable.
+
+  * Note, any version above 11 should work in theory, but JDK 11 is the only version we test with,
+  so it is the recommended version.
+
+* We recommend using [IntelliJ IDEA](https://www.jetbrains.com/idea/download/) as the IDE. This
+has the best support for Kotlin, compiler plugins, Gradle, and [Kotlin Notebook](https://kotlinlang.org/docs/kotlin-notebook-overview.html) of course.
+
+* We recommend using the [Ktlint plugin](https://plugins.jetbrains.com/plugin/15057-ktlint) for [IntelliJ IDEA](https://www.jetbrains.com/idea/download/).
+It is able to read the `.editorconfig` file and apply the same formatting rules as [Ktlint](https://pinterest.github.io/ktlint/latest/) in the CI.
 
 ## Building
 
 This library is built with Gradle.
 
-* Run `./gradlew build` to build. It also runs all the tests.
+* Run `./gradlew build` to build. It also runs all the tests and checks the linter.
 * Run `./gradlew <module>:test` to test the module you are looking at to speed
   things up during development.
 

build.gradle.kts

@@ -10,6 +10,7 @@ import org.jetbrains.kotlinx.dataframe.io.readJson
 import org.jetbrains.kotlinx.publisher.apache2
 import org.jetbrains.kotlinx.publisher.developer
 import org.jetbrains.kotlinx.publisher.githubRepo
+import org.jlleitschuh.gradle.ktlint.KtlintExtension
 
 plugins {
     with(libs.plugins) {
@@ -19,7 +20,7 @@ plugins {
         alias(jupyter.api) apply false
         alias(dokka)
         alias(kover)
-        alias(kotlinter)
+        alias(ktlint)
         alias(korro) apply false
         alias(docProcessor) apply false
         alias(simpleGit) apply false
@@ -54,7 +55,12 @@ dependencies {
 }
 
 enum class Version : Comparable<Version> {
-    SNAPSHOT, DEV, ALPHA, BETA, RC, STABLE;
+    SNAPSHOT,
+    DEV,
+    ALPHA,
+    BETA,
+    RC,
+    STABLE,
 }
 
 fun String.findVersion(): Version {
@@ -73,13 +79,11 @@ fun String.findVersion(): Version {
 val dependencyUpdateExclusions = listOf(
     // TODO Requires more work to be updated to 1.7.0+, https://github.com/Kotlin/dataframe/issues/594
     libs.plugins.kover.get().pluginId,
-    // TODO Updating requires major changes all across the project, https://github.com/Kotlin/dataframe/issues/364
-    libs.plugins.kotlinter.get().pluginId,
     // TODO 5.8.0 is not possible due to https://github.com/Kotlin/dataframe/issues/595
     libs.kotestAssertions.get().name,
     // Can't be updated to 7.4.0+ due to Java 8 compatibility
     libs.android.gradle.api.get().group,
-    // TODO 0.1.6 breaks ktlint, https://github.com/Kotlin/dataframe/issues/598
+    // TODO 0.1.6 broke kotlinter, https://github.com/Kotlin/dataframe/issues/598
     libs.plugins.korro.get().pluginId,
     // Directly dependent on the Gradle version
     "org.gradle.kotlin.kotlin-dsl",
@@ -137,29 +141,15 @@ allprojects {
         targetCompatibility = JavaVersion.VERSION_1_8.toString()
     }
 
-    // Attempts to configure kotlinter for each sub-project that uses the plugin
+    // Attempts to configure ktlint for each sub-project that uses the plugin
     afterEvaluate {
         try {
-            kotlinter {
-                ignoreFailures = false
-                reporters = arrayOf("checkstyle", "plain")
-                experimentalRules = true
-                disabledRules = arrayOf(
-                    "no-wildcard-imports",
-                    "experimental:spacing-between-declarations-with-annotations",
-                    "experimental:enum-entry-name-case",
-                    "experimental:argument-list-wrapping",
-                    "experimental:annotation",
-                    "max-line-length",
-                    "filename",
-                    "comment-spacing",
-                    "curly-spacing",
-                    "experimental:annotation-spacing",
-                    "no-unused-imports", // broken
-                )
+            configure<KtlintExtension> {
+                version = "1.3.0"
+                // rules are set up through .editorconfig
             }
         } catch (_: UnknownDomainObjectException) {
-            logger.warn("Could not set kotlinter config on :${this.name}")
+            logger.warn("Could not set ktlint config on :${this.name}")
         }
 
         // set the java toolchain version to 11 for all subprojects for CI stability
@@ -223,13 +213,13 @@ kotlinPublications {
     sonatypeSettings(
         project.findProperty("kds.sonatype.user") as String?,
         project.findProperty("kds.sonatype.password") as String?,
-        "dataframe project, v. ${project.version}"
+        "dataframe project, v. ${project.version}",
     )
 
     signingCredentials(
         project.findProperty("kds.sign.key.id") as String?,
         project.findProperty("kds.sign.key.private") as String?,
-        project.findProperty("kds.sign.key.passphrase") as String?
+        project.findProperty("kds.sign.key.passphrase") as String?,
     )
 
     pom {

core/build.gradle.kts

@@ -5,7 +5,6 @@ import nl.jolanrensen.docProcessor.defaultProcessors.ARG_DOC_PROCESSOR_LOG_NOT_F
 import nl.jolanrensen.docProcessor.gradle.creatingProcessDocTask
 import org.gradle.jvm.tasks.Jar
 import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
-import org.jmailen.gradle.kotlinter.tasks.LintTask
 import xyz.ronella.gradle.plugin.simple.git.task.GitTask
 
 plugins {
@@ -17,7 +16,7 @@ plugins {
         alias(korro)
         alias(keywordGenerator)
         alias(kover)
-        alias(kotlinter)
+        alias(ktlint)
         alias(docProcessor)
         alias(simpleGit)
 
@@ -103,10 +102,6 @@ tasks.withType<KspTask> {
     }
 }
 
-tasks.named("lintKotlinSamples") {
-    onlyIf { false }
-}
-
 val clearTestResults by tasks.creating(Delete::class) {
     delete(layout.buildDirectory.dir("dataframes"))
     delete(layout.buildDirectory.dir("korroOutputLines"))
@@ -190,58 +185,64 @@ fun pathOf(vararg parts: String) = parts.joinToString(File.separator)
 val processKDocsMainSources = (kotlinMainSources + kotlinTestSources)
     .filterNot { pathOf("build", "generated") in it.path }
 
+// sourceset of the generated sources as a result of `processKDocsMain`, this will create linter tasks
+val generatedSources by kotlin.sourceSets.creating {
+    kotlin {
+        setSrcDirs(
+            listOf(
+                "build/generated/ksp/main/kotlin/",
+                "core/build/generatedSrc",
+                "$generatedSourcesFolderName/src/main/kotlin",
+                "$generatedSourcesFolderName/src/main/java",
+            ),
+        )
+    }
+}
+
 // Task to generate the processed documentation
 val processKDocsMain by creatingProcessDocTask(processKDocsMainSources) {
     target = file(generatedSourcesFolderName)
     arguments += ARG_DOC_PROCESSOR_LOG_NOT_FOUND to false
+
+    // false, so `runKtlintFormatOverGeneratedSourcesSourceSet` can format the output
+    outputReadOnly = false
+
     exportAsHtml {
         dir = file("../docs/StardustDocs/snippets/kdocs")
     }
     task {
         group = "KDocs"
-        // making sure it always runs, so targets is set
-        outputs.upToDateWhen { false }
+        finalizedBy("runKtlintFormatOverGeneratedSourcesSourceSet")
     }
 }
 
+tasks.named("ktlintGeneratedSourcesSourceSetCheck") {
+    onlyIf { false }
+}
+tasks.named("runKtlintCheckOverGeneratedSourcesSourceSet") {
+    onlyIf { false }
+}
+
 // Exclude the generated/processed sources from the IDE
 idea {
     module {
         excludeDirs.add(file(generatedSourcesFolderName))
     }
 }
 
-// if `processKDocsMain` runs, the Jar tasks must run after it so the generated-sources are there
-tasks.withType<Jar> {
-    mustRunAfter(tasks.generateKeywordsSrc, processKDocsMain)
-}
-
 // If `changeJarTask` is run, modify all Jar tasks such that before running the Kotlin sources are set to
 // the target of `processKdocMain`, and they are returned to normal afterward.
 // This is usually only done when publishing
 val changeJarTask by tasks.creating {
     outputs.upToDateWhen { false }
     doFirst {
         tasks.withType<Jar> {
-            dependsOn(processKDocsMain)
             doFirst {
-                val targets = processKDocsMain.targets
-                require(targets.toList().isNotEmpty()) {
-                    logger.error("`processKDocsMain.targets` was empty, did it run before this task?")
+                require(generatedSources.kotlin.srcDirs.toList().isNotEmpty()) {
+                    logger.error("`processKDocsMain`'s outputs are empty, did `processKDocsMain` run before this task?")
                 }
-                val srcDirs = targets
-                    .filterNot {
-                        pathOf("src", "test", "kotlin") in it.path ||
-                            pathOf("src", "test", "java") in it.path
-                    } // filter out test sources again
-                    .plus(
-                        kotlinMainSources.filter {
-                            pathOf("build", "generated") in it.path
-                        },
-                    ) // Include generated sources (which were excluded above)
-
                 kotlin.sourceSets.main {
-                    kotlin.setSrcDirs(srcDirs)
+                    kotlin.setSrcDirs(generatedSources.kotlin.srcDirs)
                 }
                 logger.lifecycle("$this is run with modified sources: \"$generatedSourcesFolderName\"")
             }
@@ -255,6 +256,11 @@ val changeJarTask by tasks.creating {
     }
 }
 
+// if `processKDocsMain` runs, the Jar tasks must run after it so the generated-sources are there
+tasks.withType<Jar> {
+    mustRunAfter(changeJarTask, tasks.generateKeywordsSrc, processKDocsMain)
+}
+
 // modify all publishing tasks to depend on `changeJarTask` so the sources are swapped out with generated sources
 tasks.named { it.startsWith("publish") }.configureEach {
     dependsOn(processKDocsMain, changeJarTask)
@@ -316,35 +322,34 @@ tasks.withType<KspTaskJvm> {
     dependsOn(tasks.generateKeywordsSrc)
 }
 
-tasks.formatKotlinMain {
+tasks.runKtlintFormatOverMainSourceSet {
     dependsOn(tasks.generateKeywordsSrc)
     dependsOn("kspKotlin")
 }
 
-tasks.formatKotlinTest {
+tasks.runKtlintFormatOverTestSourceSet {
     dependsOn(tasks.generateKeywordsSrc)
     dependsOn("kspTestKotlin")
 }
 
-tasks.lintKotlinMain {
+tasks.named("runKtlintFormatOverGeneratedSourcesSourceSet") {
     dependsOn(tasks.generateKeywordsSrc)
     dependsOn("kspKotlin")
 }
 
-tasks.lintKotlinTest {
+tasks.runKtlintCheckOverMainSourceSet {
+    dependsOn(tasks.generateKeywordsSrc)
+    dependsOn("kspKotlin")
+}
+
+tasks.runKtlintCheckOverTestSourceSet {
     dependsOn(tasks.generateKeywordsSrc)
     dependsOn("kspTestKotlin")
 }
 
-tasks.withType<LintTask> {
-    exclude("**/*keywords*/**")
-    exclude {
-        it.name.endsWith(".Generated.kt")
-    }
-    exclude {
-        it.name.endsWith("\$Extensions.kt")
-    }
-    enabled = true
+tasks.named("runKtlintCheckOverGeneratedSourcesSourceSet") {
+    dependsOn(tasks.generateKeywordsSrc)
+    dependsOn("kspKotlin")
 }
 
 kotlin {
@@ -369,7 +374,7 @@ tasks.test {
             listOf(
                 "org.jetbrains.kotlinx.dataframe.jupyter.*",
                 "org.jetbrains.kotlinx.dataframe.jupyter.SampleNotebooksTests",
-            )
+            ),
         )
     }
 }