Skip to content

Commit 0fd147a

Browse files
authored
Add style/call-argument-format lint rule (#4894)
Phase 3, PR 6 from the pony-lint design (Discussion #4844). Implements the call argument formatting rule from the style guide (STYLE_GUIDE.md lines 436-464). For multiline calls with 2+ positional arguments, the rule checks: 1. No arguments start on the `(` line — they should begin on the next line. 2. Arguments are either all on one continuation line, or each on its own line. Handles both regular calls (TK_CALL, position at `(`) and FFI calls (TK_FFICALL, scans from `@` to find `(`). Named/where arguments are not subject to layout checks per the style guide. Single-line calls, single-argument calls, and named-argument-only calls are skipped. Also reformats all existing pony-lint Diagnostic(...) and other multiline call patterns to comply with the new rule.
1 parent f115e47 commit 0fd147a

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+1279
-276
lines changed

tools/pony-lint/_unreachable.pony

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ primitive _Unreachable
1212
fun apply(loc: SourceLoc = __loc): None =>
1313
let url = "https://github.com/ponylang/ponyc/issues"
1414
let fmt = "Unreachable at %s:%zu\nPlease report: " + url + "\n"
15-
@fprintf(@pony_os_stderr(), fmt.cstring(),
16-
loc.file().cstring(), loc.line())
15+
@fprintf(
16+
@pony_os_stderr(),
17+
fmt.cstring(),
18+
loc.file().cstring(),
19+
loc.line())
1720
@exit(1)

tools/pony-lint/acronym_casing.pony

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,13 @@ primitive AcronymCasing is ASTRule
3535
match NamingHelpers.has_lowered_acronym(name)
3636
| let acronym: String val =>
3737
return recover val
38-
[ Diagnostic(id(),
38+
[ Diagnostic(
39+
id(),
3940
"acronym '" + acronym + "' in '" + name
4041
+ "' should be fully uppercased",
41-
source.rel_path, name_node.line(), name_node.pos())]
42+
source.rel_path,
43+
name_node.line(),
44+
name_node.pos())]
4245
end
4346
end
4447
end

tools/pony-lint/array_literal_format.pony

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,20 +49,26 @@ primitive ArrayLiteralFormat is ASTRule
4949
// If it's a hanging indent, report only that and skip spacing checks
5050
// since the user needs to restructure the line anyway.
5151
if not _is_first_nonws(line_text, open_col) then
52-
diags.push(Diagnostic(id(),
52+
diags.push(Diagnostic(
53+
id(),
5354
"opening '[' of multiline array must be the first"
5455
+ " non-whitespace on its line",
55-
source.rel_path, open_line, open_col))
56+
source.rel_path,
57+
open_line,
58+
open_col))
5659
return consume diags
5760
end
5861

5962
// Check 2: space after `[`
6063
let after_idx = open_col.usize() // 0-based index after `[`
6164
if after_idx < line_text.size() then
6265
if line_text(after_idx)? != ' ' then
63-
diags.push(Diagnostic(id(),
66+
diags.push(Diagnostic(
67+
id(),
6468
"space required after '[' in multiline array",
65-
source.rel_path, open_line, open_col))
69+
source.rel_path,
70+
open_line,
71+
open_col))
6672
end
6773
end
6874
end

tools/pony-lint/assignment_indent.pony

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,12 @@ primitive AssignmentIndent is ASTRule
5151
if mlv.max_line > assign_line then
5252
// Multiline RHS starting on the `=` line — violation
5353
recover val
54-
[ Diagnostic(id(),
54+
[ Diagnostic(
55+
id(),
5556
"multiline assignment RHS should start on the line after '='",
56-
source.rel_path, assign_line, node.pos())]
57+
source.rel_path,
58+
assign_line,
59+
node.pos())]
5760
end
5861
else
5962
// Single-line RHS — clean

tools/pony-lint/blank_lines.pony

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,12 @@ primitive BlankLines is ASTRule
7373
try
7474
let prev_line = source.lines(first_content_line - 2)?
7575
if _is_blank(prev_line) then
76-
result.push(Diagnostic(id(),
76+
result.push(Diagnostic(
77+
id(),
7778
"no blank line before first body content",
78-
source.rel_path, first_content_line - 1, 1))
79+
source.rel_path,
80+
first_content_line - 1,
81+
1))
7982
end
8083
end
8184
end
@@ -101,9 +104,12 @@ primitive BlankLines is ASTRule
101104
if prev_is_field and curr_is_field then
102105
// No blank lines between consecutive fields
103106
if blanks > 0 then
104-
result.push(Diagnostic(id(),
107+
result.push(Diagnostic(
108+
id(),
105109
"no blank lines between fields",
106-
source.rel_path, curr_start, curr_member.pos()))
110+
source.rel_path,
111+
curr_start,
112+
curr_member.pos()))
107113
end
108114
elseif curr_is_method then
109115
// One blank line before methods
@@ -113,15 +119,21 @@ primitive BlankLines is ASTRule
113119
if prev_is_method and prev_one_liner and curr_one_liner then
114120
// One-liner exception: both methods are one-liners
115121
if blanks > 1 then
116-
result.push(Diagnostic(id(),
122+
result.push(Diagnostic(
123+
id(),
117124
"at most 1 blank line before method",
118-
source.rel_path, curr_start, curr_member.pos()))
125+
source.rel_path,
126+
curr_start,
127+
curr_member.pos()))
119128
end
120129
else
121130
if blanks != 1 then
122-
result.push(Diagnostic(id(),
131+
result.push(Diagnostic(
132+
id(),
123133
"exactly 1 blank line before method",
124-
source.rel_path, curr_start, curr_member.pos()))
134+
source.rel_path,
135+
curr_start,
136+
curr_member.pos()))
125137
end
126138
end
127139
end
@@ -162,15 +174,21 @@ primitive BlankLines is ASTRule
162174
if a_one_liner and b_one_liner then
163175
// Both one-liners — 0 or 1 blank lines allowed
164176
if blanks > 1 then
165-
result.push(Diagnostic(id(),
177+
result.push(Diagnostic(
178+
id(),
166179
"at most 1 blank line between entities",
167-
source.rel_path, b_start, 1))
180+
source.rel_path,
181+
b_start,
182+
1))
168183
end
169184
else
170185
if blanks != 1 then
171-
result.push(Diagnostic(id(),
186+
result.push(Diagnostic(
187+
id(),
172188
"exactly 1 blank line between entities",
173-
source.rel_path, b_start, 1))
189+
source.rel_path,
190+
b_start,
191+
1))
174192
end
175193
end
176194
end
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
use ast = "pony_compiler"
2+
3+
primitive CallArgumentFormat is ASTRule
4+
"""
5+
Checks formatting of multiline function call arguments per the style guide.
6+
7+
For calls with two or more positional arguments that span multiple lines,
8+
checks:
9+
10+
- No arguments start on the `(` line — arguments should begin on the
11+
next line.
12+
- Arguments are either all on one continuation line, or each on its own
13+
line.
14+
15+
Single-line calls, single-argument calls, and named-argument-only calls
16+
are not checked. The `where` clause is allowed on its own line and is not
17+
subject to layout checks.
18+
19+
This rule does not check whether all-on-one-line arguments fit within the
20+
80-column limit — `style/line-length` catches that separately.
21+
22+
These rules apply to both regular calls and FFI calls.
23+
"""
24+
fun id(): String val => "style/call-argument-format"
25+
fun category(): String val => "style"
26+
27+
fun description(): String val =>
28+
"multiline call argument formatting"
29+
+ " (argument layout and placement)"
30+
31+
fun default_status(): RuleStatus => RuleOn
32+
33+
fun node_filter(): Array[ast.TokenId] val =>
34+
[ ast.TokenIds.tk_call()
35+
ast.TokenIds.tk_fficall() ]
36+
37+
fun check(node: ast.AST box, source: SourceFile val)
38+
: Array[Diagnostic val] val
39+
=>
40+
"""
41+
Check formatting of multiline call arguments.
42+
"""
43+
let is_fficall = node.id() == ast.TokenIds.tk_fficall()
44+
45+
// Get TK_POSITIONALARGS: child 1 for TK_CALL, child 2 for TK_FFICALL
46+
let pos_args_idx: USize = if is_fficall then 2 else 1 end
47+
let pos_args =
48+
try
49+
let child = node(pos_args_idx)?
50+
if child.id() == ast.TokenIds.tk_none() then
51+
return recover val Array[Diagnostic val] end
52+
end
53+
if child.id() != ast.TokenIds.tk_positionalargs() then
54+
return recover val Array[Diagnostic val] end
55+
end
56+
child
57+
else
58+
return recover val Array[Diagnostic val] end
59+
end
60+
61+
let num_args = pos_args.num_children()
62+
if num_args < 2 then
63+
return recover val Array[Diagnostic val] end
64+
end
65+
66+
// Find the '(' line and column
67+
(let open_line, let open_col) =
68+
_find_open_paren(node, source)
69+
70+
// Determine if multiline using _MaxLineVisitor seeded with '(' line
71+
let mlv = _MaxLineVisitor(open_line)
72+
node.visit(mlv)
73+
if mlv.max_line == open_line then
74+
// Single-line call
75+
return recover val Array[Diagnostic val] end
76+
end
77+
78+
let diags = recover iso Array[Diagnostic val] end
79+
80+
// Collect start lines for each positional arg
81+
let arg_lines = Array[USize](num_args)
82+
var i: USize = 0
83+
while i < num_args do
84+
try
85+
arg_lines.push(_arg_start_line(pos_args(i)?))
86+
else _Unreachable()
87+
end
88+
i = i + 1
89+
end
90+
91+
// Check 1: no arg should start on the '(' line
92+
for arg_line in arg_lines.values() do
93+
if arg_line == open_line then
94+
diags.push(Diagnostic(
95+
id(),
96+
"in a multiline call, arguments should start"
97+
+ " on the line after '('",
98+
source.rel_path,
99+
open_line,
100+
open_col))
101+
break // one diagnostic for this check
102+
end
103+
end
104+
105+
// Check 2: all on one line or each on own line
106+
let distinct = _count_distinct(arg_lines)
107+
if (distinct != 1) and (distinct != arg_lines.size()) then
108+
diags.push(Diagnostic(
109+
id(),
110+
"multiline call arguments must all be on one line"
111+
+ " or each on its own line",
112+
source.rel_path,
113+
open_line,
114+
open_col))
115+
end
116+
117+
consume diags
118+
119+
fun _find_open_paren(
120+
node: ast.AST box,
121+
source: SourceFile val)
122+
: (USize, USize)
123+
=>
124+
"""
125+
Return the `(1-based line, 1-based col)` of the `(` for a call.
126+
127+
For TK_CALL the node position is at `(` (INFIX_BUILD). For TK_FFICALL
128+
the node position is at `@`, so we scan forward through source text to
129+
find the first `(` character.
130+
"""
131+
if node.id() == ast.TokenIds.tk_call() then
132+
return (node.line(), node.pos())
133+
end
134+
135+
// TK_FFICALL: scan forward from '@' to find '('
136+
let start_line_idx = node.line() - 1 // 0-based
137+
var line_idx = start_line_idx
138+
while line_idx < source.lines.size() do
139+
try
140+
let line_text = source.lines(line_idx)?
141+
let col_start: USize =
142+
if line_idx == start_line_idx then
143+
node.pos() // 1-based col of '@', so 0-based index after '@'
144+
else
145+
0
146+
end
147+
var j: USize = col_start
148+
while j < line_text.size() do
149+
if line_text(j)? == '(' then
150+
return (line_idx + 1, j + 1) // convert to 1-based
151+
end
152+
j = j + 1
153+
end
154+
end
155+
line_idx = line_idx + 1
156+
end
157+
(node.line(), node.pos()) // fallback
158+
159+
fun _arg_start_line(node: ast.AST box): USize =>
160+
"""
161+
Find the visual start line of an argument expression.
162+
163+
Arguments are wrapped in TK_SEQ nodes, and the actual expression may be
164+
an infix-built node (like TK_CALL or TK_DOT) whose position is at the
165+
operator rather than the visual start. This function unwraps to find
166+
the leftmost source position.
167+
"""
168+
if node.id() == ast.TokenIds.tk_seq() then
169+
try return _arg_start_line(node(0)?) end
170+
end
171+
if node.infix_node() then
172+
try return _arg_start_line(node(0)?) end
173+
end
174+
node.line()
175+
176+
fun _count_distinct(lines: Array[USize]): USize =>
177+
"""
178+
Count distinct values in an array of line numbers.
179+
"""
180+
var count: USize = 0
181+
var i: USize = 0
182+
while i < lines.size() do
183+
try
184+
let line = lines(i)?
185+
var seen = false
186+
var j: USize = 0
187+
while j < i do
188+
try
189+
if lines(j)? == line then
190+
seen = true
191+
break
192+
end
193+
end
194+
j = j + 1
195+
end
196+
if not seen then count = count + 1 end
197+
end
198+
i = i + 1
199+
end
200+
count

tools/pony-lint/comment_spacing.pony

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,12 @@ primitive CommentSpacing is TextRule
3333
else
3434
match _find_comment(line)
3535
| let col: USize =>
36-
result.push(Diagnostic(id(), "expected one space after //",
37-
source.rel_path, idx + 1, col))
36+
result.push(Diagnostic(
37+
id(),
38+
"expected one space after //",
39+
source.rel_path,
40+
idx + 1,
41+
col))
3842
end
3943
end
4044
end

0 commit comments

Comments
 (0)