Skip to content

Commit 9a09671

Browse files
feat(linter): add support for PartialArgumentList
1 parent ba68e7e commit 9a09671

File tree

1 file changed

+60
-16
lines changed

1 file changed

+60
-16
lines changed

crates/linter/src/rule/correctness/no_assign_in_argument.rs

Lines changed: 60 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ use mago_reporting::Annotation;
77
use mago_reporting::Issue;
88
use mago_reporting::Level;
99
use mago_span::HasSpan;
10+
use mago_span::Span;
1011
use mago_syntax::ast::Assignment;
1112
use mago_syntax::ast::AssignmentOperator;
1213
use mago_syntax::ast::Expression;
1314
use mago_syntax::ast::Node;
1415
use mago_syntax::ast::NodeKind;
16+
use mago_syntax::ast::PartialArgument;
1517

1618
use crate::category::Category;
1719
use crate::context::LintContext;
@@ -72,15 +74,14 @@ impl LintRule for NoAssignInArgumentRule {
7274
foo($x = 5);
7375
"},
7476
category: Category::Correctness,
75-
7677
requirements: RuleRequirements::None,
7778
};
7879

7980
&META
8081
}
8182

8283
fn targets() -> &'static [NodeKind] {
83-
const TARGETS: &[NodeKind] = &[NodeKind::ArgumentList];
84+
const TARGETS: &[NodeKind] = &[NodeKind::ArgumentList, NodeKind::PartialArgumentList];
8485

8586
TARGETS
8687
}
@@ -90,31 +91,52 @@ impl LintRule for NoAssignInArgumentRule {
9091
}
9192

9293
fn check<'arena>(&self, ctx: &mut LintContext<'_, 'arena>, node: Node<'_, 'arena>) {
93-
let Node::ArgumentList(argument_list) = node else {
94-
return;
95-
};
96-
97-
for argument in argument_list.arguments.iter() {
98-
let value = argument.value();
94+
match node {
95+
Node::ArgumentList(list) => {
96+
for argument in list.arguments.iter() {
97+
self.check_expression_for_assignment(ctx, argument.value(), list.span());
98+
}
99+
}
100+
Node::PartialArgumentList(partial_list) => {
101+
// Check expression for assignment on non-placeholder arguments
102+
for argument in partial_list.arguments.iter() {
103+
let value = match argument {
104+
PartialArgument::Positional(pos) => &pos.value,
105+
PartialArgument::Named(named) => &named.value,
106+
_ => continue,
107+
};
108+
self.check_expression_for_assignment(ctx, value, partial_list.span());
109+
}
110+
}
111+
_ => (),
112+
}
113+
}
114+
}
99115

100-
if let Some(assignment) = get_assignment_from_expression(value) {
101-
let mut issue = Issue::new(self.cfg.level(), "Avoid assignments in function call arguments.")
116+
impl NoAssignInArgumentRule {
117+
fn check_expression_for_assignment<'arena>(
118+
&self,
119+
ctx: &mut LintContext<'_, 'arena>,
120+
expression: &Expression<'arena>,
121+
argument_list_span: Span,
122+
) {
123+
if let Some(assignment) = get_assignment_from_expression(expression) {
124+
let mut issue = Issue::new(self.cfg.level(), "Avoid assignments in function call arguments.")
102125
.with_code(self.meta.code)
103126
.with_annotation(
104127
Annotation::primary(assignment.span()).with_message("This is an assignment"),
105128
)
106129
.with_annotation(
107-
Annotation::secondary(argument_list.span()).with_message("In this argument list"),
130+
Annotation::secondary(argument_list_span).with_message("In this argument list"),
108131
)
109132
.with_note("Assigning a value within a function call argument can lead to unexpected behavior and make the code harder to read and understand.")
110133
.with_help("Consider assigning the variable before the function call.");
111134

112-
if matches!(&assignment.operator, AssignmentOperator::Assign(_)) {
113-
issue = issue.with_note("It's easy to confuse assignment (`=`) with comparison (`==`) in this context. Ensure you're using the correct operator.");
114-
}
115-
116-
ctx.collector.report(issue);
135+
if matches!(&assignment.operator, AssignmentOperator::Assign(_)) {
136+
issue = issue.with_note("It's easy to confuse assignment (`=`) with comparison (`==`) in this context. Ensure you're using the correct operator.");
117137
}
138+
139+
ctx.collector.report(issue);
118140
}
119141
}
120142
}
@@ -204,6 +226,17 @@ mod tests {
204226
"}
205227
}
206228

229+
test_lint_failure! {
230+
name = multiple_arguments_with_assignment_in_parial_arguments,
231+
rule = NoAssignInArgumentRule,
232+
count = 1,
233+
code = indoc! {r"
234+
<?php
235+
236+
foo(?, $a, $b = 5, $c);
237+
"}
238+
}
239+
207240
test_lint_success! {
208241
name = variable_used_as_argument,
209242
rule = NoAssignInArgumentRule,
@@ -215,6 +248,17 @@ mod tests {
215248
"}
216249
}
217250

251+
test_lint_success! {
252+
name = variable_used_as_argument_in_partial_arguments,
253+
rule = NoAssignInArgumentRule,
254+
code = indoc! {r"
255+
<?php
256+
257+
$x = 5;
258+
foo(?, $x);
259+
"}
260+
}
261+
218262
test_lint_success! {
219263
name = expression_without_assignment,
220264
rule = NoAssignInArgumentRule,

0 commit comments

Comments
 (0)