dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.26k stars 1.58k forks source link

Analyzer code should start passing a language version to dart_style #56685

Open munificent opened 2 months ago

munificent commented 2 months ago

Greetings analyzer team friends!

The formatter is moving to being language version aware. This means that when it's parsing some code, it needs to be told what language version to parse it as. The DartFormatter constructor now takes an optional parameter where you can pass in the language version. In a future version of dart_style, that parameter will become mandatory.

If you own code that uses dart_style as a library, you should update your code to pass in a language version. To migrate:

  1. If this code is inside a package (as opposed to, say, the Dart core libraries), then update your constraint on dart_style to ^2.3.7. That's the version where the new parameter was added.

  2. If you know the precise language version the code should be parsed as, then pass in that version (as an instance of the pub_semver package's Version class). This is what "real" tools should do.

    For simple one-off scripts and other utilities where precise behavior doesn't matter much, you can pass in DartFormatter.latestLanguageVersion to unconditionally parse the code as the latest language version that the formatter itself supports.

When the --tall-style experiment flag ships, the passed in language version will also be used to determine whether you get the current formatting style or the new "tall" style.

I did a search through the SDK, and the constructor calls that I think are relevant are:

~/dev/dart/sdk/pkg/analysis_server/lib/src/g3/utilities.dart:
   24:   var formatter = DartFormatter();

~/dev/dart/sdk/pkg/analysis_server/lib/src/handler/legacy/edit_format.dart:
   51:     var formatter = DartFormatter(pageWidth: params.lineLength);

~/dev/dart/sdk/pkg/analysis_server/lib/src/handler/legacy/edit_format_if_enabled.dart:
   34:     var formatter = DartFormatter();

~/dev/dart/sdk/pkg/analysis_server/lib/src/lsp/source_edits.dart:
   95:     formattedResult = DartFormatter(pageWidth: lineLength).formatSource(code);

~/dev/dart/sdk/pkg/analysis_server/tool/lsp_spec/codegen_dart.dart:
   17: final formatter = DartFormatter();

~/dev/dart/sdk/pkg/analyzer_plugin/lib/src/utilities/change_builder/change_builder_dart.dart:
 1502:     var formattedResult = DartFormatter().formatSource(

Thank you!


Implementation:

Testing:

Testing is currently tricky as the formatter is not yet gating tall-style on language version (requiring an experiment flag in the meantime). We'll want to loop back add tests once that work is complete.

pq commented 1 month ago

FYI @DanTup, @bwilkerson: as per a conversation with @munificent, formatting is not yet being gated by version so testing is complicated. As suggested above, I propose we leave tests as a TODO for now and fill them in once the formatter work is complete.

munificent commented 1 month ago

Testing is currently tricky as the formatter is not yet gating tall-style on language version (requiring an experiment flag in the meantime). We'll want to loop back add tests once that work is complete.

In terms of separation of concerns, it's not really analyzer's job to validate which formatting style you get. That's the formatter's job. All this issue is about is ensuring that you're passing the correct language version to the formatter. You can test that by trying to format syntax that is only valid at certain language versions. So:

Thank you for working on this!