refactor(java): enhance postprocess keep logic, resource handling and style compliance

- Extract newKeepSet and path.Clean normalization for robust file preservation
- Refactor ExtractSnippets with extractSnippetsFromFile helper to ensure defer f.Close()
- Migrate os.IsNotExist to errors.Is(err, fs.ErrNotExist) across java package
- Standardize log/slog logging and correct unexported acronym casing
- Replace manual test assertions with cmp.Diff and remove non-thread-safe t.Chdir

Author: yangyzs

internal/librarian/java/clean.go

@@ -17,8 +17,8 @@ package java
 import (
 	"bufio"
 	"errors"
-	"fmt"
 	"io"
+	"log/slog"
 	"io/fs"
 	"os"
 	"path"
@@ -56,11 +56,8 @@ var (
 // It targets patterns like proto-*, grpc-*, and the main GAPIC module.
 func Clean(library *config.Library) error {
 	patterns := cleanPatterns(library)
-	fmt.Printf("Clean patterns for %s: %v\n", library.Output, patterns)
-	keepSet := make(map[string]bool)
-	for _, k := range library.Keep {
-		keepSet[strings.TrimSuffix(filepath.ToSlash(k), "/")] = true
-	}
+	slog.Debug("Clean patterns", "output", library.Output, "patterns", patterns)
+	keepSet := newKeepSet(library.Keep)
 	for pattern, useMarker := range patterns {
 		matches, err := filepath.Glob(filepath.Join(library.Output, pattern))
 		if err != nil {
@@ -145,8 +142,7 @@ func cleanPath(targetPath, root string, keepSet map[string]bool, useMarker bool)
 				return nil
 			}
 		}
-		fmt.Println("DELETING:", path)
-		fmt.Println("DELETING:", path)
+		slog.Info("Deleting path", "path", path)
 		return os.Remove(path)
 	})
 	if err != nil && !errors.Is(err, fs.ErrNotExist) {
@@ -173,15 +169,28 @@ func isDirNotEmpty(err error) bool {
 	return errors.Is(err, syscall.ENOTEMPTY) || errors.Is(err, syscall.EEXIST)
 }
 
-// shouldPreserve returns true if the given slash-separated path should be preserved
-// based on the keepSet or standard preservation patterns.
-// It also checks if any ancestor directory is in the keepSet.
-func shouldPreserve(p string, keepSet map[string]bool) bool {
-	if keepSet[p] || isDefaultPreserved(p) {
+// newKeepSet normalizes a list of keep paths using [path.Clean] and returns
+// a map for fast lookup.
+func newKeepSet(keep []string) map[string]bool {
+	keepSet := make(map[string]bool)
+	for _, k := range keep {
+		normalized := path.Clean(filepath.ToSlash(k))
+		if normalized == "." {
+			continue
+		}
+		keepSet[strings.TrimSuffix(normalized, "/")] = true
+	}
+	return keepSet
+}
+
+// isKept returns true if the path is explicitly kept by the user.
+func isKept(p string, keepSet map[string]bool) bool {
+	cleanP := path.Clean(p)
+	if keepSet[cleanP] {
 		return true
 	}
 
-	dir := path.Dir(p)
+	dir := path.Dir(cleanP)
 	for dir != "." && dir != "/" && dir != "" {
 		if keepSet[dir] {
 			return true
@@ -191,6 +200,13 @@ func shouldPreserve(p string, keepSet map[string]bool) bool {
 	return false
 }
 
+// shouldPreserve returns true if the given slash-separated path should be preserved
+// based on the keepSet or standard preservation patterns.
+// It also checks if any ancestor directory is in the keepSet.
+func shouldPreserve(p string, keepSet map[string]bool) bool {
+	return isKept(p, keepSet) || isDefaultPreserved(p)
+}
+
 func isDefaultPreserved(path string) bool {
 	return itTestRegexp.MatchString(path) || versionRegexp.MatchString(path)
 }

internal/librarian/java/generate_test.go

@@ -17,6 +17,7 @@ package java
 import (
 	"context"
 	"errors"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
@@ -1113,7 +1114,7 @@ func TestGenerateAPI_Gating(t *testing.T) {
 			if gotProtoDir {
 				resNameFile := filepath.Join(stagingProtoPath, "com", "google", "cloud", "secretmanager", "v1", "SecretName.java")
 				_, errRes := os.Stat(resNameFile)
-				gotResNameFiles := !os.IsNotExist(errRes)
+				gotResNameFiles := !errors.Is(errRes, fs.ErrNotExist)
 				if gotResNameFiles != test.wantResNameFiles {
 					t.Errorf("gotResNameFiles = %v, want %v (file: %s)", gotResNameFiles, test.wantResNameFiles, resNameFile)
 				}
@@ -1125,7 +1126,7 @@ func TestGenerateAPI_Gating(t *testing.T) {
 func assertDirExists(t *testing.T, path string, want bool, desc string) bool {
 	t.Helper()
 	_, err := os.Stat(path)
-	got := !os.IsNotExist(err)
+	got := !errors.Is(err, fs.ErrNotExist)
 	if got != want {
 		t.Errorf("expected %s existence to be %v, got %v (path: %s)", desc, want, got, path)
 	}

internal/librarian/java/postprocess.go

@@ -144,10 +144,7 @@ func postProcessAPI(ctx context.Context, params postProcessParams) error {
 	coords := params.coords()
 
 	if params.useGoPostprocessor {
-		keepSet := make(map[string]bool)
-		for _, k := range params.library.Keep {
-			keepSet[strings.TrimSuffix(filepath.ToSlash(k), "/")] = true
-		}
+		keepSet := newKeepSet(params.library.Keep)
 		if err := restructureModules(params, params.outDir, keepSet, params.outDir); err != nil {
 			return fmt.Errorf("failed to restructure direct to outDir: %w", err)
 		}
@@ -495,14 +492,10 @@ func copyProtos(protos []protoFileToCopy, destDir string) error {
 // then matched against the library's Keep configuration.
 func removeKeptFilesFromStaging(library *config.Library, outDir string) error {
 	stagingDir := stagingDir(outDir)
-	if _, err := os.Stat(stagingDir); os.IsNotExist(err) {
+	if _, err := os.Stat(stagingDir); errors.Is(err, fs.ErrNotExist) {
 		return nil
 	}
-	keepSet := make(map[string]bool)
-	for _, keep := range library.Keep {
-		normalized := strings.TrimSuffix(filepath.ToSlash(keep), "/")
-		keepSet[normalized] = true
-	}
+	keepSet := newKeepSet(library.Keep)
 	return filepath.WalkDir(stagingDir, func(path string, d os.DirEntry, err error) error {
 		if err != nil {
 			return err

internal/librarian/java/postprocess_new.go

@@ -30,18 +30,15 @@ import (
 // It applies post-processing operations configured in librarian.yaml, and renders the README.md directly
 // on the generated files in their final destinations.
 func postProcessLibraryNew(ctx context.Context, p libraryPostProcessParams) error {
+	keepSet := newKeepSet(p.library.Keep)
+
 	// 1. Load postprocess configuration and apply operations
 	if p.library.Postprocess != nil {
 		if err := postprocessing.Validate(p.library.Postprocess); err != nil {
 			return fmt.Errorf("invalid postprocess config: %w", err)
 		}
 		cfg := p.library.Postprocess
 
-		keepSet := make(map[string]bool)
-		for _, k := range p.library.Keep {
-			keepSet[strings.TrimSuffix(filepath.ToSlash(k), "/")] = true
-		}
-
 		// 1. Apply Copies
 		for _, c := range cfg.CopyFile {
 			if shouldPreserve(filepath.ToSlash(c.Dst), keepSet) {
@@ -138,7 +135,7 @@ func postProcessLibraryNew(ctx context.Context, p libraryPostProcessParams) erro
 		return fmt.Errorf("failed to find BOM version: %w", err)
 	}
 
-	if err := RenderREADME(p.outDir, p.metadata, bomVersion, libraryVersion); err != nil {
+	if err := RenderREADME(p.outDir, p.metadata, bomVersion, libraryVersion, keepSet); err != nil {
 		return fmt.Errorf("failed to render README: %w", err)
 	}
 
@@ -161,8 +158,11 @@ func applyToFiles(outDir string, pathPattern string, keepSet map[string]bool, ac
 	var replacedAny bool
 	var lastTextNotFoundErr error
 	for _, file := range files {
-		relPath, _ := filepath.Rel(outDir, file)
-		if shouldPreserve(filepath.ToSlash(relPath), keepSet) {
+		relPath, err := filepath.Rel(outDir, file)
+		if err != nil {
+			return fmt.Errorf("failed to get relative path for %s: %w", file, err)
+		}
+		if isKept(filepath.ToSlash(relPath), keepSet) {
 			continue
 		}
 		if err := action(file); err != nil {

internal/librarian/java/postprocess_new_test.go

@@ -25,7 +25,6 @@ import (
 
 func TestPostProcessLibraryNew(t *testing.T) {
 	tmpDir := t.TempDir()
-	t.Chdir(tmpDir)
 
 	// Setup structure directly in outDir
 	destDir := filepath.Join(tmpDir, "my-module", "src", "main", "java")

internal/librarian/java/postprocess_test.go

@@ -1172,7 +1172,6 @@ func TestPostProcessLibrary_Branching(t *testing.T) {
 
 	t.Run("UseGoPostprocessor true, no yaml, success", func(t *testing.T) {
 		outDir := t.TempDir()
-		t.Chdir(outDir)
 
 		if err := os.MkdirAll(filepath.Join(outDir, "owl-bot-staging"), 0755); err != nil {
 			t.Fatal(err)
@@ -1221,7 +1220,6 @@ func TestPostProcessLibrary_Branching(t *testing.T) {
 
 	t.Run("UseGoPostprocessor true, with postprocess config in library", func(t *testing.T) {
 		outDir := t.TempDir()
-		t.Chdir(outDir)
 
 		if err := os.MkdirAll(filepath.Join(outDir, "owl-bot-staging"), 0755); err != nil {
 			t.Fatal(err)

internal/librarian/java/readme.go

@@ -146,7 +146,7 @@ func ExtractSnippets(dir string) (map[string]string, error) {
 
 	err := filepath.WalkDir(samplesDir, func(path string, d os.DirEntry, err error) error {
 		if err != nil {
-			if errors.Is(err, fs.ErrNotExist) || os.IsNotExist(err) {
+			if errors.Is(err, fs.ErrNotExist) {
 				return nil
 			}
 			return err
@@ -181,44 +181,9 @@ func ExtractSnippets(dir string) (map[string]string, error) {
 	snippetLines := make(map[string][]string)
 
 	for _, file := range files {
-		f, err := os.Open(file)
-		if err != nil {
-			continue
-		}
-
-		openSnippets := make(map[string]bool)
-		excluding := false
-		scanner := bufio.NewScanner(f)
-		scanner.Buffer(make([]byte, 64*1024), 10*1024*1024)
-
-		for scanner.Scan() {
-			line := scanner.Text()
-			openMatch := openSnippetRegex.FindStringSubmatch(line)
-			closeMatch := closeSnippetRegex.FindStringSubmatch(line)
-
-			if len(openMatch) > 1 && !excluding {
-				name := openMatch[1]
-				openSnippets[name] = true
-				if _, exists := snippetLines[name]; !exists {
-					snippetLines[name] = []string{}
-				}
-			} else if len(closeMatch) > 1 && !excluding {
-				delete(openSnippets, closeMatch[1])
-			} else if openExcludeRegex.MatchString(line) {
-				excluding = true
-			} else if closeExcludeRegex.MatchString(line) {
-				excluding = false
-			} else if !excluding {
-				for s := range openSnippets {
-					snippetLines[s] = append(snippetLines[s], line)
-				}
-			}
-		}
-		if err := scanner.Err(); err != nil {
-			f.Close()
+		if err := extractSnippetsFromFile(file, snippetLines); err != nil {
 			return nil, err
 		}
-		f.Close()
 	}
 
 	if len(snippetLines) == 0 {
@@ -267,3 +232,45 @@ func trimLeadingWhitespace(lines []string) string {
 	}
 	return sb.String()
 }
+
+// extractSnippetsFromFile parses a single file to extract tagged code snippets.
+func extractSnippetsFromFile(file string, snippetLines map[string][]string) error {
+	f, err := os.Open(file)
+	if err != nil {
+		return fmt.Errorf("failed to open file %s: %w", file, err)
+	}
+	defer f.Close()
+
+	openSnippets := make(map[string]bool)
+	excluding := false
+	scanner := bufio.NewScanner(f)
+	scanner.Buffer(make([]byte, 64*1024), 10*1024*1024)
+
+	for scanner.Scan() {
+		line := scanner.Text()
+		openMatch := openSnippetRegex.FindStringSubmatch(line)
+		closeMatch := closeSnippetRegex.FindStringSubmatch(line)
+
+		if len(openMatch) > 1 && !excluding {
+			name := openMatch[1]
+			openSnippets[name] = true
+			if _, exists := snippetLines[name]; !exists {
+				snippetLines[name] = []string{}
+			}
+		} else if len(closeMatch) > 1 && !excluding {
+			delete(openSnippets, closeMatch[1])
+		} else if openExcludeRegex.MatchString(line) {
+			excluding = true
+		} else if closeExcludeRegex.MatchString(line) {
+			excluding = false
+		} else if !excluding {
+			for s := range openSnippets {
+				snippetLines[s] = append(snippetLines[s], line)
+			}
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		return fmt.Errorf("failed scanning file %s: %w", file, err)
+	}
+	return nil
+}

internal/librarian/java/template_render.go

@@ -18,6 +18,7 @@ import (
 	"embed"
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"strings"
@@ -28,16 +29,20 @@ import (
 )
 
 //go:embed template/README.md.go.tmpl
-var defaultTemplateFS embed.FS
+var defaultTemplateFs embed.FS
 
 // RenderREADME renders the README.md file using the template and metadata.
 // dir is the directory containing where README.md will be written.
-func RenderREADME(dir string, metadata *repoMetadata, bomVersion, libraryVersion string) error {
+func RenderREADME(dir string, metadata *repoMetadata, bomVersion, libraryVersion string, keepSet map[string]bool) error {
+	outputPath := filepath.Join(dir, "README.md")
+	if isKept("README.md", keepSet) {
+		return nil
+	}
+
 	partialsPath := filepath.Join(dir, ".readme-partials.yaml")
-	if _, err := os.Stat(partialsPath); os.IsNotExist(err) {
+	if _, err := os.Stat(partialsPath); errors.Is(err, fs.ErrNotExist) {
 		partialsPath = filepath.Join(dir, ".readme-partials.yml")
 	}
-	outputPath := filepath.Join(dir, "README.md")
 
 	// Read partials if exist
 	var partials map[string]interface{}
@@ -62,13 +67,13 @@ func RenderREADME(dir string, metadata *repoMetadata, bomVersion, libraryVersion
 	// Prepare data for template
 	distName := metadata.DistributionName
 	distParts := strings.Split(distName, ":")
-	groupID := ""
-	artifactID := ""
+	groupId := ""
+	artifactId := ""
 	if len(distParts) > 0 {
-		groupID = distParts[0]
+		groupId = distParts[0]
 	}
 	if len(distParts) > 1 {
-		artifactID = distParts[1]
+		artifactId = distParts[1]
 	}
 
 	repoName := metadata.Repo
@@ -84,7 +89,6 @@ func RenderREADME(dir string, metadata *repoMetadata, bomVersion, libraryVersion
 	if minJavaVersion == 0 {
 		minJavaVersion = 8 // Default to Java 8
 	}
-	fmt.Println("DEBUG minJavaVersion:", minJavaVersion)
 
 	samples, err := ExtractSamples(dir)
 	if err != nil {
@@ -121,8 +125,8 @@ func RenderREADME(dir string, metadata *repoMetadata, bomVersion, libraryVersion
 		LibraryVersion    string
 	}{
 		Metadata:          templateMetadata,
-		GroupID:           groupID,
-		ArtifactID:        artifactID,
+		GroupID:           groupId,
+		ArtifactID:        artifactId,
 		Version:           version,
 		RepoShort:         repoShort,
 		MigratedSplitRepo: false,
@@ -135,9 +139,9 @@ func RenderREADME(dir string, metadata *repoMetadata, bomVersion, libraryVersion
 	templatePath := filepath.Join(dir, "template", "README.md.go.tmpl")
 	tmplBytes, err := os.ReadFile(templatePath)
 	if err != nil {
-		if errors.Is(err, os.ErrNotExist) {
+		if errors.Is(err, fs.ErrNotExist) {
 			// Fallback to embedded default template
-			tmplBytes, err = defaultTemplateFS.ReadFile("template/README.md.go.tmpl")
+			tmplBytes, err = defaultTemplateFs.ReadFile("template/README.md.go.tmpl")
 		}
 		if err != nil {
 			return fmt.Errorf("failed to read template: %w", err)