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

proposal: avoid_unnecessary_gesture_detectors #4872

Open
5 tasks done
navaronbracke opened this issue Feb 8, 2024 · 4 comments
Open
5 tasks done

proposal: avoid_unnecessary_gesture_detectors #4872

navaronbracke opened this issue Feb 8, 2024 · 4 comments
Labels
lint-proposal set-flutter Affects a rule in the recommended Flutter rule set status-pending type-enhancement A request for a change that isn't a bug

Comments

@navaronbracke
Copy link

navaronbracke commented Feb 8, 2024

avoid_unnecessary_gesture_detectors

I propose the following new lint rule, to avoid a pitfall with GestureDetectors.

Description

Do not create GestureDetectors without gesture handlers.

Details

Given a GestureDetector widget from package:flutter/widgets.dart, it is redundant if it has no handlers set at all.

Kind

It does guard against errors somewhat. I.e. placing redundant widgets, or when a GestureDetector that used to do something, now does nothing because its last handler was removed.

Bad Examples

GestureDetector(child: const SizedBox(width: 200, height: 200))

or even special casing the empty function (){}, if possible without side effects. Using a (){} is sometimes used as a placeholder so I'm not sure about that.

GestureDetector(
  onTap: () {},
  child: const SizedBox(width: 200, height: 200),
)

Good Examples

void _someVoidFunction() {}

GestureDetector(
  onTap: _someVoidFunction,
  child: const SizedBox(width: 200, height: 200),
)

Discussion

It somewhat overlaps with https://dart.dev/tools/linter-rules/avoid_unnecessary_containers but they target different widgets.

I originally filed a proposal here: flutter/flutter#99686

A real-world use case would have been some code I wrote a while back.
It had a GestureDetector wrapping around a very large child definition, which made me miss the fact that it had no handlers.
I was under the assumption of "This widget has a GestureDetector, why isn't it doing anything?", and it took me a bit to figure it out (in part due to the large widget tree obscuring my view somewhat).

Discussion checklist

  • List any existing rules this proposal modifies, complements, overlaps or conflicts with.
  • List any relevant issues (reported here, the SDK Tracker, or elsewhere).
  • If there's any prior art (e.g., in other linters), please add references here.
  • If this proposal corresponds to Effective Dart or Flutter Style Guide advice, please call it out. (If there isn't any corresponding advice, should there be?)
  • If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.
@github-actions github-actions bot added the set-flutter Affects a rule in the recommended Flutter rule set label Feb 8, 2024
@bwilkerson
Copy link
Member

@goderbauer

@goderbauer
Copy link
Contributor

It's true that an "empty" gesture detector is unnecessary. Since this is the second example of this pattern after avoid_unnecessary_containers I wonder if there is a more generic way to express this instead of creating specialist lints for each of these widgets?

@bwilkerson
Copy link
Member

We could potentially add one or more annotations that would let us cover this case.

@pq was looking at a similar situation awhile back. We had a client that wanted to be able to express things like "if this argument is provided then this other argument also has to be provided", or "these two arguments can't both be provided". I believe that the effort was dropped.

It got a bit messy because of all the combinations they wanted to have support for, but we could think about whether it's enough to be able to say something like "at least one of these arguments must be provided".

@srawlins
Copy link
Member

srawlins commented Feb 8, 2024

That would be #3063 and dart-lang/sdk#47712. I think each of those discussions ground to a halt. I think it's been pointed out that this or that proposal has a downside so it shouldn't be done. Lots of opinions. We should just pick something and implement it, IMO.

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lint-proposal set-flutter Affects a rule in the recommended Flutter rule set status-pending type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants