Skip to content

Commit 0672585

Browse files
claude[bot]logbie
andcommitted
fix: Improve module system validation and restrictions
- Enhanced export constant validation to verify items are actually constants - Improved type checking for include/export statements with proper error messages - Relaxed control flow restrictions to allow return statements in included files - Clarified export statement documentation with current benefits and usage Fixes identified issues from code review while maintaining backward compatibility. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: logbie <logbie@users.noreply.github.com>
1 parent d94e5ab commit 0672585

6 files changed

Lines changed: 181 additions & 20 deletions

File tree

Docs/04-advanced-features/modules.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,9 @@ end try
510510

511511
## Limitations
512512

513-
## Export Statement (V2 Foundation)
513+
## Export Statement
514514

515-
WFL now includes an `export` statement that validates the existence of items for future module namespaces:
515+
WFL includes an `export` statement that documents and validates module interfaces:
516516

517517
```wfl
518518
# Define items in a module
@@ -527,13 +527,18 @@ end
527527
528528
store constant VERSION as "1.0.0"
529529
530-
# Export specific items for future namespace features
530+
# Document which items are intended for external use
531531
export container Person
532532
export action greet
533533
export constant VERSION
534534
```
535535

536-
The export statement currently validates that the named item exists in the current scope. In future versions, this will enable selective module exposure.
536+
**Current Benefits:**
537+
- **Documentation**: Makes module interface explicit and clear
538+
- **Validation**: Ensures exported items actually exist at compile/lint time
539+
- **Best Practices**: Encourages conscious design of module interfaces
540+
541+
**Future Enhancements:** The export statement establishes the foundation for selective module exposure and namespace control in future WFL versions.
537542

538543
### Current Limitations (V2)
539544

src/interpreter/mod.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3001,11 +3001,11 @@ impl Interpreter {
30013001
// 10. Handle result
30023002
match result {
30033003
Ok((_, ControlFlow::None)) => Ok((Value::Null, ControlFlow::None)),
3004-
Ok((_, ControlFlow::Return(_))) => Err(RuntimeError::new(
3005-
"Cannot use 'return' in included file scope".to_string(),
3006-
*line,
3007-
*column,
3008-
)),
3004+
Ok((val, ControlFlow::Return(_))) => {
3005+
// Return statements in included files are allowed and simply return the value
3006+
// This enables utility functions in included files to use return statements
3007+
Ok((val, ControlFlow::None))
3008+
}
30093009
Ok((_, ControlFlow::Break)) => Err(RuntimeError::new(
30103010
"Cannot use 'break' in included file scope".to_string(),
30113011
*line,
@@ -3097,11 +3097,20 @@ impl Interpreter {
30973097
}
30983098
}
30993099
ExportType::Constant => {
3100-
// Check if constant exists
3100+
// Check if the variable exists and is actually a constant
31013101
if let Some(_value) = env.borrow().get(name) {
3102-
// For constants, we could validate it's actually a constant
3103-
// For now, just verify it exists
3104-
Ok((Value::Null, ControlFlow::None))
3102+
if env.borrow().is_constant(name) {
3103+
Ok((Value::Null, ControlFlow::None))
3104+
} else {
3105+
Err(RuntimeError::new(
3106+
format!(
3107+
"Variable '{}' is not a constant and cannot be exported as one",
3108+
name
3109+
),
3110+
*line,
3111+
*column,
3112+
))
3113+
}
31053114
} else {
31063115
Err(RuntimeError::new(
31073116
format!("Constant '{}' not found in current scope", name),

src/typechecker/mod.rs

Lines changed: 98 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1672,15 +1672,106 @@ impl TypeChecker {
16721672
}
16731673
}
16741674

1675-
Statement::IncludeStatement { path, .. } => {
1676-
// Type check the path expression
1677-
self.infer_expression_type(path);
1678-
// Include statements execute in parent scope but don't need special type checking
1675+
Statement::IncludeStatement {
1676+
path, line, column, ..
1677+
} => {
1678+
// Type check the path expression - must be a string
1679+
let path_type = self.infer_expression_type(path);
1680+
if path_type != Type::Text && path_type != Type::Unknown && path_type != Type::Error
1681+
{
1682+
self.type_error(
1683+
"Expected string for include path".to_string(),
1684+
Some(Type::Text),
1685+
Some(path_type),
1686+
*line,
1687+
*column,
1688+
);
1689+
}
1690+
// Note: Include statements execute in parent scope, making their symbols available
1691+
// Full symbol resolution would require parsing the included file during type checking
16791692
}
16801693

1681-
Statement::ExportStatement { .. } => {
1682-
// Export statements are validated at runtime
1683-
// Type checking here would require deeper integration
1694+
Statement::ExportStatement {
1695+
export_type,
1696+
name,
1697+
line,
1698+
column,
1699+
..
1700+
} => {
1701+
// Basic type checking for export statements
1702+
// Check if the exported item exists in the current scope
1703+
match export_type {
1704+
crate::parser::ast::ExportType::Container => {
1705+
if let Some(_container) = self.analyzer.get_container(name) {
1706+
// Container exists - export is valid
1707+
} else {
1708+
self.type_error(
1709+
format!("Container '{}' not found for export", name),
1710+
None,
1711+
None,
1712+
*line,
1713+
*column,
1714+
);
1715+
}
1716+
}
1717+
crate::parser::ast::ExportType::Action => {
1718+
// Check if action exists as a symbol in the current scope
1719+
if let Some(symbol) = self.analyzer.get_symbol(name) {
1720+
match symbol.kind {
1721+
crate::analyzer::SymbolKind::Function { .. } => {
1722+
// Action exists - export is valid
1723+
}
1724+
_ => {
1725+
self.type_error(
1726+
format!(
1727+
"'{}' is not an action and cannot be exported as one",
1728+
name
1729+
),
1730+
None,
1731+
None,
1732+
*line,
1733+
*column,
1734+
);
1735+
}
1736+
}
1737+
} else {
1738+
self.type_error(
1739+
format!("Action '{}' not found for export", name),
1740+
None,
1741+
None,
1742+
*line,
1743+
*column,
1744+
);
1745+
}
1746+
}
1747+
crate::parser::ast::ExportType::Constant => {
1748+
// Check if variable exists as a symbol in the current scope
1749+
if let Some(symbol) = self.analyzer.get_symbol(name) {
1750+
match symbol.kind {
1751+
crate::analyzer::SymbolKind::Variable { .. } => {
1752+
// Variable exists, runtime will verify it's actually a constant
1753+
}
1754+
_ => {
1755+
self.type_error(
1756+
format!("'{}' is not a variable and cannot be exported as constant", name),
1757+
None,
1758+
None,
1759+
*line,
1760+
*column,
1761+
);
1762+
}
1763+
}
1764+
} else {
1765+
self.type_error(
1766+
format!("Constant '{}' not found for export", name),
1767+
None,
1768+
None,
1769+
*line,
1770+
*column,
1771+
);
1772+
}
1773+
}
1774+
}
16841775
}
16851776
}
16861777
}

test_export_validation.wfl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Test export constant validation
2+
3+
store new constant VALID_CONSTANT as "This is a constant"
4+
store invalid_variable as "This is not a constant"
5+
6+
export constant VALID_CONSTANT # This should work
7+
8+
# This should fail at runtime - trying to export a non-constant as constant
9+
export constant invalid_variable

test_fix_verification.wfl

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Test to verify the fixes for module system issues
2+
3+
# Test 1: Return statement in included files (should now work)
4+
define action called utility_function with parameters:
5+
display "This function uses return"
6+
return 42
7+
end action
8+
9+
# Test 2: Constants for export testing
10+
store new constant API_VERSION as "2.0.1"
11+
12+
# Test 3: Variable for export testing
13+
store api_endpoint as "https://api.example.com"
14+
15+
# Test 4: Container for export testing
16+
create container TestItem:
17+
property name: Text
18+
end
19+
20+
# Test 5: Export statements
21+
export action utility_function
22+
export constant API_VERSION
23+
export container TestItem
24+
25+
display "Test file loaded successfully with return handling"
26+
27+
# Use a return statement at the top level to test return handling
28+
check if API_VERSION is equal to "2.0.1":
29+
display "Version check passed"
30+
return 0
31+
otherwise:
32+
display "Version check failed"
33+
end check
34+
35+
display "This should not be displayed due to return"

test_main_fix.wfl

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# Main test file to verify include statements work with return
2+
3+
display "Before include"
4+
5+
include from "test_fix_verification.wfl"
6+
7+
display "After include - this should be displayed despite return in included file"
8+
9+
# Variables from included file should be available at runtime
10+
# display "API_VERSION from included file: " + API_VERSION
11+
12+
display "All tests passed!"

0 commit comments

Comments
 (0)