Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Commit e3d7ebc

Browse files
damiengdamieng
authored
Merge pull request #95 from atom/dg-outline-view-supports-duplicate-names
Handle and resolve duplicate named containers
2 parents 4c4a83d + 5bed4e9 commit e3d7ebc

File tree

4 files changed

+238
-46
lines changed

4 files changed

+238
-46
lines changed

‎lib/adapters/code-format-adapter.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ export default class CodeFormatAdapter {
146146
//
147147
// Returns an {Array} of Atom {TextEdit} objects.
148148
static convertLsTextEdits(textEdits: Array<TextEdit>): Array<atomIde$TextEdit> {
149-
return textEdits.map(CodeFormatAdapter.convertLsTextEdit);
149+
return (textEdits || []).map(CodeFormatAdapter.convertLsTextEdit);
150150
}
151151

152152
// Public: Convert a language server protocol {TextEdit} object to the

‎lib/adapters/outline-view-adapter.js

Lines changed: 70 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,6 @@ import {LanguageClientConnection, SymbolKind, type ServerCapabilities, type Symb
44
import Convert from '../convert';
55
import {Point} from 'atom';
66

7-
type ContainerNamedOutline = {
8-
containerName: ?string,
9-
outline: atomIde$OutlineTree,
10-
};
11-
127
// Public: Adapts the documentSymbolProvider of the language server to the Outline View
138
// supplied by Atom IDE UI.
149
export default class OutlineViewAdapter {
@@ -49,52 +44,82 @@ export default class OutlineViewAdapter {
4944
//
5045
// Returns an {OutlineTree} containing the given symbols that the Outline View can display.
5146
static createOutlineTrees(symbols: Array<SymbolInformation>): Array<atomIde$OutlineTree> {
52-
const byContainerName = OutlineViewAdapter.createContainerNamedOutline(symbols);
53-
const roots: Map<string, atomIde$OutlineTree> = new Map();
54-
byContainerName.forEach((v, k) => {
55-
const containerName = v.containerName;
56-
if (containerName === '' || containerName == null || k === containerName) {
57-
// No container name or contained within itself belong as top-level items
58-
roots.set(k, v.outline);
59-
} else {
60-
const container = byContainerName.get(containerName);
61-
if (container) {
62-
// Items with a container we know about get put in that container
63-
container.outline.children.push(v.outline);
47+
// Temporarily keep containerName through the conversion process
48+
const allItems = symbols.map(symbol => ({
49+
containerName: symbol.containerName,
50+
outline: OutlineViewAdapter.symbolToOutline(symbol),
51+
}));
52+
53+
// Create a map of containers by name with all items that have that name
54+
const containers = allItems.reduce((map, item) => {
55+
const name = item.outline.representativeName;
56+
if (name != null) {
57+
const container = map.get(name);
58+
if (container == null) {
59+
map.set(name, [item.outline]);
6460
} else {
65-
// Items with a container we don't know about we dynamically create one for
66-
let rootContainer = roots.get(containerName);
67-
if (rootContainer == null) {
68-
rootContainer = {
69-
plainText: containerName,
70-
children: [],
71-
startPosition: new Point(0, 0),
72-
};
73-
roots.set(containerName, rootContainer);
61+
container.push(item.outline);
62+
}
63+
}
64+
return map;
65+
}, new Map());
66+
67+
const roots: Array<atomIde$OutlineTree> = [];
68+
69+
// Put each item within its parent and extract out the roots
70+
for (const item of allItems) {
71+
const containerName = item.containerName;
72+
const child = item.outline;
73+
if (containerName == null || containerName === '') {
74+
roots.push(item.outline);
75+
} else {
76+
const possibleParents = containers.get(containerName);
77+
let closestParent = OutlineViewAdapter._getClosestParent(possibleParents, child);
78+
if (closestParent == null) {
79+
closestParent = {
80+
plainText: containerName,
81+
representativeName: containerName,
82+
startPosition: new Point(0, 0),
83+
children: [child],
84+
};
85+
roots.push(closestParent);
86+
if (possibleParents == null) {
87+
containers.set(containerName, [closestParent]);
88+
} else {
89+
possibleParents.push(closestParent);
7490
}
75-
rootContainer.children.push(v.outline);
91+
} else {
92+
closestParent.children.push(child);
7693
}
7794
}
78-
});
79-
return Array.from(roots.values());
95+
}
96+
97+
return roots;
8098
}
8199

82-
// Public: Converts an {Array} of {SymbolInformation} received from a language server into a
83-
// {Map} of {ContainerNamedOutline} keyed by their symbol names. This allows us to find parents
84-
// quickly when assembling the tree through containerNames.
85-
//
86-
// * `symbols` An {Array} of {SymbolInformation}s to convert into the map.
87-
//
88-
// Returns a {Map} of {ContainerNamedOutline}s each converted from a {SymbolInformation}
89-
// and keyed by its containerName.
90-
static createContainerNamedOutline(symbols: Array<SymbolInformation>): Map<string, ContainerNamedOutline> {
91-
return symbols.reduce((map, symbol) => {
92-
map.set(symbol.name, {
93-
containerName: symbol.containerName,
94-
outline: OutlineViewAdapter.symbolToOutline(symbol),
95-
});
96-
return map;
97-
}, new Map());
100+
static _getClosestParent(candidates: ?Array<atomIde$OutlineTree>, child: atomIde$OutlineTree): ?atomIde$OutlineTree {
101+
if (candidates == null || candidates.length === 0) {
102+
return null;
103+
}
104+
105+
let parent = null;
106+
for (const candidate of candidates) {
107+
if (
108+
candidate !== child &&
109+
candidate.startPosition.isLessThanOrEqual(child.startPosition) &&
110+
(candidate.endPosition == null || candidate.endPosition.isGreaterThanOrEqual(child.endPosition))
111+
) {
112+
if (
113+
parent == null ||
114+
(parent.startPosition.isLessThanOrEqual(candidate.startPosition) ||
115+
(parent.endPosition != null && parent.endPosition.isGreaterThanOrEqual(candidate.endPosition)))
116+
) {
117+
parent = candidate;
118+
}
119+
}
120+
}
121+
122+
return parent;
98123
}
99124

100125
// Public: Convert an individual {SymbolInformation} from the language server

‎test/adapters/datatip-adapter.test.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,13 @@ describe('DatatipAdapter', () => {
1313
let connection;
1414

1515
beforeEach(() => {
16+
global.sinon = sinon.sandbox.create();
1617
connection = new ls.LanguageClientConnection(createSpyConnection());
1718
fakeEditor = createFakeEditor();
1819
});
20+
afterEach(() => {
21+
global.sinon.restore();
22+
});
1923

2024
describe('canAdapt', () => {
2125
it('returns true if hoverProvider is supported', () => {

‎test/adapters/outline-view-adapter.test.js

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
// @flow
2+
3+
import OutlineViewAdapter from '../../lib/adapters/outline-view-adapter';
4+
import * as ls from '../../lib/languageclient';
5+
import sinon from 'sinon';
6+
import {expect} from 'chai';
7+
8+
describe('OutlineViewAdapter', () => {
9+
const createLocation = (a, b, c, d) => ({
10+
uri: '',
11+
range: {start: {line: a, character: b}, end: {line: c, character: d}},
12+
});
13+
14+
beforeEach(() => {
15+
global.sinon = sinon.sandbox.create();
16+
});
17+
afterEach(() => {
18+
global.sinon.restore();
19+
});
20+
21+
describe('canAdapt', () => {
22+
it('returns true if documentSymbolProvider is supported', () => {
23+
const result = OutlineViewAdapter.canAdapt({documentSymbolProvider: true});
24+
expect(result).to.be.true;
25+
});
26+
27+
it('returns false if documentSymbolProvider not supported', () => {
28+
const result = OutlineViewAdapter.canAdapt({});
29+
expect(result).to.be.false;
30+
});
31+
});
32+
33+
describe('createOutlineTrees', () => {
34+
it('creates an empty array given an empty array', () => {
35+
const result = OutlineViewAdapter.createOutlineTrees([]);
36+
expect(result).to.deep.equal([]);
37+
});
38+
39+
it('creates a single converted root item from a single source item', () => {
40+
const sourceItem = {kind: ls.SymbolKind.Namespace, name: 'R', location: createLocation(5, 6, 7, 8)};
41+
const expected = OutlineViewAdapter.symbolToOutline(sourceItem);
42+
const result = OutlineViewAdapter.createOutlineTrees([sourceItem]);
43+
expect(result).to.deep.equal([expected]);
44+
});
45+
46+
it('creates an empty root container with a single source item when containerName missing', () => {
47+
const sourceItem = {kind: ls.SymbolKind.Class, name: 'Program', location: createLocation(1, 2, 3, 4)};
48+
const expected = OutlineViewAdapter.symbolToOutline(sourceItem);
49+
const missingContainerSource = Object.assign(sourceItem, {containerName: 'missing'});
50+
const result = OutlineViewAdapter.createOutlineTrees([missingContainerSource]);
51+
expect(result.length).to.equal(1);
52+
expect(result[0].representativeName).to.equal('missing');
53+
expect(result[0].startPosition.row).to.equal(0);
54+
expect(result[0].startPosition.column).to.equal(0);
55+
expect(result[0].children).to.deep.equal([expected]);
56+
});
57+
58+
it('creates an empty root container with a single source item when containerName is missing and matches own name', () => {
59+
const sourceItem = {kind: ls.SymbolKind.Class, name: 'simple', location: createLocation(1, 2, 3, 4)};
60+
const expected = OutlineViewAdapter.symbolToOutline(sourceItem);
61+
const missingContainerSource = Object.assign(sourceItem, {containerName: 'simple'});
62+
const result = OutlineViewAdapter.createOutlineTrees([missingContainerSource]);
63+
expect(result.length).to.equal(1);
64+
expect(result[0].representativeName).to.equal('simple');
65+
expect(result[0].startPosition.row).to.equal(0);
66+
expect(result[0].startPosition.column).to.equal(0);
67+
expect(result[0].children).to.deep.equal([expected]);
68+
});
69+
70+
it('creates a simple named hierarchy', () => {
71+
const sourceItems = [
72+
{kind: ls.SymbolKind.Namespace, name: 'java.com', location: createLocation(1, 0, 10, 0)},
73+
{kind: ls.SymbolKind.Class, name: 'Program', location: createLocation(2, 0, 7, 0), containerName: 'java.com'},
74+
{kind: ls.SymbolKind.Function, name: 'main', location: createLocation(4, 0, 5, 0), containerName: 'Program'},
75+
];
76+
const result = OutlineViewAdapter.createOutlineTrees(sourceItems);
77+
expect(result.length).to.equal(1);
78+
expect(result[0].children.length).to.equal(1);
79+
expect(result[0].children[0].representativeName).to.equal('Program');
80+
expect(result[0].children[0].children.length).to.equal(1);
81+
expect(result[0].children[0].children[0].representativeName).to.equal('main');
82+
});
83+
84+
it('retains duplicate named items', () => {
85+
const sourceItems = [
86+
{kind: ls.SymbolKind.Namespace, name: 'duplicate', location: createLocation(1, 0, 5, 0)},
87+
{kind: ls.SymbolKind.Namespace, name: 'duplicate', location: createLocation(6, 0, 10, 0)},
88+
{kind: ls.SymbolKind.Function, name: 'main', location: createLocation(7, 0, 8, 0), containerName: 'duplicate'},
89+
];
90+
const result = OutlineViewAdapter.createOutlineTrees(sourceItems);
91+
expect(result.length).to.equal(2);
92+
expect(result[0].representativeName).to.equal('duplicate');
93+
expect(result[1].representativeName).to.equal('duplicate');
94+
});
95+
96+
it('disambiguates containerName based on range', () => {
97+
const sourceItems = [
98+
{kind: ls.SymbolKind.Namespace, name: 'duplicate', location: createLocation(1, 0, 5, 0)},
99+
{kind: ls.SymbolKind.Namespace, name: 'duplicate', location: createLocation(6, 0, 10, 0)},
100+
{kind: ls.SymbolKind.Function, name: 'main', location: createLocation(7, 0, 8, 0), containerName: 'duplicate'},
101+
];
102+
const result = OutlineViewAdapter.createOutlineTrees(sourceItems);
103+
expect(result[1].children.length).to.equal(1);
104+
expect(result[1].children[0].representativeName).to.equal('main');
105+
});
106+
107+
it("does not become it's own parent", () => {
108+
const sourceItems = [
109+
{kind: ls.SymbolKind.Namespace, name: 'duplicate', location: createLocation(1, 0, 10, 0)},
110+
{
111+
kind: ls.SymbolKind.Namespace,
112+
name: 'duplicate',
113+
location: createLocation(6, 0, 7, 0),
114+
containerName: 'duplicate',
115+
},
116+
];
117+
const result = OutlineViewAdapter.createOutlineTrees(sourceItems);
118+
expect(result.length).to.equal(1);
119+
const r = (result: any);
120+
expect(r[0].endPosition.row).to.equal(10);
121+
expect(r[0].children.length).to.equal(1);
122+
expect(r[0].children[0].endPosition.row).to.equal(7);
123+
});
124+
125+
it('parents to the innnermost named container', () => {
126+
const sourceItems = [
127+
{kind: ls.SymbolKind.Namespace, name: 'turtles', location: createLocation(1, 0, 10, 0)},
128+
{
129+
kind: ls.SymbolKind.Namespace,
130+
name: 'turtles',
131+
location: createLocation(4, 0, 8, 0),
132+
containerName: 'turtles',
133+
},
134+
{kind: ls.SymbolKind.Class, name: 'disc', location: createLocation(4, 0, 5, 0), containerName: 'turtles'},
135+
];
136+
const result = OutlineViewAdapter.createOutlineTrees(sourceItems);
137+
expect(result.length).to.equal(1);
138+
const r = (result: any);
139+
expect(r[0].endPosition.row).to.equal(10);
140+
expect(r[0].children.length).to.equal(1);
141+
expect(r[0].children[0].endPosition.row).to.equal(8);
142+
expect(r[0].children[0].children.length).to.equal(1);
143+
expect(r[0].children[0].children[0].endPosition.row).to.equal(5);
144+
});
145+
});
146+
147+
describe('symbolToOutline', () => {
148+
it('converts an individual item', () => {
149+
const sourceItem = {kind: ls.SymbolKind.Class, name: 'Program', location: createLocation(1, 2, 3, 4)};
150+
const result = OutlineViewAdapter.symbolToOutline(sourceItem);
151+
expect(result.icon).to.equal('type-class');
152+
expect(result.representativeName).to.equal('Program');
153+
expect(result.children).to.deep.equal([]);
154+
const r = (result: any);
155+
expect(r.tokenizedText[0].kind).to.equal('type');
156+
expect(r.tokenizedText[0].value).to.equal('Program');
157+
expect(r.startPosition.row).to.equal(1);
158+
expect(r.startPosition.column).to.equal(2);
159+
expect(r.endPosition.row).to.equal(3);
160+
expect(r.endPosition.column).to.equal(4);
161+
});
162+
});
163+
});

0 commit comments

Comments
 (0)