Fix pony-lint blank-lines rule false positives on multi-line docstrings

_MaxLineVisitor only checked node.line() for AST nodes, which
reports only the start line of string literals. For multi-line
docstrings, this meant blank lines inside the docstring body were
miscounted as blank lines between members.

The fix uses end_pos() to get the actual end line for multi-line
TK_STRING nodes, scoped to strings whose token_value contains a
newline (single-line and empty strings skip end_pos() due to a
pre-existing parsing bug in that method for short strings).

Also restores multi-paragraph docstrings in pony-lsp that were
collapsed in #5098 as a workaround for this bug.

Closes #5099

Author: SeanTAllen

.release-notes/fix-blank-lines-docstring-counting.md

@@ -0,0 +1,3 @@
+## Fix pony-lint blank-lines rule false positives on multi-line docstrings
+
+The `style/blank-lines` rule incorrectly counted blank lines inside multi-line docstrings as blank lines between members. A method or field whose docstring contained blank lines (e.g., between paragraphs) would be flagged for having too many blank lines before the next member. The rule now correctly identifies where a docstring ends rather than using only its start line.

tools/pony-lint/_ast_dispatcher.pony

@@ -22,7 +22,33 @@ class ref _MaxLineVisitor is ast.ASTVisitor
     then
       return ast.Continue
     end
-    let l = node.line()
+    // Multi-line string literals (docstrings) can span multiple
+    // lines but node.line() only reports the opening """. Use
+    // end_pos() to get the actual end line so blank lines inside
+    // the docstring aren't miscounted as blank lines between
+    // members. Only do this for strings whose value contains a
+    // newline — single-line and empty strings use node.line()
+    // because end_pos() has a parsing bug with short strings.
+    let l =
+      if node.id() == ast.TokenIds.tk_string() then
+        let is_multiline =
+          match node.token_value()
+          | let s: String val => s.contains("\n")
+          else
+            false
+          end
+        if is_multiline then
+          match node.end_pos()
+          | let p: ast.Position => p.line()
+          else
+            node.line()
+          end
+        else
+          node.line()
+        end
+      else
+        node.line()
+      end
     if l > max_line then max_line = l end
     ast.Continue
 

tools/pony-lint/test/_test_blank_lines.pony

@@ -457,7 +457,26 @@ class \nodoc\ _TestMaxLineVisitor is ast.ASTVisitor
     then
       return ast.Continue
     end
-    let l = node.line()
+    let l =
+      if node.id() == ast.TokenIds.tk_string() then
+        let is_multiline =
+          match node.token_value()
+          | let s: String val => s.contains("\n")
+          else
+            false
+          end
+        if is_multiline then
+          match node.end_pos()
+          | let p: ast.Position => p.line()
+          else
+            node.line()
+          end
+        else
+          node.line()
+        end
+      else
+        node.line()
+      end
     if l > max_line then max_line = l end
     ast.Continue
 
@@ -526,3 +545,174 @@ class \nodoc\ _TestBlankLinesBetweenDocstringEntities is UnitTest
     else
       h.fail("compilation failed")
     end
+
+class \nodoc\ _TestBlankLinesMultiLineDocMethodClean is UnitTest
+  """
+  Abstract method with multi-line docstring containing blank
+  lines, followed by another method with 1 blank line — should
+  be clean. Uses a trait so the docstring is the last descendant
+  (no body expression to extend past it). Regression test for
+  #5099.
+  """
+  fun name(): String =>
+    "BlankLines: multi-line doc method 1 blank is clean"
+
+  fun exclusion_group(): String => "ast-compile"
+
+  fun apply(h: TestHelper) =>
+    let source: String val =
+      "trait Foo\n" +
+      "  fun foo(): None\n" +
+      "    \"\"\"\n" +
+      "    First paragraph.\n" +
+      "\n" +
+      "    Second paragraph.\n" +
+      "    \"\"\"\n" +
+      "\n" +
+      "  fun bar(): None => None\n"
+    try
+      (let program, let sf) = _ASTTestHelper.compile(h, source)?
+      match program.package()
+      | let pkg: ast.Package val =>
+        match pkg.module()
+        | let mod: ast.Module val =>
+          let diags = _CollectRuleDiags(mod, sf, lint.BlankLines)
+          h.assert_eq[USize](0, diags.size())
+        else
+          h.fail("no module")
+        end
+      else
+        h.fail("no package")
+      end
+    else
+      h.fail("compilation failed")
+    end
+
+class \nodoc\ _TestBlankLinesMultiLineDocMethodNoBlank is UnitTest
+  """
+  Abstract method with multi-line docstring containing blank
+  lines, followed by another method with 0 blank lines — should
+  be flagged. Verifies that end_pos() reports the correct end
+  line and doesn't overshoot. Regression test for #5099.
+  """
+  fun name(): String =>
+    "BlankLines: multi-line doc method 0 blanks flagged"
+
+  fun exclusion_group(): String => "ast-compile"
+
+  fun apply(h: TestHelper) =>
+    let source: String val =
+      "trait Foo\n" +
+      "  fun foo(): None\n" +
+      "    \"\"\"\n" +
+      "    First paragraph.\n" +
+      "\n" +
+      "    Second paragraph.\n" +
+      "    \"\"\"\n" +
+      "  fun bar(): None => None\n"
+    try
+      (let program, let sf) = _ASTTestHelper.compile(h, source)?
+      match program.package()
+      | let pkg: ast.Package val =>
+        match pkg.module()
+        | let mod: ast.Module val =>
+          let diags = _CollectRuleDiags(mod, sf, lint.BlankLines)
+          h.assert_true(diags.size() > 0)
+          try
+            h.assert_true(
+              diags(0)?.message.contains("1 blank line"))
+          else
+            h.fail("could not access diagnostic")
+          end
+        else
+          h.fail("no module")
+        end
+      else
+        h.fail("no package")
+      end
+    else
+      h.fail("compilation failed")
+    end
+
+class \nodoc\ _TestBlankLinesMultiLineDocFieldToMethod is UnitTest
+  """
+  Field with multi-line docstring containing blank lines,
+  followed by a method with 1 blank line — should be clean.
+  Regression test for #5099.
+  """
+  fun name(): String =>
+    "BlankLines: multi-line doc field->method 1 blank is clean"
+
+  fun exclusion_group(): String => "ast-compile"
+
+  fun apply(h: TestHelper) =>
+    let source: String val =
+      "class Foo\n" +
+      "  let x: U32 = 0\n" +
+      "    \"\"\"\n" +
+      "    First paragraph.\n" +
+      "\n" +
+      "    Second paragraph.\n" +
+      "    \"\"\"\n" +
+      "\n" +
+      "  fun apply(): U32 => x\n"
+    try
+      (let program, let sf) = _ASTTestHelper.compile(h, source)?
+      match program.package()
+      | let pkg: ast.Package val =>
+        match pkg.module()
+        | let mod: ast.Module val =>
+          let diags = _CollectRuleDiags(mod, sf, lint.BlankLines)
+          h.assert_eq[USize](0, diags.size())
+        else
+          h.fail("no module")
+        end
+      else
+        h.fail("no package")
+      end
+    else
+      h.fail("compilation failed")
+    end
+
+class \nodoc\ _TestBlankLinesMultiLineDocEntities is UnitTest
+  """
+  Entity with multi-line docstring containing blank lines,
+  followed by another entity with 1 blank line — should be
+  clean (between-entity check). Regression test for #5099.
+  """
+  fun name(): String =>
+    "BlankLines: multi-line doc entities 1 blank (AST pipeline)"
+
+  fun exclusion_group(): String => "ast-compile"
+
+  fun apply(h: TestHelper) =>
+    let source: String val =
+      "primitive Foo\n" +
+      "  \"\"\"\n" +
+      "  First paragraph.\n" +
+      "\n" +
+      "  Second paragraph.\n" +
+      "  \"\"\"\n" +
+      "\n" +
+      "primitive Bar\n" +
+      "  \"\"\"Bar docs.\"\"\"\n"
+    try
+      (let program, let sf) = _ASTTestHelper.compile(h, source)?
+      match program.package()
+      | let pkg: ast.Package val =>
+        match pkg.module()
+        | let mod: ast.Module val =>
+          let collector = _TestEntityCollector
+          mod.ast.visit(collector)
+          let entities = collector.entities()
+          let diags = lint.BlankLines.check_module(entities, sf)
+          h.assert_eq[USize](0, diags.size())
+        else
+          h.fail("no module")
+        end
+      else
+        h.fail("no package")
+      end
+    else
+      h.fail("compilation failed")
+    end

tools/pony-lint/test/main.pony

@@ -274,13 +274,17 @@ actor \nodoc\ Main is TestList
     test(_TestBlankLinesMultiLineMethodsOneBlank)
     test(_TestBlankLinesMultiLineMethodsNoBlank)
     test(_TestBlankLinesOneLineMethodsNoBlank)
+    test(_TestBlankLinesMultiLineDocMethodClean)
+    test(_TestBlankLinesMultiLineDocMethodNoBlank)
+    test(_TestBlankLinesMultiLineDocFieldToMethod)
 
     // BlankLines tests (between-entity)
     test(_TestBlankLinesBetweenEntitiesOneBlank)
     test(_TestBlankLinesBetweenEntitiesNoBlank)
     test(_TestBlankLinesBetweenEntitiesTooMany)
     test(_TestBlankLinesOneLinerEntitiesNoBlank)
     test(_TestBlankLinesBetweenDocstringEntities)
+    test(_TestBlankLinesMultiLineDocEntities)
 
     // IndentationSize tests
     test(_TestIndentationSizeClean)

tools/pony-lsp/compiler_notify.pony

@@ -179,7 +179,9 @@ trait tag LspCompiler
   be apply_settings(settings: (Settings | None))
     """
     Provide settings to initialize or reconfigure the
-    compiler. `None` can be provided when no new
+    compiler.
+
+    `None` can be provided when no new
     settings should be applied, but the initialization
     step should be completed.
     """

tools/pony-lsp/settings.pony

@@ -22,8 +22,10 @@ class val Settings
     """
     Additional ponypath entries, that will be added
     to the entries that pony-lsp adds on its own:
-    the stdlib path and path list extracted from the
-    `PONYPATH` environment variable.
+
+    1. the stdlib path
+    2. path list extracted from the `PONYPATH`
+       environment variable
     """
 
   new val from_json(data: JsonObject) =>