Add sp_binop and sp_ternary rules to lint module checks (#19)

* Add sp_binop.rs to spacing rule tests module for BinaryExpressionContent
* Add sp_ternary rule for ? : conditionals for TertiaryExpressionContent

Author: jvsqzj

example_files/example_lint_cfg.json

@@ -1,6 +1,8 @@
 {
     "sp_brace": {},
     "sp_punct": {},
+    "sp_binop": {},
+    "sp_ternary" : {},
     "nsp_funpar": {},
     "nsp_inparen": {},
     "nsp_unary": {},

src/analysis/parsing/expression.rs

@@ -21,6 +21,8 @@ use crate::lint::{DMLStyleError,
                   rules::{spacing::{NspFunparArgs,
                                     NspInparenArgs,
                                     NspUnaryArgs,
+                                    SpBinopArgs,
+                                    SpTernaryArgs,
                                     SpPunctArgs},
                                     CurrentRules},
                                     AuxParams};
@@ -92,6 +94,9 @@ impl TreeElement for BinaryExpressionContent {
     fn subs(&self) -> TreeElements<'_> {
         create_subs!(&self.left, &self.operation, &self.right)
     }
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
+        rules.sp_binop.check(acc, SpBinopArgs::from_binary_expression_content(self));
+    }
 }
 
 #[derive(Debug, Clone, PartialEq)]
@@ -151,6 +156,9 @@ impl TreeElement for TertiaryExpressionContent {
         create_subs!(&self.left, &self.left_operation,
                      &self.middle, &self.right_operation, &self.right)
     }
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
+        rules.sp_ternary.check(acc, SpTernaryArgs::from_tertiary_expression_content(self));
+    }
 }
 
 #[derive(Debug, Clone, PartialEq)]

src/lint/features.md

@@ -3,6 +3,8 @@
 Below are listed the currently supported rules for linting:
 
 ## Spacing
+- SpBinop, `sp_binop`: spaces around binary operators except for derefencing operators (dot `a.b` and arrow `a->b` )
+- SpTernary, `sp_ternary`: spaces around `?` and `:` in ternary conditional expressions
 - SpBraces, `sp_brace`: spaces around braces (`{` and `}`)
 - SpPunct, `sp_punct`: spaces after but not before colon, semicolon and comma
 - NspFunpar, `nsp_funpar`: no spaces between a function/method name and its opening parenthesis

src/lint/mod.rs

@@ -2,10 +2,10 @@ use std::fmt;
 use std::fs;
 use std::path::{Path, PathBuf};
 use log::{debug, error, trace};
-use rules::spacing::{SpPtrDeclOptions, NspPtrDeclOptions};
 use serde::{Deserialize, Serialize};
 use rules::{instantiate_rules, CurrentRules, RuleType};
-use rules::{spacing::{SpBraceOptions, SpPunctOptions, NspFunparOptions,
+use rules::{spacing::{SpBraceOptions, SpPunctOptions, SpBinopOptions, NspFunparOptions,
+                      SpTernaryOptions, SpPtrDeclOptions, NspPtrDeclOptions,
                       NspInparenOptions, NspUnaryOptions, NspTrailingOptions},
                       indentation::{LongLineOptions, IndentSizeOptions, IndentCodeBlockOptions,
                                     IndentNoTabOptions, IndentClosingBraceOptions, IndentParenExprOptions, IndentSwitchCaseOptions, IndentEmptyLoopOptions},
@@ -51,6 +51,10 @@ pub struct LintCfg {
     #[serde(default)]
     pub sp_punct: Option<SpPunctOptions>,
     #[serde(default)]
+    pub sp_binop: Option<SpBinopOptions>,
+    #[serde(default)]
+    pub sp_ternary: Option<SpTernaryOptions>,
+    #[serde(default)]
     pub sp_ptrdecl: Option<SpPtrDeclOptions>,
     #[serde(default)]
     pub nsp_ptrdecl: Option<NspPtrDeclOptions>,
@@ -91,6 +95,8 @@ impl Default for LintCfg {
         LintCfg {
             sp_brace: Some(SpBraceOptions{}),
             sp_punct: Some(SpPunctOptions{}),
+            sp_binop: Some(SpBinopOptions{}),
+            sp_ternary: Some(SpTernaryOptions{}),
             sp_ptrdecl: Some(SpPtrDeclOptions{}),
             nsp_ptrdecl: Some(NspPtrDeclOptions{}),
             nsp_funpar: Some(NspFunparOptions{}),

src/lint/rules/mod.rs

@@ -4,14 +4,16 @@ pub mod indentation;
 #[cfg(test)]
 pub mod tests;
 
-use spacing::{NspFunparRule, NspInparenRule, NspTrailingRule, NspUnaryRule, SpBracesRule, SpPtrDeclRule, NspPtrDeclRule, SpPunctRule};
+use spacing::{NspFunparRule, NspInparenRule, NspTrailingRule, NspUnaryRule, SpBracesRule, SpBinopRule, SpTernaryRule, SpPtrDeclRule, NspPtrDeclRule, SpPunctRule};
 use indentation::{LongLinesRule, IndentNoTabRule, IndentCodeBlockRule, IndentClosingBraceRule, IndentParenExprRule, IndentSwitchCaseRule, IndentEmptyLoopRule};
 use crate::lint::{LintCfg, DMLStyleError};
 use crate::analysis::{LocalDMLError, parsing::tree::ZeroRange};
 
 pub struct CurrentRules {
     pub sp_brace: SpBracesRule,
     pub sp_punct: SpPunctRule,
+    pub sp_binop: SpBinopRule,
+    pub sp_ternary: SpTernaryRule,
     pub sp_ptrdecl: SpPtrDeclRule,
     pub nsp_ptrdecl: NspPtrDeclRule,
     pub nsp_funpar: NspFunparRule,
@@ -31,6 +33,8 @@ pub fn  instantiate_rules(cfg: &LintCfg) -> CurrentRules {
     CurrentRules {
         sp_brace: SpBracesRule { enabled: cfg.sp_brace.is_some() },
         sp_punct: SpPunctRule { enabled: cfg.sp_punct.is_some() },
+        sp_binop: SpBinopRule { enabled: cfg.sp_binop.is_some() },
+        sp_ternary: SpTernaryRule { enabled: cfg.sp_ternary.is_some() },
         sp_ptrdecl: SpPtrDeclRule { enabled: cfg.sp_ptrdecl.is_some() },
         nsp_ptrdecl: NspPtrDeclRule { enabled: cfg.nsp_ptrdecl.is_some() },
         nsp_funpar: NspFunparRule { enabled: cfg.nsp_funpar.is_some() },
@@ -68,6 +72,8 @@ pub trait Rule {
 pub enum RuleType {
     SpBraces,
     SpPunct,
+    SpBinop,
+    SpTernary,
     SpPtrDecl,
     NspPtrDecl,
     NspFunpar,

src/lint/rules/spacing.rs

@@ -8,8 +8,10 @@ use crate::analysis::parsing::types::{BitfieldsContent, LayoutContent,
 use crate::lint::{rules::{Rule, RuleType},
                   DMLStyleError};
 use crate::analysis::parsing::tree::{LeafToken, TreeElement, ZeroRange};
-use crate::analysis::parsing::expression::{FunctionCallContent, IndexContent,
+use crate::analysis::parsing::expression::{BinaryExpressionContent,
+                                           FunctionCallContent, IndexContent,
                                            PostUnaryExpressionContent,
+                                           TertiaryExpressionContent,
                                            UnaryExpressionContent};
 use crate::analysis::parsing::statement::{CompoundContent,
                                           ExpressionStmtContent,
@@ -127,6 +129,114 @@ impl Rule for SpBracesRule {
     }
 }
 
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
+pub struct SpBinopOptions {}
+pub struct SpBinopRule {
+    pub enabled: bool,
+}
+pub struct SpBinopArgs {
+    left: ZeroRange,
+    operator:  ZeroRange,
+    right: ZeroRange,
+}
+impl SpBinopArgs {
+    pub fn from_binary_expression_content(node: &BinaryExpressionContent) -> Option<SpBinopArgs> {
+        Some(SpBinopArgs {
+            left: node.left.range(),
+            operator: node.operation.range(),
+            right: node.right.range(),
+        })
+    }
+}
+impl SpBinopRule {
+    pub fn check(&self, acc: &mut Vec<DMLStyleError>,
+        ranges: Option<SpBinopArgs>) {
+        if !self.enabled { return; }
+        if let Some(location) = ranges {
+            if (location.left.row_end == location.operator.row_start)
+                && (location.left.col_end == location.operator.col_start) {
+                acc.push(self.create_err(location.left));
+            }
+            if (location.right.row_start == location.operator.row_end)
+                && (location.operator.col_end == location.right.col_start) {
+
+                acc.push(self.create_err(location.right));
+            }
+        }
+    }
+}
+impl Rule for SpBinopRule {
+    fn name() -> &'static str {
+        "SP_BINOP"
+    }
+    fn description() -> &'static str {
+        "Missing space around binary operator"
+    }
+    fn get_rule_type() -> RuleType {
+        RuleType::SpBinop
+    }
+}
+
+
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
+pub struct SpTernaryOptions {}
+pub struct SpTernaryRule {
+    pub enabled: bool,
+}
+pub struct SpTernaryArgs {
+    left: ZeroRange,
+    left_op: ZeroRange,
+    middle: ZeroRange,
+    right_op: ZeroRange,
+    right: ZeroRange,
+}
+impl SpTernaryArgs {
+    pub fn from_tertiary_expression_content(node: &TertiaryExpressionContent) -> Option<SpTernaryArgs> {
+        Some(SpTernaryArgs {
+            left: node.left.range(),
+            left_op: node.left_operation.range(),
+            middle: node.middle.range(),
+            right_op: node.right_operation.range(),
+            right: node.right.range(),
+        })
+    }
+}
+fn no_gap(left: ZeroRange, right: ZeroRange) -> bool {
+    left.row_end == right.row_start
+        && left.col_end == right.col_start
+}
+impl SpTernaryRule {
+    pub fn check(&self, acc: &mut Vec<DMLStyleError>,
+        ranges: Option<SpTernaryArgs>) {
+        if !self.enabled { return; }
+        if let Some(SpTernaryArgs { left, left_op, middle, right_op, right }) = ranges {
+            if no_gap(left, left_op) {
+                acc.push(self.create_err(left));
+            }
+            if no_gap(left_op, middle) {
+                acc.push(self.create_err(middle));
+            }
+            if no_gap(middle, right_op) {
+                acc.push(self.create_err(middle));
+            }
+            if no_gap(right_op, right) {
+                acc.push(self.create_err(right));
+            }
+        }
+    }
+}
+impl Rule for SpTernaryRule {
+    fn name() -> &'static str {
+        "SP_TERNARY"
+    }
+    fn description() -> &'static str {
+        "Missing space around ? or : in conditional expression"
+    }
+    fn get_rule_type() -> RuleType {
+        RuleType::SpTernary
+    }
+}
+
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct SpPunctOptions {}
 

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

@@ -1,6 +1,3 @@
-use crate::lint::rules::tests::common::assert_snippet;
-use crate::lint::rules::instantiate_rules;
-use crate::lint::LintCfg;
 mod nsp_funpar;
 mod nsp_inparen;
 mod nsp_ptrdecl;
@@ -9,6 +6,8 @@ mod nsp_unary;
 mod sp_braces;
 mod sp_ptrdecl;
 mod sp_punct;
+mod sp_binop;
+mod sp_ternary;
 
 // Put whitespace (space or newline):
 //  SP.reserved around reserved words, such as if, else, default,
@@ -23,34 +22,6 @@ if(this_some_integer == 0x666)
 }
 ";
 
-//  SP.binop around binary operators except the dereferencing operators dot
-//  (a.b) and arrow (a->b)
-#[allow(dead_code)]
-static SP_BINOP: &str = "
-method this_is_some_method() {
-local int this_some_integer = 5+6;
-if (this_some_integer == 0x666)
-    this_some_integer = this.val;
-}
-";
-
-//  SP.ternary around ? and : in the ternary ?: operator
-#[allow(dead_code)]
-static SP_TERNARY: &str = "
-method this_is_some_method(bool flag) {
-local int this_some_integer = (flag?5:7));
-}
-";
-
-//  SP.ptrdecl between a type and the * marking a pointer
-#[allow(dead_code)]
-static SP_PTRDECL: &str = "
-method this_is_some_method(conf_object_t* dummy_obj) {
-if(!dummy_obj)
-    return;
-}
-";
-
 //  SP.comment around the comment delimiters //, /* and **/
 #[allow(dead_code)]
 static SP_COMMENT: &str = "
@@ -62,11 +33,3 @@ if(!dummy_obj)//Not null
 }
 ";
 
-//  NSP.ptrdecl after the * marking a pointer in a declaration
-#[allow(dead_code)]
-static NSP_PTRDECL: &str = "
-method this_is_some_method(conf_object_t * dummy_obj) {
-if(!dummy_obj)
-    return;
-}
-";

src/lint/rules/tests/spacing/sp_binop.rs

@@ -0,0 +1,79 @@
+use crate::lint::rules::tests::common::{set_up, assert_snippet};
+use crate::lint::rules::RuleType;
+
+//  SP.binop around binary operators except the dereferencing operators dot
+//  (a.b) and arrow (a->b)
+static ARTITHMETIC_OPERATOR_CORRECT: &str = "
+method this_is_some_method() {
+    local int this_some_integer = 5 + 6;
+    if (this_some_integer == 0x666) {
+        this_some_integer = this.val;
+    }
+}
+";
+
+#[test]
+fn arithmetic_operator_correct() {
+	let rules = set_up();
+	assert_snippet(ARTITHMETIC_OPERATOR_CORRECT, vec![], &rules);
+}
+static ARTITHMETIC_OPERATOR_INCORRECT: &str = "
+method this_is_some_method() {
+    local int this_some_integer = 5+6;
+    if (this_some_integer == 0x666) {
+        this_some_integer = this.val;
+    }
+}
+";
+
+#[test]
+fn arithmetic_operator_incorrect() {
+	let rules = set_up();
+    let expected_errors = define_expected_errors!(
+		RuleType::SpBinop,
+		(2, 2, 34, 35),
+		(2, 2, 36, 37),
+	);
+	assert_snippet(ARTITHMETIC_OPERATOR_INCORRECT, expected_errors, &rules);
+}
+
+static CONDITIONAL_OPERATOR_INCORRECT: &str = "
+method this_is_some_method() {
+    local int this_some_integer = 5 + 6;
+    if (this_some_integer==0x666) {
+        this_some_integer = this.val;
+    }
+}
+";
+#[test]
+fn conditional_operator_incorrect() {
+	let rules = set_up();
+    let expected_errors = define_expected_errors!(
+		RuleType::SpBinop,
+		(3, 3, 8, 25),
+		(3, 3, 27, 32),
+	);
+	assert_snippet(CONDITIONAL_OPERATOR_INCORRECT, expected_errors, &rules);
+}
+
+static SP_BINOP: &str = "
+method this_is_some_method() {
+    local int this_some_integer = 5+6;
+    if (this_some_integer == 0x666) {
+        this_some_integer = this.val;
+    }
+}
+";
+
+#[test]
+fn rule_disable() {
+	let mut rules = set_up();
+	let expected_errors = define_expected_errors!(
+		RuleType::SpBinop,
+		(2, 2, 34, 35),
+		(2, 2, 36, 37),
+	);
+	assert_snippet(SP_BINOP, expected_errors, &rules);
+	rules.sp_binop.enabled = false;
+	assert_snippet(SP_BINOP, vec![], &rules);
+}