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.

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

Fix PreferToOverPairSyntax exception #3046

Merged
merged 2 commits into from
Sep 7, 2020
Merged

Fix PreferToOverPairSyntax exception #3046

merged 2 commits into from
Sep 7, 2020

Conversation

schalkms
Copy link
Member

@schalkms schalkms commented Sep 1, 2020

Closes #3044

This leads to a crash if the valueArguments list contains only 1 item.

@codecov
Copy link

codecov bot commented Sep 1, 2020

Codecov Report

Merging #3046 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #3046   +/-   ##
=========================================
  Coverage     79.31%   79.31%           
  Complexity     2562     2562           
=========================================
  Files           432      432           
  Lines          7725     7725           
  Branches       1466     1466           
=========================================
  Hits           6127     6127           
  Misses          814      814           
  Partials        784      784           
Impacted Files Coverage Δ Complexity Δ
...ekt/rules/style/optional/PreferToOverPairSyntax.kt 78.57% <100.00%> (ø) 5.00 <0.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 c08de81...58082de. Read the comment docs.


report(CodeSmell(issue, Entity.from(expression),
message = "Pair is created by using the pair constructor. " +
"This can replaced by `$firstArg to $secondArg`"))
"This can replaced by `$arg`"))
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a test for this case? And assert the output too.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm currently waiting for the OP to provide more information and the code, which causes this issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

Please take a look at the linked issue. I couldn't reproduce the exception with the available information.

Copy link
Member

Choose a reason for hiding this comment

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

I'm also not sure how to test this case. Should we just go on and merge this and later add a testcase if the reporter responds?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I suggest to do that.

@arturbosch arturbosch added this to the 1.13.0 milestone Sep 7, 2020
@arturbosch arturbosch merged commit cb52dd2 into master Sep 7, 2020
@arturbosch arturbosch deleted the fix-#3044 branch September 7, 2020 20:00
schalkms added a commit that referenced this pull request Sep 8, 2020
Incorporates feedback from #3046
schalkms added a commit that referenced this pull request Sep 8, 2020
BraisGabin pushed a commit that referenced this pull request Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PreferToOverPairSyntax throws exceptions
3 participants