refactor: Improve encapsulation of CaseBlock struct

ngeor · ngeor · commit d4d0f533106f · 2025-12-26T20:13:59.000+01:00
diff --git a/rusty_basic/src/instruction_generator/select_case.rs b/rusty_basic/src/instruction_generator/select_case.rs
@@ -32,9 +32,12 @@ impl InstructionGenerator {
             // where to jump out from here if the case block isn't matching
             let next_case_label =
                 labels::next_case_label(case_blocks_len, has_else, case_block_index);
-            let is_multi_expr = case_block.expression_list.len() > 1;
+
+            let (expression_list, statements) = case_block.into();
+
+            let is_multi_expr = expression_list.len() > 1;
             self.generate_case_expressions(
-                case_block.expression_list,
+                expression_list,
                 next_case_label.as_str(),
                 pos,
                 case_block_index,
@@ -45,7 +48,7 @@ impl InstructionGenerator {
                 self.label(&labels::case_statements(case_block_index), pos);
             }
             // run matched CASE block statements
-            self.visit(case_block.statements);
+            self.visit(statements);
             // jump out of SELECT
             self.jump(labels::end_select(), pos);
         }
diff --git a/rusty_linter/src/converter/statement/select_case.rs b/rusty_linter/src/converter/statement/select_case.rs
@@ -21,12 +21,10 @@ impl Convertible for SelectCase {
 
 impl Convertible for CaseBlock {
     fn convert(self, ctx: &mut Context) -> Result<Self, LintErrorPos> {
-        let expression_list = self.expression_list.convert(ctx)?;
-        let statements = self.statements.convert(ctx)?;
-        Ok(Self {
-            expression_list,
-            statements,
-        })
+        let (expression_list, statements) = self.into();
+        let expression_list = expression_list.convert(ctx)?;
+        let statements = statements.convert(ctx)?;
+        Ok(Self::new(expression_list, statements))
     }
 }
 
diff --git a/rusty_linter/src/core/visitor.rs b/rusty_linter/src/core/visitor.rs
@@ -186,7 +186,8 @@ where
     P: Visitor<Statement> + SetPosition,
 {
     fn visit(&mut self, element: &CaseBlock) -> VisitResult {
-        self.visit(&element.statements)
+        let (_, statements) = element.into();
+        self.visit(statements)
     }
 }
 
diff --git a/rusty_linter/src/post_linter/expression_reducer.rs b/rusty_linter/src/post_linter/expression_reducer.rs
@@ -203,18 +203,15 @@ pub trait ExpressionReducer {
     }
 
     fn visit_case_block(&mut self, s: CaseBlock) -> Result<CaseBlock, LintErrorPos> {
-        let CaseBlock {
-            expression_list,
-            statements,
-        } = s;
+        let (expression_list, statements) = s.into();
         let expression_list: Vec<CaseExpression> = expression_list
             .into_iter()
             .map(|case_expr| self.visit_case_expression(case_expr))
             .collect::<Result<Vec<CaseExpression>, LintErrorPos>>()?;
-        Ok(CaseBlock {
+        Ok(CaseBlock::new(
             expression_list,
-            statements: self.visit_statements(statements)?,
-        })
+            self.visit_statements(statements)?,
+        ))
     }
 
     fn visit_case_expression(&mut self, s: CaseExpression) -> Result<CaseExpression, LintErrorPos> {
diff --git a/rusty_linter/src/post_linter/post_conversion_linter.rs b/rusty_linter/src/post_linter/post_conversion_linter.rs
@@ -197,10 +197,11 @@ pub trait PostConversionLinter {
         case_block: &CaseBlock,
         select_expr: &ExpressionPos,
     ) -> Result<(), LintErrorPos> {
-        for case_expr in &case_block.expression_list {
+        let (expression_list, statements) = case_block.into();
+        for case_expr in expression_list {
             self.visit_case_expression(case_expr, select_expr)?;
         }
-        self.visit_statements(&case_block.statements)
+        self.visit_statements(statements)
     }
 
     fn visit_case_expression(
diff --git a/rusty_parser/src/specific/core/select_case.rs b/rusty_parser/src/specific/core/select_case.rs
@@ -29,12 +29,15 @@ pub fn select_case_p() -> impl Parser<RcStringView, Output = Statement> {
             let case_blocks = all_case_blocks
                 .clone()
                 .into_iter()
-                .filter(|x| !x.expression_list.is_empty())
+                .filter(|x| x.has_conditions())
                 .collect();
             let else_block = all_case_blocks
                 .into_iter()
-                .find(|x| x.expression_list.is_empty())
-                .map(|x| x.statements);
+                .find(|x| !x.has_conditions())
+                .map(|x| {
+                    let (_, right) = x.into();
+                    right
+                });
             Statement::SelectCase(SelectCase {
                 expr,
                 case_blocks,
@@ -91,10 +94,7 @@ fn continue_after_case() -> impl Parser<RcStringView, Output = CaseBlock> {
                 Box::new(case_expression_list()),
             ]),
             ZeroOrMoreStatements::new_multi(vec![Keyword::Case, Keyword::End]),
-            |expression_list, statements| CaseBlock {
-                expression_list,
-                statements,
-            },
+            CaseBlock::new,
         ),
     )
 }
@@ -175,15 +175,15 @@ mod tests {
                 GlobalStatement::Statement(Statement::SelectCase(SelectCase {
                     expr: "X".as_var_expr(2, 21),
                     inline_comments: vec![" testing for x".to_string().at_rc(2, 23)],
-                    case_blocks: vec![CaseBlock {
-                        expression_list: vec![CaseExpression::Simple(1.as_lit_expr(3, 14))],
-                        statements: vec![
+                    case_blocks: vec![CaseBlock::new(
+                        vec![CaseExpression::Simple(1.as_lit_expr(3, 14))],
+                        vec![
                             Statement::Comment(" is it one?".to_string()).at_rc(3, 23),
                             Statement::sub_call("Flint".into(), vec!["One".as_lit_expr(4, 15)])
                                 .at_rc(4, 9),
                             Statement::Comment(" print it".to_string()).at_rc(4, 23),
                         ]
-                    }],
+                    )],
                     else_block: Some(vec![
                         Statement::Comment(" something else?".to_string()).at_rc(5, 23),
                         Statement::sub_call("Flint".into(), vec!["Nope".as_lit_expr(6, 15)])
@@ -238,15 +238,15 @@ mod tests {
                         " testing for x".to_string().at_rc(2, 23),
                         " first case".to_string().at_rc(3, 9)
                     ],
-                    case_blocks: vec![CaseBlock {
-                        expression_list: vec![CaseExpression::Simple(1.as_lit_expr(4, 14))],
-                        statements: vec![
+                    case_blocks: vec![CaseBlock::new(
+                        vec![CaseExpression::Simple(1.as_lit_expr(4, 14))],
+                        vec![
                             Statement::Comment(" is it one?".to_string()).at_rc(4, 23),
                             Statement::sub_call("Flint".into(), vec!["One".as_lit_expr(5, 15)])
                                 .at_rc(5, 9),
                             Statement::Comment(" print it".to_string()).at_rc(5, 23),
                         ]
-                    }],
+                    )],
                     else_block: None
                 }))
                 .at_rc(2, 9)
@@ -271,31 +271,31 @@ mod tests {
                 expr: paren_exp!( bin_exp!( int_lit!(5 at 2:21) ; plus int_lit!(2 at 2:23) ; at 2:22 ) ; at 2:20 ),
                 inline_comments: vec![],
                 case_blocks: vec![
-                    CaseBlock {
-                        expression_list: vec![CaseExpression::Simple(
+                    CaseBlock::new(
+                        vec![CaseExpression::Simple(
                             paren_exp!( bin_exp!( int_lit!(6 at 3:14) ; plus int_lit!(5 at 3:16) ; at 3:15 ) ; at 3:13 )
                         )],
-                        statements: vec![Statement::Print(Print {
+                        vec![Statement::Print(Print {
                             file_number: None,
                             lpt1: false,
                             format_string: None,
                             args: vec![PrintArg::Expression(11.as_lit_expr(4, 19))]
                         })
                         .at_rc(4, 13)]
-                    },
-                    CaseBlock {
-                        expression_list: vec![CaseExpression::Range(
+                    ),
+                    CaseBlock::new(
+                        vec![CaseExpression::Range(
                             Expression::Parenthesis(Box::new(2.as_lit_expr(5, 14))).at_rc(5, 13),
                             Expression::Parenthesis(Box::new(5.as_lit_expr(5, 19))).at_rc(5, 18)
                         )],
-                        statements: vec![Statement::Print(Print {
+                        vec![Statement::Print(Print {
                             file_number: None,
                             lpt1: false,
                             format_string: None,
                             args: vec![PrintArg::Expression(2.as_lit_expr(6, 19))]
                         })
                         .at_rc(6, 13)]
-                    },
+                    ),
                 ],
                 else_block: None
             })
diff --git a/rusty_parser/src/specific/core/statement.rs b/rusty_parser/src/specific/core/statement.rs
@@ -265,10 +265,21 @@ pub struct SelectCase {
     pub inline_comments: Vec<Positioned<String>>,
 }
 
-#[derive(Clone, Debug, PartialEq)]
-pub struct CaseBlock {
-    pub expression_list: Vec<CaseExpression>,
-    pub statements: Statements,
+bi_tuple!(
+    /// A case block can have one or more condition expressions and
+    /// the statements to execute if the condition is met.
+    CaseBlock(Vec<CaseExpression>, Statements)
+);
+
+impl CaseBlock {
+    pub fn conditions(&self) -> &Vec<CaseExpression> {
+        &self.0
+    }
+
+    pub fn has_conditions(&self) -> bool {
+        // the CASE ELSE block does not have conditions
+        !self.conditions().is_empty()
+    }
 }
 
 #[derive(Clone, Debug, PartialEq)]

Original file line number	Diff line number	Diff line change
`@@ -186,7 +186,8 @@ where`
`186`	`186`	`P: Visitor<Statement> + SetPosition,`
`187`	`187`	`{`
`188`	`188`	`fn visit(&mut self, element: &CaseBlock) -> VisitResult {`
`189`		`- self.visit(&element.statements)`
	`189`	`+ let (_, statements) = element.into();`
	`190`	`+ self.visit(statements)`
`190`	`191`	`}`
`191`	`192`	`}`
`192`	`193`
Original file line number	Diff line number	Diff line change
`@@ -197,10 +197,11 @@ pub trait PostConversionLinter {`
`197`	`197`	`case_block: &CaseBlock,`
`198`	`198`	`select_expr: &ExpressionPos,`
`199`	`199`	`) -> Result<(), LintErrorPos> {`
`200`		`- for case_expr in &case_block.expression_list {`
	`200`	`+ let (expression_list, statements) = case_block.into();`
	`201`	`+ for case_expr in expression_list {`
`201`	`202`	`self.visit_case_expression(case_expr, select_expr)?;`
`202`	`203`	`}`
`203`		`- self.visit_statements(&case_block.statements)`
	`204`	`+ self.visit_statements(statements)`
`204`	`205`	`}`
`205`	`206`
`206`	`207`	`fn visit_case_expression(`