Add `avoid_redundant_argument_values`

[Piinks]

Describe the rule you'd like to see added and to what rule set

avoid_redundant_argument_values

This would help developers immensely to have this in the recommended set. In a recent user study, we found users were writing a bunch of code that reflected the default widget values that were already there.

Enabling the lint for these would help users not write code they don't need to.

Additional context

Transferred over from https://github.com/flutter/flutter/issues/138268

[goderbauer]

Enabling the lint for these would help users not write code they don't need to.

Ironically, people would only learn that they don't have to write this code after they have already written it since that's the only way to trigger the lint.

Maybe it will teach them what the defaults are so they avoid writing it again in the future, though?

Having said that, it would definitely result in less verbose code at the end of the day if defaults are omitted.

[devoncarew]

Docs are here: https://dart.dev/tools/linter-rules/avoid_redundant_argument_values. This rule has an associated quick fix.

[Piinks]

Thanks for the reference!

This rule has an associated quick fix.

But only if it is enabled and the analyzer flags it, correct?

[devoncarew]

But only if it is enabled and the analyzer flags it, correct?

Yes; I mention the fact that this has a quick fix since thats useful for rules we add to core sets - to help w/ more automatically updating existing code.

[pq]

But only if it is enabled and the analyzer flags it, correct?

Actually, I think you can do this and have it work even if the lint isn't enabled

dart fix --code=avoid_redundant_argument_values

(At least I think we landed that.)

Regardless though, I'm in favor of enabling this one in general. 👍

[natebosch]

This can make some breaking change migrations harder to have a backwards compatible rollout. I think I hit this trying to roll a package into flutter - the constraints are such that we need to make flutter forward compatible with breaking changes before we can release them in order to have the versions tested together internally. When rolling out a new mandatory argument, making it optional initially can allow flutter to pass a value and be prepared for when it's required, but this lint prevents it. I think I'll need an // Ignore in flutter to move forward because the PR to make it optional was not sufficient https://github.com/dart-lang/test/pull/2117

For general pub packages authors don't generally need to consider compatibility for usage across major versions, but for packages pulled into projects in any way other than a pub solve allowing cross-compatibility can be useful.

Last time we had a discussion about this internally @jacob314 pointed out that this lint would be very effective as a transient lint in your IDE when you introduce a new violation at a call site, but it's not useful as a lint blocking CI or surfacing as problems in distant call site when editing a signature.

[natebosch]

I don't think I can rule out a valid reason to duplicate the default value at the callsite. I don't see a good way to suppress this that isn't an // Ignore.

I do still think this would be a good candidate for a transient diagnostic. I think I lean away from adding this as a persistent lint, but I could be convinced. Do we have any estimates for how much code might be impacted? Do folks feel like duplicating a default value is never justified?

[goderbauer]

Do folks feel like duplicating a default value is never justified?

I do think there sometimes is justification for duplicating default values. I've often seen this done in tests to make perfectly clear that the test is supposed to test a particular setting even if it is the default one. It often improves the readability of the test.

I am also shying away from including this in any of our lint sets. It could be useful as a transient diagnostic, though.

[devoncarew]

@natebosch @goderbauer @lrhn - can you review this proposal and give an initial thumbs-up / thumbs-down (with some rational if useful)? Please ignore if you already have.

[lrhn]

We don't require subclasses to have the same default value if they override a method. A default value can be supplied by doing nullableParameter ??= DefaultValue instead of declaring it in the parameter list. All in all, a default value is more of a semantics property than a statically deductible one, and I fear the lint it's going to be too simplistic.

Maybe if the lint only applied to named non-nullable boolean parameters, it could be considered a reasonable heuristic. In general, I don't think it's a correct requirement to make.

[devoncarew]

Hi - after some discussion, I don't think we'll add this to the recommended lint set. There are use cases which benefit from this lint, but also use cases which this lint would prevent, and no clear reason (preventing errors, ...) to inhibit those use cases.

Thanks for the proposal!