Implement rule LL2 for breaking before binary ops

Author: jvsqzj

example_files/example_lint_cfg.json

@@ -22,5 +22,6 @@
     "func_call_break_on_open_paren": {},
     "method_output_break": {},
     "conditional_expression_break_before_op": {},
+    "break_before_binary_op": {},
     "annotate_lints": true
 }

src/analysis/parsing/expression.rs

@@ -27,7 +27,9 @@ use crate::lint::{DMLStyleError,
                                     CurrentRules},
                                     AuxParams};
 use crate::lint::rules::indentation::IndentParenExprArgs;
-use crate::lint::rules::linebreaking::{ConditionalExpressionBreakBeforeOperatorArgs, FuncCallBreakOnOpenParenArgs};
+use crate::lint::rules::linebreaking::{BreakBeforeBinaryOpArgs,
+    ConditionalExpressionBreakBeforeOperatorArgs,
+    FuncCallBreakOnOpenParenArgs};
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct UnaryExpressionContent {
@@ -97,6 +99,7 @@ impl TreeElement for BinaryExpressionContent {
     }
     fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
         rules.sp_binop.check(SpBinopArgs::from_binary_expression_content(self), acc);
+        rules.break_before_binary_op.check(BreakBeforeBinaryOpArgs::from_binary_expression(self), acc);
     }
 }
 

src/lint/features.md

@@ -47,6 +47,12 @@ Below are listed the currently supported rules for linting:
 
 ## Line Length
 - **LL1**, `long_lines`: Lines should be kept shorter than 80 characters. This limit can be set to a custom value
+- **LL2**, `break_before_binary_op`: Break long lines before binary operators, not after
+```
+    x = (a_very_long_expression
+         + another_very_long_expression)
+        * a_third_long_expression;
+```
 - **LL3**, `conditional_expression_break`: Break conditional expressions before the ?, or both before the ? and before the :.
 - **LL5**, `method_output_break`: Break long method declarations with output parameters before the arrow.
 ```

src/lint/mod.rs

@@ -5,7 +5,10 @@ use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use lazy_static::lazy_static;
 use log::{debug, error, trace};
-use rules::linebreaking::{FuncCallBreakOnOpenParenOptions, MethodOutputBreakOptions, ConditionalExpressionBreakBeforeOperatorOptions};
+use rules::linebreaking::{BreakBeforeBinaryOpOptions,
+    FuncCallBreakOnOpenParenOptions,
+    MethodOutputBreakOptions,
+    ConditionalExpressionBreakBeforeOperatorOptions};
 use serde::{Deserialize, Serialize};
 use regex::Regex;
 use rules::{instantiate_rules, CurrentRules, RuleType};
@@ -120,6 +123,8 @@ pub struct LintCfg {
     pub conditional_expression_break_before_op: Option<ConditionalExpressionBreakBeforeOperatorOptions>,
     #[serde(default)]
     pub method_output_break: Option<MethodOutputBreakOptions>,
+    #[serde(default)]
+    pub break_before_binary_op: Option<BreakBeforeBinaryOpOptions>,
     #[serde(default = "get_true")]
     pub annotate_lints: bool,
 }
@@ -169,6 +174,7 @@ impl Default for LintCfg {
             func_call_break_on_open_paren: Some(FuncCallBreakOnOpenParenOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
             method_output_break: Some(MethodOutputBreakOptions{}),
             conditional_expression_break_before_op: Some(ConditionalExpressionBreakBeforeOperatorOptions{}),
+            break_before_binary_op: Some(BreakBeforeBinaryOpOptions{}),
             annotate_lints: true,
         }
     }

src/lint/rules/linebreaking.rs

@@ -4,6 +4,7 @@ use crate::analysis::parsing::parser::Token;
 use crate::analysis::parsing::structure::MethodContent;
 use crate::analysis::parsing::tree::{ZeroRange, TreeElementTokenIterator, TreeElement};
 use crate::analysis::parsing::expression::{CastContent, FunctionCallContent,
+                                           BinaryExpressionContent,
                                            ParenExpressionContent, TertiaryExpressionContent};
 use crate::lint::{DMLStyleError, RuleType};
 use super::indentation::IndentParenExprArgs;
@@ -216,6 +217,60 @@ impl FuncCallBreakOnOpenParenRule {
     }
 }
 
+pub struct BreakBeforeBinaryOpRule {
+    pub enabled: bool,
+}
+
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
+pub struct BreakBeforeBinaryOpOptions {}
+
+pub struct BreakBeforeBinaryOpArgs {
+    pub left: ZeroRange,
+    pub operation: ZeroRange,
+    pub right: ZeroRange
+}
+
+impl BreakBeforeBinaryOpArgs {
+    pub fn from_binary_expression(node: &BinaryExpressionContent) 
+        -> Option<BreakBeforeBinaryOpArgs> {
+        Some(BreakBeforeBinaryOpArgs {
+            left: node.left.range(),
+            operation: node.operation.range(),
+            right: node.right.range(),
+        })
+    }
+}
+
+impl BreakBeforeBinaryOpRule {
+    pub fn from_options(options: &Option<BreakBeforeBinaryOpOptions>) -> BreakBeforeBinaryOpRule {
+        BreakBeforeBinaryOpRule {
+            enabled: options.is_some(),
+        }
+    }
+
+    pub fn check(&self, args: Option<BreakBeforeBinaryOpArgs>, acc: &mut Vec<DMLStyleError>) {
+        if !self.enabled { return; }
+        let Some(args) = args else { return; };
+        let binop_is_broken = args.left.row_start.0 != args.right.row_end.0;
+        let has_break_after_operator = args.operation.row_end.0 != args.right.row_start.0;
+        if binop_is_broken && has_break_after_operator {
+            acc.push(self.create_err(args.operation));
+        }
+    }
+}
+
+impl Rule for BreakBeforeBinaryOpRule {
+    fn name() -> &'static str {
+        "break_before_binary_op"
+    }
+    fn description() -> &'static str {
+        "Break binary expressions before the operator, not after."
+    }
+    fn get_rule_type() -> RuleType {
+        RuleType::LL2
+    }
+}
+
 pub struct ConditionalExpressionBreakBeforeOperatorRule {
     pub enabled: bool,
 }

src/lint/rules/mod.rs

@@ -24,9 +24,10 @@ use indentation::{LongLinesRule,
     IndentSwitchCaseRule,
     IndentEmptyLoopRule,
     IndentContinuationLineRule};
-use linebreaking::{FuncCallBreakOnOpenParenRule,
-    MethodOutputBreakRule,
-    ConditionalExpressionBreakBeforeOperatorRule};
+use linebreaking::{BreakBeforeBinaryOpRule,
+    FuncCallBreakOnOpenParenRule,
+    ConditionalExpressionBreakBeforeOperatorRule,
+    MethodOutputBreakRule};
 use crate::lint::{LintCfg, DMLStyleError};
 use crate::analysis::{LocalDMLError, parsing::tree::ZeroRange};
 
@@ -53,6 +54,7 @@ pub struct CurrentRules {
     pub func_call_break_on_open_paren: FuncCallBreakOnOpenParenRule,
     pub method_output_break: MethodOutputBreakRule,
     pub conditional_expression_break_before_op: ConditionalExpressionBreakBeforeOperatorRule,
+    pub break_before_binary_op: BreakBeforeBinaryOpRule, // Placeholder for future rule
 }
 
 pub fn  instantiate_rules(cfg: &LintCfg) -> CurrentRules {
@@ -79,6 +81,7 @@ pub fn  instantiate_rules(cfg: &LintCfg) -> CurrentRules {
         func_call_break_on_open_paren: FuncCallBreakOnOpenParenRule::from_options(&cfg.func_call_break_on_open_paren),
         method_output_break: MethodOutputBreakRule { enabled: cfg.method_output_break.is_some() },
         conditional_expression_break_before_op: ConditionalExpressionBreakBeforeOperatorRule::from_options(&cfg.conditional_expression_break_before_op),
+        break_before_binary_op: BreakBeforeBinaryOpRule { enabled: cfg.break_before_binary_op.is_some() },
     }
 }
 
@@ -113,6 +116,7 @@ pub enum RuleType {
     NspUnary,
     NspTrailing,
     LL1,
+    LL2,
     LL3,
     LL5,
     LL6,

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

@@ -15,8 +15,8 @@ fn continuation_line_correct() {
 
 static CONTINUATION_LINE_RETURN_CORRECT: &str = "
 method calculate_sum(uint64 a, uint64 b) -> (uint64) {
-    return (a + b) * (a - b) +
-        (a * b);
+    return (a + b) * (a - b)
+        + (a * b);
 }
 ";
 #[test]
@@ -29,8 +29,8 @@ static CONTINUATION_LINE_LOCAL_DECLA_CORRECT: &str = "
 bank regs {
     register example_register size 4 @ 0x00 {
         method read() -> (uint64) {
-            local uint64 value = (this.val + 10) *
-                (this.val - 5);
+            local uint64 value = (this.val + 10)
+                * (this.val - 5);
             return value;
         }
     }
@@ -87,8 +87,8 @@ fn continuation_line_incorrect() {
 pub static CONTINUATION_LINE_BITWISE_INCORRECT: &str = "
 method write(uint64 value) {
     local uint64 a = value;
-    local uint64 result = a <<
-                               2;
+    local uint64 result = a
+                               << 2;
     log info: 'Writing to register, result after left shift = %x', result;
 }
 ";
@@ -97,7 +97,7 @@ fn continuation_line_bitwise_incorrect() {
     let rules = set_up();
     let expected_errors = define_expected_errors!(
         RuleType::IN6,
-        (4, 4, 31, 32),
+        (4, 4, 31, 33),
     );
     assert_snippet(CONTINUATION_LINE_BITWISE_INCORRECT, expected_errors, &rules);
 }
@@ -106,8 +106,8 @@ pub static CONTINUATION_LINE_PARAM_INCORRECT: &str = "
 bank regs {
     register control size 4 @ 0x00 {
         field status @ [31:3] {
-            param init_val = 1 << 2 |
-                                            1 << 1;
+            param init_val = 1 << 2
+                                            | 1 << 1;
         }
     }
 }

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

@@ -130,8 +130,8 @@ fn funcall_nested_paren_incorrect() {
 
 static IF_PAREN_CORRECT: &str = "
 method callback() {
-    if (conditionX &&
-        conditionY) {
+    if (conditionX
+        && conditionY) {
         return;
     }
 }
@@ -144,8 +144,8 @@ fn if_paren_correct() {
 
 static IF_PAREN_INCORRECT: &str = "
 method callback() {
-    if (conditionX &&
-          conditionY) {
+    if (conditionX
+          && conditionY) {
         return;
     }
 }
@@ -155,7 +155,7 @@ fn if_paren_incorrect() {
     let rules = set_up();
     let expected_errors = define_expected_errors!(
         RuleType::IN5,
-        (3, 3, 10, 20),
+        (3, 3, 10, 12),
     );
     assert_snippet(IF_PAREN_INCORRECT, expected_errors, &rules);
 }
@@ -348,10 +348,8 @@ fn switch_paren_incorrect() {
 static NESTED_PAREN_EXPR_CORRECT: &str = "
 param result = ((reg0.val
                  * reg1.enable.val)
-                &
-                mask_reg
-                +
-                1);
+                & mask_reg
+                + 1);
 ";
 
 #[test]
@@ -363,18 +361,16 @@ fn nested_paren_expr_correct(){
 static NESTED_PAREN_EXPR_INCORRECT: &str = "
 param result = ((reg0.val
      * reg1.enable.val)
-                &
-                 mask_reg
-                +
-                1);
+                 & mask_reg
+                + 1);
 ";
 #[test]
 fn nested_paren_expr_incorrect(){
     let rules = set_up();
     let expected_errors = define_expected_errors!(
         RuleType::IN5,
         (2, 2, 5, 6),
-        (4, 4, 17, 25),
+        (3, 3, 17, 18),
     );
     assert_snippet(NESTED_PAREN_EXPR_INCORRECT, expected_errors, &rules);
 }

src/lint/rules/tests/line_length_breaking/func_call_break_open_paren.rs

@@ -59,10 +59,8 @@ static PAREN_NESTED_CORRECT: &str = "
 param result = (
     (reg0.val
      * reg1.enable.val)
-    &
-    mask_reg
-    +
-    1);
+    & mask_reg
+    + 1);
 ";
 
 #[test]
@@ -75,10 +73,8 @@ static PAREN_NESTED_INCORRECT: &str = "
 param result = (
     (reg0.val
      * reg1.enable.val)
-                &
-    mask_reg
-                +
-                1);
+                & mask_reg
+                + 1);
 ";
 
 #[test]
@@ -87,8 +83,7 @@ fn paren_nested_incorrect(){
     let expected_errors = define_expected_errors!(
         RuleType::LL6,
         (4, 4, 16, 17),
-        (6, 6, 16, 17),
-        (7, 7, 16, 17),
+        (5, 5, 16, 17),
     );
     assert_snippet(PAREN_NESTED_INCORRECT, expected_errors, &rules);
 }

src/lint/rules/tests/line_length_breaking/mod.rs

@@ -1,3 +1,4 @@
 mod func_call_break_open_paren;
 mod method_output_break;
 mod conditional_expression_break;
+mod break_before_binary_op;