Skip to content

Commit

Permalink
Address feedback on ClassNameMatchesFileNameRule PR
Browse files Browse the repository at this point in the history
- Get filepath from userData
- Ignore non .kt files
- Change offset to 0 for errror reporting
- Removed ASTNodeExtensions, moved visit() method to internal util file
  • Loading branch information
JelloRanger committed Apr 30, 2018
1 parent 790710e commit 1025de1
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 35 deletions.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -487,4 +487,9 @@ object KtLint {
}
return null
}

private fun ASTNode.visit(cb: (node: ASTNode) -> Unit) {
cb(this)
this.getChildren(null).forEach { it.visit(cb) }
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.github.shyiko.ktlint.ruleset.standard

import com.github.shyiko.ktlint.core.KtLint
import com.github.shyiko.ktlint.core.Rule
import com.github.shyiko.ktlint.core.visit
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.stubs.elements.KtStubElementTypes
Expand All @@ -12,30 +12,32 @@ import java.nio.file.Paths
*/
class ClassNameMatchesFileNameRule : Rule("class-name-matches-file-name"), Rule.Modifier.RestrictToRoot {

private class NameWithOffset(val name: String, val offset: Int)

override fun visit(
node: ASTNode,
autoCorrect: Boolean,
fileName: String,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
val topLevelClassNames = mutableListOf<NameWithOffset>()
// Ignore all non ".kt" files (including ".kts")
if (node.getUserData(KtLint.FILE_PATH_USER_DATA_KEY)?.endsWith(".kt") != true) {
return
}

val topLevelClassNames = mutableListOf<String>()

node.visit {
if (it.elementType == KtStubElementTypes.CLASS && it.treeParent.elementType == KtStubElementTypes.FILE) {
val classIdentifier = it.findChildByType(KtTokens.IDENTIFIER)
classIdentifier?.let {
val className = it.text
topLevelClassNames.add(NameWithOffset(className, it.startOffset))
topLevelClassNames.add(className)
}
}
}

val name = Paths.get(fileName).fileName.toString().substringBefore(".")
if (topLevelClassNames.size == 1 && name != topLevelClassNames.first().name) {
emit(topLevelClassNames.first().offset,
"Single top level class name [${topLevelClassNames.first().name}] does not match file name",
val name = Paths.get(node.getUserData(KtLint.FILE_PATH_USER_DATA_KEY)).fileName.toString().substringBefore(".")
if (topLevelClassNames.size == 1 && name != topLevelClassNames.first()) {
emit(0,
"Single top level class name [${topLevelClassNames.first()}] does not match file name",
false)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.github.shyiko.ktlint.ruleset.standard

import com.github.shyiko.ktlint.core.Rule
import com.github.shyiko.ktlint.core.visit
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.openapi.util.TextRange
import org.jetbrains.kotlin.com.intellij.psi.PsiComment
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.github.shyiko.ktlint.ruleset.standard

import com.github.shyiko.ktlint.core.Rule
import com.github.shyiko.ktlint.core.visit
import org.jetbrains.kotlin.KtNodeTypes
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.kdoc.lexer.KDocTokens
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.github.shyiko.ktlint.ruleset.standard

import com.github.shyiko.ktlint.core.Rule
import com.github.shyiko.ktlint.core.visit
import org.jetbrains.kotlin.KtNodeTypes
import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.lang.FileASTNode
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.github.shyiko.ktlint.ruleset.standard

import org.jetbrains.kotlin.com.intellij.lang.ASTNode
import org.jetbrains.kotlin.com.intellij.psi.PsiElement
import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.LeafPsiElement
import org.jetbrains.kotlin.com.intellij.psi.util.PsiTreeUtil
Expand All @@ -11,6 +12,10 @@ internal fun PsiElement.isPartOf(clazz: KClass<out PsiElement>) = getNonStrictPa
internal fun PsiElement.isPartOfString() = isPartOf(KtStringTemplateEntry::class)
internal fun PsiElement.prevLeaf(): LeafPsiElement? = PsiTreeUtil.prevLeaf(this) as LeafPsiElement?
internal fun PsiElement.nextLeaf(): LeafPsiElement? = PsiTreeUtil.nextLeaf(this) as LeafPsiElement?
internal fun ASTNode.visit(cb: (node: ASTNode) -> Unit) {
cb(this)
this.getChildren(null).forEach { it.visit(cb) }
}

internal fun <T> List<T>.head() = this.subList(0, this.size - 1)
internal fun <T> List<T>.tail() = this.subList(1, this.size)
Original file line number Diff line number Diff line change
@@ -1,64 +1,81 @@
package com.github.shyiko.ktlint.ruleset.standard

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

class ClassNameMatchesFileNameRuleTest {

@Test
fun testMatchingSingleClassName() {
assertThat(ClassNameMatchesFileNameRule().lintWithFileName("/some/path/A.kt",
assertThat(ClassNameMatchesFileNameRule().lint(
"""
class A
""".trimIndent()
""".trimIndent(),
fileName("/some/path/A.kt")
)).isEmpty()
}

@Test
fun testNonMatchingSingleClassName() {
assertThat(ClassNameMatchesFileNameRule().lintWithFileName("A.kt",
assertThat(ClassNameMatchesFileNameRule().lint(
"""
class B
""".trimIndent()
""".trimIndent(),
fileName("A.kt")
)).isEqualTo(listOf(
LintError(1, 7, "class-name-matches-file-name", "Single top level class name [B] does not match file name")
LintError(1, 1, "class-name-matches-file-name", "Single top level class name [B] does not match file name")
))
}

@Test
fun testMultipleTopLevelClasses() {
assertThat(ClassNameMatchesFileNameRule().lintWithFileName("A.kt",
assertThat(ClassNameMatchesFileNameRule().lint(
"""
class B
class C
""".trimIndent()
""".trimIndent(),
fileName("A.kt")
)).isEmpty()
}

@Test
fun testMultipleNonTopLevelClasses() {
assertThat(ClassNameMatchesFileNameRule().lintWithFileName("A.kt",
assertThat(ClassNameMatchesFileNameRule().lint(
"""
class B {
class C
class D
}
""".trimIndent()
""".trimIndent(),
fileName("A.kt")
)).isEqualTo(listOf(
LintError(1, 7, "class-name-matches-file-name", "Single top level class name [B] does not match file name")
LintError(1, 1, "class-name-matches-file-name", "Single top level class name [B] does not match file name")
))
}

@Test
fun testCaseSensitiveMatching() {
assertThat(ClassNameMatchesFileNameRule().lintWithFileName("woohoo.kt",
assertThat(ClassNameMatchesFileNameRule().lint(
"""
interface Woohoo
""".trimIndent()
""".trimIndent(),
fileName("woohoo.kt")
)).isEqualTo(listOf(
LintError(1, 11, "class-name-matches-file-name", "Single top level class name [Woohoo] does not match file name")
LintError(1, 1, "class-name-matches-file-name", "Single top level class name [Woohoo] does not match file name")
))
}

@Test
fun testIgnoreKotlinScriptFiles() {
assertThat(ClassNameMatchesFileNameRule().lint(
"""
class B
""".trimIndent(),
fileName("A.kts")
)).isEmpty()
}

private fun fileName(fileName: String) = mapOf("file_path" to fileName)
}

0 comments on commit 1025de1

Please # to comment.