NativePythonFinder: Code quality improvements - type safety, logging, and robustness

[karthiknadig]

Summary

Code quality improvements identified in nativePythonFinder.ts that don't cause functional issues but improve maintainability, type safety, and robustness.

---

Issues Identified

1. isNativeEnvInfo() Should Be a Type Guard

Current implementation (line 55):

export function isNativeEnvInfo(info: NativeInfo): boolean {
    return !(info as NativeEnvManagerInfo).tool;
}

Should use TypeScript's type guard syntax for proper type narrowing:

export function isNativeEnvInfo(info: NativeInfo): info is NativeEnvInfo {
    return !(info as NativeEnvManagerInfo).tool;
}

2. Fragile Configuration Comparison Using JSON.stringify

In the configure() method (line ~312):

if (JSON.stringify(options) === JSON.stringify(this.lastConfiguration || {})) {

This comparison is fragile because:

• Object property order affects the result
• undefined values may serialize differently than omitted properties

Fix: Use a deep equality comparison or compare individual properties.

3. Cache Key Collision Risk

In getKey() (line 149):

if (Array.isArray(options)) {
    return options.map((item) => item.fsPath).join(',');
}

Using , as separator could cause collisions if any path contains commas (rare but possible on some systems).

Fix: Use a safer separator like \0 or hash the paths.

4. Logging Inconsistency

Mixed usage throughout the file:

• this.outputChannel.info/warn/error/debug (instance-level logging)
• traceVerbose, traceLog, traceWarn, traceError (module-level logging)

This makes it harder to control log output and trace issues.

Recommendation: Standardize on one approach, preferring outputChannel methods for user-visible logs and trace* for internal debugging.

5. Duplicate venvFolders Fetching

venvFolders is fetched in two places:

1. getRefreshOptions() (line 195): getPythonSettingAndUntildify<string[]>('venvFolders')
2. getAllExtraSearchPaths() → getCustomVirtualEnvDirsLegacy() (line 336)

This is redundant and could lead to inconsistencies if the logic diverges.

6. No Graceful Shutdown Message

In the disposal of the process (line 236):

if (proc.exitCode === null) {
    proc.kill();
}

The process is killed immediately without sending a shutdown message. If PET supports graceful shutdown, it should be used to allow proper cleanup.

---

Files to Modify

• src/managers/common/nativePythonFinder.ts

---

Related

• Root Cause Analysis: Discovery stuck indefinitely (#1081, #956) #1140 - Critical timeout and error handling issues
• NativePythonFinder: Resource leaks in dispose() - worker pool and disposables not cleaned up #1141 - Resource leak issues