Improve localizer readability (#4860)

* Replace '%s' with %q

* Change ambiguous cli-"Arg" suffix in func arg names

* Remove repetitive "loc" in names

* Apply readability changes to localizer

* Fix comment

api/internal/localizer/localizer.go

@@ -27,7 +27,7 @@ type Localizer struct {
 	rFactory  *resmap.Factory
 	pLdr      *pLdr.Loader
 
-	// underlying type is LocLoader
+	// underlying type is Loader
 	ldr ifc.Loader
 
 	// destination directory in newDir that mirrors ldr's current root.
@@ -35,7 +35,7 @@ type Localizer struct {
 }
 
 // NewLocalizer is the factory method for Localizer
-func NewLocalizer(ldr *LocLoader, validator ifc.Validator, rFactory *resmap.Factory, pLdr *pLdr.Loader) (*Localizer, error) {
+func NewLocalizer(ldr *Loader, validator ifc.Validator, rFactory *resmap.Factory, pLdr *pLdr.Loader) (*Localizer, error) {
 	toDst, err := filepath.Rel(ldr.args.Scope.String(), ldr.Root())
 	if err != nil {
 		log.Fatalf("cannot find path from directory %q to %q inside directory: %s", ldr.args.Scope.String(),

api/internal/localizer/localizer_test.go

@@ -57,8 +57,8 @@ func addFiles(t *testing.T, fSys filesys.FileSystem, parentDir string, files map
 func createLocalizer(t *testing.T, fSys filesys.FileSystem, target string, scope string, newDir string) *Localizer {
 	t.Helper()
 
-	// no need to re-test LocLoader
-	ldr, _, err := NewLocLoader(target, scope, newDir, fSys)
+	// no need to re-test Loader
+	ldr, _, err := NewLoader(target, scope, newDir, fSys)
 	require.NoError(t, err)
 	rmFactory := resmap.NewFactory(resource.NewFactory(&hasher.Hasher{}))
 	lc, err := NewLocalizer(

api/internal/localizer/locloader.go

@@ -16,8 +16,8 @@ import (
 
 const DstPrefix = "localized"
 
-// LocArgs holds localize arguments
-type LocArgs struct {
+// Args holds localize arguments
+type Args struct {
 	// target; local copy if remote
 	Target filesys.ConfirmedDir
 
@@ -28,55 +28,54 @@ type LocArgs struct {
 	NewDir filesys.ConfirmedDir
 }
 
-// LocLoader is the Loader for kustomize localize. It is an ifc.Loader that enforces localize constraints.
-type LocLoader struct {
+// Loader is an ifc.Loader that enforces additional constraints specific to kustomize localize.
+type Loader struct {
 	fSys filesys.FileSystem
 
-	args *LocArgs
+	args *Args
 
-	// loader at locLoader's current directory
+	// loader at Loader's current directory
 	ifc.Loader
 
-	// whether locLoader and all its ancestors are the result of local references
+	// whether Loader and all its ancestors are the result of local references
 	local bool
 }
 
-var _ ifc.Loader = &LocLoader{}
+var _ ifc.Loader = &Loader{}
 
-// NewLocLoader is the factory method for LocLoader, under localize constraints, at targetArg. For invalid localize arguments,
-// NewLocLoader returns an error.
-func NewLocLoader(targetArg string, scopeArg string, newDirArg string, fSys filesys.FileSystem) (*LocLoader, LocArgs, error) {
+// NewLoader is the factory method for Loader, under localize constraints, at rawTarget. For invalid localize arguments,
+// NewLoader returns an error.
+func NewLoader(rawTarget string, rawScope string, rawNewDir string, fSys filesys.FileSystem) (*Loader, Args, error) {
 	// check earlier to avoid cleanup
-	repoSpec, err := git.NewRepoSpecFromURL(targetArg)
+	repoSpec, err := git.NewRepoSpecFromURL(rawTarget)
 	if err == nil && repoSpec.Ref == "" {
-		return nil, LocArgs{},
-			errors.Errorf("localize remote root '%s' missing ref query string parameter", targetArg)
+		return nil, Args{}, errors.Errorf("localize remote root %q missing ref query string parameter", rawTarget)
 	}
 
 	// for security, should enforce load restrictions
-	ldr, err := loader.NewLoader(loader.RestrictionRootOnly, targetArg, fSys)
+	ldr, err := loader.NewLoader(loader.RestrictionRootOnly, rawTarget, fSys)
 	if err != nil {
-		return nil, LocArgs{}, errors.WrapPrefixf(err, "unable to establish localize target '%s'", targetArg)
+		return nil, Args{}, errors.WrapPrefixf(err, "unable to establish localize target %q", rawTarget)
 	}
 
-	scope, err := establishScope(scopeArg, targetArg, ldr, fSys)
+	scope, err := establishScope(rawScope, rawTarget, ldr, fSys)
 	if err != nil {
 		_ = ldr.Cleanup()
-		return nil, LocArgs{}, errors.WrapPrefixf(err, "invalid localize scope '%s'", scopeArg)
+		return nil, Args{}, errors.WrapPrefixf(err, "invalid localize scope %q", rawScope)
 	}
 
-	newDir, err := createNewDir(newDirArg, ldr, repoSpec, fSys)
+	newDir, err := createNewDir(rawNewDir, ldr, repoSpec, fSys)
 	if err != nil {
 		_ = ldr.Cleanup()
-		return nil, LocArgs{}, errors.WrapPrefixf(err, "invalid localize destination '%s'", newDirArg)
+		return nil, Args{}, errors.WrapPrefixf(err, "invalid localize destination %q", rawNewDir)
 	}
 
-	args := LocArgs{
+	args := Args{
 		Target: filesys.ConfirmedDir(ldr.Root()),
 		Scope:  scope,
 		NewDir: newDir,
 	}
-	return &LocLoader{
+	return &Loader{
 		fSys:   fSys,
 		args:   &args,
 		Loader: ldr,
@@ -86,28 +85,28 @@ func NewLocLoader(targetArg string, scopeArg string, newDirArg string, fSys file
 
 // Load returns the contents of path if path is a valid localize file.
 // Otherwise, Load returns an error.
-func (ll *LocLoader) Load(path string) ([]byte, error) {
+func (ll *Loader) Load(path string) ([]byte, error) {
 	// checks in root, and thus in scope
 	content, err := ll.Loader.Load(path)
 	if err != nil {
 		return nil, errors.WrapPrefixf(err, "invalid file reference")
 	}
 	if filepath.IsAbs(path) {
-		return nil, errors.Errorf("absolute paths not yet supported in alpha: file path '%s' is absolute", path)
+		return nil, errors.Errorf("absolute paths not yet supported in alpha: file path %q is absolute", path)
 	}
 	if ll.local {
 		abs := filepath.Join(ll.Root(), path)
 		dir, f, err := ll.fSys.CleanedAbs(abs)
 		if err != nil {
 			// should never happen
-			log.Fatalf(errors.WrapPrefixf(err, "cannot clean validated file path '%s'", abs).Error())
+			log.Fatalf(errors.WrapPrefixf(err, "cannot clean validated file path %q", abs).Error())
 		}
 		// target cannot reference newDir, as this load would've failed prior to localize;
 		// not a problem if remote because then reference could only be in newDir if repo copy,
 		// which will be cleaned, is inside newDir
 		if dir.HasPrefix(ll.args.NewDir) {
 			return nil, errors.Errorf(
-				"file path '%s' references into localize destination '%s'", dir.Join(f), ll.args.NewDir)
+				"file path %q references into localize destination %q", dir.Join(f), ll.args.NewDir)
 		}
 	}
 	return content, nil
@@ -115,7 +114,7 @@ func (ll *LocLoader) Load(path string) ([]byte, error) {
 
 // New returns a Loader at path if path is a valid localize root.
 // Otherwise, New returns an error.
-func (ll *LocLoader) New(path string) (ifc.Loader, error) {
+func (ll *Loader) New(path string) (ifc.Loader, error) {
 	ldr, err := ll.Loader.New(path)
 	if err != nil {
 		return nil, errors.WrapPrefixf(err, "invalid root reference")
@@ -123,17 +122,17 @@ func (ll *LocLoader) New(path string) (ifc.Loader, error) {
 
 	if repo := ldr.Repo(); repo == "" {
 		if ll.local && !filesys.ConfirmedDir(ldr.Root()).HasPrefix(ll.args.Scope) {
-			return nil, errors.Errorf("root '%s' outside localize scope '%s'", ldr.Root(), ll.args.Scope)
+			return nil, errors.Errorf("root %q outside localize scope %q", ldr.Root(), ll.args.Scope)
 		}
 		if ll.local && filesys.ConfirmedDir(ldr.Root()).HasPrefix(ll.args.NewDir) {
 			return nil, errors.Errorf(
-				"root '%s' references into localize destination '%s'", ldr.Root(), ll.args.NewDir)
+				"root %q references into localize destination %q", ldr.Root(), ll.args.NewDir)
 		}
 	} else if !hasRef(path) {
 		return nil, errors.Errorf("localize remote root %q missing ref query string parameter", path)
 	}
 
-	return &LocLoader{
+	return &Loader{
 		fSys:   ll.fSys,
 		args:   ll.args,
 		Loader: ldr,

api/internal/localizer/locloader_test.go

@@ -14,9 +14,9 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
-func checkNewLocLoader(req *require.Assertions, ldr *LocLoader, args *LocArgs, target string, scope string, newDir string, fSys filesys.FileSystem) {
+func checkNewLoader(req *require.Assertions, ldr *Loader, args *Args, target string, scope string, newDir string, fSys filesys.FileSystem) {
 	checkLoader(req, ldr, target)
-	checkLocArgs(req, args, target, scope, newDir, fSys)
+	checkArgs(req, args, target, scope, newDir, fSys)
 }
 
 func checkLoader(req *require.Assertions, ldr ifc.Loader, root string) {
@@ -24,7 +24,7 @@ func checkLoader(req *require.Assertions, ldr ifc.Loader, root string) {
 	req.Empty(ldr.Repo())
 }
 
-func checkLocArgs(req *require.Assertions, args *LocArgs, target string, scope string, newDir string, fSys filesys.FileSystem) {
+func checkArgs(req *require.Assertions, args *Args, target string, scope string, newDir string, fSys filesys.FileSystem) {
 	req.Equal(target, args.Target.String())
 	req.Equal(scope, args.Scope.String())
 	req.Equal(newDir, args.NewDir.String())
@@ -38,9 +38,9 @@ func TestLocalLoadNewAndCleanup(t *testing.T) {
 	var buf bytes.Buffer
 	log.SetOutput(&buf)
 	// typical setup
-	ldr, args, err := NewLocLoader("a", "/", "/newDir", fSys)
+	ldr, args, err := NewLoader("a", "/", "/newDir", fSys)
 	req.NoError(err)
-	checkNewLocLoader(req, ldr, &args, "/a", "/", "/newDir", fSys)
+	checkNewLoader(req, ldr, &args, "/a", "/", "/newDir", fSys)
 
 	fSysCopy := makeMemoryFs(t)
 	req.NoError(fSysCopy.Mkdir("/newDir"))
@@ -85,9 +85,9 @@ func TestNewLocLoaderDefaultForRootTarget(t *testing.T) {
 			req := require.New(t)
 			fSys := makeMemoryFs(t)
 
-			ldr, args, err := NewLocLoader(params.target, params.scope, "", fSys)
+			ldr, args, err := NewLoader(params.target, params.scope, "", fSys)
 			req.NoError(err)
-			checkNewLocLoader(req, ldr, &args, "/", "/", "/"+DstPrefix, fSys)
+			checkNewLoader(req, ldr, &args, "/", "/", "/"+DstPrefix, fSys)
 
 			// file in root, but nested
 			content, err := ldr.Load("a/pod.yaml")
@@ -112,9 +112,9 @@ func TestNewMultiple(t *testing.T) {
 
 	// default destination for non-file system root target
 	// destination outside of scope
-	ldr, args, err := NewLocLoader("/alpha/beta", "/alpha", "", fSys)
+	ldr, args, err := NewLoader("/alpha/beta", "/alpha", "", fSys)
 	req.NoError(err)
-	checkNewLocLoader(req, ldr, &args, "/alpha/beta", "/alpha", "/"+DstPrefix+"-beta", fSys)
+	checkNewLoader(req, ldr, &args, "/alpha/beta", "/alpha", "/"+DstPrefix+"-beta", fSys)
 
 	// nested child root that isn't cleaned
 	descLdr, err := ldr.New("../beta/gamma/delta")
@@ -173,7 +173,7 @@ func TestNewLocLoaderCwdNotRoot(t *testing.T) {
 			req := require.New(t)
 			fSys := makeWdFs(t)[test.wd]
 
-			ldr, args, err := NewLocLoader(test.target, test.scope, test.newDir, fSys)
+			ldr, args, err := NewLoader(test.target, test.scope, test.newDir, fSys)
 			req.NoError(err)
 			checkLoader(req, ldr, "a/b/c/d/e")
 
@@ -223,7 +223,8 @@ func TestNewLocLoaderFails(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			var buf bytes.Buffer
 			log.SetOutput(&buf)
-			_, _, err := NewLocLoader(params.target, params.scope, params.dest, makeMemoryFs(t))
+
+			_, _, err := NewLoader(params.target, params.scope, params.dest, makeMemoryFs(t))
 			require.Error(t, err)
 			require.Empty(t, buf.String())
 		})
@@ -234,9 +235,9 @@ func TestNewFails(t *testing.T) {
 	req := require.New(t)
 	fSys := makeMemoryFs(t)
 
-	ldr, args, err := NewLocLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys)
+	ldr, args, err := NewLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys)
 	req.NoError(err)
-	checkNewLocLoader(req, ldr, &args, "/alpha/beta/gamma", "/alpha", "/alpha/beta/gamma/newDir", fSys)
+	checkNewLoader(req, ldr, &args, "/alpha/beta/gamma", "/alpha", "/alpha/beta/gamma/newDir", fSys)
 
 	cases := map[string]string{
 		"outside scope":     "../../../a",
@@ -250,7 +251,7 @@ func TestNewFails(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			fSys := makeMemoryFs(t)
 
-			ldr, _, err := NewLocLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys)
+			ldr, _, err := NewLoader("/alpha/beta/gamma", "alpha", "alpha/beta/gamma/newDir", fSys)
 			require.NoError(t, err)
 
 			_, err = ldr.New(root)
@@ -263,9 +264,9 @@ func TestLoadFails(t *testing.T) {
 	req := require.New(t)
 	fSys := makeMemoryFs(t)
 
-	ldr, args, err := NewLocLoader("./a/../a", "/a/../a", "/a/newDir", fSys)
+	ldr, args, err := NewLoader("./a/../a", "/a/../a", "/a/newDir", fSys)
 	req.NoError(err)
-	checkNewLocLoader(req, ldr, &args, "/a", "/a", "/a/newDir", fSys)
+	checkNewLoader(req, ldr, &args, "/a", "/a", "/a/newDir", fSys)
 
 	cases := map[string]string{
 		"absolute path":     "/a/pod.yaml",
@@ -280,7 +281,7 @@ func TestLoadFails(t *testing.T) {
 			req := require.New(t)
 			fSys := makeMemoryFs(t)
 
-			ldr, _, err := NewLocLoader("./a/../a", "/a/../a", "/a/newDir", fSys)
+			ldr, _, err := NewLoader("./a/../a", "/a/../a", "/a/newDir", fSys)
 			req.NoError(err)
 
 			req.NoError(fSys.WriteFile("/a/newDir/pod.yaml", []byte(podConfiguration)))

api/internal/localizer/util.go

@@ -14,44 +14,44 @@ import (
 	"sigs.k8s.io/kustomize/kyaml/filesys"
 )
 
-// establishScope returns the effective scope given localize arguments and targetLdr at targetArg. For remote targetArg,
+// establishScope returns the effective scope given localize arguments and targetLdr at rawTarget. For remote targetArg,
 // the effective scope is the downloaded repo.
-func establishScope(scopeArg string, targetArg string, targetLdr ifc.Loader, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) {
+func establishScope(rawScope string, rawTarget string, targetLdr ifc.Loader, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) {
 	if repo := targetLdr.Repo(); repo != "" {
-		if scopeArg != "" {
-			return "", errors.Errorf("scope '%s' specified for remote localize target '%s'", scopeArg, targetArg)
+		if rawScope != "" {
+			return "", errors.Errorf("scope %q specified for remote localize target %q", rawScope, rawTarget)
 		}
 		return filesys.ConfirmedDir(repo), nil
 	}
 	// default scope
-	if scopeArg == "" {
+	if rawScope == "" {
 		return filesys.ConfirmedDir(targetLdr.Root()), nil
 	}
-	scope, err := filesys.ConfirmDir(fSys, scopeArg)
+	scope, err := filesys.ConfirmDir(fSys, rawScope)
 	if err != nil {
 		return "", errors.WrapPrefixf(err, "unable to establish localize scope")
 	}
 	if !filesys.ConfirmedDir(targetLdr.Root()).HasPrefix(scope) {
-		return scope, errors.Errorf("localize scope '%s' does not contain target '%s' at '%s'",
-			scopeArg, targetArg, targetLdr.Root())
+		return scope, errors.Errorf("localize scope %q does not contain target %q at %q", rawScope, rawTarget,
+			targetLdr.Root())
 	}
 	return scope, nil
 }
 
 // createNewDir returns the localize destination directory or error. Note that spec is nil if targetLdr is at local
 // target.
-func createNewDir(newDirArg string, targetLdr ifc.Loader, spec *git.RepoSpec, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) {
-	if newDirArg == "" {
-		newDirArg = defaultNewDir(targetLdr, spec)
+func createNewDir(rawNewDir string, targetLdr ifc.Loader, spec *git.RepoSpec, fSys filesys.FileSystem) (filesys.ConfirmedDir, error) {
+	if rawNewDir == "" {
+		rawNewDir = defaultNewDir(targetLdr, spec)
 	}
-	if fSys.Exists(newDirArg) {
-		return "", errors.Errorf("localize destination '%s' already exists", newDirArg)
+	if fSys.Exists(rawNewDir) {
+		return "", errors.Errorf("localize destination %q already exists", rawNewDir)
 	}
 	// destination directory must sit in an existing directory
-	if err := fSys.Mkdir(newDirArg); err != nil {
+	if err := fSys.Mkdir(rawNewDir); err != nil {
 		return "", errors.WrapPrefixf(err, "unable to create localize destination directory")
 	}
-	newDir, err := filesys.ConfirmDir(fSys, newDirArg)
+	newDir, err := filesys.ConfirmDir(fSys, rawNewDir)
 	if err != nil {
 		if errCleanup := fSys.RemoveAll(newDir.String()); errCleanup != nil {
 			log.Printf("%s", errors.WrapPrefixf(errCleanup, "unable to clean localize destination").Error())