Skip to content

Commit e13feea

Browse files
mjbvzmjbvz
authored
Only compute diagnostics for opened md files (microsoft#153395)
* Only compute diagnostics for opened md files For microsoft#152494 * Make tests stable for result ordering
1 parent 87e684e commit e13feea

File tree

9 files changed

+91
-40
lines changed

9 files changed

+91
-40
lines changed

‎extensions/markdown-language-features/src/languageFeatures/diagnostics.ts‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,9 @@ export abstract class DiagnosticReporter extends Disposable {
242242

243243
public abstract delete(uri: vscode.Uri): void;
244244

245-
public abstract areDiagnosticsEnabled(uri: vscode.Uri): boolean;
245+
public abstract isOpen(uri: vscode.Uri): boolean;
246+
247+
public abstract getOpenDocuments(): ITextDocument[];
246248

247249
public addWorkItem(promise: Promise<any>): Promise<any> {
248250
this.pending.add(promise);
@@ -256,6 +258,7 @@ export abstract class DiagnosticReporter extends Disposable {
256258
}
257259

258260
export class DiagnosticCollectionReporter extends DiagnosticReporter {
261+
259262
private readonly collection: vscode.DiagnosticCollection;
260263

261264
constructor() {
@@ -269,19 +272,24 @@ export class DiagnosticCollectionReporter extends DiagnosticReporter {
269272
}
270273

271274
public set(uri: vscode.Uri, diagnostics: readonly vscode.Diagnostic[]): void {
272-
this.collection.set(uri, this.areDiagnosticsEnabled(uri) ? diagnostics : []);
275+
this.collection.set(uri, this.isOpen(uri) ? diagnostics : []);
273276
}
274277

275-
public areDiagnosticsEnabled(uri: vscode.Uri): boolean {
276-
const tabs = this.getAllTabResources();
278+
public isOpen(uri: vscode.Uri): boolean {
279+
const tabs = this.getTabResources();
277280
return tabs.has(uri);
278281
}
279282

280283
public delete(uri: vscode.Uri): void {
281284
this.collection.delete(uri);
282285
}
283286

284-
private getAllTabResources(): ResourceMap<void> {
287+
public getOpenDocuments(): ITextDocument[] {
288+
const tabs = this.getTabResources();
289+
return vscode.workspace.textDocuments.filter(doc => tabs.has(doc.uri));
290+
}
291+
292+
private getTabResources(): ResourceMap<void> {
285293
const openedTabDocs = new ResourceMap<void>();
286294
for (const group of vscode.window.tabGroups.all) {
287295
for (const tab of group.tabs) {
@@ -365,7 +373,7 @@ export class DiagnosticManager extends Disposable {
365373
return this.reporter.addWorkItem(
366374
(async () => {
367375
const triggered = new ResourceMap<Promise<void>>();
368-
for (const ref of await this.referencesProvider.getAllReferencesToFile(uri, noopToken)) {
376+
for (const ref of await this.referencesProvider.getReferencesToFileInDocs(uri, this.reporter.getOpenDocuments(), noopToken)) {
369377
const file = ref.location.uri;
370378
if (!triggered.has(file)) {
371379
triggered.set(file, this.triggerDiagnostics(file));
@@ -398,7 +406,7 @@ export class DiagnosticManager extends Disposable {
398406
const doc = await this.workspace.getOrLoadMarkdownDocument(resource);
399407
if (doc) {
400408
await this.inFlightDiagnostics.trigger(doc.uri, async (token) => {
401-
if (this.reporter.areDiagnosticsEnabled(doc.uri)) {
409+
if (this.reporter.isOpen(doc.uri)) {
402410
const state = await this.recomputeDiagnosticState(doc, token);
403411
this.linkWatcher.updateLinksForDocument(doc.uri, state.config.enabled && state.config.validateFileLinks ? state.links : []);
404412
this.reporter.set(doc.uri, state.diagnostics);
@@ -417,12 +425,7 @@ export class DiagnosticManager extends Disposable {
417425
this.inFlightDiagnostics.clear();
418426

419427
return this.reporter.addWorkItem(
420-
(async () => {
421-
// TODO: This pulls in all md files in the workspace. Instead we only care about opened text documents.
422-
// Need a new way to handle that.
423-
const allDocs = await this.workspace.getAllMarkdownDocuments();
424-
await Promise.all(Array.from(allDocs, doc => this.triggerDiagnostics(doc.uri)));
425-
})()
428+
Promise.all(Array.from(this.reporter.getOpenDocuments(), doc => this.triggerDiagnostics(doc.uri)))
426429
);
427430
}
428431

‎extensions/markdown-language-features/src/languageFeatures/fileReferences.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export class FindFileReferencesCommand implements Command {
3333
location: vscode.ProgressLocation.Window,
3434
title: localize('progress.title', "Finding file references")
3535
}, async (_progress, token) => {
36-
const references = await this.referencesProvider.getAllReferencesToFile(resource!, token);
36+
const references = await this.referencesProvider.getReferencesToFileInWorkspace(resource!, token);
3737
const locations = references.map(ref => ref.location);
3838

3939
const config = vscode.workspace.getConfiguration('references');

‎extensions/markdown-language-features/src/languageFeatures/references.ts‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ export type MdReference = MdLinkReference | MdHeaderReference;
6767
export class MdReferencesProvider extends Disposable {
6868

6969
private readonly _linkCache: MdWorkspaceInfoCache<readonly MdLink[]>;
70-
private readonly _linkComputer: MdLinkComputer;
7170

7271
public constructor(
7372
private readonly parser: IMdParser,
@@ -77,8 +76,8 @@ export class MdReferencesProvider extends Disposable {
7776
) {
7877
super();
7978

80-
this._linkComputer = new MdLinkComputer(parser);
81-
this._linkCache = this._register(new MdWorkspaceInfoCache(workspace, doc => this._linkComputer.getAllLinks(doc, noopToken)));
79+
const linkComputer = new MdLinkComputer(parser);
80+
this._linkCache = this._register(new MdWorkspaceInfoCache(workspace, doc => linkComputer.getAllLinks(doc, noopToken)));
8281
}
8382

8483
public async getReferencesAtPosition(document: ITextDocument, position: vscode.Position, token: vscode.CancellationToken): Promise<MdReference[]> {
@@ -97,11 +96,26 @@ export class MdReferencesProvider extends Disposable {
9796
}
9897
}
9998

100-
public async getAllReferencesToFile(resource: vscode.Uri, _token: vscode.CancellationToken): Promise<MdReference[]> {
101-
this.logger.verbose('ReferencesProvider', `getAllReferencesToFile: ${resource}`);
99+
public async getReferencesToFileInWorkspace(resource: vscode.Uri, token: vscode.CancellationToken): Promise<MdReference[]> {
100+
this.logger.verbose('ReferencesProvider', `getAllReferencesToFileInWorkspace: ${resource}`);
102101

103102
const allLinksInWorkspace = (await this._linkCache.values()).flat();
104-
return Array.from(this.findAllLinksToFile(resource, allLinksInWorkspace, undefined));
103+
if (token.isCancellationRequested) {
104+
return [];
105+
}
106+
107+
return Array.from(this.findLinksToFile(resource, allLinksInWorkspace, undefined));
108+
}
109+
110+
public async getReferencesToFileInDocs(resource: vscode.Uri, otherDocs: readonly ITextDocument[], token: vscode.CancellationToken): Promise<MdReference[]> {
111+
this.logger.verbose('ReferencesProvider', `getAllReferencesToFileInFiles: ${resource}`);
112+
113+
const links = (await this._linkCache.getForDocs(otherDocs)).flat();
114+
if (token.isCancellationRequested) {
115+
return [];
116+
}
117+
118+
return Array.from(this.findLinksToFile(resource, links, undefined));
105119
}
106120

107121
private async getReferencesToHeader(document: ITextDocument, header: TocEntry): Promise<MdReference[]> {
@@ -137,7 +151,7 @@ export class MdReferencesProvider extends Disposable {
137151
}
138152

139153
private async getReferencesToLinkAtPosition(document: ITextDocument, position: vscode.Position, token: vscode.CancellationToken): Promise<MdReference[]> {
140-
const docLinks = await this._linkComputer.getAllLinks(document, token);
154+
const docLinks = (await this._linkCache.getForDocs([document]))[0];
141155

142156
for (const link of docLinks) {
143157
if (link.kind === 'definition') {
@@ -223,7 +237,7 @@ export class MdReferencesProvider extends Disposable {
223237
}
224238
}
225239
} else { // Triggered on a link without a fragment so we only require matching the file and ignore fragments
226-
references.push(...this.findAllLinksToFile(resolvedResource ?? sourceLink.href.path, allLinksInWorkspace, sourceLink));
240+
references.push(...this.findLinksToFile(resolvedResource ?? sourceLink.href.path, allLinksInWorkspace, sourceLink));
227241
}
228242

229243
return references;
@@ -238,8 +252,8 @@ export class MdReferencesProvider extends Disposable {
238252
|| uri.Utils.extname(href.path) === '' && href.path.with({ path: href.path.path + '.md' }).fsPath === targetDoc.fsPath;
239253
}
240254

241-
private *findAllLinksToFile(resource: vscode.Uri, allLinksInWorkspace: readonly MdLink[], sourceLink: MdLink | undefined): Iterable<MdReference> {
242-
for (const link of allLinksInWorkspace) {
255+
private *findLinksToFile(resource: vscode.Uri, links: readonly MdLink[], sourceLink: MdLink | undefined): Iterable<MdReference> {
256+
for (const link of links) {
243257
if (link.href.kind !== 'internal' || !this.looksLikeLinkToDoc(link.href, resource)) {
244258
continue;
245259
}

‎extensions/markdown-language-features/src/logging.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export class VsCodeOutputLogger extends Disposable implements ILogger {
5858
const now = new Date();
5959
return String(now.getUTCHours()).padStart(2, '0')
6060
+ ':' + String(now.getMinutes()).padStart(2, '0')
61-
+ ':' + String(now.getUTCSeconds()).padStart(2, '0') + '.' + now.getMilliseconds();
61+
+ ':' + String(now.getUTCSeconds()).padStart(2, '0') + '.' + String(now.getMilliseconds()).padStart(3, '0');
6262
}
6363

6464
private updateConfiguration(): void {

‎extensions/markdown-language-features/src/test/diagnostic.test.ts‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { DiagnosticCollectionReporter, DiagnosticComputer, DiagnosticConfigurati
1010
import { MdLinkProvider } from '../languageFeatures/documentLinks';
1111
import { MdReferencesProvider } from '../languageFeatures/references';
1212
import { MdTableOfContentsProvider } from '../tableOfContents';
13+
import { ITextDocument } from '../types/textDocument';
1314
import { noopToken } from '../util/cancellation';
1415
import { DisposableStore } from '../util/dispose';
1516
import { InMemoryDocument } from '../util/inMemoryDocument';
@@ -79,6 +80,12 @@ class MemoryDiagnosticReporter extends DiagnosticReporter {
7980

8081
private readonly diagnostics = new ResourceMap<readonly vscode.Diagnostic[]>();
8182

83+
constructor(
84+
private readonly workspace: InMemoryMdWorkspace,
85+
) {
86+
super();
87+
}
88+
8289
override dispose(): void {
8390
super.clear();
8491
this.clear();
@@ -93,7 +100,7 @@ class MemoryDiagnosticReporter extends DiagnosticReporter {
93100
this.diagnostics.set(uri, diagnostics);
94101
}
95102

96-
areDiagnosticsEnabled(_uri: vscode.Uri): boolean {
103+
isOpen(_uri: vscode.Uri): boolean {
97104
return true;
98105
}
99106

@@ -104,6 +111,10 @@ class MemoryDiagnosticReporter extends DiagnosticReporter {
104111
get(uri: vscode.Uri): readonly vscode.Diagnostic[] {
105112
return orderDiagnosticsByRange(this.diagnostics.get(uri) ?? []);
106113
}
114+
115+
getOpenDocuments(): ITextDocument[] {
116+
return this.workspace.values();
117+
}
107118
}
108119

109120
suite('markdown: Diagnostic Computer', () => {
@@ -454,7 +465,7 @@ suite('Markdown: Diagnostics manager', () => {
454465
))
455466
]));
456467

457-
const reporter = store.add(new MemoryDiagnosticReporter());
468+
const reporter = store.add(new MemoryDiagnosticReporter(workspace));
458469
const config = new MemoryDiagnosticConfiguration({ enabled: true });
459470

460471
const manager = createDiagnosticsManager(store, workspace, config, reporter);
@@ -499,7 +510,7 @@ suite('Markdown: Diagnostics manager', () => {
499510
`[text](#no-such-2)`,
500511
));
501512
const workspace = store.add(new InMemoryMdWorkspace([doc1, doc2]));
502-
const reporter = store.add(new MemoryDiagnosticReporter());
513+
const reporter = store.add(new MemoryDiagnosticReporter(workspace));
503514

504515
const manager = createDiagnosticsManager(store, workspace, new MemoryDiagnosticConfiguration({}), reporter);
505516
await manager.ready;
@@ -554,7 +565,7 @@ suite('Markdown: Diagnostics manager', () => {
554565
`# Header`
555566
));
556567
const workspace = store.add(new InMemoryMdWorkspace([doc1, doc2]));
557-
const reporter = store.add(new MemoryDiagnosticReporter());
568+
const reporter = store.add(new MemoryDiagnosticReporter(workspace));
558569

559570
const manager = createDiagnosticsManager(store, workspace, new MemoryDiagnosticConfiguration({}), reporter);
560571
await manager.ready;

‎extensions/markdown-language-features/src/test/fileReferences.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ function getFileReferences(store: DisposableStore, resource: vscode.Uri, workspa
2222
const engine = createNewMarkdownEngine();
2323
const tocProvider = store.add(new MdTableOfContentsProvider(engine, workspace, nulLogger));
2424
const computer = store.add(new MdReferencesProvider(engine, workspace, tocProvider, nulLogger));
25-
return computer.getAllReferencesToFile(resource, noopToken);
25+
return computer.getReferencesToFileInWorkspace(resource, noopToken);
2626
}
2727

2828
function assertReferencesEqual(actualRefs: readonly MdReference[], ...expectedRefs: { uri: vscode.Uri; line: number }[]) {

‎extensions/markdown-language-features/src/test/inMemoryWorkspace.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,14 @@ export class InMemoryMdWorkspace extends Disposable implements IMdWorkspace {
2222
}
2323
}
2424

25-
public async getAllMarkdownDocuments() {
25+
public values() {
2626
return Array.from(this._documents.values());
2727
}
2828

29+
public async getAllMarkdownDocuments() {
30+
return this.values();
31+
}
32+
2933
public async getOrLoadMarkdownDocument(resource: vscode.Uri): Promise<ITextDocument | undefined> {
3034
return this._documents.get(resource);
3135
}

‎extensions/markdown-language-features/src/test/references.test.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,19 @@ import { nulLogger } from './nulLogging';
1818
import { joinLines, withStore, workspacePath } from './util';
1919

2020

21-
function getReferences(store: DisposableStore, doc: InMemoryDocument, pos: vscode.Position, workspace: IMdWorkspace) {
21+
async function getReferences(store: DisposableStore, doc: InMemoryDocument, pos: vscode.Position, workspace: IMdWorkspace) {
2222
const engine = createNewMarkdownEngine();
2323
const tocProvider = store.add(new MdTableOfContentsProvider(engine, workspace, nulLogger));
2424
const computer = store.add(new MdReferencesProvider(engine, workspace, tocProvider, nulLogger));
2525
const provider = new MdVsCodeReferencesProvider(computer);
26-
return provider.provideReferences(doc, pos, { includeDeclaration: true }, noopToken);
26+
const refs = await provider.provideReferences(doc, pos, { includeDeclaration: true }, noopToken);
27+
return refs.sort((a, b) => {
28+
const pathCompare = a.uri.toString().localeCompare(b.uri.toString());
29+
if (pathCompare !== 0) {
30+
return pathCompare;
31+
}
32+
return a.range.start.compareTo(b.range.start);
33+
});
2734
}
2835

2936
function assertReferencesEqual(actualRefs: readonly vscode.Location[], ...expectedRefs: { uri: vscode.Uri; line: number; startCharacter?: number; endCharacter?: number }[]) {
@@ -130,7 +137,7 @@ suite('Markdown: Find all references', () => {
130137
test('Should find references from header across files', withStore(async (store) => {
131138
const docUri = workspacePath('doc.md');
132139
const other1Uri = workspacePath('sub', 'other.md');
133-
const other2Uri = workspacePath('other2.md');
140+
const other2Uri = workspacePath('zOther2.md');
134141

135142
const doc = new InMemoryDocument(docUri, joinLines(
136143
`# abc`,
@@ -216,7 +223,7 @@ suite('Markdown: Find all references', () => {
216223
test('Should find references from link across files', withStore(async (store) => {
217224
const docUri = workspacePath('doc.md');
218225
const other1Uri = workspacePath('sub', 'other.md');
219-
const other2Uri = workspacePath('other2.md');
226+
const other2Uri = workspacePath('zOther2.md');
220227

221228
const doc = new InMemoryDocument(docUri, joinLines(
222229
`# abc`,
@@ -300,9 +307,9 @@ suite('Markdown: Find all references', () => {
300307

301308
const refs = await getReferences(store, doc, new vscode.Position(0, 23), workspace);
302309
assertReferencesEqual(refs!,
303-
{ uri: other1Uri, line: 1 }, // Header definition
304310
{ uri: docUri, line: 0 },
305311
{ uri: docUri, line: 1 },
312+
{ uri: other1Uri, line: 1 }, // Header definition
306313
);
307314
}));
308315

@@ -467,7 +474,7 @@ suite('Markdown: Find all references', () => {
467474
{
468475
// Check refs to header fragment
469476
const headerRefs = await getReferences(store, otherDoc, new vscode.Position(0, 16), workspace);
470-
assertReferencesEqual(headerRefs!,
477+
assertReferencesEqual(headerRefs,
471478
{ uri: docUri, line: 0 }, // Header definition
472479
{ uri: docUri, line: 2 },
473480
{ uri: other1Uri, line: 0 },
@@ -477,15 +484,15 @@ suite('Markdown: Find all references', () => {
477484
{
478485
// Check refs to file itself from link with ext
479486
const fileRefs = await getReferences(store, otherDoc, new vscode.Position(0, 9), workspace);
480-
assertReferencesEqual(fileRefs!,
487+
assertReferencesEqual(fileRefs,
481488
{ uri: other1Uri, line: 0, endCharacter: 14 },
482489
{ uri: other1Uri, line: 1, endCharacter: 19 },
483490
);
484491
}
485492
{
486493
// Check refs to file itself from link without ext
487494
const fileRefs = await getReferences(store, otherDoc, new vscode.Position(1, 17), workspace);
488-
assertReferencesEqual(fileRefs!,
495+
assertReferencesEqual(fileRefs,
489496
{ uri: other1Uri, line: 0 },
490497
{ uri: other1Uri, line: 1 },
491498
);

‎extensions/markdown-language-features/src/util/workspaceCache.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,16 @@ export class MdWorkspaceInfoCache<T> extends Disposable {
143143
return Array.from(await this._cache.entries(), x => x[1]);
144144
}
145145

146+
public async getForDocs(docs: readonly ITextDocument[]): Promise<T[]> {
147+
for (const doc of docs) {
148+
if (!this._cache.has(doc.uri)) {
149+
this.update(doc);
150+
}
151+
}
152+
153+
return Promise.all(docs.map(doc => this._cache.get(doc.uri) as Promise<T>));
154+
}
155+
146156
private async ensureInit(): Promise<void> {
147157
if (!this._init) {
148158
this._init = this.populateCache();
@@ -157,7 +167,9 @@ export class MdWorkspaceInfoCache<T> extends Disposable {
157167
private async populateCache(): Promise<void> {
158168
const markdownDocumentUris = await this.workspace.getAllMarkdownDocuments();
159169
for (const document of markdownDocumentUris) {
160-
this.update(document);
170+
if (!this._cache.has(document.uri)) {
171+
this.update(document);
172+
}
161173
}
162174
}
163175

0 commit comments

Comments
 (0)