proposal: `unnecessary_extension_name`

[bwilkerson]

unnecessary_extension_name

Description

Private extensions don't need an explicit name.

Details

Extensions that are private to a library don't need an explicit name unless that name is going to be used in an extension override. The name should be omitted for brevity.

Kind

Style

Bad Examples

extension _privateExtension on String {}

Good Examples

extension on String {}

Discussion

Add any other motivation or useful context here.

Discussion checklist

• [x] List any existing rules this proposal modifies, complements, overlaps or conflicts with.
• [x] List any relevant issues (reported here, the SDK Tracker, or elsewhere).
• [x] If there's any prior art (e.g., in other linters), please add references here.
  • There are several other lints and warnings that help users find and remove unnecessary text.
• [x] 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?)
• [x] If this proposal is motivated by real-world examples, please provide as many details as you can. Demonstrating potential impact is especially valuable.
  • There were examples of private extensions in the analyzer packages. See https://dart-review.googlesource.com/c/sdk/+/347652 for one example.

[parlough]

This makes sense to me.

"unless that name is going to be used in an extension override"

How do you see this as being determined? It not being currently used that way in the library or the extension doesn't shadow any members?

[bwilkerson]

I would expect that the lint would not produce a diagnostic if there is already a use of the name in the library, but that the lint would produce a diagnostic otherwise. It's up to the developer to figure out that they just haven't yet added the intended extension override and to ignore the lint until they do.

[pq]

This seems like a great candidate for a core or recommended lint but would be curious to get some more input.

/fyi @goderbauer @lrhn @munificent @jakemac53 @natebosch @devoncarew

[jakemac53]

I have no particular opinion on this one, I do give extensions names (even private ones), but tbh I didn't even realize I could omit the name 🤣. At the same time I don't think there is harm in the name being there 🤷‍♂️ .

So, this would have been good for me as I would have discovered I could leave them unnamed, and probably will do so in the future. But I am not sure if it meets the bar of "nobody should give extensions private names" either.

If we want to push for it in the name of consistency though, I wouldn't be opposed.

[srawlins]

At the same time I don't think there is harm in the name being there 🤷‍♂️ .

This is a particularly amusing situation, because I am surprised when seeing a private extension name (whereas you are surprised by an omitted name). In my mind a lot of lint rules help keep code consistent so that any inconsistency, which leaps out at me, must be there for a reason.

If I see final x = 7; final List<num> list = [x];, I think "whoa, why is that local type there?" And then maybe it turns out that a double is added list later, and it makes sense. And if I see final x = 7; List<int> list = [x];, I think "whoa, why is that local type there?" And I think surely, there must be a reason. And I bang my head and think I must be missing something, but then if I try removing the type, it turns out it didn't need to be there, I long to get back the 5 minutes I wasted trying to solve a Dart mystery with no mystery. The omit_local_variable_types rule helps remove surprising code like this.

Similarly, when I recently saw a privately-named extension in our code (we use a lot of extensions), my first thought was, "whoa, do we use an explicit extension override somewhere??? we never use those." And I look and look, and no, it was unnecessarily named.

[lrhn]

The name is unnecessary, and as such the very general rule of "don't write unnecessary things" can be applied.

Another exception would be if the extension contains static methods that are alled from outside of the declaration. Which means it's more of an "if the name is actually used", with static member access and extension override probably being the only two ways you can use an extension name.

But it's nothing else than unnecessary, so this is only about consistency. There is no harm in writing a name. Sometimes a name is a good documentation, sometimes it's an irrelevant noise. In this case, the unnecessary name also private, making it completely irrelevant to anyone outside of that particular library, so I have a very hard time caring what people choose to do.

If it's because people don't know you can omit the name, and we want to teach them using lints ... that still only matters if we care what they do.

So it's a style choice with only a marginal leaning towards one side - it is unnecessary, technically.

If someone does want the lint, for consistency, that's fine.

I probably won't object to the lint being recommended, but I'm also not going to make any effort myself for getting it recommended. I really don't care what people do here, in their own libraries.

(Consistency, by itself, is not a goal for me. I don't believe it's a goal for the recommended lints either. Avoiding something unnecessary is a better reason to recommend.)

[goderbauer]

I also have no opinion on this one and I also thought the name was required :)

I am not opposed to including it in recommended - under the umbrella of enforcing consistency and/or avoiding unnecessary things.

[eernstg]

Here are a couple of reasons why it might be reasonable to say that a private extension name isn't necessary: If it is used to provide access to a set of static member declarations in the declaring library (only) then perhaps those static members could just be top-level declarations with a suitable prefix as the first part of the name (_E.myStaticMethod() becomes _E_myStaticMethod()).

Similarly, if the private extension name is used to perform explicitly resolved extension member invocations then it is presumably because some other extension members have been imported, and they have some name conflicts. This implies that the name conflict would exist every single time said extension member is used. So maybe that extension member should just have a different name.

All these potential renamings shouldn't be particularly breaking, because all usages must occur in the same library.

It may be inconvenient in some rare cases to remove or avoid introducing a private extension name (and they'd use // ignore). But it sounds like, in general, the private extension name might just as well not exist, and other names would be adjusted as needed.

[natebosch]

+1 for the lint and +1 for putting it in recommended.

If we had an implementation for shown-once lints I'd be happy to have this enabled using that mechanism. Given the current capabilities I think our users would be best served having the lint in the recommended set.

[munificent]

I'm agnostic and happy to defer to the stronger opinions of others.