From 8402fccb32d1b8cdf3c881ffb2c3ae3afb184da4 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Fri, 20 Nov 2020 11:57:47 +0300 Subject: [PATCH 1/3] Added documentation for property tag usage in KDocs ### What's done: * Updated documentation * Updated build.gradle.kts for gradle plugin for better loading in IDE --- diktat-gradle-plugin/build.gradle.kts | 11 +-- info/guide/diktat-coding-convention.md | 107 +++++++++++++++++++++++-- info/guide/guide-chapter-2.md | 28 ++++++- 3 files changed, 134 insertions(+), 12 deletions(-) diff --git a/diktat-gradle-plugin/build.gradle.kts b/diktat-gradle-plugin/build.gradle.kts index 9837d19807..d75a7f5521 100644 --- a/diktat-gradle-plugin/build.gradle.kts +++ b/diktat-gradle-plugin/build.gradle.kts @@ -14,8 +14,9 @@ repositories { jcenter() } -val ktlintVersion: String by project -val diktatVersion = project.version +// default value is needed for correct gradle loading in IDEA; actual value from maven is used during build +val ktlintVersion: String = project.properties.getOrDefault("ktlintVersion", "0.39.0") as String +val diktatVersion = project.version.takeIf { it.toString() != Project.DEFAULT_VERSION } ?: "0.1.6-SNAPSHOT" dependencies { implementation(kotlin("gradle-plugin-api")) @@ -23,11 +24,11 @@ dependencies { exclude("com.pinterest.ktlint", "ktlint-ruleset-standard") } implementation("com.pinterest.ktlint:ktlint-reporter-plain:$ktlintVersion") - implementation("org.cqfn.diktat:diktat-rules:$version") + implementation("org.cqfn.diktat:diktat-rules:$diktatVersion") } val generateVersionsFile by tasks.registering { - val versionsFile = File("$buildDir/generated/src/main/generated/Versions.kt") + val versionsFile = File("$buildDir/generated/src/generated/Versions.kt") outputs.file(versionsFile) @@ -43,7 +44,7 @@ val generateVersionsFile by tasks.registering { ) } } -sourceSets.main.get().java.srcDir("$buildDir/generated/src/main") +sourceSets.main.get().java.srcDir("$buildDir/generated/src") tasks.withType { kotlinOptions { diff --git a/info/guide/diktat-coding-convention.md b/info/guide/diktat-coding-convention.md index e911270db3..afec66eea6 100644 --- a/info/guide/diktat-coding-convention.md +++ b/info/guide/diktat-coding-convention.md @@ -355,6 +355,32 @@ fun isEmptyList(list: List) = list.size == 0 3. You can skip KDocs for a method's override if the method is almost like the super class method. +4. Class properties declared in the primary constructor should be documented using `@property` tag in the class KDoc. + +Incorrect example: +```kotlin +/** + * Class description + */ +class Example( + /** + * property description + */ + val foo: Bar +) +``` + +Correct example: +```kotlin +/** + * Class description + * @property foo property description + */ +class Example( + val foo: Bar +) +``` + ### Rule 2.1.2: When the method has arguments, return value, can throw exceptions, etc., it must be described in the KDoc block: with @param, @return, @throws, etc. **Valid examples**: @@ -422,7 +448,7 @@ This comment should contain @since tag. The good style is to write the version o */ ``` -Other KDoc tags (such as @param type parameters and @see.) can be added as follow: +Other KDoc tags (such as @param type parameters and @see.) can be added as follows: ```kotlin /** * Description of functionality @@ -1901,35 +1927,37 @@ It is one of the exceptions from the [identifier names rule](#r1.2) Kotlin has a perfect mechanism of [properties](https://kotlinlang.org/docs/reference/properties.html#properties-and-fields). Kotlin compiler automatically generates `get` and `set` methods for properties and also lets the possibility to override it: ```kotlin -// Bad example ====== class A { var size: Int = 0 set(value) { println("Side effect") field = value } - get() = this.hashCode() * 2 + // user of this class does not expect calling A.size receive size * 2 + get() = field * 2 } ``` From the callee code these methods look like an access to this property: `A().isEmpty = true` for setter and `A().isEmpty` for getter. -But in all cases it is very confusing when `get` and `set` are overriden for a developer who uses this particular class. +But in all cases it is very confusing when `get` and `set` are overridden for a developer who uses this particular class. Developer expects to get the value of the property, but receives some unknown value and some extra side effect hidden by the custom getter/setter. Use extra functions for it instead. **Invalid example:** ```kotlin -// Bad example ====== class A { var size: Int = 0 fun initSize(value: Int) { // some custom logic } - fun goodNameThatDescribesThisGetter() = this.hashCode() * 2 + // this will not confuse developer and he will get exactly what he expects + fun goodNameThatDescribesThisGetter() = this.size * 2 } ``` +**Exception:** `Private setters` are only exceptions that are not prohibited by this rule. + ### Rule 6.1.9: never use the name of a variable in the custom getter or setter (possible_bug). Even if you have ignored [recommendation 6.1.8](#r6.1.8) you should be careful with using the name of the property in your custom getter/setter as it can accidentally cause a recursive call and a `StackOverflow Error`. Use `field` keyword instead. @@ -2095,4 +2123,71 @@ interface I { object O: I { override fun foo() {} } +``` +# 8. Things that will be moved to a main guide later + +### Null-safety +### 4.3.3 Explicit null checks + +Try to avoid explicit null checks (explicit comparison with `null`) +Kotlin is declared as [Null-safe](https://kotlinlang.org/docs/reference/null-safety.html) language. +But Kotlin architects wanted Kotlin to be fully compatible with Java, that's why `null` keyword was also introduced in Kotlin. + +There are several code-structures that can be used in Kotlin to avoid null-checks. For example: `?:`, `.let {}`, `.also {}`, e.t.c + +**Invalid example:** +```kotlin +// example 1 +var myVar: Int? = null +if (myVar == null) { + println("null") + return +} + +// example 2 +if (myVar != null) { + println("not null") + return +} + +// example 3 +val anotherVal = if (myVar != null) { + println("not null") + 1 + } else { + 2 + } +// example 4 +if (myVar == null) { + println("null") +} else { + println("not null") +} +``` + +**Valid example:** +```kotlin +// example 1 +var myVar: Int? = null +myVar?: run { + println("null") + return +} + +// example 2 +myVar?.let { + println("not null") + return +} + +// example 3 +val anotherVal = myVar?.also { + println("not null") + 1 + } ?: 2 + +// example 4 +myVar?.let { + println("null") +} ?: run { println("not null") } ``` \ No newline at end of file diff --git a/info/guide/guide-chapter-2.md b/info/guide/guide-chapter-2.md index 1b3e5cdba4..65dfa10811 100644 --- a/info/guide/guide-chapter-2.md +++ b/info/guide/guide-chapter-2.md @@ -53,6 +53,32 @@ fun isEmptyList(list: List) = list.size == 0 3. You can skip KDocs for a method's override if the method is almost like the super class method. +4. Class properties declared in the primary constructor should be documented using `@property` tag in the class KDoc. + +Incorrect example: +```kotlin +/** + * Class description + */ +class Example( + /** + * property description + */ + val foo: Bar +) +``` + +Correct example: +```kotlin +/** + * Class description + * @property foo property description + */ +class Example( + val foo: Bar +) +``` + ### Rule 2.1.2: When the method has arguments, return value, can throw exceptions, etc., it must be described in the KDoc block: with @param, @return, @throws, etc. **Valid examples**: @@ -120,7 +146,7 @@ This comment should contain @since tag. The good style is to write the version o */ ``` -Other KDoc tags (such as @param type parameters and @see.) can be added as follow: +Other KDoc tags (such as @param type parameters and @see.) can be added as follows: ```kotlin /** * Description of functionality From 790eb9a372f7ad7252ae1acff4f225d7200adda1 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Fri, 20 Nov 2020 12:43:11 +0300 Subject: [PATCH 2/3] Added documentation for property tag usage in KDocs ### What's done: * Updated documentation --- info/guide/guide-chapter-2.md | 53 +++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/info/guide/guide-chapter-2.md b/info/guide/guide-chapter-2.md index 65dfa10811..4a87f99ed2 100644 --- a/info/guide/guide-chapter-2.md +++ b/info/guide/guide-chapter-2.md @@ -32,28 +32,10 @@ When the entire KDoc block can be stored in one line (and there is no KDoc mark ### Rule 2.1.1: KDoc is used for each public, protected or internal code element -At a minimum, KDoc should be used for every public, protected, or internal decorated class, interface, enumeration, method, and member field (property). Other code blocks can also have KDocs if needed. - -Exceptions: - -1. For setters/getters of properties obvious comments are optional. - (Note that simple `get/set` methods are generated by Kotlin under the hood). For example, getFoo can also be `return foo`. - -2. It is optional to add comments for simple one-line methods like: -```kotlin -val isEmpty: Boolean - get() = this.size == 0 -``` - -or - -```kotlin -fun isEmptyList(list: List) = list.size == 0 -``` - -3. You can skip KDocs for a method's override if the method is almost like the super class method. - -4. Class properties declared in the primary constructor should be documented using `@property` tag in the class KDoc. +At a minimum, KDoc should be used for every public, protected, or internal decorated class, interface, enumeration, method, and member field (property). +Other code blocks can also have KDocs if needed. +Instead of using comments or KDocs before properties in the primary constructor of a class - use `@property` tag in a KDoc of a class. +All properties of the primary constructor should be also documented in a KDoc with a `@property` tag. Incorrect example: ```kotlin @@ -64,7 +46,9 @@ class Example( /** * property description */ - val foo: Bar + val foo: Foo, + // another property description + val bar: Bar ) ``` @@ -73,12 +57,33 @@ Correct example: /** * Class description * @property foo property description + * @property bar another property description */ class Example( - val foo: Bar + val foo: Foo, + val bar: Bar ) ``` +Exceptions: + +1. For setters/getters of properties obvious comments are optional. + (Note that simple `get/set` methods are generated by Kotlin under the hood). For example, getFoo can also be `return foo`. + +2. It is optional to add comments for simple one-line methods like: +```kotlin +val isEmpty: Boolean + get() = this.size == 0 +``` + +or + +```kotlin +fun isEmptyList(list: List) = list.size == 0 +``` + +3. You can skip KDocs for a method's override if the method is almost like the super class method. + ### Rule 2.1.2: When the method has arguments, return value, can throw exceptions, etc., it must be described in the KDoc block: with @param, @return, @throws, etc. **Valid examples**: From 9d5ceed02092f5de8d9b1603fe5c51aa0ce986e5 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Fri, 20 Nov 2020 12:45:38 +0300 Subject: [PATCH 3/3] Added documentation for property tag usage in KDocs ### What's done: * Reverted changes in full guide --- info/guide/diktat-coding-convention.md | 107 ++----------------------- 1 file changed, 6 insertions(+), 101 deletions(-) diff --git a/info/guide/diktat-coding-convention.md b/info/guide/diktat-coding-convention.md index afec66eea6..e911270db3 100644 --- a/info/guide/diktat-coding-convention.md +++ b/info/guide/diktat-coding-convention.md @@ -355,32 +355,6 @@ fun isEmptyList(list: List) = list.size == 0 3. You can skip KDocs for a method's override if the method is almost like the super class method. -4. Class properties declared in the primary constructor should be documented using `@property` tag in the class KDoc. - -Incorrect example: -```kotlin -/** - * Class description - */ -class Example( - /** - * property description - */ - val foo: Bar -) -``` - -Correct example: -```kotlin -/** - * Class description - * @property foo property description - */ -class Example( - val foo: Bar -) -``` - ### Rule 2.1.2: When the method has arguments, return value, can throw exceptions, etc., it must be described in the KDoc block: with @param, @return, @throws, etc. **Valid examples**: @@ -448,7 +422,7 @@ This comment should contain @since tag. The good style is to write the version o */ ``` -Other KDoc tags (such as @param type parameters and @see.) can be added as follows: +Other KDoc tags (such as @param type parameters and @see.) can be added as follow: ```kotlin /** * Description of functionality @@ -1927,37 +1901,35 @@ It is one of the exceptions from the [identifier names rule](#r1.2) Kotlin has a perfect mechanism of [properties](https://kotlinlang.org/docs/reference/properties.html#properties-and-fields). Kotlin compiler automatically generates `get` and `set` methods for properties and also lets the possibility to override it: ```kotlin +// Bad example ====== class A { var size: Int = 0 set(value) { println("Side effect") field = value } - // user of this class does not expect calling A.size receive size * 2 - get() = field * 2 + get() = this.hashCode() * 2 } ``` From the callee code these methods look like an access to this property: `A().isEmpty = true` for setter and `A().isEmpty` for getter. -But in all cases it is very confusing when `get` and `set` are overridden for a developer who uses this particular class. +But in all cases it is very confusing when `get` and `set` are overriden for a developer who uses this particular class. Developer expects to get the value of the property, but receives some unknown value and some extra side effect hidden by the custom getter/setter. Use extra functions for it instead. **Invalid example:** ```kotlin +// Bad example ====== class A { var size: Int = 0 fun initSize(value: Int) { // some custom logic } - // this will not confuse developer and he will get exactly what he expects - fun goodNameThatDescribesThisGetter() = this.size * 2 + fun goodNameThatDescribesThisGetter() = this.hashCode() * 2 } ``` -**Exception:** `Private setters` are only exceptions that are not prohibited by this rule. - ### Rule 6.1.9: never use the name of a variable in the custom getter or setter (possible_bug). Even if you have ignored [recommendation 6.1.8](#r6.1.8) you should be careful with using the name of the property in your custom getter/setter as it can accidentally cause a recursive call and a `StackOverflow Error`. Use `field` keyword instead. @@ -2123,71 +2095,4 @@ interface I { object O: I { override fun foo() {} } -``` -# 8. Things that will be moved to a main guide later - -### Null-safety -### 4.3.3 Explicit null checks - -Try to avoid explicit null checks (explicit comparison with `null`) -Kotlin is declared as [Null-safe](https://kotlinlang.org/docs/reference/null-safety.html) language. -But Kotlin architects wanted Kotlin to be fully compatible with Java, that's why `null` keyword was also introduced in Kotlin. - -There are several code-structures that can be used in Kotlin to avoid null-checks. For example: `?:`, `.let {}`, `.also {}`, e.t.c - -**Invalid example:** -```kotlin -// example 1 -var myVar: Int? = null -if (myVar == null) { - println("null") - return -} - -// example 2 -if (myVar != null) { - println("not null") - return -} - -// example 3 -val anotherVal = if (myVar != null) { - println("not null") - 1 - } else { - 2 - } -// example 4 -if (myVar == null) { - println("null") -} else { - println("not null") -} -``` - -**Valid example:** -```kotlin -// example 1 -var myVar: Int? = null -myVar?: run { - println("null") - return -} - -// example 2 -myVar?.let { - println("not null") - return -} - -// example 3 -val anotherVal = myVar?.also { - println("not null") - 1 - } ?: 2 - -// example 4 -myVar?.let { - println("null") -} ?: run { println("not null") } ``` \ No newline at end of file