diff --git a/internal/core/helpers.go b/internal/core/helpers.go index e5b71dc..aef1ed2 100644 --- a/internal/core/helpers.go +++ b/internal/core/helpers.go @@ -15,6 +15,20 @@ func NextLocation(seen map[string]int, base string) string { return base } +// LeadingSpaces returns the number of space characters at the start of line. +func LeadingSpaces(line string) int { + n := 0 + for n < len(line) && line[n] == ' ' { + n++ + } + return n +} + +// IsYAMLComment reports whether line holds only a YAML comment. +func IsYAMLComment(line string) bool { + return strings.HasPrefix(strings.TrimLeft(line, " "), "#") +} + // ForEachLine iterates over lines in content without allocating a slice. func ForEachLine(content string, fn func(line string) bool) { for len(content) > 0 { diff --git a/internal/crystal/crystal.go b/internal/crystal/crystal.go index af79445..b8b84ee 100644 --- a/internal/crystal/crystal.go +++ b/internal/crystal/crystal.go @@ -12,23 +12,23 @@ func init() { core.Register("crystal", core.Lockfile, &shardLockParser{}, core.ExactMatch("shard.lock")) } -// extractShardName extracts shard name from " name:" line -func extractShardName(line string) (string, bool) { - if len(line) < 4 || line[0] != ' ' || line[1] != ' ' || line[2] == ' ' { +// extractShardName extracts shard name from " name:" lines indented by exactly indent spaces. +func extractShardName(line string, indent int) (string, bool) { + if indent == 0 || len(line) < indent+2 || core.LeadingSpaces(line) != indent { return "", false } if line[len(line)-1] != ':' { return "", false } - return line[2 : len(line)-1], true + return line[indent : len(line)-1], true } -// extractShardValue extracts value from " key: value" lines -func extractShardValue(line, prefix string) (string, bool) { - if !strings.HasPrefix(line, prefix) { +// extractShardValue extracts value from " key: value" lines indented deeper than indent spaces. +func extractShardValue(line, prefix string, indent int) (string, bool) { + if core.LeadingSpaces(line) <= indent { return "", false } - return line[len(prefix):], true + return strings.CutPrefix(strings.TrimLeft(line, " "), prefix) } // shardYMLParser parses shard.yml files. @@ -103,6 +103,7 @@ func (p *shardLockParser) Parse(filename string, content []byte) (*core.Result, deps := make([]core.Dependency, 0, core.EstimateDeps(len(content))) inShards := false + indent := 0 var currentName string var currentVersion string @@ -110,15 +111,22 @@ func (p *shardLockParser) Parse(filename string, content []byte) (*core.Result, // Detect shards: section if line == "shards:" { inShards = true + indent = 0 return true } - if !inShards { + // Comment-only lines carry no data and must not set the indent + if !inShards || core.IsYAMLComment(line) { return true } - // Shard name (2-space indent) - if name, ok := extractShardName(line); ok { + // The first indented line sets the indent width of shard names + if indent == 0 && strings.TrimSpace(line) != "" { + indent = core.LeadingSpaces(line) + } + + // Shard name + if name, ok := extractShardName(line, indent); ok { // Save previous shard if any if currentName != "" { deps = append(deps, core.Dependency{ @@ -133,11 +141,11 @@ func (p *shardLockParser) Parse(filename string, content []byte) (*core.Result, return true } - // Version or commit (4-space indent) + // Version or commit, nested under the shard name if currentName != "" { - if v, ok := extractShardValue(line, " version: "); ok { + if v, ok := extractShardValue(line, "version: ", indent); ok { currentVersion = v - } else if v, ok := extractShardValue(line, " commit: "); ok { + } else if v, ok := extractShardValue(line, "commit: ", indent); ok { if currentVersion == "" { // version takes precedence currentVersion = v } diff --git a/internal/crystal/crystal_test.go b/internal/crystal/crystal_test.go index 738b1d9..178ffe7 100644 --- a/internal/crystal/crystal_test.go +++ b/internal/crystal/crystal_test.go @@ -2,6 +2,8 @@ package crystal import ( "os" + "reflect" + "strings" "testing" "github.com/git-pkgs/manifests/internal/core" @@ -99,3 +101,34 @@ func TestShardLock(t *testing.T) { } } } + +func TestShardLockIndentWidth(t *testing.T) { + // https://github.com/git-pkgs/manifests/issues/108 + content, err := os.ReadFile("../../testdata/crystal/shard.lock") + if err != nil { + t.Fatalf("failed to read fixture: %v", err) + } + + parser := &shardLockParser{} + want, err := parser.Parse("shard.lock", content) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + + lines := strings.Split(string(content), "\n") + for i, line := range lines { + n := core.LeadingSpaces(line) + lines[i] = strings.Repeat(" ", n*2) + line[n:] + } + + got, err := parser.Parse("shard.lock", []byte(strings.Join(lines, "\n"))) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + if len(got.Dependencies) != 7 { + t.Fatalf("expected 7 dependencies, got %d", len(got.Dependencies)) + } + if !reflect.DeepEqual(got.Dependencies, want.Dependencies) { + t.Errorf("reindented dependencies differ:\ngot %+v\nwant %+v", got.Dependencies, want.Dependencies) + } +} diff --git a/internal/npm/npm_test.go b/internal/npm/npm_test.go index ad8250a..4838446 100644 --- a/internal/npm/npm_test.go +++ b/internal/npm/npm_test.go @@ -3,6 +3,7 @@ package npm import ( "os" "reflect" + "strings" "testing" "github.com/git-pkgs/manifests/internal/core" @@ -995,23 +996,30 @@ func TestDenoLock(t *testing.T) { func TestExtractPnpmPackageKey(t *testing.T) { tests := []struct { line string + indent int wantKey string wantOk bool }{ - {" '@typescript-eslint/eslint-plugin@8.59.3':", "@typescript-eslint/eslint-plugin@8.59.3", true}, - {" acorn@5.7.4:", "acorn@5.7.4", true}, - {" /chalk/1.1.3:", "/chalk/1.1.3", true}, - // Nested keys (>2-space indent) must be rejected. + {" '@typescript-eslint/eslint-plugin@8.59.3':", 2, "@typescript-eslint/eslint-plugin@8.59.3", true}, + {" acorn@5.7.4:", 2, "acorn@5.7.4", true}, + {" /chalk/1.1.3:", 2, "/chalk/1.1.3", true}, + // Nested keys (deeper than the package indent) must be rejected. // https://github.com/git-pkgs/manifests/issues/32 - {" '@typescript-eslint/parser':", "", false}, - {" peerDependenciesMeta:", "", false}, - {"packages:", "", false}, - {" resolution: {integrity: sha512-xxx}", "", false}, + {" '@typescript-eslint/parser':", 2, "", false}, + {" peerDependenciesMeta:", 2, "", false}, + {"packages:", 2, "", false}, + {" resolution: {integrity: sha512-xxx}", 2, "", false}, + // Package keys follow the indent width of the file. + // https://github.com/git-pkgs/manifests/issues/108 + {" acorn@5.7.4:", 4, "acorn@5.7.4", true}, + {" acorn@5.7.4:", 4, "", false}, + {" peerDependenciesMeta:", 4, "", false}, + {" acorn@5.7.4:", 0, "", false}, } for _, tt := range tests { t.Run(tt.line, func(t *testing.T) { - gotKey, gotOk := extractPnpmPackageKey(tt.line) + gotKey, gotOk := extractPnpmPackageKey(tt.line, tt.indent) if gotOk != tt.wantOk { t.Errorf("ok = %v, want %v", gotOk, tt.wantOk) } @@ -1061,6 +1069,50 @@ packages: } } +// reindent multiplies the leading spaces of every line by factor. +func reindent(content []byte, factor int) []byte { + lines := strings.Split(string(content), "\n") + for i, line := range lines { + n := core.LeadingSpaces(line) + lines[i] = strings.Repeat(" ", n*factor) + line[n:] + } + return []byte(strings.Join(lines, "\n")) +} + +func TestPnpmLockIndentWidth(t *testing.T) { + // https://github.com/git-pkgs/manifests/issues/108 + for _, fixture := range []string{ + "pnpm-lock.yaml", + "pnpm-lockfile-version-5/pnpm-lock.yaml", + "pnpm-lockfile-version-6/pnpm-lock.yaml", + "pnpm-lockfile-version-9/pnpm-lock.yaml", + } { + t.Run(fixture, func(t *testing.T) { + content, err := os.ReadFile("../../testdata/npm/" + fixture) + if err != nil { + t.Fatalf("failed to read fixture: %v", err) + } + + parser := &pnpmLockParser{} + want, err := parser.Parse("pnpm-lock.yaml", content) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + if len(want.Dependencies) == 0 { + t.Fatal("expected dependencies in original fixture") + } + + got, err := parser.Parse("pnpm-lock.yaml", reindent(content, 2)) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + if !reflect.DeepEqual(got.Dependencies, want.Dependencies) { + t.Errorf("reindented dependencies differ:\ngot %+v\nwant %+v", got.Dependencies, want.Dependencies) + } + }) + } +} + func TestParsePnpmPackageKey(t *testing.T) { tests := []struct { key string diff --git a/internal/npm/pnpm.go b/internal/npm/pnpm.go index ddd1d47..25ed092 100644 --- a/internal/npm/pnpm.go +++ b/internal/npm/pnpm.go @@ -10,16 +10,17 @@ func init() { } // extractPnpmPackageKey extracts package key from " /name/ver:" or " '@scope/name@ver':" lines -func extractPnpmPackageKey(line string) (string, bool) { - // Must start with exactly 2 spaces so nested keys (peerDependenciesMeta etc) are skipped - if len(line) < 4 || line[0] != ' ' || line[1] != ' ' || line[2] == ' ' { +// indented by exactly indent spaces. +func extractPnpmPackageKey(line string, indent int) (string, bool) { + // Must start with exactly indent spaces so nested keys (peerDependenciesMeta etc) are skipped + if indent == 0 || len(line) < indent+2 || core.LeadingSpaces(line) != indent { return "", false } // Must end with colon if line[len(line)-1] != ':' { return "", false } - key := line[2 : len(line)-1] + key := line[indent : len(line)-1] // Remove surrounding quotes if present if len(key) >= 2 && (key[0] == '\'' || key[0] == '"') { key = key[1 : len(key)-1] @@ -122,11 +123,18 @@ func (p *pnpmLockParser) Parse(filename string, content []byte) (*core.Result, e deps := make([]core.Dependency, 0, core.EstimateDeps(len(content))) inPackages := false + indent := 0 var state pnpmPackageState core.ForEachLine(text, func(line string) bool { if line == "packages:" { inPackages = true + indent = 0 + return true + } + + // Comment-only lines carry no data and must not set the indent or end the section + if inPackages && core.IsYAMLComment(line) { return true } @@ -142,8 +150,13 @@ func (p *pnpmLockParser) Parse(filename string, content []byte) (*core.Result, e return true } - // Package key line (2-space indent) - if key, ok := extractPnpmPackageKey(line); ok { + // The first indented line sets the indent width of package keys + if indent == 0 && strings.TrimSpace(line) != "" { + indent = core.LeadingSpaces(line) + } + + // Package key line + if key, ok := extractPnpmPackageKey(line, indent); ok { deps = buildDependency(deps, state) state = pnpmPackageState{key: key} return true diff --git a/lockfile_indent_test.go b/lockfile_indent_test.go new file mode 100644 index 0000000..43b5ac0 --- /dev/null +++ b/lockfile_indent_test.go @@ -0,0 +1,82 @@ +package manifests + +import ( + "os" + "reflect" + "strings" + "testing" +) + +// insertAfterLine inserts extra on a new line after the first line equal to header. +func insertAfterLine(t *testing.T, content, header, extra string) string { + t.Helper() + marker := "\n" + header + "\n" + idx := strings.Index(content, marker) + if idx < 0 { + t.Fatalf("header %q not found", header) + } + idx += len(marker) + return content[:idx] + extra + "\n" + content[idx:] +} + +// doubleIndent doubles the leading spaces of every line. +func doubleIndent(content string) string { + lines := strings.Split(content, "\n") + for i, line := range lines { + trimmed := strings.TrimLeft(line, " ") + lines[i] = strings.Repeat(" ", 2*(len(line)-len(trimmed))) + trimmed + } + return strings.Join(lines, "\n") +} + +func TestLockfileIndentIgnoresComments(t *testing.T) { + // https://github.com/git-pkgs/manifests/issues/108 + fixtures := []struct { + path string + filename string + header string + count int + }{ + {"testdata/npm/pnpm-lock.yaml", "pnpm-lock.yaml", "packages:", 9}, + {"testdata/crystal/shard.lock", "shard.lock", "shards:", 7}, + } + + for _, f := range fixtures { + raw, err := os.ReadFile(f.path) + if err != nil { + t.Fatalf("failed to read fixture: %v", err) + } + original := string(raw) + + want, err := Parse(f.filename, raw) + if err != nil { + t.Fatalf("%s: Parse failed: %v", f.filename, err) + } + if len(want.Dependencies) != f.count { + t.Fatalf("%s: expected %d dependencies, got %d", f.filename, f.count, len(want.Dependencies)) + } + + cases := []struct { + name string + content string + }{ + {"deeper comment before first entry", insertAfterLine(t, original, f.header, " # Dependencies")}, + {"comment at entry indent ending in colon", insertAfterLine(t, original, f.header, " # pinned:")}, + {"top-level comment inside section", insertAfterLine(t, original, f.header, "# Dependencies")}, + {"shallower comment in doubled indent", insertAfterLine(t, doubleIndent(original), f.header, " # Dependencies")}, + } + + for _, tc := range cases { + t.Run(f.filename+"/"+tc.name, func(t *testing.T) { + got, err := Parse(f.filename, []byte(tc.content)) + if err != nil { + t.Fatalf("Parse failed: %v", err) + } + if !reflect.DeepEqual(got.Dependencies, want.Dependencies) { + t.Errorf("got %d dependencies, want %d:\ngot %+v\nwant %+v", + len(got.Dependencies), len(want.Dependencies), got.Dependencies, want.Dependencies) + } + }) + } + } +}