[dds] Add support for Log Points to DAP

Change-Id: I74dca1871d3c6b826aafecbb6425604d43b9262f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/209704
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/dds/lib/src/dap/adapters/dart.dart

@@ -550,8 +550,8 @@ abstract class DartDebugAdapter<T extends DartLaunchRequestArguments>
       supportsConfigurationDoneRequest: true,
       supportsDelayedStackTraceLoading: true,
       supportsEvaluateForHovers: true,
+      supportsLogPoints: true,
       // TODO(dantup): All of these...
-      // supportsLogPoints: true,
       // supportsRestartFrame: true,
       supportsTerminateRequest: true,
     ));

pkg/dds/lib/src/dap/isolate_manager.dart

@@ -3,7 +3,9 @@
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:async';
+import 'dart:convert';
 
+import 'package:collection/collection.dart';
 import 'package:vm_service/vm_service.dart' as vm;
 
 import 'adapters/dart.dart';
@@ -85,6 +87,12 @@ class IsolateManager {
   /// when asking for data so it's all stored together here.
   final _storedData = <int, _StoredData>{};
 
+  /// A pattern that matches an opening brace `{` that was not preceeded by a
+  /// dollar.
+  ///
+  /// Any leading character matched in place of the dollar is in the first capture.
+  final _braceNotPrefixedByDollarOrBackslashPattern = RegExp(r'(^|[^\\\$]){');
+
   IsolateManager(this._adapter);
 
   /// A list of all current active isolates.
@@ -327,19 +335,19 @@ class IsolateManager {
         final message = result.message ?? '<error ref>';
         _adapter.sendOutput(
           'console',
-          'Debugger failed to evaluate breakpoint $type "$expression": $message',
+          'Debugger failed to evaluate breakpoint $type "$expression": $message\n',
         );
       } else if (result is vm.Sentinel) {
         final message = result.valueAsString ?? '<collected>';
         _adapter.sendOutput(
           'console',
-          'Debugger failed to evaluate breakpoint $type "$expression": $message',
+          'Debugger failed to evaluate breakpoint $type "$expression": $message\n',
         );
       }
     } catch (e) {
       _adapter.sendOutput(
         'console',
-        'Debugger failed to evaluate breakpoint $type "$expression": $e',
+        'Debugger failed to evaluate breakpoint $type "$expression": $e\n',
       );
     }
   }
@@ -406,10 +414,19 @@ class IsolateManager {
         // Look up the client breakpoints that correspond to the VM breakpoint(s)
         // we hit. It's possible some of these may be missing because we could
         // hit a breakpoint that was set before we were attached.
-        final breakpoints = event.pauseBreakpoints!
+        final clientBreakpoints = event.pauseBreakpoints!
             .map((bp) => _clientBreakpointsByVmId[bp.id!])
             .toSet();
 
+        // Split into logpoints (which just print messages) and breakpoints.
+        final logPoints = clientBreakpoints
+            .whereNotNull()
+            .where((bp) => bp.logMessage?.isNotEmpty ?? false)
+            .toSet();
+        final breakpoints = clientBreakpoints.difference(logPoints);
+
+        await _processLogPoints(thread, logPoints);
+
         // Resume if there are no (non-logpoint) breakpoints, of any of the
         // breakpoints don't have false conditions.
         if (breakpoints.isEmpty ||
@@ -448,6 +465,41 @@ class IsolateManager {
     }
   }
 
+  /// Interpolates and prints messages for any log points.
+  ///
+  /// Log Points are breakpoints with string messages attached. When the VM hits
+  /// the breakpoint, we evaluate/print the message and then automatically
+  /// resume (as long as there was no other breakpoint).
+  Future<void> _processLogPoints(
+    ThreadInfo thread,
+    Set<SourceBreakpoint> logPoints,
+  ) async {
+    // Otherwise, we need to evaluate all of the conditions and see if any are
+    // true, in which case we will also hit.
+    final messages = logPoints.map((bp) => bp.logMessage!).toList();
+
+    final results = await Future.wait(messages.map(
+      (message) {
+        // Log messages are bare so use jsonEncode to make them valid string
+        // expressions.
+        final expression = jsonEncode(message)
+            // The DAP spec says "Expressions within {} are interpolated" so to
+            // avoid any clever parsing, just prefix them with $ and treat them
+            // like other Dart interpolation expressions.
+            .replaceAllMapped(_braceNotPrefixedByDollarOrBackslashPattern,
+                (match) => '${match.group(1)}\${')
+            // Remove any backslashes the user added to "escape" braces.
+            .replaceAll(r'\\{', '{');
+        return _evaluateAndPrintErrors(thread, expression, 'log message');
+      },
+    ));
+
+    for (final messageResult in results) {
+      // TODO(dantup): Format this using other existing code in protocol converter?
+      _adapter.sendOutput('console', '${messageResult?.valueAsString}\n');
+    }
+  }
+
   /// Sets breakpoints for an individual isolate.
   ///
   /// If [uri] is provided, only breakpoints for that URI will be sent (used

pkg/dds/test/dap/integration/debug_breakpoints_test.dart

@@ -411,4 +411,73 @@ void main(List<String> args) async {
     });
     // These tests can be slow due to starting up the external server process.
   }, timeout: Timeout.none);
+
+  group('debug mode logpoints', () {
+    /// A helper that tests a LogPoint using [logMessage] and expecting the
+    /// script not to pause and [expectedMessage] to show up in the output.
+    Future<void> _testLogPoint(
+      DapTestSession dap,
+      String logMessage,
+      String expectedMessage,
+    ) async {
+      final client = dap.client;
+      final testFile = dap.createTestFile(simpleBreakpointProgram);
+      final breakpointLine = lineWith(testFile, '// BREAKPOINT');
+
+      final outputEventsFuture = client.outputEvents.toList();
+
+      await client.doNotHitBreakpoint(
+        testFile,
+        breakpointLine,
+        logMessage: logMessage,
+      );
+
+      final outputEvents = await outputEventsFuture;
+      final outputMessages = outputEvents.map((e) => e.output.trim());
+
+      expect(
+        outputMessages,
+        contains(expectedMessage),
+      );
+    }
+
+    test('print simple messages', () async {
+      await _testLogPoint(
+        dap,
+        r'This is a test message',
+        'This is a test message',
+      );
+    });
+
+    test('print messages with Dart interpolation', () async {
+      await _testLogPoint(
+        dap,
+        r'This is a test message in ${DateTime.now().year}',
+        'This is a test message in ${DateTime.now().year}',
+      );
+    });
+
+    test('print messages with just {braces}', () async {
+      await _testLogPoint(
+        dap,
+        // The DAP spec says "Expressions within {} are interpolated" so in the DA
+        // we just prefix them with $ and treat them like other Dart interpolation
+        // expressions.
+        r'This is a test message in {DateTime.now().year}',
+        'This is a test message in ${DateTime.now().year}',
+      );
+    });
+
+    test('allows \\{escaped braces}', () async {
+      await _testLogPoint(
+        dap,
+        // Since we treat things in {braces} as expressions, we need to support
+        // escaping them.
+        r'This is a test message with \{escaped braces}',
+        r'This is a test message with {escaped braces}',
+      );
+    });
+
+    // These tests can be slow due to starting up the external server process.
+  }, timeout: Timeout.none);
 }

pkg/dds/test/dap/integration/test_client.dart

@@ -444,6 +444,7 @@ extension DapTestClientExtension on DapTestClient {
     File file,
     int line, {
     String? condition,
+    String? logMessage,
     Future<Response> Function()? launch,
   }) async {
     await Future.wait([
@@ -452,7 +453,13 @@ extension DapTestClientExtension on DapTestClient {
       sendRequest(
         SetBreakpointsArguments(
           source: Source(path: file.path),
-          breakpoints: [SourceBreakpoint(line: line, condition: condition)],
+          breakpoints: [
+            SourceBreakpoint(
+              line: line,
+              condition: condition,
+              logMessage: logMessage,
+            )
+          ],
         ),
       ),
       launch?.call() ?? this.launch(file.path),