Skip to content

Commit

Permalink
WRONG_INDENTATION: add the test which reproduces the regression
Browse files Browse the repository at this point in the history
### What's done:

 * See #1490.
  • Loading branch information
0x6675636b796f75676974687562 committed Aug 11, 2022
1 parent 2f51f57 commit 96dfd85
Show file tree
Hide file tree
Showing 9 changed files with 171 additions and 82 deletions.
Original file line number Diff line number Diff line change
@@ -1,18 +1,50 @@
package org.cqfn.diktat.ruleset.chapter3.spaces

import org.cqfn.diktat.common.config.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_INDENTATION
import org.cqfn.diktat.ruleset.junit.ExpectedLintError
import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationRule.Companion.NAME_ID
import com.pinterest.ktlint.core.LintError

/**
* The expected indentation error (extracted from annotated code fragments).
*
* @property line the line number (1-based).
* @property column the column number (1-based).
* @property expectedIndent the expected indentation level (in space characters).
* @property actualIndent the actual indentation level (in space characters).
*/
data class ExpectedIndentationError(
override val line: Int,
override val column: Int = 1,
val expectedIndent: Int,
val actualIndent: Int
) : ExpectedLintError
class ExpectedIndentationError(override val line: Int,
override val column: Int = 1,
private val message: String
) : ExpectedLintError {
/**
* @param line the line number (1-based).
* @param column the column number (1-based).
* @param expectedIndent the expected indentation level (in space characters).
* @param actualIndent the actual indentation level (in space characters).
*/
constructor(line: Int,
column: Int = 1,
expectedIndent: Int,
actualIndent: Int
) : this(
line,
column,
warnText(expectedIndent)(actualIndent)
)

override fun asLintError(): LintError =
LintError(
line,
column,
"$DIKTAT_RULE_SET_ID:$NAME_ID",
message,
true)

private companion object {
private val warnText: (Int) -> (Int) -> String = { expectedIndent ->
{ actualIndent ->
"${WRONG_INDENTATION.warnText()} expected $expectedIndent but was $actualIndent"
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,22 @@ class IndentationRuleTest {
fun `case 10`() = Unit
}

@Nested
@TestMethodOrder(NaturalDisplayName::class)
inner class `String templates` {
/**
* See [#1490](https://github.com/saveourtool/diktat/issues/1490).
*/
@IndentationTest(IndentedSourceCode(
"""
val value = f(
"text ${'$'}variable text".isEmpty() // diktat:WRONG_INDENTATION[message = only spaces are allowed for indentation and each indentation should equal to 4 spaces (tabs are not allowed): the same number of indents to the opening and closing quotes was expected]
)
"""),
singleConfiguration = true)
fun `issue #1490`() = Unit
}

/**
* See [#1347](https://github.com/saveourtool/diktat/issues/1347).
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ import kotlin.annotation.AnnotationTarget.FUNCTION

/**
* @property includeWarnTests whether unit tests for the "warn" mode should also
* be generated. If `false`, only fix mode tests get generated. The default is
* `true`.
* be generated. If `false`, the code is allowed to have no expected-error
* annotations, and only fix mode tests get generated. The default is `true`.
* @property singleConfiguration whether only a single code fragment is to be
* analysed. If `true`, the value of [second] is ignored, resulting in fewer
* unit tests being generated. The default is `false`.
*/
@Target(FUNCTION)
@Retention(RUNTIME)
Expand All @@ -20,6 +23,7 @@ import kotlin.annotation.AnnotationTarget.FUNCTION
@Tag(WRONG_INDENTATION)
annotation class IndentationTest(
val first: IndentedSourceCode,
val second: IndentedSourceCode,
val includeWarnTests: Boolean = true
val second: IndentedSourceCode = IndentedSourceCode(""),
val includeWarnTests: Boolean = true,
val singleConfiguration: Boolean = false,
)
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ class IndentationTestInvocationContextProvider : RuleInvocationContextProvider<I
tag: String,
properties: Map<String, String?>
): ExpectedIndentationError {
val message = properties[MESSAGE]
@Suppress("AVOID_NULL_CHECKS")
if (message != null) {
return ExpectedIndentationError(
line = lineNumber,
message = "[$WRONG_INDENTATION] $message")
}

val expectedIndent = properties.expectedIndent()
val actualIndent = line.leadingSpaceCount()

Expand All @@ -58,57 +66,81 @@ class IndentationTestInvocationContextProvider : RuleInvocationContextProvider<I
val indentationTest = findAnnotation(testMethod, annotationType().java).get()

val includeWarnTests = indentationTest.includeWarnTests
val singleConfiguration = indentationTest.singleConfiguration

val testInput0 = indentationTest.first.extractTestInput(supportedTags, includeWarnTests)
val testInput0 = indentationTest.first.extractTestInput(
supportedTags,
allowEmptyErrors = !includeWarnTests || singleConfiguration)
val (code0, expectedErrors0, customConfig0) = testInput0

val testInput1 = indentationTest.second.extractTestInput(supportedTags, includeWarnTests)
val (code1, expectedErrors1, customConfig1) = testInput1

assertThat(code0)
.describedAs("Both code fragments are the same")
.isNotEqualTo(code1)
assertThat(customConfig0)
.describedAs("Both custom configs are the same")
.isNotEqualTo(customConfig1)
assertThat(testInput0.effectiveConfig)
.describedAs("Both effective configs are the same")
.isNotEqualTo(testInput1.effectiveConfig)

return Stream.of<TestTemplateInvocationContext>(
IndentationTestFixInvocationContext(customConfig0, actualCode = code0),
IndentationTestFixInvocationContext(customConfig1, actualCode = code1),
IndentationTestFixInvocationContext(customConfig1, actualCode = code0, expectedCode = code1),
IndentationTestFixInvocationContext(customConfig0, actualCode = code1, expectedCode = code0),
).let { fixTests ->
when {
includeWarnTests -> concat(fixTests, Stream.of(
IndentationTestWarnInvocationContext(customConfig0, actualCode = code0),
IndentationTestWarnInvocationContext(customConfig1, actualCode = code1),
IndentationTestWarnInvocationContext(customConfig1, actualCode = code0, expectedErrors0),
IndentationTestWarnInvocationContext(customConfig0, actualCode = code1, expectedErrors1),
))

else -> fixTests
var contexts: Stream<TestTemplateInvocationContext> = Stream.of(
IndentationTestFixInvocationContext(customConfig0, actualCode = code0)
)

if (includeWarnTests) {
/*-
* In a double-configuration mode (the default), when the code is
* checked against its own configuration, the actual list of errors
* is expected to be empty (it's only used when the code is checked
* against the opposite configuration.
*
* In a single-configuration mode, the opposite configuration is
* empty, so let's allow a non-empty list of expected errors when
* the code is checked against its own configuration.
*/
val expectedErrors = when {
singleConfiguration -> expectedErrors0
else -> emptyList()
}
contexts += IndentationTestWarnInvocationContext(customConfig0, actualCode = code0, expectedErrors)
}

if (!singleConfiguration) {
val testInput1 = indentationTest.second.extractTestInput(
supportedTags,
allowEmptyErrors = !includeWarnTests)
val (code1, expectedErrors1, customConfig1) = testInput1

assertThat(code0)
.describedAs("Both code fragments are the same")
.isNotEqualTo(code1)
assertThat(customConfig0)
.describedAs("Both custom configs are the same")
.isNotEqualTo(customConfig1)
assertThat(testInput0.effectiveConfig)
.describedAs("Both effective configs are the same")
.isNotEqualTo(testInput1.effectiveConfig)

contexts += IndentationTestFixInvocationContext(customConfig1, actualCode = code1)
contexts += IndentationTestFixInvocationContext(customConfig1, actualCode = code0, expectedCode = code1)
contexts += IndentationTestFixInvocationContext(customConfig0, actualCode = code1, expectedCode = code0)

if (includeWarnTests) {
contexts += IndentationTestWarnInvocationContext(customConfig1, actualCode = code1)
contexts += IndentationTestWarnInvocationContext(customConfig1, actualCode = code0, expectedErrors0)
contexts += IndentationTestWarnInvocationContext(customConfig0, actualCode = code1, expectedErrors1)
}
}.sorted { left, right ->
}

return contexts.sorted { left, right ->
left.getDisplayName(0).compareTo(right.getDisplayName(0))
}
}

/**
* @param includeWarnTests whether unit tests for the "warn" mode should also
* be generated. If `false`, only fix mode tests get generated.
* @param allowEmptyErrors whether the list of expected errors is allowed to
* be empty (i.e. the code may contain no known annotations).
*/
private fun IndentedSourceCode.extractTestInput(supportedTags: List<String>,
includeWarnTests: Boolean): IndentationTestInput {
val (code, expectedErrors) = extractExpectedErrors(code, supportedTags, includeWarnTests)
allowEmptyErrors: Boolean): IndentationTestInput {
val (code, expectedErrors) = extractExpectedErrors(code, supportedTags, allowEmptyErrors)

return IndentationTestInput(code, expectedErrors, customConfig())
}

private companion object {
private const val EXPECTED_INDENT = "expectedIndent"
private const val MESSAGE = "message"

@Suppress("WRONG_NEWLINES") // False positives, see #1495.
private fun IndentedSourceCode.customConfig(): SortedMap<String, out Boolean> =
Expand Down Expand Up @@ -140,5 +172,8 @@ class IndentationTestInvocationContextProvider : RuleInvocationContextProvider<I
fail("Unparseable `$EXPECTED_INDENT`: $expectedIndentRaw")
}
}

private operator fun <T> Stream<T>.plus(value: T): Stream<T> =
concat(this, Stream.of(value))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,17 @@ internal class IndentationTestWarnExtension(
val description = NEWLINE + actualCode.annotateWith(actualErrors) + NEWLINE

when {
expectedErrors.size == 1 && actualErrors.size == 1 -> assertThat(actualErrors[0])
.describedAs(description)
.isEqualTo(expectedErrors[0])
expectedErrors.size == 1 && actualErrors.size == 1 -> {
val actual = actualErrors[0]
val expected = expectedErrors[0]

assertThat(actual)
.describedAs(description)
.isEqualTo(expected)
assertThat(actual.canBeAutoCorrected)
.describedAs("canBeAutoCorrected")
.isEqualTo(expected.canBeAutoCorrected)
}

else -> assertThat(actualErrors)
.describedAs(description)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package org.cqfn.diktat.ruleset.chapter3.spaces.junit

import org.cqfn.diktat.common.config.rules.DIKTAT_RULE_SET_ID
import org.cqfn.diktat.ruleset.chapter3.spaces.ExpectedIndentationError
import org.cqfn.diktat.ruleset.constants.Warnings.WRONG_INDENTATION
import org.cqfn.diktat.ruleset.rules.chapter3.files.IndentationRule.Companion.NAME_ID
import com.pinterest.ktlint.core.LintError
import org.cqfn.diktat.ruleset.junit.ExpectedLintError
import org.intellij.lang.annotations.Language
import org.junit.jupiter.api.extension.Extension
import java.util.SortedMap
Expand Down Expand Up @@ -36,25 +33,5 @@ internal class IndentationTestWarnInvocationContext(
listOf(IndentationTestWarnExtension(
customConfig,
actualCode,
expectedErrors.map(asLintError).toTypedArray()))

private companion object {
private val warnText: (Int) -> (Int) -> String = { expectedIndent ->
{ actualIndent ->
"${WRONG_INDENTATION.warnText()} expected $expectedIndent but was $actualIndent"
}
}

/**
* Converts this instance to a [LintError].
*/
private val asLintError: ExpectedIndentationError.() -> LintError = {
LintError(
line,
column,
"$DIKTAT_RULE_SET_ID:$NAME_ID",
warnText(expectedIndent)(actualIndent),
true)
}
}
expectedErrors.map(ExpectedLintError::asLintError).toTypedArray()))
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.cqfn.diktat.ruleset.junit

import com.pinterest.ktlint.core.LintError

/**
* The common super-interface for expected lint errors (extracted from the
* annotated code).
Expand All @@ -14,4 +16,11 @@ interface ExpectedLintError {
* The column number (1-based).
*/
val column: Int

/**
* Converts this instance to a [LintError].
*
* @return the [LintError] which corresponds to this instance.
*/
fun asLintError(): LintError
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,15 @@ interface RuleInvocationContextProvider<A : Annotation, out E : ExpectedLintErro
* @param code the annotated code.
* @param supportedTags the list of check names that should be recognized
* (implementation-dependent).
* @param includeWarnTests whether unit tests for the "warn" mode should also
* be generated. If `false`, only fix mode tests get generated.
* @param allowEmptyErrors whether the list of expected errors is allowed to
* be empty (i.e. the code may contain no known annotations).
* @return the list of expected errors as well as the filtered code.
* @see expectedLintErrorFrom
*/
@Suppress("TOO_LONG_FUNCTION")
fun extractExpectedErrors(@Language("kotlin") code: String,
supportedTags: List<String>,
includeWarnTests: Boolean
allowEmptyErrors: Boolean
): ExpectedLintErrors<E> {
require(supportedTags.isNotEmpty()) {
"The list of supported tags is empty"
Expand Down Expand Up @@ -120,7 +120,7 @@ interface RuleInvocationContextProvider<A : Annotation, out E : ExpectedLintErro
1 -> supportedTags[0]
else -> "any of $supportedTags"
}
if (includeWarnTests) {
if (!allowEmptyErrors) {
assertThat(expectedErrors)
.describedAs("The code contains no expected-error annotations or an unsupported tag is used (should be $supportedTagsDescription). " +
"Please annotate your code or set `includeWarnTests` to `false`:$NEWLINE$filteredCode")
Expand Down Expand Up @@ -177,7 +177,7 @@ interface RuleInvocationContextProvider<A : Annotation, out E : ExpectedLintErro
private const val TAG = "tag"

@Language("RegExp")
private const val VALUE = """[^,\h]*"""
private const val VALUE = """[^,\]]*?"""
private const val VALUE_GROUP = "value"
private val entryRegex = Regex("""\h*(?<$KEY_GROUP>$KEY)\h*=\h*(?<$VALUE_GROUP>$VALUE)\h*""")

Expand Down
14 changes: 11 additions & 3 deletions diktat-rules/src/test/kotlin/org/cqfn/diktat/util/LintTestBase.kt
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,17 @@ open class LintTestBase(private val ruleSupplier: (rulesConfigList: List<RulesCo
val description = "lint result for \"$code\""

when {
expectedLintErrors.size == 1 && actualLintErrors.size == 1 -> assertThat(actualLintErrors[0])
.describedAs(description)
.isEqualTo(expectedLintErrors[0])
expectedLintErrors.size == 1 && actualLintErrors.size == 1 -> {
val actual = actualLintErrors[0]
val expected = expectedLintErrors[0]

assertThat(actual)
.describedAs(description)
.isEqualTo(expected)
assertThat(actual.canBeAutoCorrected)
.describedAs("canBeAutoCorrected")
.isEqualTo(expected.canBeAutoCorrected)
}

else -> assertThat(actualLintErrors)
.describedAs(description)
Expand Down

0 comments on commit 96dfd85

Please sign in to comment.