Remediate verified compiler review findings

README.md

@@ -54,7 +54,7 @@ Generated source starts with:
 
 ```peeper
 fn main() {
-    println("Hello from Peeper!")
+    println("Hello from Peeper!");
 }
 ```
 

cmd/build.go

@@ -21,12 +21,13 @@ func compileEntry(path string, debugBuild bool, targetOS, targetArch string) (co
 	rootDir := sourceProject.RootDir
 	projectName := sourceProject.ProjectName
 	cfg := project.Config{
-		RootDir:     rootDir,
-		ProjectName: projectName,
-		Extension:   peeper.SourceExt,
-		TargetOS:    targetOS,
-		TargetArch:  targetArch,
-		BuildDebug:  debugBuild,
+		RootDir:           rootDir,
+		ProjectName:       projectName,
+		Extension:         peeper.SourceExt,
+		TargetOS:          targetOS,
+		TargetArch:        targetArch,
+		BuildDebug:        debugBuild,
+		RequireEntrypoint: true,
 	}
 	compilerContext = compiler.NewCompilerContext(cfg, diagnostics.NewDiagnosticBag())
 	if err != nil {

cmd/build_test.go

@@ -2,11 +2,26 @@ package main
 
 import (
 	"reflect"
+	"runtime"
 	"testing"
 
 	"compiler/internal/project"
 )
 
+func TestValidateNativeLinkTarget(t *testing.T) {
+	if err := validateNativeLinkTarget(runtime.GOOS, runtime.GOARCH); err != nil {
+		t.Fatalf("host target rejected: %v", err)
+	}
+
+	targetOS := "linux"
+	if runtime.GOOS == targetOS {
+		targetOS = "windows"
+	}
+	if err := validateNativeLinkTarget(targetOS, runtime.GOARCH); err == nil {
+		t.Fatalf("non-host target %s/%s accepted", targetOS, runtime.GOARCH)
+	}
+}
+
 func TestClangArgsForBuildRelease(t *testing.T) {
 	args := clangArgsForBuild(project.Config{TargetOS: "linux"}, "x86_64-unknown-linux-gnu", []string{"a.ll", "b.ll"}, "demo")
 	want := []string{"-target", "x86_64-unknown-linux-gnu", "-x", "ir", "a.ll", "-x", "ir", "b.ll", "-o", "demo"}

cmd/check_subprocess_test.go

@@ -12,11 +12,7 @@ import (
 
 func TestCheckCommandSupportsRecursiveAndMultipleTargetsWithFailureStatus(t *testing.T) {
 	root := t.TempDir()
-	binary := filepath.Join(root, "peeper")
-	build := exec.Command("go", "build", "-o", binary, ".")
-	if output, err := build.CombinedOutput(); err != nil {
-		t.Fatalf("build CLI: %v\n%s", err, output)
-	}
+	binary := buildTestCLI(t)
 	validDir := filepath.Join(root, "valid")
 	invalidOneDir := filepath.Join(root, "invalid-one")
 	invalidTwoDir := filepath.Join(root, "invalid-two")

cmd/cli/cleanup.go

@@ -8,7 +8,7 @@ import (
 	"compiler/pkg/manifest"
 )
 
-func CleanupCommand(args []string) error {
+func CleanupCommand(_ []string) error {
 	manifestPath, err := manifest.FindManifestPath(".")
 	if err != nil {
 		return err

cmd/cli/get.go

@@ -162,31 +162,18 @@ func installPackageRecursive(httpClient *http.Client, cachePath, repoPath, versi
 		packageID = manifest.PackageID(repoPath, version)
 	}
 	printPackage(repoPath, version)
-	if !registry.IsModuleCached(cachePath, repoPath, version) {
-		printDownload(fmt.Sprintf("Downloading %s@%s...", repoPath, version))
-		if err := registry.DownloadRemotePackage(httpClient, cachePath, repoPath, version, devConfig); err != nil {
-			return fmt.Errorf("download %s@%s: %w", repoPath, version, err)
-		}
-	}
-	printCached()
-
-	modulePath, err := registry.GetModulePath(cachePath, repoPath, version)
+	entry, exists := lockfile.GetDependency(packageID)
+	modulePath, checksum, err := ensurePackageContent(httpClient, cachePath, repoPath, version, devConfig, entry, exists)
 	if err != nil {
 		return err
 	}
+	printCached()
+
 	packageManifest, err := manifest.Load(filepath.Join(modulePath, manifest.FileName))
 	if err != nil {
 		return fmt.Errorf("load package manifest for %s: %w", repoPath, err)
 	}
 
-	transitiveDeps := make([]string, 0)
-	for _, dep := range packageManifest.Dependencies {
-		if dep.Type == manifest.DependencyRemote {
-			transitiveDeps = append(transitiveDeps, dep.Path)
-		}
-	}
-
-	entry, exists := lockfile.GetDependency(packageID)
 	usedBy := []string{}
 	existingDependencies := []string{}
 	if exists {
@@ -196,6 +183,7 @@ func installPackageRecursive(httpClient *http.Client, cachePath, repoPath, versi
 	newEntry := manifest.LockfileEntry{
 		Version:      version,
 		ResolvedURL:  repoPath,
+		Checksum:     checksum,
 		Direct:       directAlias != "",
 		Description:  packageManifest.Package.Name,
 		Dependencies: existingDependencies,
@@ -245,6 +233,37 @@ func installPackageRecursive(httpClient *http.Client, cachePath, repoPath, versi
 	return nil
 }
 
+func ensurePackageContent(httpClient *http.Client, cachePath, repoPath, version string, devConfig *manifest.DevConfig, entry manifest.LockfileEntry, locked bool) (string, string, error) {
+	modulePath, err := registry.GetModulePath(cachePath, repoPath, version)
+	if err != nil {
+		return "", "", err
+	}
+	expectedChecksum := ""
+	if locked && entry.Checksum != "" {
+		checksum, hashErr := registry.ModuleChecksum(modulePath)
+		if hashErr == nil && checksum == entry.Checksum {
+			return modulePath, checksum, nil
+		}
+		expectedChecksum = entry.Checksum
+	} else if locked {
+		if _, statErr := os.Lstat(modulePath); statErr == nil {
+			expectedChecksum, err = registry.ModuleChecksum(modulePath)
+			if err != nil {
+				return "", "", fmt.Errorf("hash legacy cache for %s@%s: %w", repoPath, version, err)
+			}
+		} else if !os.IsNotExist(statErr) {
+			return "", "", fmt.Errorf("inspect legacy cache for %s@%s: %w", repoPath, version, statErr)
+		}
+	}
+
+	printDownload(fmt.Sprintf("Downloading %s@%s...", repoPath, version))
+	checksum, err := registry.DownloadRemotePackage(httpClient, cachePath, repoPath, version, expectedChecksum, devConfig)
+	if err != nil {
+		return "", "", fmt.Errorf("download %s@%s: %w", repoPath, version, err)
+	}
+	return modulePath, checksum, nil
+}
+
 func installPackage(ctx *installContext, packageSpec string) (string, error) {
 	dep, err := manifest.ParseDependency(packageSpec)
 	if err != nil {

cmd/cli/get_test.go

@@ -4,9 +4,11 @@ import (
 	"bytes"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"compiler/pkg/manifest"
+	"compiler/pkg/registry"
 )
 
 func TestInstallAllDependenciesRestoresMissingLockedCache(t *testing.T) {
@@ -72,6 +74,216 @@ build = "lib"
 	if got := loadedManifest.Dependencies["peeper_test_lib"].Version; got != "v0.0.1" {
 		t.Fatalf("expected dependency to be pinned to resolved version, got %q", got)
 	}
+	lock, err := manifest.LoadLockfile(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	entry, ok := lock.GetDependency("github.com/itsfuad/peeper_test_lib@v0.0.1")
+	if !ok || entry.Checksum == "" {
+		t.Fatalf("legacy dependency was not checksum-pinned: %#v", entry)
+	}
+}
+
+func TestInstallDependencyPinsReusesAndRepairsCache(t *testing.T) {
+	root := t.TempDir()
+	mockPackage := filepath.Join(root, "mock", "acme", "pkg-v1.0.0")
+	cachePackage := filepath.Join(manifest.CacheModulesDir(root), "github.com", "acme", "pkg@v1.0.0")
+	mustWriteGetTest(t, filepath.Join(root, manifest.FileName), `name = "app"
+build = "program"
+
+[dependencies]
+pkg = "github.com/acme/pkg"
+
+[dev]
+mock_remote = true
+mock_path = "./mock"
+`)
+	mustWriteGetTest(t, filepath.Join(mockPackage, manifest.FileName), "name = \"pkg\"\nbuild = \"lib\"\n")
+	mustWriteGetTest(t, filepath.Join(mockPackage, "src", "pkg.peep"), "original")
+	mustWriteGetTest(t, filepath.Join(cachePackage, manifest.FileName), "name = \"stale\"\nbuild = \"lib\"\n")
+	mustWriteGetTest(t, filepath.Join(cachePackage, "src", "pkg.peep"), "unlocked-cache")
+	t.Chdir(root)
+
+	if err := installAllDependencies(); err != nil {
+		t.Fatal(err)
+	}
+	lock, err := manifest.LoadLockfile(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	entry, ok := lock.GetDependency("github.com/acme/pkg@v1.0.0")
+	if !ok || entry.Checksum == "" {
+		t.Fatalf("initial install entry = %#v", entry)
+	}
+	if checksum, err := registry.ModuleChecksum(cachePackage); err != nil || checksum != entry.Checksum {
+		t.Fatalf("cache checksum = %q, err=%v, want %q", checksum, err, entry.Checksum)
+	}
+	if data, err := os.ReadFile(filepath.Join(cachePackage, "src", "pkg.peep")); err != nil || string(data) != "original" {
+		t.Fatalf("new resolution reused unpinned cache: %q, err=%v", data, err)
+	}
+
+	offlineMock := mockPackage + ".offline"
+	if err := os.Rename(mockPackage, offlineMock); err != nil {
+		t.Fatal(err)
+	}
+	if err := installAllDependencies(); err != nil {
+		t.Fatalf("valid cache triggered refetch: %v", err)
+	}
+	if err := os.Rename(offlineMock, mockPackage); err != nil {
+		t.Fatal(err)
+	}
+
+	cacheSource := filepath.Join(cachePackage, "src", "pkg.peep")
+	if err := os.WriteFile(cacheSource, []byte("tampered"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := installAllDependencies(); err != nil {
+		t.Fatalf("tampered cache was not repaired: %v", err)
+	}
+	if data, err := os.ReadFile(cacheSource); err != nil || string(data) != "original" {
+		t.Fatalf("repaired source = %q, err=%v", data, err)
+	}
+
+	if err := os.WriteFile(cacheSource, []byte("tampered-again"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(mockPackage, "src", "pkg.peep"), []byte("moved-tag"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	lockBefore, err := os.ReadFile(filepath.Join(root, manifest.LockfileName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := installAllDependencies(); err == nil || !strings.Contains(err.Error(), "checksum") {
+		t.Fatalf("moved tag error = %v", err)
+	}
+	lockAfter, err := os.ReadFile(filepath.Join(root, manifest.LockfileName))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !bytes.Equal(lockAfter, lockBefore) {
+		t.Fatal("moved tag changed lockfile")
+	}
+	if data, err := os.ReadFile(cacheSource); err != nil || string(data) != "tampered-again" {
+		t.Fatalf("moved tag replaced prior cache: %q, err=%v", data, err)
+	}
+}
+
+func TestInstallDependencyPinsTransitivePackages(t *testing.T) {
+	root := t.TempDir()
+	mustWriteGetTest(t, filepath.Join(root, manifest.FileName), `name = "app"
+build = "program"
+
+[dependencies]
+parent = "github.com/acme/parent"
+
+[dev]
+mock_remote = true
+mock_path = "./mock"
+`)
+	mustWriteGetTest(t, filepath.Join(root, "mock", "acme", "parent-v1.0.0", manifest.FileName), `name = "parent"
+build = "lib"
+
+[dependencies]
+child = "github.com/acme/child"
+`)
+	mustWriteGetTest(t, filepath.Join(root, "mock", "acme", "child-v1.0.0", manifest.FileName), "name = \"child\"\nbuild = \"lib\"\n")
+	t.Chdir(root)
+
+	if err := installAllDependencies(); err != nil {
+		t.Fatal(err)
+	}
+	lock, err := manifest.LoadLockfile(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, packageID := range []string{"github.com/acme/parent@v1.0.0", "github.com/acme/child@v1.0.0"} {
+		entry, ok := lock.GetDependency(packageID)
+		if !ok || entry.Checksum == "" {
+			t.Fatalf("package %s entry = %#v", packageID, entry)
+		}
+	}
+}
+
+func TestLegacyChecksumMigrationRequiresMatchingRemote(t *testing.T) {
+	tests := []struct {
+		name          string
+		remoteContent string
+		wantError     bool
+	}{
+		{name: "matching", remoteContent: "cached"},
+		{name: "disagreement", remoteContent: "moved", wantError: true},
+		{name: "offline", wantError: true},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			root := t.TempDir()
+			cachePackage := filepath.Join(manifest.CacheModulesDir(root), "github.com", "acme", "pkg@v1.0.0")
+			mustWriteGetTest(t, filepath.Join(root, manifest.FileName), `name = "app"
+build = "program"
+
+[dependencies]
+pkg = "github.com/acme/pkg@v1.0.0"
+
+[dev]
+mock_remote = true
+mock_path = "./mock"
+`)
+			mustWriteGetTest(t, filepath.Join(cachePackage, manifest.FileName), "name = \"pkg\"\nbuild = \"lib\"\n")
+			mustWriteGetTest(t, filepath.Join(cachePackage, "src", "pkg.peep"), "cached")
+			if test.remoteContent != "" {
+				mockPackage := filepath.Join(root, "mock", "acme", "pkg-v1.0.0")
+				mustWriteGetTest(t, filepath.Join(mockPackage, manifest.FileName), "name = \"pkg\"\nbuild = \"lib\"\n")
+				mustWriteGetTest(t, filepath.Join(mockPackage, "src", "pkg.peep"), test.remoteContent)
+			}
+			lock := manifest.NewLockfile()
+			packageID := "github.com/acme/pkg@v1.0.0"
+			lock.SetDependency(packageID, manifest.LockfileEntry{Version: "v1.0.0", ResolvedURL: "github.com/acme/pkg", Direct: true})
+			lock.SetDirectDependency("pkg", packageID)
+			if err := manifest.SaveLockfile(root, lock); err != nil {
+				t.Fatal(err)
+			}
+			lockBefore, err := os.ReadFile(filepath.Join(root, manifest.LockfileName))
+			if err != nil {
+				t.Fatal(err)
+			}
+			cacheBefore, err := registry.ModuleChecksum(cachePackage)
+			if err != nil {
+				t.Fatal(err)
+			}
+			t.Chdir(root)
+
+			err = installAllDependencies()
+			if test.wantError {
+				if err == nil {
+					t.Fatal("legacy migration succeeded")
+				}
+				lockAfter, readErr := os.ReadFile(filepath.Join(root, manifest.LockfileName))
+				if readErr != nil {
+					t.Fatal(readErr)
+				}
+				if !bytes.Equal(lockAfter, lockBefore) {
+					t.Fatal("failed legacy migration changed lockfile")
+				}
+				cacheAfter, hashErr := registry.ModuleChecksum(cachePackage)
+				if hashErr != nil || cacheAfter != cacheBefore {
+					t.Fatalf("failed legacy migration changed cache: %q, err=%v", cacheAfter, hashErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			migrated, err := manifest.LoadLockfile(root)
+			if err != nil {
+				t.Fatal(err)
+			}
+			entry, ok := migrated.GetDependency(packageID)
+			if !ok || entry.Checksum != cacheBefore {
+				t.Fatalf("migrated entry = %#v, want checksum %q", entry, cacheBefore)
+			}
+		})
+	}
 }
 
 func TestPrepareInstallContextPropagatesMalformedLockfile(t *testing.T) {