Skip to content

Commit cca1d93

Browse files
committed
Refactor LL6 rule: handle function calls broken after opening paren
1 parent 698813a commit cca1d93

File tree

11 files changed

+292
-76
lines changed

11 files changed

+292
-76
lines changed

example_files/example_lint_cfg.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,6 @@
1313
"indent_paren_expr": {},
1414
"indent_switch_case": {},
1515
"indent_empty_loop": {},
16-
"indent_continuation_line": {}
16+
"indent_continuation_line": {},
17+
"func_call_break_on_open_paren": {}
1718
}

src/analysis/parsing/expression.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use crate::lint::{DMLStyleError,
2525
CurrentRules},
2626
AuxParams};
2727
use crate::lint::rules::indentation::IndentParenExprArgs;
28-
use crate::lint::rules::linebreaking::FuncCallBreakOnOpeningParenArgs;
28+
use crate::lint::rules::linebreaking::FuncCallBreakOnOpenParenArgs;
2929

3030
#[derive(Debug, Clone, PartialEq)]
3131
pub struct UnaryExpressionContent {
@@ -170,6 +170,8 @@ impl TreeElement for ParenExpressionContent {
170170
}
171171
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
172172
rules.indent_paren_expr.check(acc, IndentParenExprArgs::from_paren_expression(self));
173+
rules.func_call_break_on_open_paren.check(acc,
174+
FuncCallBreakOnOpenParenArgs::from_paren_expression(self, _aux.depth));
173175
}
174176
}
175177

@@ -220,8 +222,8 @@ impl TreeElement for FunctionCallContent {
220222
rules.nsp_inparen.check(acc, NspInparenArgs::from_function_call(self));
221223
rules.sp_punct.check(acc, SpPunctArgs::from_function_call(self));
222224
rules.indent_paren_expr.check(acc, IndentParenExprArgs::from_function_call(self));
223-
rules.func_call_break_on_opening_paren
224-
.check(acc, FuncCallBreakOnOpeningParenArgs::from_function_call(self, aux.depth));
225+
rules.func_call_break_on_open_paren
226+
.check(acc, FuncCallBreakOnOpenParenArgs::from_function_call(self, aux.depth));
225227
}
226228
}
227229

@@ -337,6 +339,8 @@ impl TreeElement for CastContent {
337339
}
338340
fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
339341
rules.indent_paren_expr.check(acc, IndentParenExprArgs::from_cast(self));
342+
rules.func_call_break_on_open_paren
343+
.check(acc, FuncCallBreakOnOpenParenArgs::from_cast(self, _aux.depth));
340344
}
341345
}
342346

src/analysis/parsing/structure.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use crate::analysis::parsing::parser::{doesnt_understand_tokens,
1616
FileParser, Parse, ParseContext,
1717
FileInfo};
1818
use crate::analysis::LocalDMLError;
19+
use crate::lint::rules::linebreaking::FuncCallBreakOnOpenParenArgs;
1920
use crate::lint::rules::spacing::{SpBracesArgs,
2021
NspInparenArgs,
2122
NspFunparArgs,
@@ -240,6 +241,7 @@ impl TreeElement for MethodContent {
240241
rules.nsp_inparen.check(acc, NspInparenArgs::from_method(self));
241242
rules.sp_punct.check(acc, SpPunctArgs::from_method(self));
242243
rules.indent_paren_expr.check(acc, IndentParenExprArgs::from_method(self));
244+
rules.func_call_break_on_open_paren.check(acc, FuncCallBreakOnOpenParenArgs::from_method(self, _aux.depth));
243245
}
244246
}
245247

src/lint/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::fmt;
22
use std::fs;
33
use std::path::{Path, PathBuf};
44
use log::{debug, error, trace};
5-
use rules::linebreaking::FuncCallBreakOnOpeningParenOptions;
5+
use rules::linebreaking::FuncCallBreakOnOpenParenOptions;
66
use serde::{Deserialize, Serialize};
77
use rules::{instantiate_rules, CurrentRules, RuleType};
88
use rules::{spacing::{SpBraceOptions, SpPunctOptions, NspFunparOptions,
@@ -77,7 +77,7 @@ pub struct LintCfg {
7777
#[serde(default)]
7878
pub indent_continuation_line: Option<IndentContinuationLineOptions>,
7979
#[serde(default)]
80-
pub func_call_break_on_opening_paren: Option<FuncCallBreakOnOpeningParenOptions>,
80+
pub func_call_break_on_open_paren: Option<FuncCallBreakOnOpenParenOptions>,
8181
}
8282

8383
impl Default for LintCfg {
@@ -98,7 +98,7 @@ impl Default for LintCfg {
9898
indent_switch_case: Some(IndentSwitchCaseOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
9999
indent_empty_loop: Some(IndentEmptyLoopOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
100100
indent_continuation_line: Some(IndentContinuationLineOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
101-
func_call_break_on_opening_paren: Some(FuncCallBreakOnOpeningParenOptions{}),
101+
func_call_break_on_open_paren: Some(FuncCallBreakOnOpenParenOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
102102
}
103103
}
104104
}

src/lint/rules/indentation.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,9 +505,14 @@ impl IndentParenExprRule {
505505
args: Option<IndentParenExprArgs>) {
506506
if !self.enabled { return; }
507507
let Some(args) = args else { return; };
508+
if args.members_ranges.is_empty() { return; }
508509
let expected_line_start = args.lparen.col_start.0 + 1;
509510
let mut last_row = args.lparen.row_start.0;
510-
511+
if last_row != args.members_ranges.first().unwrap().row_start.0 {
512+
// If the first member is not on the same line as the lparen,
513+
// then it is not a continuation line, so we do not check it.
514+
return;
515+
}
511516
for member_range in args.members_ranges {
512517
if member_range.row_start.0 != last_row {
513518
last_row = member_range.row_start.0;

src/lint/rules/linebreaking.rs

Lines changed: 99 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,86 @@
11
use serde::{Deserialize, Serialize};
22
use crate::analysis::parsing::lexer::TokenKind;
3+
use crate::analysis::parsing::structure::MethodContent;
34
use crate::analysis::parsing::tree::{ZeroRange, TreeElement};
4-
use crate::analysis::parsing::{expression::{CastContent, FunctionCallContent}};
5-
use crate::lint::{DMLStyleError, LintCfg, RuleType};
5+
use crate::analysis::parsing::expression::{CastContent, FunctionCallContent, ParenExpressionContent};
6+
use crate::lint::{DMLStyleError, RuleType};
67
use crate::lint::rules::indentation::IndentParenExprArgs;
78
use super::Rule;
89

9-
pub struct FuncCallBreakOnOpeningParen {
10+
pub const INDENTATION_LEVEL_DEFAULT: u32 = 4;
11+
12+
fn default_indentation_spaces() -> u32 {
13+
INDENTATION_LEVEL_DEFAULT
14+
}
15+
16+
pub struct FuncCallBreakOnOpenParenRule {
1017
pub enabled: bool,
18+
indentation_spaces: u32
1119
}
1220

13-
pub struct FuncCallBreakOnOpeningParenArgs {
14-
pub member_ranges: Vec<ZeroRange>,
21+
pub struct FuncCallBreakOnOpenParenArgs {
22+
pub members_ranges: Vec<ZeroRange>,
1523
pub expected_depth: u32,
24+
pub lparen: ZeroRange,
1625
}
1726

1827
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
19-
pub struct FuncCallBreakOnOpeningParenOptions{}
28+
pub struct FuncCallBreakOnOpenParenOptions{
29+
#[serde(default = "default_indentation_spaces")]
30+
pub indentation_spaces: u32,
31+
}
32+
33+
impl Rule for FuncCallBreakOnOpenParenRule {
34+
fn name() -> &'static str {
35+
"FUNC_CALL_BREAK_ON_OPEN_PAREN"
36+
}
37+
fn description() -> &'static str {
38+
"Function or method calls broken right after opening parenthesis should
39+
indent continuation lines one more level."
40+
}
41+
fn get_rule_type() -> RuleType {
42+
RuleType::LL6
43+
}
44+
}
2045

21-
// pub struct FunctionCallContent {
22-
// pub fun: Expression,
23-
// pub lparen: LeafToken,
24-
// pub arguments: Vec<(SingleInitializer, Option<LeafToken>)>,
25-
// pub rparen: LeafToken,
26-
// }
46+
impl FuncCallBreakOnOpenParenRule {
47+
pub fn from_options(options: &Option<FuncCallBreakOnOpenParenOptions>) -> FuncCallBreakOnOpenParenRule {
48+
match options {
49+
Some(options) => FuncCallBreakOnOpenParenRule {
50+
enabled: true,
51+
indentation_spaces: options.indentation_spaces
52+
},
53+
None => FuncCallBreakOnOpenParenRule {
54+
enabled: false,
55+
indentation_spaces: 0
56+
}
57+
}
58+
}
59+
60+
pub fn check(&self, acc: &mut Vec<DMLStyleError>, args: Option<FuncCallBreakOnOpenParenArgs>) {
61+
if !self.enabled { return; }
62+
let Some(args) = args else { return; };
63+
if args.members_ranges.is_empty() { return; }
64+
let expected_line_start = self.indentation_spaces * (args.expected_depth + 1);
65+
let mut last_row = args.lparen.row_start.0;
66+
if last_row == args.members_ranges.first().unwrap().row_start.0 {
67+
// If the first member is on the same line as the lparen, we don't
68+
// need to check it.
69+
return;
70+
}
71+
for member_range in args.members_ranges {
72+
if member_range.row_start.0 != last_row {
73+
last_row = member_range.row_start.0;
74+
if member_range.col_start.0 != expected_line_start {
75+
acc.push(self.create_err(member_range));
76+
}
77+
}
78+
}
79+
}
80+
}
2781

28-
impl FuncCallBreakOnOpeningParenArgs {
29-
pub fn from_function_call(node: &FunctionCallContent, depth: u32) -> Option<FuncCallBreakOnOpeningParenArgs> {
82+
impl FuncCallBreakOnOpenParenArgs {
83+
pub fn from_function_call(node: &FunctionCallContent, depth: u32) -> Option<FuncCallBreakOnOpenParenArgs> {
3084
let mut filtered_member_ranges: Vec<ZeroRange> = vec![];
3185
for (arg, _comma) in node.arguments.iter() {
3286
filtered_member_ranges.extend(
@@ -40,44 +94,44 @@ impl FuncCallBreakOnOpeningParenArgs {
4094
filtered_member_ranges.first()?.to_owned()) {
4195
return None
4296
}
43-
Some(FuncCallBreakOnOpeningParenArgs {
44-
member_ranges: filtered_member_ranges,
97+
Some(FuncCallBreakOnOpenParenArgs {
98+
members_ranges: filtered_member_ranges,
4599
expected_depth: depth,
100+
lparen: node.lparen.range(),
46101
})
47102
}
48-
}
49103

50-
impl FuncCallBreakOnOpeningParen {
51-
pub fn check(&self, acc: &mut Vec<DMLStyleError>, args: Option<FuncCallBreakOnOpeningParenArgs>) {
52-
if !self.enabled {
53-
return;
54-
}
55-
let Some(args) = args else { return; };
56-
// TODO : enable shared access to indentation_spaces param for all related rules
57-
let expected_line_start = args.expected_depth * 4; // Assuming 4 spaces per indent level
58-
let mut last_row = args.member_ranges.first().unwrap().row_start.0;
104+
pub fn from_paren_expression(node: &ParenExpressionContent, depth: u32) -> Option<FuncCallBreakOnOpenParenArgs> {
105+
Some(FuncCallBreakOnOpenParenArgs {
106+
members_ranges: IndentParenExprArgs::filter_out_parenthesized_tokens(node.expr.tokens())
107+
.iter().filter(|t| t.kind != TokenKind::RParen).map(|t| t.range).collect(),
108+
expected_depth: depth,
109+
lparen: node.lparen.range(),
110+
})
111+
}
59112

60-
for member_range in args.member_ranges {
61-
if member_range.row_start.0 != last_row {
62-
last_row = member_range.row_start.0;
63-
if member_range.col_start.0 != expected_line_start {
64-
acc.push(self.create_err(member_range));
65-
}
66-
}
113+
pub fn from_method(node: &MethodContent, depth: u32) -> Option<FuncCallBreakOnOpenParenArgs> {
114+
let mut filtered_member_ranges: Vec<ZeroRange> = vec![];
115+
for (arg, _comma) in node.arguments.iter() {
116+
filtered_member_ranges.extend(
117+
IndentParenExprArgs::filter_out_parenthesized_tokens(arg.tokens())
118+
.iter().filter(|t| t.kind != TokenKind::RParen).map(|t| t.range));
67119
}
120+
Some(FuncCallBreakOnOpenParenArgs {
121+
members_ranges: filtered_member_ranges,
122+
expected_depth: depth,
123+
lparen: node.lparen.range(),
124+
})
68125
}
69-
}
70126

71-
impl Rule for FuncCallBreakOnOpeningParen {
72-
fn name() -> &'static str {
73-
"FUNC_CALL_BREAK_ON_OPENING_PAREN"
74-
}
75-
fn description() -> &'static str {
76-
"Function or method calls broken right after opening parenthesis should
77-
indent continuation lines one more level."
78-
}
79-
fn get_rule_type() -> RuleType {
80-
RuleType::LL6
127+
pub fn from_cast(node: &CastContent, depth: u32) -> Option<FuncCallBreakOnOpenParenArgs> {
128+
let mut cast_member_tokens = node.from.tokens();
129+
cast_member_tokens.append(&mut node.to.tokens());
130+
Some(FuncCallBreakOnOpenParenArgs {
131+
members_ranges: IndentParenExprArgs::filter_out_parenthesized_tokens(cast_member_tokens)
132+
.iter().filter(|t| t.kind != TokenKind::RParen).map(|t| t.range).collect(),
133+
expected_depth: depth,
134+
lparen: node.lparen.range(),
135+
})
81136
}
82137
}
83-

src/lint/rules/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use spacing::{SpBracesRule,
99
SpPunctRule, NspFunparRule, NspInparenRule,
1010
NspUnaryRule, NspTrailingRule};
1111
use indentation::{LongLinesRule, IndentNoTabRule, IndentCodeBlockRule, IndentClosingBraceRule, IndentParenExprRule, IndentSwitchCaseRule, IndentEmptyLoopRule, IndentContinuationLineRule};
12-
use linebreaking::FuncCallBreakOnOpeningParen;
12+
use linebreaking::FuncCallBreakOnOpenParenRule;
1313
use crate::lint::{LintCfg, DMLStyleError};
1414
use crate::analysis::{LocalDMLError, parsing::tree::ZeroRange};
1515

@@ -28,7 +28,7 @@ pub struct CurrentRules {
2828
pub indent_switch_case: IndentSwitchCaseRule,
2929
pub indent_empty_loop: IndentEmptyLoopRule,
3030
pub indent_continuation_line: IndentContinuationLineRule,
31-
pub func_call_break_on_opening_paren: FuncCallBreakOnOpeningParen,
31+
pub func_call_break_on_open_paren: FuncCallBreakOnOpenParenRule,
3232
}
3333

3434
pub fn instantiate_rules(cfg: &LintCfg) -> CurrentRules {
@@ -47,7 +47,7 @@ pub fn instantiate_rules(cfg: &LintCfg) -> CurrentRules {
4747
indent_switch_case: IndentSwitchCaseRule::from_options(&cfg.indent_switch_case),
4848
indent_empty_loop: IndentEmptyLoopRule::from_options(&cfg.indent_empty_loop),
4949
indent_continuation_line: IndentContinuationLineRule::from_options(&cfg.indent_continuation_line),
50-
func_call_break_on_opening_paren: FuncCallBreakOnOpeningParen { enabled: cfg.func_call_break_on_opening_paren.is_some() },
50+
func_call_break_on_open_paren: FuncCallBreakOnOpenParenRule::from_options(&cfg.func_call_break_on_open_paren),
5151
}
5252
}
5353

src/lint/rules/tests/indentation/paren_expr.rs

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -90,20 +90,6 @@ fn funcall_paren_incorrect() {
9090
assert_snippet(FUNCALL_PAREN_INCORRECT, expected_errors, &rules);
9191
}
9292

93-
static FUNCALL_BROKEN_AFTER_PAREN_EXCEPTION_CORRECT: &str = "
94-
method effect() {
95-
callback(
96-
0xABC,
97-
identifier,
98-
false);
99-
}
100-
";
101-
#[test]
102-
fn funcall_broken_after_paren_exception_correct() {
103-
let rules = set_up();
104-
assert_snippet(FUNCALL_BROKEN_AFTER_PAREN_EXCEPTION_CORRECT, vec![], &rules);
105-
}
106-
10793
static FUNCALL_NESTED_PAREN_CORRECT: &str = "
10894
method effect() {
10995
callback(another_cb(varname,
@@ -361,8 +347,7 @@ fn switch_paren_incorrect() {
361347
}
362348

363349
static NESTED_PAREN_EXPR_CORRECT: &str = "
364-
param result = (
365-
(reg0.val
350+
param result = ((reg0.val
366351
* reg1.enable.val)
367352
&
368353
mask_reg
@@ -377,8 +362,7 @@ fn nested_paren_expr_correct(){
377362
}
378363

379364
static NESTED_PAREN_EXPR_INCORRECT: &str = "
380-
param result = (
381-
(reg0.val
365+
param result = ((reg0.val
382366
* reg1.enable.val)
383367
&
384368
mask_reg
@@ -391,8 +375,8 @@ fn nested_paren_expr_incorrect(){
391375
let rules = set_up();
392376
let expected_errors = define_expected_errors!(
393377
RuleType::IN5,
394-
(2, 2, 4, 5),
395-
(5, 5, 17, 25),
378+
(2, 2, 5, 6),
379+
(4, 4, 17, 25),
396380
);
397381
assert_snippet(NESTED_PAREN_EXPR_INCORRECT, expected_errors, &rules);
398382
}

0 commit comments

Comments
 (0)