diff --git a/package/src/handler.test.ts b/package/src/handler.test.ts index 1b9f420f4..1023a9d00 100644 --- a/package/src/handler.test.ts +++ b/package/src/handler.test.ts @@ -1,103 +1,76 @@ import * as assert from 'assert' -import { Handler } from './handler' -import { Result } from './api' -import { Position } from 'sourcegraph' - -interface SearchTest { - crossRepo?: boolean - doc: { - uri: string - text: string - } - expSearchQueries: string[] -} +import { referencesQueries, definitionQueries } from './handler' +import { TextDocument } from 'sourcegraph' describe('search requests', () => { it('makes correct search requests for goto definition', async () => { - const tests: SearchTest[] = [ - { - crossRepo: undefined, - doc: { - uri: 'git://github.com/foo/bar?rev#file.c', - text: 'token', - }, - expSearchQueries: [ - '\\btoken\\b case:yes file:.(h|c|hpp|cpp|m|cc)$ type:file', - ], - }, - { - crossRepo: true, - doc: { - uri: 'git://github.com/foo/bar?rev#file.c', - text: 'token', - }, - expSearchQueries: [ - '\\btoken\\b case:yes file:.(h|c|hpp|cpp|m|cc)$ type:symbol', - '\\btoken\\b case:yes file:.(h|c|hpp|cpp|m|cc)$ type:file', - ], - }, + interface DefinitionTest { + doc: TextDocument + definitionPatterns?: string[] + expectedSearchQueries: string[] + } + const tests: DefinitionTest[] = [ { - crossRepo: false, doc: { - uri: 'git://github.com/foo/bar?rev#file.c', + uri: 'git://github.com/foo/bar?rev#file.cpp', + languageId: 'cpp', text: 'token', }, - expSearchQueries: [ - '\\btoken\\b case:yes file:.(h|c|hpp|cpp|m|cc)$ type:symbol repo:^github.com/foo/bar$@rev', - '\\btoken\\b case:yes file:.(h|c|hpp|cpp|m|cc)$ type:file', + definitionPatterns: ['const\\s%s\\s='], + expectedSearchQueries: [ + // current file + 'const\\stoken\\s= case:yes file:.(cpp)$ type:file repo:^github.com/foo/bar$@rev file:^file.cpp$', + // current repo symbols + '^token$ case:yes file:.(cpp)$ type:symbol repo:^github.com/foo/bar$@rev', + // current repo definition patterns + 'const\\stoken\\s= case:yes file:.(cpp)$ type:file repo:^github.com/foo/bar$@rev', + // all repos definition patterns + 'const\\stoken\\s= case:yes file:.(cpp)$ type:file', ], }, ] for (const test of tests) { - const h = new Handler({ languageID: 'l' }) - const searchQueries: string[] = [] - h.api.search = (searchQuery: string): Promise => { - searchQueries.push(searchQuery) - return Promise.resolve([]) - } - await h.definition( - { - uri: test.doc.uri, - languageId: 'l', - text: test.doc.text, - }, - { line: 0, character: 0 } as Position + assert.deepStrictEqual( + definitionQueries({ + searchToken: 'token', + doc: test.doc, + fileExts: ['cpp'], + definitionPatterns: test.definitionPatterns || [], + }), + test.expectedSearchQueries ) - assert.deepStrictEqual(test.expSearchQueries, searchQueries) } }) it('makes correct search requests for references', async () => { - const tests: SearchTest[] = [ + interface ReferencesTest { + doc: TextDocument + expectedSearchQueries: string[] + } + const tests: ReferencesTest[] = [ { doc: { - uri: 'git://github.com/foo/bar?rev#file.c', + uri: 'git://github.com/foo/bar?rev#file.cpp', + languageId: 'cpp', text: 'token', }, - expSearchQueries: [ - '\\btoken\\b case:yes file:.(h|c|hpp|cpp|m|cc)$ type:file repo:^github.com/foo/bar$@rev', - '\\btoken\\b case:yes file:.(h|c|hpp|cpp|m|cc)$ type:file -repo:^github.com/foo/bar$', + expectedSearchQueries: [ + '\\btoken\\b case:yes file:.(cpp)$ type:file repo:^github.com/foo/bar$@rev', + '\\btoken\\b case:yes file:.(cpp)$ type:file -repo:^github.com/foo/bar$', ], }, ] for (const test of tests) { - const h = new Handler({ languageID: 'l' }) - const searchQueries: string[] = [] - h.api.search = (searchQuery: string): Promise => { - searchQueries.push(searchQuery) - return Promise.resolve([]) - } - await h.references( - { - uri: test.doc.uri, - languageId: 'l', - text: test.doc.text, - }, - { line: 0, character: 0 } as Position + assert.deepStrictEqual( + referencesQueries({ + searchToken: 'token', + doc: test.doc, + fileExts: ['cpp'], + }), + test.expectedSearchQueries ) - assert.deepStrictEqual(test.expSearchQueries, searchQueries) } }) }) diff --git a/package/src/handler.ts b/package/src/handler.ts index 8219bf312..0ee33de2e 100644 --- a/package/src/handler.ts +++ b/package/src/handler.ts @@ -1,6 +1,6 @@ import { API, Result, parseUri } from './api' import * as sprintf from 'sprintf-js' -import { takeWhile, dropWhile, sortBy } from 'lodash' +import { takeWhile, dropWhile, sortBy, flatten } from 'lodash' import { DocumentSelector, Location, @@ -263,6 +263,72 @@ function sortByProximity({ }) } +export function definitionQueries({ + searchToken, + doc, + fileExts, + definitionPatterns, +}: { + searchToken: string + doc: TextDocument + fileExts: string[] + definitionPatterns: string[] +}): string[] { + const patternQuery = ( + scope: Scope, + patterns: string[] + ): string | undefined => { + return patterns.length === 0 + ? undefined + : makeQuery({ + searchToken: patterns + .map(pattern => + sprintf.sprintf(`${pattern}`, searchToken) + ) + .join('|'), + searchType: 'file', + currentFileUri: doc.uri, + scope, + fileExts, + }) + } + + return [ + patternQuery('current file', definitionPatterns), + makeQuery({ + searchToken: `^${searchToken}$`, + searchType: 'symbol', + currentFileUri: doc.uri, + scope: 'current repository', + fileExts, + }), + patternQuery('current repository', definitionPatterns), + patternQuery('all repositories', definitionPatterns), + ].filter((query): query is string => Boolean(query)) +} + +export function referencesQueries({ + searchToken, + doc, + fileExts, +}: { + searchToken: string + doc: TextDocument + fileExts: string[] +}): string[] { + const from = (scope: Scope): string => + makeQuery({ + searchToken: `\\b${searchToken}\\b`, + searchType: 'file', + currentFileUri: doc.uri, + scope, + fileExts, + }) + // ⚠️ This CANNOT be simplified to `[from('all repositories')]` because + // searches that span all repositories always fail on Sourcegraph.com. + return [from('current repository'), from('other repositories')] +} + /** * @see package.json contributes.configuration section for the configuration schema. */ @@ -582,39 +648,12 @@ export class Handler { } const searchToken = tokenResult.searchToken - const patternQuery = ( - scope: Scope, - patterns: string[] - ): string | undefined => { - return patterns.length === 0 - ? undefined - : makeQuery({ - searchToken: patterns - .map(pattern => - sprintf.sprintf(`${pattern}`, searchToken) - ) - .join('|'), - searchType: 'file', - currentFileUri: doc.uri, - scope, - fileExts: this.fileExts, - }) - } - - const queries = [ - patternQuery('current file', this.definitionPatterns), - makeQuery({ - searchToken: `^${searchToken}$`, - searchType: 'symbol', - currentFileUri: doc.uri, - scope: 'current repository', - fileExts: this.fileExts, - }), - patternQuery('current repository', this.definitionPatterns), - patternQuery('all repositories', this.definitionPatterns), - ].filter((query): query is string => Boolean(query)) - - for (const query of queries) { + for (const query of definitionQueries({ + searchToken, + doc, + fileExts: this.fileExts, + definitionPatterns: this.definitionPatterns, + })) { const symbolResults = (await this.api.search(query)).map(result => resultToLocation({ result, sourcegraph: this.sourcegraph }) ) @@ -647,25 +686,19 @@ export class Handler { } const searchToken = tokenResult.searchToken - const referencesFrom = async (scope: Scope): Promise => - (await this.api.search( - makeQuery({ - searchToken: `\\b${searchToken}\\b`, - searchType: 'file', - currentFileUri: doc.uri, - scope, - fileExts: this.fileExts, - }) - )).map(result => - resultToLocation({ result, sourcegraph: this.sourcegraph }) - ) - return sortByProximity({ currentLocation: doc.uri, - locations: [ - ...(await referencesFrom('current repository')), - ...(await referencesFrom('other repositories')), - ], + locations: flatten( + await Promise.all( + referencesQueries({ + searchToken, + doc, + fileExts: this.fileExts, + }).map(query => this.api.search(query)) + ) + ).map(result => + resultToLocation({ result, sourcegraph: this.sourcegraph }) + ), }) } }