From 9b5b3d453d8beea74bbb7b7d29ed8c0b85a070af Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Mon, 28 Mar 2022 14:43:33 +0200 Subject: [PATCH] feat(ng-dev/pullapprove): support `in` operator in condition evaluation A few conditions in framework currently rely on the `in` operator. We should support this even though it potentially going away when we re-work how global approvals are performed. This makes the verify command a little more future-proof and currently unblocks the dev-infra update. It's worth noting that our Python -> JS conversion is very limited but we can always expand when needed, or start running actual Python. Right now this captures the most common cases of conditions we have seen across Angular. --- ng-dev/pullapprove/condition_evaluator.ts | 10 +++++-- ng-dev/pullapprove/pullapprove_arrays.ts | 4 +++ ng-dev/pullapprove/verify.spec.ts | 36 +++++++++++++++++++++++ 3 files changed, 48 insertions(+), 2 deletions(-) diff --git a/ng-dev/pullapprove/condition_evaluator.ts b/ng-dev/pullapprove/condition_evaluator.ts index cb90ca4cf..324b88c63 100644 --- a/ng-dev/pullapprove/condition_evaluator.ts +++ b/ng-dev/pullapprove/condition_evaluator.ts @@ -72,8 +72,14 @@ export function convertConditionToFunction( /** * Transforms a condition expression from PullApprove that is based on python * so that it can be run inside JavaScript. Current transformations: - * 1. `not <..>` -> `!<..>` + * + * 1. `aExpr not in bExpr` --> `!bExpr.includes(aExpr)` + * 2. `aExpr in bExpr` --> `bExpr.includes(aExpr`) + * 3. `not expr` --> `!expr` */ function transformExpressionToJs(expression: string): string { - return expression.replace(/not\s+/g, '!'); + return expression + .replace(/^(.+)\s+not in\s+(.+)$/, '!$2.includes($1)') + .replace(/^(.+)\s+in\s+(.+)$/, '$2.includes($1)') + .replace(/not\s+/g, '!'); } diff --git a/ng-dev/pullapprove/pullapprove_arrays.ts b/ng-dev/pullapprove/pullapprove_arrays.ts index 9886ac8c0..8dbc08831 100644 --- a/ng-dev/pullapprove/pullapprove_arrays.ts +++ b/ng-dev/pullapprove/pullapprove_arrays.ts @@ -40,6 +40,10 @@ export class PullApproveGroupArray extends Array { return new PullApproveGroupArray(...this.filter((s) => s.groupName.match(pattern))); } + get approved() { + throw new PullApproveGroupStateDependencyError(); + } + get pending() { throw new PullApproveGroupStateDependencyError(); } diff --git a/ng-dev/pullapprove/verify.spec.ts b/ng-dev/pullapprove/verify.spec.ts index 53f298fd7..7fca6e1e3 100644 --- a/ng-dev/pullapprove/verify.spec.ts +++ b/ng-dev/pullapprove/verify.spec.ts @@ -92,6 +92,42 @@ describe('group parsing', () => { expect(renovateGroup.testFile('packages/core/index.ts')).toBe(false); expect(renovateGroup.conditions[0].unverifiable).toBe(true); }); + + describe('in operator', () => { + it('should work', () => { + const groups = getGroupsFromYaml(` + groups: + pass: + conditions: + - "'a' in ['a', 'b']" + invalid: + conditions: + - "'a' in ['b']" + `); + const passGroup = getGroupByName(groups, 'pass')!; + const invalidGroup = getGroupByName(groups, 'invalid')!; + + expect(passGroup.testFile('any')).toBe(true); + expect(invalidGroup.testFile('any')).toBe(false); + }); + + it('should support exclusion check', () => { + const groups = getGroupsFromYaml(` + groups: + invalid: + conditions: + - "'a' not in ['a', 'b']" + pass: + conditions: + - "'a' not in ['b']" + `); + const invalidGroup = getGroupByName(groups, 'invalid')!; + const passGroup = getGroupByName(groups, 'pass')!; + + expect(invalidGroup.testFile('any')).toBe(false); + expect(passGroup.testFile('any')).toBe(true); + }); + }); }); function getGroupByName(groups: PullApproveGroup[], name: string): PullApproveGroup | undefined {