Search include/exclude patterns are not applied to open files - fix #29658

src/vs/base/common/glob.ts

@@ -5,6 +5,7 @@
 'use strict';
 
 import arrays = require('vs/base/common/arrays');
+import objects = require('vs/base/common/objects');
 import strings = require('vs/base/common/strings');
 import paths = require('vs/base/common/paths');
 import { BoundedMap } from 'vs/base/common/map';
@@ -19,6 +20,10 @@ export function getEmptyExpression(): IExpression {
 	return Object.create(null);
 }
 
+export function mergeExpressions(...expressions: IExpression[]): IExpression {
+	return objects.assign(getEmptyExpression(), ...expressions.filter(expr => !!expr));
+}
+
 export interface SiblingClause {
 	when: string;
 }

src/vs/base/test/node/glob.test.ts

@@ -865,4 +865,23 @@ suite('Glob', () => {
 	function nativeSep(slashPath: string): string {
 		return slashPath.replace(/\//g, path.sep);
 	}
+
+	test('mergeExpressions', () => {
+		// Empty => empty
+		assert.deepEqual(glob.mergeExpressions(), glob.getEmptyExpression());
+
+		// Doesn't modify given expressions
+		const expr1 = { 'a': true };
+		glob.mergeExpressions(expr1, { 'b': true });
+		assert.deepEqual(expr1, { 'a': true });
+
+		// Merges correctly
+		assert.deepEqual(glob.mergeExpressions({ 'a': true }, { 'b': true }), { 'a': true, 'b': true });
+
+		// Ignores null/undefined portions
+		assert.deepEqual(glob.mergeExpressions(undefined, { 'a': true }, null, { 'b': true }), { 'a': true, 'b': true });
+
+		// Later expressions take precedence
+		assert.deepEqual(glob.mergeExpressions({ 'a': true, 'b': false, 'c': true }, { 'a': false, 'b': true }), { 'a': false, 'b': true, 'c': true });
+	});
 });
\ No newline at end of file

src/vs/platform/search/common/search.ts

@@ -6,7 +6,6 @@
 
 import { PPromise, TPromise } from 'vs/base/common/winjs.base';
 import uri from 'vs/base/common/uri';
-import * as paths from 'vs/base/common/paths';
 import * as objects from 'vs/base/common/objects';
 import { IExpression } from 'vs/base/common/glob';
 import { IFilesConfiguration } from 'vs/platform/files/common/files';
@@ -175,25 +174,3 @@ export function getExcludes(configuration: ISearchConfiguration): IExpression {
 
 	return allExcludes;
 }
-
-export function getMergedExcludes(query: ISearchQuery, absolutePaths?: boolean): IExpression {
-	const globalExcludePattern: IExpression = query.excludePattern || {};
-
-	return query.folderQueries
-		.map(folderQuery => {
-			const mergedFolderExclude = objects.assign({}, globalExcludePattern, folderQuery.excludePattern || {});
-			return absolutePaths ?
-				makeExcludesAbsolute(mergedFolderExclude, folderQuery.folder) :
-				mergedFolderExclude;
-		});
-}
-
-function makeExcludesAbsolute(excludePattern: IExpression, rootFolder: uri) {
-	return Object.keys(excludePattern)
-		.reduce((absolutePattern: IExpression, key: string) => {
-			const value = excludePattern[key];
-			key = paths.join(rootFolder.fsPath, key);
-			absolutePattern[key] = value;
-			return absolutePattern;
-		}, {});
-}
\ No newline at end of file

src/vs/workbench/parts/search/common/queryBuilder.ts

@@ -60,11 +60,35 @@ export class QueryBuilder {
 			return folderConfig.search.useRipgrep;
 		});
 
+		// Filter extraFileResources against global include/exclude patterns - they are already expected to not belong to a workspace
+		let extraFileResources = options.extraFileResources && options.extraFileResources.filter(extraFile => {
+			if (excludePattern && glob.match(excludePattern, extraFile.fsPath)) {
+				return false;
+			}
+
+			if (includePattern && !glob.match(includePattern, extraFile.fsPath)) {
+				return false;
+			}
+
+			// If searchPaths are being used, the extra file must be in a subfolder and match the pattern, if present
+			if (searchPaths) {
+				return searchPaths.every(searchPath => {
+					if (paths.isEqualOrParent(extraFile.fsPath, searchPath.searchPath.fsPath)) {
+						return !searchPath.pattern || glob.match(searchPath.pattern, extraFile.fsPath);
+					} else {
+						return false;
+					}
+				});
+			}
+
+			return true;
+		});
+		extraFileResources = extraFileResources && extraFileResources.length ? extraFileResources : undefined;
 
 		return {
 			type,
 			folderQueries,
-			extraFileResources: options.extraFileResources,
+			extraFileResources,
 			filePattern: options.filePattern,
 			excludePattern,
 			includePattern,

src/vs/workbench/parts/search/test/common/queryBuilder.test.ts

@@ -240,6 +240,62 @@ suite('QueryBuilder', () => {
 			});
 	});
 
+	test('extraFileResources', () => {
+		assertEqualQueries(
+			queryBuilder.text(
+				PATTERN_INFO,
+				[ROOT_1_URI],
+				{ extraFileResources: [getUri('/foo/bar.js')] }
+			),
+			<ISearchQuery>{
+				contentPattern: PATTERN_INFO,
+				folderQueries: [{
+					folder: ROOT_1_URI
+				}],
+				extraFileResources: [getUri('/foo/bar.js')],
+				type: QueryType.Text,
+				useRipgrep: true
+			});
+
+		assertEqualQueries(
+			queryBuilder.text(
+				PATTERN_INFO,
+				[ROOT_1_URI],
+				{
+					extraFileResources: [getUri('/foo/bar.js')],
+					excludePattern: '**/*.js'
+				}
+			),
+			<ISearchQuery>{
+				contentPattern: PATTERN_INFO,
+				folderQueries: [{
+					folder: ROOT_1_URI
+				}],
+				excludePattern: patternsToIExpression(globalGlob('**/*.js')),
+				type: QueryType.Text,
+				useRipgrep: true
+			});
+
+		assertEqualQueries(
+			queryBuilder.text(
+				PATTERN_INFO,
+				[ROOT_1_URI],
+				{
+					extraFileResources: [getUri('/foo/bar.js')],
+					includePattern: '**/*.txt'
+				}
+			),
+			<ISearchQuery>{
+				contentPattern: PATTERN_INFO,
+				folderQueries: [{
+					folder: ROOT_1_URI
+				}],
+				includePattern: patternsToIExpression(globalGlob('**/*.txt')),
+				type: QueryType.Text,
+				useRipgrep: true
+			});
+	});
+
 	suite('parseSearchPaths', () => {
 		test('simple includes', () => {
 			function testSimpleIncludes(includePattern: string, expectedPatterns: string[]): void {
@@ -506,12 +562,19 @@ function assertEqualQueries(actual: ISearchQuery, expected: ISearchQuery): void
 		};
 	};
 
+	// Avoid comparing URI objects, not a good idea
 	if (expected.folderQueries) {
 		assert.deepEqual(actual.folderQueries.map(folderQueryToCompareObject), expected.folderQueries.map(folderQueryToCompareObject));
 		delete actual.folderQueries;
 		delete expected.folderQueries;
 	}
 
+	if (expected.extraFileResources) {
+		assert.deepEqual(actual.extraFileResources.map(extraFile => extraFile.fsPath), expected.extraFileResources.map(extraFile => extraFile.fsPath));
+		delete expected.extraFileResources;
+		delete actual.extraFileResources;
+	}
+
 	actual.includePattern = normalizeExpression(actual.includePattern);
 	actual.excludePattern = normalizeExpression(actual.excludePattern);
 	cleanUndefinedQueryValues(actual);

src/vs/workbench/services/search/node/searchService.ts

@@ -6,13 +6,12 @@
 
 import { PPromise, TPromise } from 'vs/base/common/winjs.base';
 import uri from 'vs/base/common/uri';
-import glob = require('vs/base/common/glob');
 import objects = require('vs/base/common/objects');
 import scorer = require('vs/base/common/scorer');
 import strings = require('vs/base/common/strings');
 import { getNextTickChannel } from 'vs/base/parts/ipc/common/ipc';
 import { Client } from 'vs/base/parts/ipc/node/ipc.cp';
-import { IProgress, LineMatch, FileMatch, ISearchComplete, ISearchProgressItem, QueryType, IFileMatch, ISearchQuery, ISearchConfiguration, ISearchService, getMergedExcludes } from 'vs/platform/search/common/search';
+import { IProgress, LineMatch, FileMatch, ISearchComplete, ISearchProgressItem, QueryType, IFileMatch, ISearchQuery, ISearchConfiguration, ISearchService } from 'vs/platform/search/common/search';
 import { IUntitledEditorService } from 'vs/workbench/services/untitled/common/untitledEditorService';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
@@ -153,42 +152,22 @@ export class SearchService implements ISearchService {
 	}
 
 	private matches(resource: uri, query: ISearchQuery): boolean {
-		const {
-			filePattern,
-			includePattern,
-		} = query;
-
 		// file pattern
-		if (filePattern) {
+		if (query.filePattern) {
 			if (resource.scheme !== 'file') {
 				return false; // if we match on file pattern, we have to ignore non file resources
 			}
 
-			if (!scorer.matches(resource.fsPath, strings.stripWildcards(filePattern).toLowerCase())) {
+			if (!scorer.matches(resource.fsPath, strings.stripWildcards(query.filePattern).toLowerCase())) {
 				return false;
 			}
 		}
 
 		// includes
-		if (includePattern) {
+		if (query.includePattern) {
 			if (resource.scheme !== 'file') {
 				return false; // if we match on file patterns, we have to ignore non file resources
 			}
-
-			if (!glob.match(includePattern, resource.fsPath)) {
-				return false;
-			}
-		}
-
-		// excludes
-
-		const allFolderExcludes = getMergedExcludes(query, /*absolutePaths=*/true);
-		if (resource.scheme !== 'file') {
-			return true; // e.g. untitled files can never be excluded with file patterns
-		}
-
-		if (glob.match(allFolderExcludes, resource.fsPath)) {
-			return false;
 		}
 
 		return true;