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

Extracted REDUNDANT_SEMICOLON rule check from WRONG_NEWLINES rule #1864

Merged
merged 4 commits into from
Dec 15, 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
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import com.saveourtool.diktat.ruleset.rules.chapter2.kdoc.KdocComments
import com.saveourtool.diktat.ruleset.rules.chapter2.kdoc.KdocFormatting
import com.saveourtool.diktat.ruleset.rules.chapter2.kdoc.KdocMethods
import com.saveourtool.diktat.ruleset.rules.chapter3.EmptyBlock
import com.saveourtool.diktat.ruleset.rules.chapter3.files.SemicolonsRule
import com.saveourtool.diktat.ruleset.rules.chapter6.classes.InlineClassesRule
import com.saveourtool.diktat.ruleset.utils.indentation.IndentationConfig
import com.saveourtool.diktat.ruleset.utils.indentation.IndentationConfig.Companion.EXTENDED_INDENT_AFTER_OPERATORS
Expand Down Expand Up @@ -369,6 +370,13 @@ abstract class DiktatSmokeTestBase {
}
}

@Test
@Tag("DiktatRuleSetProvider")
@Timeout(TEST_TIMEOUT_SECONDS, unit = SECONDS)
fun `regression - should not fail if file has unnecessary semicolons`() {
fixAndCompare(prepareOverriddenRulesConfig(), "SemicolonsExpected.kt", "SemicolonsTest.kt")
}

abstract fun fixAndCompare(
config: Path,
expected: String,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package com.saveourtool.diktat

import io.micrometer.core.instrument.MeterRegistry
import io.micrometer.core.instrument.MultiGauge
import io.micrometer.core.instrument.Tags
import org.springframework.scheduling.annotation.Scheduled
import org.springframework.stereotype.Component

@Component
class ActiveBinsMetric(meterRegistry: MeterRegistry, private val binRepository: BinRepository) {
private val metric = MultiGauge.builder(ACTIVE_BINS_METRIC_NAME)
.register(meterRegistry)

@Scheduled(fixedDelay = DELAY)
fun queryDb() {
metric.register(
binRepository
.countActiveWithPartsNumber()
.toRangeMap()
.map {
MultiGauge.Row.of(
Tags.of(NUMBER_OF_EGGS_LABEL, it.key),
it.value
)
}, true)
}

private fun List<NumberOfBinsAndParts>.toRangeMap(): MutableMap<String, Long> {
var total = 0L
val map = mutableMapOf<String, Long>()
numberOfEggsBuckets.forEach { map[it] = 0 }

this.forEach {
total += it.numberOfBins
when (it.numberOfParts) {
1 -> map[EGG_1_BUCKET_LABEL] = it.numberOfBins
2 -> map[EGG_2_BUCKET_LABEL] = it.numberOfBins
3 -> map[EGG_3_BUCKET_LABEL] = it.numberOfBins
in 4..5 -> map[EGG_4_5_BUCKET_LABEL] = it.numberOfBins
in 7..9 -> map[EGG_7_9_BUCKET_LABEL] = it.numberOfBins
in 10..Int.MAX_VALUE -> map[EGG_OVER_10_BUCKET_LABEL] = it.numberOfBins
}
}

map[ALL_ACTIVE_BINS_LABEL] = total
return map
}

companion object {
private const val ACTIVE_BINS_METRIC_NAME = "c.concurrent.bins"
private const val ALL_ACTIVE_BINS_LABEL = "total"
private const val EGG_1_BUCKET_LABEL = "1"
private const val EGG_2_BUCKET_LABEL = "2"
private const val EGG_3_BUCKET_LABEL = "3"
private const val EGG_4_5_BUCKET_LABEL = "4-5"
private const val EGG_7_9_BUCKET_LABEL = "7-9"
private const val EGG_OVER_10_BUCKET_LABEL = "10+"
private const val NUMBER_OF_EGGS_LABEL = "numberOfEggs"
private val numberOfEggsBuckets = setOf(
EGG_1_BUCKET_LABEL,
EGG_2_BUCKET_LABEL,
EGG_3_BUCKET_LABEL,
EGG_4_5_BUCKET_LABEL,
EGG_7_9_BUCKET_LABEL,
EGG_OVER_10_BUCKET_LABEL,
ALL_ACTIVE_BINS_LABEL)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import io.micrometer.core.instrument.MeterRegistry
import io.micrometer.core.instrument.MultiGauge
import io.micrometer.core.instrument.Tags;
import org.springframework.scheduling.annotation.Scheduled;
import org.springframework.stereotype.Component;

@Component
class ActiveBinsMetric(meterRegistry: MeterRegistry, private val binRepository: BinRepository) {

companion object {
private const val ACTIVE_BINS_METRIC_NAME = "c.concurrent.bins";
private const val NUMBER_OF_EGGS_LABEL = "numberOfEggs";
private const val ALL_ACTIVE_BINS_LABEL = "total";
private const val EGG_1_BUCKET_LABEL = "1";
private const val EGG_2_BUCKET_LABEL = "2";
private const val EGG_3_BUCKET_LABEL = "3";
private const val EGG_4_5_BUCKET_LABEL = "4-5";
private const val EGG_7_9_BUCKET_LABEL = "7-9";
private const val EGG_OVER_10_BUCKET_LABEL = "10+";
private val numberOfEggsBuckets = setOf(
EGG_1_BUCKET_LABEL,
EGG_2_BUCKET_LABEL,
EGG_3_BUCKET_LABEL,
EGG_4_5_BUCKET_LABEL,
EGG_7_9_BUCKET_LABEL,
EGG_OVER_10_BUCKET_LABEL,
ALL_ACTIVE_BINS_LABEL);

}

private val metric = MultiGauge.builder(ACTIVE_BINS_METRIC_NAME)
.register(meterRegistry);

@Scheduled(fixedDelay = DELAY)
fun queryDB() {
metric.register(
binRepository
.countActiveWithPartsNumber()
.toRangeMap()
.map {
MultiGauge.Row.of(
Tags.of(NUMBER_OF_EGGS_LABEL, it.key),
it.value
)
}, true);
}

private fun List<NumberOfBinsAndParts>.toRangeMap(): MutableMap<String, Long> {
var total = 0L;
val map = mutableMapOf<String, Long>();
numberOfEggsBuckets.forEach { map[it] = 0 };

this.forEach {
total += it.numberOfBins
when (it.numberOfParts) {
1 -> map[EGG_1_BUCKET_LABEL] = it.numberOfBins
2 -> map[EGG_2_BUCKET_LABEL] = it.numberOfBins
3 -> map[EGG_3_BUCKET_LABEL] = it.numberOfBins
in 4..5 -> map[EGG_4_5_BUCKET_LABEL] = it.numberOfBins
in 7..9 -> map[EGG_7_9_BUCKET_LABEL] = it.numberOfBins
in 10..Int.MAX_VALUE -> map[EGG_OVER_10_BUCKET_LABEL] = it.numberOfBins
};
};

map[ALL_ACTIVE_BINS_LABEL] = total;
return map;
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import com.saveourtool.diktat.ruleset.rules.chapter3.files.FileSize
import com.saveourtool.diktat.ruleset.rules.chapter3.files.FileStructureRule
import com.saveourtool.diktat.ruleset.rules.chapter3.files.IndentationRule
import com.saveourtool.diktat.ruleset.rules.chapter3.files.NewlinesRule
import com.saveourtool.diktat.ruleset.rules.chapter3.files.SemicolonsRule
import com.saveourtool.diktat.ruleset.rules.chapter3.files.TopLevelOrderRule
import com.saveourtool.diktat.ruleset.rules.chapter3.files.WhiteSpaceRule
import com.saveourtool.diktat.ruleset.rules.chapter3.identifiers.LocalVariablesRule
Expand Down Expand Up @@ -114,7 +115,8 @@ class DiktatRuleSetFactoryImpl : DiktatRuleSetFactory {
// 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,
// 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).
val rules = sequenceOf(
// comments & documentation
// semicolons, comments, documentation
::SemicolonsRule,
::CommentsRule,
::SingleConstructorRule, // this rule can add properties to a primary constructor, so should be before KdocComments
::KdocComments,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ import com.saveourtool.diktat.common.config.rules.RuleConfiguration
import com.saveourtool.diktat.common.config.rules.RulesConfig
import com.saveourtool.diktat.common.config.rules.getRuleConfig
import com.saveourtool.diktat.ruleset.constants.Warnings.COMPLEX_EXPRESSION
import com.saveourtool.diktat.ruleset.constants.Warnings.REDUNDANT_SEMICOLON
import com.saveourtool.diktat.ruleset.constants.Warnings.WRONG_NEWLINES
import com.saveourtool.diktat.ruleset.rules.DiktatRule
import com.saveourtool.diktat.ruleset.utils.appendNewlineMergingWhiteSpace
import com.saveourtool.diktat.ruleset.utils.emptyBlockList
import com.saveourtool.diktat.ruleset.utils.extractLineOfText
import com.saveourtool.diktat.ruleset.utils.findAllDescendantsWithSpecificType
import com.saveourtool.diktat.ruleset.utils.findAllNodesWithCondition
import com.saveourtool.diktat.ruleset.utils.findParentNodeWithSpecificType
Expand All @@ -19,7 +17,6 @@ import com.saveourtool.diktat.ruleset.utils.getIdentifierName
import com.saveourtool.diktat.ruleset.utils.getRootNode
import com.saveourtool.diktat.ruleset.utils.hasParent
import com.saveourtool.diktat.ruleset.utils.isBeginByNewline
import com.saveourtool.diktat.ruleset.utils.isEol
import com.saveourtool.diktat.ruleset.utils.isFollowedByNewline
import com.saveourtool.diktat.ruleset.utils.isGradleScript
import com.saveourtool.diktat.ruleset.utils.isSingleLineIfElse
Expand All @@ -37,7 +34,6 @@ import org.jetbrains.kotlin.KtNodeTypes.CALL_EXPRESSION
import org.jetbrains.kotlin.KtNodeTypes.CLASS
import org.jetbrains.kotlin.KtNodeTypes.CONDITION
import org.jetbrains.kotlin.KtNodeTypes.DOT_QUALIFIED_EXPRESSION
import org.jetbrains.kotlin.KtNodeTypes.ENUM_ENTRY
import org.jetbrains.kotlin.KtNodeTypes.FUN
import org.jetbrains.kotlin.KtNodeTypes.FUNCTION_LITERAL
import org.jetbrains.kotlin.KtNodeTypes.FUNCTION_TYPE
Expand Down Expand Up @@ -90,7 +86,6 @@ import org.jetbrains.kotlin.lexer.KtTokens.PLUSEQ
import org.jetbrains.kotlin.lexer.KtTokens.RETURN_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.RPAR
import org.jetbrains.kotlin.lexer.KtTokens.SAFE_ACCESS
import org.jetbrains.kotlin.lexer.KtTokens.SEMICOLON
import org.jetbrains.kotlin.lexer.KtTokens.WHITE_SPACE
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtNamedFunction
Expand All @@ -106,31 +101,29 @@ import org.jetbrains.kotlin.psi.psiUtil.siblings
private typealias ListOfList = MutableList<MutableList<ASTNode>>

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

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

/**
* Check that EOL semicolon is used only in enums
*/
private fun handleSemicolon(node: ASTNode) {
if (node.isEol() && node.treeParent.elementType != ENUM_ENTRY) {
// semicolon at the end of line which is not part of enum members declarations
REDUNDANT_SEMICOLON.warnAndFix(configRules, emitWarn, isFixMode, node.extractLineOfText(), node.startOffset, node) {
node.treeParent.removeChild(node)
}
}
}

private fun handleOperatorWithLineBreakAfter(node: ASTNode) {
// [node] should be either EQ or OPERATION_REFERENCE which has single child
if (node.elementType != EQ && node.firstChildNode.elementType !in lineBreakAfterOperators && !node.isInfixCall()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.saveourtool.diktat.ruleset.rules.chapter3.files

import com.saveourtool.diktat.common.config.rules.RulesConfig
import com.saveourtool.diktat.ruleset.constants.Warnings.REDUNDANT_SEMICOLON
import com.saveourtool.diktat.ruleset.rules.DiktatRule
import com.saveourtool.diktat.ruleset.utils.extractLineOfText
import com.saveourtool.diktat.ruleset.utils.isEol
import org.jetbrains.kotlin.KtNodeTypes.ENUM_ENTRY
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.lexer.KtTokens.SEMICOLON

/**
* Rule that checks usage of semicolons at the end of line
*/
@Suppress("ForbiddenComment")
class SemicolonsRule(configRules: List<RulesConfig>) : DiktatRule(
NAME_ID,
configRules,
listOf(REDUNDANT_SEMICOLON)
) {
override fun logic(node: ASTNode) {
if (node.elementType == SEMICOLON) {
handleSemicolon(node)
}
}

/**
* Check that EOL semicolon is used only in enums
*/
private fun handleSemicolon(node: ASTNode) {
if (node.isEol() && node.treeParent.elementType != ENUM_ENTRY) {
// semicolon at the end of line which is not part of enum members declarations
REDUNDANT_SEMICOLON.warnAndFix(configRules, emitWarn, isFixMode, node.extractLineOfText(), node.startOffset, node) {
node.treeParent.removeChild(node)
}
}
}

companion object {
const val NAME_ID = "semicolon"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import com.saveourtool.diktat.ruleset.utils.search.findAllVariablesWithUsages
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.PsiWhiteSpace
import org.jetbrains.kotlin.lexer.KtTokens.SEMICOLON
import org.jetbrains.kotlin.lexer.KtTokens.WHITE_SPACE
import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtCallExpression
Expand Down Expand Up @@ -160,7 +161,7 @@ class LocalVariablesRule(configRules: List<RulesConfig>) : DiktatRule(
) {
val numLinesToSkip = property
.siblings(forward = true, withItself = false)
.takeWhile { it is PsiWhiteSpace || it.node.isPartOfComment() }
.takeWhile { it is PsiWhiteSpace || it.node.elementType == SEMICOLON || it.node.isPartOfComment() }
.let { siblings ->
siblings
.last()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -686,4 +686,17 @@ class LocalVariablesWarnTest : LintTestBase(::LocalVariablesRule) {
""".trimMargin()
)
}

@Test
@Tag(LOCAL_VARIABLE_EARLY_DECLARATION)
fun `shouldn't fail on semicolon`() {
lintMethod(
"""
|fun bar() {
| var a = 0;
| a++
|}
""".trimMargin()
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu
mapOf("maxCallsInOneLine" to "1"))
)

@Test
@Tag(WarningNames.REDUNDANT_SEMICOLON)
fun `should remove redundant semicolons`() {
fixAndCompare("SemicolonsExpected.kt", "SemicolonsTest.kt")
}

@Test
@Tag(WarningNames.WRONG_NEWLINES)
fun `should fix newlines near operators`() {
Expand Down Expand Up @@ -54,7 +48,7 @@ class NewlinesRuleFixTest : FixTestBase("test/paragraph3/newlines", ::NewlinesRu
@Test
@Tag(WarningNames.WRONG_NEWLINES)
fun `One line parameters list sheet must contain no more than 2 parameters`() {
fixAndCompare("sizeParameterListExpected.kt", "sizeParameterListTest.kt")
fixAndCompare("SizeParameterListExpected.kt", "SizeParameterListTest.kt")
}

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

@Test
@Tag(WarningNames.WRONG_NEWLINES)
fun `should fix one line function with and without semicolon`() {
fun `should fix one line function`() {
fixAndCompare("OneLineFunctionExpected.kt", "OneLineFunctionTest.kt")
}

Expand Down
Loading
Loading