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

ComposeModifierReused false positive on a shadowed parameter name #268

Open
jzbrooks opened this issue Feb 5, 2024 · 5 comments
Open

ComposeModifierReused false positive on a shadowed parameter name #268

jzbrooks opened this issue Feb 5, 2024 · 5 comments
Labels
help wanted Extra attention is needed

Comments

@jzbrooks
Copy link
Contributor

jzbrooks commented Feb 5, 2024

I was tinkering with some ideas a couple days ago and noticed that this check fails the use of a locally scoped modifier instance if it has the same name as the composable parameter's modifier argument. I would expect the check to take scope into consideration and not fail when the modifier parameter name has been shadowed.

Notice below that the name modifier inside the Row (I wish I'd screencap'd the line numbers). Renaming that variable works around the problem.

Error

Screenshot 2024-02-02 at 10 12 50 AM

Rename Workaround

Screenshot 2024-02-02 at 10 13 27 AM

@ZacSweers
Copy link
Collaborator

I think your workaround is probably the right thing to do in this scenario if I'm honest. I'm not sure how easy it is to trace through shadowed names, and this is a compiler warning anyway :)

@jzbrooks
Copy link
Contributor Author

jzbrooks commented Feb 5, 2024

I'm assuming the local has a different UAST type than the parameter name or that maybe somehow PSI resolution might help do that tracing. I agree it isn't a huge deal, but it seems like a worthwhile endeavor to investigate if there's an easy solution.

I will try to do that soon.

@ZacSweers
Copy link
Collaborator

Yeah def open to a PR if you find a good way

@jzbrooks
Copy link
Contributor Author

I wonder if the path forward involves leveraging DataFlowAnalyzer to track uses of modifiers.

It's not much, but this detects some of the offending errors: main...jzbrooks:compose-lints:name-shadowing

If some of the test stubs were more robust (for example Modifier.then doesn't exist in the stubs so the analyzer doesn't know what to do with it). I wanted to spend more time with this over the past week, but I wasn't able to.

@ZacSweers
Copy link
Collaborator

I think the solution from #441 can help this case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants