Skip to content

Commit 9bf441a

Browse files
JoeWang1127JoeWang1127
authored
fix(internal/librarian/golang): check directory existence before formatting (#4375)
This pull request refactors the Go library formatting logic to ensure that `goimports` is only executed on directories that actually exist. Additionally, this PR extracts the `Format` function and its tests into dedicated files (`format.go` and `format_test.go`) and expands the test suite with table-driven tests to verify behavior across all directory existence scenarios. For #3617 Fixes #4376
1 parent b2de54a commit 9bf441a

4 files changed

Lines changed: 179 additions & 62 deletions

File tree

‎internal/librarian/golang/format.go‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package golang
16+
17+
import (
18+
"context"
19+
"os"
20+
"path/filepath"
21+
22+
"github.com/googleapis/librarian/internal/command"
23+
"github.com/googleapis/librarian/internal/config"
24+
)
25+
26+
// Format formats a generated Go library.
27+
func Format(ctx context.Context, library *config.Library) error {
28+
outDir, err := filepath.Abs(library.Output)
29+
if err != nil {
30+
return err
31+
}
32+
args, err := processArgs(outDir, library.Name)
33+
if err != nil {
34+
return err
35+
}
36+
if len(args) == 1 {
37+
// No need to format the library if library directory doesn't exist,
38+
// e.g., root-module.
39+
return nil
40+
}
41+
return command.Run(ctx, "goimports", args...)
42+
}
43+
44+
func processArgs(outDir, libraryName string) ([]string, error) {
45+
args := []string{"-w"}
46+
libraryDir := filepath.Join(outDir, libraryName)
47+
if _, err := os.Stat(libraryDir); err == nil {
48+
args = append(args, libraryDir)
49+
} else if !os.IsNotExist(err) {
50+
return nil, err
51+
}
52+
snippetDir := snippetDirectory(outDir, libraryName)
53+
if _, err := os.Stat(snippetDir); err == nil {
54+
args = append(args, snippetDir)
55+
} else if !os.IsNotExist(err) {
56+
return nil, err
57+
}
58+
return args, nil
59+
}

‎internal/librarian/golang/format_test.go‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
// Copyright 2026 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package golang
16+
17+
import (
18+
"os"
19+
"path/filepath"
20+
"testing"
21+
22+
"github.com/google/go-cmp/cmp"
23+
"github.com/googleapis/librarian/internal/config"
24+
"github.com/googleapis/librarian/internal/testhelper"
25+
)
26+
27+
func TestFormat(t *testing.T) {
28+
testhelper.RequireCommand(t, "goimports")
29+
for _, test := range []struct {
30+
name string
31+
library *config.Library
32+
goFilePath []string
33+
}{
34+
{
35+
name: "library path and snippet directory exist",
36+
library: &config.Library{
37+
Name: "example",
38+
},
39+
goFilePath: []string{
40+
"example",
41+
"internal/generated/snippets/example",
42+
},
43+
},
44+
{
45+
// This is true for the root module, though the snippet
46+
// directory should also not exist.
47+
name: "library path does not exist",
48+
library: &config.Library{
49+
Name: "example",
50+
},
51+
goFilePath: []string{
52+
"internal/generated/snippets/example",
53+
},
54+
},
55+
{
56+
name: "snippet directory does not exist",
57+
library: &config.Library{
58+
Name: "example",
59+
},
60+
goFilePath: []string{
61+
"example",
62+
},
63+
},
64+
{
65+
name: "library path and snippet directory do not exist",
66+
library: &config.Library{
67+
Name: "example",
68+
},
69+
},
70+
} {
71+
t.Run(test.name, func(t *testing.T) {
72+
outDir := t.TempDir()
73+
test.library.Output = outDir
74+
unformatted := `package main
75+
76+
import (
77+
"fmt"
78+
"os"
79+
)
80+
81+
func main() {
82+
fmt.Println("Hello World")
83+
}
84+
`
85+
want := `package main
86+
87+
import (
88+
"fmt"
89+
)
90+
91+
func main() {
92+
fmt.Println("Hello World")
93+
}
94+
`
95+
for _, aPath := range test.goFilePath {
96+
path := filepath.Join(outDir, aPath)
97+
if err := os.MkdirAll(path, 0755); err != nil {
98+
t.Fatal(err)
99+
}
100+
if err := os.WriteFile(filepath.Join(path, "example.go"), []byte(unformatted), 0644); err != nil {
101+
t.Fatal(err)
102+
}
103+
}
104+
if err := Format(t.Context(), test.library); err != nil {
105+
t.Fatal(err)
106+
}
107+
for _, aPath := range test.goFilePath {
108+
goFile := filepath.Join(outDir, aPath, "example.go")
109+
gotBytes, err := os.ReadFile(goFile)
110+
if err != nil {
111+
t.Fatal(err)
112+
}
113+
got := string(gotBytes)
114+
if diff := cmp.Diff(want, got); diff != "" {
115+
t.Errorf("mismatch (-want +got):\n%s", diff)
116+
}
117+
}
118+
})
119+
}
120+
}

‎internal/librarian/golang/generate.go‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,6 @@ func GenerateLibraries(ctx context.Context, libraries []*config.Library, googlea
5555
return nil
5656
}
5757

58-
// Format formats a generated Go library.
59-
func Format(ctx context.Context, library *config.Library) error {
60-
outDir, err := filepath.Abs(library.Output)
61-
if err != nil {
62-
return err
63-
}
64-
args := []string{"-w", filepath.Join(outDir, library.Name)}
65-
// TODO(https://github.com/googleapis/librarian/issues/4297), refactor this function
66-
// to use import path.
67-
snippetDir := snippetDirectory(outDir, library.Name)
68-
if _, err := os.Stat(snippetDir); err == nil {
69-
args = append(args, snippetDir)
70-
}
71-
return command.Run(ctx, "goimports", args...)
72-
}
73-
7458
// generate generates a Go client library.
7559
func generate(ctx context.Context, library *config.Library, googleapisDir string) error {
7660
if len(library.APIs) == 0 {

‎internal/librarian/golang/generate_test.go‎

Lines changed: 0 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -360,52 +360,6 @@ func TestGenerate(t *testing.T) {
360360
}
361361
}
362362

363-
func TestFormat(t *testing.T) {
364-
testhelper.RequireCommand(t, "goimports")
365-
outDir := t.TempDir()
366-
goFile := filepath.Join(outDir, "test.go")
367-
unformatted := `package main
368-
369-
import (
370-
"fmt"
371-
"os"
372-
)
373-
374-
func main() {
375-
fmt.Println("Hello World")
376-
}
377-
`
378-
if err := os.WriteFile(goFile, []byte(unformatted), 0644); err != nil {
379-
t.Fatal(err)
380-
}
381-
382-
library := &config.Library{
383-
Output: outDir,
384-
}
385-
if err := Format(t.Context(), library); err != nil {
386-
t.Fatal(err)
387-
}
388-
389-
gotBytes, err := os.ReadFile(goFile)
390-
if err != nil {
391-
t.Fatal(err)
392-
}
393-
got := string(gotBytes)
394-
want := `package main
395-
396-
import (
397-
"fmt"
398-
)
399-
400-
func main() {
401-
fmt.Println("Hello World")
402-
}
403-
`
404-
if diff := cmp.Diff(want, got); diff != "" {
405-
t.Errorf("mismatch (-want +got):\n%s", diff)
406-
}
407-
}
408-
409363
func TestGenerateREADME(t *testing.T) {
410364
dir := t.TempDir()
411365
moduleRoot := filepath.Join(dir, "secretmanager")

0 commit comments

Comments
 (0)