Fixed Trailing Slashes in cp command + added unit tests (#3921)

Fixes: #2835 

## Changes

Fixed handling of trailing directory separators in the `cp` command.
When a target path ends with `/` or `\`, the command now validates that
the directory exists before copying. If the directory doesn't exist, it
returns a clear error message instead of failing later.

Added a helper function `hasTrailingDirSeparator` to detect trailing
directory separators for both Unix-style (`/`) and Windows-style (`\`)
paths.

## Why

Previously, the `cp` command didn't properly handle trailing directory
separators. When users specified a target path like `target/dir/`
(expecting it to be treated as a directory), the command would not
validate that the directory exists first, leading to confusing error
messages later in the copy process. This change ensures that trailing
directory separators are explicitly treated as indicating a directory
target, and validates that the directory exists upfront.

## Tests

Added unit tests in `integration/cmd/fs/cp_test.go`:
- `TestFsCpFileToNonExistentDir`: Tests copying files to both existing
and non-existent directories with trailing slashes
- `TestFsCpFileToNonExistentDirWindowsPaths`: Windows-specific tests for
trailing backslashes (`\`) and forward slashes (`/`) on Windows

The tests verify that:
- Copying to an existing directory with a trailing separator succeeds
- Copying to a non-existent directory with a trailing separator returns
an appropriate error message
- Both Unix-style (`/`) and Windows-style (`\`) separators are handled
correctly

---------

Co-authored-by: Andrew Nester <[email protected]>

Author: varundeepsaini

cmd/fs/cp.go

@@ -5,8 +5,10 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
+	"os"
 	"path"
 	"path/filepath"
+	"strings"
 
 	"github.com/databricks/cli/cmd/root"
 	"github.com/databricks/cli/libs/cmdio"
@@ -126,6 +128,16 @@ func (c *copy) emitFileCopiedEvent(sourcePath, targetPath string) error {
 	return cmdio.RenderWithTemplate(c.ctx, event, "", template)
 }
 
+// hasTrailingDirSeparator checks if a path ends with a directory separator.
+func hasTrailingDirSeparator(path string) bool {
+	return strings.HasSuffix(path, string(os.PathSeparator))
+}
+
+// trimTrailingDirSeparators removes all trailing directory separators from a path.
+func trimTrailingDirSeparators(path string) string {
+	return strings.TrimRight(path, string(os.PathSeparator))
+}
+
 func newCpCommand() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "cp SOURCE_PATH TARGET_PATH",
@@ -190,6 +202,11 @@ func newCpCommand() *cobra.Command {
 			return c.cpDirToDir(sourcePath, targetPath)
 		}
 
+		// If target path has a trailing separator, trim it and let case 2 handle it
+		if hasTrailingDirSeparator(fullTargetPath) {
+			targetPath = trimTrailingDirSeparators(targetPath)
+		}
+
 		// case 2: source path is a file, and target path is a directory. In this case
 		// we copy the file to inside the directory
 		if targetInfo, err := targetFiler.Stat(ctx, targetPath); err == nil && targetInfo.IsDir() {

integration/cmd/fs/cp_test.go

@@ -374,3 +374,68 @@ func TestFsCpSourceIsDirectoryButTargetIsFile(t *testing.T) {
 		})
 	}
 }
+
+func TestFsCpFileToNonExistentDir(t *testing.T) {
+	t.Parallel()
+
+	for _, testCase := range copyTests() {
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+
+			ctx := context.Background()
+			sourceFiler, sourceDir := testCase.setupSource(t)
+			targetFiler, targetDir := testCase.setupTarget(t)
+			setupSourceFile(t, ctx, sourceFiler)
+
+			// Create a directory in the target
+			err := targetFiler.Mkdir(ctx, "existingdir")
+			require.NoError(t, err)
+
+			// Copy file to existing directory with trailing slash - should succeed
+			existingDir := path.Join(targetDir, "existingdir") + "/"
+			testcli.RequireSuccessfulRun(t, ctx, "fs", "cp", path.Join(sourceDir, "foo.txt"), existingDir)
+			assertTargetFile(t, ctx, targetFiler, "existingdir/foo.txt")
+
+			// Try to copy file to a non-existent directory with trailing slash - should fail
+			nonExistentDir := path.Join(targetDir, "nonexistent", "mydir") + "/"
+			_, _, err = testcli.RequireErrorRun(t, ctx, "fs", "cp", path.Join(sourceDir, "foo.txt"), nonExistentDir)
+			assert.Error(t, err)
+			assert.Contains(t, err.Error(), "directory")
+			assert.Contains(t, err.Error(), "no such directory")
+		})
+	}
+}
+
+func TestFsCpFileToNonExistentDirWindowsPaths(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("Skipping test on non-windows OS")
+	}
+
+	ctx := context.Background()
+	sourceFiler, sourceDir := setupLocalFiler(t)
+	targetFiler, targetDir := setupLocalFiler(t)
+	setupSourceFile(t, ctx, sourceFiler)
+
+	// Create a directory in the target
+	err := targetFiler.Mkdir(ctx, "existingdir")
+	require.NoError(t, err)
+
+	// Copy file to existing directory with trailing backslash (Windows style) - should succeed
+	windowsExistingDir := filepath.Join(filepath.FromSlash(targetDir), "existingdir") + "\\"
+	testcli.RequireSuccessfulRun(t, ctx, "fs", "cp", filepath.Join(filepath.FromSlash(sourceDir), "foo.txt"), windowsExistingDir)
+	assertTargetFile(t, ctx, targetFiler, "existingdir/foo.txt")
+
+	// Try to copy file to a non-existent directory with trailing backslash - should fail
+	windowsNonExistentDir := filepath.Join(filepath.FromSlash(targetDir), "nonexistent", "mydir") + "\\"
+	_, _, err = testcli.RequireErrorRun(t, ctx, "fs", "cp", filepath.Join(filepath.FromSlash(sourceDir), "foo.txt"), windowsNonExistentDir)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "directory")
+	assert.Contains(t, err.Error(), "no such directory")
+
+	// Also test with forward slash on Windows (should also work)
+	forwardSlashDir := path.Join(targetDir, "nonexistent2", "mydir2") + "/"
+	_, _, err = testcli.RequireErrorRun(t, ctx, "fs", "cp", path.Join(sourceDir, "foo.txt"), forwardSlashDir)
+	assert.Error(t, err)
+	assert.Contains(t, err.Error(), "directory")
+	assert.Contains(t, err.Error(), "no such directory")
+}