From c981c51b66aef536b2e795f8be3594fe3c701d42 Mon Sep 17 00:00:00 2001 From: Damien Daspit Date: Fri, 1 Nov 2024 16:55:26 -0500 Subject: [PATCH] Add a more realistic diagnostic provider --- packages/core/src/diagnostic/diagnostic.ts | 1 + packages/core/src/diagnostic/index.ts | 7 +- .../core/src/document/scripture-container.ts | 13 +- .../core/src/document/scripture-document.ts | 13 +- packages/core/src/document/scripture-leaf.ts | 4 +- packages/core/src/document/scripture-node.ts | 19 ++- packages/core/src/formatting/index.ts | 6 +- packages/vscode/package.json | 23 +--- packages/vscode/src/server.ts | 24 +--- .../vscode/src/test-diagnostic-provider.ts | 120 ------------------ .../src/verse-order-diagnostic-provider.ts | 118 +++++++++++++++++ 11 files changed, 164 insertions(+), 184 deletions(-) delete mode 100644 packages/vscode/src/test-diagnostic-provider.ts create mode 100644 packages/vscode/src/verse-order-diagnostic-provider.ts diff --git a/packages/core/src/diagnostic/diagnostic.ts b/packages/core/src/diagnostic/diagnostic.ts index 066576d..500f1af 100644 --- a/packages/core/src/diagnostic/diagnostic.ts +++ b/packages/core/src/diagnostic/diagnostic.ts @@ -13,4 +13,5 @@ export interface Diagnostic { range: Range; severity: DiagnosticSeverity; message: string; + data?: unknown; } diff --git a/packages/core/src/diagnostic/index.ts b/packages/core/src/diagnostic/index.ts index 239c920..653c2a7 100644 --- a/packages/core/src/diagnostic/index.ts +++ b/packages/core/src/diagnostic/index.ts @@ -1,4 +1,9 @@ export type { Diagnostic } from './diagnostic'; export { DiagnosticSeverity } from './diagnostic'; export type { DiagnosticFix } from './diagnostic-fix'; -export type { DiagnosticProvider, DiagnosticProviderFactory, DiagnosticsChanged } from './diagnostic-provider'; +export type { + DiagnosticProvider, + DiagnosticProviderConstructor, + DiagnosticProviderFactory, + DiagnosticsChanged, +} from './diagnostic-provider'; diff --git a/packages/core/src/document/scripture-container.ts b/packages/core/src/document/scripture-container.ts index 7a2fa28..a4ad5a2 100644 --- a/packages/core/src/document/scripture-container.ts +++ b/packages/core/src/document/scripture-container.ts @@ -1,7 +1,7 @@ import { Position } from '../common/position'; import { Range } from '../common/range'; import { ScriptureDocument } from './scripture-document'; -import { ScriptureNode, ScriptureNodeType } from './scripture-node'; +import { findNodes, ScriptureNode, ScriptureNodeType } from './scripture-node'; export abstract class ScriptureContainer implements ScriptureNode { private _parent?: ScriptureNode; @@ -49,13 +49,10 @@ export abstract class ScriptureContainer implements ScriptureNode { return this.document.getText(this.range); } - *getNodes(filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean)): IterableIterator { - for (const child of this._children) { - if (filter == null || child.type === filter || (typeof filter === 'function' && filter(child))) { - yield child; - } - yield* child.getNodes(filter); - } + findNodes( + filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean) | ScriptureNodeType[], + ): IterableIterator { + return findNodes(this, filter); } positionAt(offset: number): Position { diff --git a/packages/core/src/document/scripture-document.ts b/packages/core/src/document/scripture-document.ts index c5518bb..38b18d5 100644 --- a/packages/core/src/document/scripture-document.ts +++ b/packages/core/src/document/scripture-document.ts @@ -1,6 +1,6 @@ import { Range } from '../common'; import { Document } from './document'; -import { ScriptureNode, ScriptureNodeType } from './scripture-node'; +import { findNodes, ScriptureNode, ScriptureNodeType } from './scripture-node'; import { TextDocument } from './text-document'; export class ScriptureDocument extends TextDocument implements Document, ScriptureNode { @@ -37,13 +37,10 @@ export class ScriptureDocument extends TextDocument implements Document, Scriptu throw new Error('The method is not supported.'); } - *getNodes(filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean)): IterableIterator { - for (const child of this._children) { - if (filter == null || child.type === filter || (typeof filter === 'function' && filter(child))) { - yield child; - } - yield* child.getNodes(filter); - } + findNodes( + filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean) | ScriptureNodeType[], + ): IterableIterator { + return findNodes(this, filter); } appendChild(child: ScriptureNode): void { diff --git a/packages/core/src/document/scripture-leaf.ts b/packages/core/src/document/scripture-leaf.ts index bafe29b..74b3570 100644 --- a/packages/core/src/document/scripture-leaf.ts +++ b/packages/core/src/document/scripture-leaf.ts @@ -38,7 +38,9 @@ export abstract class ScriptureLeaf implements ScriptureNode { return this.document.getText(this.range); } - *getNodes(_filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean)): IterableIterator { + *findNodes( + _filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean) | ScriptureNodeType[], + ): IterableIterator { // return nothing } diff --git a/packages/core/src/document/scripture-node.ts b/packages/core/src/document/scripture-node.ts index 971a618..3502849 100644 --- a/packages/core/src/document/scripture-node.ts +++ b/packages/core/src/document/scripture-node.ts @@ -31,7 +31,7 @@ export interface ScriptureNode { updateParent(parent: ScriptureNode | undefined): void; remove(): void; getText(): string; - getNodes(filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean)): IterableIterator; + findNodes(filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean)): IterableIterator; positionAt(offset: number): Position; appendChild(child: ScriptureNode): void; insertChild(index: number, child: ScriptureNode): void; @@ -39,3 +39,20 @@ export interface ScriptureNode { spliceChildren(start: number, deleteCount: number, ...items: ScriptureNode[]): void; clearChildren(): void; } + +export function* findNodes( + node: ScriptureNode, + filter?: ScriptureNodeType | ((node: ScriptureNode) => boolean) | ScriptureNodeType[], +): IterableIterator { + for (const child of node.children) { + if ( + filter == null || + (Array.isArray(filter) && filter.includes(child.type)) || + (typeof filter === 'function' && filter(child)) || + child.type === filter + ) { + yield child; + } + yield* findNodes(child, filter); + } +} diff --git a/packages/core/src/formatting/index.ts b/packages/core/src/formatting/index.ts index d5b9370..d1c6a8f 100644 --- a/packages/core/src/formatting/index.ts +++ b/packages/core/src/formatting/index.ts @@ -1 +1,5 @@ -export type { OnTypeFormattingProvider, OnTypeFormattingProviderFactory } from './on-type-formatting-provider'; +export type { + OnTypeFormattingProvider, + OnTypeFormattingProviderConstructor, + OnTypeFormattingProviderFactory, +} from './on-type-formatting-provider'; diff --git a/packages/vscode/package.json b/packages/vscode/package.json index 4a74fc6..7c00a53 100644 --- a/packages/vscode/package.json +++ b/packages/vscode/package.json @@ -31,28 +31,7 @@ "USFM" ] } - ], - "configuration": { - "type": "object", - "title": "Lynx Test", - "properties": { - "lynxTest.maxNumberOfProblems": { - "type": "number", - "default": 100, - "description": "Controls the maximum number of problems produced by the server." - }, - "lynxTest.trace.server": { - "type": "string", - "enum": [ - "off", - "messages", - "verbose" - ], - "default": "off", - "description": "Traces the communication between VS Code and the language server." - } - } - } + ] }, "dependencies": { "lynx-core": "*", diff --git a/packages/vscode/src/server.ts b/packages/vscode/src/server.ts index d7c2575..4984ccb 100644 --- a/packages/vscode/src/server.ts +++ b/packages/vscode/src/server.ts @@ -14,13 +14,7 @@ import { } from 'vscode-languageserver/node'; import { SmartQuoteFormattingProvider } from './smart-quote-formatting-provider'; -import { TestDiagnosticProvider, TestDiagnosticProviderConfig } from './test-diagnostic-provider'; - -// The global settings, used when the `workspace/configuration` request is not supported by the client. -// Please note that this is not the case when using this server with the client provided in this example -// but could happen with other clients. -const defaultSettings: TestDiagnosticProviderConfig = { maxNumberOfProblems: 1000 }; -let globalSettings: TestDiagnosticProviderConfig = defaultSettings; +import { VerseOrderDiagnosticProvider } from './verse-order-diagnostic-provider'; // Create a connection for the server, using Node's IPC as a transport. // Also include all preview / proposed LSP features. @@ -29,7 +23,7 @@ const connection = createConnection(ProposedFeatures.all); // Create a simple text document manager. const workspace = new Workspace({ documentFactory: new UsfmDocumentFactory(new UsfmStylesheet('usfm.sty')), - diagnosticProviders: [TestDiagnosticProvider.factory(() => globalSettings)], + diagnosticProviders: [VerseOrderDiagnosticProvider], onTypeFormattingProviders: [SmartQuoteFormattingProvider], }); @@ -83,15 +77,6 @@ connection.onInitialized(() => { } }); -connection.onDidChangeConfiguration((change) => { - const settings = change.settings as Map; - globalSettings = (settings.get('lynxTest') as TestDiagnosticProviderConfig | undefined) ?? defaultSettings; - // Refresh the diagnostics since the `maxNumberOfProblems` could have changed. - // We could optimize things here and re-fetch the setting first can compare it - // to the existing setting, but this is out of scope for this example. - connection.languages.diagnostics.refresh(); -}); - connection.languages.diagnostics.on(async (params) => { return { kind: DocumentDiagnosticReportKind.Full, @@ -143,10 +128,5 @@ connection.onDocumentOnTypeFormatting(async (params) => { return await workspace.getOnTypeEdits(params.textDocument.uri, params.position, params.ch); }); -connection.onDidChangeWatchedFiles((_change) => { - // Monitored files have change in VSCode - connection.console.log('We received a file change event'); -}); - // Listen on the connection connection.listen(); diff --git a/packages/vscode/src/test-diagnostic-provider.ts b/packages/vscode/src/test-diagnostic-provider.ts deleted file mode 100644 index 82d154b..0000000 --- a/packages/vscode/src/test-diagnostic-provider.ts +++ /dev/null @@ -1,120 +0,0 @@ -import { - Diagnostic, - DiagnosticProvider, - DiagnosticProviderFactory, - DiagnosticsChanged, - DiagnosticSeverity, - DocumentManager, - ScriptureDocument, - ScriptureNodeType, -} from 'lynx-core'; -import { DiagnosticFix } from 'lynx-core'; -import { map, merge, Observable, switchMap } from 'rxjs'; - -export interface TestDiagnosticProviderConfig { - maxNumberOfProblems: number; -} - -export class TestDiagnosticProvider implements DiagnosticProvider { - static factory(config: () => TestDiagnosticProviderConfig): DiagnosticProviderFactory { - return (documentManager: DocumentManager) => new TestDiagnosticProvider(documentManager, config); - } - - public readonly id = 'test'; - public readonly diagnosticsChanged$: Observable; - - constructor( - private readonly documentManager: DocumentManager, - private readonly config: () => TestDiagnosticProviderConfig, - ) { - this.diagnosticsChanged$ = merge( - documentManager.opened$.pipe( - map((e) => ({ - uri: e.document.uri, - version: e.document.version, - diagnostics: this.validateTextDocument(e.document), - })), - ), - documentManager.changed$.pipe( - map((e) => ({ - uri: e.document.uri, - version: e.document.version, - diagnostics: this.validateTextDocument(e.document), - })), - ), - documentManager.closed$.pipe( - switchMap(async (e) => { - const doc = await this.documentManager.get(e.uri); - return { uri: e.uri, version: doc?.version, diagnostics: [] }; - }), - ), - ); - } - - async getDiagnostics(uri: string): Promise { - const doc = await this.documentManager.get(uri); - if (doc == null) { - return []; - } - return this.validateTextDocument(doc); - } - - async getDiagnosticFixes(uri: string, diagnostic: Diagnostic): Promise { - const doc = await this.documentManager.get(uri); - if (doc == null) { - return []; - } - return this.getFixes(doc, diagnostic); - } - - private validateTextDocument(doc: ScriptureDocument): Diagnostic[] { - // In this simple example we get the settings for every validate run. - const settings = this.config(); - - // The validator creates diagnostics for all uppercase words length 2 and more - const pattern = /\b[A-Z]{2,}\b/g; - let m: RegExpExecArray | null; - - let problems = 0; - const diagnostics: Diagnostic[] = []; - for (const node of doc.getNodes(ScriptureNodeType.Text)) { - while ((m = pattern.exec(node.getText())) && problems < settings.maxNumberOfProblems) { - problems++; - const diagnostic: Diagnostic = { - code: 1, - source: this.id, - severity: DiagnosticSeverity.Warning, - range: { - start: node.positionAt(m.index), - end: node.positionAt(m.index + m[0].length), - }, - message: `${m[0]} is all uppercase.`, - }; - diagnostics.push(diagnostic); - } - } - return diagnostics; - } - - private getFixes(doc: ScriptureDocument, diagnostic: Diagnostic): DiagnosticFix[] { - if (diagnostic.code === 1) { - const text = doc.getText(); - const start = doc.offsetAt(diagnostic.range.start); - const end = doc.offsetAt(diagnostic.range.end); - return [ - { - title: `Convert to lowercase`, - isPreferred: true, - diagnostic, - edits: [ - { - range: diagnostic.range, - newText: text.slice(start, end).toLowerCase(), - }, - ], - }, - ]; - } - return []; - } -} diff --git a/packages/vscode/src/verse-order-diagnostic-provider.ts b/packages/vscode/src/verse-order-diagnostic-provider.ts new file mode 100644 index 0000000..9c6f8f0 --- /dev/null +++ b/packages/vscode/src/verse-order-diagnostic-provider.ts @@ -0,0 +1,118 @@ +import { + Diagnostic, + DiagnosticProvider, + DiagnosticsChanged, + DiagnosticSeverity, + DocumentManager, + ScriptureChapter, + ScriptureDocument, + ScriptureNodeType, + ScriptureVerse, +} from 'lynx-core'; +import { DiagnosticFix } from 'lynx-core'; +import { map, merge, Observable, switchMap } from 'rxjs'; + +export class VerseOrderDiagnosticProvider implements DiagnosticProvider { + public readonly id = 'verse-order'; + public readonly diagnosticsChanged$: Observable; + + constructor(private readonly documentManager: DocumentManager) { + this.diagnosticsChanged$ = merge( + documentManager.opened$.pipe( + map((e) => ({ + uri: e.document.uri, + version: e.document.version, + diagnostics: this.validateDocument(e.document), + })), + ), + documentManager.changed$.pipe( + map((e) => ({ + uri: e.document.uri, + version: e.document.version, + diagnostics: this.validateDocument(e.document), + })), + ), + documentManager.closed$.pipe( + switchMap(async (e) => { + const doc = await this.documentManager.get(e.uri); + return { uri: e.uri, version: doc?.version, diagnostics: [] }; + }), + ), + ); + } + + async getDiagnostics(uri: string): Promise { + const doc = await this.documentManager.get(uri); + if (doc == null) { + return []; + } + return this.validateDocument(doc); + } + + getDiagnosticFixes(_uri: string, diagnostic: Diagnostic): Promise { + const fixes: DiagnosticFix[] = []; + if (diagnostic.code === 2) { + const verseNumber = diagnostic.data as number; + fixes.push({ + title: `Insert missing verse`, + isPreferred: true, + diagnostic, + edits: [ + { + range: { start: diagnostic.range.start, end: diagnostic.range.start }, + newText: `\\v ${verseNumber.toString()} `, + }, + ], + }); + } + return Promise.resolve(fixes); + } + + private validateDocument(doc: ScriptureDocument): Diagnostic[] { + const diagnostics: Diagnostic[] = []; + + const verseNodes: [number, ScriptureVerse][] = []; + for (const node of doc.findNodes([ScriptureNodeType.Chapter, ScriptureNodeType.Verse])) { + if (node instanceof ScriptureChapter) { + diagnostics.push(...this.findMissingVerse(verseNodes)); + verseNodes.length = 0; + } else if (node instanceof ScriptureVerse) { + const verseNumber = parseInt(node.number); + if (verseNodes.length > 0) { + const [prevVerseNumber, prevVerseNode] = verseNodes[verseNodes.length - 1]; + if (verseNumber <= prevVerseNumber) { + diagnostics.push({ + range: prevVerseNode.range, + severity: DiagnosticSeverity.Error, + code: 1, + message: 'Verses are out of order.', + source: this.id, + }); + } + } + verseNodes.push([verseNumber, node]); + } + } + + diagnostics.push(...this.findMissingVerse(verseNodes)); + return diagnostics; + } + + private findMissingVerse(verseNodes: [number, ScriptureVerse][]): Diagnostic[] { + verseNodes.sort((a, b) => a[0] - b[0]); + const diagnostics: Diagnostic[] = []; + for (const [i, [number, node]] of verseNodes.entries()) { + if (number !== i + 1) { + diagnostics.push({ + range: node.range, + severity: DiagnosticSeverity.Warning, + code: 2, + message: 'Verse is missing.', + source: this.id, + data: number - 1, + }); + } + } + return diagnostics; + } +}