From 809eb8cdeea7e053e360561e6cef33526f580655 Mon Sep 17 00:00:00 2001
From: Sebastiaan van Stijn <github@gone.nl>
Date: Sat, 3 Feb 2024 17:57:48 +0100
Subject: [PATCH] images: print hint when invoking "docker images" with
 ambiguous argument

The `docker images` top-level subcommand predates the `docker <object> <verb>`
convention (e.g. `docker image ls`), but accepts a positional argument to
search/filter images by name (globbing). It's common for users to accidentally
mistake these commands, and to use (e.g.) `docker images ls`, expecting
to see all images, but ending up with an empty list because no image named
"ls" was found.

Disallowing these search-terms would be a breaking change, but we can print
and informational message to help the users correct their mistake.

Before this patch:

    docker images ls
    REPOSITORY   TAG       IMAGE ID   CREATED   SIZE

With this patch applied:

    docker images ls
    REPOSITORY   TAG       IMAGE ID   CREATED   SIZE

    No images found matching "ls": did you mean "docker image ls"?

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
---
 cli/command/image/list.go                     | 48 ++++++++++++++++++-
 cli/command/image/list_test.go                | 14 ++++++
 .../testdata/list-command-ambiguous.golden    |  2 +
 3 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 cli/command/image/testdata/list-command-ambiguous.golden

diff --git a/cli/command/image/list.go b/cli/command/image/list.go
index a52db38261..a691efed45 100644
--- a/cli/command/image/list.go
+++ b/cli/command/image/list.go
@@ -2,6 +2,8 @@ package image
 
 import (
 	"context"
+	"fmt"
+	"io"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
@@ -21,6 +23,7 @@ type imagesOptions struct {
 	showDigests bool
 	format      string
 	filter      opts.FilterOpt
+	calledAs    string
 }
 
 // NewImagesCommand creates a new `docker images` command
@@ -35,6 +38,10 @@ func NewImagesCommand(dockerCLI command.Cli) *cobra.Command {
 			if len(args) > 0 {
 				options.matchName = args[0]
 			}
+			// Pass through how the command was invoked. We use this to print
+			// warnings when an ambiguous argument was passed when using the
+			// legacy (top-level) "docker images" subcommand.
+			options.calledAs = cmd.CalledAs()
 			return runImages(cmd.Context(), dockerCLI, options)
 		},
 		Annotations: map[string]string{
@@ -93,5 +100,44 @@ func runImages(ctx context.Context, dockerCLI command.Cli, options imagesOptions
 		},
 		Digest: options.showDigests,
 	}
-	return formatter.ImageWrite(imageCtx, images)
+	if err := formatter.ImageWrite(imageCtx, images); err != nil {
+		return err
+	}
+	if options.matchName != "" && len(images) == 0 && options.calledAs == "images" {
+		printAmbiguousHint(dockerCLI.Err(), options.matchName)
+	}
+	return nil
+}
+
+// printAmbiguousHint prints an informational warning if the provided filter
+// argument is ambiguous.
+//
+// The "docker images" top-level subcommand predates the "docker <object> <verb>"
+// convention (e.g. "docker image ls"), but accepts a positional argument to
+// search/filter images by name (globbing). It's common for users to accidentally
+// mistake these commands, and to use (e.g.) "docker images ls", expecting
+// to see all images, but ending up with an empty list because no image named
+// "ls" was found.
+//
+// Disallowing these search-terms would be a breaking change, but we can print
+// and informational message to help the users correct their mistake.
+func printAmbiguousHint(stdErr io.Writer, matchName string) {
+	switch matchName {
+	// List of subcommands for "docker image" and their aliases (see "docker image --help"):
+	case "build",
+		"history",
+		"import",
+		"inspect",
+		"list",
+		"load",
+		"ls",
+		"prune",
+		"pull",
+		"push",
+		"rm",
+		"save",
+		"tag":
+
+		_, _ = fmt.Fprintf(stdErr, "\nNo images found matching %q: did you mean \"docker image %[1]s\"?\n", matchName)
+	}
 }
diff --git a/cli/command/image/list_test.go b/cli/command/image/list_test.go
index d8b583d8d4..2b5259b1f5 100644
--- a/cli/command/image/list_test.go
+++ b/cli/command/image/list_test.go
@@ -95,3 +95,17 @@ func TestNewListCommandAlias(t *testing.T) {
 	assert.Check(t, cmd.HasAlias("list"))
 	assert.Check(t, !cmd.HasAlias("other"))
 }
+
+func TestNewListCommandAmbiguous(t *testing.T) {
+	cli := test.NewFakeCli(&fakeClient{})
+	cmd := NewImagesCommand(cli)
+	cmd.SetOut(io.Discard)
+
+	// Set the Use field to mimic that the command was called as "docker images",
+	// not "docker image ls".
+	cmd.Use = "images"
+	cmd.SetArgs([]string{"ls"})
+	err := cmd.Execute()
+	assert.NilError(t, err)
+	golden.Assert(t, cli.ErrBuffer().String(), "list-command-ambiguous.golden")
+}
diff --git a/cli/command/image/testdata/list-command-ambiguous.golden b/cli/command/image/testdata/list-command-ambiguous.golden
new file mode 100644
index 0000000000..2d8ffa9efa
--- /dev/null
+++ b/cli/command/image/testdata/list-command-ambiguous.golden
@@ -0,0 +1,2 @@
+
+No images found matching "ls": did you mean "docker image ls"?