Skip to content

Commit 5da6519

Browse files
SeanTAllenSeanTAllen
committed
Add style/docstring-leading-blank lint rule
Flags docstrings where the opening """ is immediately followed by a blank line. The first line of content should begin on the line immediately after the opening """. Also adds regression tests for the style/blank-lines rule to verify that a leading blank line inside a docstring is not miscounted as a blank line between entities. Closes #5120
1 parent a125b78 commit 5da6519

File tree

5 files changed

+664
-0
lines changed

5 files changed

+664
-0
lines changed

‎tools/pony-lint/docstring_leading_blank.pony‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
use ast = "pony_compiler"
2+
3+
primitive DocstringLeadingBlank is ASTRule
4+
"""
5+
Flags docstrings where the opening `\"\"\"` is immediately followed by a
6+
blank line.
7+
8+
A leading blank line wastes vertical space and can confuse the
9+
`style/blank-lines` rule's entity-boundary counting. The first line of
10+
content should begin on the line immediately after the opening `\"\"\"`.
11+
12+
Types and methods annotated with `\nodoc\` are exempt, as are methods
13+
inside `\nodoc\`-annotated entities — consistent with
14+
`style/docstring-format`.
15+
"""
16+
fun id(): String val => "style/docstring-leading-blank"
17+
fun category(): String val => "style"
18+
19+
fun description(): String val =>
20+
"no blank line after opening \"\"\""
21+
22+
fun default_status(): RuleStatus => RuleOn
23+
24+
fun node_filter(): Array[ast.TokenId] val =>
25+
[
26+
ast.TokenIds.tk_class(); ast.TokenIds.tk_actor()
27+
ast.TokenIds.tk_primitive(); ast.TokenIds.tk_struct()
28+
ast.TokenIds.tk_trait(); ast.TokenIds.tk_interface()
29+
ast.TokenIds.tk_fun(); ast.TokenIds.tk_new(); ast.TokenIds.tk_be()
30+
]
31+
32+
fun check(node: ast.AST box, source: SourceFile val)
33+
: Array[Diagnostic val] val
34+
=>
35+
"""
36+
Check that a docstring does not have a blank line immediately after
37+
the opening `\"\"\"`. Skips `\nodoc\`-annotated nodes and methods
38+
inside `\nodoc\`-annotated entities.
39+
"""
40+
let token_id = node.id()
41+
let is_method =
42+
(token_id == ast.TokenIds.tk_fun())
43+
or (token_id == ast.TokenIds.tk_new())
44+
or (token_id == ast.TokenIds.tk_be())
45+
46+
// Skip \nodoc\-annotated nodes
47+
if node.has_annotation("nodoc") then
48+
return recover val Array[Diagnostic val] end
49+
end
50+
51+
if is_method then
52+
// Skip methods inside \nodoc\-annotated entities
53+
try
54+
let entity =
55+
(node.parent() as ast.AST).parent() as ast.AST
56+
if entity.has_annotation("nodoc") then
57+
return recover val Array[Diagnostic val] end
58+
end
59+
end
60+
end
61+
62+
// Find the docstring node
63+
let doc_node: ast.AST box =
64+
if is_method then
65+
let found =
66+
try
67+
let c7 = node(7)?
68+
if c7.id() == ast.TokenIds.tk_string() then
69+
c7
70+
else
71+
try
72+
let body = node(6)?
73+
if body.id() != ast.TokenIds.tk_none() then
74+
let b0 = body(0)?
75+
if b0.id() == ast.TokenIds.tk_string() then
76+
b0
77+
else
78+
return recover val Array[Diagnostic val] end
79+
end
80+
else
81+
return recover val Array[Diagnostic val] end
82+
end
83+
else
84+
return recover val Array[Diagnostic val] end
85+
end
86+
end
87+
else
88+
return recover val Array[Diagnostic val] end
89+
end
90+
found
91+
else
92+
try
93+
let c6 = node(6)?
94+
if c6.id() == ast.TokenIds.tk_none() then
95+
return recover val Array[Diagnostic val] end
96+
end
97+
c6
98+
else
99+
return recover val Array[Diagnostic val] end
100+
end
101+
end
102+
103+
// We have a docstring — check for a leading blank line
104+
let start_line = doc_node.line()
105+
if start_line == 0 then
106+
return recover val Array[Diagnostic val] end
107+
end
108+
109+
_check_leading_blank(source, start_line)
110+
111+
fun _check_leading_blank(source: SourceFile val, start_line: USize)
112+
: Array[Diagnostic val] val
113+
=>
114+
"""
115+
Check whether the line after the opening `\"\"\"` at `start_line`
116+
(1-based) is blank. Only fires when the opening `\"\"\"` is on its
117+
own line (multi-line docstring).
118+
"""
119+
let open_idx = start_line - 1
120+
let open_line =
121+
try source.lines(open_idx)?
122+
else return recover val Array[Diagnostic val] end
123+
end
124+
125+
// Only check multi-line docstrings where """ is on its own line
126+
if not _is_only_triple_quote(open_line) then
127+
return recover val Array[Diagnostic val] end
128+
end
129+
130+
// Check if the next line is blank
131+
try
132+
let next_line = source.lines(open_idx + 1)?
133+
if _is_blank(next_line) then
134+
return recover val
135+
[ Diagnostic(
136+
id(),
137+
"no blank line after opening \"\"\"",
138+
source.rel_path,
139+
start_line + 1,
140+
1)]
141+
end
142+
end
143+
end
144+
recover val Array[Diagnostic val] end
145+
146+
fun _is_only_triple_quote(line: String val): Bool =>
147+
"""
148+
Check if a line contains only whitespace and a single `\"\"\"`.
149+
"""
150+
var found_quote = false
151+
var i: USize = 0
152+
let size = line.size()
153+
while i < size do
154+
try
155+
let ch = line(i)?
156+
if (ch == '"') and ((i + 3) <= size)
157+
and (line(i + 1)? == '"') and (line(i + 2)? == '"')
158+
then
159+
if found_quote then return false end
160+
found_quote = true
161+
i = i + 3
162+
elseif (ch != ' ') and (ch != '\t') then
163+
return false
164+
else
165+
i = i + 1
166+
end
167+
else
168+
i = i + 1
169+
end
170+
end
171+
found_quote
172+
173+
fun _is_blank(line: String val): Bool =>
174+
"""
175+
Check if a line is blank (empty or all whitespace).
176+
"""
177+
var i: USize = 0
178+
while i < line.size() do
179+
try
180+
let ch = line(i)?
181+
if (ch != ' ') and (ch != '\t') then return false end
182+
end
183+
i = i + 1
184+
end
185+
true

‎tools/pony-lint/main.pony‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ actor Main
8888
.> push(DotSpacing)
8989
.> push(BlankLines)
9090
.> push(DocstringFormat)
91+
.> push(DocstringLeadingBlank)
9192
.> push(PackageDocstring)
9293
.> push(PreferChaining)
9394
.> push(OperatorSpacing)

‎tools/pony-lint/test/_test_blank_lines.pony‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,3 +716,190 @@ class \nodoc\ _TestBlankLinesMultiLineDocEntities is UnitTest
716716
else
717717
h.fail("compilation failed")
718718
end
719+
720+
class \nodoc\ _TestBlankLinesLeadingBlankDocEntitiesClean is UnitTest
721+
"""
722+
Entity with docstring that starts with a blank line after the
723+
opening \"\"\", followed by another entity with 1 blank line —
724+
should be clean (between-entity check). Regression test for
725+
#5120.
726+
"""
727+
fun name(): String =>
728+
"BlankLines: leading-blank doc entities 1 blank (AST pipeline)"
729+
730+
fun exclusion_group(): String => "ast-compile"
731+
732+
fun apply(h: TestHelper) =>
733+
let source: String val =
734+
"primitive Foo\n" +
735+
" \"\"\"\n" +
736+
"\n" +
737+
" Returned by a request interceptor.\n" +
738+
" \"\"\"\n" +
739+
"\n" +
740+
"class ref Bar\n" +
741+
" fun apply(): None => None\n"
742+
try
743+
(let program, let sf) = _ASTTestHelper.compile(h, source)?
744+
match program.package()
745+
| let pkg: ast.Package val =>
746+
match pkg.module()
747+
| let mod: ast.Module val =>
748+
let collector = _TestEntityCollector
749+
mod.ast.visit(collector)
750+
let entities = collector.entities()
751+
// Verify Foo's end_line covers the closing """
752+
try
753+
(_, _, let foo_start, let foo_end) = entities(0)?
754+
h.assert_eq[USize](1, foo_start)
755+
h.assert_eq[USize](
756+
5,
757+
foo_end,
758+
"Foo end_line should be line 5 (closing \"\"\")")
759+
else
760+
h.fail("could not access Foo entity")
761+
end
762+
let diags = lint.BlankLines.check_module(entities, sf)
763+
h.assert_eq[USize](0, diags.size())
764+
else
765+
h.fail("no module")
766+
end
767+
else
768+
h.fail("no package")
769+
end
770+
else
771+
h.fail("compilation failed")
772+
end
773+
774+
class \nodoc\ _TestBlankLinesLeadingBlankDocEntitiesNoBlank is UnitTest
775+
"""
776+
Entity with docstring that starts with a blank line after the
777+
opening \"\"\", followed by another entity with 0 blank lines —
778+
should be flagged. Verifies the leading blank line inside the
779+
docstring is not mistakenly counted as a blank line between
780+
entities. Regression test for #5120.
781+
"""
782+
fun name(): String =>
783+
"BlankLines: leading-blank doc entities 0 blanks flagged"
784+
785+
fun exclusion_group(): String => "ast-compile"
786+
787+
fun apply(h: TestHelper) =>
788+
let source: String val =
789+
"primitive Foo\n" +
790+
" \"\"\"\n" +
791+
"\n" +
792+
" Returned by a request interceptor.\n" +
793+
" \"\"\"\n" +
794+
"class ref Bar\n" +
795+
" fun apply(): None => None\n"
796+
try
797+
(let program, let sf) = _ASTTestHelper.compile(h, source)?
798+
match program.package()
799+
| let pkg: ast.Package val =>
800+
match pkg.module()
801+
| let mod: ast.Module val =>
802+
let collector = _TestEntityCollector
803+
mod.ast.visit(collector)
804+
let entities = collector.entities()
805+
let diags = lint.BlankLines.check_module(entities, sf)
806+
h.assert_eq[USize](1, diags.size())
807+
try
808+
h.assert_true(
809+
diags(0)?.message.contains("between entities"))
810+
else
811+
h.fail("could not access diagnostic")
812+
end
813+
else
814+
h.fail("no module")
815+
end
816+
else
817+
h.fail("no package")
818+
end
819+
else
820+
h.fail("compilation failed")
821+
end
822+
823+
class \nodoc\ _TestBlankLinesLeadingBlankDocMethodClean is UnitTest
824+
"""
825+
Abstract method with docstring that starts with a blank line
826+
after the opening \"\"\", followed by another method with
827+
1 blank line — should be clean (within-entity check).
828+
Regression test for #5120.
829+
"""
830+
fun name(): String =>
831+
"BlankLines: leading-blank doc method 1 blank is clean"
832+
833+
fun exclusion_group(): String => "ast-compile"
834+
835+
fun apply(h: TestHelper) =>
836+
let source: String val =
837+
"trait Foo\n" +
838+
" fun foo(): None\n" +
839+
" \"\"\"\n" +
840+
"\n" +
841+
" Foo docs.\n" +
842+
" \"\"\"\n" +
843+
"\n" +
844+
" fun bar(): None => None\n"
845+
try
846+
(let program, let sf) = _ASTTestHelper.compile(h, source)?
847+
match program.package()
848+
| let pkg: ast.Package val =>
849+
match pkg.module()
850+
| let mod: ast.Module val =>
851+
let diags = _CollectRuleDiags(mod, sf, lint.BlankLines)
852+
h.assert_eq[USize](0, diags.size())
853+
else
854+
h.fail("no module")
855+
end
856+
else
857+
h.fail("no package")
858+
end
859+
else
860+
h.fail("compilation failed")
861+
end
862+
863+
class \nodoc\ _TestBlankLinesLeadingBlankDocMethodNoBlank is UnitTest
864+
"""
865+
Abstract method with docstring that starts with a blank line
866+
after the opening \"\"\", followed by another method with
867+
0 blank lines — should be flagged. Regression test for #5120.
868+
"""
869+
fun name(): String =>
870+
"BlankLines: leading-blank doc method 0 blanks flagged"
871+
872+
fun exclusion_group(): String => "ast-compile"
873+
874+
fun apply(h: TestHelper) =>
875+
let source: String val =
876+
"trait Foo\n" +
877+
" fun foo(): None\n" +
878+
" \"\"\"\n" +
879+
"\n" +
880+
" Foo docs.\n" +
881+
" \"\"\"\n" +
882+
" fun bar(): None => None\n"
883+
try
884+
(let program, let sf) = _ASTTestHelper.compile(h, source)?
885+
match program.package()
886+
| let pkg: ast.Package val =>
887+
match pkg.module()
888+
| let mod: ast.Module val =>
889+
let diags = _CollectRuleDiags(mod, sf, lint.BlankLines)
890+
h.assert_true(diags.size() > 0)
891+
try
892+
h.assert_true(
893+
diags(0)?.message.contains("1 blank line"))
894+
else
895+
h.fail("could not access diagnostic")
896+
end
897+
else
898+
h.fail("no module")
899+
end
900+
else
901+
h.fail("no package")
902+
end
903+
else
904+
h.fail("compilation failed")
905+
end

0 commit comments

Comments
 (0)