From c72f39bb4fbd38edd946c6d693e0c2b9fb9f5d10 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Thu, 8 May 2025 14:10:57 +0200 Subject: [PATCH] chore: make operator checking more ergonomic and type-ful (#9932) This is a helper PR for a refactor I'm working on for the new constraint inputs. In the refactoring, it's useful to have individual subtypes for the various subgroups of operators and to be able to easily assert whether something is X operator or not. The only change required in the code base is a single check for operators, which is now handled by using the new `isXOperator` functions instead. Yes, the operator file in constants now includes functions, but it seemed useful to put the identification functions there instead of somewhere unrelated. The tests are primarily to ensure that the identifier function works, and I'd be happy to remove them if we think it's necessary. That said, they're pretty simple unit tests, so I think it's fine to leave them. The main bulk of the change is: removing the explicit `: Operator[]` typing to the various sub-sets of operators and instead adding explicit types. Additionally, there's the new identifier functions. --- .../isCaseSensitive.test.ts | 6 +- .../ConstraintItemHeader/isCaseSensitive.ts | 8 +- frontend/src/constants/operators.test.ts | 74 +++++++++++++++++ frontend/src/constants/operators.ts | 80 ++++++++++--------- 4 files changed, 125 insertions(+), 43 deletions(-) create mode 100644 frontend/src/constants/operators.test.ts diff --git a/frontend/src/component/common/ConstraintsList/ConstraintItemHeader/isCaseSensitive.test.ts b/frontend/src/component/common/ConstraintsList/ConstraintItemHeader/isCaseSensitive.test.ts index 70578bd2a2..cdfb702782 100644 --- a/frontend/src/component/common/ConstraintsList/ConstraintItemHeader/isCaseSensitive.test.ts +++ b/frontend/src/component/common/ConstraintsList/ConstraintItemHeader/isCaseSensitive.test.ts @@ -1,6 +1,8 @@ import { allOperators, inOperators, + isInOperator, + isStringOperator, stringOperators, } from 'constants/operators'; import { isCaseSensitive } from './isCaseSensitive'; @@ -20,7 +22,7 @@ test('`IN` and `NOT_IN` are always case sensitive', () => { test('If `caseInsensitive` is true, all operators except for `IN` and `NOT_IN` are considered case insensitive', () => { expect( allOperators - .filter((operator) => !inOperators.includes(operator)) + .filter((operator) => !isInOperator(operator)) .map((operator) => isCaseSensitive(operator, true)) .every((result) => result === false), ).toBe(true); @@ -35,7 +37,7 @@ test.each([false, undefined])( const nonStringResults = allOperators .filter( (operator) => - ![...stringOperators, ...inOperators].includes(operator), + !(isStringOperator(operator) || isInOperator(operator)), ) .map((operator) => isCaseSensitive(operator, caseInsensitive)); diff --git a/frontend/src/component/common/ConstraintsList/ConstraintItemHeader/isCaseSensitive.ts b/frontend/src/component/common/ConstraintsList/ConstraintItemHeader/isCaseSensitive.ts index d9dfbf0ae1..8843cd80eb 100644 --- a/frontend/src/component/common/ConstraintsList/ConstraintItemHeader/isCaseSensitive.ts +++ b/frontend/src/component/common/ConstraintsList/ConstraintItemHeader/isCaseSensitive.ts @@ -1,12 +1,10 @@ import { - inOperators, - stringOperators, + isInOperator, + isStringOperator, type Operator, } from 'constants/operators'; export const isCaseSensitive = ( operator: Operator, caseInsensitive?: boolean, -) => - inOperators.includes(operator) || - (stringOperators.includes(operator) && !caseInsensitive); +) => isInOperator(operator) || (isStringOperator(operator) && !caseInsensitive); diff --git a/frontend/src/constants/operators.test.ts b/frontend/src/constants/operators.test.ts new file mode 100644 index 0000000000..3f440fe804 --- /dev/null +++ b/frontend/src/constants/operators.test.ts @@ -0,0 +1,74 @@ +import { + dateOperators, + inOperators, + isDateOperator, + isInOperator, + isMultiValueOperator, + isNewOperator, + isNumOperator, + isSemVerOperator, + isSingleValueOperator, + isStringOperator, + multipleValueOperators, + newOperators, + numOperators, + semVerOperators, + singleValueOperators, + stringOperators, + type Operator, +} from './operators'; + +describe('operators are correctly identified', () => { + test('date operators', () => { + expectOperatorTypes(dateOperators, ['date', 'single-value', 'new']); + }); + test('in operators', () => { + expectOperatorTypes(inOperators, ['in', 'multi-value']); + }); + test('multi-value operators', () => { + expectOperatorTypes(multipleValueOperators, ['multi-value']); + }); + test('new operators', () => { + expectOperatorTypes(newOperators, ['new']); + }); + test('number operators', () => { + expectOperatorTypes(numOperators, ['single-value', 'number', 'new']); + }); + test('semver operators', () => { + expectOperatorTypes(semVerOperators, ['single-value', 'semver', 'new']); + }); + test('single-value operators', () => { + expectOperatorTypes(singleValueOperators, ['single-value', 'new']); + }); + test('string operators', () => { + expectOperatorTypes(stringOperators, ['multi-value', 'string', 'new']); + }); +}); + +type OperatorCategory = + | 'date' + | 'in' + | 'multi-value' + | 'new' + | 'number' + | 'semver' + | 'single-value' + | 'string'; + +const expectOperatorTypes = ( + operators: Operator[], + categories: OperatorCategory[], +) => { + const successfulChecks = [ + operators.every(isDateOperator) && 'date', + operators.every(isInOperator) && 'in', + operators.every(isMultiValueOperator) && 'multi-value', + operators.every(isNumOperator) && 'number', + operators.every(isSemVerOperator) && 'semver', + operators.every(isSingleValueOperator) && 'single-value', + operators.every(isStringOperator) && 'string', + operators.every(isNewOperator) && 'new', + ].filter(Boolean); + + expect(categories.toSorted()).toStrictEqual(successfulChecks.toSorted()); +}; diff --git a/frontend/src/constants/operators.ts b/frontend/src/constants/operators.ts index 0cfcd56828..6d5d381552 100644 --- a/frontend/src/constants/operators.ts +++ b/frontend/src/constants/operators.ts @@ -15,21 +15,21 @@ export type Operator = | 'SEMVER_GT' | 'SEMVER_LT'; -export const NOT_IN = 'NOT_IN'; -export const IN = 'IN'; -export const STR_ENDS_WITH = 'STR_ENDS_WITH'; -export const STR_STARTS_WITH = 'STR_STARTS_WITH'; -export const STR_CONTAINS = 'STR_CONTAINS'; -export const NUM_EQ = 'NUM_EQ'; -export const NUM_GT = 'NUM_GT'; -export const NUM_GTE = 'NUM_GTE'; -export const NUM_LT = 'NUM_LT'; -export const NUM_LTE = 'NUM_LTE'; -export const DATE_AFTER = 'DATE_AFTER'; -export const DATE_BEFORE = 'DATE_BEFORE'; -export const SEMVER_EQ = 'SEMVER_EQ'; -export const SEMVER_GT = 'SEMVER_GT'; -export const SEMVER_LT = 'SEMVER_LT'; +export const NOT_IN = 'NOT_IN' as const; +export const IN = 'IN' as const; +export const STR_ENDS_WITH = 'STR_ENDS_WITH' as const; +export const STR_STARTS_WITH = 'STR_STARTS_WITH' as const; +export const STR_CONTAINS = 'STR_CONTAINS' as const; +export const NUM_EQ = 'NUM_EQ' as const; +export const NUM_GT = 'NUM_GT' as const; +export const NUM_GTE = 'NUM_GTE' as const; +export const NUM_LT = 'NUM_LT' as const; +export const NUM_LTE = 'NUM_LTE' as const; +export const DATE_AFTER = 'DATE_AFTER' as const; +export const DATE_BEFORE = 'DATE_BEFORE' as const; +export const SEMVER_EQ = 'SEMVER_EQ' as const; +export const SEMVER_GT = 'SEMVER_GT' as const; +export const SEMVER_LT = 'SEMVER_LT' as const; export const allOperators: Operator[] = [ IN, @@ -49,39 +49,47 @@ export const allOperators: Operator[] = [ SEMVER_LT, ]; -export const stringOperators: Operator[] = [ - STR_CONTAINS, - STR_STARTS_WITH, - STR_ENDS_WITH, -]; +const isOperator = + (operators: T[]) => + (operator: string): operator is T => + operators.includes(operator as T); -export const inOperators: Operator[] = [IN, NOT_IN]; +export const stringOperators = [STR_CONTAINS, STR_STARTS_WITH, STR_ENDS_WITH]; +export type StringOperator = (typeof stringOperators)[number]; +export const isStringOperator = isOperator(stringOperators); -export const numOperators: Operator[] = [ - NUM_EQ, - NUM_GT, - NUM_GTE, - NUM_LT, - NUM_LTE, -]; +export const inOperators = [IN, NOT_IN]; +export type InOperator = (typeof inOperators)[number]; +export const isInOperator = isOperator(inOperators); -export const dateOperators: Operator[] = [DATE_BEFORE, DATE_AFTER]; +export const numOperators = [NUM_EQ, NUM_GT, NUM_GTE, NUM_LT, NUM_LTE]; +export type NumOperator = (typeof numOperators)[number]; +export const isNumOperator = isOperator(numOperators); -export const semVerOperators: Operator[] = [SEMVER_EQ, SEMVER_GT, SEMVER_LT]; +export const dateOperators = [DATE_BEFORE, DATE_AFTER]; +export type DateOperator = (typeof dateOperators)[number]; +export const isDateOperator = isOperator(dateOperators); -export const singleValueOperators: Operator[] = [ +export const semVerOperators = [SEMVER_EQ, SEMVER_GT, SEMVER_LT]; +export type SemVerOperator = (typeof semVerOperators)[number]; +export const isSemVerOperator = isOperator(semVerOperators); + +export const singleValueOperators = [ ...semVerOperators, ...dateOperators, ...numOperators, ]; +export type SingleValueOperator = (typeof singleValueOperators)[number]; +export const isSingleValueOperator = isOperator(singleValueOperators); -export const multipleValueOperators: Operator[] = [ - ...stringOperators, - ...inOperators, -]; +export const multipleValueOperators = [...stringOperators, ...inOperators]; +export type MultiValueOperator = (typeof multipleValueOperators)[number]; +export const isMultiValueOperator = isOperator(multipleValueOperators); -export const newOperators: Operator[] = [ +export const newOperators = [ ...stringOperators, ...dateOperators, ...singleValueOperators, ]; +export type NewOperator = (typeof newOperators)[number]; +export const isNewOperator = isOperator(newOperators);