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

False positive on ComposableFunctionName when using single expression functions #35

Open
patrickhoette opened this issue Mar 25, 2024 · 3 comments

Comments

@patrickhoette
Copy link

I get a false positive on this rule when using the following syntax:

// Part of theme file
@Composable
fun AppTheme(content: @Composable () -> Unit) {
    // Theme setup stuff goes here
}

// False positive is on this function
@Preview
@Composable
private fun PreviewSomeComposable() = AppTheme {
    SomeComposable()
}

Seems like the rule doesn't detect that its still only a Unit return type so wants me to start the function with a lowercase letter.

@dimsuz
Copy link
Collaborator

dimsuz commented Mar 25, 2024

Yeah, we have discussed this here and the conclusion was that this is not something that's usually done in Сompose code, so we didn't bother.

Do you use this somewhere other than in @Preview functions?

If not maybe just add

ignoreAnnotated: [ 'Preview' ]

to this rule's config.

Detecting all cases without rewriting the rule to use type resolution can be tricky, I will check if this can be done without doing that.

@patrickhoette
Copy link
Author

patrickhoette commented Mar 26, 2024

I actually use this all the time as it's (IMO) nicer to read and also helps reduce the visual nesting a bit which Compose can get a bit heavy on. The example given in the comment you linked is actually one that I do use quite often. For now we settled on just disabling the rule entirely, but it is one I would like to have enforced. Unfortunately I have no statistics on how common using this syntax is so if it is deemed as uncommon enough to not bother handling it then we will have to discuss internally if we want to stop using this notation in Compose code so we can re-enable the rule.

If an easy solution can be found that would be great. Otherwise maybe I can find some time to look into rewriting the rule to use type resolution. I can't give a guarantee on that however as I don't have a whole lot of free time at the moment.

Thanks for looking into this issue.

@dimsuz
Copy link
Collaborator

dimsuz commented Mar 26, 2024

I see! I will try to improve this rule to account for this case and maybe rewrite to type resolution, it shouldn't be complex, it's just that I don't do that unless necessary - to keep the rules more lightweight and faster. No worries about the PR, but they are welcome :)

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

No branches or pull requests

2 participants