analyzer code presubmit checks

[srawlins]

We brought up recently checks that we might want to make presubmit (on upload) checks for analyzer, Hopefully we can make some of these only trigger when certain files are changed:

• touching pkg/analyzer/messages.yaml: run pkg/analyzer/tool/messages/messages_test.dart
• touching pkg/analyzer/messages.yaml: check that Dart error code has been generated (dart pkg/analyzer/tool/messages/generate.dart)
• touching pkg/analyzer/messages.yaml: check that Dart error code has been generated (dart pkg/analyzer/tool/diagnostics/generate.dart)
• touching pkg/analyzer/lib/src/error/error_code_values.g.dart or pkg/linter/lib/src/rules.dart: run verify_error_fix_status.
• touching pkg/{analyzer/analysis_server}/test files: check that test_all.dart files are updated.
• touching pkg/{analyzer,analysis_server,linter,...} files: check that files are sorted.
• touching any file in pkg/linter/lib/src/rules files: check that machine.json is not outdated.

CC @bwilkerson @scheglov @kallentu @pq @keertip

[kallentu]

These are great. I want these checks so badly.

Some other ones that I've burned time on:

• Touching pkg/analyzer/messages.yaml sometimes needs a regeneration of the diagnostics if you change the documentation: dart run pkg/analyzer/tool/diagnostics/generate.dart.

• Also, adding a new diagnostic in pkg/analyzer/messages.yaml, making sure that pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml is updated. (I run python3 tools/test.py -n analyzer-unittest-asserts-release-linux pkg/analysis_server/test/verify_error_fix_status_test)

[pq]

Emphatic 💯💯💯.

My understanding is that presubmits are largely written in python but hopefully we can (performantly) call out to dart so we don't have to duplicate logic we've already got?

@whesse, @athomas: maybe you could advise?

[bwilkerson]

We do need to be careful not to add to much time to the presubmits. We might want to time some of these tests to ensure that they're all reasonable. Of course, it won't be as big an issue if they can run only when a specific file has been modified.

[scheglov]

I agree that any additional checks that we add should be fast, or we don't add them.
I already manually comment out a few, such as

    // verify_diagnostics.main();
    // verify_docs.main();
    // verify_tests.main();

and pkg/analysis_server/test/benchmarks_test.dart

Because they take too much time, and I care about these checks only when I'm ready to sent CL.
No while I'm doing some work on element model that might affect many test, so I often run all of them.

Sometimes this causes me an issue that something fails on bots because I forgot to check.
But more often it would not, and I will just waste time running these checks.

[sortie]

All this logic sounds like it would fit very well into PRESUBMIT.py. It gives you a list of file paths, so it's easy to make the checks conditional, and you can shell out to dart easily. I think you're already totally empowered to just add in the logic exactly as it makes sense for you and send it to us for review. I'm happy to assist if you need help :)

The comment about expensiveness does matter, though at least if the checks are conditional on file paths, then at least they're only paid for by the relevant teams :)

[srawlins]

On my laptop, verify_sorted_test.dart takes 20 seconds, which validates that files in all of our packages are sorted. Breaking into smaller tests, like just validating pkg/analyzer cuts down the time. pkg/analyzer takes 9 seconds, pkg/analysis_server takes 9 seconds.

messages_test.dart takes 2 seconds.
verify_tests_test.dart takes 3 seconds.
verify_error_fix_status_test.dart takes ~0 seconds.