Skip to content

Commit cd3feac

Browse files
nikhil-mittal-165nikhil-mittal-165
committed
optimize logging and test
1 parent 31753b7 commit cd3feac

9 files changed

Lines changed: 1163 additions & 247 deletions

File tree

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

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { RuleDescription, SeverityLevel, COMMON_TAGS } from '@salesforce/code-an
1010
*/
1111
export const APEXGURU_RULES: RuleDescription[] = [
1212
// =================================================================================================================
13-
// PERFORMANCE RULES - HIGH SEVERITY
13+
// PERFORMANCE RULES - HIGH SEVERITY (CRITICAL - RECOMMENDED)
1414
// =================================================================================================================
1515

1616
{
@@ -30,7 +30,7 @@ export const APEXGURU_RULES: RuleDescription[] = [
3030
},
3131

3232
// =================================================================================================================
33-
// PERFORMANCE RULES - MODERATE SEVERITY
33+
// PERFORMANCE RULES - MODERATE SEVERITY (CRITICAL - RECOMMENDED)
3434
// =================================================================================================================
3535

3636
{
@@ -50,97 +50,105 @@ export const APEXGURU_RULES: RuleDescription[] = [
5050
},
5151

5252
{
53-
name: 'Soql Aggregation',
53+
name: 'SchemaGetGlobalDescribeNotEfficient',
5454
severityLevel: SeverityLevel.Moderate,
5555
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
56+
description: 'Using Schema.getGlobalDescribe() causes unnecessary overhead and decreases performance',
57+
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_schema_getglobaldescribe_not_efficient.htm&type=5']
58+
},
59+
60+
// =================================================================================================================
61+
// PERFORMANCE RULES - MODERATE SEVERITY (PERFORMANCE ONLY - NOT RECOMMENDED)
62+
// =================================================================================================================
63+
64+
{
65+
name: 'Soql Aggregation',
66+
severityLevel: SeverityLevel.Moderate,
67+
tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
5668
description: 'Manual aggregation in Apex instead of using SOQL aggregate functions causes performance issues',
5769
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_aggregating_in_apex.htm&type=5']
5870
},
5971

6072
{
6173
name: 'SoqlWithApexFilter',
6274
severityLevel: SeverityLevel.Moderate,
63-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
75+
tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
6476
description: 'Filtering records in Apex instead of using SOQL WHERE clause causes performance issues',
6577
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_with_apex_filter.htm&type=5']
6678
},
6779

6880
{
6981
name: 'CopyingListOrSetElementsUsingAForLoop',
7082
severityLevel: SeverityLevel.Moderate,
71-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
83+
tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
7284
description: 'Copying list or set elements using a for loop is inefficient - use addAll() instead',
7385
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_copying_elements_with_for_loop.htm&type=5']
7486
},
7587

7688
{
7789
name: 'Redundant Soql',
7890
severityLevel: SeverityLevel.Moderate,
79-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
91+
tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
8092
description: 'Multiple identical SOQL queries cause unnecessary database round trips',
8193
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_redundant_soql.htm&type=5']
8294
},
8395

84-
{
85-
name: 'SchemaGetGlobalDescribeNotEfficient',
86-
severityLevel: SeverityLevel.Moderate,
87-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
88-
description: 'Using Schema.getGlobalDescribe() causes unnecessary overhead and decreases performance',
89-
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_schema_getglobaldescribe_not_efficient.htm&type=5']
90-
},
91-
9296
{
9397
name: 'SoqlWithNegativeExpressions',
9498
severityLevel: SeverityLevel.Moderate,
95-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
99+
tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
96100
description: 'SOQL queries using negative expressions (NOT IN, !=) don\'t use indexes and cause full table scans',
97101
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_with_negative_expressions.htm&type=5']
98102
},
99103

100104
{
101105
name: 'SObjectMapInAForLoop',
102106
severityLevel: SeverityLevel.Moderate,
103-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
107+
tags: [COMMON_TAGS.CATEGORIES.PERFORMANCE, COMMON_TAGS.LANGUAGES.APEX],
104108
description: 'Building Map<Id, SObject> using .put() in a for loop is inefficient - use map constructor or putAll()',
105109
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_sobject_map_in_for_loop.htm&type=5']
106110
},
107111

108112
// =================================================================================================================
109-
// BEST PRACTICES - LOW SEVERITY
113+
// BEST PRACTICES - LOW SEVERITY (RECOMMENDED)
110114
// =================================================================================================================
111115

112116
{
113-
name: 'SortingInApex',
117+
name: 'UsingTheTestMethodKeyword',
114118
severityLevel: SeverityLevel.Low,
115119
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.BEST_PRACTICES, COMMON_TAGS.LANGUAGES.APEX],
120+
description: 'The testMethod keyword is deprecated - use @isTest annotation instead',
121+
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_test_case_antipattern_using_testmethod.htm&type=5']
122+
},
123+
124+
// =================================================================================================================
125+
// BEST PRACTICES - LOW SEVERITY (NOT RECOMMENDED)
126+
// =================================================================================================================
127+
128+
{
129+
name: 'SortingInApex',
130+
severityLevel: SeverityLevel.Low,
131+
tags: [COMMON_TAGS.CATEGORIES.BEST_PRACTICES, COMMON_TAGS.LANGUAGES.APEX],
116132
description: 'Sorting records in Apex wastes CPU time and can exceed governor limits - use ORDER BY in SOQL',
117133
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_sorting_in_apex.htm&type=5']
118134
},
119135

120136
{
121137
name: 'BusyLoopDelay',
122138
severityLevel: SeverityLevel.Low,
123-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.BEST_PRACTICES, COMMON_TAGS.LANGUAGES.APEX],
139+
tags: [COMMON_TAGS.CATEGORIES.BEST_PRACTICES, COMMON_TAGS.LANGUAGES.APEX],
124140
description: 'Using empty loops to delay execution wastes CPU time - use System.enqueueJob with delay parameter',
125141
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_busy_loop_delay.htm&type=5']
126142
},
127143

128144
{
129145
name: 'SoqlWithUnusedFields',
130146
severityLevel: SeverityLevel.Low,
131-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.BEST_PRACTICES, COMMON_TAGS.LANGUAGES.APEX],
147+
tags: [COMMON_TAGS.CATEGORIES.BEST_PRACTICES, COMMON_TAGS.LANGUAGES.APEX],
132148
description: 'SOQL query selecting unused fields increases resource consumption unnecessarily',
133149
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_antipattern_soql_with_unused_fields.htm&type=5']
134150
},
135151

136-
{
137-
name: 'UsingTheTestMethodKeyword',
138-
severityLevel: SeverityLevel.Low,
139-
tags: [COMMON_TAGS.RECOMMENDED, COMMON_TAGS.CATEGORIES.BEST_PRACTICES, COMMON_TAGS.LANGUAGES.APEX],
140-
description: 'The testMethod keyword is deprecated - use @isTest annotation instead',
141-
resourceUrls: ['https://help.salesforce.com/s/articleView?id=xcloud.apexguru_test_case_antipattern_using_testmethod.htm&type=5']
142-
},
143-
144152
// =================================================================================================================
145153
// FALLBACK RULE
146154
// =================================================================================================================

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

Lines changed: 66 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine {
5353

5454
async describeRules(describeOptions: DescribeOptions): Promise<RuleDescription[]> {
5555
this.emitDescribeRulesProgressEvent(0);
56-
this.emitLogEvent(LogLevel.Fine, 'Returning known ApexGuru rules plus fallback for new rules');
5756

5857
// ApexGuru is dynamic - new rules can be added by Salesforce at any time.
5958
// We declare known rules explicitly (in apexguru-rules.ts), plus a fallback rule.
@@ -64,11 +63,12 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine {
6463
}
6564

6665
async runRules(ruleNames: string[], runOptions: RunOptions): Promise<EngineRunResults> {
67-
// Note: ruleNames parameter is ignored. ApexGuru API analyzes code and returns
68-
// all detected violations. Individual rules cannot be enabled/disabled.
69-
// This is by design - ApexGuru determines which rules to apply dynamically.
66+
// Note: ApexGuru API analyzes code and returns ALL detected violations.
67+
// Individual rules cannot be enabled/disabled via the API.
68+
// We filter violations to match the selected rules after analysis completes.
7069

71-
this.emitLogEvent(LogLevel.Info, 'Starting ApexGuru analysis...');
70+
// Create a Set for faster rule name lookup
71+
const selectedRulesSet = new Set(ruleNames);
7272

7373
// Extract targetOrg from workspace (if available)
7474
const targetOrg = this.getTargetOrgFromWorkspace(runOptions);
@@ -98,63 +98,72 @@ export class ApexGuruEngine extends EngineEventEmitter implements Engine {
9898

9999
if (apexFiles.length === 0) {
100100
this.emitLogEvent(LogLevel.Warn, 'No Apex class files found to analyze');
101+
this.apexGuruService.cleanup(); // Cleanup even on early return
101102
return { violations: [] };
102103
}
103104

104-
this.emitLogEvent(LogLevel.Info, `Found ${apexFiles.length} Apex class(es) to analyze`);
105-
106-
// Analyze each file
107-
const allViolations: Violation[] = [];
108-
let filesProcessed = 0;
109-
110-
for (let i = 0; i < apexFiles.length; i++) {
111-
const filePath = apexFiles[i];
112-
113-
try {
114-
this.emitLogEvent(LogLevel.Fine, `Analyzing: ${filePath}`);
115-
116-
// Emit progress at start of file
117-
const baseProgress = (filesProcessed / apexFiles.length) * 100;
118-
this.emitRunRulesProgressEvent(baseProgress);
119-
120-
// Set up progress callback for polling
121-
// Each file gets a slice of the total progress (0-95% of that slice during polling)
122-
const progressSlicePerFile = 100 / apexFiles.length;
123-
this.apexGuruService.setProgressCallback((pollingProgress: number) => {
124-
// Map polling progress (0-95) to this file's slice
125-
const fileProgress = baseProgress + (pollingProgress / 100) * progressSlicePerFile;
126-
this.emitRunRulesProgressEvent(fileProgress);
127-
});
128-
129-
const fileContent = await fs.readFile(filePath, 'utf-8');
130-
const apexGuruViolations: ApexGuruViolation[] = await this.apexGuruService.analyzeApexClass(
131-
fileContent,
132-
filePath
133-
);
134-
135-
const violations = this.violationMapper.mapViolations(apexGuruViolations, filePath);
136-
allViolations.push(...violations);
137-
138-
filesProcessed++;
139-
const endProgress = (filesProcessed / apexFiles.length) * 100;
140-
this.emitRunRulesProgressEvent(endProgress);
141-
142-
this.emitLogEvent(
143-
LogLevel.Fine,
144-
`Found ${violations.length} violation(s) in ${path.basename(filePath)}`
145-
);
146-
} catch (error: any) {
147-
this.emitLogEvent(
148-
LogLevel.Warn,
149-
`Failed to analyze ${path.basename(filePath)}: ${error.message}`
150-
);
151-
// Continue with other files
105+
try {
106+
// Analyze each file
107+
const allViolations: Violation[] = [];
108+
let filesProcessed = 0;
109+
110+
for (let i = 0; i < apexFiles.length; i++) {
111+
const filePath = apexFiles[i];
112+
113+
try {
114+
// Emit progress at start of file
115+
const baseProgress = (filesProcessed / apexFiles.length) * 100;
116+
this.emitRunRulesProgressEvent(baseProgress);
117+
118+
// Set up progress callback for polling
119+
// Each file gets a slice of the total progress (0-95% of that slice during polling)
120+
const progressSlicePerFile = 100 / apexFiles.length;
121+
this.apexGuruService.setProgressCallback((pollingProgress: number) => {
122+
// Map polling progress (0-95) to this file's slice
123+
const fileProgress = baseProgress + (pollingProgress / 100) * progressSlicePerFile;
124+
this.emitRunRulesProgressEvent(fileProgress);
125+
});
126+
127+
const fileContent = await fs.readFile(filePath, 'utf-8');
128+
const apexGuruViolations: ApexGuruViolation[] = await this.apexGuruService.analyzeApexClass(
129+
fileContent,
130+
filePath
131+
);
132+
133+
const violations = this.violationMapper.mapViolations(
134+
apexGuruViolations,
135+
filePath,
136+
runOptions.includeSuggestions ?? false
137+
);
138+
139+
// Filter violations to only include selected rules
140+
const filteredViolations = violations.filter(v => selectedRulesSet.has(v.ruleName));
141+
allViolations.push(...filteredViolations);
142+
143+
if (violations.length !== filteredViolations.length) {
144+
this.emitLogEvent(
145+
LogLevel.Fine,
146+
`Filtered ${violations.length - filteredViolations.length} violation(s) for unselected rules`
147+
);
148+
}
149+
150+
filesProcessed++;
151+
const endProgress = (filesProcessed / apexFiles.length) * 100;
152+
this.emitRunRulesProgressEvent(endProgress);
153+
} catch (error: any) {
154+
this.emitLogEvent(
155+
LogLevel.Warn,
156+
`Failed to analyze ${path.basename(filePath)}: ${error.message}`
157+
);
158+
// Continue with other files
159+
}
152160
}
153-
}
154161

155-
this.emitLogEvent(LogLevel.Info, `ApexGuru analysis complete. Total violations: ${allViolations.length}`);
156-
157-
return { violations: allViolations };
162+
return { violations: allViolations };
163+
} finally {
164+
// Always cleanup resources to allow process to exit
165+
this.apexGuruService.cleanup();
166+
}
158167
}
159168

160169
/**

‎packages/code-analyzer-apexguru-engine/src/mappers/ViolationMapper.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ export class ViolationMapper {
1515
/**
1616
* Map ApexGuru violations to Code Analyzer violations
1717
*/
18-
mapViolations(apexGuruViolations: ApexGuruViolation[], filePath: string): Violation[] {
19-
return apexGuruViolations.map(av => this.mapSingleViolation(av, filePath));
18+
mapViolations(apexGuruViolations: ApexGuruViolation[], filePath: string, includeSuggestions: boolean): Violation[] {
19+
return apexGuruViolations.map(av => this.mapSingleViolation(av, filePath, includeSuggestions));
2020
}
2121

2222
/**
2323
* Map a single ApexGuru violation
2424
*
2525
* If the rule is unknown (not declared in describeRules), map it to the fallback rule.
2626
*/
27-
private mapSingleViolation(av: ApexGuruViolation, filePath: string): Violation {
27+
private mapSingleViolation(av: ApexGuruViolation, filePath: string, includeSuggestions: boolean): Violation {
2828
// Map unknown rules to fallback to ensure Core validation passes
2929
const ruleName = isKnownRule(av.rule) ? av.rule : FALLBACK_RULE_NAME;
3030

@@ -35,7 +35,9 @@ export class ViolationMapper {
3535
primaryLocationIndex: av.primaryLocationIndex,
3636
resourceUrls: av.resources,
3737
//fixes: av.fixes?.map(fix => this.mapFix(fix, filePath)),
38-
suggestions: av.suggestions?.map(suggestion => this.mapSuggestion(suggestion, filePath))
38+
suggestions: includeSuggestions && av.suggestions?.length ?
39+
av.suggestions.map(suggestion => this.mapSuggestion(suggestion, filePath)) :
40+
undefined
3941
};
4042
}
4143

0 commit comments

Comments
 (0)