Skip to content

Commit 96105bc

Browse files
jfeingold35jfeingold35
committed
@W-17915877@ Implemented path start points for SFGE
1 parent 6bcbdad commit 96105bc

File tree

10 files changed

+164
-21
lines changed

10 files changed

+164
-21
lines changed

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,9 @@ export class SfgeEngine extends Engine {
3131

3232
public constructor(config: SfgeEngineConfig, clock: Clock = new RealClock()) {
3333
super();
34-
// TODO: When we support custom Java commands, we'll need to use the config property instead of the hardcoded string here.
35-
const javaCommandExecutor: JavaCommandExecutor = new JavaCommandExecutor('java', this.emitLogEvent.bind(this));
36-
this.sfgeWrapper = new RuntimeSfgeWrapper(javaCommandExecutor, clock, this.emitLogEvent.bind(this));
3734
this.config = config;
35+
const javaCommandExecutor: JavaCommandExecutor = new JavaCommandExecutor(this.config.java_command, this.emitLogEvent.bind(this));
36+
this.sfgeWrapper = new RuntimeSfgeWrapper(javaCommandExecutor, clock, this.emitLogEvent.bind(this));
3837
}
3938

4039
public override getName(): string {
@@ -96,7 +95,7 @@ export class SfgeEngine extends Engine {
9695

9796
const sfgeResults: SfgeRunResult[] = await this.sfgeWrapper.invokeRunCommand(
9897
selectedRuleInfoList,
99-
relevantFiles, // TODO: WHEN WE ADD PATH-START TARGETING, THIS NEEDS TO CHANGE.
98+
runOptions.pathStartPoints ?? [],
10099
relevantFiles,
101100
sfgeRunOptions,
102101
(innerPerc: number, message?: string) => this.emitRunRulesProgressEvent(5 + 93*innerPerc/100, message) // 5%-98%

‎packages/code-analyzer-sfge-engine/src/sfge-wrapper.ts

+25-10
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import fs from 'node:fs';
33
import {
44
getMessageFromCatalog,
55
LogLevel,
6+
PathPoint,
67
SHARED_MESSAGE_CATALOG
78
} from '@salesforce/code-analyzer-engine-api';
89
import {createTempDir, JavaCommandExecutor} from '@salesforce/code-analyzer-engine-api/utils';
@@ -108,15 +109,15 @@ export class RuntimeSfgeWrapper {
108109
}
109110
}
110111

111-
public async invokeRunCommand(selectedRuleInfos: SfgeRuleInfo[], targetPaths: string[], projectFilePaths: string[], sfgeRunOptions: SfgeRunOptions, emitProgress: (percComplete: number) => void): Promise<SfgeRunResult[]> {
112+
public async invokeRunCommand(selectedRuleInfos: SfgeRuleInfo[], pathStarts: PathPoint[], projectFilePaths: string[], sfgeRunOptions: SfgeRunOptions, emitProgress: (percComplete: number) => void): Promise<SfgeRunResult[]> {
112113
const tmpDir: string = await this.getTemporaryWorkingDir();
113114
emitProgress(2);
114115

115116
const inputFileName: string = path.join(tmpDir, 'sfgeInput.json');
116117
const logFilePath: string = path.join(sfgeRunOptions.logFolder, this.logFileName);
117118
const ruleNames: string[] = selectedRuleInfos.map(sri => sri.name);
118119

119-
await this.createSfgeInputFile(inputFileName, ruleNames, targetPaths, projectFilePaths);
120+
await this.createSfgeInputFile(inputFileName, ruleNames, pathStarts, projectFilePaths);
120121
const resultsOutputFile: string = path.join(tmpDir, 'resultsFile.json');
121122
this.emitLogEvent(LogLevel.Debug, getMessage('LoggingToFile', 'run', logFilePath));
122123
emitProgress(10);
@@ -163,15 +164,29 @@ export class RuntimeSfgeWrapper {
163164
return this.temporaryWorkingDir;
164165
}
165166

166-
private async createSfgeInputFile(filePath: string, rules: string[], targets: string[], allWorkspaceFiles: string[]): Promise<void> {
167-
const sfgeTargets: SfgeTarget[] = targets.map(target => {
168-
return {
169-
targetFile: target,
170-
targetMethods: []
171-
};
172-
});
167+
private async createSfgeInputFile(filePath: string, rules: string[], pathStarts: PathPoint[], allWorkspaceFiles: string[]): Promise<void> {
168+
const sfgeTargetsByFile: Map<string, SfgeTarget> = new Map();
169+
if (pathStarts.length > 0) {
170+
pathStarts.forEach(pathStart => {
171+
const sfgeTarget: SfgeTarget = sfgeTargetsByFile.get(pathStart.file) ?? {
172+
targetFile: pathStart.file,
173+
targetMethods: []
174+
};
175+
if (pathStart.methodName) {
176+
sfgeTarget.targetMethods = [...sfgeTarget.targetMethods, pathStart.methodName];
177+
}
178+
sfgeTargetsByFile.set(pathStart.file, sfgeTarget);
179+
});
180+
} else {
181+
allWorkspaceFiles.map(file => {
182+
sfgeTargetsByFile.set(file, {
183+
targetFile: file,
184+
targetMethods: []
185+
});
186+
});
187+
}
173188
const inputFileContents: SfgeInputFile = {
174-
targets: sfgeTargets,
189+
targets: [...sfgeTargetsByFile.values()],
175190
projectDirs: allWorkspaceFiles,
176191
rulesToRun: rules
177192
};

‎packages/code-analyzer-sfge-engine/test/engine.test.ts

+48-7
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
EventType,
99
LogEvent,
1010
LogLevel,
11+
PathPoint,
1112
RuleDescription,
1213
RunOptions,
1314
RunRulesProgressEvent,
@@ -182,10 +183,10 @@ describe('SfgeEngine', () => {
182183
path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace', 'SomeOtherClass.cls')
183184
]
184185
}
185-
])('When workspace is $case, those violations are returned', async () => {
186+
])('When workspace is $case, those violations are returned', async ({workspacePaths}) => {
186187
// ====== SETUP ======
187188
const engine: SfgeEngine = new SfgeEngine(DEFAULT_SFGE_ENGINE_CONFIG, fixedClock);
188-
const workspace: Workspace = new Workspace([path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace')]);
189+
const workspace: Workspace = new Workspace(workspacePaths);
189190
const progressEvents: RunRulesProgressEvent[] = [];
190191
engine.onEvent(EventType.RunRulesProgressEvent, (e: RunRulesProgressEvent) => progressEvents.push(e));
191192
const ruleNames: string[] = ['ApexFlsViolationRule', 'UseWithSharingOnDatabaseOperation', 'UnimplementedTypeRule', 'RemoveUnusedMethod'];
@@ -194,7 +195,46 @@ describe('SfgeEngine', () => {
194195
const results: EngineRunResults = await engine.runRules(ruleNames, createRunOptions(workspace));
195196

196197
// ====== ASSERTIONS ======
197-
await expectResultsToMatchGoldfile(results, 'all_sampleRelevantWorkspace_violations.goldfile.json', path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace'));
198+
await expectResultsToMatchGoldfile(results, path.join('sampleRelevantWorkspace', 'all_violations.goldfile.json'), path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace'));
199+
const progressPercents: number[] = progressEvents.map(pe => pe.percentComplete);
200+
expect(progressPercents[0]).toEqual(2);
201+
expect(progressPercents[progressPercents.length - 1]).toEqual(100);
202+
expectProgressEventsToAscend(progressPercents);
203+
});
204+
205+
it.each([
206+
{
207+
case: 'a file that violates the selected rules',
208+
pathStartPoints: [
209+
{
210+
file: path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace2', 'EntryClass1.cls')
211+
}
212+
],
213+
goldfile: 'EntryClass1_violations.goldfile.json'
214+
},
215+
{
216+
case: 'a single method that violates the selected rules',
217+
pathStartPoints: [
218+
{
219+
file: path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace2', 'EntryClass1.cls'),
220+
methodName: 'entryPointMethod1'
221+
}
222+
],
223+
goldfile: 'EntryClass1_entryPointMethod1_violations.goldfile.json'
224+
}
225+
])('When path start point is $case, the appropriate violations are returned', async ({pathStartPoints, goldfile}) => {
226+
// ====== SETUP ======
227+
const engine: SfgeEngine = new SfgeEngine(DEFAULT_SFGE_ENGINE_CONFIG, fixedClock);
228+
const workspace: Workspace = new Workspace([path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace2')]);
229+
const progressEvents: RunRulesProgressEvent[] = [];
230+
engine.onEvent(EventType.RunRulesProgressEvent, (e: RunRulesProgressEvent) => progressEvents.push(e));
231+
const ruleNames: string[] = ['ApexFlsViolationRule', 'UseWithSharingOnDatabaseOperation', 'UnimplementedTypeRule', 'RemoveUnusedMethod'];
232+
233+
// ====== TESTED BEHAVIOR ======
234+
const results: EngineRunResults = await engine.runRules(ruleNames, createRunOptions(workspace, pathStartPoints));
235+
236+
// ====== ASSERTIONS ======
237+
await expectResultsToMatchGoldfile(results, path.join('sampleRelevantWorkspace2', goldfile), path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace2'));
198238
const progressPercents: number[] = progressEvents.map(pe => pe.percentComplete);
199239
expect(progressPercents[0]).toEqual(2);
200240
expect(progressPercents[progressPercents.length - 1]).toEqual(100);
@@ -213,7 +253,7 @@ describe('SfgeEngine', () => {
213253
const results: EngineRunResults = await engine.runRules(ruleNames, createRunOptions(workspace));
214254

215255
// ====== ASSERTIONS ======
216-
await expectResultsToMatchGoldfile(results, 'ApexFlsViolationRule_sampleRelevantWorkspace_violations.goldfile.json', path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace'));
256+
await expectResultsToMatchGoldfile(results, path.join('sampleRelevantWorkspace', 'ApexFlsViolationRule_violations.goldfile.json'), path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace'));
217257
const expectedProgressDescriptors: {percent: number, message?: string}[] = [
218258
{percent: 2, message: undefined},
219259
{percent: 2.3, message: undefined},
@@ -292,7 +332,7 @@ describe('SfgeEngine', () => {
292332
const results: EngineRunResults = await engine.runRules(ruleNames, createRunOptions(workspace));
293333

294334
// ====== ASSERTIONS ======
295-
await expectResultsToMatchGoldfile(results, 'ApexFlsViolationRule_sampleRelevantWorkspace_violations.goldfile.json', path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace'));
335+
await expectResultsToMatchGoldfile(results, path.join('sampleRelevantWorkspace', 'ApexFlsViolationRule_violations.goldfile.json'), path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace'));
296336
const warningLogEvents: LogEvent[] = logEvents.filter(e => e.logLevel === LogLevel.Warn);
297337
expect(warningLogEvents.length).toBeGreaterThanOrEqual(1);
298338
expect(warningLogEvents[0].message).toContain(`Specified workspace is missing 1 possibly-relevant file(s) from the folder ${path.join(TEST_DATA_FOLDER, 'sampleRelevantWorkspace')}.`);
@@ -333,9 +373,10 @@ function createDescribeOptions(workspace?: Workspace): DescribeOptions {
333373
};
334374
}
335375

336-
function createRunOptions(workspace: Workspace): RunOptions {
376+
function createRunOptions(workspace: Workspace, pathStartPoints?: PathPoint[]): RunOptions {
337377
return {
338378
logFolder: os.tmpdir(),
339-
workspace
379+
workspace,
380+
pathStartPoints
340381
};
341382
}

‎packages/code-analyzer-sfge-engine/test/test-data/goldfiles/ApexFlsViolationRule_sampleRelevantWorkspace_violations.goldfile.json ‎packages/code-analyzer-sfge-engine/test/test-data/goldfiles/sampleRelevantWorkspace/ApexFlsViolationRule_violations.goldfile.json

File renamed without changes.

‎packages/code-analyzer-sfge-engine/test/test-data/goldfiles/all_sampleRelevantWorkspace_violations.goldfile.json ‎packages/code-analyzer-sfge-engine/test/test-data/goldfiles/sampleRelevantWorkspace/all_violations.goldfile.json

File renamed without changes.

‎packages/code-analyzer-sfge-engine/test/test-data/goldfiles/sampleRelevantWorkspace2/EntryClass1_entryPointMethod1_violations.goldfile.json

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
{
2+
"violations": [
3+
{
4+
"ruleName": "ApexFlsViolationRule",
5+
"message": "FLS validation is missing for [READ] operation on [Account] with field(s) [CustomField__c,Name].",
6+
"codeLocations": [
7+
{
8+
"file": "{{RUNDIR}}EntryClass1.cls",
9+
"startLine": 2,
10+
"startColumn": 24
11+
},
12+
{
13+
"file": "{{RUNDIR}}InternalOnlyClass.cls",
14+
"startLine": 5,
15+
"startColumn": 26
16+
}
17+
],
18+
"primaryLocationIndex": 1
19+
}
20+
]
21+
}

‎packages/code-analyzer-sfge-engine/test/test-data/goldfiles/sampleRelevantWorkspace2/EntryClass1_violations.goldfile.json

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"violations": [
3+
{
4+
"ruleName": "ApexFlsViolationRule",
5+
"message": "FLS validation is missing for [READ] operation on [Account] with field(s) [CustomField__c,Name].",
6+
"codeLocations": [
7+
{
8+
"file": "{{RUNDIR}}EntryClass1.cls",
9+
"startLine": 2,
10+
"startColumn": 24
11+
},
12+
{
13+
"file": "{{RUNDIR}}InternalOnlyClass.cls",
14+
"startLine": 5,
15+
"startColumn": 26
16+
}
17+
],
18+
"primaryLocationIndex": 1
19+
},
20+
{
21+
"ruleName": "ApexFlsViolationRule",
22+
"message": "FLS validation is missing for [READ] operation on [Account] with field(s) [CustomField__c,Name].",
23+
"codeLocations": [
24+
{
25+
"file": "{{RUNDIR}}EntryClass1.cls",
26+
"startLine": 7,
27+
"startColumn": 24
28+
},
29+
{
30+
"file": "{{RUNDIR}}InternalOnlyClass.cls",
31+
"startLine": 5,
32+
"startColumn": 26
33+
}
34+
],
35+
"primaryLocationIndex": 1
36+
}
37+
]
38+
}

‎packages/code-analyzer-sfge-engine/test/test-data/sampleRelevantWorkspace2/EntryClass1.cls

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
global with sharing class EntryClass1 {
2+
global static void entryPointMethod1() {
3+
InternalOnlyClass i = new InternalOnlyClass();
4+
i.doAnInsecureQuery();
5+
}
6+
7+
global static void entryPointMethod2() {
8+
InternalOnlyClass i = new InternalOnlyClass();
9+
i.doAnInsecureQuery();
10+
}
11+
}

‎packages/code-analyzer-sfge-engine/test/test-data/sampleRelevantWorkspace2/EntryClass2.cls

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
global with sharing class EntryClass2 {
2+
global static void entryPointMethod1() {
3+
InternalOnlyClass i = new InternalOnlyClass();
4+
i.doAnInsecureQuery();
5+
}
6+
7+
global static void entryPointMethod2() {
8+
InternalOnlyClass i = new InternalOnlyClass();
9+
i.doAnInsecureQuery();
10+
}
11+
}

‎packages/code-analyzer-sfge-engine/test/test-data/sampleRelevantWorkspace2/InternalOnlyClass.cls

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
public inherited sharing class InternalOnlyClass {
2+
3+
public void doAnInsecureQuery() {
4+
// This query should be using the USER_MODE syntax for CRUD/FLS reasons. It doesn't, and is therefore insecure.
5+
Account[] accs = [SELECT Name, CustomField__c FROM Account];
6+
}
7+
}

0 commit comments

Comments
 (0)