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

Replacing the linter #754

Merged
merged 14 commits into from
Jul 25, 2024
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.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
39 changes: 30 additions & 9 deletions .editorconfig
Original file line number Diff line number Diff line change
@@ -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
33 changes: 23 additions & 10 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand All @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's with the formatting here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a double indent

  • something
    • like this

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.

Expand Down
42 changes: 16 additions & 26 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
91 changes: 48 additions & 43 deletions core/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -17,7 +16,7 @@ plugins {
alias(korro)
alias(keywordGenerator)
alias(kover)
alias(kotlinter)
alias(ktlint)
alias(docProcessor)
alias(simpleGit)

Expand Down Expand Up @@ -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"))
Expand Down Expand Up @@ -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\"")
}
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -369,7 +374,7 @@ tasks.test {
listOf(
"org.jetbrains.kotlinx.dataframe.jupyter.*",
"org.jetbrains.kotlinx.dataframe.jupyter.SampleNotebooksTests",
)
),
)
}
}
Expand Down
Loading
Loading