Merge pull request #88 from Fabccc/main

feature: support null coalescing expression and array assignment

src/rules/e12.rs

@@ -3,7 +3,9 @@ use std::str;
 use php_parser_rs::parser::ast::arguments::{Argument, NamedArgument, PositionalArgument};
 use php_parser_rs::parser::ast::classes::{ClassMember, ClassStatement};
 use php_parser_rs::parser::ast::identifiers::Identifier;
-use php_parser_rs::parser::ast::operators::ArithmeticOperationExpression;
+use php_parser_rs::parser::ast::operators::{
+    ArithmeticOperationExpression, AssignmentOperationExpression,
+};
 use php_parser_rs::parser::ast::variables::Variable;
 use php_parser_rs::parser::ast::{
     ArrayItem, Expression, MethodCallExpression, PropertyFetchExpression, ShortArrayExpression,
@@ -90,6 +92,7 @@ impl crate::rules::Rule for Rule {
             }
             _ => None,
         };
+
         let mut flatten_property_expressions: Vec<&Expression> = Vec::new();
         if let Some(expression) = expression {
             flatten_property_expressions =
@@ -219,26 +222,25 @@ impl Rule {
 
         let expressions = match expression {
             Expression::AssignmentOperation(assignment) => {
-                let mut assigment_expressions = vec![assignment.left()];
-
-                let right = assignment.right();
-                match right {
+                self.get_assignment_expression(assignment)
+            }
+            Expression::Coalesce(coalesce) => {
+                let mut coalesce_expressions = vec![coalesce.rhs.as_ref()];
+                match coalesce.lhs.as_ref() {
                     Expression::PropertyFetch(_) => None,
+                    Expression::StaticPropertyFetch(_) => None,
                     _ => {
-                        assigment_expressions.push(right);
+                        coalesce_expressions.push(coalesce.lhs.as_ref());
                         Some(())
                     }
                 };
-
-                assigment_expressions
+                coalesce_expressions
             }
             Expression::Concat(concat) => vec![concat.left.as_ref(), concat.right.as_ref()],
             Expression::Parenthesized(parenthesized) => vec![parenthesized.expr.as_ref()],
-            Expression::ArithmeticOperation(arithmetic) => match arithmetic {
-                ArithmeticOperationExpression::PreIncrement { right, .. } => vec![right.as_ref()],
-                ArithmeticOperationExpression::PostIncrement { left, .. } => vec![left.as_ref()],
-                _ => vec![],
-            },
+            Expression::ArithmeticOperation(arithmetic) => {
+                self.get_arithmetic_expressions(arithmetic)
+            }
             Expression::ShortArray(short_array) => self.get_short_array_expressions(short_array),
             Expression::MethodCall(method_call) => self.get_method_expressions(method_call),
             _ => vec![],
@@ -274,6 +276,79 @@ impl Rule {
         expressions
     }
 
+    fn get_assignment_expression<'a>(
+        &'a self,
+        assignment: &'a AssignmentOperationExpression,
+    ) -> Vec<&Expression> {
+        let mut assigment_expressions = vec![];
+
+        let left = assignment.left();
+        match left {
+            Expression::ArrayIndex(array) => match array.array.as_ref() {
+                Expression::PropertyFetch(_) => {
+                    assigment_expressions.push(array.array.as_ref());
+                    Some(())
+                }
+                _ => None,
+            },
+            Expression::Coalesce(coalesce) => match coalesce.lhs.as_ref() {
+                Expression::ArrayIndex(array) => match array.array.as_ref() {
+                    Expression::PropertyFetch(_) => {
+                        assigment_expressions.push(array.array.as_ref());
+                        Some(())
+                    }
+                    _ => None,
+                },
+                _ => {
+                    assigment_expressions.push(coalesce.lhs.as_ref());
+                    Some(())
+                }
+            },
+            _ => {
+                assigment_expressions.push(&left);
+                Some(())
+            }
+        };
+
+        let right = assignment.right();
+        match right {
+            Expression::PropertyFetch(_) => None,
+            _ => {
+                assigment_expressions.push(&right);
+                Some(())
+            }
+        };
+
+        assigment_expressions
+    }
+
+    fn get_arithmetic_expressions<'a>(
+        &'a self,
+        arithmetic: &'a ArithmeticOperationExpression,
+    ) -> Vec<&Expression> {
+        let mut expressions = vec![];
+
+        match arithmetic {
+            ArithmeticOperationExpression::PreIncrement { right, .. } => match right.as_ref() {
+                Expression::ArrayIndex(array) => match array.array.as_ref() {
+                    Expression::PropertyFetch(_) => expressions.push(array.array.as_ref()),
+                    _ => {}
+                },
+                _ => expressions.push(right.as_ref()),
+            },
+            ArithmeticOperationExpression::PostIncrement { left, .. } => match left.as_ref() {
+                Expression::ArrayIndex(array) => match array.array.as_ref() {
+                    Expression::PropertyFetch(_) => expressions.push(array.array.as_ref()),
+                    _ => {}
+                },
+                _ => expressions.push(left.as_ref()),
+            },
+            _ => {}
+        }
+
+        expressions
+    }
+
     fn get_method_expressions<'a>(
         &'a self,
         method_call: &'a MethodCallExpression,
@@ -513,4 +588,89 @@ mod tests {
             "Setting service properties leads to issues with Shared Memory Model (FrankenPHP/Swoole/RoadRunner). Trying to set $this->counter property".to_string()
         );
     }
+
+    #[test]
+    fn set_in_null_coalescing() {
+        let violations = analyze_file_for_rule("e12/set_in_null_coalescing.php", CODE);
+
+        assert!(violations.len().gt(&0));
+        assert_eq!(
+            violations.first().unwrap().suggestion,
+            "Setting service properties leads to issues with Shared Memory Model (FrankenPHP/Swoole/RoadRunner). Trying to set $this->counter property".to_string()
+        );
+    }
+
+    #[test]
+    fn set_array_in_method() {
+        let violations = analyze_file_for_rule("e12/set_array_in_method.php", CODE);
+
+        assert!(violations.len().gt(&0));
+        assert_eq!(
+            violations.first().unwrap().suggestion,
+            "Setting service properties leads to issues with Shared Memory Model (FrankenPHP/Swoole/RoadRunner). Trying to set $this->counter property".to_string()
+        );
+    }
+
+    #[test]
+    fn set_array_in_null_coalescing() {
+        let violations = analyze_file_for_rule("e12/set_array_in_null_coalescing.php", CODE);
+
+        assert!(violations.len().gt(&0));
+        assert_eq!(
+            violations.first().unwrap().suggestion,
+            "Setting service properties leads to issues with Shared Memory Model (FrankenPHP/Swoole/RoadRunner). Trying to set $this->counter property".to_string()
+        );
+    }
+
+    #[test]
+    fn increment_array_in_method() {
+        let violations = analyze_file_for_rule("e12/increment_array_in_method.php", CODE);
+
+        assert!(violations.len().gt(&0));
+    }
+
+    #[test]
+    fn read_array_null_coalescing_or_null() {
+        let violations = analyze_file_for_rule("e12/read_array_null_coalescing_or_null.php", CODE);
+
+        assert!(violations.len().eq(&0));
+    }
+
+    #[test]
+    fn read_array_null_coalescing_or_array() {
+        let violations = analyze_file_for_rule("e12/read_array_null_coalescing_or_array.php", CODE);
+
+        assert!(violations.len().eq(&0));
+    }
+
+    #[test]
+    fn read_null_coalescing_or_throw() {
+        let violations = analyze_file_for_rule("e12/read_null_coalescing_or_throw.php", CODE);
+
+        assert!(violations.len().eq(&0));
+    }
+
+    #[test]
+    fn read_static_null_coalescing_or_throw() {
+        let violations =
+            analyze_file_for_rule("e12/read_static_null_coalescing_or_throw.php", CODE);
+
+        assert!(violations.len().eq(&0));
+    }
+
+    #[test]
+    fn read_static_array_null_coalescing_or_null() {
+        let violations =
+            analyze_file_for_rule("e12/read_static_array_null_coalescing_or_null.php", CODE);
+
+        assert!(violations.len().eq(&0));
+    }
+
+    #[test]
+    fn read_static_array_null_coalescing_or_array() {
+        let violations =
+            analyze_file_for_rule("e12/read_static_array_null_coalescing_or_array.php", CODE);
+
+        assert!(violations.len().eq(&0));
+    }
 }

src/rules/examples/e12/increment_array_in_method.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace App\Service\e12;
+
+class IncrementArrayInMethod {
+
+    private array $counter = [];
+
+    private function __construct(private bool $debug = false) {
+    }
+
+    public function incrementCounter(): void {
+        $this->counter[1]++;
+    }
+}

src/rules/examples/e12/read_array_null_coalescing_or_array.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace App\Service\e12;
+
+class ReadArrayNullCoalescingOrArray
+{
+    private array $variablesSet1 = [
+        'var1' => 'test1',
+    ];
+
+    private array $variablesSet2 = [
+        'var2' => 'test2',
+    ];
+
+    public function getVariable(string $key): string
+    {
+        return $this->variablesSet1[$key] ?? $this->variablesSet2[$key];
+    }
+}

src/rules/examples/e12/read_array_null_coalescing_or_null.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace App\Service\e12;
+
+class ReadArrayNullCoalescingOrNull
+{
+    private array $variables = [
+        'var1' => 'test1',
+        'var2' => 'test2',
+    ];
+
+    public function getVariable(string $key): ?string
+    {
+        return $this->variables[$key] ?? null;
+    }
+}

src/rules/examples/e12/read_null_coalescing_or_throw.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace App\Service\e12;
+
+class ReadArrayNullCoalescingOrThrow
+{
+    public function __construct(private ?string $guid = null)
+    {
+    }
+
+    public function getGuidNotNull(): string
+    {
+        return $this->guid ?? throw new \RuntimeException('Missing guid');
+    }
+}

src/rules/examples/e12/read_static_array_null_coalescing_or_array.php

@@ -0,0 +1,19 @@
+<?php
+
+namespace App\Service\e12;
+
+class ReadArrayNullCoalescingOrArray
+{
+    private array $variablesSet1 = [
+        'var1' => 'test1',
+    ];
+
+    private array $variablesSet2 = [
+        'var2' => 'test2',
+    ];
+
+    public static function getVariable(string $key): string
+    {
+        return self::$variablesSet1[$key] ?? self::$variablesSet2[$key];
+    }
+}

src/rules/examples/e12/read_static_array_null_coalescing_or_null.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace App\Service\e12;
+
+class ReadStaticArrayNullCoalescingOrNull
+{
+    private array $variables = [
+        'var1' => 'test1',
+        'var2' => 'test2',
+    ];
+
+    public static function getVariable(string $key): ?string
+    {
+        return self::$variables[$key] ?? null;
+    }
+}

src/rules/examples/e12/read_static_null_coalescing_or_throw.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace App\Service\e12;
+
+class ReadArrayNullCoalescingOrThrow
+{
+    private static ?string $guid = null;
+
+    public function __construct()
+    {
+    }
+
+    public static function getGuidNotNull(): string
+    {
+        return self::$guid ?? throw new \RuntimeException('Missing static guid');
+    }
+}

src/rules/examples/e12/set_array_in_method.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace App\Service\e12;
+
+class SetArrayInMethod {
+
+    private array $counter = [];
+
+    public function setCounter(int $key, int $val): void {
+        $this->counter[$key] = $val;
+    }
+}

src/rules/examples/e12/set_array_in_null_coalescing.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace App\Service\e12;
+
+class SetArrayInNullCoalescing {
+
+    private array $counter = [];
+
+    public function process(int $key, int $val): int {
+        return $this->counter[$key] ?? $this->counter[$key] = $val;
+    }
+}

src/rules/examples/e12/set_in_null_coalescing.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace App\Service\e12;
+
+class SetInNullCoalescing {
+
+    private int $counter;
+
+    public function process(int $value): int {
+        return $this->counter ?? $this->counter = $value;
+    }
+}