NEW @W-22393694@ - Gracefully skip ApexGuru when user is not authenticated

packages/code-analyzer-apexguru-engine/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@salesforce/code-analyzer-apexguru-engine",
   "description": "ApexGuru Engine Package for the Salesforce Code Analyzer",
-  "version": "0.39.0",
+  "version": "0.40.0-SNAPSHOT",
   "author": "The Salesforce Code Analyzer Team",
   "license": "BSD-3-Clause",
   "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",

packages/code-analyzer-apexguru-engine/src/apexguru-rules.ts

@@ -29,6 +29,26 @@ export const APEXGURU_RULES: RuleDescription[] = [
         resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_dml_in_loop.htm&type=5']
     },
 
+    // =================================================================================================================
+    //   PERFORMANCE RULES - HIGH SEVERITY (PERFORMANCE ONLY - NOT RECOMMENDED)
+    // =================================================================================================================
+
+    {
+        name: 'SoqlInALoopOneHop',
+        severityLevel: SeverityLevel.High,
+        tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
+        description: 'SOQL query reached one method-hop away inside a loop causes performance issues and can hit governor limits',
+        resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_in_loop_one_hop.htm&type=5']
+    },
+
+    {
+        name: 'ExpensiveMethods',
+        severityLevel: SeverityLevel.High,
+        tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
+        description: 'Method accounts for a large share of observed Apex CPU time and is a hotspot for performance work',
+        resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_expensive_methods.htm&type=5']
+    },
+
     // =================================================================================================================
     //   PERFORMANCE RULES - MODERATE SEVERITY (CRITICAL - RECOMMENDED)
     // =================================================================================================================
@@ -109,6 +129,42 @@ export const APEXGURU_RULES: RuleDescription[] = [
         resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_sobject_map_in_for_loop.htm&type=5']
     },
 
+    {
+        name: 'SoqlWithoutPlatformCache',
+        severityLevel: SeverityLevel.Moderate,
+        tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
+        description: 'Frequently executed SOQL query whose results could be served from Platform Cache to reduce database load',
+        resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_without_platform_cache.htm&type=5']
+    },
+
+    // =================================================================================================================
+    //   PERFORMANCE RULES - LOW SEVERITY (PERFORMANCE ONLY - NOT RECOMMENDED)
+    // =================================================================================================================
+
+    {
+        name: 'LimitsGetHeapsizeMethods',
+        severityLevel: SeverityLevel.Low,
+        tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
+        description: 'Frequent Limits.getHeapSize() calls add runtime overhead',
+        resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_limits_getheapsize_methods.htm&type=5']
+    },
+
+    {
+        name: 'ExpensiveStringComparison',
+        severityLevel: SeverityLevel.Low,
+        tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
+        description: 'Inefficient string comparison wastes CPU time',
+        resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_expensive_string_comparison.htm&type=5']
+    },
+
+    {
+        name: 'ExpensiveDebugStatements',
+        severityLevel: SeverityLevel.Low,
+        tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
+        description: 'Expensive System.debug() statements add runtime overhead',
+        resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_expensive_debug_statements.htm&type=5']
+    },
+
     // =================================================================================================================
     //   BEST PRACTICES - LOW SEVERITY (RECOMMENDED)
     // =================================================================================================================
@@ -149,6 +205,14 @@ export const APEXGURU_RULES: RuleDescription[] = [
         resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_with_unused_fields.htm&type=5']
     },
 
+    {
+        name: 'WritingFillerStatements',
+        severityLevel: SeverityLevel.Low,
+        tags: [COMMON_TAGS.CATEGORIES.BEST_PRACTICES, COMMON_TAGS.LANGUAGES.APEX],
+        description: 'Filler statements written to inflate code coverage instead of testing real behavior',
+        resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_test_case_antipattern_filler_statements.htm&type=5']
+    },
+
     // =================================================================================================================
     //   FALLBACK RULE
     // =================================================================================================================

packages/code-analyzer-apexguru-engine/src/engine.ts

@@ -1,5 +1,3 @@
-
-
 import {
     Engine,
     EngineEventEmitter,
@@ -18,6 +16,7 @@ import { ApexGuruEngineConfig, DEFAULT_APEXGURU_ENGINE_CONFIG } from './config';
 import { ENGINE_NAME, APEXGURU_FILE_EXTENSIONS } from './constants';
 import { APEXGURU_RULES, isKnownRule, FALLBACK_RULE_NAME } from './apexguru-rules';
 import * as fs from 'node:fs/promises';
+import { existsSync } from 'node:fs';
 import * as path from 'node:path';
 
 /**
@@ -53,14 +52,6 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine {
     async describeRules(describeOptions: DescribeOptions): Promise<RuleDescription[]> {
         this.emitDescribeRulesProgressEvent(0);
 
-        // The SFAP API endpoint is environment-specific and supplied externally.
-        // When unset, this engine has nowhere to scan against, so it advertises no rules.
-        if (!process.env.SFAP_API_BASE_URL) {
-            this.emitLogEvent(LogLevel.Debug, 'SFAP API base URL not configured. ApexGuru engine is disabled.');
-            this.emitDescribeRulesProgressEvent(100);
-            return [];
-        }
-
         // Check if targeted files contain any Apex files
         if (describeOptions.workspace) {
             const targetedFiles = await describeOptions.workspace.getTargetedFiles();
@@ -97,15 +88,15 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine {
         // Get target org alias/username from config (passed by CLI --target-org flag)
         const targetOrg = this.getTargetOrg();
 
-        // Initialize authentication
+        // Initialize authentication — skip gracefully if user is not authenticated
         try {
             await this.apexGuruService.initialize(targetOrg);
         } catch (error) {
-            const message = error instanceof Error ? error.message : String(error);
-            throw new Error(
-                `Failed to authenticate: ${message}\n` +
-                'Please authenticate with: sf org login web'
-            );
+            const detail = error instanceof Error ? error.message : String(error);
+            this.apexGuruService.cleanup();
+            return this.skipWithError('NO_ORG_CONNECTION',
+                `Failed to authenticate: ${detail}`,
+                "Please authenticate with 'sf org login web' or pass --target-org");
         }
 
         // Get workspace root path
@@ -126,7 +117,15 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine {
             throw new Error('ApexGuru requires a common workspace root, but the targeted files do not share one.');
         }
 
-        const pathsToZip = [workspaceRoot];
+        // Zip only the Apex files the user actually targeted (via --target or the workspace itself).
+        // Entry names are computed relative to workspaceRoot in createWorkspaceZip so project layout is preserved.
+        const pathsToZip = apexFiles;
+
+        // TEMP DIAGNOSTIC: log the exact set of files being zipped and shipped to SFAP.
+        // Pair this with the [apexguru-diag] "Raw SFAP violation paths" log to confirm
+        // that only what we zipped comes back in violations. Remove once verified.
+        this.emitLogEvent(LogLevel.Fine,
+            `[apexguru-diag] Zipping ${pathsToZip.length} file(s) for SFAP: ${JSON.stringify(pathsToZip)}`);
 
         try {
             // Set up progress callback for polling
@@ -135,28 +134,110 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine {
             });
 
             // Scan (creates zip -> submits -> polls -> decodes)
-            const { violations: apexGuruViolations, scanMetadata } = await this.apexGuruService.scanWorkspace(workspaceRoot, pathsToZip);
+            const { violations: apexGuruViolations, scanMetadata, analysisMode } = await this.apexGuruService.scanWorkspace(workspaceRoot, pathsToZip);
+
+            // TEMP DIAGNOSTIC: dump the raw file paths ApexGuru returned so we can see
+            // exactly what SFAP is sending back (inner-class notation? relative? absolute?).
+            // Remove once the "file does not exist" edge case is fully understood.
+            const rawFilePaths = apexGuruViolations.map(av => ({
+                rule: av.rule,
+                file: av.locations[av.primaryLocationIndex]?.file ?? av.locations[0]?.file,
+                startLine: av.locations[av.primaryLocationIndex]?.startLine ?? av.locations[0]?.startLine
+            }));
+            this.emitLogEvent(LogLevel.Fine,
+                `[apexguru-diag] Raw SFAP violation paths (${rawFilePaths.length}): ${JSON.stringify(rawFilePaths)}`);
+
+            // SFAP normalizes the paths it returns — it strips the longest common leading directory
+            // shared by all files in the zip. e.g. if we zip `unpackaged/config/foo.cls` and
+            // `unpackaged/config/bar.cls`, SFAP returns `foo.cls` and `bar.cls`. Reconstruct the
+            // real absolute path by matching each returned path as a suffix of what we actually zipped.
+            const resolveReturnedPath = (returnedFile: string | undefined): string | undefined => {
+                if (!returnedFile) return undefined;
+                const normalized = returnedFile.replace(/\\/g, '/');
+                const match = pathsToZip.find(absPath => {
+                    const absNormalized = absPath.replace(/\\/g, '/');
+                    return absNormalized === normalized || absNormalized.endsWith(`/${normalized}`);
+                });
+                return match;
+            };
 
             // Convert all ApexGuru violations to Code Analyzer format
             const allViolations = apexGuruViolations.map(av => {
-                // SFAP response includes file path in location.file
-                const filePath = av.locations[0]?.file ?? 'unknown';
-                return toViolation(av, filePath, runOptions.includeSuggestions ?? false);
+                const returnedFile = av.locations[av.primaryLocationIndex]?.file ?? av.locations[0]?.file;
+                const resolvedFile = resolveReturnedPath(returnedFile) ?? returnedFile ?? 'unknown';
+                return toViolation(av, resolvedFile, workspaceRoot, runOptions.includeSuggestions ?? false, resolveReturnedPath);
+            });
+
+            // Drop violations whose primary code location points at a non-existent file.
+            // Safety net for edge cases the suffix-match couldn't resolve (e.g. synthetic paths
+            // like inner-class notation `Foo.InnerHelper.cls`) so we never hand Core a bad path.
+            const validViolations = allViolations.filter(v => {
+                const file = v.codeLocations[v.primaryLocationIndex]?.file;
+                if (file && existsSync(file)) {
+                    return true;
+                }
+                this.emitLogEvent(LogLevel.Warn,
+                    `Dropping ${v.ruleName} violation: primary location file does not exist on disk: ${file ?? '(missing)'}`);
+                return false;
             });
 
             // Filter violations to only include selected rules
-            const filteredViolations = allViolations.filter(v => selectedRulesSet.has(v.ruleName));
+            const filteredViolations = validViolations.filter(v => selectedRulesSet.has(v.ruleName));
 
-            // Return insights as scan metadata (workspace-level)
-            const insights: Record<string, unknown> | undefined = scanMetadata ? { scan: scanMetadata } : undefined;
+            // Return insights with status: "completed", analysis mode, and scan metadata
+            const insights: Record<string, unknown> = {
+                status: 'completed',
+                ...(analysisMode ? { analysisMode } : {}),
+                ...(scanMetadata ? { scan: scanMetadata } : {})
+            };
 
             return { violations: filteredViolations, insights };
+        } catch (error) {
+            // Catch API failures (5xx, timeout, connection refused) and unexpected errors
+            const detail = error instanceof Error ? error.message : String(error);
+            if (this.isApiUnavailableError(error)) {
+                return this.skipWithError('API_UNAVAILABLE',
+                    `ApexGuru service is unavailable: ${detail}`,
+                    'The ApexGuru service is temporarily unavailable. Please try again later.');
+            }
+            return this.skipWithError('UNEXPECTED_ERROR',
+                `An unexpected error occurred: ${detail}`,
+                'An unexpected error occurred. Please try again or file a support ticket if the issue persists.');
         } finally {
             // Always cleanup resources
             this.apexGuruService.cleanup();
         }
     }
 
+    /**
+     * Returns a graceful skip result with structured error insights.
+     * Emits a warn-level log event and completes progress before returning.
+     * NOTE: Caller is responsible for cleanup() — do NOT call cleanup here since
+     * this may be invoked from within a try-finally that already handles cleanup.
+     */
+    private skipWithError(code: string, message: string, remediation: string): EngineRunResults {
+        this.emitLogEvent(LogLevel.Warn, `ApexGuru skipped: ${message}`);
+        this.emitRunRulesProgressEvent(100);
+        return {
+            violations: [],
+            insights: {
+                status: 'skipped',
+                error: { code, message, remediation }
+            }
+        };
+    }
+
+    /**
+     * Determines whether an error is an API unavailability issue (network/timeout/5xx).
+     */
+    private isApiUnavailableError(error: unknown): boolean {
+        if (!(error instanceof Error)) return false;
+        const msg = error.message.toLowerCase();
+        const networkIndicators = ['econnrefused', 'etimedout', 'enotfound', 'socket hang up',
+            'connection refused', 'timeout', 'network', '502', '503', '504', '500'];
+        return networkIndicators.some(indicator => msg.includes(indicator));
+    }
+
     /**
      * Check if file is an Apex file based on extension
      */
@@ -187,22 +268,24 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine {
 function toViolation(
     av: ApexGuruViolation,
     filePath: string,
-    includeSuggestions: boolean
+    workspaceRoot: string,
+    includeSuggestions: boolean,
+    resolveReturnedPath: (returnedFile: string | undefined) => string | undefined
 ): Violation {
     // Map unknown rules to fallback to ensure Core validation passes
     const ruleName = isKnownRule(av.rule) ? av.rule : FALLBACK_RULE_NAME;
 
     const violation: Violation = {
         ruleName,
         message: av.message,
-        codeLocations: av.locations.map(loc => normalizeLocation(loc, filePath)),
+        codeLocations: av.locations.map(loc => normalizeLocation(loc, filePath, workspaceRoot, resolveReturnedPath)),
         primaryLocationIndex: av.primaryLocationIndex,
         resourceUrls: av.resources
     };
 
     // Add suggestions if requested and available
     if (includeSuggestions && av.suggestions?.length) {
-        violation.suggestions = av.suggestions.map(suggestion => toSuggestion(suggestion, filePath));
+        violation.suggestions = av.suggestions.map(suggestion => toSuggestion(suggestion, filePath, workspaceRoot, resolveReturnedPath));
     }
 
     return violation;
@@ -212,9 +295,14 @@ function toViolation(
  * Convert ApexGuru suggestion to Code Analyzer Suggestion format
  * Note: suggestion.message contains "// explanation\ncode" - we keep it as-is
  */
-function toSuggestion(apexGuruSuggestion: ApexGuruSuggestion, filePath: string): Suggestion {
+function toSuggestion(
+    apexGuruSuggestion: ApexGuruSuggestion,
+    filePath: string,
+    workspaceRoot: string,
+    resolveReturnedPath: (returnedFile: string | undefined) => string | undefined
+): Suggestion {
     return {
-        location: normalizeLocation(apexGuruSuggestion.location, filePath),
+        location: normalizeLocation(apexGuruSuggestion.location, filePath, workspaceRoot, resolveReturnedPath),
         message: apexGuruSuggestion.message  // Keep "// explanation\ncode" as-is
     };
 }
@@ -223,21 +311,31 @@ function toSuggestion(apexGuruSuggestion: ApexGuruSuggestion, filePath: string):
  * Normalize location by filling in required fields
  *
  * SFAP ApexGuru API provides:
- * - file (from SFAP response, workspace-relative path)
+ * - file (from SFAP response, path normalized against zip contents — may have common leading dirs stripped)
  * - startLine (required)
  * - comment (optional)
  *
  * We fill in:
  * - startColumn = 1 (required by Code Analyzer, reasonable default if not provided)
- * - Use file from location if provided, else use filePath parameter
+ * - Prefer the fallback filePath (already resolved via suffix-match against zipped files);
+ *   if the location's own file matches a zipped path by suffix, use that instead
+ * - Resolve any remaining workspace-relative paths against workspaceRoot
  * - endLine/endColumn are left undefined (optional fields)
  */
-function normalizeLocation(location: ApexGuruLocation, filePath: string): CodeLocation {
+function normalizeLocation(
+    location: ApexGuruLocation,
+    filePath: string,
+    workspaceRoot: string,
+    resolveReturnedPath: (returnedFile: string | undefined) => string | undefined
+): CodeLocation {
     const startLine = location.startLine ?? 1;
     const startColumn = location.startColumn ?? 1;  // Default to column 1 if not provided
+    const locationResolved = resolveReturnedPath(location.file);
+    const rawFile = locationResolved ?? location.file ?? filePath;
+    const resolvedFile = path.isAbsolute(rawFile) ? rawFile : path.resolve(workspaceRoot, rawFile);
 
     return {
-        file: location.file ?? filePath,  // SFAP includes file path in response
+        file: resolvedFile,
         startLine,
         startColumn,
         endLine: location.endLine,      // undefined if not provided (optional)