Match PyPI tool dependencies by canonical name

cmd/brief/dependency_names_test.go

@@ -0,0 +1,67 @@
+package main
+
+import (
+	"encoding/json"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+
+	"github.com/git-pkgs/brief"
+)
+
+func TestScanPyPIDependencyNames(t *testing.T) {
+	const helperEnv = "BRIEF_PYPI_NAMES_ROOT"
+	if root := os.Getenv(helperEnv); root != "" {
+		cmdScan([]string{"-json", root})
+		return
+	}
+	root, err := filepath.Abs("../../testdata/pypi-names-project")
+	if err != nil {
+		t.Fatal(err)
+	}
+	cmd := exec.Command(os.Args[0], "-test.run=^TestScanPyPIDependencyNames$")
+	cmd.Env = append(os.Environ(), helperEnv+"="+root, "PATH=")
+	out, err := cmd.Output()
+	if err != nil {
+		t.Fatalf("scan failed: %v", err)
+	}
+	var report brief.Report
+	if err := json.Unmarshal(out, &report); err != nil {
+		t.Fatalf("parsing scan output: %v\n%s", err, out)
+	}
+	if !reportHasTool(&report, "build", "Flask") || !reportHasTool(&report, "docs", "MyST-Parser") {
+		t.Fatalf("tools = %+v, want Flask and MyST-Parser", report.Tools)
+	}
+}
+
+func TestDiffPyPIDependencyNames(t *testing.T) {
+	const helperEnv = "BRIEF_DIFF_PYPI_NAMES"
+	if os.Getenv(helperEnv) != "" {
+		cmdDiff([]string{"-json", "HEAD"})
+		os.Exit(0)
+	}
+	if _, err := exec.LookPath("git"); err != nil {
+		t.Skip("git not installed")
+	}
+	root := t.TempDir()
+	initGitScanFixture(t, root)
+	writeScanFixture(t, root, "requirements.txt", "FLask\n")
+	runGitFixture(t, root, "add", "requirements.txt")
+	runGitFixture(t, root, "commit", "-q", "-m", "initial")
+	writeScanFixture(t, root, "requirements.txt", "FLask\nMYST_PARSER\n")
+	cmd := exec.Command(os.Args[0], "-test.run=^TestDiffPyPIDependencyNames$")
+	cmd.Dir = root
+	cmd.Env = append(os.Environ(), helperEnv+"=1")
+	out, err := cmd.Output()
+	if err != nil {
+		t.Fatalf("diff failed: %v", err)
+	}
+	var report brief.Report
+	if err := json.Unmarshal(out, &report); err != nil {
+		t.Fatalf("parsing diff output: %v\n%s", err, out)
+	}
+	if !reportHasTool(&report, "build", "Flask") || !reportHasTool(&report, "docs", "MyST-Parser") {
+		t.Fatalf("tools = %+v, want Flask and MyST-Parser", report.Tools)
+	}
+}

detect/dependency_names_test.go

@@ -0,0 +1,103 @@
+package detect
+
+import (
+	"testing"
+	"testing/fstest"
+)
+
+func TestPyPIDependencyNamesThroughScan(t *testing.T) {
+	for _, tt := range []struct{ name, tool, category string }{
+		{"FLask", "Flask", "build"},
+		{"MYST_PARSER", "MyST-Parser", "docs"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			engine := NewFS(loadKB(t), fstest.MapFS{"requirements.txt": {Data: []byte(tt.name + "\n")}})
+			report, err := engine.Run()
+			if err != nil {
+				t.Fatal(err)
+			}
+			assertToolDetected(t, report, tt.category, tt.tool)
+			if len(report.Dependencies) != 1 || report.Dependencies[0].Name != tt.name {
+				t.Fatalf("dependency spelling changed: %+v", report.Dependencies)
+			}
+		})
+	}
+}
+
+func TestPyPIDependencyNamesRespectDevelopmentScope(t *testing.T) {
+	for _, tt := range []struct {
+		name, content string
+		want          bool
+	}{
+		{"runtime", "[project]\nname = 'example'\ndependencies = ['PyTest']\n", false},
+		{"development", "[tool.poetry.group.test.dependencies]\nPyTest = '*'\n", true},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			base := loadKB(t)
+			base.ByName["pytest"].Detect.Dependencies = nil
+			engine := NewFS(base, fstest.MapFS{"pyproject.toml": {Data: []byte(tt.content)}})
+			report, err := engine.Run()
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tt.want {
+				assertToolDetected(t, report, "test", "pytest")
+			} else {
+				assertToolNotDetected(t, report, "test", "pytest")
+			}
+		})
+	}
+}
+
+func TestNonPyPIDependencyNamesRemainCaseSensitive(t *testing.T) {
+	for _, tt := range []struct {
+		name string
+		want bool
+	}{
+		{"testthat", true},
+		{"TestThat", false},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			content := "Package: example\nVersion: 1.0.0\nSuggests: " + tt.name + "\n"
+			engine := NewFS(loadKB(t), fstest.MapFS{"DESCRIPTION": {Data: []byte(content)}})
+			report, err := engine.Run()
+			if err != nil {
+				t.Fatal(err)
+			}
+			if tt.want {
+				assertToolDetected(t, report, "test", "testthat")
+			} else {
+				assertToolNotDetected(t, report, "test", "testthat")
+			}
+		})
+	}
+}
+
+func TestPyPIDependencyNamesInChangedFiles(t *testing.T) {
+	base := loadKB(t)
+	engine := NewFS(base, fstest.MapFS{
+		"requirements.txt": {Data: []byte("MYST_PARSER\n")},
+		"app.py":           {Data: []byte("print('example')\n")},
+	})
+	report, err := engine.Run()
+	if err != nil {
+		t.Fatal(err)
+	}
+	assertToolDetected(t, FilterByChangedFiles(report, base, []string{"requirements.txt"}), "docs", "MyST-Parser")
+	assertToolNotDetected(t, FilterByChangedFiles(report, base, []string{"app.py"}), "docs", "MyST-Parser")
+}
+
+func TestPyPIDevelopmentDeclarationMatchesLockfileSpelling(t *testing.T) {
+	base := loadKB(t)
+	base.ByName["pytest"].Detect.Dependencies = nil
+	engine := NewFS(base, fstest.MapFS{
+		"pyproject.toml": {Data: []byte("[tool.poetry.group.test.dependencies]\nPyTest = '*'\n")},
+		"poetry.lock":    {Data: []byte("[[package]]\nname = 'pytest'\nversion = '8.0.0'\ncategory = 'main'\n")},
+	})
+	report, err := engine.Run()
+	if err != nil {
+		t.Fatal(err)
+	}
+	assertToolDetected(t, report, "test", "pytest")
+	assertToolDetected(t, FilterByChangedFiles(report, base, []string{"poetry.lock"}), "test", "pytest")
+}

detect/detect.go

@@ -26,6 +26,7 @@ import (
 	"github.com/git-pkgs/brief/kb"
 	"github.com/git-pkgs/licenses"
 	"github.com/git-pkgs/manifests"
+	"github.com/git-pkgs/purl"
 	"github.com/git-pkgs/roles"
 	"go.yaml.in/yaml/v3"
 )
@@ -86,8 +87,8 @@ type Engine struct {
 	fileRoles            map[string]roles.Set
 	dirCache             map[string][]string
 	depsLoaded           bool
-	runtimeDeps          map[string]bool // all runtime/unscoped dependency names
-	devDeps              map[string]bool // development/test/build dependency names
+	runtimeDeps          map[string]bool // all runtime/unscoped dependency keys
+	devDeps              map[string]bool // development/test/build dependency keys
 	allDeps              map[string]bool // union of both
 	parsedDeps           []brief.DepInfo // direct dependencies with PURLs
 	manifests            []brief.ManifestInfo
@@ -1064,12 +1065,13 @@ func (e *Engine) loadDeps() {
 		})
 
 		for _, dep := range result.Dependencies {
-			e.allDeps[dep.Name] = true
+			key := dependencyKey(dep.Ecosystem, dep.Name)
+			e.allDeps[key] = true
 			switch dep.Scope {
 			case manifests.Development, manifests.Test, manifests.Build:
-				e.devDeps[dep.Name] = true
+				e.devDeps[key] = true
 			default:
-				e.runtimeDeps[dep.Name] = true
+				e.runtimeDeps[key] = true
 			}
 			// Include transitive deps from lockfiles and Go manifests.
 			// Go is special: go.mod acts as both manifest and lockfile,
@@ -1455,18 +1457,27 @@ func (e *Engine) hasDependency(tool *kb.ToolDef) bool {
 	e.loadDeps()
 
 	for _, dep := range tool.Detect.Dependencies {
-		if e.allDeps[dep] {
+		if e.allDeps[dep] || e.allDeps[dependencyKey("pypi", dep)] {
 			return true
 		}
 	}
 	for _, dep := range tool.Detect.DevDependencies {
-		if e.devDeps[dep] {
+		if e.devDeps[dep] || e.devDeps[dependencyKey("pypi", dep)] {
 			return true
 		}
 	}
 	return false
 }
 
+func dependencyKey(ecosystem, name string) string {
+	if ecosystem == "pypi" {
+		if key := purl.BuildPURLString(ecosystem, name, "", ""); key != "" {
+			return key
+		}
+	}
+	return name
+}
+
 // hasKey checks if a structured file (JSON, TOML) contains any of the given
 // dot-separated key paths (e.g. "scripts.test" in package.json).
 func (e *Engine) hasKey(file string, keys []string) bool {

detect/filter.go

@@ -159,7 +159,8 @@ func (fc *filterContext) setChangedDependencies(dependencies []brief.DepInfo, ma
 		}
 		manifest := manifestByPath[filepath.ToSlash(filepath.Clean(dependency.Manifest))]
 		if manifest.Kind == "manifest" {
-			devDependencyManifests[dependency.Name] = append(devDependencyManifests[dependency.Name], manifest)
+			key := reportDependencyKey(dependency, manifest)
+			devDependencyManifests[key] = append(devDependencyManifests[key], manifest)
 		}
 	}
 
@@ -168,15 +169,24 @@ func (fc *filterContext) setChangedDependencies(dependencies []brief.DepInfo, ma
 		if !fc.changed[manifest] {
 			continue
 		}
-		fc.changedDependencies[dependency.Name] = true
+		key := reportDependencyKey(dependency, manifestByPath[manifest])
+		fc.changedDependencies[key] = true
 		if isDevelopmentScope(dependency.Scope) || lockfileHasDevelopmentDeclaration(
-			manifestByPath[manifest], devDependencyManifests[dependency.Name],
+			manifestByPath[manifest], devDependencyManifests[key],
 		) {
-			fc.changedDevDependencies[dependency.Name] = true
+			fc.changedDevDependencies[key] = true
 		}
 	}
 }
 
+func reportDependencyKey(dependency brief.DepInfo, manifest brief.ManifestInfo) string {
+	ecosystem := manifest.Ecosystem
+	if strings.HasPrefix(dependency.PURL, "pkg:pypi/") {
+		ecosystem = "pypi"
+	}
+	return dependencyKey(ecosystem, dependency.Name)
+}
+
 func isDevelopmentScope(scope string) bool {
 	switch brief.Scope(scope) {
 	case brief.ScopeDevelopment, brief.ScopeTest, brief.ScopeBuild:
@@ -205,12 +215,12 @@ func lockfileHasDevelopmentDeclaration(lockfile brief.ManifestInfo, manifests []
 
 func (fc *filterContext) dependencyAffected(tool *kb.ToolDef) bool {
 	for _, dependency := range tool.Detect.Dependencies {
-		if fc.changedDependencies[dependency] {
+		if fc.changedDependencies[dependency] || fc.changedDependencies[dependencyKey("pypi", dependency)] {
 			return true
 		}
 	}
 	for _, dependency := range tool.Detect.DevDependencies {
-		if fc.changedDevDependencies[dependency] {
+		if fc.changedDevDependencies[dependency] || fc.changedDevDependencies[dependencyKey("pypi", dependency)] {
 			return true
 		}
 	}

testdata/pypi-names-project/requirements.txt

@@ -0,0 +1,2 @@
+FLask
+MYST_PARSER