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

New rule: Forbid nesting scope functions #4015

Closed
BraisGabin opened this issue Aug 7, 2021 · 4 comments
Closed

New rule: Forbid nesting scope functions #4015

BraisGabin opened this issue Aug 7, 2021 · 4 comments

Comments

@BraisGabin
Copy link
Member

Expected Behavior of the rule

non-compilant:

a?.let {
  a.also {
    // ...
  }
}

A rule to forbid nesting scope functions.

Context

Extracted from Kotlin Scope functions (emphasis mine):

Although the scope functions are a way of making the code more concise, avoid overusing them: it can decrease your code readability and lead to errors. Avoid nesting scope functions and be careful when chaining them: it's easy to get confused about the current context object and the value of this or it.

@marschwar
Copy link
Contributor

I like it but this will be very controversial and hard to turn on in any code base.
Maybe We should already think about config parameters such as depth of nesting and allowing functions references.

@BraisGabin
Copy link
Member Author

It should not be that controversial. The official documentation say that.

And about adopting the rule, I don't think it should be hard. You enable it, you recreate the baseline and you are good to go. Any new nesting scope function will be reported. How to handle the ones in the baseline is another thing.

I would vote to launch the rule without any nesting paramater. If there are complains and good reasons to add it we can add it later. But, right now, I can't think a good reason to allow 2 nesting scope functions and forbid 3.

@atulgpt
Copy link
Contributor

atulgpt commented Apr 12, 2023

Hi @BraisGabin IG NestedScopeFunctions does the same. It was fixed by #4780

@cortinico
Copy link
Member

Yup closing #4788

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

No branches or pull requests

4 participants