unnecessary_lambdas false negative

[FMorschel]

Describe the issue

If I have a Widget state similar to:

class _HomeState extends State<Home> with TickerProviderStateMixin {
  AnimationController get animationController => {...}

  @override
  Widget build(BuildContext context) {
    var c = animationController;
    return IconButton(
      icon: const Icon(Icons.settings),
      onPressed: () {
        c.toggle();
      },
    );
  }
}

I see no indication that I can use a tear-off like c.toggle. In cases where the variable is final it does trigger.

Originally I thought that class members could also be false-negatives but after a discussion on Discord (with @abitofevrything), I saw that if the class is extended and the field is replaced for a getter, then it can change and that would be a true negative.

Here is the old description.

If I have a Widget state similar to:

class _HomeState extends State<Home> with TickerProviderStateMixin {
  late final AnimationController animationController;

  @override
  void initState() {
    super.initState();
    animationController = AnimationController(
      duration: const Duration(milliseconds: 500),
      vsync: this,
    );
  }

  @override
  Widget build(BuildContext context) {
    return IconButton(
      icon: const Icon(Icons.settings),
      onPressed: () {
        animationController.toggle();
      },
    );
  }
}

I see no indication that I can use a tear-off like animationController.toggle. In cases where the variable is final/const or is a local variable where we can prove that it hasn't changed, this is a viable trigger.

To Reproduce
Sample above.

Expected behavior
unnecessary_lambdas should trigger for unchanging variables.

Additional context
Maybe there could also be a lint to warn users off of using tear-off whenever the variable might change. WDYT?

[FMorschel]

Just pinging you since I've changed a bit of the description here @pq.

[pq]

Thanks!

[Levi-Lesches]

I pointed this out in the discord thread as well, but causing the lint to trigger here could lead to subtle bugs.

If the variable is final, tear offs are okay. But since this variable is mutable, it could be changed by later code (even if that code doesn't exist today, it should be able to change without causing unexpected behavior).

If the variable is changed after the tear off is used, then the object that the method is called on changes, so there is an observable difference between a tear off and a closure.

There is a separate lint to consider making your variable final, which would then safely trigger this lint afterwards. I think it should probably stay that way since being final is the only thing that makes this safe

[FMorschel]

If things stay as they are today, you are probably right.

[...] there could also be a lint to warn users off of using tear-off whenever the variable might change.

If the above is ever created and active by default or some base set, then this issue should be considered.

---

I can use your logic to argue to simply stop using variable tear-offs today:

• If the variable for the above example is created final, it would trigger this lint today.
• The dev would probably use the quick-fix/run dart fix.
• In the future the variable changes to not be final (the opposite of your example) and changes value later in the function.
• It would cause inconsistencies as well.

---

So yes, I agree with you. But I still think this is worth fixing. I do agree we could wait for the new lint to be implemented to follow on this thread.

Edit

This way, the new lint would warn the user that the existing tear-off might not be doing what they thought.

Later I'll create the new issue for the proposal and mention this one there and vice-versa.

[Levi-Lesches]

Well, the key difference is that making a final variable suddenly mutable and changing it is a deliberate choice that requires thought and consideration that changing the value won't mess up other code. So if someone does that and now the tear-off uses the wrong value, that should be part of the decision when making the variable mutable (an alternative would be to make a copy of the final variable, and make the copy mutable).

But yes, if a lint like avoid_mutable_tearoffs existed, this could be pursued (though the linter would have to be sure this variable does not change to suggest the quick-fix).

[FMorschel]

Here it is #5058. Also, thanks for the naming suggestion.

[lrhn]

Maybe we should just stop recommending using tearoffs.
You can, but () => foo() is just as good.
(I hear it might even be more efficient for some compilers. Maybe because it doesn't have to worry about equality.)

That would mean just not having unnecessary_lambdas as a recommended lint. You can still choose it if you want to.

[FMorschel]

Hey! A little update, with final variables the lint is triggering now. Also, I found this issue wich is somewhat the opposite, similar to what I asked on #5058.