[DAP] Normalize Directory.current in DAP before launching the process

We normalize paths to have uppercase drive letters in many places in DAP to avoid mismatches of things like breakpoint paths if the client sends mixed casing.

It's not clear why the change to not host DDS internally affected this, but it seems sensible to perform this normalization here because we're normalizing everywhere else and right now the paths are treated as case-sensitive in many places.

Fixes https://github.com/dart-lang/sdk/issues/56965
Fixes https://github.com/dart-lang/sdk/issues/57011

Change-Id: Ie63ef66058ff9d37955659a780f7bd40cdfc2dc6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/392442
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>

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

@@ -394,7 +394,7 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
     if (args is DartLaunchRequestArguments)
       path.dirname((args as DartLaunchRequestArguments).program),
     ...?args.additionalProjectPaths,
-  ].nonNulls.toList();
+  ].nonNulls.map(normalizePath).toList();
 
   /// Whether we have already sent the [TerminatedEvent] to the client.
   ///
@@ -2682,6 +2682,10 @@ abstract class DartDebugAdapter<TL extends LaunchRequestArguments,
       Directory.current = Directory(cwd);
     }
 
+    // Also ensure the cwd always has uppercase drive letters to match what
+    // we'll normalize to everywhere else.
+    Directory.current = Directory(normalizePath(Directory.current.path));
+
     // Notify IsolateManager if we'll be debugging so it knows whether to set
     // up breakpoints etc. when isolates are registered.
     final debug = !(noDebug ?? false);

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

@@ -115,7 +115,7 @@ main() {
           .nonNulls
           .toList();
       // Ensure we had a frame with the absolute path of the test script.
-      expect(stackFramePaths, contains(testFile.path));
+      expect(stackFramePaths, contains(uppercaseDriveLetter(testFile.path)));
     });
 
     test('can hit and resume from a breakpoint', () async {

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

@@ -207,26 +207,6 @@ void main(List<String> args) async {
       await testHitBreakpointAndResume();
     });
 
-    test(
-        'stops at a line breakpoint and can be resumed '
-        'when breakpoint requests have lowercase drive letters '
-        'and program/VM have uppercase drive letters', () async {
-      final client = dap.client;
-      client.forceDriveLetterCasingUpper = true;
-      client.forceBreakpointDriveLetterCasingLower = true;
-      await testHitBreakpointAndResume();
-    }, skip: !Platform.isWindows);
-
-    test(
-        'stops at a line breakpoint and can be resumed '
-        'when breakpoint requests have uppercase drive letters '
-        'and program/VM have lowercase drive letters', () async {
-      final client = dap.client;
-      client.forceDriveLetterCasingLower = true;
-      client.forceBreakpointDriveLetterCasingUpper = true;
-      await testHitBreakpointAndResume();
-    }, skip: !Platform.isWindows);
-
     test('stops at a line breakpoint and can step over (next)', () async {
       final testFile = dap.createTestFile('''
 void main(List<String> args) async {

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

@@ -104,7 +104,7 @@ main() {
       // expect.
       expect(
         mainStackFrameEvent.source?.path,
-        dap.client.uppercaseDriveLetter(testFile.path),
+        uppercaseDriveLetter(testFile.path),
       );
       expect(mainStackFrameEvent.line, exceptionLine);
       expect(mainStackFrameEvent.column, 5);

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

@@ -112,7 +112,7 @@ main() {
         runInTerminalArgs!.args,
         containsAllInOrder([
           Platform.resolvedExecutable,
-          dap.client.uppercaseDriveLetter(testFile.path),
+          uppercaseDriveLetter(testFile.path),
         ]),
       );
       expect(proc!.pid, isPositive);
@@ -520,9 +520,8 @@ main() {
       ({String name, Uri fileLikeUri}) getExpectedMacroSource(File testFile) {
         // Drive letters are always normalized to uppercase so expect
         // uppercase in the path part of the macro URI.
-        final fileLikeUri =
-            Uri.file(dap.client.uppercaseDriveLetter(testFile.path))
-                .replace(scheme: 'dart-macro+file');
+        final fileLikeUri = Uri.file(uppercaseDriveLetter(testFile.path))
+            .replace(scheme: 'dart-macro+file');
         // The expected source name will differ for inside/outside the lib
         // folder.
         final name = folderName == 'lib'

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

@@ -72,7 +72,7 @@ main() {
         runInTerminalArgs!.args,
         containsAllInOrder([
           Platform.resolvedExecutable,
-          dap.client.uppercaseDriveLetter(testFile.path),
+          uppercaseDriveLetter(testFile.path),
         ]),
       );
     });

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

@@ -16,6 +16,7 @@ import 'package:test/test.dart';
 import 'package:vm_service/vm_service.dart' as vm;
 
 import 'test_server.dart';
+import 'test_support.dart';
 
 /// A helper class to simplify acting as a client for interacting with the
 /// [DapTestServer] in tests.
@@ -51,19 +52,17 @@ class DapTestClient {
 
   late final Future<Uri?> vmServiceUri;
 
-  /// Used to control drive letter casing on Windows for testing.
-  bool? forceDriveLetterCasingUpper;
-
-  /// Used to control drive letter casing on Windows for testing.
-  bool? forceDriveLetterCasingLower;
-
-  /// Used to control drive letter casing for breakpoint requests on Windows for
+  /// Used to control drive letter casing for the 'program' value on Windows for
   /// testing.
-  bool? forceBreakpointDriveLetterCasingUpper;
+  DriveLetterCasing? forceProgramDriveLetterCasing;
 
-  /// Used to control drive letter casing for breakpoint requests on Windows for
+  /// Used to control drive letter casing for the 'cwd' value on Windows for
   /// testing.
-  bool? forceBreakpointDriveLetterCasingLower;
+  DriveLetterCasing? forceCwdDriveLetterCasing;
+
+  /// Used to control drive letter casing for breakpoint paths on Windows for
+  /// testing.
+  DriveLetterCasing? forceBreakpointDriveLetterCasing;
 
   /// All stderr OutputEvents that have occurred so far.
   final StringBuffer _stderr = StringBuffer();
@@ -174,8 +173,11 @@ class DapTestClient {
       DartAttachRequestArguments(
         vmServiceUri: vmServiceUri,
         vmServiceInfoFile: vmServiceInfoFile,
-        cwd: cwd != null ? _normalizePath(cwd) : null,
-        additionalProjectPaths: additionalProjectPaths,
+        cwd: cwd != null
+            ? setDriveLetterCasing(cwd, forceCwdDriveLetterCasing)
+            : null,
+        additionalProjectPaths:
+            additionalProjectPaths?.map(uppercaseDriveLetter).toList(),
         debugSdkLibraries: debugSdkLibraries,
         debugExternalPackageLibraries: debugExternalPackageLibraries,
         showGettersInDebugViews: showGettersInDebugViews,
@@ -339,13 +341,15 @@ class DapTestClient {
     return sendRequest(
       DartLaunchRequestArguments(
         noDebug: noDebug,
-        program: _normalizePath(program),
-        cwd: cwd != null ? _normalizePath(cwd) : null,
+        program: setDriveLetterCasing(program, forceProgramDriveLetterCasing),
+        cwd: cwd != null
+            ? setDriveLetterCasing(cwd, forceCwdDriveLetterCasing)
+            : null,
         args: args,
         vmAdditionalArgs: vmAdditionalArgs,
         toolArgs: toolArgs,
         additionalProjectPaths:
-            additionalProjectPaths?.map(_normalizePath).toList(),
+            additionalProjectPaths?.map(uppercaseDriveLetter).toList(),
         console: console,
         debugSdkLibraries: debugSdkLibraries,
         debugExternalPackageLibraries: debugExternalPackageLibraries,
@@ -777,50 +781,11 @@ extension DapTestClientExtension on DapTestClient {
     );
   }
 
-  /// Normalizes a non-breakpoint path being sent to the debug adapter based on
-  /// the values of [forceDriveLetterCasingUpper] and
-  /// [forceDriveLetterCasingLower].
-  String _normalizePath(String path) {
-    return _forceDriveLetterCasing(
-      path,
-      upper: forceDriveLetterCasingUpper,
-      lower: forceDriveLetterCasingLower,
-    );
-  }
-
-  /// Normalizes a a path to have an uppercase drive letter. All paths verified
-  /// that come out of the adapter are normalized this way so test expectations
-  /// should be normalized the same way before comparing.
-  String uppercaseDriveLetter(String path) {
-    return _forceDriveLetterCasing(
-      path,
-      upper: true,
-    );
-  }
-
   /// Normalizes a breakpoint path being sent to the debug adapter based on
   /// the values of [forceBreakpointDriveLetterCasingUpper] and
   /// [forceBreakpointDriveLetterCasingLower].
   String _normalizeBreakpointPath(String path) {
-    return _forceDriveLetterCasing(
-      path,
-      upper: forceBreakpointDriveLetterCasingUpper,
-      lower: forceBreakpointDriveLetterCasingLower,
-    );
-  }
-
-  String _forceDriveLetterCasing(String path, {bool? upper, bool? lower}) {
-    assert(upper != true || lower != true);
-    if (!Platform.isWindows || path.isEmpty) {
-      return path;
-    }
-    if (upper ?? false) {
-      return path.substring(0, 1).toUpperCase() + path.substring(1);
-    } else if (lower ?? false) {
-      return path.substring(0, 1).toLowerCase() + path.substring(1);
-    } else {
-      return path;
-    }
+    return setDriveLetterCasing(path, forceBreakpointDriveLetterCasing);
   }
 
   /// Sets the exception pause mode to [pauseMode] and expects to pause after

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

@@ -365,3 +365,28 @@ void testWithUriConfigurations(
     });
   }
 }
+
+/// Sets the casing of a drive letter according to [casing].
+String setDriveLetterCasing(String path, DriveLetterCasing? casing) {
+  if (!Platform.isWindows || path.isEmpty) {
+    return path;
+  }
+  var (driveLetter, rest) = (path.substring(0, 1), path.substring(1));
+  return switch (casing) {
+    DriveLetterCasing.uppercase => driveLetter.toUpperCase() + rest,
+    DriveLetterCasing.lowercase => driveLetter.toLowerCase() + rest,
+    null => path,
+  };
+}
+
+/// Normalizes a a path to have an uppercase drive letter. All paths verified
+/// that come out of the adapter are normalized this way so test expectations
+/// should be normalized the same way before comparing.
+String uppercaseDriveLetter(String path) {
+  return setDriveLetterCasing(path, DriveLetterCasing.uppercase);
+}
+
+enum DriveLetterCasing {
+  uppercase,
+  lowercase,
+}

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

@@ -0,0 +1,66 @@
+// Copyright (c) 2024, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:async';
+import 'dart:io';
+
+import 'package:test/test.dart';
+
+import 'test_client.dart';
+import 'test_scripts.dart';
+import 'test_support.dart';
+
+main() {
+  group('windows path casing', () {
+    DapTestSession? dap;
+    tearDown(() => dap?.tearDown());
+
+    for (final actualCwdCasing in [null, ...DriveLetterCasing.values]) {
+      for (final programCasing in DriveLetterCasing.values) {
+        for (final cwdCasing in [null, ...DriveLetterCasing.values]) {
+          for (final breakpointCasing in DriveLetterCasing.values) {
+            test(
+                'stops at a breakpoint and can resume (casing: '
+                'actualCwd: $actualCwdCasing, '
+                'program: $programCasing, '
+                'cwd: $cwdCasing, '
+                'breakpoint requests: $breakpointCasing)', () async {
+              // Set the correct casing of drive letters for this test.
+              if (actualCwdCasing != null) {
+                Directory.current = Directory(
+                  setDriveLetterCasing(Directory.current.path, actualCwdCasing),
+                );
+              }
+
+              dap = await DapTestSession.setUp();
+              final client = dap!.client;
+
+              client.forceProgramDriveLetterCasing = programCasing;
+              client.forceCwdDriveLetterCasing = cwdCasing;
+              client.forceBreakpointDriveLetterCasing = breakpointCasing;
+
+              final testFile = dap!.createTestFile(simpleBreakpointProgram);
+              final breakpointLine = lineWith(testFile, breakpointMarker);
+
+              // Hit the initial breakpoint.
+              final stop = await client.hitBreakpoint(
+                testFile,
+                breakpointLine,
+                cwd: cwdCasing == null ? null : dap!.testAppDir.path,
+              );
+
+              // Resume and expect termination (as the script will get to the end).
+              await Future.wait([
+                client.event('terminated'),
+                client.continue_(stop.threadId!),
+              ], eagerError: true);
+            });
+          }
+        }
+      }
+    }
+
+    // These tests can be slow due to starting up the external server process.
+  }, timeout: Timeout.none, skip: !Platform.isWindows);
+}