From 16662bc05874bd9a641affa6ef5b3ec6048d93cf Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 11 Aug 2026 12:11:32 +0100 Subject: [PATCH 1/2] Add heuristic resolver, Chain, and case-insensitive matching heuristic.Resolver derives conventional source names from a package's PURL type and name for npm, pypi, golang, gem, cargo, composer, hex, and maven, tagged with EvidenceHeuristic. It reads no files and makes no network requests. Chain runs several SurfaceResolvers over the same package and merges their results, so a curated catalog or artifact resolver can be tried first and the naming convention fills whatever it does not cover. ProvidedName.CaseInsensitive folds ASCII case during matching for languages whose module or namespace lookup is itself case-insensitive. The composer heuristic sets it since PHP resolves 'use GuzzleHttp\X' and 'use guzzlehttp\x' identically and no naming rule can recover the canonical spelling from the vendor name alone. SurfaceResolverFunc adapts a plain function to a SurfaceResolver. --- README.md | 16 +++- heuristic/heuristic.go | 143 ++++++++++++++++++++++++++++++++++++ heuristic/heuristic_test.go | 140 +++++++++++++++++++++++++++++++++++ import.go | 26 ++++--- match.go | 12 ++- match_test.go | 21 ++++++ merge.go | 33 +++++---- resolver.go | 38 ++++++++++ resolver_test.go | 94 ++++++++++++++++++++++++ types.go | 18 +++-- 10 files changed, 505 insertions(+), 36 deletions(-) create mode 100644 heuristic/heuristic.go create mode 100644 heuristic/heuristic_test.go create mode 100644 resolver_test.go diff --git a/README.md b/README.md index 5bd5371..2a684ce 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ go get github.com/git-pkgs/provides `Surface` maps a versioned PURL to its provided source names. `Binding` connects a PURL to the imported and local names used by one project. An aliased binding may also retain the package-side target name. Both retain the evidence used to produce the mapping. -`ProvidedName.Name` is the exact source-visible spelling. Package-manager normalisation does not apply to it, and matching is case-sensitive. `flask` therefore does not match `Flask`. +`ProvidedName.Name` is the exact source-visible spelling. Package-manager normalisation does not apply to it, and matching is case-sensitive by default. `flask` therefore does not match `Flask`. Set `CaseInsensitive` for languages whose module or namespace lookup folds case, for example PHP, where `use GuzzleHttp\Client` and `use guzzlehttp\client` resolve identically. ## Matching names @@ -116,6 +116,20 @@ result, err := provides.ResolveProjectSurfaces( This path reads no files, runs no package-manager commands, and makes no network requests. PyPI distribution names are normalised for catalog lookup, while each returned `Surface.PURL` retains the caller's spelling and version. Unknown packages are omitted without producing a diagnostic. +## Heuristic surfaces + +The `heuristic` package derives conventional source names from a package's PURL type and name alone: an npm package `ws` provides module `ws` and any `ws/...` subpath, PyPI `Engine-IO-Parser` provides `engine_io_parser`, gem `active_support` provides both feature `active_support` and constant `ActiveSupport`, Cargo `tokio-util` provides crate `tokio_util`. It covers `npm`, `pypi`, `golang`, `gem`, `cargo`, `composer`, `hex`, and `maven`; other PURL types resolve to an empty surface. Every returned name carries `EvidenceHeuristic` so callers can distinguish a naming-convention guess from a verified mapping. + +Packages whose importable name is not a mechanical transform of their registry name (PyYAML → `yaml`, Pillow → `PIL`, most Composer PSR-4 roots) need curated data or an artifact resolver. `Chain` runs several resolvers over the same package and merges their results, so an authoritative source can be tried first and the naming convention fills whatever it does not cover: + +```go +resolver := provides.Chain(curated.Python(), heuristic.Resolver()) + +project, err := provides.ResolveProjectSurfaces(ctx, resolver, packages, provides.SurfaceOptions{}) +``` + +For PyYAML this returns both `yaml` (curated) and `pyyaml` (heuristic) with their respective evidence; `MatchImport("python", "yaml", project)` matches on the curated entry while a package the catalog does not list still resolves via the heuristic. + ## Resolving an import `ResolveImport` combines project-surface resolution with a reverse lookup. Every matching dependency is returned when an import is ambiguous: diff --git a/heuristic/heuristic.go b/heuristic/heuristic.go new file mode 100644 index 0000000..64105c4 --- /dev/null +++ b/heuristic/heuristic.go @@ -0,0 +1,143 @@ +// Package heuristic provides a SurfaceResolver that maps a package identity +// to its conventional source-level name using per-ecosystem naming rules +// alone. It reads no files, runs no commands, and makes no network requests. +// +// The mappings cover the common case where a package's importable name is a +// mechanical transform of its registry name: an npm package `ws` provides +// module `ws`, a PyPI distribution `Engine-IO-Parser` provides module +// `engine_io_parser`, a Ruby gem `active_support` provides constant +// `ActiveSupport`. Packages whose importable name is unrelated to their +// registry name (PyYAML → yaml, Pillow → PIL) need curated data or an +// artifact resolver; chain this resolver after one of those so the +// convention fills gaps the authoritative source did not cover. +// +// Every returned ProvidedName carries EvidenceHeuristic so downstream code +// can distinguish a naming-convention guess from a verified mapping. +package heuristic + +import ( + "context" + "strings" + + "github.com/git-pkgs/provides" + "github.com/git-pkgs/purl" +) + +const source = "heuristic" + +// Resolver returns a SurfaceResolver that derives conventional source names +// from a package's PURL type and name. Ecosystems without a registered +// convention resolve to an empty surface with no diagnostic. +func Resolver() provides.SurfaceResolverFunc { + return resolve +} + +func resolve(ctx context.Context, pkg provides.Package, _ provides.SurfaceOptions) (provides.SurfaceResult, error) { + if err := ctx.Err(); err != nil { + return provides.SurfaceResult{}, err + } + p, err := purl.Parse(pkg.PURL) + if err != nil { + return provides.SurfaceResult{ + Diagnostics: []provides.Diagnostic{{Source: source, Message: err.Error()}}, + }, nil + } + fn, ok := conventions[p.Type] + if !ok { + return provides.SurfaceResult{Surface: provides.Surface{PURL: pkg.PURL}}, nil + } + return provides.SurfaceResult{ + Surface: provides.Surface{PURL: pkg.PURL, Provides: fn(p)}, + }, nil +} + +// conventions maps a PURL type to the source names its packages +// conventionally provide. +var conventions = map[string]func(*purl.PURL) []provides.ProvidedName{ + "npm": func(p *purl.PURL) []provides.ProvidedName { + // Bare specifier and any subpath under it: `ws`, `ws/lib/sender`. + name := p.Name + if p.Namespace != "" { + name = p.Namespace + "/" + p.Name + } + return []provides.ProvidedName{prefix("javascript", name, "module", "/", false)} + }, + "pypi": func(p *purl.PURL) []provides.ProvidedName { + // PEP 503 treats -, _, . as equivalent and the registry name is + // case-insensitive; module names are lowercase with underscores. + return []provides.ProvidedName{ + prefix("python", strings.ToLower(underscore(p.Name)), "module", ".", false), + } + }, + "golang": func(p *purl.PURL) []provides.ProvidedName { + // Import paths are the module path or a package under it. + module := p.Name + if p.Namespace != "" { + module = p.Namespace + "/" + p.Name + } + return []provides.ProvidedName{prefix("go", module, "package", "/", false)} + }, + "gem": func(p *purl.PURL) []provides.ProvidedName { + return []provides.ProvidedName{ + // require 'gem' or 'gem/sub' + prefix("ruby", p.Name, "feature", "/", false), + // Bundler-autoloaded top-level constant. + prefix("ruby", camelize(p.Name), "constant", "::", false), + } + }, + "cargo": func(p *purl.PURL) []provides.ProvidedName { + // Crate identifiers replace hyphens with underscores. + return []provides.ProvidedName{prefix("rust", underscore(p.Name), "crate", "::", false)} + }, + "composer": func(p *purl.PURL) []provides.ProvidedName { + // PSR-4 root is conventionally the vendor segment titlecased. PHP + // namespace resolution is case-insensitive so the guess need only + // match after case folding. + vendor := p.Namespace + if vendor == "" { + vendor = p.Name + } + return []provides.ProvidedName{prefix("php", camelize(vendor), "namespace", `\`, true)} + }, + "hex": func(p *purl.PURL) []provides.ProvidedName { + return []provides.ProvidedName{prefix("elixir", camelize(p.Name), "module", ".", false)} + }, + "maven": func(p *purl.PURL) []provides.ProvidedName { + // Java packages conventionally follow the reversed-domain group ID. + return []provides.ProvidedName{prefix("java", p.Namespace, "package", ".", false)} + }, +} + +func prefix(lang, name, kind, sep string, ci bool) provides.ProvidedName { + return provides.ProvidedName{ + Language: lang, + Name: name, + Kind: kind, + Match: provides.MatchPrefix, + Separator: sep, + CaseInsensitive: ci, + Evidence: []provides.Evidence{{Method: provides.EvidenceHeuristic, Source: source}}, + } +} + +// underscore replaces hyphens with underscores. +func underscore(s string) string { return strings.ReplaceAll(s, "-", "_") } + +// camelize turns a hyphen/underscore-separated name into UpperCamelCase. +func camelize(s string) string { + var b strings.Builder + up := true + for i := 0; i < len(s); i++ { + c := s[i] + if c == '_' || c == '-' { + up = true + continue + } + if up && 'a' <= c && c <= 'z' { + c -= 'a' - 'A' + } + up = false + b.WriteByte(c) + } + return b.String() +} diff --git a/heuristic/heuristic_test.go b/heuristic/heuristic_test.go new file mode 100644 index 0000000..84d09de --- /dev/null +++ b/heuristic/heuristic_test.go @@ -0,0 +1,140 @@ +package heuristic + +import ( + "context" + "testing" + + "github.com/git-pkgs/provides" +) + +func resolveOne(t *testing.T, purl string) provides.Surface { + t.Helper() + res, err := Resolver().ResolveSurface(context.Background(), provides.Package{PURL: purl}, provides.SurfaceOptions{}) + if err != nil { + t.Fatalf("resolve %s: %v", purl, err) + } + return res.Surface +} + +func firstName(t *testing.T, s provides.Surface) provides.ProvidedName { + t.Helper() + if len(s.Provides) == 0 { + t.Fatalf("no provided names: %+v", s) + } + return s.Provides[0] +} + +func TestNPM(t *testing.T) { + n := firstName(t, resolveOne(t, "pkg:npm/ws@8.17.1")) + if n.Language != "javascript" || n.Name != "ws" || !n.Matches("ws/lib/sender") { + t.Errorf("ws: %+v", n) + } + scoped := firstName(t, resolveOne(t, "pkg:npm/%40babel/core")) + if scoped.Name != "@babel/core" || !scoped.Matches("@babel/core/lib/parse") { + t.Errorf("scoped: %+v", scoped) + } + if scoped.Matches("@babel/core-utils") { + t.Error("prefix over-match") + } +} + +func TestPyPI(t *testing.T) { + n := firstName(t, resolveOne(t, "pkg:pypi/Engine-IO-Parser@1.0")) + if n.Name != "engine_io_parser" || !n.Matches("engine_io_parser.decode") { + t.Errorf("pypi normalisation: %+v", n) + } + f := firstName(t, resolveOne(t, "pkg:pypi/Flask")) + if !f.Matches("flask") || f.Matches("Flask") { + t.Errorf("pypi case: %+v", f) + } +} + +func TestGo(t *testing.T) { + n := firstName(t, resolveOne(t, "pkg:golang/github.com/gin-contrib/sse")) + if n.Name != "github.com/gin-contrib/sse" || !n.Matches("github.com/gin-contrib/sse/v2") { + t.Errorf("go module: %+v", n) + } + if n.Matches("github.com/gin-contrib/sse-other") { + t.Error("prefix over-match") + } +} + +func TestGem(t *testing.T) { + s := resolveOne(t, "pkg:gem/active_support") + if len(s.Provides) != 2 { + t.Fatalf("gem should provide feature + constant: %+v", s.Provides) + } + var feat, konst provides.ProvidedName + for _, n := range s.Provides { + switch n.Kind { + case "feature": + feat = n + case "constant": + konst = n + } + } + if !feat.Matches("active_support/core_ext") { + t.Errorf("feature subpath: %+v", feat) + } + if konst.Name != "ActiveSupport" || !konst.Matches("ActiveSupport::Duration") { + t.Errorf("constant: %+v", konst) + } +} + +func TestCargo(t *testing.T) { + n := firstName(t, resolveOne(t, "pkg:cargo/tokio-util")) + if n.Name != "tokio_util" || !n.Matches("tokio_util::codec") { + t.Errorf("cargo hyphen→underscore: %+v", n) + } +} + +func TestComposer(t *testing.T) { + n := firstName(t, resolveOne(t, "pkg:composer/guzzlehttp/guzzle")) + if n.Language != "php" || !n.CaseInsensitive { + t.Errorf("composer should be case-insensitive: %+v", n) + } + if !n.Matches(`GuzzleHttp\Client`) { + t.Errorf("case-folded PSR-4 root: %+v", n) + } + if n.Matches(`App\Models\User`) { + t.Error("unrelated namespace matched") + } +} + +func TestHex(t *testing.T) { + n := firstName(t, resolveOne(t, "pkg:hex/phoenix_html")) + if n.Name != "PhoenixHtml" || !n.Matches("PhoenixHtml.Safe") { + t.Errorf("hex: %+v", n) + } +} + +func TestMaven(t *testing.T) { + n := firstName(t, resolveOne(t, "pkg:maven/com.google.guava/guava")) + if n.Name != "com.google.guava" || !n.Matches("com.google.guava.collect") { + t.Errorf("maven group: %+v", n) + } +} + +func TestUnknownEcosystem(t *testing.T) { + s := resolveOne(t, "pkg:conan/zlib@1.3.1") + if len(s.Provides) != 0 { + t.Errorf("unknown ecosystem should be empty: %+v", s) + } +} + +func TestInvalidPURL(t *testing.T) { + res, err := Resolver().ResolveSurface(context.Background(), provides.Package{PURL: "not a purl"}, provides.SurfaceOptions{}) + if err != nil { + t.Fatalf("invalid purl should be a diagnostic, not an error: %v", err) + } + if len(res.Diagnostics) == 0 { + t.Error("expected diagnostic for invalid purl") + } +} + +func TestEvidence(t *testing.T) { + n := firstName(t, resolveOne(t, "pkg:npm/ws")) + if len(n.Evidence) == 0 || n.Evidence[0].Method != provides.EvidenceHeuristic { + t.Errorf("heuristic evidence: %+v", n.Evidence) + } +} diff --git a/import.go b/import.go index d676ddb..2780529 100644 --- a/import.go +++ b/import.go @@ -62,12 +62,13 @@ func MatchImport(language, name string, project ProjectSurfaceResult) ImportResu provided.Separator = "" } key := importMatchKey{ - purl: surface.PURL, - language: provided.Language, - name: provided.Name, - kind: provided.Kind, - match: provided.Match, - separator: provided.Separator, + purl: surface.PURL, + language: provided.Language, + name: provided.Name, + kind: provided.Kind, + match: provided.Match, + separator: provided.Separator, + caseInsensitive: provided.CaseInsensitive, } if existing, ok := matches[key]; ok { provided.Evidence = mergeEvidence(existing.Provided.Evidence, provided.Evidence) @@ -112,10 +113,11 @@ func MatchImport(language, name string, project ProjectSurfaceResult) ImportResu } type importMatchKey struct { - purl string - language string - name string - kind string - match MatchMode - separator string + purl string + language string + name string + kind string + match MatchMode + separator string + caseInsensitive bool } diff --git a/match.go b/match.go index bfed279..71fa8f9 100644 --- a/match.go +++ b/match.go @@ -3,14 +3,20 @@ package provides import "strings" // Matches reports whether imported is covered by the provided name. Matching -// is case-sensitive because Name retains its exact source-visible spelling. +// is case-sensitive by default because Name retains its exact source-visible +// spelling; CaseInsensitive folds ASCII case for languages whose lookup does. func (name ProvidedName) Matches(imported string) bool { - if imported == name.Name { + target, candidate := name.Name, imported + if name.CaseInsensitive { + target = strings.ToLower(target) + candidate = strings.ToLower(candidate) + } + if candidate == target { return true } return normalizedMatchMode(name.Match) == MatchPrefix && name.Separator != "" && - strings.HasPrefix(imported, name.Name+name.Separator) + strings.HasPrefix(candidate, target+name.Separator) } // Matches reports whether imported is covered by the project binding. Prefix diff --git a/match_test.go b/match_test.go index 697f5a5..64ca2e4 100644 --- a/match_test.go +++ b/match_test.go @@ -45,6 +45,27 @@ func TestProvidedNameMatchesPrefixAtSeparatorBoundary(t *testing.T) { } } +func TestProvidedNameCaseInsensitive(t *testing.T) { + t.Parallel() + + name := ProvidedName{ + Language: "php", + Name: "GuzzleHttp", + Match: MatchPrefix, + Separator: `\`, + CaseInsensitive: true, + } + + for _, imported := range []string{"GuzzleHttp", `GuzzleHttp\Client`, `guzzlehttp\client`, `GUZZLEHTTP\Client`} { + if !name.Matches(imported) { + t.Errorf("Matches(%q) = false, want case-folded true", imported) + } + } + if name.Matches(`GuzzleHttpClient`) { + t.Error("prefix boundary must still be enforced after case folding") + } +} + func TestProvidedNameExactSubpathDoesNotMatchDescendants(t *testing.T) { t.Parallel() diff --git a/merge.go b/merge.go index b5d8d1d..43c913b 100644 --- a/merge.go +++ b/merge.go @@ -39,20 +39,22 @@ func MergeSurfaceResults(purl string, results ...SurfaceResult) SurfaceResult { separator = "" } key := providedNameKey{ - language: name.Language, - name: name.Name, - kind: name.Kind, - match: match, - separator: separator, + language: name.Language, + name: name.Name, + kind: name.Kind, + match: match, + separator: separator, + caseInsensitive: name.CaseInsensitive, } current, ok := provided[key] if !ok { current = ProvidedName{ - Language: name.Language, - Name: name.Name, - Kind: name.Kind, - Match: match, - Separator: separator, + Language: name.Language, + Name: name.Name, + Kind: name.Kind, + Match: match, + Separator: separator, + CaseInsensitive: name.CaseInsensitive, } } current.Evidence = mergeEvidence(current.Evidence, name.Evidence) @@ -154,11 +156,12 @@ func MergeBindingResults(results ...BindingResult) BindingResult { } type providedNameKey struct { - language string - name string - kind string - match MatchMode - separator string + language string + name string + kind string + match MatchMode + separator string + caseInsensitive bool } type bindingKey struct { diff --git a/resolver.go b/resolver.go index 997856a..f1c7f5b 100644 --- a/resolver.go +++ b/resolver.go @@ -11,3 +11,41 @@ type SurfaceResolver interface { type BindingResolver interface { ResolveBindings(ctx context.Context, projectDir string) (BindingResult, error) } + +// SurfaceResolverFunc adapts a function to a SurfaceResolver. +type SurfaceResolverFunc func(ctx context.Context, pkg Package, options SurfaceOptions) (SurfaceResult, error) + +// ResolveSurface calls f. +func (f SurfaceResolverFunc) ResolveSurface(ctx context.Context, pkg Package, options SurfaceOptions) (SurfaceResult, error) { + return f(ctx, pkg, options) +} + +// Chain returns a SurfaceResolver that queries each resolver in order and +// merges every non-empty result for a package. Later resolvers still run +// after an earlier one produces a result so callers can combine, for +// example, a curated catalog with a heuristic fallback and receive both +// mappings with their distinct evidence. A resolver that returns an error +// contributes a diagnostic and the chain continues. +func Chain(resolvers ...SurfaceResolver) SurfaceResolverFunc { + return func(ctx context.Context, pkg Package, options SurfaceOptions) (SurfaceResult, error) { + var results []SurfaceResult + var diagnostics []Diagnostic + for _, r := range resolvers { + if r == nil { + continue + } + res, err := r.ResolveSurface(ctx, pkg, options) + diagnostics = append(diagnostics, res.Diagnostics...) + if err != nil { + diagnostics = append(diagnostics, Diagnostic{Source: "chain", Message: err.Error()}) + continue + } + if len(res.Surface.Provides) > 0 { + results = append(results, res) + } + } + merged := MergeSurfaceResults(pkg.PURL, results...) + merged.Diagnostics = mergeDiagnostics(append(merged.Diagnostics, diagnostics...)) + return merged, nil + } +} diff --git a/resolver_test.go b/resolver_test.go new file mode 100644 index 0000000..4a987d6 --- /dev/null +++ b/resolver_test.go @@ -0,0 +1,94 @@ +package provides + +import ( + "context" + "errors" + "testing" +) + +func fixedResolver(name string, method EvidenceMethod) SurfaceResolverFunc { + return func(_ context.Context, pkg Package, _ SurfaceOptions) (SurfaceResult, error) { + return SurfaceResult{Surface: Surface{ + PURL: pkg.PURL, + Provides: []ProvidedName{{ + Language: "python", Name: name, Kind: "module", + Evidence: []Evidence{{Method: method, Source: "test"}}, + }}, + }}, nil + } +} + +func TestChainMergesResults(t *testing.T) { + t.Parallel() + + chained := Chain( + fixedResolver("yaml", EvidenceCurated), + fixedResolver("pyyaml", EvidenceHeuristic), + ) + res, err := chained.ResolveSurface(context.Background(), Package{PURL: "pkg:pypi/PyYAML@6.0"}, SurfaceOptions{}) + if err != nil { + t.Fatal(err) + } + if len(res.Surface.Provides) != 2 { + t.Fatalf("chain should keep both distinct names: %+v", res.Surface.Provides) + } + for _, n := range res.Surface.Provides { + if len(n.Evidence) != 1 { + t.Errorf("evidence not carried through merge: %+v", n) + } + } +} + +func TestChainDeduplicatesEvidence(t *testing.T) { + t.Parallel() + + chained := Chain( + fixedResolver("flask", EvidenceCurated), + fixedResolver("flask", EvidenceHeuristic), + ) + res, err := chained.ResolveSurface(context.Background(), Package{PURL: "pkg:pypi/flask"}, SurfaceOptions{}) + if err != nil { + t.Fatal(err) + } + if len(res.Surface.Provides) != 1 { + t.Fatalf("identical names should merge: %+v", res.Surface.Provides) + } + if len(res.Surface.Provides[0].Evidence) != 2 { + t.Errorf("evidence from both resolvers should be retained: %+v", res.Surface.Provides[0].Evidence) + } +} + +func TestChainContinuesAfterError(t *testing.T) { + t.Parallel() + + failing := SurfaceResolverFunc(func(_ context.Context, _ Package, _ SurfaceOptions) (SurfaceResult, error) { + return SurfaceResult{}, errors.New("boom") + }) + res, err := Chain(failing, fixedResolver("ok", EvidenceHeuristic)). + ResolveSurface(context.Background(), Package{PURL: "pkg:npm/x"}, SurfaceOptions{}) + if err != nil { + t.Fatalf("chain should not propagate resolver error: %v", err) + } + if len(res.Surface.Provides) != 1 || res.Surface.Provides[0].Name != "ok" { + t.Errorf("later resolver should still run: %+v", res.Surface) + } + if len(res.Diagnostics) == 0 { + t.Error("resolver error should surface as a diagnostic") + } +} + +func TestChainSkipsNilAndEmpty(t *testing.T) { + t.Parallel() + + empty := SurfaceResolverFunc(func(_ context.Context, pkg Package, _ SurfaceOptions) (SurfaceResult, error) { + return SurfaceResult{Surface: Surface{PURL: pkg.PURL}}, nil + }) + res, err := Chain(nil, empty, fixedResolver("x", EvidenceHeuristic)). + ResolveSurface(context.Background(), Package{PURL: "pkg:npm/x"}, SurfaceOptions{}) + if err != nil { + t.Fatal(err) + } + if len(res.Surface.Provides) != 1 { + t.Errorf("nil/empty resolvers should not affect the result: %+v", res.Surface) + } +} diff --git a/types.go b/types.go index d05195b..a00e155 100644 --- a/types.go +++ b/types.go @@ -47,12 +47,20 @@ type Surface struct { // ProvidedName is a source-level name supplied by a package. type ProvidedName struct { - Language string - Name string - Kind string - Match MatchMode + Language string + Name string + Kind string + Match MatchMode + // Separator is the boundary between a prefix name and a matched + // descendant, for example "." for a Python module or "/" for an npm + // subpath. It is unused for MatchExact. Separator string - Evidence []Evidence + // CaseInsensitive folds ASCII case when matching. Set it for languages + // whose module or namespace lookup is itself case-insensitive, for + // example PHP, where `use GuzzleHttp\Client` and `use guzzlehttp\client` + // resolve to the same class. Name still retains its canonical spelling. + CaseInsensitive bool + Evidence []Evidence } // Binding connects a package to the name used by one project. From 29fb3d0f91d03c1ea573d9f73f6975a2a4931314 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 11 Aug 2026 12:44:06 +0100 Subject: [PATCH 2/2] Address review: PyPI comment accuracy, Chain diagnostic dedup, sort determinism The PyPI heuristic comment referenced PEP 503 equivalence of -/_/., but the code only maps hyphens to underscores. That is intentional: dotted distribution names conventionally install as namespace packages with the dot preserved (zope.interface, ruamel.yaml). The comment now states what the code does and a test covers the dotted case. Chain collected each resolver's diagnostics and also passed the full SurfaceResult (including diagnostics) to MergeSurfaceResults, which merged them again. mergeDiagnostics deduplicates so the output was correct, but the work was redundant. Chain now passes only the Surface to the merge and adds diagnostics once. MergeSurfaceResults' sort comparator did not include CaseInsensitive, so two entries differing only in that field could order nondeterministically via map iteration. The comparator now falls through to CaseInsensitive after Separator, sorting the case-sensitive entry first. --- heuristic/heuristic.go | 9 +++++++-- heuristic/heuristic_test.go | 6 ++++++ merge.go | 7 ++++++- resolver.go | 4 +++- resolver_test.go | 24 ++++++++++++++++++++++++ 5 files changed, 46 insertions(+), 4 deletions(-) diff --git a/heuristic/heuristic.go b/heuristic/heuristic.go index 64105c4..34a0718 100644 --- a/heuristic/heuristic.go +++ b/heuristic/heuristic.go @@ -63,8 +63,13 @@ var conventions = map[string]func(*purl.PURL) []provides.ProvidedName{ return []provides.ProvidedName{prefix("javascript", name, "module", "/", false)} }, "pypi": func(p *purl.PURL) []provides.ProvidedName { - // PEP 503 treats -, _, . as equivalent and the registry name is - // case-insensitive; module names are lowercase with underscores. + // Distribution names are case-insensitive; the purl spec + // lowercases and replaces _ with - so p.Name arrives normalised. + // Hyphens are not valid in Python identifiers, so a hyphenated + // distribution conventionally installs an underscored module. A + // dot is preserved because dotted distribution names + // (`zope.interface`, `ruamel.yaml`) conventionally install as + // namespace packages with the same dotted import path. return []provides.ProvidedName{ prefix("python", strings.ToLower(underscore(p.Name)), "module", ".", false), } diff --git a/heuristic/heuristic_test.go b/heuristic/heuristic_test.go index 84d09de..fb7dd6e 100644 --- a/heuristic/heuristic_test.go +++ b/heuristic/heuristic_test.go @@ -47,6 +47,12 @@ func TestPyPI(t *testing.T) { if !f.Matches("flask") || f.Matches("Flask") { t.Errorf("pypi case: %+v", f) } + // Dotted distributions install as namespace packages with the dot + // retained in the import path. + z := firstName(t, resolveOne(t, "pkg:pypi/zope.interface")) + if z.Name != "zope.interface" || !z.Matches("zope.interface.declarations") { + t.Errorf("dotted distribution should keep dot: %+v", z) + } } func TestGo(t *testing.T) { diff --git a/merge.go b/merge.go index 43c913b..494d223 100644 --- a/merge.go +++ b/merge.go @@ -82,7 +82,12 @@ func MergeSurfaceResults(purl string, results ...SurfaceResult) SurfaceResult { if names[i].Match != names[j].Match { return names[i].Match < names[j].Match } - return names[i].Separator < names[j].Separator + if names[i].Separator != names[j].Separator { + return names[i].Separator < names[j].Separator + } + // Case-sensitive before case-insensitive so the exact-spelling + // entry sorts first when both exist. + return !names[i].CaseInsensitive && names[j].CaseInsensitive }) return SurfaceResult{ diff --git a/resolver.go b/resolver.go index f1c7f5b..8125dca 100644 --- a/resolver.go +++ b/resolver.go @@ -35,13 +35,15 @@ func Chain(resolvers ...SurfaceResolver) SurfaceResolverFunc { continue } res, err := r.ResolveSurface(ctx, pkg, options) + // Diagnostics are collected once here; strip them from what + // goes to MergeSurfaceResults so they are not merged twice. diagnostics = append(diagnostics, res.Diagnostics...) if err != nil { diagnostics = append(diagnostics, Diagnostic{Source: "chain", Message: err.Error()}) continue } if len(res.Surface.Provides) > 0 { - results = append(results, res) + results = append(results, SurfaceResult{Surface: res.Surface}) } } merged := MergeSurfaceResults(pkg.PURL, results...) diff --git a/resolver_test.go b/resolver_test.go index 4a987d6..6d7c94c 100644 --- a/resolver_test.go +++ b/resolver_test.go @@ -77,6 +77,30 @@ func TestChainContinuesAfterError(t *testing.T) { } } +func TestChainDiagnosticsNotDoubleCounted(t *testing.T) { + t.Parallel() + + noisy := SurfaceResolverFunc(func(_ context.Context, pkg Package, _ SurfaceOptions) (SurfaceResult, error) { + return SurfaceResult{ + Surface: Surface{PURL: pkg.PURL, Provides: []ProvidedName{{Language: "x", Name: "n"}}}, + Diagnostics: []Diagnostic{{Source: "noisy", Message: "once"}}, + }, nil + }) + res, err := Chain(noisy).ResolveSurface(context.Background(), Package{PURL: "pkg:npm/x"}, SurfaceOptions{}) + if err != nil { + t.Fatal(err) + } + count := 0 + for _, d := range res.Diagnostics { + if d.Source == "noisy" { + count++ + } + } + if count != 1 { + t.Errorf("diagnostic emitted %d times, want 1: %+v", count, res.Diagnostics) + } +} + func TestChainSkipsNilAndEmpty(t *testing.T) { t.Parallel()