Repository: alecthomas/kong
Pull request #297 (2022-05-01 18:51:12)

fix: existing file/dir always checks default
Updated existing-file/dir mappers to early return if already set successfully, such that the default value is not checked if a user-provided value has already been verified successfully.  This is intended to avoid failures when a default value does not exist and the user-provided value does, allowing for successful continuation.

Added tests for existing-file/dir mappers.

Intended to fix https://github.com/alecthomas/kong/issues/26

pyq-lsa: cc @alecthomas Friendly bump
alecthomas: LGTM thanks!

Mind rebasing and I'll merge?
pyq-lsa: Pulled in changes, but looks like that introduced an unused import (and it looks like https://github.com/alecthomas/kong/pull/305 is already intended to address).  I'll wait for that PR to land, re-sync, then we should be good!
alecthomas: #305 is merged!
pyq-lsa: Rebased, I'm GTG.

Patch:
diff --git a/mapper.go b/mapper.go
index daf09bd..796a6cc 100644
--- a/mapper.go
+++ b/mapper.go
@@ -612,6 +612,14 @@ func existingFileMapper(r *Registry) MapperFunc {
 		if err != nil {
 			return err
 		}
+
+		if ctx.Value.Set {
+			// early return to avoid checking extra files that may not exist;
+			// this hack only works because the value provided on the cli is
+			// checked before the default value is checked (if default is set).
+			return nil
+		}
+
 		if path != "-" {
 			path = ExpandPath(path)
 			stat, err := os.Stat(path)
@@ -640,6 +648,14 @@ func existingDirMapper(r *Registry) MapperFunc {
 		if err != nil {
 			return err
 		}
+
+		if ctx.Value.Set {
+			// early return to avoid checking extra dirs that may not exist;
+			// this hack only works because the value provided on the cli is
+			// checked before the default value is checked (if default is set).
+			return nil
+		}
+
 		path = ExpandPath(path)
 		stat, err := os.Stat(path)
 		if err != nil {


Test patch:
diff --git a/mapper_test.go b/mapper_test.go
index c162cb8..43818ec 100644
--- a/mapper_test.go
+++ b/mapper_test.go
@@ -423,6 +423,72 @@ func TestFileMapper(t *testing.T) {
 	require.Equal(t, os.Stdin, cli.File)
 }
 
+//nolint:dupl
+func TestExistingFileMapper(t *testing.T) {
+	type CLI struct {
+		File string `type:"existingfile"`
+	}
+	var cli CLI
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"--file", "testdata/file.txt"})
+	require.NoError(t, err)
+	require.NotNil(t, cli.File)
+	p = mustNew(t, &cli)
+	_, err = p.Parse([]string{"--file", "testdata/missing.txt"})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "missing.txt: no such file or directory")
+	p = mustNew(t, &cli)
+	_, err = p.Parse([]string{"--file", "testdata/"})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "exists but is a directory")
+}
+
+func TestExistingFileMapperDefaultMissing(t *testing.T) {
+	type CLI struct {
+		File string `type:"existingfile" default:"testdata/missing.txt"`
+	}
+	var cli CLI
+	p := mustNew(t, &cli)
+	file := "testdata/file.txt"
+	_, err := p.Parse([]string{"--file", file})
+	require.NoError(t, err)
+	require.NotNil(t, cli.File)
+	require.Contains(t, cli.File, file)
+}
+
+//nolint:dupl
+func TestExistingDirMapper(t *testing.T) {
+	type CLI struct {
+		Dir string `type:"existingdir"`
+	}
+	var cli CLI
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"--dir", "testdata/"})
+	require.NoError(t, err)
+	require.NotNil(t, cli.Dir)
+	p = mustNew(t, &cli)
+	_, err = p.Parse([]string{"--dir", "missingdata/"})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "missingdata: no such file or directory")
+	p = mustNew(t, &cli)
+	_, err = p.Parse([]string{"--dir", "testdata/file.txt"})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "exists but is not a directory")
+}
+
+func TestExistingDirMapperDefaultMissing(t *testing.T) {
+	type CLI struct {
+		Dir string `type:"existingdir" default:"missing-dir"`
+	}
+	var cli CLI
+	p := mustNew(t, &cli)
+	dir := "testdata"
+	_, err := p.Parse([]string{"--dir", dir})
+	require.NoError(t, err)
+	require.NotNil(t, cli.Dir)
+	require.Contains(t, cli.Dir, dir)
+}
+
 func TestMapperPlaceHolder(t *testing.T) {
 	var cli struct {
 		Flag string