refactor: comprehensive cleanup and interface improvements

This commit performs a comprehensive cleanup of deprecated/unused code and improves the Tool interface design:

**Removed Deprecated/Unused Code:**
- Removed deprecated InstallTool(), InstallTools(), InstallToolsParallel(), InstallToolsWithOptions() methods
- Removed unused InstallOptions struct (replaced with EnsureTools/EnsureTool)
- Removed redundant Name() methods from all tools (consolidated on GetToolName())
- Removed deprecated Pre/Post hook fields from CommandConfig (part of removed built-in command system)
- Removed unused ClearAllCaches(), ClearPathCache() methods and CacheManager interface
- Updated all callers to use EnsureTools/EnsureTool instead of deprecated methods

**Enhanced GetChecksum Interface:**
- Updated GetChecksum signature from (version, filename) to (version, cfg, filename)
- Provides tools access to full ToolConfig including distribution info and options
- Enables more efficient and context-aware checksum fetching
- Updated all tool implementations: JavaTool, GoTool, NodeTool, MavenTool, MvndTool
- Updated call sites in download.go and tests

**Removed Unused Internal Methods:**
- Removed getChecksumURL() from JavaTool, GoTool, NodeTool (not used)
- Removed getVersionsURL() from all tools (not used anywhere)
- Only kept getChecksumURL() in MavenTool and MvndTool where actually used

**Interface Simplification:**
- Eliminated entire ToolMetadataProvider interface
- Moved GetDisplayName() into main Tool interface
- Removed GetEmoji() method entirely (using default emoji in display code)
- Made GetChecksumURL() and GetVersionsURL() private implementation details

**Java Checksum Architecture Clarification:**
- Java checksums are handled during installation via getDownloadURLWithChecksum()
- GetChecksum() method returns informative error since checksums are pre-fetched
- Maintains existing efficient integration with Disco API during URL resolution

**Benefits:**
- Cleaner Tool interface with only necessary public methods
- Removed 243+ lines of deprecated/unused code
- More consistent and extensible GetChecksum signature
- Better separation of concerns between installation-time and on-demand checksum fetching
- Improved maintainability by eliminating dead code
- Follows SOLID principles with focused interfaces

All tests pass and functionality is preserved while significantly reducing code complexity.

Author: gnodet

cmd/root.go

@@ -185,13 +185,7 @@ func autoSetupEnvironment() error {
 		tempCfg := *cfg
 		tempCfg.Tools = filteredToolsToInstall
 
-		opts := &tools.InstallOptions{
-			MaxConcurrent: 3,
-			Parallel:      true,
-			Verbose:       verbose,
-		}
-
-		if err := manager.InstallToolsWithOptions(&tempCfg, opts); err != nil {
+		if err := manager.EnsureTools(&tempCfg, 3); err != nil {
 			return fmt.Errorf("failed to auto-install tools: %w", err)
 		}
 

cmd/setup.go

@@ -92,24 +92,18 @@ func setupEnvironment() error {
 	// Install tools with options
 	printInfo("📦 Installing tools...")
 
-	// Configure installation options
-	opts := &tools.InstallOptions{
-		MaxConcurrent: parallelDownloads,
-		Parallel:      !sequentialInstall,
-		Verbose:       verbose,
+	// Configure concurrency
+	maxConcurrent := parallelDownloads
+	if maxConcurrent == 0 {
+		maxConcurrent = tools.GetDefaultConcurrency()
 	}
 
-	// Use default concurrency if not specified
-	if parallelDownloads == 0 {
-		opts.MaxConcurrent = tools.GetDefaultConcurrency()
-	}
-
-	// Override parallel setting if sequential flag is set
+	// Use sequential if requested
 	if sequentialInstall {
-		opts.Parallel = false
+		maxConcurrent = 1
 	}
 
-	if err := manager.InstallToolsWithOptions(cfg, opts); err != nil {
+	if err := manager.EnsureTools(cfg, maxConcurrent); err != nil {
 		return fmt.Errorf("failed to install tools: %w", err)
 	}
 

cmd/tools.go

@@ -106,12 +106,8 @@ func listTools() error {
 		}
 
 		// Get display metadata
-		emoji := "📦" // default emoji
-		displayName := toolName
-		if metadataProvider, ok := tool.(tools.ToolMetadataProvider); ok {
-			emoji = metadataProvider.GetEmoji()
-			displayName = metadataProvider.GetDisplayName()
-		}
+		emoji := "📦" // default emoji for all tools
+		displayName := tool.GetDisplayName()
 
 		printInfo("%s %s", emoji, displayName)
 

pkg/config/config.go

@@ -52,9 +52,6 @@ type CommandConfig struct {
 	Environment map[string]string  `json:"environment,omitempty" yaml:"environment,omitempty"`
 	Interpreter string             `json:"interpreter,omitempty" yaml:"interpreter,omitempty"` // "native" (default), "mvx-shell"
 
-	// Hooks (deprecated - kept for backward compatibility but not used)
-	Pre  interface{} `json:"pre,omitempty" yaml:"pre,omitempty"`   // Deprecated: no longer used
-	Post interface{} `json:"post,omitempty" yaml:"post,omitempty"` // Deprecated: no longer used
 }
 
 // PlatformScript represents platform-specific script definitions

pkg/tools/base_tool.go

@@ -138,11 +138,6 @@ func (b *BaseTool) clearPathCache() {
 	b.pathCache = make(map[string]pathCacheEntry)
 }
 
-// ClearPathCache clears the path cache (public method for CacheManager interface)
-func (b *BaseTool) ClearPathCache() {
-	b.clearPathCache()
-}
-
 // getPlatformBinaryPath returns the platform-specific binary path
 func (b *BaseTool) getPlatformBinaryPath(binPath, binaryName string) string {
 	return b.getPlatformBinaryPathWithExtension(binPath, binaryName)
@@ -502,16 +497,10 @@ func (b *BaseTool) IsInstalled(binPath string) bool {
 	return err == nil
 }
 
-// GetDisplayName returns the display name for this tool
-// Tools can implement GetDisplayName() to provide their own display name
+// GetDisplayName returns the default display name for this tool
+// Tools should override this method to provide their own display name
 func (b *BaseTool) GetDisplayName() string {
-	// Use reflection to check if the concrete tool implements GetDisplayName
-	if tool, err := b.manager.GetTool(b.toolName); err == nil {
-		if nameProvider, hasMethod := tool.(interface{ GetDisplayName() string }); hasMethod {
-			return nameProvider.GetDisplayName()
-		}
-	}
-	// Fall back to title case of tool name
+	// Default implementation: title case of tool name
 	return strings.Title(b.toolName)
 }
 

pkg/tools/checksum_test.go

@@ -6,6 +6,8 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/gnodet/mvx/pkg/config"
 )
 
 func TestChecksumVerifier_VerifyFile(t *testing.T) {
@@ -179,12 +181,13 @@ func TestJavaToolChecksum(t *testing.T) {
 
 	javaTool := NewJavaTool(manager)
 
-	// Test the GetChecksum method (should return error since Java checksums come from config)
-	_, err = javaTool.GetChecksum("21", "test-file.tar.gz")
+	// Test the GetChecksum method (should return error since Java checksums are handled during installation)
+	cfg := config.ToolConfig{Distribution: "temurin"}
+	checksum, err := javaTool.GetChecksum("21", cfg, "test-file.tar.gz")
 	if err == nil {
-		t.Errorf("Expected error for Java checksum without configuration")
+		t.Errorf("Expected error for Java checksum since it's handled during installation, but got checksum: %v", checksum)
 	}
-	t.Logf("Java checksum correctly returns error: %v", err)
+	t.Logf("Java checksum correctly returns error (handled during installation): %v", err)
 }
 
 func TestNodeToolChecksum(t *testing.T) {
@@ -196,7 +199,8 @@ func TestNodeToolChecksum(t *testing.T) {
 	nodeTool := NewNodeTool(manager)
 
 	// Test with a known Node.js version (this is a real API call)
-	checksum, err := nodeTool.GetChecksum("22.14.0", "node-v22.14.0-linux-x64.tar.xz")
+	nodeCfg := config.ToolConfig{}
+	checksum, err := nodeTool.GetChecksum("22.14.0", nodeCfg, "node-v22.14.0-linux-x64.tar.xz")
 	if err != nil {
 		t.Logf("Node.js checksum fetch failed (expected in CI): %v", err)
 		return // Skip test if API is not accessible

pkg/tools/download.go

@@ -442,26 +442,15 @@ func verifyChecksum(filePath string, config *DownloadConfig) error {
 		fmt.Printf("  🔍 Attempting to find checksum for file: %s\n", filename)
 
 		// Use tool's GetChecksum method for dynamic checksum resolution
-		if dynamicChecksum, err := config.Tool.GetChecksum(config.Version, filename); err == nil {
+		if dynamicChecksum, err := config.Tool.GetChecksum(config.Version, config.Config, filename); err == nil {
 			checksumInfo = dynamicChecksum
 			hasChecksum = true
 		} else {
 			fmt.Printf("  ⚠️  Tool checksum lookup failed: %v\n", err)
 		}
 
-		if !hasChecksum {
-			// Fallback to URL patterns for tools with static checksum URLs
-			checksumURL := config.Tool.GetChecksumURL(config.Version, filename)
-			if checksumURL != "" {
-				fmt.Printf("  🔍 Using checksum URL pattern: %s\n", checksumURL)
-				checksumInfo = ChecksumInfo{
-					Type:     SHA256,
-					URL:      checksumURL,
-					Filename: filename,
-				}
-				hasChecksum = true
-			}
-		}
+		// Tools should handle checksum fetching in their GetChecksum method
+		// No fallback URL pattern logic needed
 	}
 
 	if !hasChecksum {

pkg/tools/go.go

@@ -15,7 +15,6 @@ import (
 
 // Compile-time interface validation
 var _ Tool = (*GoTool)(nil)
-var _ ToolMetadataProvider = (*GoTool)(nil)
 var _ EnvironmentProvider = (*GoTool)(nil)
 
 // GoTool implements Tool interface for Go toolchain management
@@ -38,11 +37,6 @@ func NewGoTool(manager *Manager) *GoTool {
 	}
 }
 
-// Name returns the tool name
-func (g *GoTool) Name() string {
-	return ToolGo
-}
-
 // Install downloads and installs the specified Go version
 func (g *GoTool) Install(version string, cfg config.ToolConfig) error {
 	return g.StandardInstall(version, cfg, g.getDownloadURL)
@@ -64,7 +58,7 @@ func (g *GoTool) GetBinaryName() string {
 
 // getInstalledPath returns the path for an installed Go version
 func (g *GoTool) getInstalledPath(version string, cfg config.ToolConfig) (string, error) {
-	installDir := g.manager.GetToolVersionDir(g.Name(), version, "")
+	installDir := g.manager.GetToolVersionDir(g.GetToolName(), version, "")
 	pathResolver := NewPathResolver(g.manager.GetToolsDir())
 	binDir, err := pathResolver.FindBinaryParentDir(installDir, g.GetBinaryName())
 	if err != nil {
@@ -105,11 +99,6 @@ func (g *GoTool) GetDisplayName() string {
 	return "Go Programming Language"
 }
 
-// GetEmoji returns the emoji icon for Go (implements ToolMetadataProvider)
-func (g *GoTool) GetEmoji() string {
-	return "🐹"
-}
-
 // SetupEnvironment sets up Go-specific environment variables (implements EnvironmentProvider)
 func (g *GoTool) SetupEnvironment(version string, cfg config.ToolConfig, envManager *EnvironmentManager) error {
 	util.LogVerbose("Go SetupEnvironment called for version %s", version)
@@ -249,7 +238,7 @@ func (g *GoTool) ResolveVersion(versionSpec, distribution string) (string, error
 }
 
 // GetChecksum implements ChecksumProvider interface for Go
-func (g *GoTool) GetChecksum(version, filename string) (ChecksumInfo, error) {
+func (g *GoTool) GetChecksum(version string, cfg config.ToolConfig, filename string) (ChecksumInfo, error) {
 	fmt.Printf("  🔍 Fetching Go checksum from go.dev API...\n")
 
 	checksum, err := g.fetchGoChecksum(version, filename)
@@ -321,13 +310,3 @@ func (g *GoTool) fetchGoChecksum(version, filename string) (string, error) {
 func (g *GoTool) GetDownloadURL(version string) string {
 	return g.getDownloadURL(version)
 }
-
-// GetChecksumURL implements URLProvider interface for Go
-func (g *GoTool) GetChecksumURL(version, filename string) string {
-	return GoDevAPIBase + "/?mode=json&include=all"
-}
-
-// GetVersionsURL implements URLProvider interface for Go
-func (g *GoTool) GetVersionsURL() string {
-	return GitHubAPIBase + "/repos/golang/go/tags?per_page=100"
-}

pkg/tools/java.go

@@ -20,7 +20,6 @@ import (
 var _ Tool = (*JavaTool)(nil)
 var _ DistributionProvider = (*JavaTool)(nil)
 var _ DistributionVersionProvider = (*JavaTool)(nil)
-var _ ToolMetadataProvider = (*JavaTool)(nil)
 var _ VersionValidator = (*JavaTool)(nil)
 var _ EnvironmentProvider = (*JavaTool)(nil)
 
@@ -122,11 +121,6 @@ func NewJavaTool(manager *Manager) *JavaTool {
 	}
 }
 
-// Name returns the tool name
-func (j *JavaTool) Name() string {
-	return ToolJava
-}
-
 // Install downloads and installs the specified Java version
 func (j *JavaTool) Install(version string, cfg config.ToolConfig) error {
 	distribution := cfg.Distribution
@@ -503,26 +497,18 @@ func (j *JavaTool) GetDisplayName() string {
 	return "Java Development Kit"
 }
 
-// GetEmoji returns the emoji icon for Java (implements ToolMetadataProvider)
-func (j *JavaTool) GetEmoji() string {
-	return "📦"
-}
-
 // SetupEnvironment sets up Java-specific environment variables (implements EnvironmentProvider)
 func (j *JavaTool) SetupEnvironment(version string, cfg config.ToolConfig, envManager *EnvironmentManager) error {
-	binPath, err := j.GetPath(version, cfg)
-	if err != nil {
-		util.LogVerbose("Could not determine %s for %s %s: %v", EnvJavaHome, j.toolName, version, err)
-		return nil
-	}
-
-	if strings.HasSuffix(binPath, "/bin") {
-		homeDir := strings.TrimSuffix(binPath, "/bin")
-		envManager.SetEnv(EnvJavaHome, homeDir)
-		util.LogVerbose("Set %s=%s for %s %s", EnvJavaHome, homeDir, j.toolName, version)
+	// Convert EnvironmentManager to map for the existing helper
+	envVars := envManager.ToMap()
+	err := j.SetupHomeEnvironment(version, cfg, envVars, EnvJavaHome, j.GetPath)
+	// Update the environment manager with any changes
+	for key, value := range envVars {
+		if key != "PATH" { // PATH is handled separately by EnvironmentManager
+			envManager.SetEnv(key, value)
+		}
 	}
-
-	return nil
+	return err
 }
 
 // DiscoMajorVersion represents a major version entry from Disco API
@@ -954,11 +940,12 @@ func (j *JavaTool) getChecksumFromDiscoAPI(packageID string) (ChecksumInfo, erro
 }
 
 // GetChecksum implements ChecksumProvider interface for Java
-func (j *JavaTool) GetChecksum(version, filename string) (ChecksumInfo, error) {
-	// Java checksums are provided via configuration from the Java tool
-	// which extracts them from the Foojay Disco API during URL resolution
-	fmt.Printf("  🔍 Checking for Java checksum from Disco API...\n")
-	return ChecksumInfo{}, fmt.Errorf("Java checksums should be provided via configuration")
+func (j *JavaTool) GetChecksum(version string, cfg config.ToolConfig, filename string) (ChecksumInfo, error) {
+	// Java checksums are handled during the installation process via getDownloadURLWithChecksum
+	// and getChecksumFromDiscoAPI, which adds checksums to the configuration automatically.
+	// This method is not used for Java since checksums are fetched during URL resolution.
+	fmt.Printf("  ℹ️  Java checksums are handled during installation via Disco API\n")
+	return ChecksumInfo{}, fmt.Errorf("Java checksums are provided via configuration during installation")
 }
 
 // ValidateVersion validates that a Java version exists (implements VersionValidator)
@@ -1032,14 +1019,3 @@ func (j *JavaTool) GetDownloadURL(version string) string {
 	}
 	return url
 }
-
-// GetChecksumURL implements Tool interface for Java
-func (j *JavaTool) GetChecksumURL(version, filename string) string {
-	// Java checksums are provided via Disco API package info
-	return ""
-}
-
-// GetVersionsURL implements Tool interface for Java
-func (j *JavaTool) GetVersionsURL() string {
-	return FoojayDiscoAPIBase + "/major_versions?distribution=temurin&maintained=true"
-}