microsoft / microsoft/vscode

Search: async exclude evaluation ignores a matching exclude when several exclude expressions are configured

Open
#332,631 0 comments 0 reactions 1 assignee Claimed by @osortega View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.