Merge pull request #109 from kevv87/feature/73-line-breaking

Implement line length and breaking rules

Author: JonatanWaern

CHANGELOG.md

@@ -5,7 +5,8 @@
 # Change Log
 
 # 0.9.15
-
+- Added support for line length and breaking rules regarding line-breaks after opening parentheses, method output arguments, conditional expressions and binary operands.
+- Added support for indendation rule indent_continuation_line.
 
 ## 0.9.14
 - Slight optimization to the memory usage of device-level analysis which

example_files/example_lint_cfg.json

@@ -18,5 +18,10 @@
     "indent_paren_expr": {},
     "indent_switch_case": {},
     "indent_empty_loop": {},
+    "indent_continuation_line": {},
+    "break_func_call_open_paren": {},
+    "break_method_output": {},
+    "break_conditional_expression": {},
+    "break_before_binary_op": {},
     "annotate_lints": true
 }

src/analysis/parsing/expression.rs

@@ -26,7 +26,10 @@ use crate::lint::{DMLStyleError,
                                     SpPunctArgs},
                                     CurrentRules},
                                     AuxParams};
-use crate::lint::rules::indentation::{IndentParenExprArgs};
+use crate::lint::rules::indentation::IndentParenExprArgs;
+use crate::lint::rules::linelength::{BreakBeforeBinaryOpArgs,
+    BreakConditionalExpression,
+    BreakFuncCallOpenParenArgs};
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct UnaryExpressionContent {
@@ -96,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);
     }
 }
 
@@ -158,6 +162,8 @@ impl TreeElement for TertiaryExpressionContent {
     }
     fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
         rules.sp_ternary.check(SpTernaryArgs::from_tertiary_expression_content(self), acc);
+        rules.break_conditional_expression
+            .check(BreakConditionalExpression::from_tertiary_expression(self), acc);
     }
 }
 
@@ -175,8 +181,10 @@ impl TreeElement for ParenExpressionContent {
     fn subs(&self) -> TreeElements<'_> {
         create_subs!(&self.lparen, &self.expr, &self.rparen)
     }
-    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
         rules.indent_paren_expr.check(IndentParenExprArgs::from_paren_expression(self), acc);
+        rules.break_func_call_open_paren.check(
+            BreakFuncCallOpenParenArgs::from_paren_expression(self, aux.depth), acc);
     }
 }
 
@@ -222,11 +230,13 @@ impl TreeElement for FunctionCallContent {
                 noderef, ReferenceKind::Callable));
         }
     }
-    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
         rules.nsp_funpar.check(NspFunparArgs::from_function_call(self), acc);
         rules.nsp_inparen.check(NspInparenArgs::from_function_call(self), acc);
         rules.sp_punct.check(SpPunctArgs::from_function_call(self), acc);
         rules.indent_paren_expr.check(IndentParenExprArgs::from_function_call(self), acc);
+        rules.break_func_call_open_paren
+            .check(BreakFuncCallOpenParenArgs::from_function_call(self, aux.depth), acc);
     }
 }
 
@@ -340,8 +350,10 @@ impl TreeElement for CastContent {
         create_subs!(&self.cast, &self.lparen, &self.from,
                      &self.comma, &self.to, &self.rparen)
     }
-    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
         rules.indent_paren_expr.check(IndentParenExprArgs::from_cast(self), acc);
+        rules.break_func_call_open_paren
+            .check(BreakFuncCallOpenParenArgs::from_cast(self, aux.depth), acc);
     }
 }
 

src/analysis/parsing/statement.rs

@@ -2,7 +2,8 @@
 //  SPDX-License-Identifier: Apache-2.0 and MIT
 use log::error;
 
-use crate::lint::rules::indentation::IndentEmptyLoopArgs;
+use crate::lint::rules::indentation::{IndentEmptyLoopArgs,
+    IndentContinuationLineArgs};
 use crate::lint::rules::spacing::SpReservedArgs;
 use crate::span::Range;
 use crate::analysis::parsing::lexer::TokenKind;
@@ -26,7 +27,10 @@ use crate::analysis::parsing::structure::{parse_vardecl, VarDecl};
 use crate::analysis::LocalDMLError;
 use crate::lint::{DMLStyleError,
                   rules::{CurrentRules,
-                          indentation::{IndentCodeBlockArgs, IndentClosingBraceArgs, IndentParenExprArgs, IndentSwitchCaseArgs},
+                          indentation::{IndentCodeBlockArgs,
+                            IndentClosingBraceArgs,
+                            IndentParenExprArgs,
+                            IndentSwitchCaseArgs},
                           spacing::{NspInparenArgs,
                                     SpBracesArgs,
                                     SpPunctArgs}},
@@ -1847,6 +1851,9 @@ impl TreeElement for StatementContent {
             Self::Return(content) => create_subs![content],
         }
     }
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
+        rules.indent_continuation_line.check(acc, IndentContinuationLineArgs::from_statement_content(self, aux.depth));
+    }
 }
 
 pub type Statement = AstObject<StatementContent>;

src/analysis/parsing/structure.rs

@@ -16,8 +16,15 @@ use crate::analysis::parsing::parser::{doesnt_understand_tokens,
                                        FileParser, Parse, ParseContext,
                                        FileInfo};
 use crate::analysis::LocalDMLError;
-use crate::lint::rules::spacing::{NspFunparArgs, NspInparenArgs, SpBracesArgs, SpPunctArgs};
-use crate::lint::rules::indentation::{IndentCodeBlockArgs, IndentClosingBraceArgs, IndentParenExprArgs};
+use crate::lint::rules::linelength::{BreakFuncCallOpenParenArgs, BreakMethodOutputArgs};
+use crate::lint::rules::spacing::{SpBracesArgs,
+                                  NspInparenArgs,
+                                  NspFunparArgs,
+                                  SpPunctArgs};
+use crate::lint::rules::indentation::{IndentCodeBlockArgs,
+    IndentClosingBraceArgs,
+    IndentParenExprArgs,
+    IndentContinuationLineArgs};
 use crate::lint::{rules::CurrentRules, AuxParams, DMLStyleError};
 use crate::analysis::reference::{Reference, ReferenceKind};
 use crate::analysis::FileSpec;
@@ -232,11 +239,13 @@ impl TreeElement for MethodContent {
         }
         errors
     }
-    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, _aux: AuxParams) {
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
         rules.nsp_funpar.check(NspFunparArgs::from_method(self), acc);
         rules.nsp_inparen.check(NspInparenArgs::from_method(self), acc);
         rules.sp_punct.check(SpPunctArgs::from_method(self), acc);
         rules.indent_paren_expr.check(IndentParenExprArgs::from_method(self), acc);
+        rules.break_func_call_open_paren.check(BreakFuncCallOpenParenArgs::from_method(self, aux.depth), acc);
+        rules.break_method_output.check(BreakMethodOutputArgs::from_method(self), acc);
     }
 }
 
@@ -1928,6 +1937,9 @@ impl TreeElement for DMLObjectContent {
             Self::Template(content) => create_subs![content],
         }
     }
+    fn evaluate_rules(&self, acc: &mut Vec<DMLStyleError>, rules: &CurrentRules, aux: AuxParams) {
+        rules.indent_continuation_line.check(acc, IndentContinuationLineArgs::from_dml_object_content(self, aux.depth));
+    }
 }
 
 pub type DMLObject = AstObject<DMLObjectContent>;

src/lint/features.md

@@ -51,5 +51,24 @@ 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**, `break_conditional_expression`: Break conditional expressions before the ?, or both before the ? and before the :.
+- **LL5**, `break_method_output`: Break long method declarations with output parameters before the arrow.
+```
+    method inquiry_status(uint64 physical_address)
+        -> (uint16 status) {
+        ...
+    }
+```
+- **LL6**, `break_func_call_open_paren`: Function and method invocations can be broken after the opening parenthesis, with the continuation lines indented one level.
+```
+    signal_interface *my_interface = SIM_get_interface(
+        other_object, SIGNAL_INTERFACE);
+```
 
 ##### Check [Issue #76 For remaining and planned checks](https://github.com/intel/dml-language-server/issues/76)

src/lint/mod.rs

@@ -7,24 +7,33 @@ use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use lazy_static::lazy_static;
 use log::{debug, error, trace};
+use rules::linelength::{BreakBeforeBinaryOpOptions,
+    BreakFuncCallOpenParenOptions,
+    BreakMethodOutputOptions,
+    BreakConditionalExpressionOptions};
 use serde::{Deserialize, Serialize};
 use regex::Regex;
 use rules::{instantiate_rules, CurrentRules, RuleType};
 use rules::{spacing::{SpReservedOptions,
-                      SpBraceOptions,
-                      SpPunctOptions,
-                      SpBinopOptions,
-                      NspFunparOptions,
-                      SpTernaryOptions,
-                      SpPtrDeclOptions,
-                      NspPtrDeclOptions,
-                      NspInparenOptions,
-                      NspUnaryOptions,
-                      NspTrailingOptions},
-            indentation::{LongLineOptions, IndentSizeOptions, IndentCodeBlockOptions,
-                          IndentNoTabOptions, IndentClosingBraceOptions, IndentParenExprOptions,
-                          IndentSwitchCaseOptions, IndentEmptyLoopOptions},
-                    };
+                SpBraceOptions,
+                SpPunctOptions,
+                SpBinopOptions,
+                NspFunparOptions,
+                SpTernaryOptions,
+                SpPtrDeclOptions,
+                NspPtrDeclOptions,
+                NspInparenOptions,
+                NspUnaryOptions,
+                NspTrailingOptions},
+            indentation::{LongLineOptions,
+                IndentSizeOptions,
+                IndentCodeBlockOptions,
+                IndentNoTabOptions,
+                IndentClosingBraceOptions,
+                IndentParenExprOptions,
+                IndentSwitchCaseOptions,
+                IndentEmptyLoopOptions,
+                IndentContinuationLineOptions}};
 use crate::analysis::{DMLError, IsolatedAnalysis, LocalDMLError, ZeroRange};
 use crate::analysis::parsing::tree::TreeElement;
 use crate::file_management::CanonPath;
@@ -109,6 +118,16 @@ pub struct LintCfg {
     pub indent_switch_case: Option<IndentSwitchCaseOptions>,
     #[serde(default)]
     pub indent_empty_loop: Option<IndentEmptyLoopOptions>,
+    #[serde(default)]
+    pub indent_continuation_line: Option<IndentContinuationLineOptions>,
+    #[serde(default)]
+    pub break_func_call_open_paren: Option<BreakFuncCallOpenParenOptions>,
+    #[serde(default)]
+    pub break_conditional_expression: Option<BreakConditionalExpressionOptions>,
+    #[serde(default)]
+    pub break_method_output: Option<BreakMethodOutputOptions>,
+    #[serde(default)]
+    pub break_before_binary_op: Option<BreakBeforeBinaryOpOptions>,
     #[serde(default = "get_true")]
     pub annotate_lints: bool,
 }
@@ -154,6 +173,11 @@ impl Default for LintCfg {
             indent_paren_expr: Some(IndentParenExprOptions{}),
             indent_switch_case: Some(IndentSwitchCaseOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
             indent_empty_loop: Some(IndentEmptyLoopOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
+            indent_continuation_line: Some(IndentContinuationLineOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
+            break_func_call_open_paren: Some(BreakFuncCallOpenParenOptions{indentation_spaces: INDENTATION_LEVEL_DEFAULT}),
+            break_method_output: Some(BreakMethodOutputOptions{}),
+            break_conditional_expression: Some(BreakConditionalExpressionOptions{}),
+            break_before_binary_op: Some(BreakBeforeBinaryOpOptions{}),
             annotate_lints: true,
         }
     }
@@ -218,7 +242,7 @@ impl LinterAnalysis {
     }
 }
 
-pub fn begin_style_check(ast: TopAst, file: &str, rules: &CurrentRules) -> Result<Vec<DMLStyleError>, Error> {
+fn begin_style_check(ast: TopAst, file: &str, rules: &CurrentRules) -> Result<Vec<DMLStyleError>, Error> {
     let (mut invalid_lint_annot, lint_annot) = obtain_lint_annotations(file);
     let mut linting_errors: Vec<DMLStyleError> = vec![];
     ast.style_check(&mut linting_errors, rules, AuxParams { depth: 0 });
@@ -290,8 +314,7 @@ fn obtain_lint_annotations(file: &str) -> (Vec<DMLStyleError>,
         last_line = row;
         if let Some(capture) = LINT_ANNOTATION.captures(line) {
             let has_pre = capture.get(1)
-                .map_or(false,
-                        |m|!m.is_empty() &&
+                .is_some_and(|m|!m.is_empty() &&
                         JUST_WHITESPACE.captures(m.as_str()).is_none());
             let op_capture = capture.get(2).unwrap();
             let operation = match op_capture.as_str() {
@@ -397,7 +420,7 @@ fn remove_disabled_lints(errors: &mut Vec<DMLStyleError>,
             !annotations.whole_file.contains(
                 &LintAnnotation::Allow(error.rule_type)) &&
                 !annotations.line_specific.get(&error.error.range.row_start.0)
-                .map_or(false, |annots|annots.contains(
+                .is_some_and(|annots|annots.contains(
                     &LintAnnotation::Allow(error.rule_type)))
         }
     );
@@ -564,7 +587,7 @@ pub mod tests {
         crate::logging::init();
 
         let source =
-            "
+"
 dml 1.4;
 
 // dml-lint: allow-file=nsp_unary