Add SP Reserved spacing rule (#21)

* Add SP Reserved spacing rule
* Add support for `while` `for` and `else`
* Add rule documentation
* Add rule to example_lint_cfg.json

---------

Co-authored-by: Vasquez Alfaro, Juan J <juan.j.vasquez.alfaro@intel.com>

Author: alecalvop

example_files/example_lint_cfg.json

@@ -1,4 +1,5 @@
 {
+    "sp_reserved": {},
     "sp_brace": {},
     "sp_punct": {},
     "sp_binop": {},

src/analysis/parsing/statement.rs

@@ -3,6 +3,7 @@
 use log::error;
 
 use crate::lint::rules::indentation::IndentEmptyLoopArgs;
+use crate::lint::rules::spacing::SpReservedArgs;
 use crate::span::Range;
 use crate::analysis::parsing::lexer::TokenKind;
 use crate::analysis::parsing::statement;
@@ -437,6 +438,7 @@ impl TreeElement for IfContent {
     fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
         rules.nsp_inparen.check(acc, NspInparenArgs::from_if(self));
         rules.indent_paren_expr.check(acc, IndentParenExprArgs::from_if(self));
+        rules.sp_reserved.check(acc, SpReservedArgs::from_if(self));
     }
 }
 
@@ -550,6 +552,7 @@ impl TreeElement for WhileContent {
     fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
         rules.indent_paren_expr.check(acc, IndentParenExprArgs::from_while(self));
         rules.indent_empty_loop.check(acc, IndentEmptyLoopArgs::from_while_content(self, aux.depth));
+        rules.sp_reserved.check(acc, SpReservedArgs::from_while(self));
     }
 }
 
@@ -865,6 +868,7 @@ impl TreeElement for ForContent {
     fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
         rules.indent_paren_expr.check(acc, IndentParenExprArgs::from_for(self));
         rules.indent_empty_loop.check(acc, IndentEmptyLoopArgs::from_for_content(self, aux.depth));
+        rules.sp_reserved.check(acc, SpReservedArgs::from_for(self));
     }
 }
 
@@ -1284,6 +1288,9 @@ impl TreeElement for AfterContent {
                      &self.callexpression,
                      &self.semi)
     }
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
+        rules.sp_reserved.check(acc, SpReservedArgs::from_after_content(self));
+    }
 }
 
 impl Parse<StatementContent> for AfterContent {

src/lint/features.md

@@ -3,6 +3,7 @@
 Below are listed the currently supported rules for linting:
 
 ## Spacing
+- SpReserved, `sp_reserved`: around reserved words, such as `if`, `else`, `default`, `size`, `const` and `in`, except when a reserved word is used as an identifier (e.g., `local uint8 *data;`). Currently supported reserved words: `if`, `for` and `while`.
 - 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 `}`)

src/lint/mod.rs

@@ -4,11 +4,20 @@ use std::path::{Path, PathBuf};
 use log::{debug, error, trace};
 use serde::{Deserialize, Serialize};
 use rules::{instantiate_rules, CurrentRules, RuleType};
-use rules::{spacing::{SpBraceOptions, SpPunctOptions, SpBinopOptions, NspFunparOptions,
-                      SpTernaryOptions, SpPtrDeclOptions, NspPtrDeclOptions,
-                      NspInparenOptions, NspUnaryOptions, NspTrailingOptions},
-                      indentation::{LongLineOptions, IndentSizeOptions, IndentCodeBlockOptions,
-                                    IndentNoTabOptions, IndentClosingBraceOptions, IndentParenExprOptions, IndentSwitchCaseOptions, IndentEmptyLoopOptions},
+use rules::{spacing::{SpReservedOptions,
+                      SpBraceOptions,
+                      SpPunctOptions,
+                      SpBinopOptions,
+                      NspFunparOptions,
+                      SpTernaryOptions,
+                      SpPtrDeclOptions,
+                      NspPtrDeclOptions,
+                      NspInparenOptions,
+                      NspUnaryOptions,
+                      NspTrailingOptions},
+            indentation::{LongLineOptions, IndentSizeOptions, IndentCodeBlockOptions,
+                          IndentNoTabOptions, IndentClosingBraceOptions, IndentParenExprOptions,
+                          IndentSwitchCaseOptions, IndentEmptyLoopOptions},
                     };
 use crate::analysis::{DMLError, IsolatedAnalysis, LocalDMLError};
 use crate::analysis::parsing::tree::TreeElement;
@@ -46,6 +55,8 @@ pub fn maybe_parse_lint_cfg(path: PathBuf) -> Option<LintCfg> {
 #[serde(default)]
 #[serde(deny_unknown_fields)]
 pub struct LintCfg {
+    #[serde(default)]
+    pub sp_reserved: Option<SpReservedOptions>,
     #[serde(default)]
     pub sp_brace: Option<SpBraceOptions>,
     #[serde(default)]
@@ -93,6 +104,7 @@ fn get_true() -> bool {
 impl Default for LintCfg {
     fn default() -> LintCfg {
         LintCfg {
+            sp_reserved: Some(SpReservedOptions{}),
             sp_brace: Some(SpBraceOptions{}),
             sp_punct: Some(SpPunctOptions{}),
             sp_binop: Some(SpBinopOptions{}),
@@ -242,6 +254,8 @@ pub mod tests {
                                    env!("CARGO_MANIFEST_DIR"),
                                    EXAMPLE_CFG);
         let example_cfg = parse_lint_cfg(example_path.into()).unwrap();
+        println!("Example LintCfg: {:#?}", example_cfg);
+        println!("LintCfg::default(): {:#?}", LintCfg::default());
         assert_eq!(example_cfg, LintCfg::default());
     }
 }

src/lint/rules/mod.rs

@@ -4,12 +4,23 @@ pub mod indentation;
 #[cfg(test)]
 pub mod tests;
 
-use spacing::{NspFunparRule, NspInparenRule, NspTrailingRule, NspUnaryRule, SpBracesRule, SpBinopRule, SpTernaryRule, SpPtrDeclRule, NspPtrDeclRule, SpPunctRule};
+use spacing::{NspFunparRule,
+              NspInparenRule,
+              NspTrailingRule,
+              NspUnaryRule,
+              SpBracesRule,
+              SpBinopRule,
+              SpTernaryRule,
+              SpPtrDeclRule,
+              NspPtrDeclRule,
+              SpPunctRule,
+              SpReservedRule};
 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_reserved: SpReservedRule,
     pub sp_brace: SpBracesRule,
     pub sp_punct: SpPunctRule,
     pub sp_binop: SpBinopRule,
@@ -31,6 +42,7 @@ pub struct CurrentRules {
 
 pub fn  instantiate_rules(cfg: &LintCfg) -> CurrentRules {
     CurrentRules {
+        sp_reserved: SpReservedRule { enabled: cfg.sp_reserved.is_some() },
         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() },
@@ -70,6 +82,7 @@ pub trait Rule {
 
 #[derive(PartialEq, Debug, Clone, Eq, Hash)]
 pub enum RuleType {
+    SpReserved,
     SpBraces,
     SpPunct,
     SpBinop,

src/lint/rules/spacing.rs

@@ -13,14 +13,117 @@ use crate::analysis::parsing::expression::{BinaryExpressionContent,
                                            PostUnaryExpressionContent,
                                            TertiaryExpressionContent,
                                            UnaryExpressionContent};
-use crate::analysis::parsing::statement::{CompoundContent,
-                                          ExpressionStmtContent,
-                                          IfContent, VariableDeclContent};
+use crate::analysis::parsing::statement::{AfterContent, CompoundContent, ExpressionStmtContent,
+                                          ForContent, IfContent, VariableDeclContent,
+                                          WhileContent};
 use crate::analysis::parsing::structure::{MethodContent,
                                           ObjectStatementsContent};
 
 use crate::span::{ZeroIndexed, Range};
 
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
+pub struct SpReservedOptions {}
+
+pub struct SpReservedRule {
+    pub enabled: bool,
+}
+pub struct SpReservedArgs {
+    before_range: Option<ZeroRange>,
+    token_range: ZeroRange,
+    after_range: Option<ZeroRange>,
+}
+impl SpReservedArgs {
+    pub fn from_after_content(node: &AfterContent) -> Vec<SpReservedArgs> {
+        let mut args_list = vec![];
+        if let Some(timer) = &node.timer {
+            args_list.push(SpReservedArgs {
+                before_range: None,
+                token_range: node.after.range(),
+                after_range: Some(timer.range()),
+            });
+        }
+        args_list
+    }
+    pub fn from_if(node: &IfContent) -> Vec<SpReservedArgs> {
+        let mut args_list = vec![];
+
+        args_list.push(SpReservedArgs {
+            before_range: None,
+            token_range: node.iftok.range(),
+            after_range: Some(node.lparen.range()),
+        });
+
+        if let Some((else_tok, elsebranch)) = &node.elsebranch {
+            args_list.push(SpReservedArgs {
+                before_range: Some(node.truebranch.range()),
+                token_range: else_tok.range(),
+                after_range: Some(elsebranch.range()),
+            });
+        }
+
+        args_list
+    }
+    pub fn from_for(node: &ForContent) -> Vec<SpReservedArgs> {
+        let mut args_list = vec![];
+        args_list.push(SpReservedArgs {
+            before_range: None,
+            token_range: node.fortok.range(),
+            after_range: Some(node.lparen.range()),
+        });
+        args_list
+    }
+    pub fn from_while(node: &WhileContent) -> Vec<SpReservedArgs> {
+        let mut args_list = vec![];
+        args_list.push(SpReservedArgs {
+            before_range: None,
+            token_range: node.whiletok.range(),
+            after_range: Some(node.lparen.range()),
+        });
+        args_list
+    }
+}
+
+impl SpReservedRule {
+    pub fn check(&self, acc: &mut Vec<DMLStyleError>,
+        args: Vec<SpReservedArgs>) {
+        if !self.enabled { return; }
+        for arg in args {
+            if let Some(before_range) = &arg.before_range {
+                if (before_range.row_end == arg.token_range.row_start)
+                    && (before_range.col_end == arg.token_range.col_start) {
+                    acc.push(
+                        self.create_err(Range::combine(
+                            *before_range, arg.token_range
+                        ))
+                    );
+                }
+            }
+            if let Some(after_range) = &arg.after_range {
+                if (arg.token_range.row_end == after_range.row_start)
+                    && (arg.token_range.col_end == after_range.col_start) {
+                    acc.push(
+                        self.create_err(Range::combine(
+                            arg.token_range, *after_range
+                        ))
+                    );
+                }
+            }
+        }
+    }
+}
+
+impl Rule for SpReservedRule {
+    fn name() -> &'static str {
+        "SP_RESERVED"
+    }
+    fn description() -> &'static str {
+        "Missing space around reserved words"
+    }
+    fn get_rule_type() -> RuleType {
+        RuleType::SpReserved
+    }
+}
+
 #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)]
 pub struct SpBraceOptions {}
 

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

@@ -6,29 +6,18 @@ mod nsp_unary;
 mod sp_braces;
 mod sp_ptrdecl;
 mod sp_punct;
+mod sp_reserved;
 mod sp_binop;
 mod sp_ternary;
 
-// Put whitespace (space or newline):
-//  SP.reserved around reserved words, such as if, else, default,
-//  size, const and in, except when a reserved word is used as an identifier
-//  (e.g., local uint8 *data;)
-#[allow(dead_code)]
-static SP_RESERVED: &str = "
-method this_is_some_method() {
-local int this_some_integer = 0x666;
-if(this_some_integer == 0x666)
-    return;
-}
-";
 
 //  SP.comment around the comment delimiters //, /* and **/
 #[allow(dead_code)]
 static SP_COMMENT: &str = "
 /*Function
 documentation*/
 method this_is_some_method(conf_object_t *dummy_obj) {
-if(!dummy_obj)//Not null
+if (!dummy_obj)//Not null
     return;
 }
 ";

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

@@ -5,7 +5,7 @@ use crate::lint::rules::RuleType;
 //  NSP.funpar between a function/method name and its opening parenthesis
 static NO_SPACE_METHOD_FUNC_INCORRECT: &str = "
 method this_is_some_method (conf_object_t *dummy_obj) {
-    if(!dummy_obj)
+    if (!dummy_obj)
         other_method_called ();
 }
 ";
@@ -25,7 +25,7 @@ fn no_space_method_func_incorrect() {
 
 static NO_SPACE_METHOD_FUNC_CORRECT: &str = "
 method this_is_some_method(conf_object_t *dummy_obj) {
-    if(!dummy_obj)
+    if (!dummy_obj)
         other_method_called();
 }
 ";

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

@@ -4,7 +4,7 @@ use crate::lint::rules::RuleType;
 //  NSP.inparen immediately inside parentheses or brackets
 static NO_SPACE_INPAREN_METHOD_FUNC_INDEX_INCORRECT: &str = "
 method this_is_some_method( conf_object_t *dummy_obj ) {
-    if( !dummy_obj[ 0 ] )
+    if ( !dummy_obj[ 0 ] )
         return;
 }
 ";
@@ -15,10 +15,10 @@ fn no_space_inparen_method_func_index_incorrect() {
         RuleType::NspInparen,
         (1, 1, 27, 28),
         (1, 1, 52, 53),
-        (2, 2, 7, 8),
-        (2, 2, 23, 24),
-        (2, 2, 19, 20),
-        (2, 2, 21, 22),
+        (2, 2, 8, 9),
+        (2, 2, 24, 25),
+        (2, 2, 20, 21),
+        (2, 2, 22, 23),
     );
     assert_snippet(NO_SPACE_INPAREN_METHOD_FUNC_INDEX_INCORRECT, expected_errors, &rules);
     // Test rule disable
@@ -29,7 +29,7 @@ fn no_space_inparen_method_func_index_incorrect() {
 //  NSP.inparen immediately inside parentheses or brackets
 static NO_SPACE_INPAREN_METHOD_FUNC_INDEX_CORRECT: &str = "
 method this_is_some_method(conf_object_t *dummy_obj) {
-    if(!dummy_obj[0])
+    if (!dummy_obj[0])
         return;
 }
 ";