Merge pull request docker#4849 from thaJeztah/image_list_dedup

images: print hint when invoking "docker images" with ambiguous argument

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,10 +23,11 @@ type imagesOptions struct {
 	showDigests bool
 	format      string
 	filter      opts.FilterOpt
+	calledAs    string
 }
 
 // NewImagesCommand creates a new `docker images` command
-func NewImagesCommand(dockerCli command.Cli) *cobra.Command {
+func NewImagesCommand(dockerCLI command.Cli) *cobra.Command {
 	options := imagesOptions{filter: opts.NewFilterOpt()}
 
 	cmd := &cobra.Command{
@@ -35,7 +38,11 @@ func NewImagesCommand(dockerCli command.Cli) *cobra.Command {
 			if len(args) > 0 {
 				options.matchName = args[0]
 			}
-			return runImages(cmd.Context(), dockerCli, options)
+			// 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{
 			"category-top": "7",
@@ -55,45 +62,82 @@ func NewImagesCommand(dockerCli command.Cli) *cobra.Command {
 	return cmd
 }
 
-func newListCommand(dockerCli command.Cli) *cobra.Command {
-	cmd := *NewImagesCommand(dockerCli)
+func newListCommand(dockerCLI command.Cli) *cobra.Command {
+	cmd := *NewImagesCommand(dockerCLI)
 	cmd.Aliases = []string{"list"}
 	cmd.Use = "ls [OPTIONS] [REPOSITORY[:TAG]]"
 	return &cmd
 }
 
-func runImages(ctx context.Context, dockerCli command.Cli, options imagesOptions) error {
+func runImages(ctx context.Context, dockerCLI command.Cli, options imagesOptions) error {
 	filters := options.filter.Value()
 	if options.matchName != "" {
 		filters.Add("reference", options.matchName)
 	}
 
-	listOptions := image.ListOptions{
+	images, err := dockerCLI.Client().ImageList(ctx, image.ListOptions{
 		All:     options.all,
 		Filters: filters,
-	}
-
-	images, err := dockerCli.Client().ImageList(ctx, listOptions)
+	})
 	if err != nil {
 		return err
 	}
 
 	format := options.format
 	if len(format) == 0 {
-		if len(dockerCli.ConfigFile().ImagesFormat) > 0 && !options.quiet {
-			format = dockerCli.ConfigFile().ImagesFormat
+		if len(dockerCLI.ConfigFile().ImagesFormat) > 0 && !options.quiet {
+			format = dockerCLI.ConfigFile().ImagesFormat
 		} else {
 			format = formatter.TableFormatKey
 		}
 	}
 
 	imageCtx := formatter.ImageContext{
 		Context: formatter.Context{
-			Output: dockerCli.Out(),
+			Output: dockerCLI.Out(),
 			Format: formatter.NewImageFormat(format, options.quiet, options.showDigests),
 			Trunc:  !options.noTrunc,
 		},
 		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)
+	}
 }

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")
+}

cli/command/image/testdata/list-command-ambiguous.golden

@@ -0,0 +1,2 @@
+
+No images found matching "ls": did you mean "docker image ls"?