Skip to content

Commit 0eef55c

Browse files
jfeingold35jfeingold35
authored
NEW @W-19772057@ Enhanced rule selector support (#363)
1 parent ab7da45 commit 0eef55c

File tree

6 files changed

+235
-19
lines changed

6 files changed

+235
-19
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: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,14 @@ 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 can't 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:
167-
`Could to get results for engine '%s' since they are missing from the overall run results. Most likely the engine did not run.`,
173+
`Couldn't get results for engine '%s' since they're missing from the overall run results. Most likely the engine didn't run.`,
168174

169175
EngineReturnedMultipleRulesWithSameName:
170176
`Engine failure. The engine '%s' returned more than one rule with the name '%s'.`,

‎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: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
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+
// We parse the selector back-to-front, so that the front-most selectors end up at the bottom of the tree we create
9+
// and therefore get resolved first.
10+
if (selectorString === '') {
11+
// ERROR CASE: The selector is empty. Possible if you do something like "()" or "a:()".
12+
throw new Error(getMessage("SelectorCannotBeEmpty"));
13+
} else if (selectorString.endsWith(')')) {
14+
// If the selector ends in close-paren, then we need to find the open-paren that matches it.
15+
const correspondingOpenParen: number = identifyCorrespondingOpenParen(selectorString);
16+
if (correspondingOpenParen === 0) {
17+
// RECURSIVE CASE: The entire selector is wrapped in parens. Pop them off and call recursively.
18+
return toSelector(selectorString.slice(1, -1))
19+
} else {
20+
// RECURSIVE CASE: The open-paren is somewhere in the middle of the selector and accompanied by an operator.
21+
const left: string = selectorString.slice(0, correspondingOpenParen - 1);
22+
const right: string = selectorString.slice(correspondingOpenParen);
23+
const op: string = selectorString[correspondingOpenParen - 1];
24+
return toComplexSelector(left, right, op);
25+
}
26+
} else {
27+
const lastComma: number = selectorString.lastIndexOf(',');
28+
const lastColon: number = selectorString.lastIndexOf(':');
29+
30+
// BASE CASE: The selector contains no commas or colons.
31+
if (lastComma === -1 && lastColon === -1) {
32+
// Parens only make sense in conjunction with operators, so if we find any, the selector is malformed.
33+
if (selectorString.includes(')') || selectorString.includes('(')) {
34+
throw new Error(getMessage('SelectorLooksIncorrect', selectorString));
35+
}
36+
return new SimpleSelector(selectorString);
37+
} else if (lastComma !== -1) {
38+
// Commas resolve before colons, so that "x,a:b" and "a:b,x" both resolve equivalently the combination of
39+
// "x" and "a:b".
40+
const left: string = selectorString.slice(0, lastComma);
41+
const right: string = selectorString.slice(lastComma + 1);
42+
return toComplexSelector(left, right, ',');
43+
} else {
44+
const left: string = selectorString.slice(0, lastColon);
45+
const right: string = selectorString.slice(lastColon + 1);
46+
return toComplexSelector(left, right, ':');
47+
}
48+
}
49+
}
50+
51+
function identifyCorrespondingOpenParen(selectorString: string): number {
52+
const reversedLetters: string[] = selectorString.split('').reverse();
53+
let parenBalance: number = 0;
54+
let idx = 0;
55+
for (const letter of reversedLetters) {
56+
if (letter === ')') {
57+
parenBalance += 1;
58+
} else if (letter === '(') {
59+
parenBalance -= 1;
60+
}
61+
if (parenBalance === 0) {
62+
break;
63+
}
64+
idx += 1;
65+
}
66+
67+
if (parenBalance > 0) {
68+
throw new Error(getMessage("SelectorLooksIncorrect", selectorString));
69+
}
70+
71+
return selectorString.length - idx - 1;
72+
}
73+
74+
function toComplexSelector(left: string, right: string, op: string): Selector {
75+
if (op === ',') {
76+
return new OrSelector(toSelector(left), toSelector(right));
77+
} else if (op === ':') {
78+
return new AndSelector(toSelector(left), toSelector(right));
79+
} else {
80+
throw new Error(getMessage("SelectorLooksIncorrect", `${left}${op}${right}`));
81+
}
82+
}
83+
84+
class SimpleSelector implements Selector {
85+
private readonly selector: string;
86+
87+
constructor(selector: string) {
88+
this.selector = selector;
89+
}
90+
91+
public matchesSelectables(selectables: string[]): boolean {
92+
return selectables.some(s => s === this.selector.toLowerCase());
93+
}
94+
}
95+
96+
class AndSelector implements Selector {
97+
private readonly left: Selector;
98+
private readonly right: Selector;
99+
100+
constructor(left: Selector, right: Selector) {
101+
this.left = left;
102+
this.right = right;
103+
}
104+
105+
public matchesSelectables(selectables: string[]): boolean {
106+
return this.left.matchesSelectables(selectables) && this.right.matchesSelectables(selectables);
107+
}
108+
}
109+
110+
class OrSelector implements Selector {
111+
private readonly left: Selector;
112+
private readonly right: Selector;
113+
114+
constructor(left: Selector, right: Selector) {
115+
this.left = left;
116+
this.right = right;
117+
}
118+
119+
public matchesSelectables(selectables: string[]): boolean {
120+
return this.left.matchesSelectables(selectables) || this.right.matchesSelectables(selectables);
121+
}
122+
}

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

Lines changed: 96 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,93 @@ 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"
179+
engines: ['stubEngine1', 'stubEngine2'],
180+
stubEngine1Rules: ['stub1RuleB', 'stub1RuleC'],
181+
stubEngine2Rules: ['stub2RuleC'],
182+
stubEngine3Rules: []
183+
},
184+
{
185+
selector: '2,Recommended:Performance', // Equivalent to "2,(Recommended:Performance),2"
186+
engines: ['stubEngine1', 'stubEngine2'],
187+
stubEngine1Rules: ['stub1RuleB', 'stub1RuleC'],
188+
stubEngine2Rules: ['stub2RuleC'],
189+
stubEngine3Rules: []
190+
},
191+
{
192+
selector: 'Recommended,3:Performance', // Equivalent to "Recommended,(3:Performance)"
193+
engines: ['stubEngine1', 'stubEngine2', 'stubEngine3'],
194+
stubEngine1Rules: ['stub1RuleA', 'stub1RuleB', 'stub1RuleC', 'stub1RuleE'],
195+
stubEngine2Rules: ['stub2RuleA', 'stub2RuleC'],
196+
stubEngine3Rules: ['stub3RuleA']
197+
},
198+
{
199+
selector: '3:Performance,Recommended', // Equivalent to "(3:Performance),Recommended"
200+
engines: ['stubEngine1', 'stubEngine2', 'stubEngine3'],
201+
stubEngine1Rules: ['stub1RuleA', 'stub1RuleB', 'stub1RuleC', 'stub1RuleE'],
202+
stubEngine2Rules: ['stub2RuleA', 'stub2RuleC'],
203+
stubEngine3Rules: ['stub3RuleA']
204+
}
205+
])('In the absence of parenthesis-defined ordering, commas are applied after colons. Case: $selector', async ({selector, engines, stubEngine1Rules, stubEngine2Rules, stubEngine3Rules}) => {
206+
const selection: RuleSelection = await codeAnalyzer.selectRules([selector]);
207+
208+
expect(selection.getEngineNames()).toEqual(engines);
209+
expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(stubEngine1Rules);
210+
expect(ruleNamesFor(selection, 'stubEngine2')).toEqual(stubEngine2Rules);
211+
expect(ruleNamesFor(selection, 'stubEngine3')).toEqual(stubEngine3Rules);
212+
});
213+
214+
it.each([
215+
{
216+
case: 'colons are used and multiple selectors are provided',
217+
selectors: ['Recommended:Performance', 'stubEngine2:2', 'stubEngine2:DoesNotExist']
218+
},
219+
{
220+
case: 'colons and commas are nested via parentheses',
221+
selectors: ['(Recommended:Performance),(stubEngine2:2),(stubEngine2:DoesNotExist)']
222+
}
223+
])('When $case, then we get correct union and intersection behavior', async ({selectors}) => {
224+
const selection: RuleSelection = await codeAnalyzer.selectRules(selectors);
169225

170226
expect(selection.getEngineNames()).toEqual(['stubEngine1', 'stubEngine2']);
171227
expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleC']);
172228
expect(ruleNamesFor(selection, 'stubEngine2')).toEqual(['stub2RuleC']);
173229
});
174230

231+
it('Parentheses cannot be empty', async () => {
232+
await expect(codeAnalyzer.selectRules(['()'])).rejects.toThrow('empty');
233+
});
234+
235+
it('Redundant parentheses are accepted', async () => {
236+
const selection: RuleSelection = await codeAnalyzer.selectRules(['((((((((stub1RuleC))))))))']);
237+
238+
expect(selection.getEngineNames()).toEqual(['stubEngine1']);
239+
expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleC']);
240+
})
241+
242+
it.each([
243+
{selector: 'a,b)'},
244+
{selector: '(a,b'},
245+
{selector: '((a,b)'},
246+
{selector: '(a),b)'},
247+
{selector: ')a,b)'},
248+
{selector: 'a,b('}
249+
])('When parentheses are unbalanced, an error is thrown. Case: $selector', async ({selector}) => {
250+
await expect(codeAnalyzer.selectRules([selector])).rejects.toThrow('looks incorrect');
251+
});
252+
253+
it.each([
254+
{selector: '2(a,b)'},
255+
{selector: '(a,b)2'},
256+
{selector: '2(a:b)'},
257+
{selector: '(a:b)2'}
258+
])('When parentheses are not accompanied by valid joiners, an error is thrown. Case: $selector', async ({selector}) => {
259+
await expect(codeAnalyzer.selectRules([selector])).rejects.toThrow('looks incorrect');
260+
});
261+
262+
175263
it('When selecting rules based on severity names instead of severity number, then we correctly return the rules', async () => {
176264
const selection: RuleSelection = await codeAnalyzer.selectRules(['High', 'Recommended:Low']);
177265

0 commit comments

Comments
 (0)