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/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;