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

New rule to check if @Preview (Jetpack Compose) functions end with 'Preview' suffix and are also private. Part 1 #1726

Merged
merged 6 commits into from
Sep 1, 2023
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
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,9 @@
# Checks that annotation is on a single line
- name: ANNOTATION_NEW_LINE
enabled: true
# Checks that method annotated with `Preview` annotation is private and has Preview suffix
- name: PREVIEW_ANNOTATION
enabled: true
# Checks that enum structure is correct: enum entries should be separated by comma and line break and last entry should have semicolon in the end.
- name: ENUMS_SEPARATED
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ enum class Warnings(
WRONG_WHITESPACE(true, "3.8.1", "incorrect usage of whitespaces for code separation"),
TOO_MANY_CONSECUTIVE_SPACES(true, "3.8.1", "too many consecutive spaces"),
ANNOTATION_NEW_LINE(true, "3.12.1", "annotations must be on new line"),
PREVIEW_ANNOTATION(false, "3.12.2", "method, annotated with `@Preview` annotation should be private and has `Preview` suffix"),
ENUMS_SEPARATED(true, "3.9.1", "enum is incorrectly formatted"),
WHEN_WITHOUT_ELSE(true, "3.11.1", "each 'when' statement must have else at the end"),
LONG_NUMERICAL_VALUES_SEPARATED(true, "3.14.2", "long numerical values should be separated with underscore"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.saveourtool.diktat.ruleset.rules.chapter3.LongNumericalValuesSeparate
import com.saveourtool.diktat.ruleset.rules.chapter3.MagicNumberRule
import com.saveourtool.diktat.ruleset.rules.chapter3.MultipleModifiersSequence
import com.saveourtool.diktat.ruleset.rules.chapter3.NullableTypeRule
import com.saveourtool.diktat.ruleset.rules.chapter3.PreviewAnnotationRule
import com.saveourtool.diktat.ruleset.rules.chapter3.RangeConventionalRule
import com.saveourtool.diktat.ruleset.rules.chapter3.SingleLineStatementsRule
import com.saveourtool.diktat.ruleset.rules.chapter3.SortRule
Expand Down Expand Up @@ -159,6 +160,7 @@ class DiktatRuleSetFactoryImpl : DiktatRuleSetFactory {
::LongNumericalValuesSeparatedRule,
::NestedFunctionBlock,
::AnnotationNewLineRule,
::PreviewAnnotationRule,
::SortRule,
::EnumsSeparated,
::StringConcatenationRule,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package com.saveourtool.diktat.ruleset.rules.chapter3

import com.saveourtool.diktat.common.config.rules.RulesConfig
import com.saveourtool.diktat.ruleset.constants.Warnings.PREVIEW_ANNOTATION
import com.saveourtool.diktat.ruleset.rules.DiktatRule
import com.saveourtool.diktat.ruleset.utils.getAllChildrenWithType
import com.saveourtool.diktat.ruleset.utils.getIdentifierName

import org.jetbrains.kotlin.KtNodeTypes.ANNOTATION_ENTRY
import org.jetbrains.kotlin.KtNodeTypes.FUN
import org.jetbrains.kotlin.KtNodeTypes.MODIFIER_LIST
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.psiUtil.isPrivate

/**
* This rule checks, whether the method has `@Preview` annotation (Jetpack Compose)
* If so, such method should be private and function name should end with `Preview` suffix
*/
class PreviewAnnotationRule(configRules: List<RulesConfig>) : DiktatRule(
NAME_ID,
configRules,
listOf(PREVIEW_ANNOTATION)
) {
override fun logic(node: ASTNode) {
if (node.elementType == FUN) {
checkFunctionSignature(node)
}
}

private fun checkFunctionSignature(node: ASTNode) {
node.findChildByType(MODIFIER_LIST)?.let { modList ->
doCheck(node, modList)
}
}

private fun doCheck(functionNode: ASTNode, modeList: ASTNode) {
if (modeList.getAllChildrenWithType(ANNOTATION_ENTRY).isEmpty()) {
return

Check warning on line 39 in diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt

View check run for this annotation

Codecov / codecov/patch

diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt#L39

Added line #L39 was not covered by tests
}

val previewAnnotationNode = modeList.getAllChildrenWithType(ANNOTATION_ENTRY).firstOrNull {
it.text.contains("$ANNOTATION_SYMBOL$PREVIEW_ANNOTATION_TEXT")
}

previewAnnotationNode?.let {
val functionName = functionNode.getIdentifierName()?.text ?: return

// warn if function is not private
if (!((functionNode.psi as KtNamedFunction).isPrivate())) {
PREVIEW_ANNOTATION.warnAndFix(
configRules,
emitWarn,
isFixMode,
"$functionName method should be private",
functionNode.startOffset,
functionNode
) {
// provide fix

Check warning on line 59 in diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt

View check run for this annotation

Codecov / codecov/patch

diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt#L59

Added line #L59 was not covered by tests
}
}

// warn if function has no `Preview` suffix
if (!isMethodHasPreviewSuffix(functionName)) {
PREVIEW_ANNOTATION.warnAndFix(
configRules,
emitWarn,
isFixMode,
"$functionName method should has `Preview` suffix",
functionNode.startOffset,
functionNode
) {
// provide fix

Check warning on line 73 in diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt

View check run for this annotation

Codecov / codecov/patch

diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/PreviewAnnotationRule.kt#L73

Added line #L73 was not covered by tests
kgevorkyan marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

private fun isMethodHasPreviewSuffix(functionName: String) =
functionName.contains(PREVIEW_ANNOTATION_TEXT)

companion object {
const val ANNOTATION_SYMBOL = "@"
const val NAME_ID = "preview-annotation"
const val PREVIEW_ANNOTATION_TEXT = "Preview"
}
}
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis-huawei.yml
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,9 @@
# Checks that annotation is on a single line
- name: ANNOTATION_NEW_LINE
enabled: true
# Checks that method annotated with `Preview` annotation is private and has Preview suffix
- name: PREVIEW_ANNOTATION
enabled: true
# Checks that enum structure is correct: enum entries should be separated by comma and line break and last entry should have semicolon in the end.
- name: ENUMS_SEPARATED
enabled: true
Expand Down
3 changes: 3 additions & 0 deletions diktat-rules/src/main/resources/diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,9 @@
# Checks that annotation is on a single line
- name: ANNOTATION_NEW_LINE
enabled: true
# Checks that method annotated with `Preview` annotation is private and has Preview suffix
- name: PREVIEW_ANNOTATION
enabled: true
# Checks that enum structure is correct: enum entries should be separated by comma and line break and last entry should have semicolon in the end.
- name: ENUMS_SEPARATED
enabled: true
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package com.saveourtool.diktat.ruleset.chapter3

import com.saveourtool.diktat.api.DiktatError
import com.saveourtool.diktat.common.config.rules.DIKTAT_RULE_SET_ID
import com.saveourtool.diktat.ruleset.constants.Warnings
import com.saveourtool.diktat.ruleset.rules.chapter3.PreviewAnnotationRule
import com.saveourtool.diktat.util.LintTestBase
import generated.WarningNames
import org.junit.jupiter.api.Tag
import org.junit.jupiter.api.Test


class PreviewAnnotationWarnTest : LintTestBase(::PreviewAnnotationRule) {
private val ruleId = "$DIKTAT_RULE_SET_ID:${PreviewAnnotationRule.NAME_ID}"

@Test
@Tag(WarningNames.PREVIEW_ANNOTATION)
fun `no warn`() {
lintMethod(
"""
|@Preview
|@Composable
|private fun BannerPreview() {}
""".trimMargin()
)
}

@Test
@Tag(WarningNames.PREVIEW_ANNOTATION)
fun `method is not private`() {
lintMethod(
"""
|@Preview
|@Composable
|fun BannerPreview() {}
""".trimMargin(),
DiktatError(1, 1, ruleId, "${Warnings.PREVIEW_ANNOTATION.warnText()} BannerPreview method should be private", false),
)
}

@Test
@Tag(WarningNames.PREVIEW_ANNOTATION)
fun `method has no preview suffix`() {
lintMethod(
"""
|@Preview
|@Composable
|private fun Banner() {}
""".trimMargin(),
DiktatError(1, 1, ruleId, "${Warnings.PREVIEW_ANNOTATION.warnText()} Banner method should has `Preview` suffix", false),
)
}

@Test
@Tag(WarningNames.PREVIEW_ANNOTATION)
fun `method has no preview suffix and is not private`() {
lintMethod(
"""
|@Preview
|@Composable
|fun Banner() {}
""".trimMargin(),
DiktatError(1, 1, ruleId, "${Warnings.PREVIEW_ANNOTATION.warnText()} Banner method should be private", false),
DiktatError(1, 1, ruleId, "${Warnings.PREVIEW_ANNOTATION.warnText()} Banner method should has `Preview` suffix", false),
)
}
}
1 change: 1 addition & 0 deletions info/available-rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
| 3 | 3.10.2 | LOCAL_VARIABLE_EARLY_DECLARATION | Check: warns if a local variable is declared not immediately before its usage.<br>Fix (not implemented yet): moves the variable declaration. | no | no | add auto fix |
| 3 | 3.11.1 | WHEN_WITHOUT_ELSE | Check: warns if a `when` statement does not have `else` in the end.<br>Fix: adds `else` when a statement doesn't have it. | yes | no | - | If a `when` statement of the enum or sealed type contains all values of the enum, there is no need to have the "else" branch. |
| 3 | 3.12.1 | ANNOTATION_NEW_LINE | Check: warns if an annotation is not on a new single line. | yes | no | - |
| 3 | 3.12.2 | PREVIEW_ANNOTATION | Check: warns if method, annotated with `@Preview` is not private or has no `Preview` suffix. | yes | no | - |
| 3 | 3.14.1 | WRONG_MULTIPLE_MODIFIERS_ORDER | Check: warns if the multiple modifiers in the sequence are in the wrong order. Value identifier supported in Kotlin 1.5 | yes | no | - |
| 3 | 3.14.2 | LONG_NUMERICAL_VALUES_SEPARATED | Check: warns if the value of the integer or float constant is too big. | no | maxNumberLength maxBlockLength | - |
| 3 | 3.14.3 | MAGIC_NUMBER | Check: warns if there are magic numbers in the code. | no | ignoreNumbers, ignoreHashCodeFunction, ignorePropertyDeclaration, ignoreLocalVariableDeclaration, ignoreConstantDeclaration, ignoreCompanionObjectPropertyDeclaration, ignoreEnums, ignoreRanges, ignoreExtensionFunctions | no |
Expand Down
13 changes: 12 additions & 1 deletion info/guide/guide-chapter-3.md
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,8 @@ The compiler can issue a warning when it is missing.

<!-- =============================================================================== -->
### <a name="c3.12"></a> 3.12 Annotations

This section contains recommendations regarding annotations.
#### <a name="r3.12.1"></a> 3.12.1 Whitespaces and newlines for annotations
Each annotation applied to a class, method or constructor should be placed on its own line. Consider the following examples:
1. Annotations applied to the class, method or constructor are placed on separate lines (one annotation per line).

Expand All @@ -988,6 +989,16 @@ fun getNameIfPresent() { /* ... */ }
```kotlin
@MustBeDocumented @CustomAnnotation val loader: DataLoader
```
#### <a name="r3.12.2"></a> 3.12.2 Preview annotation
`@Preview` (Jetpack Compose) functions should end with 'Preview' suffix and are also be private

**Valid example**:
```kotlin
@Preview
@Composable
private fun BannerPreview() {}
```


<!-- =============================================================================== -->
### <a name="c3.13"></a> 3.13 Block comments
Expand Down
Loading