From f045bb5c85e11f37c40363a9e31a0e6bfd5f79d3 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sat, 22 Dec 2018 09:59:46 -0600 Subject: [PATCH] Strict null check SearchModel #60565 --- src/tsconfig.strictNullChecks.json | 2 +- src/vs/platform/search/common/search.ts | 8 +- .../electron-browser/mainThreadWorkspace.ts | 6 +- .../parts/search/browser/searchView.ts | 10 +- .../parts/search/common/searchModel.ts | 147 ++++++++++++------ .../test/browser/openFileHandler.test.ts | 3 +- .../services/search/node/searchService.ts | 2 +- .../search/test/common/searchHelpers.test.ts | 1 + 8 files changed, 116 insertions(+), 63 deletions(-) diff --git a/src/tsconfig.strictNullChecks.json b/src/tsconfig.strictNullChecks.json index 694e689c115..4c3db08a6e4 100644 --- a/src/tsconfig.strictNullChecks.json +++ b/src/tsconfig.strictNullChecks.json @@ -137,7 +137,6 @@ "./vs/base/test/common/json.test.ts", "./vs/base/test/common/jsonEdit.test.ts", "./vs/base/test/common/jsonFormatter.test.ts", - "./vs/base/test/common/labels.test.ts", "./vs/base/test/common/lifecycle.test.ts", "./vs/base/test/common/linkedList.test.ts", @@ -664,6 +663,7 @@ "./vs/workbench/parts/search/common/constants.ts", "./vs/workbench/parts/search/common/queryBuilder.ts", "./vs/workbench/parts/search/common/search.ts", + "./vs/workbench/parts/search/common/searchModel.ts", "./vs/workbench/parts/search/test/browser/mockSearchTree.ts", "./vs/workbench/parts/snippets/electron-browser/configureSnippets.ts", "./vs/workbench/parts/snippets/electron-browser/snippetCompletionProvider.ts", diff --git a/src/vs/platform/search/common/search.ts b/src/vs/platform/search/common/search.ts index cb91c57ef78..bcd142fb73c 100644 --- a/src/vs/platform/search/common/search.ts +++ b/src/vs/platform/search/common/search.ts @@ -76,7 +76,7 @@ export interface ICommonQueryProps { /** For telemetry - indicates what is triggering the source */ _reason?: string; - folderQueries?: IFolderQuery[]; + folderQueries: IFolderQuery[]; includePattern?: glob.IExpression; excludePattern?: glob.IExpression; extraFileResources?: U[]; @@ -153,7 +153,7 @@ export interface IExtendedExtensionSearchOptions { } export interface IFileMatch { - resource?: U; + resource: U; results?: ITextSearchResult[]; } @@ -200,9 +200,7 @@ export interface IProgress { message?: string; } -export interface ISearchProgressItem extends IFileMatch, IProgress { - // Marker interface to indicate the possible values for progress calls from the engine -} +export type ISearchProgressItem = IFileMatch | IProgress; export interface ISearchCompleteStats { limitHit?: boolean; diff --git a/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts b/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts index 3170bcc79b1..b5f6f32d13c 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadWorkspace.ts @@ -11,7 +11,7 @@ import { CommandsRegistry } from 'vs/platform/commands/common/commands'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { ILabelService } from 'vs/platform/label/common/label'; -import { IFolderQuery, IPatternInfo, ISearchConfiguration, ISearchProgressItem, ISearchService, QueryType, IFileQuery } from 'vs/platform/search/common/search'; +import { IFolderQuery, IPatternInfo, ISearchConfiguration, ISearchProgressItem, ISearchService, QueryType, IFileQuery, IFileMatch } from 'vs/platform/search/common/search'; import { IStatusbarService } from 'vs/platform/statusbar/common/statusbar'; import { IWindowService } from 'vs/platform/windows/common/windows'; import { IWorkspaceContextService, WorkbenchState } from 'vs/platform/workspace/common/workspace'; @@ -183,8 +183,8 @@ export class MainThreadWorkspace implements MainThreadWorkspaceShape { query._reason = 'startTextSearch'; const onProgress = (p: ISearchProgressItem) => { - if (p.results) { - this._proxy.$handleTextSearchResult(p, requestId); + if ((p).results) { + this._proxy.$handleTextSearchResult(p, requestId); } }; diff --git a/src/vs/workbench/parts/search/browser/searchView.ts b/src/vs/workbench/parts/search/browser/searchView.ts index 57728adb3fb..cab73eadbbc 100644 --- a/src/vs/workbench/parts/search/browser/searchView.ts +++ b/src/vs/workbench/parts/search/browser/searchView.ts @@ -37,7 +37,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { TreeResourceNavigator2, WorkbenchObjectTree } from 'vs/platform/list/browser/listService'; import { INotificationService, Severity } from 'vs/platform/notification/common/notification'; import { IProgressService } from 'vs/platform/progress/common/progress'; -import { IPatternInfo, ISearchComplete, ISearchConfiguration, ISearchConfigurationProperties, ISearchHistoryService, ISearchHistoryValues, ISearchProgressItem, ITextQuery, SearchErrorCode, VIEW_ID } from 'vs/platform/search/common/search'; +import { IPatternInfo, ISearchComplete, ISearchConfiguration, ISearchConfigurationProperties, ISearchHistoryService, ISearchHistoryValues, ISearchProgressItem, ITextQuery, SearchErrorCode, VIEW_ID, IProgress } from 'vs/platform/search/common/search'; import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { diffInserted, diffInsertedOutline, diffRemoved, diffRemovedOutline, editorFindMatchHighlight, editorFindMatchHighlightBorder, listActiveSelectionForeground } from 'vs/platform/theme/common/colorRegistry'; @@ -1388,11 +1388,11 @@ export class SearchView extends Viewlet implements IViewlet, IPanel { let visibleMatches = 0; let onProgress = (p: ISearchProgressItem) => { // Progress - if (p.total) { - total = p.total; + if ((p).total) { + total = (p).total; } - if (p.worked) { - worked = p.worked; + if ((p).worked) { + worked = (p).worked; } }; diff --git a/src/vs/workbench/parts/search/common/searchModel.ts b/src/vs/workbench/parts/search/common/searchModel.ts index d647233f229..7fbfde4459d 100644 --- a/src/vs/workbench/parts/search/common/searchModel.ts +++ b/src/vs/workbench/parts/search/common/searchModel.ts @@ -94,6 +94,9 @@ export class Match { public get replaceString(): string { const searchModel = this.parent().parent().searchModel; + if (!searchModel.replacePattern) { + throw new Error('searchModel.replacePattern must be set before accessing replaceString'); + } const fullMatchText = this.fullMatchText(); let replaceString = searchModel.replacePattern.getReplaceString(fullMatchText); @@ -169,17 +172,18 @@ export class FileMatch extends Disposable { public readonly onDispose: Event = this._onDispose.event; private _resource: URI; - private _model: ITextModel; + private _model: ITextModel | null; private _modelListener: IDisposable; private _matches: Map; private _removedMatches: Set; - private _selectedMatch: Match; + private _selectedMatch: Match | null; private _updateScheduler: RunOnceScheduler; private _modelDecorations: string[] = []; - constructor(private _query: IPatternInfo, private _previewOptions: ITextSearchPreviewOptions, private _maxResults: number, private _parent: FolderMatch, private rawMatch: IFileMatch, - @IModelService private modelService: IModelService, @IReplaceService private replaceService: IReplaceService) { + constructor(private _query: IPatternInfo, private _previewOptions: ITextSearchPreviewOptions, private _maxResults: number, private _parent: BaseFolderMatch, private rawMatch: IFileMatch, + @IModelService private modelService: IModelService, @IReplaceService private replaceService: IReplaceService + ) { super(); this._resource = this.rawMatch.resource; this._matches = new Map(); @@ -195,7 +199,7 @@ export class FileMatch extends Disposable { this.bindModel(model); this.updateMatchesForModel(); } else { - this.rawMatch.results + this.rawMatch.results! .filter(resultIsMatch) .forEach(rawMatch => { textSearchResultToMatches(rawMatch, this) @@ -235,13 +239,19 @@ export class FileMatch extends Disposable { return; } this._matches = new Map(); + + const wordSeparators = this._query.isWordMatch && this._query.wordSeparators ? this._query.wordSeparators : null; let matches = this._model - .findMatches(this._query.pattern, this._model.getFullModelRange(), this._query.isRegExp, this._query.isCaseSensitive, this._query.isWordMatch ? this._query.wordSeparators : null, false, this._maxResults); + .findMatches(this._query.pattern, this._model.getFullModelRange(), !!this._query.isRegExp, !!this._query.isCaseSensitive, wordSeparators, false, this._maxResults); this.updateMatches(matches, true); } private updatesMatchesForLineAfterReplace(lineNumber: number, modelChange: boolean): void { + if (!this._model) { + return; + } + const range = { startLineNumber: lineNumber, startColumn: this._model.getLineMinColumn(lineNumber), @@ -251,11 +261,16 @@ export class FileMatch extends Disposable { const oldMatches = values(this._matches).filter(match => match.range().startLineNumber === lineNumber); oldMatches.forEach(match => this._matches.delete(match.id())); - const matches = this._model.findMatches(this._query.pattern, range, this._query.isRegExp, this._query.isCaseSensitive, this._query.isWordMatch ? this._query.wordSeparators : null, false, this._maxResults); + const wordSeparators = this._query.isWordMatch && this._query.wordSeparators ? this._query.wordSeparators : null; + const matches = this._model.findMatches(this._query.pattern, range, !!this._query.isRegExp, !!this._query.isCaseSensitive, wordSeparators, false, this._maxResults); this.updateMatches(matches, modelChange); } - private updateMatches(matches: FindMatch[], modelChange: boolean) { + private updateMatches(matches: FindMatch[], modelChange: boolean): void { + if (!this._model) { + return; + } + const textSearchResults = editorMatchesToTextSearchResults(matches, this._model, this._previewOptions); textSearchResults.forEach(textSearchResult => { textSearchResultToMatches(textSearchResult, this).forEach(match => { @@ -291,7 +306,7 @@ export class FileMatch extends Disposable { return this.resource().toString(); } - public parent(): FolderMatch { + public parent(): BaseFolderMatch { return this._parent; } @@ -310,7 +325,7 @@ export class FileMatch extends Disposable { .then(() => this.updatesMatchesForLineAfterReplace(toReplace.range().startLineNumber, false)); } - public setSelectedMatch(match: Match) { + public setSelectedMatch(match: Match | null): void { if (match) { if (!this._matches.has(match.id())) { return; @@ -319,16 +334,17 @@ export class FileMatch extends Disposable { return; } } + this._selectedMatch = match; this.updateHighlights(); } - public getSelectedMatch(): Match { + public getSelectedMatch(): Match | null { return this._selectedMatch; } public isMatchSelected(match: Match): boolean { - return this._selectedMatch && this._selectedMatch.id() === match.id(); + return !!this._selectedMatch && this._selectedMatch.id() === match.id(); } public count(): number { @@ -373,7 +389,7 @@ export interface IChangeEvent { removed?: boolean; } -export class FolderMatch extends Disposable { +export class BaseFolderMatch extends Disposable { private _onChange = this._register(new Emitter()); public readonly onChange: Event = this._onChange.event; @@ -385,7 +401,7 @@ export class FolderMatch extends Disposable { private _unDisposedFileMatches: ResourceMap; private _replacingAll: boolean = false; - constructor(private _resource: URI | null, private _id: string, private _index: number, private _query: ITextQuery, private _parent: SearchResult, private _searchModel: SearchModel, + constructor(protected _resource: URI | null, private _id: string, private _index: number, private _query: ITextQuery, private _parent: SearchResult, private _searchModel: SearchModel, @IReplaceService private replaceService: IReplaceService, @IInstantiationService private instantiationService: IInstantiationService ) { @@ -419,7 +435,7 @@ export class FolderMatch extends Disposable { } public name(): string { - return getBaseLabel(this.resource()); + return getBaseLabel(this.resource() || undefined) || ''; } public parent(): SearchResult { @@ -440,11 +456,11 @@ export class FolderMatch extends Disposable { public add(raw: IFileMatch[], silent: boolean): void { const added: FileMatch[] = []; const updated: FileMatch[] = []; - raw.forEach((rawFileMatch) => { + raw.forEach(rawFileMatch => { if (this._fileMatches.has(rawFileMatch.resource)) { const existingFileMatch = this._fileMatches.get(rawFileMatch.resource); rawFileMatch - .results + .results! .filter(resultIsMatch) .forEach(m => { textSearchResultToMatches(m, existingFileMatch) @@ -556,6 +572,23 @@ export class FolderMatch extends Disposable { } } +/** + * BaseFolderMatch => optional resource ("other files" node) + * FolderMatch => required resource (normal folder node) + */ +export class FolderMatch extends BaseFolderMatch { + constructor(_resource: URI, _id: string, _index: number, _query: ITextQuery, _parent: SearchResult, _searchModel: SearchModel, + @IReplaceService replaceService: IReplaceService, + @IInstantiationService instantiationService: IInstantiationService + ) { + super(_resource, _id, _index, _query, _parent, _searchModel, replaceService, instantiationService); + } + + public resource(): URI { + return this._resource!; + } +} + /** * Compares instances of the same match type. Different match types should not be siblings * and their sort order is undefined. @@ -573,7 +606,7 @@ export function searchMatchComparer(elementA: RenderableMatch, elementB: Rendera return Range.compareRangesUsingStarts(elementA.range(), elementB.range()); } - return undefined; + return 0; } export class SearchResult extends Disposable { @@ -582,7 +615,7 @@ export class SearchResult extends Disposable { public readonly onChange: Event = this._onChange.event; private _folderMatches: FolderMatch[] = []; - private _otherFilesMatch: FolderMatch; + private _otherFilesMatch: BaseFolderMatch; private _folderMatchesMap: TernarySearchTree = TernarySearchTree.forPaths(); private _showHighlights: boolean; private _query: ITextQuery; @@ -612,9 +645,10 @@ export class SearchResult extends Disposable { this._folderMatches = (query.folderQueries || []) .map(fq => fq.folder) .map((resource, index) => this.createFolderMatch(resource, resource.toString(), index, query)); + this._folderMatches.forEach(fm => this._folderMatchesMap.set(fm.resource().toString(), fm)); + this._otherFilesMatch = this.createOtherFilesFolderMatch('otherFiles', this._folderMatches.length + 1, query); - this._otherFilesMatch = this.createFolderMatch(null, 'otherFiles', this._folderMatches.length + 1, query); this._query = query; } @@ -625,8 +659,16 @@ export class SearchResult extends Disposable { } } - private createFolderMatch(resource: URI | null, id: string, index: number, query: ITextQuery): FolderMatch { - const folderMatch = this.instantiationService.createInstance(FolderMatch, resource, id, index, query, this, this._searchModel); + private createFolderMatch(resource: URI, id: string, index: number, query: ITextQuery): FolderMatch { + return this._createBaseFolderMatch(FolderMatch, resource, id, index, query); + } + + private createOtherFilesFolderMatch(id: string, index: number, query: ITextQuery): BaseFolderMatch { + return this._createBaseFolderMatch(BaseFolderMatch, null, id, index, query); + } + + private _createBaseFolderMatch(folderMatchClass: typeof BaseFolderMatch | typeof FolderMatch, resource: URI | null, id: string, index: number, query: ITextQuery): BaseFolderMatch { + const folderMatch = this.instantiationService.createInstance(folderMatchClass, resource, id, index, query, this, this._searchModel); const disposable = folderMatch.onChange((event) => this._onChange.fire(event)); folderMatch.onDispose(() => disposable.dispose()); return folderMatch; @@ -640,7 +682,8 @@ export class SearchResult extends Disposable { // Split up raw into a list per folder so we can do a batch add per folder. const rawPerFolder = new ResourceMap(); const otherFileMatches: IFileMatch[] = []; - this._folderMatches.forEach((folderMatch) => rawPerFolder.set(folderMatch.resource(), [])); + this._folderMatches.forEach(fm => rawPerFolder.set(fm.resource(), [])); + allRaw.forEach(rawFileMatch => { const folderMatch = this.getFolderMatch(rawFileMatch.resource); if (!folderMatch) { @@ -649,7 +692,7 @@ export class SearchResult extends Disposable { } if (folderMatch.resource()) { - rawPerFolder.get(folderMatch.resource()).push(rawFileMatch); + rawPerFolder.get(folderMatch.resource()!).push(rawFileMatch); } else { otherFileMatches.push(rawFileMatch); } @@ -666,7 +709,7 @@ export class SearchResult extends Disposable { } }); - this.otherFiles.add(otherFileMatches, silent); + this._otherFilesMatch!.add(otherFileMatches, silent); } public clear(): void { @@ -706,18 +749,24 @@ export class SearchResult extends Disposable { }); } - public folderMatches(): FolderMatch[] { + public folderMatches(): BaseFolderMatch[] { return this._otherFilesMatch ? - this._folderMatches.concat(this._otherFilesMatch) : - this._folderMatches.concat(); + [ + ...this._folderMatches, + this._otherFilesMatch + ] : + [ + ...this._folderMatches + ]; } public matches(): FileMatch[] { let matches: FileMatch[][] = []; - this.folderMatches().forEach((folderMatch) => { + this.folderMatches().forEach(folderMatch => { matches.push(folderMatch.matches()); }); - return [].concat(...matches); + + return ([]).concat(...matches); } public isEmpty(): boolean { @@ -749,9 +798,10 @@ export class SearchResult extends Disposable { } }); if (this._showHighlights && selectedMatch) { + // TS? this._rangeHighlightDecorations.highlightRange( - selectedMatch.parent().resource(), - selectedMatch.range() + (selectedMatch).parent().resource(), + (selectedMatch).range() ); } else { this._rangeHighlightDecorations.removeHighlightRange(); @@ -762,13 +812,9 @@ export class SearchResult extends Disposable { return this._rangeHighlightDecorations; } - private getFolderMatch(resource: URI): FolderMatch { + private getFolderMatch(resource: URI): BaseFolderMatch { const folderMatch = this._folderMatchesMap.findSubstr(resource.toString()); - return folderMatch ? folderMatch : this.otherFiles; - } - - private get otherFiles(): FolderMatch { - return this._otherFilesMatch; + return folderMatch ? folderMatch : this._otherFilesMatch; } private set replacingAll(running: boolean) { @@ -780,7 +826,6 @@ export class SearchResult extends Disposable { private disposeMatches(): void { this.folderMatches().forEach(folderMatch => folderMatch.dispose()); this._folderMatches = []; - this._otherFilesMatch = null; this._folderMatchesMap = TernarySearchTree.forPaths(); this._rangeHighlightDecorations.removeHighlightRange(); } @@ -805,7 +850,11 @@ export class SearchModel extends Disposable { private currentCancelTokenSource: CancellationTokenSource; - constructor(@ISearchService private searchService: ISearchService, @ITelemetryService private telemetryService: ITelemetryService, @IInstantiationService private instantiationService: IInstantiationService) { + constructor( + @ISearchService private searchService: ISearchService, + @ITelemetryService private telemetryService: ITelemetryService, + @IInstantiationService private instantiationService: IInstantiationService + ) { super(); this._searchResult = this.instantiationService.createInstance(SearchResult, this); } @@ -818,12 +867,12 @@ export class SearchModel extends Disposable { this._replaceActive = replaceActive; } - public get replacePattern(): ReplacePattern { + public get replacePattern(): ReplacePattern | null { return this._replacePattern; } public get replaceString(): string { - return this._replaceString; + return this._replaceString || ''; } public set replaceString(replaceString: string) { @@ -846,7 +895,7 @@ export class SearchModel extends Disposable { this._searchResult.query = this._searchQuery; const progressEmitter = new Emitter(); - this._replacePattern = new ReplacePattern(this._replaceString, this._searchQuery.contentPattern); + this._replacePattern = new ReplacePattern(this.replaceString, this._searchQuery.contentPattern); const tokenSource = this.currentCancelTokenSource = new CancellationTokenSource(); const currentRequest = this.searchService.textSearch(this._searchQuery, this.currentCancelTokenSource.token, p => { @@ -888,7 +937,11 @@ export class SearchModel extends Disposable { return currentRequest; } - private onSearchCompleted(completed: ISearchComplete, duration: number): ISearchComplete { + private onSearchCompleted(completed: ISearchComplete | null, duration: number): ISearchComplete | null { + if (!this._searchQuery) { + throw new Error('onSearchCompleted must be called after a search is started'); + } + const options: IPatternInfo = objects.assign({}, this._searchQuery.contentPattern); delete options.pattern; @@ -930,8 +983,8 @@ export class SearchModel extends Disposable { } private onSearchProgress(p: ISearchProgressItem): void { - if (p.resource) { - this._searchResult.add([p], true); + if ((p).resource) { + this._searchResult.add([p], true); } } @@ -1001,7 +1054,7 @@ export class RangeHighlightDecorations implements IDisposable { } public highlightRange(resource: URI | ITextModel, range: Range, ownerId: number = 0): void { - let model: ITextModel; + let model: ITextModel | null; if (URI.isUri(resource)) { model = this._modelService.getModel(resource); } else { diff --git a/src/vs/workbench/parts/search/test/browser/openFileHandler.test.ts b/src/vs/workbench/parts/search/test/browser/openFileHandler.test.ts index ced5dcf2132..df2516f3941 100644 --- a/src/vs/workbench/parts/search/test/browser/openFileHandler.test.ts +++ b/src/vs/workbench/parts/search/test/browser/openFileHandler.test.ts @@ -180,7 +180,8 @@ suite('CacheState', () => { private _awaitDisposal: (() => void)[][] = []; public baseQuery: IFileQuery = { - type: QueryType.File + type: QueryType.File, + folderQueries: [] }; public query(cacheKey: string): IFileQuery { diff --git a/src/vs/workbench/services/search/node/searchService.ts b/src/vs/workbench/services/search/node/searchService.ts index 26ca82eeecb..bf9bdcb9de9 100644 --- a/src/vs/workbench/services/search/node/searchService.ts +++ b/src/vs/workbench/services/search/node/searchService.ts @@ -521,7 +521,7 @@ export class DiskSearch implements ISearchResultProvider { let event: Event; event = this.raw.fileSearch(query); - const onProgress = (p: ISearchProgressItem) => { + const onProgress = (p: IProgress) => { if (p.message) { // Should only be for logs this.logService.debug('SearchService#search', p.message); diff --git a/src/vs/workbench/services/search/test/common/searchHelpers.test.ts b/src/vs/workbench/services/search/test/common/searchHelpers.test.ts index 758ccd272d1..3bd14e02209 100644 --- a/src/vs/workbench/services/search/test/common/searchHelpers.test.ts +++ b/src/vs/workbench/services/search/test/common/searchHelpers.test.ts @@ -73,6 +73,7 @@ suite('SearchHelpers', () => { function getQuery(beforeContext?: number, afterContext?: number): ITextQuery { return { + folderQueries: [], type: QueryType.Text, contentPattern: { pattern: 'test' }, beforeContext, -- GitLab