Fix: Buildifier cannot find .buildifier-tables.json when run from a subdirectory (#1312)

* Fix: Resolve Relative Path Issues for .buildifier-tables.json in AddTables Configuration

This commit addresses an issue where Buildifier fails to locate the .buildifier-tables.json file when specifying a relative path in the AddTables configuration and running Buildifier from subdirectories within the workspace.

* Fix TestFindTablePath: wrong tmp path, removing, result comparing

* Refactoring: minor fix test style

* Small code improvements

* Fix macOs test failuer: On MacOS '/tmp' is a symlink to '/private/tmp'

Author: dpleshakov

buildifier/config/config.go

@@ -67,6 +67,28 @@ func FindConfigPath(rootDir string) string {
 	return filepath.Join(dirname, buildifierJSONFilename)
 }
 
+// findTablesPath locates the specified table file starting from the process's
+// current working directory. It searches upward through the directory tree
+// until the file is found or the root of the workspace is reached.
+func findTablesPath(file string) (string, error) {
+	if wspace.IsRegularFile(file) {
+		return file, nil
+	}
+	rootDir, _ := os.Getwd()
+	dirname, err := wspace.Find(
+		rootDir,
+		map[string]func(os.FileInfo) bool{
+			file: func(fi os.FileInfo) bool {
+				return fi.Mode()&os.ModeType == 0
+			},
+		},
+	)
+	if err != nil {
+		return file, err
+	}
+	return filepath.Join(dirname, file), nil
+}
+
 // Config is used to configure buildifier
 type Config struct {
 	// InputType determines the input file type: build (for BUILD files), bzl
@@ -190,14 +212,22 @@ func (c *Config) Validate(args []string) error {
 	}
 
 	if c.TablesPath != "" {
-		if err := tables.ParseAndUpdateJSONDefinitions(c.TablesPath, false); err != nil {
-			return fmt.Errorf("failed to parse %s for -tables: %w", c.TablesPath, err)
+		foundTablesPath, err := findTablesPath(c.TablesPath)
+		if err != nil {
+			return fmt.Errorf("failed to find %s for -tables: %w", c.TablesPath, err)
+		}
+		if err := tables.ParseAndUpdateJSONDefinitions(foundTablesPath, false); err != nil {
+			return fmt.Errorf("failed to parse %s for -tables: %w", foundTablesPath, err)
 		}
 	}
 
 	if c.AddTablesPath != "" {
-		if err := tables.ParseAndUpdateJSONDefinitions(c.AddTablesPath, true); err != nil {
-			return fmt.Errorf("failed to parse %s for -add_tables: %w", c.AddTablesPath, err)
+		foundTablesPath, err := findTablesPath(c.AddTablesPath)
+		if err != nil {
+			return fmt.Errorf("failed to find %s for -add_tables: %w", c.AddTablesPath, err)
+		}
+		if err := tables.ParseAndUpdateJSONDefinitions(foundTablesPath, true); err != nil {
+			return fmt.Errorf("failed to parse %s for -add_tables: %w", foundTablesPath, err)
 		}
 	}
 

buildifier/config/config_test.go

@@ -581,3 +581,96 @@ func TestFindConfigPath(t *testing.T) {
 		})
 	}
 }
+
+func TestFindTablePath(t *testing.T) {
+	tests := []struct {
+		name    string
+		file    string
+		files   []string
+		wd      string
+		want    string
+		wantErr error
+	}{
+		{
+			name:    "default",
+			file:    ".buildifier-tables.json",
+			files:   []string{".buildifier-tables.json"},
+			wd:      "",
+			want:    ".buildifier-tables.json",
+			wantErr: nil,
+		},
+		{
+			name:    "working-dir-is-subdir",
+			file:    ".buildifier-tables.json",
+			files:   []string{".buildifier-tables.json", "foo/BUILD.bazel"},
+			wd:      "foo",
+			want:    ".buildifier-tables.json",
+			wantErr: nil,
+		},
+		{
+			name:    "relative-subdir",
+			file:    "bar/.buildifier-tables.json",
+			files:   []string{"bar/.buildifier-tables.json", "foo/BUILD.bazel"},
+			wd:      "foo",
+			want:    "bar/.buildifier-tables.json",
+			wantErr: nil,
+		},
+		{
+			name:    "file-not-found",
+			file:    "nonexistentFile.json",
+			files:   []string{".buildifier-tables.json"},
+			wd:      "",
+			want:    "nonexistentFile.json",
+			wantErr: os.ErrNotExist,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			tmp := t.TempDir()
+
+			// On MacOS "/tmp" is a symlink to "/private/tmp". Resolve it to make the testing easier
+			tmp, err := filepath.EvalSymlinks(tmp)
+			if err != nil {
+				t.Fatalf("failed to resolve symlink for temporary directory: %v", err)
+			}
+			t.Log("tmp:", tmp)
+
+			if tc.wd != "" {
+				if err := os.MkdirAll(filepath.Join(tmp, tc.wd), os.ModePerm); err != nil {
+					t.Fatalf("failed to create working directory: %v", err)
+				}
+				if err := os.Chdir(filepath.Join(tmp, tc.wd)); err != nil {
+					t.Fatalf("failed to change working directory: %v", err)
+				}
+			} else {
+				if err := os.Chdir(tmp); err != nil {
+					t.Fatalf("failed to change working directory: %v", err)
+				}
+			}
+
+			for _, file := range tc.files {
+				filePath := filepath.Join(tmp, file)
+				dir := filepath.Dir(filePath)
+				if err := os.MkdirAll(dir, os.ModePerm); err != nil {
+					t.Fatalf("failed to create directory %v: %v", dir, err)
+				}
+				if err := os.WriteFile(filePath, []byte("{}"), 0644); err != nil {
+					t.Fatalf("failed to create file %v: %v", filePath, err)
+				}
+			}
+
+			got, err := findTablesPath(tc.file)
+			got = strings.TrimPrefix(got, tmp)
+			got = strings.TrimPrefix(got, "/")
+
+			if (err != nil) != (tc.wantErr != nil) || (err != nil && tc.wantErr.Error() != err.Error()) {
+				t.Errorf("FindTablePath wantErr = %q, error = %q", tc.wantErr, err)
+			}
+
+			if tc.want != got {
+				t.Errorf("FindTablePath want = %q, got = %q", tc.want, got)
+			}
+		})
+	}
+}