diff --git a/packages/code-analyzer-core/package.json b/packages/code-analyzer-core/package.json index df1f0f03..a5902d16 100644 --- a/packages/code-analyzer-core/package.json +++ b/packages/code-analyzer-core/package.json @@ -1,7 +1,7 @@ { "name": "@salesforce/code-analyzer-core", "description": "Core Package for the Salesforce Code Analyzer", - "version": "0.36.0", + "version": "0.37.0-SNAPSHOT", "author": "The Salesforce Code Analyzer Team", "license": "BSD-3-Clause", "homepage": "https://developer.salesforce.com/docs/platform/salesforce-code-analyzer/overview", @@ -72,4 +72,4 @@ "!src/index.ts" ] } -} \ No newline at end of file +} diff --git a/packages/code-analyzer-core/src/code-analyzer.ts b/packages/code-analyzer-core/src/code-analyzer.ts index 10421f91..1268d68c 100644 --- a/packages/code-analyzer-core/src/code-analyzer.ts +++ b/packages/code-analyzer-core/src/code-analyzer.ts @@ -22,6 +22,7 @@ import { import {getMessage} from "./messages"; import * as engApi from "@salesforce/code-analyzer-engine-api" import {Clock, RealClock} from '@salesforce/code-analyzer-engine-api/utils'; +import {Selector, toSelector} from "./selectors"; import {EventEmitter} from "node:events"; import {CodeAnalyzerConfig, ConfigDescription, EngineOverrides, FIELDS, RuleOverride} from "./config"; import { @@ -288,11 +289,13 @@ export class CodeAnalyzer { this.emitEvent({type: EventType.RuleSelectionProgressEvent, timestamp: this.clock.now(), percentComplete: 0}); selectors = selectors.length > 0 ? selectors : [engApi.COMMON_TAGS.RECOMMENDED]; + const selectorObjects: Selector[] = selectors.map(toSelector); + const allRules: RuleImpl[] = await this.getAllRules(selectOptions?.workspace); const ruleSelection: RuleSelectionImpl = new RuleSelectionImpl(); for (const rule of allRules) { - if (selectors.some(s => rule.matchesRuleSelector(s))) { + if (selectorObjects.some(o => rule.matchesRuleSelector(o))) { ruleSelection.addRule(rule); } } diff --git a/packages/code-analyzer-core/src/messages.ts b/packages/code-analyzer-core/src/messages.ts index 47089666..8984345c 100644 --- a/packages/code-analyzer-core/src/messages.ts +++ b/packages/code-analyzer-core/src/messages.ts @@ -163,8 +163,14 @@ const MESSAGE_CATALOG : MessageCatalog = { RuleDoesNotExistInSelection: `No rule with name '%s' and engine '%s' exists among the selected rules.`, + SelectorCannotBeEmpty: + `Rule selectors can't be empty strings.`, + + SelectorLooksIncorrect: + `Rule selector '%s' looks incorrect. Make sure that parentheses are balanced and that all subselectors are joined by a colon (:) or comma (,).`, + EngineRunResultsMissing: - `Could to get results for engine '%s' since they are missing from the overall run results. Most likely the engine did not run.`, + `Couldn't get results for engine '%s' since they're missing from the overall run results. Most likely the engine didn't run.`, EngineReturnedMultipleRulesWithSameName: `Engine failure. The engine '%s' returned more than one rule with the name '%s'.`, diff --git a/packages/code-analyzer-core/src/rules.ts b/packages/code-analyzer-core/src/rules.ts index c08de75a..85444b40 100644 --- a/packages/code-analyzer-core/src/rules.ts +++ b/packages/code-analyzer-core/src/rules.ts @@ -1,5 +1,6 @@ import * as engApi from "@salesforce/code-analyzer-engine-api"; import { getMessage } from "./messages"; +import { Selector } from "./selectors"; import { OutputFormat, RuleSelectionFormatter } from "./output-format"; /** @@ -104,7 +105,7 @@ export class RuleImpl implements Rule { return this.ruleDesc.tags; } - matchesRuleSelector(ruleSelector: string): boolean { + matchesRuleSelector(ruleSelector: Selector): boolean { const sevNumber: number = this.getSeverityLevel().valueOf(); const sevName: string = SeverityLevel[sevNumber]; const selectables: string[] = [ @@ -115,11 +116,7 @@ export class RuleImpl implements Rule { String(sevNumber), ...this.getTags().map(t => t.toLowerCase()) ] - for (const selectorPart of ruleSelector.toLowerCase().split(':')) { - const partMatched: boolean = selectables.some(s => s == selectorPart); - if (!partMatched) return false; - } - return true; + return ruleSelector.matchesSelectables(selectables); } } @@ -218,4 +215,4 @@ export class RuleSelectionImpl implements RuleSelection { toFormattedOutput(format: OutputFormat): string { return RuleSelectionFormatter.forFormat(format).format(this); } -} \ No newline at end of file +} diff --git a/packages/code-analyzer-core/src/selectors.ts b/packages/code-analyzer-core/src/selectors.ts new file mode 100644 index 00000000..252a034e --- /dev/null +++ b/packages/code-analyzer-core/src/selectors.ts @@ -0,0 +1,122 @@ +import { getMessage } from "./messages"; + +export interface Selector { + matchesSelectables(selectables: string[]): boolean; +} + +export function toSelector(selectorString: string): Selector { + // We parse the selector back-to-front, so that the front-most selectors end up at the bottom of the tree we create + // and therefore get resolved first. + if (selectorString === '') { + // ERROR CASE: The selector is empty. Possible if you do something like "()" or "a:()". + throw new Error(getMessage("SelectorCannotBeEmpty")); + } else if (selectorString.endsWith(')')) { + // If the selector ends in close-paren, then we need to find the open-paren that matches it. + const correspondingOpenParen: number = identifyCorrespondingOpenParen(selectorString); + if (correspondingOpenParen === 0) { + // RECURSIVE CASE: The entire selector is wrapped in parens. Pop them off and call recursively. + return toSelector(selectorString.slice(1, -1)) + } else { + // RECURSIVE CASE: The open-paren is somewhere in the middle of the selector and accompanied by an operator. + const left: string = selectorString.slice(0, correspondingOpenParen - 1); + const right: string = selectorString.slice(correspondingOpenParen); + const op: string = selectorString[correspondingOpenParen - 1]; + return toComplexSelector(left, right, op); + } + } else { + const lastComma: number = selectorString.lastIndexOf(','); + const lastColon: number = selectorString.lastIndexOf(':'); + + // BASE CASE: The selector contains no commas or colons. + if (lastComma === -1 && lastColon === -1) { + // Parens only make sense in conjunction with operators, so if we find any, the selector is malformed. + if (selectorString.includes(')') || selectorString.includes('(')) { + throw new Error(getMessage('SelectorLooksIncorrect', selectorString)); + } + return new SimpleSelector(selectorString); + } else if (lastComma !== -1) { + // Commas resolve before colons, so that "x,a:b" and "a:b,x" both resolve equivalently the combination of + // "x" and "a:b". + const left: string = selectorString.slice(0, lastComma); + const right: string = selectorString.slice(lastComma + 1); + return toComplexSelector(left, right, ','); + } else { + const left: string = selectorString.slice(0, lastColon); + const right: string = selectorString.slice(lastColon + 1); + return toComplexSelector(left, right, ':'); + } + } +} + +function identifyCorrespondingOpenParen(selectorString: string): number { + const reversedLetters: string[] = selectorString.split('').reverse(); + let parenBalance: number = 0; + let idx = 0; + for (const letter of reversedLetters) { + if (letter === ')') { + parenBalance += 1; + } else if (letter === '(') { + parenBalance -= 1; + } + if (parenBalance === 0) { + break; + } + idx += 1; + } + + if (parenBalance > 0) { + throw new Error(getMessage("SelectorLooksIncorrect", selectorString)); + } + + return selectorString.length - idx - 1; +} + +function toComplexSelector(left: string, right: string, op: string): Selector { + if (op === ',') { + return new OrSelector(toSelector(left), toSelector(right)); + } else if (op === ':') { + return new AndSelector(toSelector(left), toSelector(right)); + } else { + throw new Error(getMessage("SelectorLooksIncorrect", `${left}${op}${right}`)); + } +} + +class SimpleSelector implements Selector { + private readonly selector: string; + + constructor(selector: string) { + this.selector = selector; + } + + public matchesSelectables(selectables: string[]): boolean { + return selectables.some(s => s === this.selector.toLowerCase()); + } +} + +class AndSelector implements Selector { + private readonly left: Selector; + private readonly right: Selector; + + constructor(left: Selector, right: Selector) { + this.left = left; + this.right = right; + } + + public matchesSelectables(selectables: string[]): boolean { + return this.left.matchesSelectables(selectables) && this.right.matchesSelectables(selectables); + } +} + +class OrSelector implements Selector { + private readonly left: Selector; + private readonly right: Selector; + + constructor(left: Selector, right: Selector) { + this.left = left; + this.right = right; + } + + public matchesSelectables(selectables: string[]): boolean { + return this.left.matchesSelectables(selectables) || this.right.matchesSelectables(selectables); + } +} diff --git a/packages/code-analyzer-core/test/rule-selection.test.ts b/packages/code-analyzer-core/test/rule-selection.test.ts index af66adb0..a3646780 100644 --- a/packages/code-analyzer-core/test/rule-selection.test.ts +++ b/packages/code-analyzer-core/test/rule-selection.test.ts @@ -149,12 +149,21 @@ describe('Tests for selecting rules', () => { expect(ruleNamesFor(selection7,'stubEngine2')).toEqual([]) }); - it('When multiple selectors are provided, then they act as a union', async () => { - const selection: RuleSelection = await codeAnalyzer.selectRules([ - 'Security', // a tag - 'stubEngine2', // an engine name - 'stub1RuleD' // a rule name - ]); + it.each([ + { + case: 'multiple selectors are provided', + selectors: [ + 'Security', // a tag + 'stubEngine2', // an engine name + 'stub1RuleD' // a rule name + ] + }, + { + case: 'using a comma to join two rule selectors', + selectors: ['Security,stubEngine2,stub1RuleD'] + } + ])('When $case, then it acts like a union', async ({selectors}) => { + const selection: RuleSelection = await codeAnalyzer.selectRules(selectors); expect(selection.getEngineNames()).toEqual(['stubEngine1', 'stubEngine2']); expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleB', 'stub1RuleD']); @@ -164,14 +173,93 @@ describe('Tests for selecting rules', () => { expect(await codeAnalyzer.selectRules(['all', 'Performance', 'DoesNotExist'])).toEqual(await codeAnalyzer.selectRules(['all'])); }); - it('When colons are used and multiple selectors are provided then we get correct union and intersection behavior', async () => { - const selection: RuleSelection = await codeAnalyzer.selectRules(['Recommended:Performance', 'stubEngine2:2', 'stubEngine2:DoesNotExist']); + it.each([ + { + selector: 'Recommended:Performance,2', // Equivalent to "(Recommended:Performance),2" + engines: ['stubEngine1', 'stubEngine2'], + stubEngine1Rules: ['stub1RuleB', 'stub1RuleC'], + stubEngine2Rules: ['stub2RuleC'], + stubEngine3Rules: [] + }, + { + selector: '2,Recommended:Performance', // Equivalent to "2,(Recommended:Performance),2" + engines: ['stubEngine1', 'stubEngine2'], + stubEngine1Rules: ['stub1RuleB', 'stub1RuleC'], + stubEngine2Rules: ['stub2RuleC'], + stubEngine3Rules: [] + }, + { + selector: 'Recommended,3:Performance', // Equivalent to "Recommended,(3:Performance)" + engines: ['stubEngine1', 'stubEngine2', 'stubEngine3'], + stubEngine1Rules: ['stub1RuleA', 'stub1RuleB', 'stub1RuleC', 'stub1RuleE'], + stubEngine2Rules: ['stub2RuleA', 'stub2RuleC'], + stubEngine3Rules: ['stub3RuleA'] + }, + { + selector: '3:Performance,Recommended', // Equivalent to "(3:Performance),Recommended" + engines: ['stubEngine1', 'stubEngine2', 'stubEngine3'], + stubEngine1Rules: ['stub1RuleA', 'stub1RuleB', 'stub1RuleC', 'stub1RuleE'], + stubEngine2Rules: ['stub2RuleA', 'stub2RuleC'], + stubEngine3Rules: ['stub3RuleA'] + } + ])('In the absence of parenthesis-defined ordering, commas are applied after colons. Case: $selector', async ({selector, engines, stubEngine1Rules, stubEngine2Rules, stubEngine3Rules}) => { + const selection: RuleSelection = await codeAnalyzer.selectRules([selector]); + + expect(selection.getEngineNames()).toEqual(engines); + expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(stubEngine1Rules); + expect(ruleNamesFor(selection, 'stubEngine2')).toEqual(stubEngine2Rules); + expect(ruleNamesFor(selection, 'stubEngine3')).toEqual(stubEngine3Rules); + }); + + it.each([ + { + case: 'colons are used and multiple selectors are provided', + selectors: ['Recommended:Performance', 'stubEngine2:2', 'stubEngine2:DoesNotExist'] + }, + { + case: 'colons and commas are nested via parentheses', + selectors: ['(Recommended:Performance),(stubEngine2:2),(stubEngine2:DoesNotExist)'] + } + ])('When $case, then we get correct union and intersection behavior', async ({selectors}) => { + const selection: RuleSelection = await codeAnalyzer.selectRules(selectors); expect(selection.getEngineNames()).toEqual(['stubEngine1', 'stubEngine2']); expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleC']); expect(ruleNamesFor(selection, 'stubEngine2')).toEqual(['stub2RuleC']); }); + it('Parentheses cannot be empty', async () => { + await expect(codeAnalyzer.selectRules(['()'])).rejects.toThrow('empty'); + }); + + it('Redundant parentheses are accepted', async () => { + const selection: RuleSelection = await codeAnalyzer.selectRules(['((((((((stub1RuleC))))))))']); + + expect(selection.getEngineNames()).toEqual(['stubEngine1']); + expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleC']); + }) + + it.each([ + {selector: 'a,b)'}, + {selector: '(a,b'}, + {selector: '((a,b)'}, + {selector: '(a),b)'}, + {selector: ')a,b)'}, + {selector: 'a,b('} + ])('When parentheses are unbalanced, an error is thrown. Case: $selector', async ({selector}) => { + await expect(codeAnalyzer.selectRules([selector])).rejects.toThrow('looks incorrect'); + }); + + it.each([ + {selector: '2(a,b)'}, + {selector: '(a,b)2'}, + {selector: '2(a:b)'}, + {selector: '(a:b)2'} + ])('When parentheses are not accompanied by valid joiners, an error is thrown. Case: $selector', async ({selector}) => { + await expect(codeAnalyzer.selectRules([selector])).rejects.toThrow('looks incorrect'); + }); + + it('When selecting rules based on severity names instead of severity number, then we correctly return the rules', async () => { const selection: RuleSelection = await codeAnalyzer.selectRules(['High', 'Recommended:Low']);