Skip to content

Simplify EqualsOnSignatureLine rule#3224

Merged
schalkms merged 1 commit into
detekt:masterfrom
t-kameyama:simplify_EqualsOnSignatureLine
Nov 12, 2020
Merged

Simplify EqualsOnSignatureLine rule#3224
schalkms merged 1 commit into
detekt:masterfrom
t-kameyama:simplify_EqualsOnSignatureLine

Conversation

@t-kameyama

Copy link
Copy Markdown
Contributor

No description provided.

@codecov

codecov Bot commented Nov 11, 2020

Copy link
Copy Markdown

Codecov Report

Merging #3224 (519ea08) into master (cfde886) will decrease coverage by 0.01%.
The diff coverage is 71.42%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3224      +/-   ##
============================================
- Coverage     80.09%   80.08%   -0.02%     
- Complexity     2647     2648       +1     
============================================
  Files           440      440              
  Lines          8010     8010              
  Branches       1529     1528       -1     
============================================
- Hits           6416     6415       -1     
  Misses          775      775              
- Partials        819      820       +1     
Impacted Files Coverage Δ Complexity Δ
...rbosch/detekt/rules/style/EqualsOnSignatureLine.kt 77.77% <71.42%> (-11.12%) 6.00 <5.00> (+1.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 cfde886...ed43348. Read the comment docs.

@schalkms schalkms left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for this PR!
I’m not sure whether those changes really simplify the rule. Could you please elaborate why? The codecov report shows an increase in complexity of a very small margin.

@t-kameyama t-kameyama left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  1. I think it's simpler to go back from the equals token.
  2. It can be processed as Sequence.

val afterParams = function.valueParameterList?.siblings(forward = true, withItself = false) ?: return

// Collect tokens until we find the equals sign
val untilEquals = afterParams.takeWhile { it !== equals }.toList()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

toList() is useless. It should be processed as Sequence.

@schalkms

Copy link
Copy Markdown
Member

Thanks for the explanation. Let’s see what others think about the change.

@schalkms
schalkms merged commit 9737776 into detekt:master Nov 12, 2020
@t-kameyama
t-kameyama deleted the simplify_EqualsOnSignatureLine branch November 12, 2020 11:17
@arturbosch arturbosch added the housekeeping Marker for housekeeping tasks and refactorings label Nov 12, 2020
@arturbosch arturbosch added this to the 1.15.0 milestone Nov 12, 2020
arturbosch pushed a commit that referenced this pull request Dec 21, 2020
arturbosch pushed a commit that referenced this pull request Jan 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

housekeeping Marker for housekeeping tasks and refactorings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants