unused_element reports on unused parameters whose presence is idiomatic

[maxlapides]

Assumptions:

• Flutter 3.0
• user_super_parameters lint rule is enabled
• unused_element analyzer rule is enabled

This code chunk produces an analyzer issue:

class _MyWidget extends StatelessWidget {
  const _MyWidget({super.key});

  @override
  Widget build(BuildContext context) {
    return Container();
  }
}

"A value for optional parameter 'key' isn't ever given. Try removing the unused parameter."

But, when the same widget is not private, there is no issue:

class MyWidget extends StatelessWidget {
  const MyWidget({super.key});

  @override
  Widget build(BuildContext context) {
    return Container();
  }
}

I would like to keep the super.key on the private widgets, but I also want to keep the unused_element rule enabled. Is there any way to achieve this?

❯ flutter doctor -v
[✓] Flutter (Channel stable, 3.0.0, on macOS 12.3.1 21E258 darwin-arm, locale en-US)
    • Flutter version 3.0.0 at /Users/maxlapides/fvm/versions/3.0.0
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision ee4e09cce0 (2 days ago), 2022-05-09 16:45:18 -0700
    • Engine revision d1b9a6938a
    • Dart version 2.17.0
    • DevTools version 2.12.2

[✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
    • Android SDK at /Users/maxlapides/Library/Android/sdk
    • Platform android-31, build-tools 31.0.0
    • Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7772763)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 13.3.1)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • CocoaPods version 1.11.3

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2021.1)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7772763)

[✓] VS Code (version 1.67.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.40.0

[✓] Connected device (3 available)
    • iPhone 13 (mobile) • 8A362B17-9737-4B87-B0E4-77BF160D0A50 • ios            •
      com.apple.CoreSimulator.SimRuntime.iOS-15-4 (simulator)
    • macOS (desktop)    • macos                                • darwin-arm64   • macOS 12.3.1 21E258 darwin-arm
    • Chrome (web)       • chrome                               • web-javascript • Google Chrome 101.0.4951.64

[✓] HTTP Host Availability
    • All required HTTP hosts are available

• No issues found!

[bwilkerson]

@srawlins

[srawlins]

More or less a duplicate of https://github.com/dart-lang/sdk/issues/48401

[srawlins]

I would like to keep the super.key on the private widgets, but I also want to keep the unused_element rule enabled. Is there any way to achieve this?

No I don't think so. If you want to keep an unused parameter on your private widget, you will keep getting an unused_element report.

[srawlins]

How does this relate to user_super_parameters? If you remove the named parameter (and pass null directly) do you get a user_super_parameters report?

[maxlapides]

@srawlins It's related to the super parameters because this breaks unused_element:

class _MyWidget extends StatelessWidget {
  const _MyWidget({super.key});

  @override
  Widget build(BuildContext context) {
    return Container();
  }
}

But this does not:

class _MyWidget extends StatelessWidget {
  const _MyWidget({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Container();
  }
}

This doesn't really have much to do with the lint rule use_super_parameters, but rather just the new Dart 2.17 super parameters syntax.

[srawlins]

Oops, that second example should also have an unused_element report, since no argument is ever given.

[maxlapides]

It's super common in Flutter to have this key parameter on all widgets, regardless of whether the key parameter is ever used. It would be great to be able to disable unused_element just for the keys in Flutter.

[srawlins]

I can update the title of this issue to make that the ask.

[maxlapides]

@srawlins That would be great, thanks!

[orevial]

@srawlins @maxlapides this issue looks perfectly legit to me, why would you want to shut a warning about an unused element ?

Implementing super with key is legit on a public widget because you could want to export it as a library widget in which case you want to make sure you offer the possibility to give a key to anyone using your widget. However in the case of a private widget, if you are not using the key when calling your widget, why declare the key ?

To go further, I don't think that the new super parameters syntax has an issue, on the contrary I think that the old syntax should also trigger this unused_element warning on private widgets, to me that's the real issue 🙂

[goderbauer]

I agree with the previous comment. There's no reason to have the key parameter on a private widget if nobody is using it. If you need to pass the key, you can always add it to the private widget since you're in full control of the code.

For a public widget (that may get exported from a library you don't have control over) you want to make sure it can always take a key. If the key parameter is omitted, you'd have to ask the library author to release a new version with the key parameter added.

[maxlapides]

I'm not strongly opposed to removing the key parameters on the private widgets, but I do think there's a reasonable argument to be made that it is less work to have the super.key on all the private widgets because it's generated by a code snippet. Then, it's always available when you need it later, you don't need to go add it manually by editing a file that you may not have otherwise needed to touch.

[khobotin-elopage]

I agree with Max. It's common for widgets to have key parameter whether they are public or private. And it seems there have been an exception for the key parameter in unused_element rule because this

const _MyWidget({Key? key}) : super(key: key);

looks legit for it.

Also it's more likely to forget to add the key whenever I want to make my private widget a public.

[srawlins]

If there is not a report for unused_element for const _MyWidget({Key? key}) : super(key: key);, then I believe there is a bug in unused_element.

[orevial]

I understand your point of view although I disagree :

• IDE's snippets adding the key automatically when generating widgets is not a good reason IMO to implement an exception to this rule, the IDE snippet should follow language conventions and not the opposite
• I understand the argument about the risk of forgetting to add key on public widgets but there is a linter rule specifically designed for this case : use_key_in_widget_constructors which will make sure you never forget a key on public widgets 😉

[srawlins]

Generally speaking, the unused_* and unnecessary_* rules come from a policy of rejecting the "but I might need it later" argument in favor of highlighting all unused and unnecessary code. I think these rules will always report as much unused and unnecessary code as is technically correct.

If a dev or a team firmly believes some code should remain, however unused, they can disable the rules or suppress them by various means. If disabling "unused_element" across a codebase is too big a hammer, we may split a diagnostic out, as per https://github.com/dart-lang/sdk/issues/48401, for unused parameters, as this argument has come up a few times, "but I might need it later."

[rrousselGit]

Clearly, this behavior is controversial. The reactions speak for themselves I think. There's a decent amount of upvotes on both sides of the argument.

That's the issue.
This feature is controversial, yet it got added to one of the most used lints. As such, disabling the lint globally is unreasonable. Adding // ignore for all cases is unreasonable too.

This ends up negatively impacting a large number of people with no real solution.

Extracting this behavior into a new lint rule would lead to a happy ending for everyone. Those who like this can keep it. Those who don't can disable it.

[dnfield]

In https://github.com/flutter/tests/pull/172, I believe this is the cause of dart fix removing a parameter that never gets set, which results in compilation errors since the program is depending on the parameter to initialize a default value fo ra final variable.

[s0nerik]

Any updates?

The IntelliJ IDEA plugin generates a widget with super.key for extracted widgets (Cmd + Ctrl + W), which I need to manually adjust each time now to Key? key just to bypass the lint.

I'd love to have either a separate lint that ignores this specific case, as mentioned above, or at the very least have an option for the IDEA plugin to generate Key? key in the widget constructors.

[srawlins]

No updates.

I'd love to have either a separate lint that ignores this specific case, as mentioned above

That's probably the direction we'll go.

[srawlins]

Oops, I think we implemented that. You can now // ignore_for_file: unused_element_parameter or ignore unused_element_parameter in your analysis_options.yaml, and you're good to go... I don't think there is anything else to do here.

@dnfield the dart fix issue would be a separate issue.

[maxlapides]

@srawlins Great, unused_element_parameter is exactly what I'm looking for! Could you please show how I can use it in analysis_options.yaml to ignore this for the entire project? I can't seem to figure out the syntax :)

[srawlins]

No worries, I always forget it too :)

Here's the syntax: https://dart.dev/guides/language/analysis-options#ignoring-rules

[maxlapides]

@srawlins Strangely, I am getting this warning 'unused_element_parameter' isn't a recognized error code. dart(unrecognized_error_code):

analyzer:
  errors:
    unused_element_parameter: ignore

[srawlins]

Ah ok if that doesn't work then there is remaining work. I have an idea.

[xVemu]

Any news?

[srawlins]

No news.

[srawlins]

Had to revert the fix; broke various flutter things. See https://github.com/flutter/flutter/issues/131096

[antonxandre]

@srawlins Strangely, I am getting this warning 'unused_element_parameter' isn't a recognized error code. dart(unrecognized_error_code):

analyzer:
  errors:
    unused_element_parameter: ignore

Works to me with:

analyzer:
  errors:
    unused_element: ignore

[iska9der]

Don't worry, I'm just a reminder: we still need a solution to this problem

[kuyazee]

@srawlins Strangely, I am getting this warning 'unused_element_parameter' isn't a recognized error code. dart(unrecognized_error_code):

analyzer:
  errors:
    unused_element_parameter: ignore

Works to me with:

analyzer:
  errors:
    unused_element: ignore

This worked for me on flutter version 3.19.4

[lore-co]

Any news?

[srawlins]

The splitting CL is reverted; re-opening.

[athomas]

3.6.0-149.0.dev includes the fix, in case someone wants to try it. However, it's still reverted on main so the next dev release will likely not contain the fix anymore.

[srawlins]

@iinozemtsev this is still reverted on main, right? re-open?

[iinozemtsev]

oh, I didn't expect that pushing into a fork can close the issue, sorry!

[srawlins]

Oh, I see, haha! Thanks much!