[TypeChecker] Implement type checking for server statements and pattern definitions - #383
Conversation
…\n\n- Add `check_pattern_expression_types` to type-check `PatternExpression` AST nodes, specifically resolving and verifying `ListReference` elements against the environment.\n- Implement type checking for `WaitForRequestStatement`, `StopAcceptingConnectionsStatement`, and `CloseServerStatement`, enforcing that `server` expressions are `Text` and `timeout` expressions are `Number`.\n- Add unit tests for both web server statements and pattern definitions to prevent regressions and verify correct behavior. Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds comprehensive type checking for pattern expressions and server expressions in the type checker. Pattern definitions now recursively validate expression trees including literals, character classes, anchors, backreferences, and list references. Server expressions are validated to resolve to text types, with new checks integrated into web server statement handling (WaitForRequestStatement, StopAcceptingConnectionsStatement, CloseServerStatement). Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 587f7676e4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| PatternExpression::ListReference(name) => { | ||
| // Ensure the referenced list actually exists and is a List | ||
| if let Some(symbol) = self.analyzer.get_symbol(name) { |
There was a problem hiding this comment.
check_pattern_expression_types now treats any ListReference missing from analyzer.get_symbol as an error, but the analyzer has already popped function/local scopes by this point, so action parameters and local list variables are no longer resolvable from current_scope. This means valid code such as a pattern defined inside an action that references a local list will now fail type checking with "Undefined list reference", even though runtime pattern compilation resolves list references from the current execution environment.
Useful? React with 👍 / 👎.
…cker/mod.rs` passes formatting checks. Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Implements previously-TODO type checking in the Rust TypeChecker for (1) web server-related statements and (2) pattern definitions, and adds unit tests to lock the behavior in.
Changes:
- Added recursive validation for
PatternExpressiontrees duringStatement::PatternDefinitiontype checking. - Implemented concrete type checks for
WaitForRequestStatement,StopAcceptingConnectionsStatement, andCloseServerStatement(server must beText, timeout must beNumber). - Added unit tests covering valid/invalid server statement usage and invalid pattern list references.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match var_type { | ||
| Type::List(_) | Type::Unknown => { | ||
| // Valid | ||
| } | ||
| _ => { | ||
| self.type_error( | ||
| format!("Pattern list reference '{name}' must be a List, got {var_type}"), | ||
| Some(Type::List(Box::new(Type::Text))), // Could be any list, but usually strings in patterns | ||
| Some(var_type.clone()), | ||
| line, | ||
| column, | ||
| ); | ||
| } |
There was a problem hiding this comment.
PatternExpression::ListReference currently accepts any Type::List(_) without validating the element type. The pattern compiler requires referenced lists to contain only text values (non-text elements are a compile/runtime error), so List<Number> will incorrectly pass type checking. Consider checking the inner list type when known (e.g., require List<Text> / compatible with Text, allow Unknown element types), and update/add a unit test that covers List<Number> being referenced from a pattern.
| let server_type = self.infer_expression_type(server); | ||
| if server_type != Type::Text | ||
| && server_type != Type::Unknown | ||
| && server_type != Type::Error | ||
| { | ||
| self.type_error( | ||
| "Server must be a text string".to_string(), | ||
| Some(Type::Text), | ||
| Some(server_type), | ||
| *line, | ||
| *column, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The server-type validation logic (infer_expression_type + the same Text/Unknown/Error check and error message) is duplicated across WaitForRequestStatement, StopAcceptingConnectionsStatement, and CloseServerStatement. Consider extracting a small helper (e.g., check_server_name_expr(server, line, column)) to avoid divergence if the rules/message change later.
|
@google-labs-jules[bot] extra-depth triage review for this Jules PR. Blocking checklist before merge:
Non-blocking nit:
CI is green, but the two items above are substantive correctness concerns and should be fixed before merge. |
Thank you for the review. I have addressed the blockers and the non-blocking nit.
|
…oping for pattern list references to support action scope.\n- Enforce pattern list element-type constraints (`List<Text>`, `Unknown`, or `Any`).\n- Refactor `WaitForRequestStatement`, `StopAcceptingConnectionsStatement`, and `CloseServerStatement` to use `check_server_expression_type` helper. Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/typechecker/mod.rs (1)
3862-3907: Tests provide good basic coverage, consider adding edge cases.The tests cover the primary success and failure paths. To strengthen confidence in the element-type constraints for pattern list references, consider adding:
- A test with a valid
List<Text>reference (should pass)- A test with
List<Number>reference (should fail with element type mismatch error)These would exercise the element-type validation logic at lines 270-283.
📝 Suggested additional test cases
// Test valid list reference in pattern (List<Text>) let valid_list_ref_program = Program { statements: vec![ Statement::CreateListStatement { name: "valid_list".to_string(), initial_values: vec![Expression::Literal(Literal::String(Arc::from("item")), 1, 1)], line: 1, column: 1, }, Statement::PatternDefinition { name: "my_pattern".to_string(), pattern: PatternExpression::ListReference("valid_list".to_string()), line: 2, column: 1, }, ], }; let mut type_checker = TypeChecker::new(); let result = type_checker.check_types(&valid_list_ref_program); assert!( result.is_ok(), "Expected valid List<Text> reference to pass type checking" ); // Test invalid element type (List<Number> should fail) let number_list_program = Program { statements: vec![ Statement::CreateListStatement { name: "number_list".to_string(), initial_values: vec![Expression::Literal(Literal::Integer(42), 1, 1)], line: 1, column: 1, }, Statement::PatternDefinition { name: "my_pattern".to_string(), pattern: PatternExpression::ListReference("number_list".to_string()), line: 2, column: 1, }, ], }; let mut type_checker = TypeChecker::new(); let result = type_checker.check_types(&number_list_program); assert!( result.is_err(), "Expected List<Number> reference to fail type checking" ); let errors = result.err().unwrap(); assert!(errors.iter().any(|e| e.message.contains("must contain Text")));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/typechecker/mod.rs` around lines 3862 - 3907, Add two new unit cases exercising PatternExpression::ListReference through TypeChecker::check_types: one that creates a CreateListStatement named e.g. "valid_list" with initial_values containing a Text/String literal and then a PatternDefinition referencing "valid_list" (assert result.is_ok()), and another that creates a CreateListStatement named e.g. "number_list" with an Integer literal and a PatternDefinition referencing "number_list" (assert result.is_err() and that the returned errors contain a message about element type / "must contain Text"). Use the same test function style and the Program/Statement/PatternExpression variants already used in the file so the tests exercise the element-type validation paths around the List element checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/typechecker/mod.rs`:
- Around line 3862-3907: Add two new unit cases exercising
PatternExpression::ListReference through TypeChecker::check_types: one that
creates a CreateListStatement named e.g. "valid_list" with initial_values
containing a Text/String literal and then a PatternDefinition referencing
"valid_list" (assert result.is_ok()), and another that creates a
CreateListStatement named e.g. "number_list" with an Integer literal and a
PatternDefinition referencing "number_list" (assert result.is_err() and that the
returned errors contain a message about element type / "must contain Text"). Use
the same test function style and the Program/Statement/PatternExpression
variants already used in the file so the tests exercise the element-type
validation paths around the List element checks.
|
@google-labs-jules[bot] follow-up triage after latest updates. Thanks for the fixes — CI is green and most checklist items are improved. I still see one substantive blocker before merge:
Non-blocking:
I’ll re-check next run and merge once the blocker above is resolved and no new substantive notes remain. |
Thank you for pointing this out. I have updated the implementation to correctly handle lexical scoping for pattern list references.
|
…\n- Move pattern list-reference existence check from `TypeChecker` to `Analyzer` so local scope variables inside actions are resolved before `Analyzer` scopes are popped.\n- Ensure `TypeChecker` no longer flags valid local list refs as undefined due to popped `Analyzer` scopes, relying on `Analyzer` for semantic validation.\n- Add explicit tests ensuring `List<Text>` pattern references pass while `List<Number>` references fail. Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/analyzer/mod.rs (1)
1977-1977: Consider closing the action-call parameter type-validation TODO.Leaving this deferred in analyzer can weaken standalone semantic diagnostics before type-checking runs.
Based on learnings
Analyzer must perform semantic validation and static analysis in src/analyzer/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/analyzer/mod.rs` at line 1977, The TODO about "Add parameter type validation" should be implemented: add a new helper (e.g., validate_action_call_parameters or validate_action_call in the Analyzer impl) that, given an action call node (call expression) and its resolved action signature, iterates parameters, checks each argument's static/semantic type against the expected parameter type, and emits diagnostics via the Analyzer's diagnostics collector (e.g., self.diagnostics.emit or self.report_error) for mismatches or wrong arity; call this helper from the existing analyze_action_call / analyze_node path where calls are resolved so validation runs during semantic analysis, and remove the TODO comment. Ensure you reference the resolved signature lookup code and use existing symbol names for diagnostic emission so behavior integrates with current Analyzer flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/typechecker/mod.rs`:
- Around line 259-266: The current branch that sets symbol_type uses
self.analyzer.get_symbol(name) and falls back to Type::Unknown when not found,
letting scoped/local bindings evade list element checks; change this to attempt
resolving scoped/action-scoped bindings before defaulting to Unknown: call an
analyzer resolver (e.g., add/use a method like Analyzer::resolve_scoped_symbol
or Analyzer::resolve_reference) from the same code path where get_symbol(name)
and get_action_parameters() are checked, and if that resolver returns a symbol
use its symbol_type, otherwise proceed with a clear unresolved handling (not
silent Unknown) so pattern list element checks still run; update the same logic
blocks referenced around get_symbol, get_action_parameters, and Type::Unknown
(also apply to the analogous code in the 267-305 region).
- Around line 316-317: Add semantic analysis for
StopAcceptingConnectionsStatement and CloseServerStatement by invoking
analyze_expression(server) for their server expressions (same as
WaitForRequestStatement does) before calling check_server_expression_type;
ensure check_server_expression_type treats Type::Unknown from
infer_expression_type as an error by calling self.type_error with a clear
message (use the existing type_error path used when server_type is not
Type::Text/Unknown/Error), referencing the functions
check_server_expression_type, analyze_expression, infer_expression_type and the
statement types StopAcceptingConnectionsStatement and CloseServerStatement so
undefined server identifiers are caught during type checking.
In `@test_typechecker.rs`:
- Around line 1-31: Convert the standalone program into a test: replace fn
main() with a #[test] fn (e.g., fn typechecker_basic()) that constructs the
TypeChecker via TypeChecker::new(), builds the same Program, calls
tc.check_types(&program) and assert!(res.is_ok()) (or assert_eq! on the expected
result) instead of println!; move this code into a test source (so Cargo runs it
as part of cargo test) and keep the same symbols (TypeChecker::new, check_types,
Program, Statement variants) to preserve behavior.
In `@tests/test_local_pattern.wfl`:
- Line 1: The action definition uses non-canonical syntax: change the line
defining "test_action" from using "needs some_param" to the canonical WFL form
"with parameters some_param"; update the declaration for the action named
test_action accordingly so it reads "define action called test_action with
parameters some_param:" to match the required grammar.
---
Nitpick comments:
In `@src/analyzer/mod.rs`:
- Line 1977: The TODO about "Add parameter type validation" should be
implemented: add a new helper (e.g., validate_action_call_parameters or
validate_action_call in the Analyzer impl) that, given an action call node (call
expression) and its resolved action signature, iterates parameters, checks each
argument's static/semantic type against the expected parameter type, and emits
diagnostics via the Analyzer's diagnostics collector (e.g.,
self.diagnostics.emit or self.report_error) for mismatches or wrong arity; call
this helper from the existing analyze_action_call / analyze_node path where
calls are resolved so validation runs during semantic analysis, and remove the
TODO comment. Ensure you reference the resolved signature lookup code and use
existing symbol names for diagnostic emission so behavior integrates with
current Analyzer flow.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/analyzer/mod.rssrc/typechecker/mod.rstest_typechecker.rstest_typechecker.wfltests/test_local_pattern.wfl
| if server_type != Type::Text && server_type != Type::Unknown && server_type != Type::Error { | ||
| self.type_error( |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify analyzer/typechecker coverage for server statements.
rg -n 'WaitForRequestStatement|StopAcceptingConnectionsStatement|CloseServerStatement' src/analyzer/mod.rs src/typechecker/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 787
🏁 Script executed:
# First, let's find the check_server_expression_type function
rg -n 'fn check_server_expression_type' src/typechecker/Repository: WebFirstLanguage/wfl
Length of output: 127
🏁 Script executed:
# Let's look at the specific lines mentioned in the review comment
sed -n '310,325p' src/typechecker/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 571
🏁 Script executed:
# Check the other range mentioned (1671-1684)
sed -n '1665,1690p' src/typechecker/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 927
🏁 Script executed:
# Let's understand the Type enum to see what Unknown type represents
rg -n 'enum Type|Unknown' src/typechecker/mod.rs | head -30Repository: WebFirstLanguage/wfl
Length of output: 2131
🏁 Script executed:
# Check WaitForRequestStatement handling in the analyzer
sed -n '1346,1365p' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 847
🏁 Script executed:
# Let's see what the analyzer does with undefined identifiers
rg -n 'undefined|not defined|unknown variable' src/analyzer/mod.rs | head -20Repository: WebFirstLanguage/wfl
Length of output: 1511
🏁 Script executed:
# Check how infer_expression_type handles identifiers
rg -n 'fn infer_expression_type' src/typechecker/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 143
🏁 Script executed:
# Look at the infer_expression_type function to understand how it handles undefined vars
sed -n '139,200p' src/typechecker/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 3204
🏁 Script executed:
# Check if semantic analysis validates variable existence before type checking
rg -n 'validate.*variable|undefined.*identifier|symbol.*not.*found' src/analyzer/mod.rs | head -15Repository: WebFirstLanguage/wfl
Length of output: 46
🏁 Script executed:
# Let's examine the analyze_expression method to see if it validates variable references
rg -n 'fn analyze_expression' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 132
🏁 Script executed:
# Check how Expression::Identifier is handled in the analyzer
rg -B5 -A10 'Expression::Identifier.*=>' src/analyzer/mod.rs | head -50Repository: WebFirstLanguage/wfl
Length of output: 46
🏁 Script executed:
# Let's see the full infer_expression_type function implementation
sed -n '1962,2050p' src/typechecker/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 4362
🏁 Script executed:
# Check if infer_expression_type returns Unknown for undefined variables/identifiers
rg -B5 -A10 'Expression::Identifier' src/typechecker/mod.rs | head -60Repository: WebFirstLanguage/wfl
Length of output: 46
🏁 Script executed:
# Look for how symbols are resolved in the typechecker
rg -n 'get_symbol|lookup.*symbol' src/typechecker/mod.rs | head -20Repository: WebFirstLanguage/wfl
Length of output: 1788
🏁 Script executed:
# Let's check if StopAcceptingConnectionsStatement and CloseServerStatement are analyzed in analyzer
sed -n '1340,1380p' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 1615
🏁 Script executed:
# Search more broadly for these statements in the analyzer
rg -B3 -A10 'StopAcceptingConnectionsStatement|CloseServerStatement' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 46
🏁 Script executed:
# Let's see the full analyze_expression method
sed -n '1767,1850p' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 3980
🏁 Script executed:
# Check what analyze_expression does with identifiers
rg -B5 -A15 'Expression::Identifier.*=>' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 46
🏁 Script executed:
# Check if StopAcceptingConnectionsStatement exists in analyzer
rg -n 'StopAcceptingConnectionsStatement' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 46
🏁 Script executed:
# Check if CloseServerStatement exists in analyzer
rg -n 'CloseServerStatement' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 46
🏁 Script executed:
# Let's look at the full Statement match in the analyzer to see what's covered
rg -n 'Statement::' src/analyzer/mod.rs | head -40Repository: WebFirstLanguage/wfl
Length of output: 2398
🏁 Script executed:
# Check file size to understand structure better
wc -l src/analyzer/mod.rs src/typechecker/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 133
🏁 Script executed:
# Let's see what statements are handled in the analyzer's analyze_statement
rg -B2 'fn analyze_statement' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 131
🏁 Script executed:
# Get the full list of Statement:: patterns handled in the analyzer
rg -n 'Statement::' src/analyzer/mod.rs | wc -lRepository: WebFirstLanguage/wfl
Length of output: 66
🏁 Script executed:
# Search for all statement handlers in analyzer to see the full picture
rg -n 'Statement::' src/analyzer/mod.rs | tail -50Repository: WebFirstLanguage/wfl
Length of output: 2951
🏁 Script executed:
# Now let's check what statements ARE handled in the typechecker
rg -n 'Statement::' src/typechecker/mod.rs | grep -E 'StopAcceptingConnections|CloseServer|WaitForRequest'Repository: WebFirstLanguage/wfl
Length of output: 529
🏁 Script executed:
# Let's check if there's a catch-all or default case in the analyzer's statement handler
sed -n '380,450p' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 2646
🏁 Script executed:
# Look at where the analyzer statement matching begins and how comprehensive it is
rg -B5 'fn analyze_statement' src/analyzer/mod.rs -A 20Repository: WebFirstLanguage/wfl
Length of output: 888
🏁 Script executed:
# Let's see if StopAcceptingConnectionsStatement and CloseServerStatement are truly missing from analyzer
# by checking the full list of statements in the match statement
sed -n '388,1530p' src/analyzer/mod.rs | grep -E 'Statement::[A-Za-z]+ *\{|Statement::[A-Za-z]+ *=>' | tail -30Repository: WebFirstLanguage/wfl
Length of output: 1674
🏁 Script executed:
# Let's check the end of the match statement in analyze_statement to see if there's a catch-all
sed -n '1500,1550p' src/analyzer/mod.rsRepository: WebFirstLanguage/wfl
Length of output: 1675
🏁 Script executed:
# Count unique Statement types in analyzer vs typechecker
echo "=== Analyzer Statement types ===" && rg 'Statement::' src/analyzer/mod.rs | grep -oE 'Statement::[A-Za-z]+' | sort -u | wc -l && echo "=== Typechecker Statement types ===" && rg 'Statement::' src/typechecker/mod.rs | grep -oE 'Statement::[A-Za-z]+' | sort -u | wc -lRepository: WebFirstLanguage/wfl
Length of output: 138
🏁 Script executed:
# Get the unique statement types
echo "=== Statements in analyzer ===" && rg 'Statement::' src/analyzer/mod.rs | grep -oE 'Statement::[A-Za-z]+' | sort -u && echo "=== Statements in typechecker ===" && rg 'Statement::' src/typechecker/mod.rs | grep -oE 'Statement::[A-Za-z]+' | sort -uRepository: WebFirstLanguage/wfl
Length of output: 3435
Add semantic analysis for StopAcceptingConnectionsStatement and CloseServerStatement to catch undefined server identifiers.
These statements are missing from the analyzer's semantic checks (lines 1671 and 1678 in typechecker only). Unlike WaitForRequestStatement which calls analyze_expression(server) in the analyzer, the Stop/Close variants have no analyzer coverage. When a type checker encounters an undefined server variable, infer_expression_type returns Type::Unknown, which check_server_expression_type accepts without error (line 316). This allows undefined server identifiers to pass type checking.
Also applies to: 1671-1684
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/typechecker/mod.rs` around lines 316 - 317, Add semantic analysis for
StopAcceptingConnectionsStatement and CloseServerStatement by invoking
analyze_expression(server) for their server expressions (same as
WaitForRequestStatement does) before calling check_server_expression_type;
ensure check_server_expression_type treats Type::Unknown from
infer_expression_type as an error by calling self.type_error with a clear
message (use the existing type_error path used when server_type is not
Type::Text/Unknown/Error), referencing the functions
check_server_expression_type, analyze_expression, infer_expression_type and the
statement types StopAcceptingConnectionsStatement and CloseServerStatement so
undefined server identifiers are caught during type checking.
| use wfl::typechecker::TypeChecker; | ||
| use wfl::parser::ast::*; | ||
|
|
||
| fn main() { | ||
| let mut tc = TypeChecker::new(); | ||
| let program = Program { | ||
| statements: vec![Statement::ActionDefinition { | ||
| name: "test".to_string(), | ||
| parameters: vec![], | ||
| body: vec![ | ||
| Statement::VariableDeclaration { | ||
| name: "local_var".to_string(), | ||
| value: Expression::Literal(Literal::Integer(10), 1, 1), | ||
| is_constant: false, | ||
| line: 1, | ||
| column: 1, | ||
| }, | ||
| Statement::DisplayStatement { | ||
| value: Expression::Variable("local_var".to_string(), 2, 1), | ||
| line: 2, | ||
| column: 1, | ||
| } | ||
| ], | ||
| return_type: None, | ||
| line: 1, | ||
| column: 1, | ||
| }], | ||
| }; | ||
| let res = tc.check_types(&program); | ||
| println!("{:?}", res); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for the test_typechecker.rs file in the repository
echo "=== Searching for test_typechecker.rs ==="
find . -name "test_typechecker.rs" -type f 2>/dev/null
echo -e "\n=== Checking tests/ directory structure ==="
fd -t f '\.rs$' tests/ 2>/dev/null | head -20
echo -e "\n=== Checking root-level .rs files ==="
fd -t f -maxdepth 1 '\.rs$' 2>/dev/nullRepository: WebFirstLanguage/wfl
Length of output: 877
🏁 Script executed:
#!/bin/bash
# If file exists, check its content for #[test] or fn main()
if [ -f "test_typechecker.rs" ]; then
echo "=== Content of test_typechecker.rs ==="
cat -n test_typechecker.rs
elif [ -f "tests/test_typechecker.rs" ]; then
echo "=== Content of tests/test_typechecker.rs ==="
cat -n tests/test_typechecker.rs
else
echo "File not found in expected locations"
# Search more broadly
rg -l "test_typechecker" . 2>/dev/null
fiRepository: WebFirstLanguage/wfl
Length of output: 1283
Move to tests/ and convert to #[test] so CI discovers it.
This file at the repository root using fn main() won't run under cargo test. Following project conventions, place it in tests/typechecker_test.rs, add the #[test] attribute, and replace println! with assert!.
Suggested structure
-use wfl::typechecker::TypeChecker;
-use wfl::parser::ast::*;
-
-fn main() {
+use wfl::parser::ast::*;
+use wfl::typechecker::TypeChecker;
+
+#[test]
+fn local_action_variable_typechecks() {
let mut tc = TypeChecker::new();
let program = Program {
statements: vec![Statement::ActionDefinition {
@@ -27,5 +27,5 @@
}],
};
let res = tc.check_types(&program);
- println!("{:?}", res);
+ assert!(res.is_ok(), "Expected local variable usage to type-check");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| use wfl::typechecker::TypeChecker; | |
| use wfl::parser::ast::*; | |
| fn main() { | |
| let mut tc = TypeChecker::new(); | |
| let program = Program { | |
| statements: vec![Statement::ActionDefinition { | |
| name: "test".to_string(), | |
| parameters: vec![], | |
| body: vec![ | |
| Statement::VariableDeclaration { | |
| name: "local_var".to_string(), | |
| value: Expression::Literal(Literal::Integer(10), 1, 1), | |
| is_constant: false, | |
| line: 1, | |
| column: 1, | |
| }, | |
| Statement::DisplayStatement { | |
| value: Expression::Variable("local_var".to_string(), 2, 1), | |
| line: 2, | |
| column: 1, | |
| } | |
| ], | |
| return_type: None, | |
| line: 1, | |
| column: 1, | |
| }], | |
| }; | |
| let res = tc.check_types(&program); | |
| println!("{:?}", res); | |
| } | |
| use wfl::parser::ast::*; | |
| use wfl::typechecker::TypeChecker; | |
| #[test] | |
| fn local_action_variable_typechecks() { | |
| let mut tc = TypeChecker::new(); | |
| let program = Program { | |
| statements: vec![Statement::ActionDefinition { | |
| name: "test".to_string(), | |
| parameters: vec![], | |
| body: vec![ | |
| Statement::VariableDeclaration { | |
| name: "local_var".to_string(), | |
| value: Expression::Literal(Literal::Integer(10), 1, 1), | |
| is_constant: false, | |
| line: 1, | |
| column: 1, | |
| }, | |
| Statement::DisplayStatement { | |
| value: Expression::Variable("local_var".to_string(), 2, 1), | |
| line: 2, | |
| column: 1, | |
| } | |
| ], | |
| return_type: None, | |
| line: 1, | |
| column: 1, | |
| }], | |
| }; | |
| let res = tc.check_types(&program); | |
| assert!(res.is_ok(), "Expected local variable usage to type-check"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test_typechecker.rs` around lines 1 - 31, Convert the standalone program into
a test: replace fn main() with a #[test] fn (e.g., fn typechecker_basic()) that
constructs the TypeChecker via TypeChecker::new(), builds the same Program,
calls tc.check_types(&program) and assert!(res.is_ok()) (or assert_eq! on the
expected result) instead of println!; move this code into a test source (so
Cargo runs it as part of cargo test) and keep the same symbols
(TypeChecker::new, check_types, Program, Statement variants) to preserve
behavior.
| @@ -0,0 +1,5 @@ | |||
| define action called test_action needs some_param: | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find and examine the test file
find . -name "test_local_pattern.wfl" -type f 2>/dev/null
# Search for all WFL action definitions to understand the syntax patterns
echo "=== Action definitions with 'needs' ==="
rg -n 'define action called .* needs ' --glob '**/*.wfl' 2>/dev/null | head -20
echo "=== Action definitions with 'with parameters' ==="
rg -n 'define action called .* with parameters' --glob '**/*.wfl' 2>/dev/null | head -20Repository: WebFirstLanguage/wfl
Length of output: 2813
🏁 Script executed:
#!/bin/bash
# Check examples/bad.wfl to see if it intentionally demonstrates wrong syntax
echo "=== examples/bad.wfl ==="
head -20 examples/bad.wfl
echo ""
echo "=== Nexus/test_action_syntax.wfl ==="
cat Nexus/test_action_syntax.wfl
echo ""
echo "=== test_fix_verification.wfl ==="
cat test_fix_verification.wflRepository: WebFirstLanguage/wfl
Length of output: 2032
Use the canonical action-parameter syntax.
needs some_param should be with parameters some_param to match the required WFL grammar.
🔧 Suggested fix
-define action called test_action needs some_param:
+define action called test_action with parameters some_param:Per coding guidelines, WFL action definitions must use define action called name with parameters x: syntax, not needs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| define action called test_action needs some_param: | |
| define action called test_action with parameters some_param: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_local_pattern.wfl` at line 1, The action definition uses
non-canonical syntax: change the line defining "test_action" from using "needs
some_param" to the canonical WFL form "with parameters some_param"; update the
declaration for the action named test_action accordingly so it reads "define
action called test_action with parameters some_param:" to match the required
grammar.
|
@google-labs-jules[bot] extra-depth follow-up after latest updates. Thanks for the iteration — checks are green, but I see blocking substantive issues before merge:
Non-blocking:
I’ll re-check next run and merge once these blockers are resolved and no substantive review notes remain. |
Thank you for the continued review. Here are the latest updates to address your concerns:
|
…gression of #382 parameter logic and version metadata from outdated branch sync.\n- Replace ad-hoc local scope fallback in `TypeChecker::check_pattern_expression_types` with standard `self.infer_expression_type(&Expression::Variable(name))`, ensuring perfectly aligned semantics between `ListReference` lookups and ordinary variables.\n- Remove temporary test fixture files. Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/typechecker/mod.rs (2)
254-277:⚠️ Potential issue | 🟠 MajorDo not silently accept
Unknownfor pattern list references.On Line 275,
Type::Unknown | Type::Any | Type::Erroris treated as a no-op. That can bypassList<Text>enforcement when resolution/inference is incomplete, so invalid list references can pass without diagnostics.🛠️ Proposed fix
PatternExpression::ListReference(name) => { - // TypeChecker delegates undefined variable checks to Analyzer. - // If it can be inferred, we enforce List<Text> semantics. - let var_type = - self.infer_expression_type(&Expression::Variable(name.clone(), line, column)); + let var_type = + self.infer_expression_type(&Expression::Variable(name.clone(), line, column)); + + if var_type == Type::Unknown { + self.type_error( + format!("Pattern list reference '{name}' could not be resolved to a typed list"), + Some(Type::List(Box::new(Type::Text))), + Some(Type::Unknown), + line, + column, + ); + return; + } + match var_type { Type::List(ref item_type) => { // Check item type - should be text/unknown for pattern listsBased on learnings
Type Checker must perform static type analysis with intelligent inference in src/typechecker/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/typechecker/mod.rs` around lines 254 - 277, PatternExpression::ListReference currently treats Type::Unknown, Type::Any, and Type::Error as a no-op after calling infer_expression_type, which lets invalid pattern list references bypass the List<Text> constraint; change the branch for Type::Unknown | Type::Any | Type::Error to invoke the same enforcement path as Type::List mismatches by calling type_error (using the same message/context as the existing mismatch) or otherwise emit a diagnostic that the pattern expects List<Text> but type inference yielded Unknown/Any/Error; reference infer_expression_type, PatternExpression::ListReference, Type::Unknown/Any/Error, and type_error to locate and update the logic so Unknown/Any/Error no longer silently pass.
301-302:⚠️ Potential issue | 🟠 MajorUndefined server identifiers can still pass stop/close validation.
Line 301 allows
Type::Unknown, soStopAcceptingConnectionsStatementandCloseServerStatementcan accept unresolvedserverexpressions without error.🛠️ Proposed fix
fn check_server_expression_type( &mut self, server_expr: &Expression, line: usize, column: usize, ) { let server_type = self.infer_expression_type(server_expr); - if server_type != Type::Text && server_type != Type::Unknown && server_type != Type::Error { + if server_type == Type::Unknown { + self.type_error( + "Server reference must resolve to Text".to_string(), + Some(Type::Text), + Some(Type::Unknown), + line, + column, + ); + return; + } + if server_type != Type::Text && server_type != Type::Error { self.type_error( "Server must be a text string".to_string(), Some(Type::Text), Some(server_type), line, column, ); } }Based on learnings
Analyzer must perform semantic validation and static analysis in src/analyzer/andType Checker must perform static type analysis with intelligent inference in src/typechecker/.Also applies to: 1657-1669
🧹 Nitpick comments (1)
src/typechecker/mod.rs (1)
3736-3947: Please add regression tests for unresolved-identifier paths introduced here.The new tests cover concrete type mismatches, but they miss unresolved identifier cases for:
serverinStopAcceptingConnectionsStatement/CloseServerStatementtimeoutinWaitForRequestStatement- action/local-scope
PatternExpression::ListReferenceelement checks🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/typechecker/mod.rs` around lines 3736 - 3947, Add regression tests that assert unresolved-identifier errors are produced by TypeChecker::check_types when referenced names are missing: create programs that call StopAcceptingConnectionsStatement and CloseServerStatement with a non-declared server variable, a WaitForRequestStatement with a timeout expression referencing a non-declared identifier, and a PatternExpression::ListReference that refers to an identifier not present in the local/action scope; for each, call TypeChecker::new().check_types(...) and assert result.is_err() and that the returned errors contain an unresolved-identifier message for the specific symbol referenced (e.g., the missing server name, timeout name, or list name) so these cases are covered alongside the existing concrete type-mismatch tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/typechecker/mod.rs`:
- Around line 1577-1582: The current check in the timeout handling incorrectly
allows Type::Unknown, letting unresolved identifiers slip through; update the
logic in the timeout validation (the branch that calls infer_expression_type for
the timeout expression) to treat Type::Unknown as invalid and emit a diagnostic
instead of accepting it; specifically modify the conditional that currently
permits Type::Unknown so that only Type::Number (and possibly Type::Error) are
allowed, and ensure you create a proper error/diagnostic when
infer_expression_type(timeout_expr) returns Type::Unknown (use the same
diagnostic/reporting utilities the typechecker uses elsewhere).
In `@tests/test_local_type.wfl`:
- Around line 1-4: The test file defining action my_action in
tests/test_local_type.wfl is not discovered by CI; either move this file (and
its counterpart test_global_type.wfl) into the TestPrograms/ directory so the CI
run-wfl-programs job executes them, or convert them to the canonical test format
by renaming to *.test.wfl and wrapping the WFL program in describe/test blocks
per the testing guide so the test harness picks them up; update references to
the action name (my_action) if any test runner expectations rely on its
location.
---
Duplicate comments:
In `@src/typechecker/mod.rs`:
- Around line 254-277: PatternExpression::ListReference currently treats
Type::Unknown, Type::Any, and Type::Error as a no-op after calling
infer_expression_type, which lets invalid pattern list references bypass the
List<Text> constraint; change the branch for Type::Unknown | Type::Any |
Type::Error to invoke the same enforcement path as Type::List mismatches by
calling type_error (using the same message/context as the existing mismatch) or
otherwise emit a diagnostic that the pattern expects List<Text> but type
inference yielded Unknown/Any/Error; reference infer_expression_type,
PatternExpression::ListReference, Type::Unknown/Any/Error, and type_error to
locate and update the logic so Unknown/Any/Error no longer silently pass.
---
Nitpick comments:
In `@src/typechecker/mod.rs`:
- Around line 3736-3947: Add regression tests that assert unresolved-identifier
errors are produced by TypeChecker::check_types when referenced names are
missing: create programs that call StopAcceptingConnectionsStatement and
CloseServerStatement with a non-declared server variable, a
WaitForRequestStatement with a timeout expression referencing a non-declared
identifier, and a PatternExpression::ListReference that refers to an identifier
not present in the local/action scope; for each, call
TypeChecker::new().check_types(...) and assert result.is_err() and that the
returned errors contain an unresolved-identifier message for the specific symbol
referenced (e.g., the missing server name, timeout name, or list name) so these
cases are covered alongside the existing concrete type-mismatch tests.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/analyzer/mod.rssrc/typechecker/mod.rstests/test_global_type.wfltests/test_local_type.wfl
| define action called my_action: | ||
| store x as 10 | ||
| change x to "string" | ||
| end action |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Direct references to test_local_type.wfl ==="
rg -n 'test_local_type\.wfl' . || echo "No direct references found"
echo ""
echo "=== Potential WFL test harness entrypoints ==="
rg -n 'wfl --test|--test .*\.wfl' . || echo "No harness patterns found"
echo ""
echo "=== WFL files currently under tests/ ==="
fd -a '\.wfl$' tests 2>/dev/null || echo "No .wfl files in tests/"
echo ""
echo "=== Check if TestPrograms directory exists and its structure ==="
ls -la TestPrograms/ 2>/dev/null | head -20 || echo "TestPrograms not found"
echo ""
echo "=== Rust test files that might load WFL fixtures ==="
fd -a 'test\.rs$' tests 2>/dev/null | head -10 || echo "No rust test files found"Repository: WebFirstLanguage/wfl
Length of output: 3187
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Content of test_local_type.wfl (the file under review) ==="
cat -n tests/test_local_type.wfl
echo ""
echo "=== Content of test_global_type.wfl (related file) ==="
cat -n tests/test_global_type.wfl
echo ""
echo "=== Check if any Rust test file references these .wfl files ==="
rg -l 'test_local_type|test_global_type|tests/.*\.wfl' tests/*.rs
echo ""
echo "=== Sample Rust test file to understand fixture pattern ==="
head -50 tests/binary_io_test.rs
echo ""
echo "=== Check if tests/ .wfl files use describe/test blocks ==="
rg -n 'describe|test' tests/*.wflRepository: WebFirstLanguage/wfl
Length of output: 406
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== CI configuration files ==="
fd -a '\.(yml|yaml|toml|sh)$' . -path './.*' -prune -o -type f | grep -E '(ci|github|test|Makefile)' | head -20
echo ""
echo "=== Check Cargo.toml for test configuration ==="
rg -A 10 '\[\[test\]\]|\[dev-dependencies\]' Cargo.toml || echo "No test configuration found"
echo ""
echo "=== Any scripts that run tests ==="
fd -a '\.sh$' . | xargs grep -l 'wfl --test|TestPrograms' 2>/dev/null || echo "No test runner scripts found"
echo ""
echo "=== GitHub Actions workflows ==="
find .github -name '*.yml' -o -name '*.yaml' 2>/dev/null | xargs cat 2>/dev/null || echo "No workflows found"Repository: WebFirstLanguage/wfl
Length of output: 280
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== CI workflow files ==="
find .github -name '*.yml' -o -name '*.yaml' 2>/dev/null || echo "No GitHub Actions workflows"
echo ""
echo "=== Cargo.toml test configuration ==="
rg -A 10 '\[\[test\]\]|\[dev-dependencies\]' Cargo.toml || echo "No test configuration"
echo ""
echo "=== Shell scripts that might run tests ==="
fd '\.sh$' . 2>/dev/null | xargs grep -l 'wfl --test\|TestPrograms\|test' 2>/dev/null | head -5 || echo "No test runner scripts"
echo ""
echo "=== Makefile or build scripts ==="
fd 'Makefile|build.sh' . 2>/dev/null || echo "No Makefile/build.sh"
echo ""
echo "=== README or test documentation ==="
rg -n 'tests/.*\.wfl|test_local_type|test_global_type' README.md TESTING.md CONTRIBUTING.md 2>/dev/null || echo "No mentions in docs"Repository: WebFirstLanguage/wfl
Length of output: 744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Content of ci.yml workflow ==="
cat .github/workflows/ci.yml
echo ""
echo "=== Content of run_integration_tests.sh ==="
cat ./scripts/run_integration_tests.shRepository: WebFirstLanguage/wfl
Length of output: 25190
Move these test files to TestPrograms/ or convert to the proper test format.
These files in tests/ are not discovered by the CI harness. The project runs WFL programs only from TestPrograms/ directory (see ci.yml run-wfl-programs job and run_integration_tests.sh). They also don't use the canonical test framework format (describe/test blocks). Either:
- Move to
TestPrograms/test_local_type.wflandTestPrograms/test_global_type.wfl, or - Rename to
.test.wfland adddescribe/testblocks per the testing guide
Without this, these files won't be executed and won't prevent regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_local_type.wfl` around lines 1 - 4, The test file defining action
my_action in tests/test_local_type.wfl is not discovered by CI; either move this
file (and its counterpart test_global_type.wfl) into the TestPrograms/ directory
so the CI run-wfl-programs job executes them, or convert them to the canonical
test format by renaming to *.test.wfl and wrapping the WFL program in
describe/test blocks per the testing guide so the test harness picks them up;
update references to the action name (my_action) if any test runner expectations
rely on its location.
|
@google-labs-jules[bot] extra-depth follow-up after latest changes. Thanks for the cleanup and re-sync with main. CI is green, but I still see substantive blockers to close before merge: Blocking checklist:
Non-blocking:
I’ll re-check next run and merge once these substantive items are resolved and no new substantive notes remain. |
Thank you for the review. I have completed the following blocking items:
All CI checks, tests, clippy, and rustfmt passes locally. |
…pe::Unknown` and `Type::Error` fallbacks from `TypeChecker`'s pattern list references, `server`, and `timeout` expressions to ensure strict constraint enforcement for unresolved identifiers.\n- Remove temporary exploratory test scripts. Co-authored-by: logbie <1138960+logbie@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Some(timeout_expr) = timeout { | ||
| let timeout_type = self.infer_expression_type(timeout_expr); | ||
| if timeout_type != Type::Number { | ||
| self.type_error( | ||
| "Timeout must be a number".to_string(), | ||
| Some(Type::Number), | ||
| Some(timeout_type), | ||
| *line, | ||
| *column, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The timeout validation flags any non-Number type, including Type::Error, which can lead to duplicate/cascading errors when the timeout expression is already invalid. Consider allowing Type::Error to pass through (and optionally special-casing Type::Unknown to produce a clearer “must resolve to a number” diagnostic).
|
|
||
| // Analyze the pattern expression to catch undefined list references | ||
| self.analyze_pattern_expression(pattern, *line, *column); |
There was a problem hiding this comment.
analyze_pattern_expression is now responsible for catching undefined list references in patterns, but there’s no unit test exercising the new failure mode (e.g., a pattern referencing an undefined list name). Adding an analyzer-level test would help prevent regressions in this semantic check.
| let var_type = | ||
| self.infer_expression_type(&Expression::Variable(name.clone(), line, column)); | ||
| match var_type { | ||
| Type::List(ref item_type) => { | ||
| if **item_type != Type::Text { | ||
| self.type_error( | ||
| format!("Pattern list reference '{name}' must contain Text, got List of {item_type}"), | ||
| Some(Type::List(Box::new(Type::Text))), | ||
| Some(var_type.clone()), | ||
| line, | ||
| column, | ||
| ); | ||
| } | ||
| } | ||
| _ => { | ||
| self.type_error( | ||
| format!( | ||
| "Pattern list reference '{name}' must be a List of Text, got {var_type}" | ||
| ), | ||
| Some(Type::List(Box::new(Type::Text))), | ||
| Some(var_type.clone()), | ||
| line, | ||
| column, | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
match var_type { ... } moves var_type, but both match arms call var_type.clone(), which is not valid after the move and should not compile. Match on &var_type (or bind with t @ ...) so you can still pass the found type into type_error without moving it.
| if server_type != Type::Text { | ||
| self.type_error( | ||
| "Server must be a text string".to_string(), | ||
| Some(Type::Text), | ||
| Some(server_type), | ||
| line, | ||
| column, | ||
| ); |
There was a problem hiding this comment.
check_server_expression_type reports an error for any non-Text type, including Type::Error. Since Type::Error indicates the expression already produced a type error, this can create noisy cascading diagnostics; consider skipping the server-type check when infer_expression_type returns Error (and potentially tailoring messaging for Unknown).
| if server_type != Type::Text { | |
| self.type_error( | |
| "Server must be a text string".to_string(), | |
| Some(Type::Text), | |
| Some(server_type), | |
| line, | |
| column, | |
| ); | |
| match server_type { | |
| // If the server expression has already produced a type error, avoid | |
| // emitting an additional, less-specific diagnostic here. | |
| Type::Error => {} | |
| _ => { | |
| if server_type != Type::Text { | |
| self.type_error( | |
| "Server must be a text string".to_string(), | |
| Some(Type::Text), | |
| Some(server_type), | |
| line, | |
| column, | |
| ); | |
| } | |
| } |
Fix TODOs in
src/typechecker/mod.rsrelated to web server statements and pattern definition type checking.WaitForRequestStatement,StopAcceptingConnectionsStatement, andCloseServerStatement. It now infers the type of theserverargument and verifies it evaluates toText. It also checks thattimeoutevaluates to aNumber.PatternDefinition. Added a new function,check_pattern_expression_types, which recursively traverses thePatternExpressionAST. It specifically checksListReferencenodes to ensure the referenced symbol exists in the environment and is aList.test_web_server_statements_type_checkingandtest_pattern_definition_type_checking) to ensure the correctness of the new type-checking logic, catching errors like passing a number as a server name or referencing a non-list variable in a pattern definition.All tests, including
cargo clippyandcargo fmt, pass successfully.PR created automatically by Jules for task 17688715437332225663 started by @logbie
Summary by CodeRabbit
Release Notes
New Features
Tests