refactor(hql): Hql todo resolution #424
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a major parser refactor for the HelixQL language compiler, reorganizing the codebase for better maintainability and resolving numerous TODO items. The changes consolidate parser functionality into specialized modules and update import paths throughout the analyzer and generator components.
- Major reorganization of the parser module structure from
helix_parserto organized sub-modules - Resolution and removal of TODO comments throughout the codebase
- Import path updates across analyzer, generator, and CLI components to use the new module structure
Reviewed Changes
Copilot reviewed 65 out of 71 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| helix-db/src/helixc/parser/mod.rs | Complete restructure with new module organization and main parsing logic |
| helix-db/src/helixc/parser/types.rs | Large new file containing all AST type definitions (958 lines) |
| helix-db/src/helixc/parser/utils.rs | New utility functions for common parsing operations |
| helix-db/src/helixc/parser/parser_methods.rs | Updated import path from old module structure |
| helix-db/src/helixc/analyzer/ | Multiple files with import path updates and TODO resolution |
| helix-db/src/helixc/generator/ | Multiple files with import path updates and code cleanup |
| helix-db/src/helix_engine/ | Various files with updated method calls and TODO removal |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| &self, | ||
| pair: Pair<Rule>, | ||
| ) -> Result<Vec<Expression>, ParserError> { | ||
| println!("pair: {pair:?}"); |
There was a problem hiding this comment.
Debug print statement should be removed from production code as it can affect performance and clutter logs.
| println!("pair: {pair:?}"); | |
Copilot uses AI. Check for mistakes.
| } | ||
| (FieldType::Date, value) => match value { | ||
| Value::String(date) => { | ||
| println!("date: {}, {:?}", date, date.parse::<NaiveDate>()); |
There was a problem hiding this comment.
Debug print statements should be removed from production code as they can affect performance and clutter logs.
Copilot uses AI. Check for mistakes.
| ) => true, | ||
| (FieldType::Boolean, DefaultValue::Boolean(_)) => true, | ||
| (FieldType::Date, DefaultValue::String(date)) => { | ||
| println!("date: {}, {:?}", date, date.parse::<NaiveDate>()); |
There was a problem hiding this comment.
Debug print statements should be removed from production code as they can affect performance and clutter logs.
| println!("date: {}, {:?}", date, date.parse::<NaiveDate>()); |
Copilot uses AI. Check for mistakes.
| _ => false, | ||
| }, | ||
| l => { | ||
| println!("l: {l:?}"); |
There was a problem hiding this comment.
Debug print statement should be removed from production code as it can affect performance and clutter logs.
Copilot uses AI. Check for mistakes.
| ) -> Result<bool, GraphError> { | ||
| let val = match &self.inner.next() { | ||
| Some(Ok(TraversalValue::Value(val))) => { | ||
| println!("value : {val:?}"); |
There was a problem hiding this comment.
Debug print statements should be removed from production code as they can affect performance and clutter logs.
Copilot uses AI. Check for mistakes.
| Some(Err(err)) => Err(GraphError::from(err.to_string())), | ||
| None => Ok(default), | ||
| }; | ||
| println!("result: {val:?}"); |
There was a problem hiding this comment.
Debug print statements should be removed from production code as they can affect performance and clutter logs.
| println!("result: {val:?}"); |
Copilot uses AI. Check for mistakes.
Description
implementing major parser refactor and code tidy up which I think was much needed. also resolved or removed many of the old todos
Related Issues
Closes #426 #425 #423
Checklist when merging to main
rustfmthelix-cli/Cargo.tomlandhelixdb/Cargo.tomlAdditional Notes