Linter: update `non_constant_identifier_names` to include local variables

[pq]

Currently we don't check local variables in non_constant_identifiers but should. I checked w/ @munificent who confirmed this is consistent w/ the intent of the style guide entry:

https://dart.dev/guides/language/effective-dart/style#do-name-other-identifiers-using-lowercamelcase

I'm guessing this was simply overlooked in implementation.

We'll want to get a sense for how breaking this will be to current adopters and look into internal compatibility but at least @jefflim-google's team has asked for it.

/fyi @davidmorgan

Also, see https://github.com/dart-lang/linter/issues/330 for other issues related to internal adoption.

[pq]

Looking at the proposed core lints, this is included there so we should probably seek consensus from folks thinking about canonical lints.

/fyi @lrhn @jakemac53 @natebosch @mit-mit

[davidmorgan]

I think you mean non_constant_identifier_names :)

Since we don't/can't blanket enforce in google3, changes are not scary. But it might be nice to split out the parts we _could_ enforce in google3, per #330.

[pq]

I think you mean non_constant_identifier_names :)

🤦 Ha. Yes. Thanks. Fixed.

As for blocked blanket enforcement internally, maybe we could revisit? This seems like generally a win and could really help w/ readability.

[jakemac53]

LGTM

[pq]

@goderbauer, @hixie: I'd be curious to hear how this sits with you from the Flutter perspective?

[goderbauer]

Enforcing lowerCamelCase for local variables with that lint sounds good to me. I wonder how many offenders of this we have in the Flutter code base...

[pq]

I wonder how many offenders of this we have in the Flutter code base...

Good deal. I'll implement and do a dry-run and we can go from there.

[pq]

While landed, there's a chance we'll un-land after we assess impact on Flutter. Try running here: https://dart-review.googlesource.com/c/sdk/+/195052

[jakemac53]

~Please make sure to also asses the impact on internal code, assuming this lint is enabled there.~ nevermind david commented above that we couldn't enable this anyways

[pq]

Please make sure to also asses the impact on internal code, assuming this lint is enabled there.

Well, interpreted another way, we _should_ check to see if this breaks any SDK packages at least. doing a quick grep, I see that a number of third_party packages do enable the lint:

☁  sdk [linter_1_22_pre] ⚡  grep -R --include "analysis_options.yaml" non_constant_identifier_names .
./third_party/pkg_tested/http_io/analysis_options.yaml:    #- non_constant_identifier_names
./third_party/pkg/benchmark_harness/analysis_options.yaml:    #- non_constant_identifier_names
./third_party/pkg/crypto/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/sse/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/shelf_web_socket/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/collection/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/sync_http/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/intl/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/file/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/linter/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/shelf_proxy/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/markdown/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/source_span/analysis_options.yaml:  - non_constant_identifier_names
./third_party/pkg/path/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/platform/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/usage/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/html/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/matcher/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/bazel_worker/analysis_options.yaml:    # - non_constant_identifier_names
./third_party/pkg/http_parser/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/pub_semver/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/shelf_static/analysis_options.yaml:  - non_constant_identifier_names
./third_party/pkg/test_descriptor/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/boolean_selector/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/http/analysis_options.yaml:  - non_constant_identifier_names
./third_party/pkg/fixnum/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/http_retry/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/webdriver/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/browser_launcher/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/typed_data/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/yaml/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/mime/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/dartdoc/analysis_options.yaml:    #    - non_constant_identifier_names
./third_party/pkg/glob/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/mockito/analysis_options.yaml:     - non_constant_identifier_names
./third_party/pkg/pub/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/shelf/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/test_process/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/string_scanner/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/args/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/web_socket_channel/analysis_options.yaml:  - non_constant_identifier_names
./third_party/pkg/pool/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/http_multi_server/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/logging/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/clock/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/shelf_packages_handler/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/convert/analysis_options.yaml:    - non_constant_identifier_names
./third_party/pkg/process/analysis_options.yaml:    - non_constant_identifier_names

I think we can leave it to authors to fix those up if and as needed but this one in pkg I'll take a look at up front:

./pkg/js/analysis_options.yaml:  - non_constant_identifier_names

[pq]

OK. I think we're good:

☁  sdk [linter_1_22_pre] ⚡  dart analyze pkg/js
Analyzing js...                        0.7s
No issues found!

[pq]

The third_party packages are another matter.

~Do folks feel strongly about vetting impact there before landing this?~

EDIT: it was easy enough to just find out.

☁  pkg [linter_1_22_pre] ⚡  dart analyze . | grep non_constant_identifier_names
☁  pkg [linter_1_22_pre] ⚡

🌈

Feels pretty safe to me to land pending results from the flutter-engine try-bot (flutter-analyze is already green).

[pq]

Try run here. All green. 🟢

I think we're good to go. 🏁

[pq]

Closing as we prep a 1.3.0 release: #2581.