Skip to content

Avoid not necessary parenthesis on 'or' and 'and' expressions #241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
otaviojacobi merged 2 commits into from
Jun 30, 2026
Merged

Avoid not necessary parenthesis on 'or' and 'and' expressions #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63cf798
Avoid not necessary parenthesis on 'or' and 'and' expressions
otaviojacobi Jun 24, 2026
84d63fb
Type the bracketJoin separator as precise literals
otaviojacobi Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 35 additions & 4 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,11 +402,42 @@ const join = (strOrArray: string | string[], separator = ','): string => {
}
};

type RemoveDollarPrefix<T extends string> = T extends `$${infer Rest}`
? Rest
: T;
const stripDollarPrefix = <T extends `$${string}`>(
key: T,
): RemoveDollarPrefix<T> => key.slice(1) as RemoveDollarPrefix<T>;

// Every separator `bracketJoin` is ever called with: `,` for `$in` lists, the
// logical `$and`/`$or` joins, and the comparison/arithmetic operators.
type FilterSeparator =
| ','
| ' and '
| ' or '
| ` ${RemoveDollarPrefix<FilterOperationKey>} `;
Comment thread
Page- marked this conversation as resolved.

// Join together a bunch of statements making sure the whole lot is correctly parenthesised
const bracketJoin = (arr: string[][], separator: string): string[] => {
const bracketJoin = (arr: string[][], separator: FilterSeparator): string[] => {
if (arr.length === 1) {
return arr[0];
}
if (separator === ' or ' || separator === ' and ') {
const otherSeparator = separator === ' or ' ? ' and ' : ' or ';
const $resultArr: string[] = [];
for (let i = 0; i < arr.length; i++) {
if (i !== 0) {
$resultArr.push(separator);
}
const subArr = arr[i];
if (subArr.includes(otherSeparator)) {
$resultArr.push(`(${subArr.join('')})`);
} else {
$resultArr.push(...subArr);
}
}
return $resultArr;
}
const resultArr: string[] = [];
for (let i = 0; i < arr.length; i++) {
if (i !== 0) {
Expand Down Expand Up @@ -470,7 +501,7 @@ const filterOperation = <T extends Resource['Read']>(
operator: FilterOperationKey,
parentKey?: string[],
): string[] => {
const op = ' ' + operator.slice(1) + ' ';
const op = ` ${stripDollarPrefix(operator)} ` as const;
if (isPrimitive(filter)) {
const filterStr = escapeValue(filter);
return addParentKey(filterStr, parentKey, op);
Expand Down Expand Up @@ -702,7 +733,7 @@ const handleFilterOperator = <
const filterStr = buildFilter(
filter as FilterType<typeof operator, T>,
undefined,
` ${operator.slice(1)} `,
` ${stripDollarPrefix(operator)} `,
);
return addParentKey(filterStr, parentKey);
}
Expand Down Expand Up @@ -852,7 +883,7 @@ const handleFilterArray = <T extends Resource['Read']>(
const buildFilter = <T extends Resource['Read']>(
filter: Filter<T>,
parentKey?: string[],
joinStr?: string,
joinStr?: FilterSeparator,
): string[] => {
if (isPrimitive(filter)) {
const filterStr = escapeValue(filter);
Expand Down
56 changes: 44 additions & 12 deletions test/filter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,15 @@ testFilter(
a: 'b',
d: 'e',
},
"(a eq 'b') and (d eq 'e')",
"a eq 'b' and d eq 'e'",
);

testFilter(
{
a: "b'c",
d: "e''f'''g",
},
"(a eq 'b''c') and (d eq 'e''''f''''''g')",
"a eq 'b''c' and d eq 'e''''f''''''g'",
);

const testOperator = function (operator: string) {
Expand Down Expand Up @@ -331,7 +331,7 @@ testFilter(
},
},
],
"((a eq 'b') eq (c eq 'd')) or ((e eq 'f') ne (g eq 'h'))",
"(a eq 'b') eq (c eq 'd') or (e eq 'f') ne (g eq 'h')",
);

testFilter(
Expand Down Expand Up @@ -373,7 +373,7 @@ testFilter(
{
a: [{ b: 'c' }, { d: 'e' }],
},
"a eq ((b eq 'c') or (d eq 'e'))",
"a eq (b eq 'c' or d eq 'e')",
);

testFilter(
Expand All @@ -398,7 +398,7 @@ testFilter(
{
a: [{ b: 'c' }, 'd'],
},
"a eq ((b eq 'c') or 'd')",
"a eq (b eq 'c' or 'd')",
);

testFilter(
Expand Down Expand Up @@ -733,14 +733,14 @@ testFilter(
c: 'd',
},
},
"not((a eq 'b') and (c eq 'd'))",
"not(a eq 'b' and c eq 'd')",
);

testFilter(
{
$not: [{ a: 'b' }, { c: 'd' }],
},
"not((a eq 'b') or (c eq 'd'))",
"not(a eq 'b' or c eq 'd')",
);

testFilter(
Expand Down Expand Up @@ -770,7 +770,7 @@ testFilter(
},
},
},
"a eq not((b eq 'c') and (d eq 'e'))",
"a eq not(b eq 'c' and d eq 'e')",
);

testFilter(
Expand All @@ -779,7 +779,7 @@ testFilter(
$not: [{ b: 'c' }, { d: 'e' }],
},
},
"a eq not((b eq 'c') or (d eq 'e'))",
"a eq not(b eq 'c' or d eq 'e')",
);

// Test $add
Expand Down Expand Up @@ -997,7 +997,7 @@ const testLambda = function (operator: string) {
},
}),
},
`a/${op}(x:(x/b/${op}(y:y/c eq 'd')) and (x/e/${op}(z:z/f eq 'g')))`,
`a/${op}(x:x/b/${op}(y:y/c eq 'd') and x/e/${op}(z:z/f eq 'g'))`,
);
};

Expand Down Expand Up @@ -1074,7 +1074,7 @@ testFilter(
},
},
},
'(o/E.f()) or ((a eq true) and (b/any(c:c/d/E.f())))',
'o/E.f() or (a eq true and b/any(c:c/d/E.f()))',
);

testFilter(
Expand Down Expand Up @@ -1107,5 +1107,37 @@ testFilter(
},
},
},
`(o/E.f()) or ((a eq true) and (b/any(c:c/d/B.k('arg1','arg2'))))`,
`o/E.f() or (a eq true and b/any(c:c/d/B.k('arg1','arg2')))`,
);

// `or`/`and` operands that are comparisons or paths never need parentheses.
testFilter({ $or: [{ a: 1 }, { b: 2 }] }, 'a eq 1 or b eq 2');
testFilter({ $and: [{ a: 1 }, { b: 2 }] }, 'a eq 1 and b eq 2');
testFilter(
{ $or: [{ a: 1 }, { b: 2 }, { c: 3 }] },
'a eq 1 or b eq 2 or c eq 3',
);

// A nested `and` under `or` (or vice versa) keeps its parentheses regardless of
// position, since the mixed precedence would otherwise be ambiguous.
testFilter(
{ $or: [{ $and: [{ a: 1 }, { b: 2 }] }, { c: 3 }] },
'(a eq 1 and b eq 2) or c eq 3',
);
testFilter(
{ $or: [{ c: 3 }, { $and: [{ a: 1 }, { b: 2 }] }] },
'c eq 3 or (a eq 1 and b eq 2)',
);
testFilter(
{ $and: [{ $or: [{ a: 1 }, { b: 2 }] }, { c: 3 }] },
'(a eq 1 or b eq 2) and c eq 3',
);
testFilter(
{ $and: [{ $or: [{ a: 1 }, { b: 2 }] }, { $or: [{ c: 3 }, { d: 4 }] }] },
'(a eq 1 or b eq 2) and (c eq 3 or d eq 4)',
);
// Same-operator nesting is associative, so no extra parentheses are added.
testFilter(
{ $or: [{ $or: [{ a: 1 }, { b: 2 }] }, { c: 3 }] },
'a eq 1 or b eq 2 or c eq 3',
);