fix(generate): refuse an unstampable package manifest before generating

pkg/generators/client.go

@@ -166,6 +166,12 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error
 		// Refused before anything is generated, so the library is untouched.
 		return fmt.Errorf("library %s declares no version: its producer's version is empty", req.Name)
 	}
+	for _, lang := range req.Languages {
+		if err := checkPackageVersionStampable(lang, filepath.Join(req.Output, string(lang))); err != nil {
+			// Refused before anything is generated, so the library is untouched.
+			return fmt.Errorf("cannot set the %s client's version: %w", lang, err)
+		}
+	}
 
 	// Every language is generated before anything under req.Output is
 	// touched. The companion runs are what fail — a contract that does not

pkg/generators/client_library.go

@@ -793,7 +793,7 @@ func writeTypeScriptScaffold(langDir, npmPackage string) error {
 
 // pyprojectVersionRe matches pyproject.toml's project version line, the shape
 // writePythonScaffold writes.
-var pyprojectVersionRe = regexp.MustCompile(`(?m)^version = "[^"]*"$`)
+var pyprojectVersionRe = regexp.MustCompile(`(?m)^version = ("[^"]*")$`)
 
 // stampPackageVersion sets the version a language's package manifest declares
 // to the library's own. The scaffold is written once and kept, so without this
@@ -805,74 +805,109 @@ func stampPackageVersion(lang languages.Language, langDir, version string) error
 	if strings.TrimSpace(version) == "" {
 		return fmt.Errorf("the library declares no version")
 	}
-	switch lang {
-	case languages.TYPESCRIPT:
-		return stampPackageJSONVersion(filepath.Join(langDir, "package.json"), version)
-	case languages.PYTHON:
-		return replaceVersion(filepath.Join(langDir, "pyproject.toml"), pyprojectVersionRe, "version = "+strconv.Quote(version))
-	default:
+	path, ok := packageManifest(lang, langDir)
+	if !ok {
 		return nil
 	}
-}
-
-// stampPackageJSONVersion replaces the value of package.json's top-level
-// "version" member in place, whatever the file's formatting: the member is
-// found by walking the document's tokens, so a nested "version" key or a
-// reformatted file never takes the edit.
-func stampPackageJSONVersion(path, version string) error {
 	data, err := os.ReadFile(path) //nolint:gosec // path is a file this command just wrote
 	if err != nil {
 		return err
 	}
-	quoted, err := json.Marshal(version)
+	start, end, err := locatePackageVersion(lang, path, data)
 	if err != nil {
 		return err
 	}
+	value := strconv.Quote(version)
+	if lang == languages.TYPESCRIPT {
+		quoted, err := json.Marshal(version)
+		if err != nil {
+			return err
+		}
+		value = string(quoted)
+	}
+	updated := append([]byte{}, data[:start]...)
+	updated = append(updated, value...)
+	updated = append(updated, data[end:]...)
+	return os.WriteFile(path, updated, 0o644) //nolint:gosec
+}
+
+// checkPackageVersionStampable refuses, before anything is generated, a package
+// manifest already in langDir whose version stampPackageVersion could not set,
+// so that refusal leaves the library exactly as it was. A manifest that does
+// not exist yet is written from the scaffold, which declares one.
+func checkPackageVersionStampable(lang languages.Language, langDir string) error {
+	path, ok := packageManifest(lang, langDir)
+	if !ok {
+		return nil
+	}
+	data, err := os.ReadFile(path) //nolint:gosec // path is the library's own package manifest
+	if os.IsNotExist(err) {
+		return nil
+	}
+	if err != nil {
+		return err
+	}
+	_, _, err = locatePackageVersion(lang, path, data)
+	return err
+}
+
+// packageManifest is the file in langDir that declares a language's package
+// version, and false for a language whose version is not in a file.
+func packageManifest(lang languages.Language, langDir string) (string, bool) {
+	switch lang {
+	case languages.TYPESCRIPT:
+		return filepath.Join(langDir, "package.json"), true
+	case languages.PYTHON:
+		return filepath.Join(langDir, "pyproject.toml"), true
+	default:
+		return "", false
+	}
+}
+
+// locatePackageVersion returns where in data the manifest's version value
+// lies, quotes included.
+func locatePackageVersion(lang languages.Language, path string, data []byte) (start, end int, err error) {
+	if lang == languages.TYPESCRIPT {
+		return locatePackageJSONVersion(path, data)
+	}
+	match := pyprojectVersionRe.FindSubmatchIndex(data)
+	if match == nil {
+		return 0, 0, fmt.Errorf("cannot find a version in %s", path)
+	}
+	return match[2], match[3], nil
+}
+
+// locatePackageJSONVersion finds the value of package.json's top-level
+// "version" member, whatever the file's formatting: the member is found by
+// walking the document's tokens, so a nested "version" key or a reformatted
+// file never takes the edit.
+func locatePackageJSONVersion(path string, data []byte) (start, end int, err error) {
 	decoder := json.NewDecoder(bytes.NewReader(data))
-	if token, err := decoder.Token(); err != nil || token != json.Delim('{') {
-		return fmt.Errorf("%s is not a JSON object", path)
+	if token, tokenErr := decoder.Token(); tokenErr != nil || token != json.Delim('{') {
+		return 0, 0, fmt.Errorf("%s is not a JSON object", path)
 	}
 	for decoder.More() {
-		key, err := decoder.Token()
-		if err != nil {
-			return fmt.Errorf("cannot parse %s: %w", path, err)
+		key, keyErr := decoder.Token()
+		if keyErr != nil {
+			return 0, 0, fmt.Errorf("cannot parse %s: %w", path, keyErr)
 		}
 		afterKey := decoder.InputOffset()
 		if key != "version" {
 			var skip json.RawMessage
-			if err := decoder.Decode(&skip); err != nil {
-				return fmt.Errorf("cannot parse %s: %w", path, err)
+			if skipErr := decoder.Decode(&skip); skipErr != nil {
+				return 0, 0, fmt.Errorf("cannot parse %s: %w", path, skipErr)
 			}
 			continue
 		}
 		var current string
-		if err := decoder.Decode(&current); err != nil {
-			return fmt.Errorf("%s: \"version\" is not a string: %w", path, err)
+		if valueErr := decoder.Decode(&current); valueErr != nil {
+			return 0, 0, fmt.Errorf("%s: \"version\" is not a string: %w", path, valueErr)
 		}
 		afterValue := decoder.InputOffset()
 		valueStart := afterKey + int64(bytes.IndexByte(data[afterKey:afterValue], '"'))
-		updated := append([]byte{}, data[:valueStart]...)
-		updated = append(updated, quoted...)
-		updated = append(updated, data[afterValue:]...)
-		return os.WriteFile(path, updated, 0o644) //nolint:gosec
+		return int(valueStart), int(afterValue), nil
 	}
-	return fmt.Errorf("cannot find a version in %s", path)
-}
-
-// replaceVersion replaces the first match of re in path with replacement, as is.
-func replaceVersion(path string, re *regexp.Regexp, replacement string) error {
-	data, err := os.ReadFile(path) //nolint:gosec // path is a file this command just wrote
-	if err != nil {
-		return err
-	}
-	match := re.FindIndex(data)
-	if match == nil {
-		return fmt.Errorf("cannot find a version in %s", path)
-	}
-	updated := append([]byte{}, data[:match[0]]...)
-	updated = append(updated, replacement...)
-	updated = append(updated, data[match[1]:]...)
-	return os.WriteFile(path, updated, 0o644) //nolint:gosec
+	return 0, 0, fmt.Errorf("cannot find a version in %s", path)
 }
 
 func writePythonScaffold(langDir, pyPackage string) error {

pkg/generators/client_version_test.go

@@ -1,12 +1,14 @@
 package generators
 
 import (
+	"context"
 	"encoding/json"
 	"os"
 	"path/filepath"
 	"strings"
 	"testing"
 
+	"github.com/codefly-dev/core/composition"
 	"github.com/codefly-dev/core/languages"
 )
 
@@ -119,3 +121,64 @@ func packageJSON(t *testing.T, langDir string) map[string]any {
 	}
 	return doc
 }
+
+// A package manifest whose version cannot be set is refused before anything
+// is generated, so the library is left exactly as it was: never generated
+// and applied, then refused with its manifest half-written.
+func TestAnUnstampableManifestLeavesTheLibraryUntouched(t *testing.T) {
+	output := t.TempDir()
+	langDir := filepath.Join(output, "typescript")
+	if err := os.MkdirAll(filepath.Join(langDir, "src"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(langDir, "package.json"), []byte(`{"name":"@acme/billing-client","private":true}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(langDir, "src", "notes.ts"), []byte("// hand-written\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	before := snapshotTree(t, output)
+
+	err := GenerateClientLibrary(context.Background(), &ClientLibraryRequest{
+		Name:       "billing-client",
+		Output:     output,
+		Languages:  []languages.Language{languages.TYPESCRIPT},
+		NpmPackage: "@acme/billing-client",
+		Entries: []ContractEntry{{
+			Endpoint: composition.APIContractEndpoint{
+				Service: "api", Endpoint: "grpc", Kind: composition.APIContractKindProtobuf, Package: "billing.v1",
+			},
+			PackageVersion: "0.1.14",
+			ModuleName:     "billing",
+		}},
+	})
+	if err == nil || !strings.Contains(err.Error(), "cannot find a version") {
+		t.Fatalf("err = %v, want a refusal naming the missing version", err)
+	}
+	if after := snapshotTree(t, output); after != before {
+		t.Fatalf("a refused generation changed the library:\nbefore:\n%s\nafter:\n%s", before, after)
+	}
+}
+
+func TestCheckPackageVersionStampable(t *testing.T) {
+	langDir := t.TempDir()
+	// Not written yet: the scaffold declares a version.
+	if err := checkPackageVersionStampable(languages.PYTHON, langDir); err != nil {
+		t.Fatalf("a missing pyproject.toml: %v", err)
+	}
+	if err := writePythonScaffold(langDir, "billing_client"); err != nil {
+		t.Fatal(err)
+	}
+	if err := checkPackageVersionStampable(languages.PYTHON, langDir); err != nil {
+		t.Fatalf("the scaffold's pyproject.toml: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(langDir, "pyproject.toml"), []byte("[project]\nname = \"billing\"\ndynamic = [\"version\"]\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := checkPackageVersionStampable(languages.PYTHON, langDir); err == nil {
+		t.Fatal("a pyproject.toml with no version line was accepted")
+	}
+	if err := checkPackageVersionStampable(languages.GO, langDir); err != nil {
+		t.Fatalf("go: %v", err)
+	}
+}