From c58853ff34d43cd974fee0dc16f3afba2ffad678 Mon Sep 17 00:00:00 2001 From: Yuya Tanaka Date: Fri, 30 Dec 2016 08:10:07 +0900 Subject: [PATCH 1/6] Refactor and increase infos in locate.js --- locate/locate.js | 73 ++++++++++++++++++++++-------------------------- package.json | 1 + 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/locate/locate.js b/locate/locate.js index 19ada5fb8..f42264648 100644 --- a/locate/locate.js +++ b/locate/locate.js @@ -3,24 +3,28 @@ const locator = require('ruby-method-locate'), minimatch = require('minimatch'); const fs = require('fs'), path = require('path'); +const _ = require('lodash'); -function flatten(tree, result) { - let t; - for (let a in tree) { - t = tree[a]; - if (typeof t === 'string' || typeof t === 'number') continue; - if (t.posn) { - if (a in result) result[a].push({ - line: t.posn.line, - char: t.posn.char - }); - else result[a] = [{ - line: t.posn.line, - char: t.posn.char - }]; - } - flatten(t, result); - } +function flatten(locateInfo, file, parentName) { + return _.flatMap(locateInfo, (symbols, type) => { + // TODO: return accessor or other types. + // TODO: parse include and inherit to find inherited symbols. + if (!_.includes(['module', 'class', 'method', 'classMethod'], type)) return []; + return _.flatMap(symbols, (children, name) => { + const sep = { method: '#', classMethod: '.' }[type] || '::'; + const posn = children.posn || { line: 0, char: 0 }; + const fullName = parentName ? `${parentName}${sep}${name}` : name; + const symbolInfo = { + // TODO: parse names like 'ActiveRecord::Base' + name: name, + file: file, + line: posn.line, + char: posn.char, + fullName: fullName + }; + return [symbolInfo].concat(flatten(children, file, fullName)); + }); + }); } module.exports = class Locate { constructor(root, settings) { @@ -34,26 +38,16 @@ module.exports = class Locate { // add lookup hooks } find(name) { - let result = []; - let tree; - for (let file in this.tree) { - tree = this.tree[file]; - //jshint -W083 - const extract = obj => ({ - file: file, - line: obj.line, - char: obj.char - }); - //jshint +W083 - for (let n in tree) { - // because our word pattern is designed to match symbols - // things like Gem::RequestSet may request a search for ':RequestSet' - if (n === name || n === name + '=' || ':' + n === name) { - result = result.concat(tree[n].map(extract)); - } - } - } - return result; + // because our word pattern is designed to match symbols + // things like Gem::RequestSet may request a search for ':RequestSet' + const escapedName = _.escapeRegExp(_.trimStart(name, ':')); + const regexp = new RegExp(`^${escapedName}=?\$`); + return _(this.tree) + .values() + .flatten() + .filter(symbol => regexp.test(symbol.name)) + .map(_.clone) + .value(); } rm(absPath) { if (absPath in this.tree) delete this.tree[absPath]; @@ -66,8 +60,7 @@ module.exports = class Locate { locator(absPath) .then(result => { if (!result) return; - this.tree[absPath] = {}; - flatten(result, this.tree[absPath]); + this.tree[absPath] = flatten(result, absPath); }, err => { if (err.code === 'EMFILE') { // if there are too many open files @@ -97,4 +90,4 @@ module.exports = class Locate { }); }); } -}; \ No newline at end of file +}; diff --git a/package.json b/package.json index a5a5f0475..90b56979f 100644 --- a/package.json +++ b/package.json @@ -45,6 +45,7 @@ "vscode-debugprotocol": "~1.6.0-pre4", "vscode-debugadapter": "~1.6.0-pre8", "xmldom": "^0.1.19", + "lodash": "^4.17.3", "minimatch": "^3.0.3" }, "devDependencies": { From f5f9ccb45e89e52873d6a2cb3c6d01465f954961 Mon Sep 17 00:00:00 2001 From: Yuya Tanaka Date: Sat, 31 Dec 2016 01:41:41 +0900 Subject: [PATCH 2/6] Implement DocumentSymbolProvider --- locate/locate.js | 26 +++++++++++++++++--------- ruby.js | 25 ++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/locate/locate.js b/locate/locate.js index f42264648..1abf5e5a0 100644 --- a/locate/locate.js +++ b/locate/locate.js @@ -5,24 +5,29 @@ const fs = require('fs'), path = require('path'); const _ = require('lodash'); -function flatten(locateInfo, file, parentName) { +const DECLARATION_TYPES = ['class', 'module', 'method', 'classMethod']; + +function flatten(locateInfo, file, parent) { return _.flatMap(locateInfo, (symbols, type) => { - // TODO: return accessor or other types. - // TODO: parse include and inherit to find inherited symbols. - if (!_.includes(['module', 'class', 'method', 'classMethod'], type)) return []; - return _.flatMap(symbols, (children, name) => { + if (!_.includes(DECLARATION_TYPES, type)) { + // Skip top-level include or posn property etc. + return []; + } + return _.flatMap(symbols, (inner, name) => { const sep = { method: '#', classMethod: '.' }[type] || '::'; - const posn = children.posn || { line: 0, char: 0 }; - const fullName = parentName ? `${parentName}${sep}${name}` : name; + const posn = inner.posn || { line: 0, char: 0 }; + const fullName = parent ? `${parent.fullName}${sep}${name}` : name; const symbolInfo = { - // TODO: parse names like 'ActiveRecord::Base' name: name, + type: type, file: file, line: posn.line, char: posn.char, + parent: parent, fullName: fullName }; - return [symbolInfo].concat(flatten(children, file, fullName)); + _.extend(symbolInfo, _.omit(inner, DECLARATION_TYPES)); + return [symbolInfo].concat(flatten(inner, file, symbolInfo)); }); }); } @@ -37,6 +42,9 @@ module.exports = class Locate { // always: do this file now (if it's in the tree) // add lookup hooks } + listInFile(absPath) { + return _.clone(this.tree[absPath] || []); + } find(name) { // because our word pattern is designed to match symbols // things like Gem::RequestSet may request a search for ':RequestSet' diff --git a/ruby.js b/ruby.js index 6b5292001..6c3680bcf 100644 --- a/ruby.js +++ b/ruby.js @@ -1,5 +1,6 @@ "use strict"; -let vscode = require('vscode'); +const vscode = require('vscode'); +const { Location, Position, SymbolKind, SymbolInformation } = vscode; let Locate = require('./locate/locate'); let cp = require('child_process'); @@ -177,6 +178,28 @@ function activate(context) { } }; subs.push(vscode.languages.registerDefinitionProvider(['ruby', 'erb'], defProvider)); + const symbolKindTable = { + class: () => SymbolKind.Class, + module: () => SymbolKind.Module, + method: symbolInfo => symbolInfo.name === 'initialize' ? SymbolKind.Constructor : SymbolKind.Method, + classMethod: () => SymbolKind.Method, + }; + const defaultSymbolKind = symbolInfo => { + console.warn(`Unknown symbol type: ${symbolInfo.type}`); + return SymbolKind.Variable; + }; + const docSymbolProvider = { + provideDocumentSymbols: (document, token) => { + return locate.listInFile(document.fileName).map(match => { + const symbolKind = (symbolKindTable[match.type] || defaultSymbolKind)(match); + const parentName = match.parent ? match.parent.fullName : ''; + const uri = vscode.Uri.file(match.file); + const location = new Location(uri, new Position(match.line, match.char)); + return new SymbolInformation(match.name, symbolKind, parentName, location); + }) + } + }; + subs.push(vscode.languages.registerDocumentSymbolProvider(['ruby', 'erb'], docSymbolProvider)); } subs.push(vscode.window.onDidChangeActiveTextEditor(balanceEvent)); From e3b253f11b62789886fdcff36146019178c9f29c Mon Sep 17 00:00:00 2001 From: Yuya Tanaka Date: Sat, 31 Dec 2016 02:01:08 +0900 Subject: [PATCH 3/6] Parse file ahead when document symbol provider is used --- locate/locate.js | 10 +++++----- ruby.js | 15 ++++++++------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/locate/locate.js b/locate/locate.js index 1abf5e5a0..a3792d5a2 100644 --- a/locate/locate.js +++ b/locate/locate.js @@ -43,7 +43,8 @@ module.exports = class Locate { // add lookup hooks } listInFile(absPath) { - return _.clone(this.tree[absPath] || []); + const waitForParse = (absPath in this.tree) ? Promise.resolve() : this.parse(absPath); + return waitForParse.then(() => _.clone(this.tree[absPath] || [])); } find(name) { // because our word pattern is designed to match symbols @@ -64,11 +65,9 @@ module.exports = class Locate { const relPath = path.relative(this.root, absPath); if (this.settings.exclude && minimatch(relPath, this.settings.exclude)) return; if (this.settings.include && !minimatch(relPath, this.settings.include)) return; - this.rm(absPath); - locator(absPath) + return locator(absPath) .then(result => { - if (!result) return; - this.tree[absPath] = flatten(result, absPath); + this.tree[absPath] = result ? flatten(result, absPath) : []; }, err => { if (err.code === 'EMFILE') { // if there are too many open files @@ -77,6 +76,7 @@ module.exports = class Locate { } else { // otherwise, report it console.log(err); + this.rm(absPath); } }); } diff --git a/ruby.js b/ruby.js index 6c3680bcf..83f4be5fc 100644 --- a/ruby.js +++ b/ruby.js @@ -190,13 +190,14 @@ function activate(context) { }; const docSymbolProvider = { provideDocumentSymbols: (document, token) => { - return locate.listInFile(document.fileName).map(match => { - const symbolKind = (symbolKindTable[match.type] || defaultSymbolKind)(match); - const parentName = match.parent ? match.parent.fullName : ''; - const uri = vscode.Uri.file(match.file); - const location = new Location(uri, new Position(match.line, match.char)); - return new SymbolInformation(match.name, symbolKind, parentName, location); - }) + return locate.listInFile(document.fileName) + .then(matches => matches.map(match => { + const symbolKind = (symbolKindTable[match.type] || defaultSymbolKind)(match); + const parentName = match.parent ? match.parent.fullName : ''; + const uri = vscode.Uri.file(match.file); + const location = new Location(uri, new Position(match.line, match.char)); + return new SymbolInformation(match.name, symbolKind, parentName, location); + })); } }; subs.push(vscode.languages.registerDocumentSymbolProvider(['ruby', 'erb'], docSymbolProvider)); From eb929e5367b9d9bc2c9da6be3658bf1b80a59ddc Mon Sep 17 00:00:00 2001 From: Yuya Tanaka Date: Sat, 31 Dec 2016 04:55:48 +0900 Subject: [PATCH 4/6] Implement WorkspaceSymbolProvider --- locate/locate.js | 49 ++++++++++++++++++++++++++++++++++++++++++------ ruby.js | 21 +++++++++++++-------- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/locate/locate.js b/locate/locate.js index a3792d5a2..cd0d9a9e8 100644 --- a/locate/locate.js +++ b/locate/locate.js @@ -7,30 +7,55 @@ const _ = require('lodash'); const DECLARATION_TYPES = ['class', 'module', 'method', 'classMethod']; -function flatten(locateInfo, file, parent) { +function flatten(locateInfo, file, containerName = '') { return _.flatMap(locateInfo, (symbols, type) => { if (!_.includes(DECLARATION_TYPES, type)) { // Skip top-level include or posn property etc. return []; } return _.flatMap(symbols, (inner, name) => { - const sep = { method: '#', classMethod: '.' }[type] || '::'; const posn = inner.posn || { line: 0, char: 0 }; - const fullName = parent ? `${parent.fullName}${sep}${name}` : name; + // TODO: parse name with multiple segments, e.g. File.read or ActiveRecord::Base, if necessary. const symbolInfo = { name: name, type: type, file: file, line: posn.line, char: posn.char, - parent: parent, - fullName: fullName + containerName: containerName || '' }; _.extend(symbolInfo, _.omit(inner, DECLARATION_TYPES)); - return [symbolInfo].concat(flatten(inner, file, symbolInfo)); + const sep = { method: '#', classMethod: '.' }[type] || '::'; + const fullName = containerName ? `${containerName}${sep}${name}` : name; + return [symbolInfo].concat(flatten(inner, file, fullName)); }); }); } +function camelCaseRegExp(query) { + const escaped = _.escapeRegExp(query) + const prefix = escaped.charAt(0); + return new RegExp( + `[${prefix.toLowerCase()}${prefix.toUpperCase()}]` + + escaped.slice(1).replace(/[A-Z]|([a-z])/g, (char, lower) => { + if (lower) return `[${char.toUpperCase()}${char}]`; + const lowered = char.toLowerCase() + return `.*(?:${char}|_${lowered})`; + }) + ); +} +function filter(symbols, query, stringProvider) { + // TODO: Ask MS to expose or separate matchesFuzzy method. + // https://github.com/Microsoft/vscode/blob/a1d3c8a3006d0a3d68495122ea09a2a37bca69db/src/vs/base/common/filters.ts + const isLowerCase = (query.toLowerCase() === query) + const exact = new RegExp('^' + _.escapeRegExp(query) + '$', 'i'); + const prefix = new RegExp('^' + _.escapeRegExp(query), 'i'); + const substring = new RegExp(_.escapeRegExp(query), isLowerCase ? 'i' : ''); + const camelCase = camelCaseRegExp(query); + return _([exact, prefix, substring, camelCase]) + .flatMap(regexp => symbols.filter(symbolInfo => regexp.test(stringProvider(symbolInfo)))) + .uniq() + .value(); +} module.exports = class Locate { constructor(root, settings) { this.settings = settings; @@ -58,6 +83,18 @@ module.exports = class Locate { .map(_.clone) .value(); } + query(query) { + const match = query.match(/^(?:(.*)[.#])?([^.#]*)$/); + const containerQuery = match[1]; + const nameQuery = match[2]; + if (!nameQuery) return []; + + const symbols = _(this.tree).values().flatten().value(); + const matchedSymbols = filter(symbols, nameQuery, symbolInfo => symbolInfo.name); + if (!containerQuery) return matchedSymbols; + + return filter(matchedSymbols, containerQuery, symbolInfo => symbolInfo.containerName); + } rm(absPath) { if (absPath in this.tree) delete this.tree[absPath]; } diff --git a/ruby.js b/ruby.js index 83f4be5fc..b5aa816eb 100644 --- a/ruby.js +++ b/ruby.js @@ -188,19 +188,24 @@ function activate(context) { console.warn(`Unknown symbol type: ${symbolInfo.type}`); return SymbolKind.Variable; }; + const symbolConverter = matches => matches.map(match => { + const symbolKind = (symbolKindTable[match.type] || defaultSymbolKind)(match); + const uri = vscode.Uri.file(match.file); + const location = new Location(uri, new Position(match.line, match.char)); + return new SymbolInformation(match.name, symbolKind, match.containerName, location); + }); const docSymbolProvider = { provideDocumentSymbols: (document, token) => { - return locate.listInFile(document.fileName) - .then(matches => matches.map(match => { - const symbolKind = (symbolKindTable[match.type] || defaultSymbolKind)(match); - const parentName = match.parent ? match.parent.fullName : ''; - const uri = vscode.Uri.file(match.file); - const location = new Location(uri, new Position(match.line, match.char)); - return new SymbolInformation(match.name, symbolKind, parentName, location); - })); + return locate.listInFile(document.fileName).then(symbolConverter); } }; subs.push(vscode.languages.registerDocumentSymbolProvider(['ruby', 'erb'], docSymbolProvider)); + const workspaceSymbolProvider = { + provideWorkspaceSymbols: (query, token) => { + return symbolConverter(locate.query(query)); + } + }; + subs.push(vscode.languages.registerWorkspaceSymbolProvider(workspaceSymbolProvider)); } subs.push(vscode.window.onDidChangeActiveTextEditor(balanceEvent)); From f76af29e061a5c892abf69421c79e6a172a4d88b Mon Sep 17 00:00:00 2001 From: Yuya Tanaka Date: Sat, 31 Dec 2016 05:51:36 +0900 Subject: [PATCH 5/6] Match both whole name and container name --- locate/locate.js | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/locate/locate.js b/locate/locate.js index cd0d9a9e8..96498f4f0 100644 --- a/locate/locate.js +++ b/locate/locate.js @@ -15,7 +15,6 @@ function flatten(locateInfo, file, containerName = '') { } return _.flatMap(symbols, (inner, name) => { const posn = inner.posn || { line: 0, char: 0 }; - // TODO: parse name with multiple segments, e.g. File.read or ActiveRecord::Base, if necessary. const symbolInfo = { name: name, type: type, @@ -43,7 +42,7 @@ function camelCaseRegExp(query) { }) ); } -function filter(symbols, query, stringProvider) { +function filter(symbols, query, matcher) { // TODO: Ask MS to expose or separate matchesFuzzy method. // https://github.com/Microsoft/vscode/blob/a1d3c8a3006d0a3d68495122ea09a2a37bca69db/src/vs/base/common/filters.ts const isLowerCase = (query.toLowerCase() === query) @@ -52,7 +51,7 @@ function filter(symbols, query, stringProvider) { const substring = new RegExp(_.escapeRegExp(query), isLowerCase ? 'i' : ''); const camelCase = camelCaseRegExp(query); return _([exact, prefix, substring, camelCase]) - .flatMap(regexp => symbols.filter(symbolInfo => regexp.test(stringProvider(symbolInfo)))) + .flatMap(regexp => symbols.filter(symbolInfo => matcher(symbolInfo, regexp))) .uniq() .value(); } @@ -84,16 +83,28 @@ module.exports = class Locate { .value(); } query(query) { - const match = query.match(/^(?:(.*)[.#])?([^.#]*)$/); - const containerQuery = match[1]; - const nameQuery = match[2]; - if (!nameQuery) return []; - + const segmentMatch = query.match(/^(?:([^.#:]+)(.|#|::))([^.#:]+)$/) || []; + const containerQuery = segmentMatch[1]; + const separator = segmentMatch[2]; + const nameQuery = segmentMatch[3]; + const separatorToTypesTable = { + '.': ['classMethod', 'method'], + '#': ['method'], + '::': ['class', 'module'], + }; + const segmentTypes = separatorToTypesTable[separator]; const symbols = _(this.tree).values().flatten().value(); - const matchedSymbols = filter(symbols, nameQuery, symbolInfo => symbolInfo.name); - if (!containerQuery) return matchedSymbols; - return filter(matchedSymbols, containerQuery, symbolInfo => symbolInfo.containerName); + // query whole name (matches `class Foo::Bar` or `def File.read`) + const plainMatches = filter(symbols, query, (symbolInfo, regexp) => regexp.test(symbolInfo.name)); + if (!containerQuery) return plainMatches; + + // query name and containerName separatedly (matches `def foo` in `class Bar`) + const nameMatches = filter(symbols, nameQuery, (symbolInfo, regexp) => { + return _.includes(segmentTypes, symbolInfo.type) && regexp.test(symbolInfo.name); + }); + const containerMatches = filter(nameMatches, containerQuery, (symbolInfo, regexp) => regexp.test(symbolInfo.containerName)) + return _.uniq(plainMatches.concat(containerMatches)); } rm(absPath) { if (absPath in this.tree) delete this.tree[absPath]; From 0f4e1bb7fa6b1821b94d942cd045250a2c3123c6 Mon Sep 17 00:00:00 2001 From: Yuya Tanaka Date: Sat, 31 Dec 2016 06:00:47 +0900 Subject: [PATCH 6/6] Limit num of symbols returned to workaround high CPU usage --- ruby.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ruby.js b/ruby.js index b5aa816eb..278503853 100644 --- a/ruby.js +++ b/ruby.js @@ -188,7 +188,10 @@ function activate(context) { console.warn(`Unknown symbol type: ${symbolInfo.type}`); return SymbolKind.Variable; }; - const symbolConverter = matches => matches.map(match => { + // NOTE: Workaround for high CPU usage on IPC (channel.onread) when too many symbols returned. + // For channel.onread see issue like this: https://github.com/Microsoft/vscode/issues/6026 + const numOfSymbolLimit = 3000; + const symbolConverter = matches => matches.slice(0, numOfSymbolLimit).map(match => { const symbolKind = (symbolKindTable[match.type] || defaultSymbolKind)(match); const uri = vscode.Uri.file(match.file); const location = new Location(uri, new Position(match.line, match.char));