[analysis_server] An error occurred while handling workspace/executeCommand request about sdk

[incendial]

@DanTup fyi

[DanTup]

@incendial I don't seem to be able to reproduce. The fixes run (the backslashes are removed), but no errors:

Recording.2024-03-04.143530.mp4

Can you reproduce in a new project, or only a specific one? Could you show a list of all the diagnostics you see for this file (and which fixes appear for each of the diagnostics)? It seems like we're trying to use both a FileEditBuilder and DartFileEditBuilder on the same file but I'm not sure which fix the FileEditBuilder is coming from.

[DanTup]

btw, looks like I don't have any diagnostics on the last line which might be why I'm not triggering it - I presume if removing that line fixes it, you might have a diagnostic/fix showing up for it?

[incendial]

Can you try enabling "use_raw_strings"?

[DanTup]

Aha, that triggers it. Thanks!

[DanTup]

@bwilkerson this is happening because the fixes use a mix of generic and Dart file builders. If there is a fix using each in the same file it results in this error when trying to fix both (both for the LSP server's "fix all" and dart fix).

For example remove_unnecessary_string_escapes uses the generic builder:

sdk/pkg/analysis_server/lib/src/services/correction/dart/remove_unnecessary_string_escape.dart

Line 32 in 22d7592

await builder.addGenericFileEdit(file, (builder) {

But convert_to_raw_string uses a Dart builder:

sdk/pkg/analysis_server/lib/src/services/correction/dart/convert_to_raw_string.dart

Line 44 in 22d7592

await builder.addDartFileEdit(file, (builder) {

I thought the right fix would be for the fixes to always use Dart edit builders and we should change the interface so they can't accidentally create other kinds of edit builders. However, we have a fix "Ignore diagnostic in analysis_options.yaml" which is a Dart fix but edits a non-Dart file and I realise that's not quite right.

One possibility would be to have addGenericFileEdit() return a Dart edit builder if called for a Dart file - however a Dart edit builder is not currently a superclass of a generic edit builder. So another option could be to just remove addGenericFileEdit entirely and force all code through a specific edit builder (Dart or YAML, and if we have other file types, make more specifically-named methods for each of them).

Any thoughts/opinions?

[DanTup]

Maybe a slightly simpler idea: add an assert to addGenericFileEdit to ensure it's not called for Dart or YAML files. As long as all callers have some test it should avoid this happening accidentally (but avoids having to make a breaking API change).

I opened https://dart-review.googlesource.com/c/sdk/+/355441 along these lines.

[bwilkerson]

For example remove_unnecessary_string_escapes uses the generic builder

That's definitely a bug. The fix clearly only applies to Dart files.

Maybe a slightly simpler idea: add an assert to addGenericFileEdit to ensure it's not called for Dart or YAML files.

I like that idea.