Handling ScriptureDocuments correctly

sillsdev · Dec 23, 2024 · ff8bf90 · ff8bf90
1 parent 5ec1ff9
commit ff8bf90
Show file tree

Hide file tree

Showing 28 changed files with 3,560 additions and 2,392 deletions.
diff --git a/packages/punctuation-checker/src/abstract-checker.ts b/packages/punctuation-checker/src/abstract-checker.ts
@@ -4,32 +4,37 @@ import {
   DiagnosticProvider,
   DiagnosticsChanged,
   DocumentManager,
-  Localizer,
+  ScriptureDocument,
+  ScriptureNode,
+  ScriptureNodeType,
   TextDocument,
 } from '@sillsdev/lynx';
 import { map, merge, Observable, switchMap } from 'rxjs';
 
+import { DiagnosticFactory } from './diagnostic-factory';
+import { IssueFinder, IssueFinderFactory } from './issue-finder';
+
 export abstract class AbstractChecker implements DiagnosticProvider {
   public readonly diagnosticsChanged$: Observable<DiagnosticsChanged>;
 
   constructor(
     public readonly id: string,
-    protected readonly localizer: Localizer,
-    protected readonly documentManager: DocumentManager<TextDocument>,
+    private readonly documentManager: DocumentManager<TextDocument | ScriptureDocument>,
+    private readonly issueFinderFactory: IssueFinderFactory,
   ) {
     this.diagnosticsChanged$ = merge(
       documentManager.opened$.pipe(
         map((e) => ({
           uri: e.document.uri,
           version: e.document.version,
-          diagnostics: this.validateTextDocument(e.document),
+          diagnostics: this.validateDocument(e.document),
         })),
       ),
       documentManager.changed$.pipe(
         map((e) => ({
           uri: e.document.uri,
           version: e.document.version,
-          diagnostics: this.validateTextDocument(e.document),
+          diagnostics: this.validateDocument(e.document),
         })),
       ),
       documentManager.closed$.pipe(
@@ -50,7 +55,7 @@ export abstract class AbstractChecker implements DiagnosticProvider {
     if (doc == null) {
       return [];
     }
-    return this.validateTextDocument(doc);
+    return this.validateDocument(doc);
   }
 
   async getDiagnosticFixes(uri: string, diagnostic: Diagnostic): Promise<DiagnosticFix[]> {
@@ -61,7 +66,67 @@ export abstract class AbstractChecker implements DiagnosticProvider {
     return this.getFixes(doc, diagnostic);
   }
 
-  protected abstract validateTextDocument(textDocument: TextDocument): Diagnostic[];
+  private validateDocument(document: TextDocument | ScriptureDocument): Diagnostic[] {
+    if (document instanceof ScriptureDocument) {
+      return this.validateScriptureDocument(document);
+    }
+    return this.validateTextDocument(document);
+  }
+
+  protected validateTextDocument(textDocument: TextDocument): Diagnostic[] {
+    const diagnosticFactory: DiagnosticFactory = new DiagnosticFactory(this.id, textDocument);
+
+    const issueFinder: IssueFinder = this.issueFinderFactory.createIssueFinder(diagnosticFactory);
+    return issueFinder.produceDiagnostics(textDocument.getText());
+  }
+
+  protected validateScriptureDocument(scriptureDocument: ScriptureDocument): Diagnostic[] {
+    let diagnostics: Diagnostic[] = [];
+    const diagnosticFactory: DiagnosticFactory = new DiagnosticFactory(this.id, scriptureDocument);
+
+    const issueFinder: IssueFinder = this.issueFinderFactory.createIssueFinder(diagnosticFactory);
+
+    const scriptureNodeGrouper: ScriptureNodeGrouper = new ScriptureNodeGrouper(
+      scriptureDocument.findNodes([ScriptureNodeType.Text]),
+    );
+    for (const nonVerseNode of scriptureNodeGrouper.getNonVerseNodes()) {
+      diagnostics = diagnostics.concat(issueFinder.produceDiagnosticsForScripture(nonVerseNode));
+    }
+    diagnostics = diagnostics.concat(issueFinder.produceDiagnosticsForScripture(scriptureNodeGrouper.getVerseNodes()));
+    return diagnostics;
+  }
+
+  protected abstract getFixes(document: TextDocument | ScriptureDocument, diagnostic: Diagnostic): DiagnosticFix[];
+}
+
+class ScriptureNodeGrouper {
+  private readonly nonVerseNodes: ScriptureNode[] = [];
+  private readonly verseNodes: ScriptureNode[] = [];
+
+  constructor(allNodes: IterableIterator<ScriptureNode>) {
+    for (const node of allNodes) {
+      if (this.isVerseNode(node)) {
+        this.verseNodes.push(node);
+      } else {
+        this.nonVerseNodes.push(node);
+      }
+    }
+  }
+
+  private isVerseNode(node: ScriptureNode): boolean {
+    for (const sibling of node.parent?.children ?? []) {
+      if (sibling.type === ScriptureNodeType.Verse) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  public getVerseNodes(): ScriptureNode[] {
+    return this.verseNodes;
+  }
 
-  protected abstract getFixes(textDocument: TextDocument, diagnostic: Diagnostic): DiagnosticFix[];
+  public getNonVerseNodes(): ScriptureNode[] {
+    return this.nonVerseNodes;
+  }
 }
diff --git a/packages/punctuation-checker/src/allowed-character/allowed-character-checker.ts b/packages/punctuation-checker/src/allowed-character/allowed-character-checker.ts
@@ -1,26 +1,22 @@
-import {
-  Diagnostic,
-  DiagnosticFix,
-  DiagnosticSeverity,
-  DocumentManager,
-  Localizer,
-  TextDocument,
-} from '@sillsdev/lynx';
+import { Diagnostic, DiagnosticFix, DocumentManager, Localizer, TextDocument } from '@sillsdev/lynx';
 
 import { AbstractChecker } from '../abstract-checker';
-import { DiagnosticFactory } from '../diagnostic-factory';
-import { DiagnosticList } from '../diagnostic-list';
+import { AllowedCharacterIssueFinderFactory } from './allowed-character-issue-finder';
 import { AllowedCharacterSet } from './allowed-character-set';
 
-const LOCALIZER_NAMESPACE = 'allowedCharacters';
+export const ALLOWED_CHARACTER_CHECKER_LOCALIZER_NAMESPACE = 'allowedCharacters';
 
 export class AllowedCharacterChecker extends AbstractChecker {
   constructor(
-    localizer: Localizer,
+    private readonly localizer: Localizer,
     documentManager: DocumentManager<TextDocument>,
-    private readonly allowedCharacterSet: AllowedCharacterSet,
+    allowedCharacterSet: AllowedCharacterSet,
   ) {
-    super('allowed-character-set-checker', localizer, documentManager);
+    super(
+      'allowed-character-set-checker',
+      documentManager,
+      new AllowedCharacterIssueFinderFactory(localizer, allowedCharacterSet),
+    );
   }
 
   async init(): Promise<void> {
@@ -29,81 +25,16 @@ export class AllowedCharacterChecker extends AbstractChecker {
     // Ideally, we'd like to be able to inject an initialization function, so that
     // tests can provide different messages, but due to the way variable dynamic imports
     // work, the namespace loading function can only appear in this file at this location
-    if (!this.localizer.hasNamespace(LOCALIZER_NAMESPACE)) {
+    if (!this.localizer.hasNamespace(ALLOWED_CHARACTER_CHECKER_LOCALIZER_NAMESPACE)) {
       this.localizer.addNamespace(
-        LOCALIZER_NAMESPACE,
+        ALLOWED_CHARACTER_CHECKER_LOCALIZER_NAMESPACE,
         (language: string) => import(`./locales/${language}.json`, { with: { type: 'json' } }),
       );
     }
   }
 
-  protected validateTextDocument(textDocument: TextDocument): Diagnostic[] {
-    const diagnosticFactory: DiagnosticFactory = new DiagnosticFactory(this.id, textDocument);
-
-    const allowedCharacterIssueFinder: AllowedCharacterIssueFinder = new AllowedCharacterIssueFinder(
-      this.localizer,
-      diagnosticFactory,
-      this.allowedCharacterSet,
-    );
-    return allowedCharacterIssueFinder.produceDiagnostics(textDocument.getText());
-  }
   protected getFixes(_textDocument: TextDocument, _diagnostic: Diagnostic): DiagnosticFix[] {
     // no fixes available for disallowed characters
     return [];
   }
 }
-
-class AllowedCharacterIssueFinder {
-  private diagnosticList: DiagnosticList;
-  private readonly characterRegex: RegExp = /./gu;
-  private static readonly DIAGNOSTIC_CODE: string = 'disallowed-character';
-
-  constructor(
-    private readonly localizer: Localizer,
-    private readonly diagnosticFactory: DiagnosticFactory,
-    private readonly allowedCharacterSet: AllowedCharacterSet,
-  ) {
-    this.diagnosticList = new DiagnosticList();
-  }
-
-  public produceDiagnostics(text: string): Diagnostic[] {
-    this.diagnosticList = new DiagnosticList();
-
-    let match: RegExpExecArray | null;
-    while ((match = this.characterRegex.exec(text))) {
-      const character = match[0];
-
-      this.checkCharacter(character, match.index, match.index + match[0].length);
-    }
-
-    return this.diagnosticList.toArray();
-  }
-
-  private checkCharacter(character: string, characterStartIndex: number, characterEndIndex: number): void {
-    if (!this.allowedCharacterSet.isCharacterAllowed(character)) {
-      this.addDisallowedCharacterWarning(character, characterStartIndex, characterEndIndex);
-    }
-  }
-
-  private addDisallowedCharacterWarning(character: string, characterStartIndex: number, characterEndIndex: number) {
-    const code: string = AllowedCharacterIssueFinder.DIAGNOSTIC_CODE;
-    const diagnostic: Diagnostic = this.diagnosticFactory
-      .newBuilder()
-      .setCode(code)
-      .setSeverity(DiagnosticSeverity.Warning)
-      .setRange(characterStartIndex, characterEndIndex)
-      .setMessage(
-        this.localizer.t(`diagnosticMessagesByCode.${code}`, {
-          ns: LOCALIZER_NAMESPACE,
-          character: character,
-        }),
-      )
-      .build();
-
-    this.diagnosticList.addDiagnostic(diagnostic);
-  }
-}
-
-export const _privateTestingClasses = {
-  AllowedCharacterIssueFinder,
-};
diff --git a/packages/punctuation-checker/src/allowed-character/allowed-character-issue-finder.ts b/packages/punctuation-checker/src/allowed-character/allowed-character-issue-finder.ts
@@ -0,0 +1,98 @@
+import { Diagnostic, DiagnosticSeverity, Localizer, Range, ScriptureNode } from '@sillsdev/lynx';
+
+import { DiagnosticFactory } from '../diagnostic-factory';
+import { DiagnosticList } from '../diagnostic-list';
+import { IssueFinder, IssueFinderFactory } from '../issue-finder';
+import { ALLOWED_CHARACTER_CHECKER_LOCALIZER_NAMESPACE } from './allowed-character-checker';
+import { AllowedCharacterSet } from './allowed-character-set';
+
+export class AllowedCharacterIssueFinderFactory implements IssueFinderFactory {
+  constructor(
+    private readonly localizer: Localizer,
+    private readonly allowedCharacterSet: AllowedCharacterSet,
+  ) {}
+
+  public createIssueFinder(diagnosticFactory: DiagnosticFactory): IssueFinder {
+    return new AllowedCharacterIssueFinder(this.localizer, diagnosticFactory, this.allowedCharacterSet);
+  }
+}
+
+export class AllowedCharacterIssueFinder implements IssueFinder {
+  private diagnosticList: DiagnosticList;
+  private readonly characterRegex: RegExp = /./gu;
+  private static readonly DIAGNOSTIC_CODE: string = 'disallowed-character';
+
+  constructor(
+    private readonly localizer: Localizer,
+    private readonly diagnosticFactory: DiagnosticFactory,
+    private readonly allowedCharacterSet: AllowedCharacterSet,
+  ) {
+    this.diagnosticList = new DiagnosticList();
+  }
+
+  public produceDiagnostics(input: string): Diagnostic[] {
+    this.diagnosticList = new DiagnosticList();
+
+    let match: RegExpExecArray | null;
+    while ((match = this.characterRegex.exec(input))) {
+      const character = match[0];
+
+      this.checkCharacter(character, match.index, match.index + match[0].length);
+    }
+
+    return this.diagnosticList.toArray();
+  }
+
+  public produceDiagnosticsForScripture(nodes: ScriptureNode | ScriptureNode[]): Diagnostic[] {
+    this.diagnosticList = new DiagnosticList();
+
+    if (!Array.isArray(nodes)) {
+      nodes = [nodes];
+    }
+
+    for (const node of nodes) {
+      let match: RegExpExecArray | null;
+      while ((match = this.characterRegex.exec(node.getText()))) {
+        const character = match[0];
+
+        this.checkCharacter(character, match.index, match.index + match[0].length, node.range);
+      }
+    }
+
+    return this.diagnosticList.toArray();
+  }
+
+  private checkCharacter(
+    character: string,
+    characterStartIndex: number,
+    characterEndIndex: number,
+    enclosingRange?: Range,
+  ): void {
+    if (!this.allowedCharacterSet.isCharacterAllowed(character)) {
+      this.addDisallowedCharacterWarning(character, characterStartIndex, characterEndIndex, enclosingRange);
+    }
+  }
+
+  private addDisallowedCharacterWarning(
+    character: string,
+    characterStartIndex: number,
+    characterEndIndex: number,
+    enclosingRange?: Range,
+  ) {
+    const code: string = AllowedCharacterIssueFinder.DIAGNOSTIC_CODE;
+    const diagnostic: Diagnostic = this.diagnosticFactory
+      .newBuilder()
+      .setCode(code)
+      .setSeverity(DiagnosticSeverity.Warning)
+      .setRange(characterStartIndex, characterEndIndex, enclosingRange)
+      .setMessage(
+        this.localizer.t(`diagnosticMessagesByCode.${code}`, {
+          ns: ALLOWED_CHARACTER_CHECKER_LOCALIZER_NAMESPACE,
+          character: character,
+        }),
+      )
+      .build();
+
+    this.diagnosticList.addDiagnostic(diagnostic);
+  }
+}
diff --git a/packages/punctuation-checker/src/diagnostic-factory.ts b/packages/punctuation-checker/src/diagnostic-factory.ts
@@ -33,11 +33,18 @@ class DiagnosticBuilder {
     return this;
   }
 
-  public setRange(startIndex: number, endIndex: number): this {
-    this.range = {
-      start: this.textDocument.positionAt(startIndex),
-      end: this.textDocument.positionAt(endIndex),
-    };
+  public setRange(startIndex: number, endIndex: number, enclosingRange?: Range): this {
+    if (enclosingRange === undefined) {
+      this.range = {
+        start: this.textDocument.positionAt(startIndex),
+        end: this.textDocument.positionAt(endIndex),
+      };
+    } else {
+      this.range = {
+        start: this.textDocument.positionAt(startIndex, enclosingRange),
+        end: this.textDocument.positionAt(endIndex, enclosingRange),
+      };
+    }
     return this;
   }
 

diff --git a/packages/punctuation-checker/src/issue-finder.ts b/packages/punctuation-checker/src/issue-finder.ts
@@ -0,0 +1,13 @@
+import { Diagnostic, ScriptureNode } from '@sillsdev/lynx';
+
+import { DiagnosticFactory } from './diagnostic-factory';
+
+export interface IssueFinder {
+  produceDiagnostics(text: string): Diagnostic[];
+
+  produceDiagnosticsForScripture(nodes: ScriptureNode | ScriptureNode[]): Diagnostic[];
+}
+
+export interface IssueFinderFactory {
+  createIssueFinder(diagnosticFactory: DiagnosticFactory): IssueFinder;
+}