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

Add excludesRawStrings in MaxLineLength #5171

Merged
merged 1 commit into from Aug 2, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions detekt-core/src/main/resources/default-detekt-config.yml
Expand Up @@ -597,6 +597,7 @@ style:
excludePackageStatements: true
excludeImportStatements: true
excludeCommentStatements: false
excludeRawStrings: true
MayBeConst:
active: true
ModifierOrder:
Expand Down
Expand Up @@ -13,6 +13,8 @@ import io.gitlab.arturbosch.detekt.api.internal.Configuration
import io.gitlab.arturbosch.detekt.rules.lastArgumentMatchesUrl
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.psi.KtFile
import org.jetbrains.kotlin.psi.KtStringTemplateExpression
import org.jetbrains.kotlin.psi.psiUtil.getParentOfType

/**
* This rule reports lines of code which exceed a defined maximum line length.
Expand Down Expand Up @@ -43,14 +45,17 @@ class MaxLineLength(config: Config = Config.empty) : Rule(config) {
@Configuration("if comment statements should be ignored")
private val excludeCommentStatements: Boolean by config(false)

@Configuration("if comment statements should be ignored")
private val excludeRawStrings: Boolean by config(true)

fun visit(element: KtFileContent) {
var offset = 0
val lines = element.content
val file = element.file

for (line in lines) {
offset += line.length
if (!isValidLine(line)) {
if (!isValidLine(file, offset, line)) {
val ktElement = findFirstMeaningfulKtElementInParents(file, offset, line)
if (ktElement != null) {
report(CodeSmell(issue, Entity.from(ktElement), issue.description))
Expand All @@ -63,15 +68,22 @@ class MaxLineLength(config: Config = Config.empty) : Rule(config) {
}
}

private fun isValidLine(line: String): Boolean {
private fun isValidLine(file: KtFile, offset: Int, line: String): Boolean {
val isUrl = line.lastArgumentMatchesUrl()
return line.length <= maxLineLength || isIgnoredStatement(line) || isUrl
return line.length <= maxLineLength || isIgnoredStatement(file, offset, line) || isUrl
}

private fun isIgnoredStatement(line: String): Boolean {
private fun isIgnoredStatement(file: KtFile, offset: Int, line: String): Boolean {
return containsIgnoredPackageStatement(line) ||
containsIgnoredImportStatement(line) ||
containsIgnoredCommentStatement(line)
containsIgnoredCommentStatement(line) ||
containsIgnoredRawString(file, offset, line)
}

private fun containsIgnoredRawString(file: KtFile, offset: Int, line: String): Boolean {
if (!excludeRawStrings) return false

return findKtElementInParents(file, offset, line).lastOrNull()?.isInsideRawString() == true
}

private fun containsIgnoredPackageStatement(line: String): Boolean {
Expand Down Expand Up @@ -104,3 +116,7 @@ class MaxLineLength(config: Config = Config.empty) : Rule(config) {
}
}
}

private fun PsiElement.isInsideRawString(): Boolean {
return this is KtStringTemplateExpression || getParentOfType<KtStringTemplateExpression>(false) != null
}
Expand Up @@ -19,10 +19,9 @@ class MaxLineLengthSpec {

@Nested
inner class `a kt file with some long lines` {

val file = compileForTest(Case.MaxLineLength.path())
val lines = file.text.splitToSequence("\n")
val fileContent = KtFileContent(file, lines)
private val file = compileForTest(Case.MaxLineLength.path())
private val lines = file.text.splitToSequence("\n")
private val fileContent = KtFileContent(file, lines)

@Test
fun `should report no errors when maxLineLength is set to 200`() {
Expand All @@ -36,6 +35,14 @@ class MaxLineLengthSpec {
fun `should report all errors with default maxLineLength`() {
val rule = MaxLineLength()

rule.visit(fileContent)
assertThat(rule.findings).hasSize(3)
}

@Test
fun `should report all errors with default maxLineLength including raw strings`() {
val rule = MaxLineLength(TestConfig("excludeRawStrings" to false))

rule.visit(fileContent)
assertThat(rule.findings).hasSize(7)
}
Expand All @@ -52,10 +59,9 @@ class MaxLineLengthSpec {

@Nested
inner class `a kt file with long but suppressed lines` {

val file = compileForTest(Case.MaxLineLengthSuppressed.path())
val lines = file.text.splitToSequence("\n")
val fileContent = KtFileContent(file, lines)
private val file = compileForTest(Case.MaxLineLengthSuppressed.path())
private val lines = file.text.splitToSequence("\n")
private val fileContent = KtFileContent(file, lines)

@Test
fun `should not report as lines are suppressed`() {
Expand All @@ -77,9 +83,9 @@ class MaxLineLengthSpec {
}
"""

val file = compileContentForTest(code)
val lines = file.text.splitToSequence("\n")
val fileContent = KtFileContent(file, lines)
private val file = compileContentForTest(code)
private val lines = file.text.splitToSequence("\n")
private val fileContent = KtFileContent(file, lines)

@Test
fun `should not report the package statement and import statements by default`() {
Expand Down Expand Up @@ -115,11 +121,9 @@ class MaxLineLengthSpec {
fun `should not report anything if both package and import statements are disabled`() {
val rule = MaxLineLength(
TestConfig(
mapOf(
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "true",
EXCLUDE_IMPORT_STATEMENTS to "true"
)
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "true",
EXCLUDE_IMPORT_STATEMENTS to "true",
)
)

Expand All @@ -130,19 +134,14 @@ class MaxLineLengthSpec {

@Nested
inner class `a kt file with a long package name, long import statements, a long line and long comments` {

val file = compileForTest(Case.MaxLineLengthWithLongComments.path())
val lines = file.text.splitToSequence("\n")
val fileContent = KtFileContent(file, lines)
private val file = compileForTest(Case.MaxLineLengthWithLongComments.path())
private val lines = file.text.splitToSequence("\n")
private val fileContent = KtFileContent(file, lines)

@Test
fun `should report the package statement, import statements, line and comments by default`() {
val rule = MaxLineLength(
TestConfig(
mapOf(
MAX_LINE_LENGTH to "60"
)
)
TestConfig(MAX_LINE_LENGTH to "60")
)

rule.visit(fileContent)
Expand All @@ -153,12 +152,10 @@ class MaxLineLengthSpec {
fun `should report the package statement, import statements, line and comments if they're enabled`() {
val rule = MaxLineLength(
TestConfig(
mapOf(
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "false",
EXCLUDE_IMPORT_STATEMENTS to "false",
EXCLUDE_COMMENT_STATEMENTS to "false"
)
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "false",
EXCLUDE_IMPORT_STATEMENTS to "false",
EXCLUDE_COMMENT_STATEMENTS to "false",
)
)

Expand Down Expand Up @@ -194,18 +191,14 @@ class MaxLineLengthSpec {
}
""".trimIndent()

val file = compileContentForTest(code)
val lines = file.text.splitToSequence("\n")
val fileContent = KtFileContent(file, lines)
private val file = compileContentForTest(code)
private val lines = file.text.splitToSequence("\n")
private val fileContent = KtFileContent(file, lines)

@Test
fun `should only the function line by default`() {
val rule = MaxLineLength(
TestConfig(
mapOf(
MAX_LINE_LENGTH to "60"
)
)
TestConfig(MAX_LINE_LENGTH to "60")
)

rule.visit(fileContent)
Expand All @@ -216,11 +209,9 @@ class MaxLineLengthSpec {
fun `should report the package statement, import statements and line if they're not excluded`() {
val rule = MaxLineLength(
TestConfig(
mapOf(
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "false",
EXCLUDE_IMPORT_STATEMENTS to "false"
)
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "false",
EXCLUDE_IMPORT_STATEMENTS to "false",
)
)

Expand All @@ -232,11 +223,9 @@ class MaxLineLengthSpec {
fun `should report only method if both package and import statements are disabled`() {
val rule = MaxLineLength(
TestConfig(
mapOf(
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "true",
EXCLUDE_IMPORT_STATEMENTS to "true"
)
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "true",
EXCLUDE_IMPORT_STATEMENTS to "true",
)
)

Expand All @@ -248,11 +237,9 @@ class MaxLineLengthSpec {
fun `should report correct line and column for function with excessive length`() {
val rule = MaxLineLength(
TestConfig(
mapOf(
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "true",
EXCLUDE_IMPORT_STATEMENTS to "true"
)
MAX_LINE_LENGTH to "60",
EXCLUDE_PACKAGE_STATEMENTS to "true",
EXCLUDE_IMPORT_STATEMENTS to "true",
)
)

Expand Down