Skip to content

Commit 3e3740e

Browse files
DrAlexDDrAlexD
authored
Extracted REDUNDANT_SEMICOLON rule check from WRONG_NEWLINES rule (#1864)
### What's done: - `REDUNDANT_SEMICOLON` rule check moved to earlier checks to remove unnecessary semicolons before executing the rest of the rules. - Moved several warning and fix tests related to `REDUNDANT_SEMICOLON` rule. - Added smoke test for logic related to semicolons. - Added warning test for `LocalVariablesRule`. Closes #1863, #1783
1 parent 6cb0093 commit 3e3740e

File tree

18 files changed

+269
-66
lines changed

18 files changed

+269
-66
lines changed

‎diktat-cli/src/test/kotlin/com/saveourtool/diktat/smoke/DiktatSmokeTestBase.kt

+8
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import com.saveourtool.diktat.ruleset.rules.chapter2.kdoc.KdocComments
2727
import com.saveourtool.diktat.ruleset.rules.chapter2.kdoc.KdocFormatting
2828
import com.saveourtool.diktat.ruleset.rules.chapter2.kdoc.KdocMethods
2929
import com.saveourtool.diktat.ruleset.rules.chapter3.EmptyBlock
30+
import com.saveourtool.diktat.ruleset.rules.chapter3.files.SemicolonsRule
3031
import com.saveourtool.diktat.ruleset.rules.chapter6.classes.InlineClassesRule
3132
import com.saveourtool.diktat.ruleset.utils.indentation.IndentationConfig
3233
import com.saveourtool.diktat.ruleset.utils.indentation.IndentationConfig.Companion.EXTENDED_INDENT_AFTER_OPERATORS
@@ -369,6 +370,13 @@ abstract class DiktatSmokeTestBase {
369370
}
370371
}
371372

373+
@Test
374+
@Tag("DiktatRuleSetProvider")
375+
@Timeout(TEST_TIMEOUT_SECONDS, unit = SECONDS)
376+
fun `regression - should not fail if file has unnecessary semicolons`() {
377+
fixAndCompare(prepareOverriddenRulesConfig(), "SemicolonsExpected.kt", "SemicolonsTest.kt")
378+
}
379+
372380
abstract fun fixAndCompare(
373381
config: Path,
374382
expected: String,

‎diktat-cli/src/test/resources/test/smoke/src/main/kotlin/SemicolonsExpected.kt

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package com.saveourtool.diktat
2+
3+
import io.micrometer.core.instrument.MeterRegistry
4+
import io.micrometer.core.instrument.MultiGauge
5+
import io.micrometer.core.instrument.Tags
6+
import org.springframework.scheduling.annotation.Scheduled
7+
import org.springframework.stereotype.Component
8+
9+
@Component
10+
class ActiveBinsMetric(meterRegistry: MeterRegistry, private val binRepository: BinRepository) {
11+
private val metric = MultiGauge.builder(ACTIVE_BINS_METRIC_NAME)
12+
.register(meterRegistry)
13+
14+
@Scheduled(fixedDelay = DELAY)
15+
fun queryDb() {
16+
metric.register(
17+
binRepository
18+
.countActiveWithPartsNumber()
19+
.toRangeMap()
20+
.map {
21+
MultiGauge.Row.of(
22+
Tags.of(NUMBER_OF_EGGS_LABEL, it.key),
23+
it.value
24+
)
25+
}, true)
26+
}
27+
28+
private fun List<NumberOfBinsAndParts>.toRangeMap(): MutableMap<String, Long> {
29+
var total = 0L
30+
val map = mutableMapOf<String, Long>()
31+
numberOfEggsBuckets.forEach { map[it] = 0 }
32+
33+
this.forEach {
34+
total += it.numberOfBins
35+
when (it.numberOfParts) {
36+
1 -> map[EGG_1_BUCKET_LABEL] = it.numberOfBins
37+
2 -> map[EGG_2_BUCKET_LABEL] = it.numberOfBins
38+
3 -> map[EGG_3_BUCKET_LABEL] = it.numberOfBins
39+
in 4..5 -> map[EGG_4_5_BUCKET_LABEL] = it.numberOfBins
40+
in 7..9 -> map[EGG_7_9_BUCKET_LABEL] = it.numberOfBins
41+
in 10..Int.MAX_VALUE -> map[EGG_OVER_10_BUCKET_LABEL] = it.numberOfBins
42+
}
43+
}
44+
45+
map[ALL_ACTIVE_BINS_LABEL] = total
46+
return map
47+
}
48+
49+
companion object {
50+
private const val ACTIVE_BINS_METRIC_NAME = "c.concurrent.bins"
51+
private const val ALL_ACTIVE_BINS_LABEL = "total"
52+
private const val EGG_1_BUCKET_LABEL = "1"
53+
private const val EGG_2_BUCKET_LABEL = "2"
54+
private const val EGG_3_BUCKET_LABEL = "3"
55+
private const val EGG_4_5_BUCKET_LABEL = "4-5"
56+
private const val EGG_7_9_BUCKET_LABEL = "7-9"
57+
private const val EGG_OVER_10_BUCKET_LABEL = "10+"
58+
private const val NUMBER_OF_EGGS_LABEL = "numberOfEggs"
59+
private val numberOfEggsBuckets = setOf(
60+
EGG_1_BUCKET_LABEL,
61+
EGG_2_BUCKET_LABEL,
62+
EGG_3_BUCKET_LABEL,
63+
EGG_4_5_BUCKET_LABEL,
64+
EGG_7_9_BUCKET_LABEL,
65+
EGG_OVER_10_BUCKET_LABEL,
66+
ALL_ACTIVE_BINS_LABEL)
67+
}
68+
}

‎diktat-cli/src/test/resources/test/smoke/src/main/kotlin/SemicolonsTest.kt

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import io.micrometer.core.instrument.MeterRegistry
2+
import io.micrometer.core.instrument.MultiGauge
3+
import io.micrometer.core.instrument.Tags;
4+
import org.springframework.scheduling.annotation.Scheduled;
5+
import org.springframework.stereotype.Component;
6+
7+
@Component
8+
class ActiveBinsMetric(meterRegistry: MeterRegistry, private val binRepository: BinRepository) {
9+
10+
companion object {
11+
private const val ACTIVE_BINS_METRIC_NAME = "c.concurrent.bins";
12+
private const val NUMBER_OF_EGGS_LABEL = "numberOfEggs";
13+
private const val ALL_ACTIVE_BINS_LABEL = "total";
14+
private const val EGG_1_BUCKET_LABEL = "1";
15+
private const val EGG_2_BUCKET_LABEL = "2";
16+
private const val EGG_3_BUCKET_LABEL = "3";
17+
private const val EGG_4_5_BUCKET_LABEL = "4-5";
18+
private const val EGG_7_9_BUCKET_LABEL = "7-9";
19+
private const val EGG_OVER_10_BUCKET_LABEL = "10+";
20+
private val numberOfEggsBuckets = setOf(
21+
EGG_1_BUCKET_LABEL,
22+
EGG_2_BUCKET_LABEL,
23+
EGG_3_BUCKET_LABEL,
24+
EGG_4_5_BUCKET_LABEL,
25+
EGG_7_9_BUCKET_LABEL,
26+
EGG_OVER_10_BUCKET_LABEL,
27+
ALL_ACTIVE_BINS_LABEL);
28+
29+
}
30+
31+
private val metric = MultiGauge.builder(ACTIVE_BINS_METRIC_NAME)
32+
.register(meterRegistry);
33+
34+
@Scheduled(fixedDelay = DELAY)
35+
fun queryDB() {
36+
metric.register(
37+
binRepository
38+
.countActiveWithPartsNumber()
39+
.toRangeMap()
40+
.map {
41+
MultiGauge.Row.of(
42+
Tags.of(NUMBER_OF_EGGS_LABEL, it.key),
43+
it.value
44+
)
45+
}, true);
46+
}
47+
48+
private fun List<NumberOfBinsAndParts>.toRangeMap(): MutableMap<String, Long> {
49+
var total = 0L;
50+
val map = mutableMapOf<String, Long>();
51+
numberOfEggsBuckets.forEach { map[it] = 0 };
52+
53+
this.forEach {
54+
total += it.numberOfBins
55+
when (it.numberOfParts) {
56+
1 -> map[EGG_1_BUCKET_LABEL] = it.numberOfBins
57+
2 -> map[EGG_2_BUCKET_LABEL] = it.numberOfBins
58+
3 -> map[EGG_3_BUCKET_LABEL] = it.numberOfBins
59+
in 4..5 -> map[EGG_4_5_BUCKET_LABEL] = it.numberOfBins
60+
in 7..9 -> map[EGG_7_9_BUCKET_LABEL] = it.numberOfBins
61+
in 10..Int.MAX_VALUE -> map[EGG_OVER_10_BUCKET_LABEL] = it.numberOfBins
62+
};
63+
};
64+
65+
map[ALL_ACTIVE_BINS_LABEL] = total;
66+
return map;
67+
}
68+
69+
70+
}

‎diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/DiktatRuleSetFactoryImpl.kt

+3-1
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import com.saveourtool.diktat.ruleset.rules.chapter3.files.FileSize
4040
import com.saveourtool.diktat.ruleset.rules.chapter3.files.FileStructureRule
4141
import com.saveourtool.diktat.ruleset.rules.chapter3.files.IndentationRule
4242
import com.saveourtool.diktat.ruleset.rules.chapter3.files.NewlinesRule
43+
import com.saveourtool.diktat.ruleset.rules.chapter3.files.SemicolonsRule
4344
import com.saveourtool.diktat.ruleset.rules.chapter3.files.TopLevelOrderRule
4445
import com.saveourtool.diktat.ruleset.rules.chapter3.files.WhiteSpaceRule
4546
import com.saveourtool.diktat.ruleset.rules.chapter3.identifiers.LocalVariablesRule
@@ -114,7 +115,8 @@ class DiktatRuleSetFactoryImpl : DiktatRuleSetFactory {
114115
// We don't have a way to enforce a specific order, so we should just be careful when adding new rules to this list and, when possible,
115116
// cover new rules in smoke test as well. If a rule needs to be at a specific position in a list, please add comment explaining it (like for NewlinesRule).
116117
val rules = sequenceOf(
117-
// comments & documentation
118+
// semicolons, comments, documentation
119+
::SemicolonsRule,
118120
::CommentsRule,
119121
::SingleConstructorRule, // this rule can add properties to a primary constructor, so should be before KdocComments
120122
::KdocComments,

‎diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/files/NewlinesRule.kt

+11-30
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,10 @@ import com.saveourtool.diktat.common.config.rules.RuleConfiguration
44
import com.saveourtool.diktat.common.config.rules.RulesConfig
55
import com.saveourtool.diktat.common.config.rules.getRuleConfig
66
import com.saveourtool.diktat.ruleset.constants.Warnings.COMPLEX_EXPRESSION
7-
import com.saveourtool.diktat.ruleset.constants.Warnings.REDUNDANT_SEMICOLON
87
import com.saveourtool.diktat.ruleset.constants.Warnings.WRONG_NEWLINES
98
import com.saveourtool.diktat.ruleset.rules.DiktatRule
109
import com.saveourtool.diktat.ruleset.utils.appendNewlineMergingWhiteSpace
1110
import com.saveourtool.diktat.ruleset.utils.emptyBlockList
12-
import com.saveourtool.diktat.ruleset.utils.extractLineOfText
1311
import com.saveourtool.diktat.ruleset.utils.findAllDescendantsWithSpecificType
1412
import com.saveourtool.diktat.ruleset.utils.findAllNodesWithCondition
1513
import com.saveourtool.diktat.ruleset.utils.findParentNodeWithSpecificType
@@ -19,7 +17,6 @@ import com.saveourtool.diktat.ruleset.utils.getIdentifierName
1917
import com.saveourtool.diktat.ruleset.utils.getRootNode
2018
import com.saveourtool.diktat.ruleset.utils.hasParent
2119
import com.saveourtool.diktat.ruleset.utils.isBeginByNewline
22-
import com.saveourtool.diktat.ruleset.utils.isEol
2320
import com.saveourtool.diktat.ruleset.utils.isFollowedByNewline
2421
import com.saveourtool.diktat.ruleset.utils.isGradleScript
2522
import com.saveourtool.diktat.ruleset.utils.isSingleLineIfElse
@@ -37,7 +34,6 @@ import org.jetbrains.kotlin.KtNodeTypes.CALL_EXPRESSION
3734
import org.jetbrains.kotlin.KtNodeTypes.CLASS
3835
import org.jetbrains.kotlin.KtNodeTypes.CONDITION
3936
import org.jetbrains.kotlin.KtNodeTypes.DOT_QUALIFIED_EXPRESSION
40-
import org.jetbrains.kotlin.KtNodeTypes.ENUM_ENTRY
4137
import org.jetbrains.kotlin.KtNodeTypes.FUN
4238
import org.jetbrains.kotlin.KtNodeTypes.FUNCTION_LITERAL
4339
import org.jetbrains.kotlin.KtNodeTypes.FUNCTION_TYPE
@@ -90,7 +86,6 @@ import org.jetbrains.kotlin.lexer.KtTokens.PLUSEQ
9086
import org.jetbrains.kotlin.lexer.KtTokens.RETURN_KEYWORD
9187
import org.jetbrains.kotlin.lexer.KtTokens.RPAR
9288
import org.jetbrains.kotlin.lexer.KtTokens.SAFE_ACCESS
93-
import org.jetbrains.kotlin.lexer.KtTokens.SEMICOLON
9489
import org.jetbrains.kotlin.lexer.KtTokens.WHITE_SPACE
9590
import org.jetbrains.kotlin.psi.KtBinaryExpression
9691
import org.jetbrains.kotlin.psi.KtNamedFunction
@@ -106,31 +101,29 @@ import org.jetbrains.kotlin.psi.psiUtil.siblings
106101
private typealias ListOfList = MutableList<MutableList<ASTNode>>
107102

108103
/**
109-
* Rule that checks line break styles.
110-
* 1. Prohibits usage of semicolons at the end of line
111-
* 2. Checks that some operators are followed by newline, while others are prepended by it
112-
* 3. Statements that follow `!!` behave in the same way
113-
* 4. Forces functional style of chained dot call expressions with exception
114-
* 5. Checks that newline is placed after assignment operator, not before
115-
* 6. Ensures that function or constructor name isn't separated from `(` by space or newline
116-
* 7. Ensures that in multiline lambda newline follows arrow or, in case of lambda without explicit parameters, opening brace
117-
* 8. Checks that functions with single `return` are simplified to functions with expression body
118-
* 9. parameter or argument lists and supertype lists that have more than 2 elements should be separated by newlines
119-
* 10. Complex expression inside condition replaced with new variable
104+
* Rule that checks line break styles:
105+
* 1. Checks that some operators are followed by newline, while others are prepended by it
106+
* 2. Statements that follow `!!` behave in the same way
107+
* 3. Forces functional style of chained dot call expressions with exception
108+
* 4. Checks that newline is placed after assignment operator, not before
109+
* 5. Ensures that function or constructor name isn't separated from `(` by space or newline
110+
* 6. Ensures that in multiline lambda newline follows arrow or, in case of lambda without explicit parameters, opening brace
111+
* 7. Checks that functions with single `return` are simplified to functions with expression body
112+
* 8. Parameter or argument lists and supertype lists that have more than 2 elements should be separated by newlines
113+
* 9. Complex expression inside condition replaced with new variable
120114
*/
121115
@Suppress("ForbiddenComment")
122116
class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
123117
NAME_ID,
124118
configRules,
125-
listOf(COMPLEX_EXPRESSION, REDUNDANT_SEMICOLON, WRONG_NEWLINES)
119+
listOf(COMPLEX_EXPRESSION, WRONG_NEWLINES)
126120
) {
127121
private val configuration by lazy {
128122
NewlinesRuleConfiguration(configRules.getRuleConfig(WRONG_NEWLINES)?.configuration ?: emptyMap())
129123
}
130124

131125
override fun logic(node: ASTNode) {
132126
when (node.elementType) {
133-
SEMICOLON -> handleSemicolon(node)
134127
OPERATION_REFERENCE, EQ -> handleOperatorWithLineBreakAfter(node)
135128
// this logic regulates indentation with elements - so that the symbol and the subsequent expression are on the same line
136129
in lineBreakBeforeOperators -> handleOperatorWithLineBreakBefore(node)
@@ -206,18 +199,6 @@ class NewlinesRule(configRules: List<RulesConfig>) : DiktatRule(
206199
private fun isDotQuaOrSafeAccessOrPostfixExpression(node: ASTNode) =
207200
node.elementType == DOT_QUALIFIED_EXPRESSION || node.elementType == SAFE_ACCESS_EXPRESSION || node.elementType == POSTFIX_EXPRESSION
208201

209-
/**
210-
* Check that EOL semicolon is used only in enums
211-
*/
212-
private fun handleSemicolon(node: ASTNode) {
213-
if (node.isEol() && node.treeParent.elementType != ENUM_ENTRY) {
214-
// semicolon at the end of line which is not part of enum members declarations
215-
REDUNDANT_SEMICOLON.warnAndFix(configRules, emitWarn, isFixMode, node.extractLineOfText(), node.startOffset, node) {
216-
node.treeParent.removeChild(node)
217-
}
218-
}
219-
}
220-
221202
private fun handleOperatorWithLineBreakAfter(node: ASTNode) {
222203
// [node] should be either EQ or OPERATION_REFERENCE which has single child
223204
if (node.elementType != EQ && node.firstChildNode.elementType !in lineBreakAfterOperators && !node.isInfixCall()) {

‎diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/files/SemicolonsRule.kt

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package com.saveourtool.diktat.ruleset.rules.chapter3.files
2+
3+
import com.saveourtool.diktat.common.config.rules.RulesConfig
4+
import com.saveourtool.diktat.ruleset.constants.Warnings.REDUNDANT_SEMICOLON
5+
import com.saveourtool.diktat.ruleset.rules.DiktatRule
6+
import com.saveourtool.diktat.ruleset.utils.extractLineOfText
7+
import com.saveourtool.diktat.ruleset.utils.isEol
8+
import org.jetbrains.kotlin.KtNodeTypes.ENUM_ENTRY
9+
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
10+
import org.jetbrains.kotlin.lexer.KtTokens.SEMICOLON
11+
12+
/**
13+
* Rule that checks usage of semicolons at the end of line
14+
*/
15+
@Suppress("ForbiddenComment")
16+
class SemicolonsRule(configRules: List<RulesConfig>) : DiktatRule(
17+
NAME_ID,
18+
configRules,
19+
listOf(REDUNDANT_SEMICOLON)
20+
) {
21+
override fun logic(node: ASTNode) {
22+
if (node.elementType == SEMICOLON) {
23+
handleSemicolon(node)
24+
}
25+
}
26+
27+
/**
28+
* Check that EOL semicolon is used only in enums
29+
*/
30+
private fun handleSemicolon(node: ASTNode) {
31+
if (node.isEol() && node.treeParent.elementType != ENUM_ENTRY) {
32+
// semicolon at the end of line which is not part of enum members declarations
33+
REDUNDANT_SEMICOLON.warnAndFix(configRules, emitWarn, isFixMode, node.extractLineOfText(), node.startOffset, node) {
34+
node.treeParent.removeChild(node)
35+
}
36+
}
37+
}
38+
39+
companion object {
40+
const val NAME_ID = "semicolon"
41+
}
42+
}

‎diktat-rules/src/main/kotlin/com/saveourtool/diktat/ruleset/rules/chapter3/identifiers/LocalVariablesRule.kt

+2-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import com.saveourtool.diktat.ruleset.utils.search.findAllVariablesWithUsages
1414
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
1515
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
1616
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
17+
import org.jetbrains.kotlin.lexer.KtTokens.SEMICOLON
1718
import org.jetbrains.kotlin.lexer.KtTokens.WHITE_SPACE
1819
import org.jetbrains.kotlin.psi.KtBlockExpression
1920
import org.jetbrains.kotlin.psi.KtCallExpression
@@ -160,7 +161,7 @@ class LocalVariablesRule(configRules: List<RulesConfig>) : DiktatRule(
160161
) {
161162
val numLinesToSkip = property
162163
.siblings(forward = true, withItself = false)
163-
.takeWhile { it is PsiWhiteSpace || it.node.isPartOfComment() }
164+
.takeWhile { it is PsiWhiteSpace || it.node.elementType == SEMICOLON || it.node.isPartOfComment() }
164165
.let { siblings ->
165166
siblings
166167
.last()

‎diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/LocalVariablesWarnTest.kt

+13
Original file line numberDiff line numberDiff line change
@@ -686,4 +686,17 @@ class LocalVariablesWarnTest : LintTestBase(::LocalVariablesRule) {
686686
""".trimMargin()
687687
)
688688
}
689+
690+
@Test
691+
@Tag(LOCAL_VARIABLE_EARLY_DECLARATION)
692+
fun `shouldn't fail on semicolon`() {
693+
lintMethod(
694+
"""
695+
|fun bar() {
696+
| var a = 0;
697+
| a++
698+
|}
699+
""".trimMargin()
700+
)
701+
}
689702
}

‎diktat-rules/src/test/kotlin/com/saveourtool/diktat/ruleset/chapter3/files/NewlinesRuleFixTest.kt

+2-8
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,6 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu
1515
mapOf("maxCallsInOneLine" to "1"))
1616
)
1717

18-
@Test
19-
@Tag(WarningNames.REDUNDANT_SEMICOLON)
20-
fun `should remove redundant semicolons`() {
21-
fixAndCompare("SemicolonsExpected.kt", "SemicolonsTest.kt")
22-
}
23-
2418
@Test
2519
@Tag(WarningNames.WRONG_NEWLINES)
2620
fun `should fix newlines near operators`() {
@@ -54,7 +48,7 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu
5448
@Test
5549
@Tag(WarningNames.WRONG_NEWLINES)
5650
fun `One line parameters list sheet must contain no more than 2 parameters`() {
57-
fixAndCompare("sizeParameterListExpected.kt", "sizeParameterListTest.kt")
51+
fixAndCompare("SizeParameterListExpected.kt", "SizeParameterListTest.kt")
5852
}
5953

6054
@Test
@@ -77,7 +71,7 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu
7771

7872
@Test
7973
@Tag(WarningNames.WRONG_NEWLINES)
80-
fun `should fix one line function with and without semicolon`() {
74+
fun `should fix one line function`() {
8175
fixAndCompare("OneLineFunctionExpected.kt", "OneLineFunctionTest.kt")
8276
}
8377

0 commit comments

Comments
 (0)