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 rule ModifierListSpacingRule #1361

Merged
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Please welcome [paul-dingemans](https://github.com/paul-dingemans) as an officia
- Basic tests for CLI ([#540](https://github.com/pinterest/ktlint/issues/540))
- Add experimental rule for unexpected spaces in a type reference before a function identifier (`function-type-reference-spacing`) ([#1341](https://github.com/pinterest/ktlint/issues/1341))
- Add experimental rule for unnecessary parentheses in function call followed by lambda ([#1068](https://github.com/pinterest/ktlint/issues/1068))
- Add experimental rules for unnecessary spacing between modifiers in and after the last modifier in a modifier list ([#1361](https://github.com/pinterest/ktlint/pull/1361))

### Fixed
- Fix indentation of function literal ([#1247](https://github.com/pinterest/ktlint/issues/1247))
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ by passing the `--experimental` flag to `ktlint`.
- `experimental:annotation-spacing`: Annotations should be separated by the annotated declaration by a single line break
- `experimental:double-colon-spacing`: No spaces around `::`
- `experimental:function-type-reference-spacing`: Consistent spacing in the type reference before a function
- `experimental:modifier-list-spacing`: Consistent spacing between modifiers in and after the last modifier in a modifier list
- `experimental:spacing-around-angle-brackets`: No spaces around angle brackets
- `experimental:spacing-between-declarations-with-annotations`: Declarations with annotations should be separated by a blank line
- `experimental:spacing-between-declarations-with-comments`: Declarations with comments should be separated by a blank line
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ public class ExperimentalRuleSetProvider : RuleSetProvider {
SpacingAroundAngleBracketsRule(),
SpacingAroundUnaryOperatorRule(),
AnnotationSpacingRule(),
UnnecessaryParenthesesBeforeTrailingLambdaRule(),
FunctionTypeReferenceSpacingRule(),
UnnecessaryParenthesesBeforeTrailingLambdaRule()
ModifierListSpacingRule()
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
package com.pinterest.ktlint.ruleset.experimental

import com.pinterest.ktlint.core.Rule
import com.pinterest.ktlint.core.ast.ElementType.ANNOTATION_ENTRY
import com.pinterest.ktlint.core.ast.ElementType.MODIFIER_LIST
import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE
import com.pinterest.ktlint.core.ast.children
import com.pinterest.ktlint.core.ast.isPartOfComment
import com.pinterest.ktlint.core.ast.lineIndent
import com.pinterest.ktlint.core.ast.nextLeaf
import com.pinterest.ktlint.core.ast.nextSibling
import com.pinterest.ktlint.core.ast.prevLeaf
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement

/**
* Lint and format the spacing between the modifiers in and after the last modifier in a modifier list.
*/
public class ModifierListSpacingRule : Rule("modifier-list-spacing") {
override fun visit(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
if (node.elementType == MODIFIER_LIST) {
node
.children()
.forEach { visitModifierChild(it, autoCorrect, emit) }
// The whitespace of the last entry of the modifier list is actually placed outside the modifier list
visitModifierChild(node, autoCorrect, emit)
}
}

private fun visitModifierChild(
node: ASTNode,
autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
if (node.elementType == WHITE_SPACE) {
return
}
node.nextSibling { it.elementType == WHITE_SPACE && it.nextLeaf()?.isPartOfComment() != true }
?.takeIf { it.elementType == WHITE_SPACE }
?.takeUnless {
// Regardless of element type, a single white space is always ok and does not need to be checked.
it.text == " "
}
?.takeUnless {
// An annotation entry followed by a single newline (and possibly an indent for the next line) is
// always ok and does not need further checking.
it.elementType == ANNOTATION_ENTRY && it.text.trimEnd(' ', '\t') == "\n"
}
?.takeUnless {
// A single newline after a comment is always ok and does not need further checking.
it.text.trim(' ', '\t').contains('\n') && it.prevLeaf()?.isPartOfComment() == true
}
?.let { whitespace ->
if (node.elementType == ANNOTATION_ENTRY ||
(node.elementType == MODIFIER_LIST && node.lastChildNode?.elementType == ANNOTATION_ENTRY)
) {
val expectedWhiteSpace = if (whitespace.textContains('\n')) {
"\n" + node.lineIndent()
} else {
" "
}
if (whitespace.text != expectedWhiteSpace) {
emit(
whitespace.startOffset,
"Single whitespace or newline expected after annotation",
true
)
if (autoCorrect) {
(whitespace as LeafPsiElement).rawReplaceWithText(expectedWhiteSpace)
}
}
} else {
emit(
whitespace.startOffset,
"Single whitespace expected after modifier",
true
)
if (autoCorrect) {
(whitespace as LeafPsiElement).rawReplaceWithText(" ")
}
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
package com.pinterest.ktlint.ruleset.experimental

import com.pinterest.ktlint.core.LintError
import com.pinterest.ktlint.test.format
import com.pinterest.ktlint.test.lint
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test

class ModifierListSpacingRuleTest {
@Test
fun `Given a function preceded by multiple modifiers separated by multiple space then remove redundant spaces`() {
val code =
"""
abstract class Foo {
@Throws(RuntimeException::class)
protected abstract suspend fun execute()
}
""".trimIndent()
val formattedCode =
"""
abstract class Foo {
@Throws(RuntimeException::class)
protected abstract suspend fun execute()
}
""".trimIndent()
assertThat(ModifierListSpacingRule().lint(code)).containsExactly(
LintError(1, 9, "modifier-list-spacing", "Single whitespace expected after modifier"),
LintError(3, 14, "modifier-list-spacing", "Single whitespace expected after modifier"),
LintError(3, 24, "modifier-list-spacing", "Single whitespace expected after modifier"),
LintError(3, 33, "modifier-list-spacing", "Single whitespace expected after modifier")
)
assertThat(ModifierListSpacingRule().format(code)).isEqualTo(formattedCode)
}

@Test
fun `Given a function preceded by multiple modifiers separated by newlines then remove redundant spaces`() {
val code =
"""
abstract
class Foo {
@Throws(RuntimeException::class)
protected
abstract
suspend
fun execute()
}
""".trimIndent()
val formattedCode =
"""
abstract class Foo {
@Throws(RuntimeException::class)
protected abstract suspend fun execute()
}
""".trimIndent()
assertThat(ModifierListSpacingRule().lint(code)).containsExactly(
LintError(1, 9, "modifier-list-spacing", "Single whitespace expected after modifier"),
LintError(4, 14, "modifier-list-spacing", "Single whitespace expected after modifier"),
LintError(5, 13, "modifier-list-spacing", "Single whitespace expected after modifier"),
LintError(6, 12, "modifier-list-spacing", "Single whitespace expected after modifier")
)
assertThat(ModifierListSpacingRule().format(code)).isEqualTo(formattedCode)
}

@Test
fun `Given a modifier list followed by multiple space then remove the redundant spaces`() {
val code =
"""
fun foo(vararg bar) = "some-result"
fun foo(
vararg
bar
) = "some-result"
""".trimIndent()
val formattedCode =
"""
fun foo(vararg bar) = "some-result"
fun foo(
vararg bar
) = "some-result"
""".trimIndent()
assertThat(ModifierListSpacingRule().lint(code)).containsExactly(
LintError(1, 15, "modifier-list-spacing", "Single whitespace expected after modifier"),
LintError(3, 11, "modifier-list-spacing", "Single whitespace expected after modifier")
)
assertThat(ModifierListSpacingRule().format(code)).isEqualTo(formattedCode)
}

@Test
fun `Annotation modifiers may be followed by a newline or a space`() {
val code =
"""
@Foo1 @Foo2
class Bar {}
""".trimIndent()
assertThat(ModifierListSpacingRule().format(code)).isEqualTo(code)
}

@Test
fun `Annotation modifiers may not be followed by multiple spaces`() {
val code =
"""
@Foo1 @Foo2 class Bar {}
""".trimIndent()
val formattedCode =
"""
@Foo1 @Foo2 class Bar {}
""".trimIndent()
assertThat(ModifierListSpacingRule().lint(code)).containsExactly(
LintError(1, 6, "modifier-list-spacing", "Single whitespace or newline expected after annotation"),
LintError(1, 13, "modifier-list-spacing", "Single whitespace or newline expected after annotation")
)
assertThat(ModifierListSpacingRule().format(code)).isEqualTo(formattedCode)
}

@Test
fun `Annotation modifiers may not be followed by multiple newlines`() {
val code =
"""
@Foo1

@Foo2

class Bar {}
""".trimIndent()
val formattedCode =
"""
@Foo1
@Foo2
class Bar {}
""".trimIndent()
assertThat(ModifierListSpacingRule().lint(code)).containsExactly(
LintError(1, 6, "modifier-list-spacing", "Single whitespace or newline expected after annotation"),
LintError(3, 6, "modifier-list-spacing", "Single whitespace or newline expected after annotation")
)
assertThat(ModifierListSpacingRule().format(code)).isEqualTo(formattedCode)
}

@Test
fun `Given annotations that correctly indented then do no emit warnings`() {
val code =
"""
@Foo1
@Foo2
class Bar {}
""".trimIndent()
assertThat(ModifierListSpacingRule().lint(code)).isEmpty()
assertThat(ModifierListSpacingRule().format(code)).isEqualTo(code)
}

@Test
fun `Given annotations followed by comments that correctly indented then do no emit warnings`() {
val code =
"""
@Foo1 // some-comment
@Foo2
/**
* Some comment
*/
@Foo3
class Bar {}
""".trimIndent()
assertThat(ModifierListSpacingRule().lint(code)).isEmpty()
assertThat(ModifierListSpacingRule().format(code)).isEqualTo(code)
}
}