Skip to content

Commit 204e13f

Browse files
jfeingold35jfeingold35
committed
@W-19772057@ Enhanced rule selector support
1 parent 80f058c commit 204e13f

File tree

6 files changed

+207
-18
lines changed

6 files changed

+207
-18
lines changed

‎packages/code-analyzer-core/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@salesforce/code-analyzer-core",
33
"description": "Core Package for the Salesforce Code Analyzer",
4-
"version": "0.36.0",
4+
"version": "0.37.0-SNAPSHOT",
55
"author": "The Salesforce Code Analyzer Team",
66
"license": "BSD-3-Clause",
77
"homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview",
@@ -72,4 +72,4 @@
7272
"!src/index.ts"
7373
]
7474
}
75-
}
75+
}

‎packages/code-analyzer-core/src/code-analyzer.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
import {getMessage} from "./messages";
2323
import * as engApi from "@salesforce/code-analyzer-engine-api"
2424
import {Clock, RealClock} from '@salesforce/code-analyzer-engine-api/utils';
25+
import {Selector, toSelector} from "./selectors";
2526
import {EventEmitter} from "node:events";
2627
import {CodeAnalyzerConfig, ConfigDescription, EngineOverrides, FIELDS, RuleOverride} from "./config";
2728
import {
@@ -288,11 +289,13 @@ export class CodeAnalyzer {
288289
this.emitEvent({type: EventType.RuleSelectionProgressEvent, timestamp: this.clock.now(), percentComplete: 0});
289290

290291
selectors = selectors.length > 0 ? selectors : [engApi.COMMON_TAGS.RECOMMENDED];
292+
const selectorObjects: Selector[] = selectors.map(toSelector);
293+
291294
const allRules: RuleImpl[] = await this.getAllRules(selectOptions?.workspace);
292295

293296
const ruleSelection: RuleSelectionImpl = new RuleSelectionImpl();
294297
for (const rule of allRules) {
295-
if (selectors.some(s => rule.matchesRuleSelector(s))) {
298+
if (selectorObjects.some(o => rule.matchesRuleSelector(o))) {
296299
ruleSelection.addRule(rule);
297300
}
298301
}

‎packages/code-analyzer-core/src/messages.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ const MESSAGE_CATALOG : MessageCatalog = {
163163
RuleDoesNotExistInSelection:
164164
`No rule with name '%s' and engine '%s' exists among the selected rules.`,
165165

166+
SelectorCannotBeEmpty:
167+
`Rule selectors cannot be empty strings.`,
168+
169+
SelectorLooksIncorrect:
170+
`Rule selector '%s' looks incorrect. Make sure that parentheses are balanced, and that all subselectors are joined by a colon (:) or comma (,).`,
171+
166172
EngineRunResultsMissing:
167173
`Could to get results for engine '%s' since they are missing from the overall run results. Most likely the engine did not run.`,
168174

‎packages/code-analyzer-core/src/rules.ts‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as engApi from "@salesforce/code-analyzer-engine-api";
22
import { getMessage } from "./messages";
3+
import { Selector } from "./selectors";
34
import { OutputFormat, RuleSelectionFormatter } from "./output-format";
45

56
/**
@@ -104,7 +105,7 @@ export class RuleImpl implements Rule {
104105
return this.ruleDesc.tags;
105106
}
106107

107-
matchesRuleSelector(ruleSelector: string): boolean {
108+
matchesRuleSelector(ruleSelector: Selector): boolean {
108109
const sevNumber: number = this.getSeverityLevel().valueOf();
109110
const sevName: string = SeverityLevel[sevNumber];
110111
const selectables: string[] = [
@@ -115,11 +116,7 @@ export class RuleImpl implements Rule {
115116
String(sevNumber),
116117
...this.getTags().map(t => t.toLowerCase())
117118
]
118-
for (const selectorPart of ruleSelector.toLowerCase().split(':')) {
119-
const partMatched: boolean = selectables.some(s => s == selectorPart);
120-
if (!partMatched) return false;
121-
}
122-
return true;
119+
return ruleSelector.matchesSelectables(selectables);
123120
}
124121
}
125122

@@ -218,4 +215,4 @@ export class RuleSelectionImpl implements RuleSelection {
218215
toFormattedOutput(format: OutputFormat): string {
219216
return RuleSelectionFormatter.forFormat(format).format(this);
220217
}
221-
}
218+
}

‎packages/code-analyzer-core/src/selectors.ts‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
import { getMessage } from "./messages";
2+
3+
export interface Selector {
4+
matchesSelectables(selectables: string[]): boolean;
5+
}
6+
7+
export function toSelector(selectorString: string): Selector {
8+
if (selectorString === '') {
9+
throw new Error(getMessage("SelectorCannotBeEmpty"));
10+
} else if (selectorString.endsWith(')')) {
11+
const correspondingOpenParen: number = identifyCorrespondingOpenParen(selectorString);
12+
if (correspondingOpenParen === 0) {
13+
return toSelector(selectorString.slice(1, -1))
14+
} else {
15+
const left: string = selectorString.slice(0, correspondingOpenParen - 1);
16+
const right: string = selectorString.slice(correspondingOpenParen);
17+
const op: string = selectorString[correspondingOpenParen - 1];
18+
return toComplexSelector(left, right, op);
19+
}
20+
} else {
21+
const lastComma: number = selectorString.lastIndexOf(',');
22+
const lastColon: number = selectorString.lastIndexOf(':');
23+
24+
if (lastComma === -1 && lastColon === -1) {
25+
if (selectorString.includes(')') || selectorString.includes('(')) {
26+
throw new Error(getMessage('SelectorLooksIncorrect', selectorString));
27+
}
28+
return new SimpleSelector(selectorString);
29+
} else if (lastComma > lastColon) {
30+
const left: string = selectorString.slice(0, lastComma);
31+
const right: string = selectorString.slice(lastComma + 1);
32+
return toComplexSelector(left, right, ',');
33+
} else {
34+
const left: string = selectorString.slice(0, lastColon);
35+
const right: string = selectorString.slice(lastColon + 1);
36+
return toComplexSelector(left, right, ':');
37+
}
38+
}
39+
}
40+
41+
function identifyCorrespondingOpenParen(selectorString: string): number {
42+
const reversedLetters: string[] = selectorString.split('').reverse();
43+
let parenBalance: number = 0;
44+
let idx = 0;
45+
for (const letter of reversedLetters) {
46+
if (letter === ')') {
47+
parenBalance += 1;
48+
} else if (letter === '(') {
49+
parenBalance -= 1;
50+
}
51+
if (parenBalance === 0) {
52+
break;
53+
}
54+
idx += 1;
55+
}
56+
57+
if (parenBalance > 0) {
58+
throw new Error(getMessage("SelectorLooksIncorrect", selectorString));
59+
}
60+
61+
return selectorString.length - idx - 1;
62+
}
63+
64+
function toComplexSelector(left: string, right: string, op: string): Selector {
65+
if (op === ',') {
66+
return new OrSelector(toSelector(left), toSelector(right));
67+
} else if (op === ':') {
68+
return new AndSelector(toSelector(left), toSelector(right));
69+
} else {
70+
throw new Error(getMessage("SelectorLooksIncorrect", `${left}${op}${right}`));
71+
}
72+
}
73+
74+
class SimpleSelector implements Selector {
75+
private readonly selector: string;
76+
77+
constructor(selector: string) {
78+
this.selector = selector;
79+
}
80+
81+
public matchesSelectables(selectables: string[]): boolean {
82+
return selectables.some(s => s === this.selector.toLowerCase());
83+
}
84+
}
85+
86+
class AndSelector implements Selector {
87+
private readonly left: Selector;
88+
private readonly right: Selector;
89+
90+
constructor(left: Selector, right: Selector) {
91+
this.left = left;
92+
this.right = right;
93+
}
94+
95+
public matchesSelectables(selectables: string[]): boolean {
96+
return this.left.matchesSelectables(selectables) && this.right.matchesSelectables(selectables);
97+
}
98+
}
99+
100+
class OrSelector implements Selector {
101+
private readonly left: Selector;
102+
private readonly right: Selector;
103+
104+
constructor(left: Selector, right: Selector) {
105+
this.left = left;
106+
this.right = right;
107+
}
108+
109+
public matchesSelectables(selectables: string[]): boolean {
110+
return this.left.matchesSelectables(selectables) || this.right.matchesSelectables(selectables);
111+
}
112+
}

‎packages/code-analyzer-core/test/rule-selection.test.ts‎

Lines changed: 79 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -149,12 +149,21 @@ describe('Tests for selecting rules', () => {
149149
expect(ruleNamesFor(selection7,'stubEngine2')).toEqual([])
150150
});
151151

152-
it('When multiple selectors are provided, then they act as a union', async () => {
153-
const selection: RuleSelection = await codeAnalyzer.selectRules([
154-
'Security', // a tag
155-
'stubEngine2', // an engine name
156-
'stub1RuleD' // a rule name
157-
]);
152+
it.each([
153+
{
154+
case: 'multiple selectors are provided',
155+
selectors: [
156+
'Security', // a tag
157+
'stubEngine2', // an engine name
158+
'stub1RuleD' // a rule name
159+
]
160+
},
161+
{
162+
case: 'using a comma to join two rule selectors',
163+
selectors: ['Security,stubEngine2,stub1RuleD']
164+
}
165+
])('When $case, then it acts like a union', async ({selectors}) => {
166+
const selection: RuleSelection = await codeAnalyzer.selectRules(selectors);
158167

159168
expect(selection.getEngineNames()).toEqual(['stubEngine1', 'stubEngine2']);
160169
expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleB', 'stub1RuleD']);
@@ -164,14 +173,76 @@ describe('Tests for selecting rules', () => {
164173
expect(await codeAnalyzer.selectRules(['all', 'Performance', 'DoesNotExist'])).toEqual(await codeAnalyzer.selectRules(['all']));
165174
});
166175

167-
it('When colons are used and multiple selectors are provided then we get correct union and intersection behavior', async () => {
168-
const selection: RuleSelection = await codeAnalyzer.selectRules(['Recommended:Performance', 'stubEngine2:2', 'stubEngine2:DoesNotExist']);
176+
it.each([
177+
{
178+
selector: 'Recommended:Performance,2', // Equivalent to "(Recommended:Performance),2", or "(Recommended,2):(Performance,2)"
179+
engines: ['stubEngine1', 'stubEngine2'],
180+
stubEngine1Rules: ['stub1RuleB', 'stub1RuleC'],
181+
stubEngine2Rules: ['stub2RuleC']
182+
},
183+
{
184+
selector: 'Recommended,3:Performance', // Equivalent to "(Recommended,4):Performance", or "(Recommended:Performance),(4:Performance)"
185+
engines: ['stubEngine1'],
186+
stubEngine1Rules: ['stub1RuleC', 'stub1RuleE'],
187+
stubEngine2Rules: []
188+
}
189+
])('In the absence of parentheses, colons and commas are resolved from left to right. Case: $selector', async ({selector, engines, stubEngine1Rules, stubEngine2Rules}) => {
190+
const selection: RuleSelection = await codeAnalyzer.selectRules([selector]);
191+
192+
expect(selection.getEngineNames()).toEqual(engines);
193+
expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(stubEngine1Rules);
194+
expect(ruleNamesFor(selection, 'stubEngine2')).toEqual(stubEngine2Rules);
195+
});
196+
197+
it.each([
198+
{
199+
case: 'colons are used and multiple selectors are provided',
200+
selectors: ['Recommended:Performance', 'stubEngine2:2', 'stubEngine2:DoesNotExist']
201+
},
202+
{
203+
case: 'colons and commas are nested via parentheses',
204+
selectors: ['(Recommended:Performance),(stubEngine2:2),(stubEngine2:DoesNotExist)']
205+
}
206+
])('When $case, then we get correct union and intersection behavior', async ({selectors}) => {
207+
const selection: RuleSelection = await codeAnalyzer.selectRules(selectors);
169208

170209
expect(selection.getEngineNames()).toEqual(['stubEngine1', 'stubEngine2']);
171210
expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleC']);
172211
expect(ruleNamesFor(selection, 'stubEngine2')).toEqual(['stub2RuleC']);
173212
});
174213

214+
it('Parentheses cannot be empty', async () => {
215+
await expect(codeAnalyzer.selectRules(['()'])).rejects.toThrow('empty');
216+
});
217+
218+
it('Redundant parentheses are accepted', async () => {
219+
const selection: RuleSelection = await codeAnalyzer.selectRules(['((((((((stub1RuleC))))))))']);
220+
221+
expect(selection.getEngineNames()).toEqual(['stubEngine1']);
222+
expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleC']);
223+
})
224+
225+
it.each([
226+
{selector: 'a,b)'},
227+
{selector: '(a,b'},
228+
{selector: '((a,b)'},
229+
{selector: '(a),b)'},
230+
{selector: ')a,b)'},
231+
{selector: 'a,b('}
232+
])('When parentheses are unbalanced, an error is thrown. Case: $selector', async ({selector}) => {
233+
await expect(codeAnalyzer.selectRules([selector])).rejects.toThrow('looks incorrect');
234+
});
235+
236+
it.each([
237+
{selector: '2(a,b)'},
238+
{selector: '(a,b)2'},
239+
{selector: '2(a:b)'},
240+
{selector: '(a:b)2'}
241+
])('When parentheses are not accompanied by valid joiners, an error is thrown. Case: $selector', async ({selector}) => {
242+
await expect(codeAnalyzer.selectRules([selector])).rejects.toThrow('looks incorrect');
243+
});
244+
245+
175246
it('When selecting rules based on severity names instead of severity number, then we correctly return the rules', async () => {
176247
const selection: RuleSelection = await codeAnalyzer.selectRules(['High', 'Recommended:Low']);
177248

0 commit comments

Comments
 (0)