`Convert to wildcard variable` fix for `unnecessary_underscores` could be applied automatically

[FMorschel]

Repro:

void foo(void Function(int) fn) {}

void bar() {
  foo((__) {}); `unnecessary_underscores`
}

For the above case, we could apply this automatically.

CC @bwilkerson

[bwilkerson]

By "automatically" do you mean "by running dart fix"?

[FMorschel]

Yes, exactly (CorrectionApplicability.automatically).

This fix is also applied today for one other case:

void f() {
  var x = ''; // unused_local_variable
}

But I'd say this case could be excluded from dart fix. I'm open to adding both if you are, just wasn't so sure about this one.

[bwilkerson]

I wouldn't make the second case automatic either. Automatic fixes are also often run on save, and a user could easily want to save their work before adding code that references x. It's less likely that they're planning on referencing __ because of common conventions, but it's only a convention, so it's less clear in that case.

So, it's definitely not a good idea in the second case and questionable in the first case.

Do we actually have a way to enable a fix for automatic application in some cases and not others? I think the answer is 'no', but haven't checked. If not, I'm not convinced that this one case justified the complexity needed in order to support it.

[FMorschel]

Do we actually have a way to enable a fix for automatic application in some cases and not others? I think the answer is 'no', but haven't checked. If not, I'm not convinced that this one case justified the complexity needed in order to support it.

I don't think we have any precedence, but I'd simply make the ResolvedCorrectionProducer.applicability a final field instead of a getter. I don't believe it would break any existing code, but if we believe that, I can also make this fix have two subclasses, one for each case. We can differ the fixKind too if that is what you're worried about.

So, it's definitely not a good idea in the second case and questionable in the first case.

Agreed. I just suggested this because the current fix can be a bit "useless" if it is not automatic. For VS Code at least, users have to open every one of the files where they have this diagnostic and use the context menu to apply the fix (for each case since it is currently singleLocation). So it is almost as fast (usually slower) as simply removing the second _ (a bit more useful if there are many _, but still not that different).

[FMorschel]

I found one case that varies by diagnostic on whether to apply in bulk or not:

sdk/pkg/analysis_server/lib/src/services/correction/dart/use_curly_braces.dart

Lines 17 to 31 in 83b2c5d

	class UseCurlyBraces extends ParsedCorrectionProducer {
	@override
	final CorrectionApplicability applicability;
	UseCurlyBraces({required super.context})
	: applicability = CorrectionApplicability.acrossFiles;
	/// Create an instance that is prevented from being applied automatically in
	/// bulk.
	///
	/// This is used in places where "Use Curly Braces" is a valid manual fix, but
	/// not clearly the only/correct fix to apply automatically, such as the
	/// `always_put_control_body_on_new_line` lint.
	UseCurlyBraces.nonBulk({required super.context})
	: applicability = CorrectionApplicability.acrossSingleFile;

[bwilkerson]

I just suggested this because the current fix can be a bit "useless" if it is not automatic.

I'd agree if I thought that having the diagnostic being reported in lots of places was a common case, but I'm not convinced that it's all that common.

So it is almost as fast (usually slower) as simply removing the second _ ...

I suspect that the biggest value is that the user doesn't need to try to compute how many underscores to remove.

But given that it's easy to support, I have no problem allowing the fix to be automatically applied in the case of an unreferenced positional parameter in a closure (where 'unreferenced' includes not being referenced in doc comments) where the name consists of two or more underscores.

[FMorschel]

I'll see about handling this when I have some time, then. Thanks! Of course, if anyone else wants to take over, feel free to do so, just please comment below.

[FMorschel]

Here is the WIP CL for this issue https://dart-review.googlesource.com/c/sdk/+/464880.

It should be fine to add it, but considering the false-positive we found at #61558, I'll wait for that CL to land before this one.