Fix inflated match count on collapsed folders in Explorer Find (#234830)

* fixes #234129

* correctly invoce

* 💄

* fix tests

Author: benibenj

src/vs/workbench/contrib/files/browser/views/explorerView.ts

@@ -435,7 +435,7 @@ export class ExplorerView extends ViewPane implements IExplorerView {
 		const explorerLabels = this.instantiationService.createInstance(ResourceLabels, { onDidChangeVisibility: this.onDidChangeBodyVisibility });
 		this._register(explorerLabels);
 
-		this.findProvider = this.instantiationService.createInstance(ExplorerFindProvider, () => this.tree);
+		this.findProvider = this.instantiationService.createInstance(ExplorerFindProvider, this.filter, () => this.tree);
 
 		const updateWidth = (stat: ExplorerItem) => this.tree.updateWidth(stat);
 		this.renderer = this.instantiationService.createInstance(FilesRenderer, container, explorerLabels, this.findProvider.highlightTree, updateWidth);

src/vs/workbench/contrib/files/browser/views/explorerViewer.ts

@@ -291,6 +291,7 @@ export class ExplorerFindProvider implements IAsyncFindProvider<ExplorerItem> {
 	}
 
 	constructor(
+		private readonly filesFilter: FilesFilter,
 		private readonly treeProvider: () => WorkbenchCompressibleAsyncDataTree<ExplorerItem | ExplorerItem[], ExplorerItem, FuzzyScore>,
 		@ISearchService private readonly searchService: ISearchService,
 		@IFileService private readonly fileService: IFileService,
@@ -621,7 +622,10 @@ export class ExplorerFindProvider implements IAsyncFindProvider<ExplorerItem> {
 		const fileResultResources = fileResults.results.map(result => result.resource);
 		const directoryResources = getMatchingDirectoriesFromFiles(folderResults.results.map(result => result.resource), root, segmentMatchPattern);
 
-		return { explorerRoot: root, files: fileResultResources, directories: directoryResources, hitMaxResults: !!fileResults.limitHit || !!folderResults.limitHit };
+		const filteredFileResources = fileResultResources.filter(resource => !this.filesFilter.isIgnored(resource, root.resource, false));
+		const filteredDirectoryResources = directoryResources.filter(resource => !this.filesFilter.isIgnored(resource, root.resource, true));
+
+		return { explorerRoot: root, files: filteredFileResources, directories: filteredDirectoryResources, hitMaxResults: !!fileResults.limitHit || !!folderResults.limitHit };
 	}
 }
 
@@ -1404,12 +1408,9 @@ export class FilesFilter implements ITreeFilter<ExplorerItem, FuzzyScore> {
 		// Hide those that match Hidden Patterns
 		const cached = this.hiddenExpressionPerRoot.get(stat.root.resource.toString());
 		const globMatch = cached?.parsed(path.relative(stat.root.resource.path, stat.resource.path), stat.name, name => !!(stat.parent && stat.parent.getChild(name)));
-		// Small optimization to only traverse gitIgnore if the globMatch from fileExclude returned nothing
-		const ignoreFile = globMatch ? undefined : this.ignoreTreesPerRoot.get(stat.root.resource.toString())?.findSubstr(stat.resource);
-		const isIncludedInTraversal = ignoreFile?.isPathIncludedInTraversal(stat.resource.path, stat.isDirectory);
-		// Doing !undefined returns true and we want it to be false when undefined because that means it's not included in the ignore file
-		const isIgnoredByIgnoreFile = isIncludedInTraversal === undefined ? false : !isIncludedInTraversal;
-		if (isIgnoredByIgnoreFile || globMatch || stat.parent?.isExcluded) {
+		// Small optimization to only run isHiddenResource (traverse gitIgnore) if the globMatch from fileExclude returned nothing
+		const isHiddenResource = !!globMatch ? true : this.isIgnored(stat.resource, stat.root.resource, stat.isDirectory);
+		if (isHiddenResource || stat.parent?.isExcluded) {
 			stat.isExcluded = true;
 			const editors = this.editorService.visibleEditors;
 			const editor = editors.find(e => e.resource && this.uriIdentityService.extUri.isEqualOrParent(e.resource, stat.resource));
@@ -1424,6 +1425,14 @@ export class FilesFilter implements ITreeFilter<ExplorerItem, FuzzyScore> {
 		return true;
 	}
 
+	isIgnored(resource: URI, rootResource: URI, isDirectory: boolean): boolean {
+		const ignoreFile = this.ignoreTreesPerRoot.get(rootResource.toString())?.findSubstr(resource);
+		const isIncludedInTraversal = ignoreFile?.isPathIncludedInTraversal(resource.path, isDirectory);
+
+		// Doing !undefined returns true and we want it to be false when undefined because that means it's not included in the ignore file
+		return isIncludedInTraversal === undefined ? false : !isIncludedInTraversal;
+	}
+
 	dispose(): void {
 		dispose(this.toDispose);
 	}

src/vs/workbench/contrib/files/test/browser/explorerFindProvider.test.ts

@@ -9,11 +9,11 @@ import { ICompressedTreeNode } from '../../../../../base/browser/ui/tree/compres
 import { ExplorerItem } from '../../common/explorerModel.js';
 import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
 import { ITreeCompressionDelegate } from '../../../../../base/browser/ui/tree/asyncDataTree.js';
-import { ITreeNode, IAsyncDataSource } from '../../../../../base/browser/ui/tree/tree.js';
+import { ITreeNode, IAsyncDataSource, ITreeFilter, TreeFilterResult } from '../../../../../base/browser/ui/tree/tree.js';
 import { DisposableStore } from '../../../../../base/common/lifecycle.js';
 import { TestFileService, workbenchInstantiationService } from '../../../../test/browser/workbenchTestServices.js';
 import { NullFilesConfigurationService } from '../../../../test/common/workbenchTestServices.js';
-import { ExplorerFindProvider } from '../../browser/views/explorerViewer.js';
+import { ExplorerFindProvider, FilesFilter } from '../../browser/views/explorerViewer.js';
 import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js';
 import { IWorkbenchCompressibleAsyncDataTreeOptions, WorkbenchCompressibleAsyncDataTree } from '../../../../../platform/list/browser/listService.js';
 import { IListAccessibilityProvider } from '../../../../../base/browser/ui/list/listWidget.js';
@@ -120,7 +120,13 @@ class CompressionDelegate implements ITreeCompressionDelegate<ExplorerItem> {
 	}
 }
 
-suite('Files - ExplorerView', () => {
+class TestFilesFilter implements ITreeFilter<ExplorerItem> {
+	filter(): TreeFilterResult<void> { return true; }
+	isIgnored(): boolean { return false; }
+	dispose() { }
+}
+
+suite('Find Provider - ExplorerView', () => {
 	const disposables = ensureNoDisposablesAreLeakedInTestSuite();
 
 	const fileService = new TestFileService();
@@ -196,14 +202,15 @@ suite('Files - ExplorerView', () => {
 		const compressionDelegate = new CompressionDelegate(dataSource);
 		const keyboardNavigationLabelProvider = new KeyboardNavigationLabelProvider();
 		const accessibilityProvider = new AccessibilityProvider();
+		const filter = instantiationService.createInstance(TestFilesFilter) as unknown as FilesFilter;
 
 		const options: IWorkbenchCompressibleAsyncDataTreeOptions<ExplorerItem, FuzzyScore> = { identityProvider: new IdentityProvider(), keyboardNavigationLabelProvider, accessibilityProvider };
 		const tree = disposables.add(instantiationService.createInstance(WorkbenchCompressibleAsyncDataTree<ExplorerItem | ExplorerItem[], ExplorerItem, FuzzyScore>, 'test', container, new VirtualDelegate(), compressionDelegate, [new Renderer()], dataSource, options));
 		tree.layout(200);
 
 		await tree.setInput(root);
 
-		const findProvider = instantiationService.createInstance(ExplorerFindProvider, () => tree);
+		const findProvider = instantiationService.createInstance(ExplorerFindProvider, filter, () => tree);
 
 		findProvider.startSession();