slackhq/compose-lints

ComposeModifierReused false positive on a shadowed parameter name

jzbrooks opened this issue · 4 comments

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

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 :)

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.

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

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.