diff --git a/packages/code-analyzer-core/package.json b/packages/code-analyzer-core/package.json index 2ae363b2..ca3a67c8 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.38.0", + "version": "0.38.1-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/messages.ts b/packages/code-analyzer-core/src/messages.ts index 8984345c..8f2ad2e5 100644 --- a/packages/code-analyzer-core/src/messages.ts +++ b/packages/code-analyzer-core/src/messages.ts @@ -167,7 +167,10 @@ const MESSAGE_CATALOG : MessageCatalog = { `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 (,).`, + `Rule selector '%s' looks incorrect. Make sure that the expression contains no whitespace, the parentheses are balanced, and that all subselectors are joined by a colon (:) or comma (,).`, + + SelectorStartsOrEndsWithOperator: + `Rule selectors should not start or end with an operator. Selector: '%s'`, EngineRunResultsMissing: `Couldn't get results for engine '%s' since they're missing from the overall run results. Most likely the engine didn't run.`, diff --git a/packages/code-analyzer-core/src/selectors.ts b/packages/code-analyzer-core/src/selectors.ts index c7b6128e..fc13a6fc 100644 --- a/packages/code-analyzer-core/src/selectors.ts +++ b/packages/code-analyzer-core/src/selectors.ts @@ -5,81 +5,72 @@ export interface Selector { } 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 { - // If there's a close-paren in the string, only look for operators after it. - const lastCloseParen: number = Math.max(selectorString.lastIndexOf(')'), 0); - const lastComma: number = selectorString.slice(lastCloseParen).lastIndexOf(','); - const lastColon: number = selectorString.slice(lastCloseParen).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 + lastCloseParen); - const right: string = selectorString.slice(lastComma + lastCloseParen + 1); - return toComplexSelector(left, right, ','); - } else { - const left: string = selectorString.slice(0, lastColon + lastCloseParen); - const right: string = selectorString.slice(lastColon + lastCloseParen + 1); - return toComplexSelector(left, right, ':'); - } } -} -function identifyCorrespondingOpenParen(selectorString: string): number { - const reversedLetters: string[] = selectorString.split('').reverse(); + let commaIdx: number|null = null; + let colonIdx: number|null = null; let parenBalance: number = 0; - let idx = 0; - for (const letter of reversedLetters) { - if (letter === ')') { + let hasParens: boolean = false; + for (let i = 0; i < selectorString.length; i++) { + const char: string = selectorString[i]; + if ((i === 0 || i === selectorString.length - 1) && (char === ',' || char === ':')) { + // ERROR CASE: The selector cannot start or end with a binary operator, because that's nonsense. + throw new Error(getMessage("SelectorStartsOrEndsWithOperator", selectorString)); + } else if (char === ' ') { + // ERROR CASE: The selector contains whitespace. + throw new Error(getMessage("SelectorLooksIncorrect", selectorString)); + } else if (char === '(') { parenBalance += 1; - } else if (letter === '(') { + hasParens = true; + } else if (char === ')') { parenBalance -= 1; + hasParens = true; + // If our parenthesis balance is negative, it means there are more close-parens than open-parens, which is a problem. + if (parenBalance < 0) { + throw new Error(getMessage("SelectorLooksIncorrect", selectorString)); + } + } else if (char === ',') { + // If we're not inside of parentheses, and we haven't already found a comma, note the location of this one. + if (parenBalance === 0) { + commaIdx = i; + // Commas trump everything else, so we can just break. + break; + } + } else if (char === ':') { + // If we're not inside of parentheses, and we haven't already found a colon, note the location of this one. + if (parenBalance === 0 && colonIdx === null) { + colonIdx = i; + } } - if (parenBalance === 0) { - break; - } - idx += 1; } + // If our final parenthesis balance is negative, it means there are more open-parens than close-parens, which is a problem. 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 === ',') { + // Commas trump colons, so if we have a comma, split along that. + if (commaIdx != null) { + const left: string = selectorString.slice(0, commaIdx); + const right: string = selectorString.slice(commaIdx + 1); return new OrSelector(toSelector(left), toSelector(right)); - } else if (op === ':') { + } else if (colonIdx != null) { + // If there are colons but no commas, split along the first colon. + const left: string = selectorString.slice(0, colonIdx); + const right: string = selectorString.slice(colonIdx + 1); return new AndSelector(toSelector(left), toSelector(right)); + } else if (selectorString[0] === '(' && selectorString[selectorString.length - 1] === ')') { + // If the first and last character are parentheses, then pop those off and run again. + return toSelector(selectorString.slice(1, selectorString.length - 1)); + } else if (hasParens) { + // There shouldn't be parentheses in the middle of a selector that has no operators. + throw new Error(getMessage('SelectorLooksIncorrect', selectorString)); } else { - throw new Error(getMessage("SelectorLooksIncorrect", `${left}${op}${right}`)); + // A string with no operators or problems is just a simple string-selector. + return new SimpleSelector(selectorString); } } diff --git a/packages/code-analyzer-core/test/rule-selection.test.ts b/packages/code-analyzer-core/test/rule-selection.test.ts index 2a93240b..d5074014 100644 --- a/packages/code-analyzer-core/test/rule-selection.test.ts +++ b/packages/code-analyzer-core/test/rule-selection.test.ts @@ -249,6 +249,17 @@ describe('Tests for selecting rules', () => { expect(ruleNamesFor(selection, 'stubEngine3')).toEqual(stubEngine3Rules); }) + it('Operations with nested parentheses are properly resolved', async () => { + const selection: RuleSelection = await codeAnalyzer.selectRules(['stubEngine1:(2,Performance:(3,4))']) + + expect(selection.getEngineNames()).toEqual(['stubEngine1']); + expect(ruleNamesFor(selection, 'stubEngine1')).toEqual(['stub1RuleB', 'stub1RuleC', 'stub1RuleE']) + }); + + it('Whitespace within selectors is not allowed', async () => { + await expect(codeAnalyzer.selectRules([' ( stubEngine1 ) , ( stubEngine2 ) '])).rejects.toThrow('looks incorrect'); + }) + it.each([ { case: 'colons are used and multiple selectors are provided', @@ -266,8 +277,35 @@ describe('Tests for selecting rules', () => { expect(ruleNamesFor(selection, 'stubEngine2')).toEqual(['stub2RuleC']); }); - it('Parentheses cannot be empty', async () => { - await expect(codeAnalyzer.selectRules(['()'])).rejects.toThrow('empty'); + it.each([ + { + case: 'empty string', + selector: '' + }, + { + case: 'empty parentheses', + selector: '()' + } + ])('Empty selectors are rejected. Case: $case', async ({selector}) => { + await expect(codeAnalyzer.selectRules([selector])).rejects.toThrow('empty'); + }); + + it.each([ + { + case: 'leading commas', + selector: ',asdfasdf' + },{ + case: 'leading colons', + selector: ':asdfasdf' + },{ + case: 'trailing commas', + selector: 'asdfasdf,' + },{ + case: 'trailing colons', + selector: 'asdfasdf:' + } + ])('$case are rejected', async ({selector}) => { + await expect(codeAnalyzer.selectRules([selector])).rejects.toThrow('start or end with an operator'); }); it('Redundant parentheses are accepted', async () => {