non_constant_identifier_names should allow "__"

[nex3]

The non_constant_identifier_names lint currently forbids the name __ (two underscores). This name is regularly used when a class requires multiple private constructors, especially when working around dart-lang/language#622.

[pq]

@nex3: is this ask limited to named constructors?

[nex3]

No, I think it should be allowed for all identifiers.

[pq]

Ok, thanks!

[pq]

@bwilkerson: we currently make an exception, allowing just underscores for FormalParameterLists:

  listen((__) {}); // OK!

The proposal here is to allow it for all identifiers.

Curious if you have any reservations. (I don't feel strongly.)

@munificent: any opinions?

[bwilkerson]

We originally decided to allow underscores for parameters because it was (and probably still is) a convention to use underscores for parameters that are expected to be unreferenced. (I don't think we check that they really are unreferenced, but we could.) I can't think of a use case for defining other variables that are intended to be unreferenced, so in the absence of a new reason for allowing underscores I'd be disinclined.

That said, I don't understand how this is being used to address the cited issue, so there might well be a new reason. @nex3 Could you elaborate?

[pq]

@bwilkerson: the cited use case involved private constructors. A modest proposal might be to just allow for those:

  A._(this.bar_bar); //OK
  A.__(this.bar_bar); //OK

(which works for me).

[bwilkerson]

The cited feature request says

However, if you want to access an explicit initializer's value from a later initializer, you're out of luck ...

I'm not sure how using multiple private constructors helps with that request.

But if this is a request to allow multiple private constructors of the form you gave, then my initial reaction is that it's going to quickly become unreadable:

A a = A_____( ... );

How many underscores are there? What's the difference between A___ and A____? Seems like private constructors (at least when there's more than one) ought to have meaningful names in order to make the code more comprehensible.

But that's just my personal opinion and if there's enough support for allowing it then I suppose we should.

[nex3]

@pq

the cited use case involved private constructors. A modest proposal might be to just allow for those:

  A._(this.bar_bar); //OK
  A.__(this.bar_bar); //OK

(which works for me).

This would definitely be an improvement, although I suspect there may be other cases where underscore identifiers are lurking in the language that will come up later on.

@bwilkerson

The cited feature request says

However, if you want to access an explicit initializer's value from a later initializer, you're out of luck ...

I'm not sure how using multiple private constructors helps with that request.

This helps because the only way to fake local variables in a generative constructor is to pass intermediate values as arguments to another generative constructor. For example, if I wanted to write:

class Foo {
  final int _sum;
  final int _sumPlus1;

  Foo._(int num1, int num2)
    : _sum = num1 + num2,
      _sumPlus1 = _sum + 1;
}

I'd have to write it instead as:

class Foo {
  final int _sum;
  final int _sumPlus1;

  Foo._(int num1, int num2)
    : Foo.__(num1 + num2);

  Foo.__(this._sum) : _sumPlus1 = _sum + 1;
}

But if this is a request to allow multiple private constructors of the form you gave, then my initial reaction is that it's going to quickly become unreadable:

A a = A_____( ... );

How many underscores are there? What's the difference between A___ and A____? Seems like private constructors (at least when there's more than one) ought to have meaningful names in order to make the code more comprehensible.

But that's just my personal opinion and if there's enough support for allowing it then I suppose we should.

In examples like the above, there isn't a meaningful name to distinguish the two constructors, because they aren't really doing anything meaningfully different. The latter is just an extension of the former that exists to work around dart-lang/language#622.

[bwilkerson]

It's true that it would be of questionable value to name those two constructors, but not terrible:

class Foo {
  final int _sum;
  final int _sumPlus1;

  Foo._sum(int num1, int num2)
    : Foo._sum_helper(num1 + num2);

  Foo._sum_helper(this._sum) : _sumPlus1 = _sum + 1;
}

but one question is whether it's more helpful or harmful to allow arbitrarily long sequences of underscores outside of parameter lists:

class Bar {
  final int _value;
  final int _valuePlus1;

  Bar._(int left, int right) : Foo._____(num1 + num2);
  Bar.__(int left, int right) : Foo._____(num1 - num2);
  Bar.___(int left, int right) : Foo._____(num1 * num2);
  Bar.____(int left, int right) : Foo._____(num1 / num2);
  Bar._____(this._value) : __valuePlus1 = __value + 1;
}

Another question is whether we would want to revert this change if the language added the requested feature. I think the answer is "yes", and given that it would be a breaking change to revert it, I'd be somewhat reluctant to make the change.

Personally, if we think this is a reasonable way to name some things I'd like to see the style guide updated first to specify when and where, and then make the lint match that.

... although I suspect there may be other cases where underscore identifiers are lurking in the language that will come up later on.

That's quite possible, but I don't remember seeing any requests to allow them. It's possible that people who use underscores as identifiers just don't enable this lint.

[nex3]

I think talking about constructors named ______ is kind of a slippery slope fallacy—no one's actually proposing writing a constructor that long. But two layers of private default constructors do come up in practice, and this would be useful for them.

dart-lang/language#622 is just one example of a use case for this. The broader point is that, regardless of use-cases, underscore identifiers are a convention for anonymous member "names" in more places than just formal parameter lists.

It's possible that people who use underscores as identifiers just don't enable this lint.

This lint is enabled by pedantic, which means it's also used to rank packages on pub.dev. It's not something people can really opt out of.

[pq]

@nex3: would a baby step allowing named constructors with underscores be a reasonable compromise? We can expand to other identifiers if the need arises.

@bwilkerson: how does that sit with you?

@munificent: how do you feel about a supporting style guide revision?

[bwilkerson]

I'm not thrilled with the change, but I'm not strongly enough opposed to continue arguing against it. It would be interesting to get Bob's opinion first, though.

[pq]

@munificent: what do you think?

[munificent]

I do see constructors named __ sometimes, and it seems like a reasonable pattern to me. Certainly reasonable enough that I don't think package maintainers should be punished for using it. But that maybe says more about using pedantic for package scoring than it does anything about this lint in particular.

I don't have a strong opinion but I think if the lint allows _, it's reasonable for it to allow any multiple of that too.