Address create_err, remove is_ok assert and use hashset instead of vector for assert

jvsqzj · jvsqzj · commit 02ade43a8a90 · 2025-05-22T11:02:50.000-06:00
diff --git a/src/lint/rules/indentation.rs b/src/lint/rules/indentation.rs
@@ -64,7 +64,9 @@ impl LongLinesRule {
         let len = line.len().try_into().unwrap();
         if len > self.max_length {
             let rowu32 = row.try_into().unwrap();
-            self.push_err(acc, Range::<ZeroIndexed>::from_u32(rowu32,rowu32, self.max_length, len));
+            acc.push(
+                self.create_err(Range::<ZeroIndexed>::from_u32(rowu32,rowu32, self.max_length, len))
+            );
         }
     }
 }
@@ -99,7 +101,9 @@ impl IndentNoTabRule {
 
         for (col, _) in line.match_indices('\t') {
             let colu32 = col.try_into().unwrap();
-            self.push_err(acc, Range::<ZeroIndexed>::from_u32(rowu32, rowu32, colu32, colu32 + 1));
+            acc.push(
+                self.create_err(Range::<ZeroIndexed>::from_u32(rowu32, rowu32, colu32, colu32 + 1))
+            );
         }
     }
 }
@@ -203,7 +207,7 @@ impl IndentCodeBlockRule {
             args.lbrace.row_start == args.members_ranges[0].row_start { return; }
         for member_range in args.members_ranges {
             if self.indentation_is_not_aligned(member_range, args.expected_depth) {
-                self.push_err(acc, member_range);
+                acc.push(self.create_err(member_range));
             }
         }
     }
@@ -347,13 +351,14 @@ impl IndentClosingBraceRule {
         let last_member_on_same_row_than_rbrace:bool = args.last_member.row_end
             == args.rbrace.row_start;
         if last_member_on_same_row_than_rbrace {
-            return self.push_err(acc, args.rbrace);
+            acc.push(self.create_err(args.rbrace));
+            return;
         }
 
         let rbrace_on_same_ind_level_than_switchtok:bool = args.rbrace.col_start.0
             == args.expected_depth * self.indentation_spaces;
         if !rbrace_on_same_ind_level_than_switchtok {
-            self.push_err(acc, args.rbrace);
+            acc.push(self.create_err(args.rbrace));
         }
     }
 }
@@ -495,7 +500,7 @@ impl IndentParenExprRule {
             if member_range.row_start.0 != last_row {
                 last_row = member_range.row_start.0;
                 if member_range.col_start.0 != expected_line_start {
-                    self.push_err(acc, member_range);
+                    acc.push(self.create_err(member_range));
                 }
             }
         }
@@ -574,7 +579,7 @@ impl IndentSwitchCaseRule {
         if !self.enabled { return; }
         let Some(args) = args else { return; };
         if self.indentation_is_not_aligned(args.case_range, args.expected_depth) {
-            self.push_err(acc, args.case_range);
+            acc.push(self.create_err(args.case_range));
         }
     }
     fn indentation_is_not_aligned(&self, member_range: ZeroRange, depth: u32) -> bool {
@@ -661,7 +666,9 @@ impl IndentEmptyLoopRule {
         let Some(args) = args else { return; };
         if self.indentation_is_not_aligned(args.semicolon_range, args.expected_depth) ||
             args.loop_keyword_range.row_start == args.semicolon_range.row_start {
-            self.push_err(acc, Range::combine(args.loop_keyword_range, args.semicolon_range))
+            acc.push(
+                self.create_err(Range::combine(args.loop_keyword_range, args.semicolon_range))
+            );
         }
     }
     fn indentation_is_not_aligned(&self, member_range: ZeroRange, depth: u32) -> bool {
diff --git a/src/lint/rules/mod.rs b/src/lint/rules/mod.rs
@@ -50,19 +50,18 @@ pub trait Rule {
     fn name() -> &'static str;
     fn description() -> &'static str;
     fn get_rule_type() -> RuleType;
-    fn push_err(&self, acc: &mut Vec<DMLStyleError>, range: ZeroRange) {
-        let dmlerror = DMLStyleError {
+    fn create_err(&self, range: ZeroRange) -> DMLStyleError {
+        DMLStyleError {
             error: LocalDMLError {
                 range,
                 description: Self::description().to_string(),
             },
             rule_type: Self::get_rule_type(),
-        };
-        acc.push(dmlerror);
+        }
     }
 }
 
-#[derive(PartialEq, Debug, Clone)]
+#[derive(PartialEq, Debug, Clone, Eq, Hash)]
 pub enum RuleType {
     SpBraces,
     SpPunct,
diff --git a/src/lint/rules/spacing.rs b/src/lint/rules/spacing.rs
@@ -103,11 +103,11 @@ impl SpBracesRule {
         if let Some(location) = ranges {
             if (location.lbrace.row_end == location.body_start.row_start)
                 && (location.lbrace.col_end == location.body_start.col_start) {
-                self.push_err(acc, location.lbrace);
+                acc.push(self.create_err(location.lbrace));
             }
             if (location.rbrace.row_start == location.body_end.row_end)
                 && (location.rbrace.col_start == location.body_end.col_end) {
-                self.push_err(acc, location.rbrace);
+                acc.push(self.create_err(location.rbrace));
             }
         }
     }
@@ -239,7 +239,7 @@ impl SpPunctRule {
                         before_range.row_end, punct_range.row_start,
                         before_range.col_end, punct_range.col_start
                     );
-                    self.push_err(acc, error_range);
+                    acc.push(self.create_err(error_range));
                 }
 
                 if after_range.is_none() {continue;}
@@ -250,7 +250,7 @@ impl SpPunctRule {
                         punct_range.row_start, after_range.unwrap().row_end,
                         punct_range.col_start, after_range.unwrap().col_end,
                     );
-                    self.push_err(acc, error_range);
+                    acc.push(self.create_err(error_range));
                 }
             }
         }
@@ -303,7 +303,7 @@ impl NspFunparRule {
                  range: Option<NspFunparArgs>) {
         if !self.enabled { return; }
         if let Some(gap) = range {
-            self.push_err(acc, gap);
+            acc.push(self.create_err(gap));
         }
     }
 }
@@ -396,14 +396,14 @@ impl NspInparenRule {
                 let mut gap = location.opening;
                 gap.col_start = location.opening.col_end;
                 gap.col_end = location.content_start.col_start;
-                self.push_err(acc, gap);
+                acc.push(self.create_err(gap));
             }
             if (location.closing.row_start == location.content_end.row_end)
                 && (location.closing.col_start != location.content_end.col_end) { 
                 let mut gap = location.closing;
                 gap.col_end = location.closing.col_start;
                 gap.col_start = location.content_end.col_end;
-                self.push_err(acc, gap);
+                acc.push(self.create_err(gap));
             }
         }
     }
@@ -454,7 +454,7 @@ impl NspUnaryRule {
                  range: Option<NspUnaryArgs>) {
         if !self.enabled { return; }
         if let Some(gap) = range {
-            self.push_err(acc, gap);
+            acc.push(self.create_err(gap));
         }
     }
 }
@@ -483,10 +483,10 @@ impl NspTrailingRule {
         let row_u32 = row.try_into().unwrap();
         let tokens_end = line.trim_end().len().try_into().unwrap();
         if tokens_end < len {
-            self.push_err(acc, Range::<ZeroIndexed>::from_u32(row_u32,
+            acc.push(self.create_err(Range::<ZeroIndexed>::from_u32(row_u32,
                                                         row_u32,
                                                         tokens_end,
-                                                        len));
+                                                        len)));
         }
     }
 }
diff --git a/src/lint/rules/tests/common.rs b/src/lint/rules/tests/common.rs
@@ -3,6 +3,7 @@ use crate::lint::rules::{instantiate_rules, CurrentRules, RuleType};
 use crate::lint::tests::create_ast_from_snippet;
 use crate::analysis::ZeroRange;
 use crate::vfs::Error;
+use std::collections::HashSet;
 
 #[derive(Debug)]
 pub struct ExpectedDMLStyleError {
@@ -44,15 +45,13 @@ pub fn assert_snippet(source_code: &str, expected_errors: usize, rules: &Current
 }
 
 pub fn robust_assert_snippet(source_code: &str, expected_errors: Vec<ExpectedDMLStyleError>, rules: &CurrentRules) {
-    let lint_errors = run_linter(source_code, rules);
-    assert!(lint_errors.is_ok());
-    let lint_errors = lint_errors.unwrap();
+    let lint_errors = run_linter(source_code, rules).unwrap();
     assert_eq!(lint_errors.len(), expected_errors.len(), "{:#?}", lint_errors);
 
-    for (actual, expected) in lint_errors.iter().zip(expected_errors.iter()) {
-        assert_eq!(actual.error.range, expected.range, "Range mismatch: {:#?} vs {:#?}", actual, expected);
-        assert_eq!(actual.rule_type, expected.rule_type, "RuleType mismatch: {:#?} vs {:#?}", actual, expected);
-    }
+    let actual_errors: HashSet<_> = lint_errors.iter().map(|e| (e.error.range.clone(), e.rule_type.clone())).collect();
+    let expected_errors: HashSet<_> = expected_errors.iter().map(|e| (e.range.clone(), e.rule_type.clone())).collect();
+
+    assert_eq!(actual_errors, expected_errors, "Mismatch between actual and expected errors:\nActual: {:#?}\nExpected: {:#?}", actual_errors, expected_errors);
 }
 
 pub fn set_up() -> CurrentRules {
diff --git a/src/lint/rules/tests/indentation/closing_brace.rs b/src/lint/rules/tests/indentation/closing_brace.rs
@@ -204,12 +204,10 @@ fn composite_indent_incorrect() {
     let rules = set_up();
     let expected_errors = define_expected_errors!(
         RuleType::IN4,
-        (12, 12, 4, 5),
-        (12, 12, 2, 3),
-        (12, 12, 0, 1),
         (11, 11, 16, 17),
-    ); // Order of expected errors is not intuitive here
-    // and has to be defined in a reverse
-    // fashion to be asserted properly
+        (12, 12, 0, 1),
+        (12, 12, 2, 3),
+        (12, 12, 4, 5),
+    );
     assert_snippet(COMPOSITE_INDENT_INCORRECT, expected_errors, &rules);
 }

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,9 @@ impl LongLinesRule {`
`64`	`64`	`let len = line.len().try_into().unwrap();`
`65`	`65`	`if len > self.max_length {`
`66`	`66`	`let rowu32 = row.try_into().unwrap();`
`67`		`- self.push_err(acc, Range::<ZeroIndexed>::from_u32(rowu32,rowu32, self.max_length, len));`
	`67`	`+ acc.push(`
	`68`	`+ self.create_err(Range::<ZeroIndexed>::from_u32(rowu32,rowu32, self.max_length, len))`
	`69`	`+ );`
`68`	`70`	`}`
`69`	`71`	`}`
`70`	`72`	`}`
`@@ -99,7 +101,9 @@ impl IndentNoTabRule {`
`99`	`101`
`100`	`102`	`for (col, _) in line.match_indices('\t') {`
`101`	`103`	`let colu32 = col.try_into().unwrap();`
`102`		`- self.push_err(acc, Range::<ZeroIndexed>::from_u32(rowu32, rowu32, colu32, colu32 + 1));`
	`104`	`+ acc.push(`
	`105`	`+ self.create_err(Range::<ZeroIndexed>::from_u32(rowu32, rowu32, colu32, colu32 + 1))`
	`106`	`+ );`
`103`	`107`	`}`
`104`	`108`	`}`
`105`	`109`	`}`
`@@ -203,7 +207,7 @@ impl IndentCodeBlockRule {`
`203`	`207`	`args.lbrace.row_start == args.members_ranges[0].row_start { return; }`
`204`	`208`	`for member_range in args.members_ranges {`
`205`	`209`	`if self.indentation_is_not_aligned(member_range, args.expected_depth) {`
`206`		`- self.push_err(acc, member_range);`
	`210`	`+ acc.push(self.create_err(member_range));`
`207`	`211`	`}`
`208`	`212`	`}`
`209`	`213`	`}`
`@@ -347,13 +351,14 @@ impl IndentClosingBraceRule {`
`347`	`351`	`let last_member_on_same_row_than_rbrace:bool = args.last_member.row_end`
`348`	`352`	`== args.rbrace.row_start;`
`349`	`353`	`if last_member_on_same_row_than_rbrace {`
`350`		`- return self.push_err(acc, args.rbrace);`
	`354`	`+ acc.push(self.create_err(args.rbrace));`
	`355`	`+ return;`
`351`	`356`	`}`
`352`	`357`
`353`	`358`	`let rbrace_on_same_ind_level_than_switchtok:bool = args.rbrace.col_start.0`
`354`	`359`	`== args.expected_depth * self.indentation_spaces;`
`355`	`360`	`if !rbrace_on_same_ind_level_than_switchtok {`
`356`		`- self.push_err(acc, args.rbrace);`
	`361`	`+ acc.push(self.create_err(args.rbrace));`
`357`	`362`	`}`
`358`	`363`	`}`
`359`	`364`	`}`
`@@ -495,7 +500,7 @@ impl IndentParenExprRule {`
`495`	`500`	`if member_range.row_start.0 != last_row {`
`496`	`501`	`last_row = member_range.row_start.0;`
`497`	`502`	`if member_range.col_start.0 != expected_line_start {`
`498`		`- self.push_err(acc, member_range);`
	`503`	`+ acc.push(self.create_err(member_range));`
`499`	`504`	`}`
`500`	`505`	`}`
`501`	`506`	`}`
`@@ -574,7 +579,7 @@ impl IndentSwitchCaseRule {`
`574`	`579`	`if !self.enabled { return; }`
`575`	`580`	`let Some(args) = args else { return; };`
`576`	`581`	`if self.indentation_is_not_aligned(args.case_range, args.expected_depth) {`
`577`		`- self.push_err(acc, args.case_range);`
	`582`	`+ acc.push(self.create_err(args.case_range));`
`578`	`583`	`}`
`579`	`584`	`}`
`580`	`585`	`fn indentation_is_not_aligned(&self, member_range: ZeroRange, depth: u32) -> bool {`
`@@ -661,7 +666,9 @@ impl IndentEmptyLoopRule {`
`661`	`666`	`let Some(args) = args else { return; };`
`662`	`667`	`if self.indentation_is_not_aligned(args.semicolon_range, args.expected_depth) \|\|`
`663`	`668`	`args.loop_keyword_range.row_start == args.semicolon_range.row_start {`
`664`		`- self.push_err(acc, Range::combine(args.loop_keyword_range, args.semicolon_range))`
	`669`	`+ acc.push(`
	`670`	`+ self.create_err(Range::combine(args.loop_keyword_range, args.semicolon_range))`
	`671`	`+ );`
`665`	`672`	`}`
`666`	`673`	`}`
`667`	`674`	`fn indentation_is_not_aligned(&self, member_range: ZeroRange, depth: u32) -> bool {`
Original file line number	Diff line number	Diff line change
`@@ -103,11 +103,11 @@ impl SpBracesRule {`
`103`	`103`	`if let Some(location) = ranges {`
`104`	`104`	`if (location.lbrace.row_end == location.body_start.row_start)`
`105`	`105`	`&& (location.lbrace.col_end == location.body_start.col_start) {`
`106`		`- self.push_err(acc, location.lbrace);`
	`106`	`+ acc.push(self.create_err(location.lbrace));`
`107`	`107`	`}`
`108`	`108`	`if (location.rbrace.row_start == location.body_end.row_end)`
`109`	`109`	`&& (location.rbrace.col_start == location.body_end.col_end) {`
`110`		`- self.push_err(acc, location.rbrace);`
	`110`	`+ acc.push(self.create_err(location.rbrace));`
`111`	`111`	`}`
`112`	`112`	`}`
`113`	`113`	`}`
`@@ -239,7 +239,7 @@ impl SpPunctRule {`
`239`	`239`	`before_range.row_end, punct_range.row_start,`
`240`	`240`	`before_range.col_end, punct_range.col_start`
`241`	`241`	`);`
`242`		`- self.push_err(acc, error_range);`
	`242`	`+ acc.push(self.create_err(error_range));`
`243`	`243`	`}`
`244`	`244`
`245`	`245`	`if after_range.is_none() {continue;}`
`@@ -250,7 +250,7 @@ impl SpPunctRule {`
`250`	`250`	`punct_range.row_start, after_range.unwrap().row_end,`
`251`	`251`	`punct_range.col_start, after_range.unwrap().col_end,`
`252`	`252`	`);`
`253`		`- self.push_err(acc, error_range);`
	`253`	`+ acc.push(self.create_err(error_range));`
`254`	`254`	`}`
`255`	`255`	`}`
`256`	`256`	`}`
`@@ -303,7 +303,7 @@ impl NspFunparRule {`
`303`	`303`	`range: Option<NspFunparArgs>) {`
`304`	`304`	`if !self.enabled { return; }`
`305`	`305`	`if let Some(gap) = range {`
`306`		`- self.push_err(acc, gap);`
	`306`	`+ acc.push(self.create_err(gap));`
`307`	`307`	`}`
`308`	`308`	`}`
`309`	`309`	`}`
`@@ -396,14 +396,14 @@ impl NspInparenRule {`
`396`	`396`	`let mut gap = location.opening;`
`397`	`397`	`gap.col_start = location.opening.col_end;`
`398`	`398`	`gap.col_end = location.content_start.col_start;`
`399`		`- self.push_err(acc, gap);`
	`399`	`+ acc.push(self.create_err(gap));`
`400`	`400`	`}`
`401`	`401`	`if (location.closing.row_start == location.content_end.row_end)`
`402`	`402`	`&& (location.closing.col_start != location.content_end.col_end) {`
`403`	`403`	`let mut gap = location.closing;`
`404`	`404`	`gap.col_end = location.closing.col_start;`
`405`	`405`	`gap.col_start = location.content_end.col_end;`
`406`		`- self.push_err(acc, gap);`
	`406`	`+ acc.push(self.create_err(gap));`
`407`	`407`	`}`
`408`	`408`	`}`
`409`	`409`	`}`
`@@ -454,7 +454,7 @@ impl NspUnaryRule {`
`454`	`454`	`range: Option<NspUnaryArgs>) {`
`455`	`455`	`if !self.enabled { return; }`
`456`	`456`	`if let Some(gap) = range {`
`457`		`- self.push_err(acc, gap);`
	`457`	`+ acc.push(self.create_err(gap));`
`458`	`458`	`}`
`459`	`459`	`}`
`460`	`460`	`}`
`@@ -483,10 +483,10 @@ impl NspTrailingRule {`
`483`	`483`	`let row_u32 = row.try_into().unwrap();`
`484`	`484`	`let tokens_end = line.trim_end().len().try_into().unwrap();`
`485`	`485`	`if tokens_end < len {`
`486`		`- self.push_err(acc, Range::<ZeroIndexed>::from_u32(row_u32,`
	`486`	`+ acc.push(self.create_err(Range::<ZeroIndexed>::from_u32(row_u32,`
`487`	`487`	`row_u32,`
`488`	`488`	`tokens_end,`
`489`		`- len));`
	`489`	`+ len)));`
`490`	`490`	`}`
`491`	`491`	`}`
`492`	`492`	`}`