Fix problems with check result offset calculations and filtering (#1198)

c-sharp-tests/Checks/CheckRunResultTests.cs

@@ -28,13 +28,13 @@ bool expectedResult
         CheckLocation start1 = new(vrefStart1, 1);
         VerseRef vrefEnd1 = new("GEN 1:2");
         CheckLocation end1 = new(vrefEnd1, 5);
-        CheckRunResult checkRunResult1 = new("checkId", "projectId", "message", start1, end1);
+        CheckRunResult checkRunResult1 = new("checkId", "projectId", "message", "", start1, end1);
 
         VerseRef vrefStart2 = new(verseRefStart2);
         CheckLocation start2 = new(vrefStart2, offsetStart2);
         VerseRef vrefEnd2 = new(verseRefEnd2);
         CheckLocation end2 = new(vrefEnd2, offsetEnd2);
-        CheckRunResult checkRunResult2 = new(checkId2, projectId2, message2, start2, end2);
+        CheckRunResult checkRunResult2 = new(checkId2, projectId2, message2, "", start2, end2);
 
         Assert.That(checkRunResult1 == checkRunResult2, Is.EqualTo(expectedResult));
         Assert.That(checkRunResult1.Equals(checkRunResult2), Is.EqualTo(expectedResult));

c-sharp-tests/ParatextUtils/UsfnBookIndexerTests.cs

@@ -0,0 +1,103 @@
+using Paranext.DataProvider.ParatextUtils;
+
+namespace ParatextUtils.Tests;
+
+[TestFixture]
+public class UsfmBookIndexerTests
+{
+    private static readonly string BOOK_CONTENT = """
+\id PHM Philemon
+\h Philemon
+\toc1 Paul's Letter to Philemon
+\c 1
+\p
+\v 1 Verse 1
+\p
+\v 2 Verse 2
+\v 3 Verse 3
+\c 2
+\v 1 Another verse 1
+\v 2 Another verse 2
+""";
+
+    private UsfmBookIndexer _indexer;
+
+    [SetUp]
+    public void Setup()
+    {
+        _indexer = new UsfmBookIndexer(BOOK_CONTENT);
+    }
+
+    [TestCase(1, 0, 0)]
+    [TestCase(1, 1, 69)]
+    [TestCase(1, 2, 85)]
+    [TestCase(1, 3, 98)]
+    [TestCase(2, 0, 111)]
+    [TestCase(2, 1, 116)]
+    [TestCase(2, 2, 137)]
+    public void GetIndex_NormalInput_ReturnsNormalValues(
+        int chapterNum,
+        int verseNum,
+        int? expectedIndex
+    )
+    {
+        var result = _indexer.GetIndex(chapterNum, verseNum);
+        Assert.That(result, Is.Not.Null);
+        Assert.That(result, Is.EqualTo(expectedIndex));
+    }
+
+    [TestCase(1, 4)]
+    [TestCase(2, 3)]
+    [TestCase(3, 1)]
+    [TestCase(999999, 1)]
+    public void GetIndex_NormalInput_ReturnsNullValues(int chapterNum, int verseNum)
+    {
+        var result = _indexer.GetIndex(chapterNum, verseNum);
+        Assert.That(result, Is.Null);
+    }
+
+    [TestCase(0, 1)]
+    [TestCase(1, -1)]
+    [TestCase(-1, 1)]
+    [TestCase(-1, -1)]
+    public void GetIndex_InvalidInput_Throws(int chapterNum, int verseNum)
+    {
+        Assert.Throws<ArgumentOutOfRangeException>(() => _indexer.GetIndex(chapterNum, verseNum));
+    }
+
+    [TestCase(1, 0, 69)]
+    [TestCase(1, 1, 85)]
+    [TestCase(1, 2, 98)]
+    [TestCase(1, 3, 111)]
+    [TestCase(1, 999, 111)]
+    [TestCase(2, 0, 116)]
+    [TestCase(2, 1, 137)]
+    public void GetIndexFollowing_NormalInput_ReturnsNormalValues(
+        int chapterNum,
+        int verseNum,
+        int? expectedIndex
+    )
+    {
+        var result = _indexer.GetIndexFollowing(chapterNum, verseNum);
+        Assert.That(result, Is.Not.Null);
+        Assert.That(result, Is.EqualTo(expectedIndex));
+    }
+
+    [TestCase(2, 2)]
+    public void GetIndexFollowing_NormalInput_ReturnsNullValues(int chapterNum, int verseNum)
+    {
+        var result = _indexer.GetIndexFollowing(chapterNum, verseNum);
+        Assert.That(result, Is.Null);
+    }
+
+    [TestCase(0, 1)]
+    [TestCase(1, -1)]
+    [TestCase(-1, 1)]
+    [TestCase(-1, -1)]
+    public void GetIndexFollowing_InvalidInput_Throws(int chapterNum, int verseNum)
+    {
+        Assert.Throws<ArgumentOutOfRangeException>(
+            () => _indexer.GetIndexFollowing(chapterNum, verseNum)
+        );
+    }
+}

c-sharp-tests/Projects/LocalParatextProjectsTests.cs

@@ -1,5 +1,4 @@
 using System.Diagnostics.CodeAnalysis;
-using Paranext.DataProvider.ParatextUtils;
 using Paranext.DataProvider.Projects;
 
 namespace TestParanextDataProvider.Projects;

c-sharp/Checks/CheckLocation.cs

@@ -17,10 +17,10 @@ namespace Paranext.DataProvider.Checks;
 /// nothing about USJ and JsonPath, so we can only create <see cref="SIL.Scripture.VerseRef"/>
 /// representations in this class. See the TypeScript check data types for more details.
 /// </summary>
-public sealed class CheckLocation(VerseRef verseRef, int offset): IEquatable<CheckLocation>
+public sealed class CheckLocation(VerseRef verseRef, int offset) : IEquatable<CheckLocation>
 {
     public VerseRef VerseRef { get; } = verseRef;
-    public int Offset { get; } = offset;
+    public int Offset { get; set; } = offset;
 
     public override bool Equals(object? obj)
     {

c-sharp/Checks/CheckResultsRecorder.cs

@@ -1,3 +1,4 @@
+using Paranext.DataProvider.ParatextUtils;
 using Paratext.Checks;
 using Paratext.Data;
 using Paratext.Data.Checking;
@@ -15,7 +16,7 @@ namespace Paranext.DataProvider.Checks;
 /// </summary>
 public sealed class CheckResultsRecorder(string checkId, string projectId) : IRecordCheckError
 {
-    public int CurrentBookNumber { get; set; } = 0;
+    public List<CheckRunResult> CheckRunResults { get; } = [];
 
     public void RecordError(
         ITextToken token,
@@ -27,16 +28,18 @@ public void RecordError(
         VerseListItemType type = VerseListItemType.Error
     )
     {
-        var chapterVerse = token.ScrRefString.Split(":");
-        int chapterNumber = int.Parse(chapterVerse[0]);
-        int verseNumber = GetVerseNumber(chapterVerse[1]);
-        VerseRef verseRef = new (CurrentBookNumber, chapterNumber, verseNumber);
-        CheckRunResults.Add(new CheckRunResult(
-            checkId,
-            projectId,
-            message,
-            new CheckLocation(verseRef, offset),
-            new CheckLocation(verseRef, offset + length)));
+        CheckRunResults.Add(
+            new CheckRunResult(
+                checkId,
+                projectId,
+                message,
+                // ParatextData adds a space at the end sometimes that isn't in the text
+                token.Text.TrimEnd(),
+                // Actual offsets will be calculated below after results have been filtered
+                new CheckLocation(token.VerseRef, offset),
+                new CheckLocation(token.VerseRef, 0)
+            )
+        );
     }
 
     public void RecordError(
@@ -49,34 +52,77 @@ public void RecordError(
         VerseListItemType type = VerseListItemType.Error
     )
     {
-        CheckRunResults.Add(new CheckRunResult(
-            checkId,
-            projectId,
-            message,
-            new CheckLocation(vref, selectionStart),
-            new CheckLocation(vref, selectionStart + text.Length)));
+        CheckRunResults.Add(
+            new CheckRunResult(
+                checkId,
+                projectId,
+                message,
+                // ParatextData adds a space at the end sometimes that isn't in the text
+                text.TrimEnd(),
+                // Actual offsets will be calculated below after results have been filtered
+                new CheckLocation(vref, selectionStart),
+                new CheckLocation(vref, 0)
+            )
+        );
     }
 
-    public List<CheckRunResult> CheckRunResults { get; } = [];
-
-    private static int GetVerseNumber(string verseNumber)
+    /// <summary>
+    /// Remove all results that are within the given book and return them
+    /// </summary>
+    /// <returns>All results that were removed</returns>
+    public List<CheckRunResult> TrimResultsFromBook(int bookNum)
     {
-        ArgumentException.ThrowIfNullOrWhiteSpace(verseNumber);
-        if (!IsDigit(verseNumber[0]))
-            throw new ArgumentException($"verseNumber must start with an integer: {verseNumber}");
+        var retVal = new List<CheckRunResult>();
+        for (int i = CheckRunResults.Count - 1; i >= 0; i--)
+        {
+            var result = CheckRunResults[i];
+            var verseRef = result.Start.VerseRef;
+            if (verseRef.BookNum == bookNum)
+            {
+                retVal.Add(result);
+                CheckRunResults.RemoveAt(i);
+            }
+        }
+        return retVal;
+    }
 
-        int lastIndex = 1;
-        while (lastIndex < verseNumber.Length)
+    /// <summary>
+    /// Remove all results that are not within the given range
+    /// </summary>
+    public void FilterResults(CheckInputRange range)
+    {
+        for (int i = CheckRunResults.Count - 1; i >= 0; i--)
         {
-            if (!IsDigit(verseNumber[lastIndex]))
-                break;
-            lastIndex++;
+            var result = CheckRunResults[i];
+            var verseRef = result.Start.VerseRef;
+            if (!range.IsWithinRange(result.ProjectId, verseRef.BookNum, verseRef.ChapterNum))
+                CheckRunResults.RemoveAt(i);
         }
-        return int.Parse(verseNumber[..lastIndex]);
     }
 
-    private static bool IsDigit(char c)
+    /// <summary>
+    /// Given an indexed view of USFM text, determine the actual offsets to include for each result
+    /// </summary>
+    public void CalculateActualOffsets(UsfmBookIndexer indexer)
     {
-        return c >= '0' && c <= '9';
+        foreach (var result in CheckRunResults)
+        {
+            var verseIndex = indexer.GetIndex(result.Start.VerseRef);
+            if (!verseIndex.HasValue)
+            {
+                result.Start.Offset = 0;
+                continue;
+            }
+
+            var textIndex = indexer.Usfm.IndexOf(result.Text, verseIndex.Value);
+            if (textIndex < 0)
+            {
+                result.Start.Offset = 0;
+                continue;
+            }
+
+            result.Start.Offset += textIndex - verseIndex.Value;
+            result.End.Offset = result.Start.Offset + result.Text.Length;
+        }
     }
 }

c-sharp/Checks/CheckRunResult.cs

@@ -1,3 +1,5 @@
+using System.Text.Json.Serialization;
+
 namespace Paranext.DataProvider.Checks;
 
 /// <summary>
@@ -8,13 +10,17 @@ public sealed class CheckRunResult(
     string checkId,
     string projectId,
     string messageFormatString,
+    string text,
     CheckLocation start,
-    CheckLocation end)
-    : IEquatable<CheckRunResult>
+    CheckLocation end
+) : IEquatable<CheckRunResult>
 {
     public string CheckId { get; } = checkId;
     public string ProjectId { get; } = projectId;
     public string MessageFormatString { get; } = messageFormatString;
+
+    [JsonIgnore]
+    public string Text { get; } = text;
     public CheckLocation Start { get; } = start;
     public CheckLocation End { get; } = end;
 
@@ -29,10 +35,11 @@ public bool Equals(CheckRunResult? other)
             return false;
 
         return CheckId == other.CheckId
-           && ProjectId == other.ProjectId
-           && MessageFormatString == other.MessageFormatString
-           && Start == other.Start
-           && End == other.End;
+            && ProjectId == other.ProjectId
+            && MessageFormatString == other.MessageFormatString
+            && Text == other.Text
+            && Start == other.Start
+            && End == other.End;
     }
 
     public static bool operator ==(CheckRunResult a, CheckRunResult b)
@@ -47,6 +54,6 @@ public bool Equals(CheckRunResult? other)
 
     public override int GetHashCode()
     {
-        return HashCode.Combine(CheckId, ProjectId, MessageFormatString, Start, End);
+        return HashCode.Combine(CheckId, ProjectId, MessageFormatString, Text, Start, End);
     }
 }