Skip to content

Commit

Permalink
Fixed PARAMETER_NAME_IN_OUTER_LAMBDA (#1882)
Browse files Browse the repository at this point in the history
- fixed positive `PARAMETER_NAME_IN_OUTER_LAMBDA`
- suppress false positive `PARAMETER_NAME_IN_OUTER_LAMBDA`
  • Loading branch information
nulls authored Dec 20, 2023
1 parent a0c564e commit de959e2
Show file tree
Hide file tree
Showing 19 changed files with 64 additions and 50 deletions.
2 changes: 1 addition & 1 deletion diktat-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@
enabled: true
# Checks that outer lambda has explicit parameter name
- name: PARAMETER_NAME_IN_OUTER_LAMBDA
enabled: false
enabled: true
# Checks that property in constructor doesn't contain comment
- name: KDOC_NO_CONSTRUCTOR_PROPERTY
enabled: true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ fun interface ResourceReader : Function1<String, Path?> {
.getResource(resourceName)
?.toURI()
?.toPath()
.also {
if (it == null || !it.isRegularFile()) {
.also { path ->
if (path == null || !path.isRegularFile()) {
log.error { "Not able to find file for running test: $resourceName" }
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ class EnumNamesSymbolProcessor(

private fun KSAnnotation.getArgumentValue(argumentName: String): String = arguments
.singleOrNull { it.name?.asString() == argumentName }
.let {
requireNotNull(it) {
.let { argument ->
requireNotNull(argument) {
"Not found $argumentName in $this"
}
}
.value
?.let { it as? String }
.let {
requireNotNull(it) {
.let { argumentValue ->
requireNotNull(argumentValue) {
"Not found a value for $argumentName in $this"
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,9 @@ class PackageNaming(configRules: List<RulesConfig>) : DiktatRule(
// all words should be in a lower case (lower case letters/digits/underscore)
wordsInPackageName
.filter { word -> word.text.hasUppercaseLetter() }
.forEach {
PACKAGE_NAME_INCORRECT_CASE.warnAndFix(configRules, emitWarn, isFixMode, it.text, it.startOffset, it) {
it.toLower()
.forEach { word ->
PACKAGE_NAME_INCORRECT_CASE.warnAndFix(configRules, emitWarn, isFixMode, word.text, word.startOffset, word) {
word.toLower()
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ class CommentsFormatting(configRules: List<RulesConfig>) : DiktatRule(
}
}

@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
private fun checkBlankLineAfterKdoc(node: ASTNode) {
commentType.forEach {
val kdoc = node.getFirstChildWithType(it)
Expand Down Expand Up @@ -138,8 +139,8 @@ class CommentsFormatting(configRules: List<RulesConfig>) : DiktatRule(
else -> null
}
comment?.let {
IF_ELSE_COMMENTS.warnAndFix(configRules, emitWarn, isFixMode, it.text, node.startOffset, node) {
moveCommentToElse(node, elseBlock, elseKeyWord, it)
IF_ELSE_COMMENTS.warnAndFix(configRules, emitWarn, isFixMode, comment.text, node.startOffset, node) {
moveCommentToElse(node, elseBlock, elseKeyWord, comment)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,10 @@ class BracesInConditionalsAndLoopsRule(configRules: List<RulesConfig>) : DiktatR
block.statements.size == 1 &&
block.findChildrenMatching { it.isPartOfComment() }.isEmpty()
}
.forEach {
.forEach { block ->
NO_BRACES_IN_CONDITIONALS_AND_LOOPS.warnAndFix(configRules, emitWarn, isFixMode,
"WHEN", it.node.startOffset, it.node) {
it.astReplace(it.firstStatement!!.node.psi)
"WHEN", block.node.startOffset, block.node) {
block.astReplace(block.firstStatement!!.node.psi)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ class EnumsSeparated(configRules: List<RulesConfig>) : DiktatRule(
if (enumEntries.isEmpty() || (isEnumSimple(enumEntries) && isEnumOneLine(enumEntries))) {
return
}
enumEntries.forEach {
if (!it.treeNext.isWhiteSpaceWithNewline()) {
enumEntries.forEach { enumEntry ->
if (!enumEntry.treeNext.isWhiteSpaceWithNewline()) {
ENUMS_SEPARATED.warnAndFix(configRules, emitWarn, isFixMode, "enum entries must end with a line break",
it.startOffset, it) {
it.appendNewline()
enumEntry.startOffset, enumEntry) {
enumEntry.appendNewline()
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
return null
}

@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
private fun checkArgumentsList(node: ASTNode, configuration: LineLengthConfiguration): LongLineFixableCases {
node.findParentNodeWithSpecificType(WHEN_ENTRY)?.let {
it.findChildByType(BLOCK)?.run {
Expand Down Expand Up @@ -476,7 +477,11 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
/**
* Finds the first dot or safe access closer to the separator
*/
@Suppress("MAGIC_NUMBER", "MagicNumber")
@Suppress(
"MAGIC_NUMBER",
"MagicNumber",
"PARAMETER_NAME_IN_OUTER_LAMBDA"
)
private fun searchRightSplitBeforeDotOrSafeAccess(
parent: ASTNode,
configuration: LineLengthConfiguration,
Expand Down Expand Up @@ -819,8 +824,8 @@ class LineLength(configRules: List<RulesConfig>) : DiktatRule(
node.appendNewlineMergingWhiteSpace(commaSplit.treeNext, commaSplit.treeNext)
}
}
node.getFirstChildWithType(RPAR)?.let {
if (positionByOffset(it.treePrev.startOffset).second + it.treePrev.text.length - offset > lineLength * lineNumber && listComma.isNotEmpty()) {
node.getFirstChildWithType(RPAR)?.let { child ->
if (positionByOffset(child.treePrev.startOffset).second + child.treePrev.text.length - offset > lineLength * lineNumber && listComma.isNotEmpty()) {
listComma.last().first.let {
node.appendNewlineMergingWhiteSpace(it.treeNext, it.treeNext)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class RangeConventionalRule(configRules: List<RulesConfig>) : DiktatRule(
}
}

@Suppress("TOO_MANY_LINES_IN_LAMBDA")
@Suppress("TOO_MANY_LINES_IN_LAMBDA", "PARAMETER_NAME_IN_OUTER_LAMBDA")
private fun handleQualifiedExpression(node: ASTNode) {
(node.psi as KtDotQualifiedExpression).selectorExpression?.node?.let {
if (it.findChildByType(REFERENCE_EXPRESSION)?.getIdentifierName()?.text == "rangeTo") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ class FileStructureRule(configRules: List<RulesConfig>) : DiktatRule(
) {
val imports = node.getChildren(TokenSet.create(IMPORT_DIRECTIVE)).toList()
// importPath can be null if import name cannot be parsed, which should be a very rare case, therefore !! should be safe here
@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
imports
.filter {
(it.psi as KtImportDirective).importPath!!.run {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,10 @@ class IndentationRule(configRules: List<RulesConfig>) : DiktatRule(
return true
}
}
.forEach {
WRONG_INDENTATION.warnAndFix(configRules, emitWarn, isFixMode, "tabs are not allowed for indentation", it.startOffset + it.text.indexOf(TAB), it) {
(it as LeafPsiElement).rawReplaceWithText(it.text.replace(TAB.toString(), configuration.indentationSize.spaces))
.forEach { whiteSpaceNode ->
WRONG_INDENTATION.warnAndFix(configRules, emitWarn, isFixMode, "tabs are not allowed for indentation",
whiteSpaceNode.startOffset + whiteSpaceNode.text.indexOf(TAB), whiteSpaceNode) {
(whiteSpaceNode as LeafPsiElement).rawReplaceWithText(whiteSpaceNode.text.replace(TAB.toString(), configuration.indentationSize.spaces))
}
}
return isFixMode // true if we changed all tabs to spaces
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ class WhiteSpaceRule(configRules: List<RulesConfig>) : DiktatRule(
) {
// note: the conditions in the following `if`s cannot be collapsed into simple conjunctions
if (isFromLambdaAsArgument) {
@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
val isFirstArgument = node
.parent { it.elementType == VALUE_ARGUMENT }
.let { it?.prevSibling { prevNode -> prevNode.elementType == COMMA } == null }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,15 @@ class SmartCastRule(configRules: List<RulesConfig>) : DiktatRule(
}

val groupedExprs: MutableMap<KtNameReferenceExpression, List<KtNameReferenceExpression>> = mutableMapOf()
isExpr.forEach {
isExpr.forEach { ref ->
val list: MutableList<KtNameReferenceExpression> = mutableListOf()
asExpr.forEach { asCall ->
if (asCall.node.findParentNodeWithSpecificType(IF)
== it.node.findParentNodeWithSpecificType(IF)) {
== ref.node.findParentNodeWithSpecificType(IF)) {
list.add(asCall)
}
}
groupedExprs[it] = list
groupedExprs[ref] = list
}
return groupedExprs
}
Expand Down Expand Up @@ -187,25 +187,26 @@ class SmartCastRule(configRules: List<RulesConfig>) : DiktatRule(
private fun checkAsExpressions(asList: List<ASTNode>, blocks: List<IsExpressions>) {
val asExpr: MutableList<AsExpressions> = mutableListOf()

@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
asList.forEach {
val split = it.text.split("as").map { part -> part.trim() }
asExpr.add(AsExpressions(split[0], split[1], it))
}

val exprToChange = asExpr.filter {
val exprToChange = asExpr.filter { asCall ->
blocks.any { isExpr ->
isExpr.identifier == it.identifier &&
isExpr.type == it.type
isExpr.identifier == asCall.identifier &&
isExpr.type == asCall.type
}
}

if (exprToChange.isNotEmpty()) {
exprToChange.forEach {
SMART_CAST_NEEDED.warnAndFix(configRules, emitWarn, isFixMode, "${it.identifier} as ${it.type}", it.node.startOffset,
it.node) {
val dotExpr = it.node.findParentNodeWithSpecificType(DOT_QUALIFIED_EXPRESSION)!!
exprToChange.forEach { asCall ->
SMART_CAST_NEEDED.warnAndFix(configRules, emitWarn, isFixMode, "${asCall.identifier} as ${asCall.type}", asCall.node.startOffset,
asCall.node) {
val dotExpr = asCall.node.findParentNodeWithSpecificType(DOT_QUALIFIED_EXPRESSION)!!
val afterDotPart = dotExpr.text.split(".")[1]
val text = "${it.identifier}.$afterDotPart"
val text = "${asCall.identifier}.$afterDotPart"
dotExpr.treeParent.addChild(KotlinParser().createNode(text), dotExpr)
dotExpr.treeParent.removeChild(dotExpr)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ class AccurateCalculationsRule(configRules: List<RulesConfig>) : DiktatRule(
?: false
}
?: false

@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
private fun KtDotQualifiedExpression.isComparisonWithAbs() =
takeIf {
it.selectorExpression.run {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class UselessSupertype(configRules: List<RulesConfig>) : DiktatRule(
(if (classBody.treeParent.hasChildOfType(CLASS_KEYWORD)) it.findChildByType(MODIFIER_LIST)!!.hasChildOfType(OPEN_KEYWORD) else true) &&
it.getIdentifierName()!!.text in methodsName
}
@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
overrideFunctions.forEach {
functionNameMap.compute(it.getIdentifierName()!!.text) { _, oldValue -> (oldValue ?: 0) + 1 }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class CompactInitialization(configRules: List<RulesConfig>) : DiktatRule(
* Check property's initializer: if it is a method call, we find all consecutive statements that are this property's
* fields accessors and wrap them in an `apply` function.
*/
@Suppress("UnsafeCallOnNullableType")
@Suppress("UnsafeCallOnNullableType", "PARAMETER_NAME_IN_OUTER_LAMBDA")
private fun handleProperty(property: KtProperty) {
property.run {
val propertyName = name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,15 @@ class DataClassesRule(configRules: List<RulesConfig>) : DiktatRule(
isValue() || isAnnotation() || isInterface() || isData() ||
isSealed() || isInline() || isAbstract() || isEnum()

@Suppress("UnsafeCallOnNullableType")
@Suppress("UnsafeCallOnNullableType", "PARAMETER_NAME_IN_OUTER_LAMBDA")
private fun areGoodAccessors(accessors: List<ASTNode>): Boolean {
accessors.forEach {
if (it.hasChildOfType(BLOCK)) {
val block = it.getFirstChildWithType(BLOCK)!!

return block
.getChildren(null)
.filter { expr -> expr.psi is KtExpression }
.count() <= 1
.count { expr -> expr.psi is KtExpression } <= 1
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ fun ASTNode.isEol() = parent(false) { it.treeNext != null }?.isFollowedByNewline
* Same is true also for semicolons in some cases.
* Therefore, to check if they are followed by newline we need to check their parents.
*/
@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
fun ASTNode.isFollowedByNewline() =
parent(false) { it.treeNext != null }?.let {
val probablyWhitespace = it.treeNext
Expand Down Expand Up @@ -312,11 +313,11 @@ fun ASTNode.findChildBefore(beforeThisNodeType: IElementType, childNodeType: IEl
.find { it.elementType == beforeThisNodeType }
getChildren(null)
.toList()
.let {
.let { children ->
anchorNode?.run {
it.subList(0, it.indexOf(anchorNode))
children.subList(0, children.indexOf(anchorNode))
}
?: it
?: children
}
.reversed()
.find { it.elementType == childNodeType }
Expand Down Expand Up @@ -1026,9 +1027,9 @@ private fun <T> Sequence<T>.takeWhileInclusive(pred: (T) -> Boolean): Sequence<T
}

private fun Collection<KtAnnotationEntry>.containSuppressWithName(name: String) =
this.any {
it.shortName.toString() == (Suppress::class.simpleName) &&
(it.valueArgumentList
this.any { annotationEntry ->
annotationEntry.shortName.toString() == (Suppress::class.simpleName) &&
(annotationEntry.valueArgumentList
?.arguments
?.any { annotation -> annotation.text.trim('"') == name }
?: false)
Expand Down Expand Up @@ -1109,9 +1110,9 @@ private fun ASTNode.calculateLineNumber() = getRootNode()
.indexOfFirst {
it > startOffset
}
.let {
require(it >= 0) { "Cannot calculate line number correctly, node's offset $startOffset is greater than file length ${getRootNode().textLength}" }
it + 1
.let { index ->
require(index >= 0) { "Cannot calculate line number correctly, node's offset $startOffset is greater than file length ${getRootNode().textLength}" }
index + 1
}

private fun ASTNode.hasExplicitIt(): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ internal class ValueParameterListChecker(configuration: IndentationConfig) : Cus
.elementType
.let { it == VALUE_PARAMETER_LIST || it == VALUE_ARGUMENT_LIST } &&
whiteSpace.siblings(forward = false, withItself = false).none { it is PsiWhiteSpace && it.textContains('\n') } &&
@Suppress("PARAMETER_NAME_IN_OUTER_LAMBDA")
// no need to trigger when there are no more parameters in the list
whiteSpace.siblings(forward = true, withItself = false).any {
it.node.elementType.run { this == VALUE_ARGUMENT || this == VALUE_PARAMETER }
Expand Down

0 comments on commit de959e2

Please sign in to comment.