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.

Sign up for GitHub

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

Space after property accessor #56

Merged
merged 3 commits into from
Jun 7, 2017
Merged

Space after property accessor #56

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0a34689
Add test cases for spacing after setter and getter
arturbosch Jun 7, 2017
468dcd4
Implement spacing after setter and getter
arturbosch Jun 7, 2017
adbbc3d
Fix regression for spacing around else keyword
arturbosch Jun 7, 2017
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
45 changes: 29 additions & 16 deletions ...ard/src/main/kotlin/com/github/shyiko/ktlint/ruleset/standard/SpacingAroundKeywordRule.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl
import org.jetbrains.kotlin.com.intellij.psi.tree.TokenSet
import org.jetbrains.kotlin.com.intellij.psi.util.PsiTreeUtil
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.lexer.KtTokens.CATCH_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.DO_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.ELSE_KEYWORD
Expand All @@ -16,32 +17,44 @@ import org.jetbrains.kotlin.lexer.KtTokens.IF_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.TRY_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.WHEN_KEYWORD
import org.jetbrains.kotlin.lexer.KtTokens.WHILE_KEYWORD
import org.jetbrains.kotlin.psi.KtPropertyAccessor
import org.jetbrains.kotlin.psi.KtWhenEntry
import org.jetbrains.kotlin.psi.psiUtil.nextLeaf

class SpacingAroundKeywordRule : Rule("keyword-spacing") {

private val noLFBeforeSet = TokenSet.create(ELSE_KEYWORD, CATCH_KEYWORD, FINALLY_KEYWORD)
private val tokenSet = TokenSet.create(FOR_KEYWORD, IF_KEYWORD, ELSE_KEYWORD, WHILE_KEYWORD, DO_KEYWORD,
TRY_KEYWORD, CATCH_KEYWORD, FINALLY_KEYWORD, WHEN_KEYWORD)
// todo: but not after fun(, get(, set(

private val keywordsWithoutSpaces = TokenSet.create(KtTokens.GET_KEYWORD, KtTokens.SET_KEYWORD)

override fun visit(node: ASTNode, autoCorrect: Boolean,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) {
if (tokenSet.contains(node.elementType) && node is LeafPsiElement &&
PsiTreeUtil.nextLeaf(node) !is PsiWhiteSpace) {
emit(node.startOffset + node.text.length, "Missing spacing after \"${node.text}\"", true)
if (autoCorrect) {
node.rawInsertAfterMe(PsiWhiteSpaceImpl(" "))
}
}
if (noLFBeforeSet.contains(node.elementType) && node is LeafPsiElement) {
val prevLeaf = PsiTreeUtil.prevLeaf(node)
if (prevLeaf is PsiWhiteSpaceImpl && prevLeaf.textContains('\n') &&
(node.elementType != ELSE_KEYWORD || node.parent !is KtWhenEntry) &&
(PsiTreeUtil.prevLeaf(prevLeaf)?.textMatches("}") ?: false)) {
emit(node.startOffset, "Unexpected newline before \"${node.text}\"", true)
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) {

if (node is LeafPsiElement) {
if (tokenSet.contains(node.elementType) && node.nextLeaf() !is PsiWhiteSpace) {
emit(node.startOffset + node.text.length, "Missing spacing after \"${node.text}\"", true)
if (autoCorrect) {
prevLeaf.rawReplaceWithText(" ")
node.rawInsertAfterMe(PsiWhiteSpaceImpl(" "))
}
} else if (keywordsWithoutSpaces.contains(node.elementType) && node.nextLeaf() is PsiWhiteSpace) {
val parent = node.parent
if (parent is KtPropertyAccessor && parent.hasBody()) {
emit(node.startOffset, "Unexpected spacing after \"${node.text}\"", true)
if (autoCorrect) {
node.nextLeaf()?.delete()
}
}
} else if (noLFBeforeSet.contains(node.elementType)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arturbosch I think we have a regression here. Previously this block would be executed regardless of the preceding if clause (there was no else) (i.e. there could be 2 LintError's emitted for the same node: Unexpected newline before "else" & Missing spacing after "else".) but no anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes your right, my fault - the else keyword is in both sets.

val prevLeaf = PsiTreeUtil.prevLeaf(node)
if (prevLeaf is PsiWhiteSpaceImpl && prevLeaf.textContains('\n') &&
(node.elementType != ELSE_KEYWORD || node.parent !is KtWhenEntry) &&
(PsiTreeUtil.prevLeaf(prevLeaf)?.textMatches("}") ?: false)) {
emit(node.startOffset, "Unexpected newline before \"${node.text}\"", true)
if (autoCorrect) {
prevLeaf.rawReplaceWithText(" ")
}
}
}
}
Expand Down
44 changes: 43 additions & 1 deletion ...src/test/kotlin/com/github/shyiko/ktlint/ruleset/standard/SpacingAroundKeywordRuleTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.github.shyiko.ktlint.ruleset.standard

import com.github.shyiko.ktlint.core.LintError
import com.github.shyiko.ktlint.test.lint
import com.github.shyiko.ktlint.test.format
import com.github.shyiko.ktlint.test.lint
import org.assertj.core.api.Assertions.assertThat
import org.testng.annotations.Test

Expand Down Expand Up @@ -94,5 +94,47 @@ class SpacingAroundKeywordRuleTest {
)
}

@Test
fun getterAndSetterFunction() {
assertThat(SpacingAroundKeywordRule().format(
"""
var x: String
get () {
return ""
}
private set (value) {
x = value
}
""".trimIndent()
)).isEqualTo(
"""
var x: String
get() {
return ""
}
private set(value) {
x = value
}
""".trimIndent()
)
}

@Test
fun visibilityOrInjectProperty() {
assertThat(SpacingAroundKeywordRule().lint(
"""
var setterVisibility: String = "abc"
private set
var setterWithAnnotation: Any? = null
@Inject set
var setterOnNextLine: String
private set
(value) { setterOnNextLine = value}
"""
)).isEqualTo(listOf(
LintError(7, 21, "keyword-spacing", "Unexpected spacing after \"set\"")
))
}

}