Search: async exclude evaluation ignores a matching exclude when several exclude expressions are configured
- Dominant language
- TypeScript
- Stars
- 193k
- Forks
- 42.4k
- PR merge metrics
- PR metrics pending
Description
### Bug description
`QueryGlobTester.includedInQuery` (`src/vs/workbench/services/search/common/search.ts`) evaluates the parsed exclude expressions asynchronously whenever sibling clauses are involved, and used to combine the per-expression results like this:
```ts
const excluded = e(testPath, basename, hasSibling);
if (isThenable(excluded)) {
return excluded.then(excluded => {
if (excluded) { return false; }
return isIncluded();
});
}
return isIncluded();
...
}).then(e => e.some(e => !!e));
```
Two defects fall out of that shape once a folder query carries more than one distinct exclude expression:
1. Each expression contributes either `false` (it excluded the path) or the include-expression verdict, and `.some()` ORs those together. One non-matching exclude therefore overrides an exclusion produced by a different expression: excluded-by-A plus included-by-B resolves to `true`.
2. The synchronous (non-promise) branch never consults `excluded` at all, so any exclude expression that matches without needing a sibling check silently stops excluding in this code path.
The sync twin `includedInQuerySync` right above already implements the correct semantics (any matching exclude wins); the async twin just disagreed with it.
### Steps to reproduce
Configure a folder search with two distinct exclude patterns where one uses a `when` sibling clause, e.g.
```json
"search.exclude": {
"**/node_modules/**": true,
"**/vendor/**": { "when": "$(basename).bak" }
}
```
and evaluate a path under `node_modules`: with the old logic the promise-based result could report the path as included even though the first pattern excludes it.
### Expected behavior
Any exclude expression that matches excludes the path, whether its evaluation is synchronous or sibling-clause asynchronous; only when no exclude matches does the include expression decide. Identical to `includedInQuerySync`.
### Version tested
Commit `38ec3d57f91b` on `main`; both failure modes pinned by new unit tests in `src/vs/workbench/services/search/test/common/search.test.ts`. A fix is ready.
Contributor guide
Assessment
This issue has not been assessed yet.