Skip to content

Implement LambdaParameterNaming#4147

Merged
BraisGabin merged 1 commit intomainfrom
issue-4131
Nov 6, 2021
Merged

Implement LambdaParameterNaming#4147
BraisGabin merged 1 commit intomainfrom
issue-4131

Conversation

@BraisGabin
Copy link
Member

This implements part of #4131

data class D(val i: Int, val j: Int)
fun doStuff() {
val (_, HOLY_GRAIL) = D(5, 4)
emptyMap<String, String>().forEach { _, V -> println(V) }
Copy link
Member Author

Choose a reason for hiding this comment

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

Why was this here? Should we support this? Is there any documentation that talks about this case?

Copy link
Member

Choose a reason for hiding this comment

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

Is the problem about why we allow the capital V by default?

It feels like this is a test case that verifying that we do not have any naming requirement for lambda parameters 🙈 . I think your change makes sense.

@codecov
Copy link

codecov bot commented Oct 2, 2021

Codecov Report

Merging #4147 (56d008b) into main (e07c297) will decrease coverage by 0.00%.
The diff coverage is 88.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #4147      +/-   ##
============================================
- Coverage     83.44%   83.44%   -0.01%     
- Complexity     3177     3185       +8     
============================================
  Files           465      466       +1     
  Lines          9104     9128      +24     
  Branches       1775     1777       +2     
============================================
+ Hits           7597     7617      +20     
- Misses          572      576       +4     
  Partials        935      935              
Impacted Files Coverage Δ
...tlab/arturbosch/detekt/rules/naming/NamingRules.kt 88.23% <50.00%> (-5.42%) ⬇️
...bosch/detekt/rules/naming/LambdaParameterNaming.kt 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e07c297...56d008b. Read the comment docs.

@chao2zhang chao2zhang added this to the 1.19.0 milestone Oct 3, 2021
Comment on lines +30 to +31
override fun visitParameter(parameter: KtParameter) {
val parameters: List<KtNamedDeclaration> = parameter.destructuringDeclaration?.entries ?: listOf(parameter)
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I can tell there is nothing restricting this to lambda expressions.

Should this fail or pass?

it("ignores invalid function parameters") {
    val code = """
        fun foo(BAR: String) = Unit
    """
    assertThat(LambdaParameterNaming().compileAndLint(code))
        .isEmpty()
}

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, there is nothing restrictin this to lambda. But all the naming rules are kind of strange. They aren't called by the normal rule engine. They are called from detekt-rules-naming/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/naming/NamingRules.kt. And it only cal this function when the parameter is from a lambda. You can check my changes in that file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants