From 7f1633be06164a9759a18d44ac0898e10c2141ea Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 22:03:22 -0400 Subject: [PATCH] fix(generate): refuse an unstampable package manifest before generating Review r97 on #1164: the version stamp ran after each language was applied, so a package.json without a top-level "version" (or a pyproject.toml without a version line) was refused with the language already rewritten and library.codefly.yaml not. Every requested language's existing manifest is now checked before anything is staged, with the same locator the stamp uses, so the refusal leaves the library exactly as it was. A manifest not written yet comes from the scaffold, which declares a version. Co-Authored-By: Claude Opus 5.5 --- pkg/generators/client.go | 6 ++ pkg/generators/client_library.go | 125 ++++++++++++++++---------- pkg/generators/client_version_test.go | 63 +++++++++++++ 3 files changed, 149 insertions(+), 45 deletions(-) diff --git a/pkg/generators/client.go b/pkg/generators/client.go index 5a1998de..840f3f44 100644 --- a/pkg/generators/client.go +++ b/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 diff --git a/pkg/generators/client_library.go b/pkg/generators/client_library.go index ac2557e9..443c87fa 100644 --- a/pkg/generators/client_library.go +++ b/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(¤t); err != nil { - return fmt.Errorf("%s: \"version\" is not a string: %w", path, err) + if valueErr := decoder.Decode(¤t); 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 { diff --git a/pkg/generators/client_version_test.go b/pkg/generators/client_version_test.go index e358989d..9713d98c 100644 --- a/pkg/generators/client_version_test.go +++ b/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) + } +}