[dds/dap] Fix an issue with DAP Source.name being file paths when package: and dart: URIs are expected

The move to URIs introduced a subtle bug here - we would use the mapped URI (which is almost always `file:`) to decide whether to show a file path, instead of doing it only if the unmapped URI was a `file` (eg. it could have been `package:` or `dart:`.

This wasn't caught by any tests here, but was caught in the Flutter roll (https://github.com/flutter/flutter/pull/145235). This fixes it and adds a new test that we verify the Source names for local files, packages, and dart: URIs.

Change-Id: I282bf935b9fa4016abeafd556dfd368950ee9611
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/357325
Commit-Queue: Helin Shiah <helinx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
This commit is contained in:
Danny Tuppeny 2024-03-18 15:53:49 +00:00 committed by Commit Queue
parent 88bb72f47f
commit d931bec5d3
4 changed files with 38 additions and 5 deletions

View file

@ -1,3 +1,6 @@
# 3.3.1
- [DAP] Fixed an issue introduced in 3.3.0 where `Source.name` could contain a file paths when a `package:` or `dart:` URI should have been used.
# 3.3.0
- **Breaking change:** [DAP] Several signatures in DAP debug adapter classes have been updated to use `Uri`s where they previously used `String path`s. This is to support communicating with the DAP client using URIs instead of file paths. URIs may be used only when the client sets the custom `supportsDartUris` client capability during initialization.
- [DAP] Added support for using mapping `dart-macro+file:///` URIs in communication with the client if the `supportsDartUris` flag is set in arguments for `initializeRequest`.

View file

@ -2249,11 +2249,11 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
// then we leave the formatting as-is and don't fade anything out.
final isUserProject = pathInfo?.isUserCode ?? true;
// For the name, we usually use the package URI, but if we only ended up
// with a file URI, try to make it relative to cwd so it's not so long.
// For the name, we usually use the package URI, but if we only had a file
// URI to begin with, try to make it relative to cwd so it's not so long.
final name = uri != null && fileLikeUri != null
? (fileLikeUri.isScheme('file')
? _converter.convertToRelativePath(fileLikeUri.toFilePath())
? (uri.isScheme('file')
? _converter.convertToRelativePath(uri.toFilePath())
: uri.toString())
: null;

View file

@ -1,5 +1,5 @@
name: dds
version: 3.3.0
version: 3.3.1
description: >-
A library used to spawn the Dart Developer Service, used to communicate with
a Dart VM Service instance.

View file

@ -242,6 +242,36 @@ main() {
]);
});
test('includes correct Source.name for SDK and package sources', () async {
// Use a sample program that prints output to stderr that includes:
// - non stack frame lines
// - stack frames with file:// URIs
// - stack frames with package URIs (that need asynchronously resolving)
// - stack frames with dart URIs (that need asynchronously resolving)
final fileUri = Uri.file(dap.createTestFile('').path);
final (packageUri, _) = await dap.createFooPackage();
final dartUri = Uri.parse('dart:isolate-patch/isolate_patch.dart');
final testFile = dap.createTestFile(
stderrPrintingProgram(fileUri, packageUri, dartUri),
);
final outputEvents = await dap.client.collectOutput(file: testFile);
final outputSourceNames = outputEvents
.where((e) => e.category == 'stderr')
.map((output) => output.source?.name)
.where((sourceName) => (sourceName?.isNotEmpty ?? false))
.toList();
expect(
outputSourceNames,
[
fileUri.toFilePath(),
packageUri.toString(),
dartUri.toString(),
],
);
});
group('progress notifications', () {
/// Helper to verify [events] are the expected start/update/end events
/// in-order for a debug session starting.