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

Jump to bottom

Added rule to detect commented code #1

Merged
merged 6 commits into from
Jul 3, 2020
Merged

Added rule to detect commented code #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3941e3e
Added rule to detect commented code
petertrr Jun 30, 2020
696056c
Added rule to detect commented code
petertrr Jul 2, 2020
6ed0819
Merge remote-tracking branch 'origin/master' into feature/rule-2.7-co…
petertrr Jul 2, 2020
10cb2b0
Fixes after merge
petertrr Jul 2, 2020
860a1e5
Fixes after review
petertrr Jul 2, 2020
7cae2df
Merge remote-tracking branch 'origin/feature/rule-2.7-commented-code'…
petertrr Jul 2, 2020
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 README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,3 +77,4 @@ To install git hooks using gradle run `gradle installGitHooks`.
|HEADER_MISSING_OR_WRONG_COPYRIGHT|Checks: copyright exists on top of file and is properly formatted (as a block comment). Fix: adds copyright if it is missing and required|
|HEADER_CONTAINS_DATE_OR_AUTHOR|Checks: header KDoc contains `@author` tag|
|HEADER_MISSING_IN_NON_SINGLE_CLASS_FILE|Check: warns if file with zero or >1 classes doesn't have header KDoc|
|COMMENTED_OUT_CODE|Check: warns if valid kotlin code is detected in commented blocks (both single-line and block comments)|
petertrr marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions diktat-rules/rules-config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,5 +173,10 @@
"name": "HEADER_MISSING_IN_LONG_FILE",
"enabled": true,
"configuration": {}
},
{
"name": "COMMENTED_OUT_CODE",
"enabled": true,
"configuration": {}
}
]
1 change: 1 addition & 0 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ enum class Warnings(private val id: Int, private val canBeAutoCorrected: Boolean
HEADER_MISSING_OR_WRONG_COPYRIGHT(37, true, "file header comment must include copyright information inside a block comment"),
HEADER_CONTAINS_DATE_OR_AUTHOR(38, false, "file header comment should not contain creation date and author name"),
HEADER_MISSING_IN_NON_SINGLE_CLASS_FILE(39, false, "files that contain multiple or no classes should contain description of what is inside of this file"),
COMMENTED_OUT_CODE(39, false, "you should not comment out code, use VCS to save it in history and delete this block"),
;

override fun ruleName(): String = this.name
Expand Down
4 changes: 3 additions & 1 deletion diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/DiktatRuleSetProvider.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.pinterest.ktlint.core.RuleSet
import com.pinterest.ktlint.core.RuleSetProvider
import org.cqfn.diktat.common.config.rules.RulesConfig
import org.cqfn.diktat.common.config.rules.RulesConfigReader
import org.cqfn.diktat.ruleset.rules.comments.CommentsRule

/**
* this constant will be used everywhere in the code to mark usage of Diktat ruleset
Expand All @@ -15,13 +16,14 @@ const val DIKTAT_RULE_SET_ID = "diktat-ruleset"
class RuleSetDiktat(val rulesConfig: List<RulesConfig>, vararg rules: Rule) : RuleSet(DIKTAT_RULE_SET_ID, *rules)

fun KtLint.Params.getDiktatConfigRules(): List<RulesConfig> {
return (this.ruleSets.find { it.id == DIKTAT_RULE_SET_ID } as RuleSetDiktat).rulesConfig?: listOf()
return (this.ruleSets.find { it.id == DIKTAT_RULE_SET_ID } as RuleSetDiktat).rulesConfig
}

class DiktatRuleSetProvider(private val jsonRulesConfig: String = "rules-config.json") : RuleSetProvider {
override fun get(): RuleSet {
return RuleSetDiktat(
RulesConfigReader().readResource(jsonRulesConfig)?: listOf(),
CommentsRule(),
KdocComments(),
KdocMethods(),
KdocFormatting(),
Expand Down
8 changes: 4 additions & 4 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/FileNaming.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class FileNaming : Rule("file-naming") {
val VALID_EXTENSIONS = listOf(".kt", ".kts")
}

private lateinit var confiRules: List<RulesConfig>
private lateinit var configRules: List<RulesConfig>
private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit)
private var fileName: String? = null
private var isFixMode: Boolean = false
Expand All @@ -37,7 +37,7 @@ class FileNaming : Rule("file-naming") {
autoCorrect: Boolean,
params: KtLint.Params,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) {
confiRules = params.getDiktatConfigRules()
configRules = params.getDiktatConfigRules()
fileName = params.fileName
emitWarn = emit
isFixMode = autoCorrect
Expand All @@ -52,7 +52,7 @@ class FileNaming : Rule("file-naming") {
if (fileName != null) {
val (name, extension) = getFileParts()
if (!name.isPascalCase() || !VALID_EXTENSIONS.contains(extension)) {
FILE_NAME_INCORRECT.warnAndFix(confiRules, emitWarn, isFixMode, "$name$extension", 0) {
FILE_NAME_INCORRECT.warnAndFix(configRules, emitWarn, isFixMode, "$name$extension", 0) {
// FixMe: we can add an autocorrect here in future, but is there any purpose to change file or class name?
}
}
Expand All @@ -66,7 +66,7 @@ class FileNaming : Rule("file-naming") {
if (classes.size == 1) {
val className = classes[0].getFirstChildWithType(IDENTIFIER)!!.text
if (className != fileNameWithoutSuffix) {
FILE_NAME_MATCH_CLASS.warnAndFix(confiRules, emitWarn, isFixMode, "$fileNameWithoutSuffix$fileNameSuffix vs $className", 0) {
FILE_NAME_MATCH_CLASS.warnAndFix(configRules, emitWarn, isFixMode, "$fileNameWithoutSuffix$fileNameSuffix vs $className", 0) {
// FixMe: we can add an autocorrect here in future, but is there any purpose to change file name?
}
}
Expand Down
28 changes: 14 additions & 14 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/IdentifierNaming.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class IdentifierNaming : Rule("identifier-naming") {
val BOOLEAN_METHOD_PREFIXES = setOf("has", "is")
}

private lateinit var confiRules: List<RulesConfig>
private lateinit var configRules: List<RulesConfig>
private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit)
private var isFixMode: Boolean = false

Expand All @@ -42,7 +42,7 @@ class IdentifierNaming : Rule("identifier-naming") {
params: KtLint.Params,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
confiRules = params.getDiktatConfigRules()
configRules = params.getDiktatConfigRules()
isFixMode = autoCorrect
emitWarn = emit

Expand Down Expand Up @@ -77,7 +77,7 @@ class IdentifierNaming : Rule("identifier-naming") {
if (!ONE_CHAR_IDENTIFIERS.contains(variableName!!.text)) {
// generally variables with prefixes are not allowed (like mVariable)
if (variableName.text.hasPrefix()) {
VARIABLE_HAS_PREFIX.warnAndFix(confiRules, emitWarn, isFixMode, variableName.text, variableName.startOffset) {
VARIABLE_HAS_PREFIX.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset) {
// FixMe: this correction should be done only after we checked variable case (below)
(variableName as LeafPsiElement).replaceWithText(variableName.text.removePrefix())
}
Expand All @@ -86,15 +86,15 @@ class IdentifierNaming : Rule("identifier-naming") {
// variable should not contain only one letter in it's name. This is a bad example: b512
// but no need to raise a warning here if length of a variable. In this case we will raise IDENTIFIER_LENGTH
if (variableName.text.containsOneLetterOrZero() && variableName.text.length > 1) {
VARIABLE_NAME_INCORRECT.warnAndFix(confiRules, emitWarn, isFixMode, variableName.text, variableName.startOffset) {
VARIABLE_NAME_INCORRECT.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset) {
}
}

// check for constant variables - check for val from companion object or on global file level
// it should be in UPPER_CASE, no need to raise this warning if it is one-letter variable
if ((node.isNodeFromCompanionObject() || node.isNodeFromFileLevel()) && node.isValProperty() && node.isConst()) {
if (!variableName.text.isUpperSnakeCase() && variableName.text.length > 1) {
CONSTANT_UPPERCASE.warnAndFix(confiRules, emitWarn, isFixMode, variableName.text, variableName.startOffset) {
CONSTANT_UPPERCASE.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset) {
(variableName as LeafPsiElement).replaceWithText(variableName.text.toUpperCase())
}
}
Expand All @@ -103,7 +103,7 @@ class IdentifierNaming : Rule("identifier-naming") {

// variable name should be in camel case. The only exception is a list of industry standard variables like i, j, k.
if (!variableName.text.isLowerCamelCase()) {
VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(confiRules, emitWarn, isFixMode, variableName.text, variableName.startOffset) {
VARIABLE_NAME_INCORRECT_FORMAT.warnAndFix(configRules, emitWarn, isFixMode, variableName.text, variableName.startOffset) {
// FixMe: cover fixes with tests
(variableName as LeafPsiElement).replaceWithText(variableName.text.toLowerCamelCase())
}
Expand All @@ -120,14 +120,14 @@ class IdentifierNaming : Rule("identifier-naming") {
private fun checkCLassNamings(node: ASTNode): List<ASTNode> {
val genericType: ASTNode? = node.getTypeParameterList()
if (genericType != null && !validGenericTypeName(genericType.text)) {
GENERIC_NAME.warnAndFix(confiRules, emitWarn, isFixMode, genericType.text, genericType.startOffset) {
GENERIC_NAME.warnAndFix(configRules, emitWarn, isFixMode, genericType.text, genericType.startOffset) {
// FixMe: should fix generic name here
}
}

val className: ASTNode? = node.getIdentifierName()
if (!(className!!.text.isPascalCase())) {
CLASS_NAME_INCORRECT.warnAndFix(confiRules, emitWarn, isFixMode, className.text, className.startOffset) {
CLASS_NAME_INCORRECT.warnAndFix(configRules, emitWarn, isFixMode, className.text, className.startOffset) {
(className as LeafPsiElement).replaceWithText(className.text.toPascalCase())
}
}
Expand All @@ -153,7 +153,7 @@ class IdentifierNaming : Rule("identifier-naming") {
?.text

if (superClassName != null && hasExceptionSuffix(superClassName) && !hasExceptionSuffix(classNameNode!!.text)) {
EXCEPTION_SUFFIX.warnAndFix(confiRules, emitWarn, isFixMode, classNameNode.text, classNameNode.startOffset) {
EXCEPTION_SUFFIX.warnAndFix(configRules, emitWarn, isFixMode, classNameNode.text, classNameNode.startOffset) {
// FixMe: need to add tests for this
(classNameNode as LeafPsiElement).replaceWithText(classNameNode.text + "Exception")
}
Expand All @@ -168,7 +168,7 @@ class IdentifierNaming : Rule("identifier-naming") {
val objectName: ASTNode? = node.getIdentifierName()
// checking object naming, the only extension is "companion" keyword
if (!(objectName!!.text.isPascalCase()) && objectName.text != "companion") {
OBJECT_NAME_INCORRECT.warnAndFix(confiRules, emitWarn, isFixMode, objectName.text, objectName.startOffset) {
OBJECT_NAME_INCORRECT.warnAndFix(configRules, emitWarn, isFixMode, objectName.text, objectName.startOffset) {
(objectName as LeafPsiElement).replaceWithText(objectName.text.toPascalCase())

}
Expand All @@ -185,7 +185,7 @@ class IdentifierNaming : Rule("identifier-naming") {
val enumValues: List<ASTNode> = node.getChildren(null).filter { it.elementType == ElementType.IDENTIFIER }
enumValues.forEach { value ->
if (!value.text.isUpperSnakeCase()) {
ENUM_VALUE.warnAndFix(confiRules, emitWarn, isFixMode, value.text, value.startOffset) {
ENUM_VALUE.warnAndFix(configRules, emitWarn, isFixMode, value.text, value.startOffset) {
// FixMe: add tests for this
(value as LeafPsiElement).replaceWithText(value.text.toUpperSnakeCase())
}
Expand All @@ -205,7 +205,7 @@ class IdentifierNaming : Rule("identifier-naming") {

// basic check for camel case
if (!functionName!!.text.isLowerCamelCase()) {
FUNCTION_NAME_INCORRECT_CASE.warnAndFix(confiRules, emitWarn, isFixMode, functionName.text, functionName.startOffset) {
FUNCTION_NAME_INCORRECT_CASE.warnAndFix(configRules, emitWarn, isFixMode, functionName.text, functionName.startOffset) {
// FixMe: add tests for this
(functionName as LeafPsiElement).replaceWithText(functionName.text.toLowerCamelCase())
}
Expand All @@ -217,7 +217,7 @@ class IdentifierNaming : Rule("identifier-naming") {
// if function has Boolean return type in 99% of cases it is much better to name it with isXXX or hasXXX prefix
if (functionReturnType != null && functionReturnType == PrimitiveType.BOOLEAN.typeName.asString()) {
if (!(BOOLEAN_METHOD_PREFIXES.any { functionReturnType.startsWith(it) })) {
FUNCTION_BOOLEAN_PREFIX.warnAndFix(confiRules, emitWarn, isFixMode, functionName.text, functionName.startOffset) {
FUNCTION_BOOLEAN_PREFIX.warnAndFix(configRules, emitWarn, isFixMode, functionName.text, functionName.startOffset) {
// FixMe: add agressive autofix for this
}
}
Expand Down Expand Up @@ -247,7 +247,7 @@ class IdentifierNaming : Rule("identifier-naming") {
isVariable: Boolean) {
nodes.forEach {
if (!(it.checkLength(2..64) || (ONE_CHAR_IDENTIFIERS.contains(it.text)) && isVariable)) {
IDENTIFIER_LENGTH.warn(confiRules, emitWarn, isFixMode, it.text, it.startOffset)
IDENTIFIER_LENGTH.warn(configRules, emitWarn, isFixMode, it.text, it.startOffset)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/KdocComments.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import org.jetbrains.kotlin.com.intellij.lang.ASTNode
*/
class KdocComments : Rule("kdoc-comments") {

private lateinit var confiRules: List<RulesConfig>
private lateinit var configRules: List<RulesConfig>
private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit)
private var isFixMode: Boolean = false

Expand All @@ -36,7 +36,7 @@ class KdocComments : Rule("kdoc-comments") {
params: KtLint.Params,
emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit
) {
confiRules = params.getDiktatConfigRules()
configRules = params.getDiktatConfigRules()
emitWarn = emit
isFixMode = autoCorrect

Expand Down Expand Up @@ -72,7 +72,7 @@ class KdocComments : Rule("kdoc-comments") {
val name = node.getIdentifierName()

if (modifier.isAccessibleOutside() && kDoc == null) {
warning.warn(confiRules, emitWarn, isFixMode, name!!.text, node.startOffset)
warning.warn(configRules, emitWarn, isFixMode, name!!.text, node.startOffset)
}
}
}