Refactoring the quotation checker to use the data field in Diagnostic

sillsdev · Dec 12, 2024 · 0e72e5a · 0e72e5a
1 parent cfbccce
commit 0e72e5a
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 47 deletions.
diff --git a/packages/punctuation-checker/src/quotation/locales/en.json b/packages/punctuation-checker/src/quotation/locales/en.json
@@ -2,8 +2,8 @@
   "diagnosticMessagesByCode": {
     "unmatched-opening-quotation-mark": "Opening quotation mark with no closing mark.",
     "unmatched-closing-quotation-mark": "Closing quotation mark with no opening mark.",
-    "incorrectly-nested-quotation-mark-level-": "Incorrectly nested quotation mark.",
-    "ambiguous-quotation-mark-": "This quotation mark is ambiguous.",
+    "incorrectly-nested-quotation-mark": "Incorrectly nested quotation mark.",
+    "ambiguous-quotation-mark": "This quotation mark is ambiguous.",
     "deeply-nested-quotation-mark": "Too many levels of quotation marks. Consider rephrasing to avoid this."
   }
 }
diff --git a/packages/punctuation-checker/src/quotation/quotation-checker.ts b/packages/punctuation-checker/src/quotation/quotation-checker.ts
@@ -20,10 +20,8 @@ const LOCALIZER_NAMESPACE = 'quotation';
 
 const UNMATCHED_OPENING_QUOTE_DIAGNOSTIC_CODE = 'unmatched-opening-quotation-mark';
 const UNMATCHED_CLOSING_QUOTE_DIAGNOSTIC_CODE = 'unmatched-closing-quotation-mark';
-const INCORRECTLY_NESTED_QUOTE_DIAGNOSTIC_CODE = 'incorrectly-nested-quotation-mark-level-';
-const INCORRECTLY_NESTED_QUOTE_DIAGNOSTIC_CODE_REGEX = /incorrectly-nested-quotation-mark-level-(\d+)/;
-const AMBIGUOUS_QUOTE_DIAGNOSTIC_CODE = 'ambiguous-quotation-mark-';
-const AMBIGUOUS_QUOTE_DIAGNOSTIC_CODE_REGEX = /ambiguous-quotation-mark-(.)-to-(.)/;
+const INCORRECTLY_NESTED_QUOTE_DIAGNOSTIC_CODE = 'incorrectly-nested-quotation-mark';
+const AMBIGUOUS_QUOTE_DIAGNOSTIC_CODE = 'ambiguous-quotation-mark';
 const TOO_DEEPLY_NESTED_QUOTE_DIAGNOSTIC_CODE = 'deeply-nested-quotation-mark';
 
 export class QuotationChecker extends AbstractChecker {
@@ -72,10 +70,13 @@ export class QuotationChecker extends AbstractChecker {
       return [this.standardFixProvider.punctuationRemovalFix(diagnostic)];
     }
 
-    if (typeof diagnostic.code === 'string' && diagnostic.code.startsWith(INCORRECTLY_NESTED_QUOTE_DIAGNOSTIC_CODE)) {
+    if (diagnostic.code === INCORRECTLY_NESTED_QUOTE_DIAGNOSTIC_CODE) {
       const fixes: DiagnosticFix[] = [this.standardFixProvider.punctuationRemovalFix(diagnostic)];
-      const expectedQuotationMark: string | undefined = this.getExpectedQuotationMarkFromIncorrectlyNestedCode(
-        diagnostic.code,
+      interface QuoteParentDepth {
+        depth: number;
+      }
+      const expectedQuotationMark: string | undefined = this.getExpectedQuotationMarkForDepth(
+        (diagnostic.data as QuoteParentDepth).depth,
       );
       if (expectedQuotationMark !== undefined) {
         fixes.push(this.standardFixProvider.punctuationReplacementFix(diagnostic, expectedQuotationMark));
@@ -84,37 +85,24 @@ export class QuotationChecker extends AbstractChecker {
       return fixes;
     }
 
-    if (typeof diagnostic.code === 'string' && diagnostic.code.startsWith(AMBIGUOUS_QUOTE_DIAGNOSTIC_CODE)) {
-      const expectedQuotationMark: string | undefined = this.getExpectedQuotationMarkFromAmbiguousCode(diagnostic.code);
-      if (expectedQuotationMark !== undefined) {
-        return [this.standardFixProvider.punctuationReplacementFix(diagnostic, expectedQuotationMark)];
+    if (diagnostic.code === AMBIGUOUS_QUOTE_DIAGNOSTIC_CODE) {
+      interface QuotationMarkCorrection {
+        existingQuotationMark: string;
+        correctedQuotationMark: string;
       }
+      const expectedQuotationMark: string = (diagnostic.data as QuotationMarkCorrection).correctedQuotationMark;
+      return [this.standardFixProvider.punctuationReplacementFix(diagnostic, expectedQuotationMark)];
     }
     return [];
   }
 
-  private getExpectedQuotationMarkFromIncorrectlyNestedCode(code: string): string | undefined {
-    const match: RegExpMatchArray | null = INCORRECTLY_NESTED_QUOTE_DIAGNOSTIC_CODE_REGEX.exec(code);
-    if (match !== null) {
-      const expectedQuotationDepth = Number(match[1]) + 1;
-      return this.quotationConfig.getUnambiguousQuotationMarkByType(
-        QuotationDepth.fromNumber(expectedQuotationDepth),
-        PairedPunctuationDirection.Opening,
-      );
-    }
+  private getExpectedQuotationMarkForDepth(parentDepth: number): string | undefined {
+    const expectedQuotationDepth = parentDepth + 1;
     return this.quotationConfig.getUnambiguousQuotationMarkByType(
-      QuotationDepth.Primary,
+      QuotationDepth.fromNumber(expectedQuotationDepth),
       PairedPunctuationDirection.Opening,
     );
   }
-
-  private getExpectedQuotationMarkFromAmbiguousCode(code: string): string | undefined {
-    const match: RegExpMatchArray | null = AMBIGUOUS_QUOTE_DIAGNOSTIC_CODE_REGEX.exec(code);
-    if (match !== null) {
-      return match[2];
-    }
-    return undefined;
-  }
 }
 
 class QuotationErrorFinder {
@@ -177,10 +165,11 @@ class QuotationErrorFinder {
 
     const diagnostic: Diagnostic = this.diagnosticFactory
       .newBuilder()
-      .setCode(code + (quotationMark.parentDepth?.asNumber().toFixed() ?? '0'))
+      .setCode(code)
       .setSeverity(DiagnosticSeverity.Warning)
       .setRange(quotationMark.startIndex, quotationMark.endIndex)
       .setMessage(this.getErrorMessageByCode(code))
+      .setData({ depth: quotationMark.parentDepth?.asNumber() ?? 0 })
       .build();
 
     this.diagnosticList.addDiagnostic(diagnostic);
@@ -228,10 +217,14 @@ class QuotationErrorFinder {
     const code: string = AMBIGUOUS_QUOTE_DIAGNOSTIC_CODE;
     const diagnostic: Diagnostic = this.diagnosticFactory
       .newBuilder()
-      .setCode(code + quoteCorrection.existingQuotationMark.text + '-to-' + quoteCorrection.correctedQuotationMark.text)
+      .setCode(code)
       .setSeverity(DiagnosticSeverity.Warning)
       .setRange(quoteCorrection.existingQuotationMark.startIndex, quoteCorrection.existingQuotationMark.endIndex)
       .setMessage(this.getErrorMessageByCode(code))
+      .setData({
+        existingQuotationMark: quoteCorrection.existingQuotationMark.text,
+        correctedQuotationMark: quoteCorrection.correctedQuotationMark.text,
+      })
       .build();
     this.diagnosticList.addDiagnostic(diagnostic);
   }

diff --git a/packages/punctuation-checker/test/quotation/quotation-checker.test.ts b/packages/punctuation-checker/test/quotation/quotation-checker.test.ts
@@ -24,8 +24,8 @@ defaultLocalizer.addNamespace('quotation', (_language: string) => {
     diagnosticMessagesByCode: {
       'unmatched-opening-quotation-mark': 'Opening quotation mark with no closing mark.',
       'unmatched-closing-quotation-mark': 'Closing quotation mark with no opening mark.',
-      'incorrectly-nested-quotation-mark-level-': 'Incorrectly nested quotation mark.',
-      'ambiguous-quotation-mark-': 'This quotation mark is ambiguous.',
+      'incorrectly-nested-quotation-mark': 'Incorrectly nested quotation mark.',
+      'ambiguous-quotation-mark': 'This quotation mark is ambiguous.',
       'deeply-nested-quotation-mark': 'Too many levels of quotation marks. Consider rephrasing to avoid this.',
     },
   };
@@ -84,7 +84,7 @@ function createIncorrectlyNestedDiagnostic(
   parentDepth: QuotationDepth,
 ): Diagnostic {
   return {
-    code: 'incorrectly-nested-quotation-mark-level-' + parentDepth.asNumber().toFixed(),
+    code: 'incorrectly-nested-quotation-mark',
     severity: DiagnosticSeverity.Warning,
     range: {
       start: {
@@ -98,7 +98,9 @@ function createIncorrectlyNestedDiagnostic(
     },
     source: 'quotation-mark-checker',
     message: `Incorrectly nested quotation mark.`,
-    data: '',
+    data: {
+      depth: parentDepth.asNumber(),
+    },
   };
 }
 
@@ -109,7 +111,7 @@ function createAmbiguousDiagnostic(
   unambiguousMark: string,
 ): Diagnostic {
   return {
-    code: 'ambiguous-quotation-mark-' + ambiguousMark + '-to-' + unambiguousMark,
+    code: 'ambiguous-quotation-mark',
     severity: DiagnosticSeverity.Warning,
     range: {
       start: {
@@ -123,7 +125,10 @@ function createAmbiguousDiagnostic(
     },
     source: 'quotation-mark-checker',
     message: `This quotation mark is ambiguous.`,
-    data: '',
+    data: {
+      existingQuotationMark: ambiguousMark,
+      correctedQuotationMark: unambiguousMark,
+    },
   };
 }
 
@@ -599,7 +604,7 @@ describe('QuotationChecker tests', () => {
     await localizer.init();
 
     const incorrectlyNestedDiagnostic: Diagnostic = {
-      code: 'incorrectly-nested-quotation-mark-level-2',
+      code: 'incorrectly-nested-quotation-mark',
       severity: DiagnosticSeverity.Warning,
       range: {
         start: {
@@ -613,6 +618,9 @@ describe('QuotationChecker tests', () => {
       },
       source: 'quotation-mark-checker',
       message: `Incorrectly nested quotation mark.`,
+      data: {
+        depth: 2,
+      },
     };
 
     expect(await quotationChecker.getDiagnosticFixes('', incorrectlyNestedDiagnostic)).toEqual([
@@ -648,7 +656,7 @@ describe('QuotationChecker tests', () => {
     await localizer.init();
 
     const ambiguousDiagnostic: Diagnostic = {
-      code: 'ambiguous-quotation-mark-"-to-\u201C',
+      code: 'ambiguous-quotation-mark',
       severity: DiagnosticSeverity.Warning,
       range: {
         start: {
@@ -662,6 +670,10 @@ describe('QuotationChecker tests', () => {
       },
       source: 'quotation-mark-checker',
       message: `This quotation mark is ambiguous.`,
+      data: {
+        existingQuotationMark: '"',
+        correctedQuotationMark: '\u201C',
+      },
     };
 
     expect(await quotationChecker.getDiagnosticFixes('', ambiguousDiagnostic)).toEqual([

diff --git a/packages/punctuation-checker/test/rule-set/standard-rule-sets.test.ts b/packages/punctuation-checker/test/rule-set/standard-rule-sets.test.ts
@@ -359,7 +359,7 @@ describe('Standard English rule set tests', () => {
           data: '',
         },
         {
-          code: 'incorrectly-nested-quotation-mark-level-2',
+          code: 'incorrectly-nested-quotation-mark',
           severity: DiagnosticSeverity.Warning,
           range: {
             start: {
@@ -373,10 +373,12 @@ describe('Standard English rule set tests', () => {
           },
           message: 'Incorrectly nested quotation mark.',
           source: 'quotation-mark-checker',
-          data: '',
+          data: {
+            depth: 2,
+          },
         },
         {
-          code: 'incorrectly-nested-quotation-mark-level-2',
+          code: 'incorrectly-nested-quotation-mark',
           severity: DiagnosticSeverity.Warning,
           range: {
             start: {
@@ -390,10 +392,12 @@ describe('Standard English rule set tests', () => {
           },
           message: 'Incorrectly nested quotation mark.',
           source: 'quotation-mark-checker',
-          data: '',
+          data: {
+            depth: 2,
+          },
         },
         {
-          code: 'incorrectly-nested-quotation-mark-level-2',
+          code: 'incorrectly-nested-quotation-mark',
           severity: DiagnosticSeverity.Warning,
           range: {
             start: {
@@ -407,7 +411,9 @@ describe('Standard English rule set tests', () => {
           },
           message: 'Incorrectly nested quotation mark.',
           source: 'quotation-mark-checker',
-          data: '',
+          data: {
+            depth: 2,
+          },
         },
         {
           code: 'deeply-nested-quotation-mark',