Skip to content

ExplicitCollectionElementAccessMethod rule update #5654

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

Merged
merged 5 commits into from
Jan 12, 2023

Conversation

atulgpt
Copy link
Contributor

@atulgpt atulgpt commented Dec 31, 2022

Fixes #5640

Fix false positive in ExplicitCollectionElementAccessMethod by
checking if any parameter(excluding the first) is spread parameter
if yes then return false as we can use access method syntax with
spread parameter
@vercel
Copy link

vercel bot commented Dec 31, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
detekt 🔄 Building (Inspect) Jan 2, 2023 at 0:02AM (UTC)

Co-authored-by: Brais Gabín <braisgabin@gmail.com>
@github-actions
Copy link

github-actions bot commented Jan 2, 2023

Warnings
⚠️ This PR is approved with no milestone set. If merged, it won't appear in the Detekt release notes.

Generated by 🚫 dangerJS against f9a6d58

@@ -96,6 +100,7 @@ class ExplicitCollectionElementAccessMethod(config: Config = Config.empty) : Rul
return function.isOperator
}

@Suppress("ReturnCount")
Copy link
Member

Choose a reason for hiding this comment

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

Why is this suppress needed? It wasn't needed before and you didn't add any new code to this function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was already giving the warning as it has three return statements(while only 2 are allowed). I suppressed the warning

@atulgpt
Copy link
Contributor Author

atulgpt commented Jan 5, 2023

Hi, @BraisGabin/ @cortinico this is good for a merge if we can close the comment thread at #5654 (comment). Also, want to know what is the process of the merge. Will one of the maintainers be merging this PR or if approved, the author can merge it?

@BraisGabin BraisGabin merged commit e152f88 into detekt:main Jan 12, 2023
@BraisGabin BraisGabin added this to the 1.23.0 milestone Jan 12, 2023
@atulgpt atulgpt deleted the fix-5640 branch May 1, 2023 09:50
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.

False positive on ExplicitCollectionElementAccessMethod with spread operator
3 participants