Too strict: non_constant_identifier_names

[matanlurey]

When code (such as but not limited to tests - it's also true for static final fields on a class and real-top level final variables), it's common to define constant-like identifiers as such:

group('MyThing', () {
  final UNIQUE_ID = new Int64(1);

  test('should return non-null with a valid ID', () {
    expect(myThing.doThing(UNIQUE_ID), isNotNull);
  });
});

non_constant_identifier_names triggers saying:

final UNIQUE_ID = new Int64(1);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Name non-constant identifiers using lowerCamelCase.

Conceptually however, a final is used similar to const - things like Int64, RegExp, etc, do not have const constructors - but they are immutable objects where final is used. This seems to conflict with constant_identifier_names, where it's legal to use final.

Suggestions:

1. Opt-out fields that are final from this lint
2. Fork this lint to have a less strict variant we can use for the Google style guide
   (There is a lot of existing code, including in the SDK, that uses ALL_CAPS)

[zoechi]

consts shouldn't be all-uppercase https://www.dartlang.org/guides/language/effective-dart/style#prefer-using-lowercamelcase-for-constant-names

[srawlins]

Since nothing is expected to be screaming-snake anymore... is this request stale?

[davidmorgan]

This stops us from enabling this lint in google3; there's no practical way we can get rid of UPPER_CASE finals, since it used to be recommended and is still allowed by effective dart.

Could the analyzer team please comment on whether it makes more sense to fix or fork?

Thanks!

[bwilkerson]

Conceptually however, a final is used similar to const - things like Int64, RegExp, etc, do not have const constructors - but they are immutable objects where final is used. This seems to conflict with constant_identifier_names, where it's legal to use final.

I think there are two questions here: whether some final variables should be treated as const, and whether SCREAMING_CAPS should be allowed for some constants.

As for the first, neither constant_identifier_names nor non_constant_identifier_names currently treat any final variables as constants (despite the example in constant_identifier_names that incorrectly implies that it does). I understand the argument in favor of doing so, and that there isn't any way to mark an effectively constant value as a const if there is no const constructor. And I agree that it makes sense to name effectively constant values the same way we name constant values.

That said, it's impossible for the linter to always accurately determine when a variable is effectively constant. The class RegExp is a good example. It's an abstract class with an external factory constructor. That means that linter doesn't have access to the implementation of the class and can't verify whether the state of the object ever changes, so it can't assume that it's immutable. It might be possible to check for immutability in some, even most, cases, but I don't think we can cover all the cases. That doesn't mean that it isn't worth doing the best we can, just that we need to be aware of the limitations of the tool.

An alternative we might want to consider is to change the immutable classes in question so that they do have const constructors and the variables that are now only effectively constant can actually be constants.

As for the second, the style guide does allows the use of all cap names under one condition:

You may use SCREAMING_CAPS for consistency with existing code, as in the following cases:

• When adding code to a file or library that already uses SCREAMING_CAPS.
• When generating Dart code that’s parallel to Java code — for example, in enumerated types generated from protobufs.

The linter can't check for "consistency with existing code", but could check for the specific cases listed. Specifically, it could allow it when:

• all of the other static constants in the class/file are SCREAMING_CAPS or
• when the file is a generated file (using the same list of known patters used elsewhere).

I think it would be less harmful for users that are trying to use constant_identifier_names to prevent the use of SCREAMING_CAPS for constants if we special case only those situations that are explicitly listed. Given that constraint, I have no problem updating constant_identifier_names to sometimes allow SCREAMING_CAPS.

[davidmorgan]

Treating final declarations as constants for the purpose of this lint moves the SCREAMING_CAPS discussion out of this lint, which is sufficient to unblock us :)

Looking at pre-existing violations in google3 there are plenty that are neither consts/finals and could benefit from this lint. Here (google internal only, sorry!): http://go/non_constant_identifier_names_prios

What do you think, please?

[matanlurey]

I think this is stale at this point, though /cc @davidmorgan if you disagree and want to re-open.