|
1 | | -# Implementation Summary: Task 0.1.10.7 - Import Code Generation |
| 1 | +# Implementation Plan: Task 0.1.10.CG7 - Update Integration Tests |
2 | 2 |
|
3 | | -## Overview |
| 3 | +## Summary |
4 | 4 |
|
5 | | -**Task ID:** 0.1.10.7 |
6 | | -**Title:** Implement Import Code Generation |
7 | | -**Status:** ✅ **COMPLETED** |
8 | | -**Objective:** Generate C# `using` statements from Sharpy imports with proper handling of both Sharpy modules and .NET framework namespaces. |
| 5 | +**Task:** Fix Phase 0.1.10 integration tests to pass after code generation updates |
| 6 | +**Current Status:** 29 failing, 4 passing, 1 skipped (34 total tests) |
| 7 | +**Root Causes:** Multiple code generation issues in `RoslynEmitter.cs` related to module imports and namespace resolution |
9 | 8 |
|
10 | 9 | --- |
11 | 10 |
|
12 | | -## What Was Implemented |
| 11 | +## Step-by-Step Implementation Approach |
13 | 12 |
|
14 | | -### 1. Updated `GenerateFromImportUsings` Method |
| 13 | +### Phase 1: Fix Using Directive Namespace Resolution (Highest Priority) |
| 14 | +**Problem:** `import utils` generates `using utils = Utils.Exports;` but should be `using utils = TestProject.Utils.Utils;` |
15 | 15 |
|
16 | | -**File:** `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` (Lines 273-292) |
| 16 | +The using directive for regular imports (not `from X import Y`) is missing the project namespace prefix and using the old `Exports` class pattern instead of the module class name pattern. |
17 | 17 |
|
18 | | -**Changes:** |
19 | | -- Added logic to distinguish between .NET framework namespaces and Sharpy modules |
20 | | -- For .NET framework: Generate standard `using` directive (e.g., `using System.IO;`) |
21 | | -- For Sharpy modules: Generate `using static` for the module's `Exports` class (e.g., `using static Utils.Helpers.Exports;`) |
| 18 | +**File:** `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` |
| 19 | +**Location:** `GenerateImportUsings()` method (line ~245-281) |
22 | 20 |
|
23 | | -**Example:** |
| 21 | +**Changes Required:** |
| 22 | +1. Update the non-aliased Sharpy module import to include project namespace prefix |
| 23 | +2. Change from `.Exports` to `.<ModuleClassName>` pattern (matching `GenerateFromImportUsings`) |
| 24 | + |
| 25 | +**Before:** |
24 | 26 | ```csharp |
25 | | -// from system.io import File |
26 | | -using System.IO; |
| 27 | +yield return UsingDirective( |
| 28 | + NameEquals(sanitizedAlias), |
| 29 | + ParseName($"{namespaceName}.Exports")); |
| 30 | +``` |
27 | 31 |
|
28 | | -// from utils.helpers import format_text |
29 | | -using static Utils.Helpers.Exports; |
| 32 | +**After:** |
| 33 | +```csharp |
| 34 | +// Build: ProjectNamespace.ModuleNamespace.ModuleClassName |
| 35 | +var moduleClassName = /* extract last part of namespaceName */; |
| 36 | +var fullPath = string.IsNullOrEmpty(_context.ProjectNamespace) |
| 37 | + ? $"{namespaceName}.{moduleClassName}" |
| 38 | + : $"{_context.ProjectNamespace}.{namespaceName}.{moduleClassName}"; |
| 39 | +yield return UsingDirective( |
| 40 | + NameEquals(sanitizedAlias), |
| 41 | + ParseName(fullPath)); |
30 | 42 | ``` |
31 | 43 |
|
32 | | ---- |
| 44 | +**Tests Fixed:** BasicImport_SimpleModule_Works, BasicImport_ImportFromSubdirectory_Works, BasicImport_MultipleImports_Works, and ~15 more |
33 | 45 |
|
34 | | -### 2. Updated `GenerateImportUsings` Method |
| 46 | +--- |
35 | 47 |
|
36 | | -**File:** `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` (Lines 245-271) |
| 48 | +### Phase 2: Fix Empty Module Code Generation |
| 49 | +**Problem:** Empty modules or modules with only comments don't generate a class, causing "namespace not found" errors. |
37 | 50 |
|
38 | | -**Changes:** |
39 | | -- Added logic to distinguish between .NET framework namespaces and Sharpy modules |
40 | | -- For .NET framework with alias: `using alias = Namespace;` |
41 | | -- For .NET framework without alias: `using Namespace;` |
42 | | -- For Sharpy modules with alias: `using alias = Module.Exports;` |
43 | | -- For Sharpy modules without alias: `using module_name = Module.Exports;` |
| 51 | +**File:** `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` |
| 52 | +**Location:** `GenerateModuleClass()` method (line ~419-529) |
44 | 53 |
|
45 | | -**Example:** |
46 | | -```csharp |
47 | | -// import system.io |
48 | | -using System.IO; |
| 54 | +**Changes Required:** |
| 55 | +1. Always generate a module class, even if empty |
| 56 | +2. Currently the class is generated, but the issue is that an empty class with no members triggers other issues |
49 | 57 |
|
50 | | -// import system.io as io |
51 | | -using io = System.IO; |
| 58 | +**Investigation Needed:** Check if the class is being generated for empty modules - the error suggests it's not being found by the importing module. |
52 | 59 |
|
53 | | -// import utils.helpers |
54 | | -using utils_helpers = Utils.Helpers.Exports; |
55 | | - |
56 | | -// import utils.helpers as h |
57 | | -using h = Utils.Helpers.Exports; |
58 | | -``` |
| 60 | +**Tests Fixed:** EdgeCase_EmptyModule_CompilesSuccessfully, EdgeCase_ModuleWithOnlyComments_CompilesSuccessfully |
59 | 61 |
|
60 | 62 | --- |
61 | 63 |
|
62 | | -### 3. Added `IsNetFrameworkNamespace` Helper Method |
| 64 | +### Phase 3: Fix Entry Point Main() Generation |
| 65 | +**Problem:** Some tests fail with "Program does not contain a static 'Main' method suitable for an entry point" |
63 | 66 |
|
64 | | -**File:** `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` (Lines 286-301) |
| 67 | +**File:** `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` |
| 68 | +**Location:** `GenerateModuleClass()` method - entry point detection logic (line ~477-515) |
65 | 69 |
|
66 | | -**Purpose:** Determines if a module name refers to a .NET framework namespace (which doesn't have an `Exports` class). |
| 70 | +**Current Logic:** |
| 71 | +- Main() is generated if: no user main function AND is entry point AND has executable statements |
| 72 | +- Main() is NOT generated if the file only has declarations (like `x: int = 42`) |
67 | 73 |
|
68 | | -**Recognized .NET Prefixes:** |
69 | | -- `system` |
70 | | -- `microsoft` |
71 | | -- `windows` |
72 | | -- `xamarin` |
73 | | -- `mono` |
74 | | -- `netstandard` |
| 74 | +**Investigation Needed:** |
| 75 | +- Verify `_context.IsEntryPoint` is being set correctly |
| 76 | +- Variable declarations (`x: int = 42`) need to generate a Main() or be treated as executable |
75 | 77 |
|
76 | | ---- |
| 78 | +**Tests Fixed:** ProjectFile_CustomSourceDirectory_FindsSourceFiles |
77 | 79 |
|
78 | | -## Import Mapping Reference |
| 80 | +--- |
79 | 81 |
|
80 | | -### For .NET Framework Namespaces |
| 82 | +### Phase 4: Fix Aliased Import Using Generation |
| 83 | +**Problem:** `import config as cfg` should generate correct using directive with alias |
81 | 84 |
|
82 | | -| Sharpy Import | Generated C# | |
83 | | -|---------------|-------------| |
84 | | -| `import system.io` | `using System.IO;` | |
85 | | -| `import system.io as io` | `using io = System.IO;` | |
86 | | -| `from system.io import File` | `using System.IO;` | |
87 | | -| `from system.text import *` | `using System.Text;` | |
| 85 | +**File:** `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` |
| 86 | +**Location:** `GenerateImportUsings()` method, aliased branch (line ~254-261) |
88 | 87 |
|
89 | | -### For Sharpy Modules |
| 88 | +**Current Code:** |
| 89 | +```csharp |
| 90 | +var targetName = isNetFramework ? namespaceName : $"{namespaceName}.Exports"; |
| 91 | +``` |
90 | 92 |
|
91 | | -| Sharpy Import | Generated C# | |
92 | | -|---------------|-------------| |
93 | | -| `import utils.helpers` | `using utils_helpers = Utils.Helpers.Exports;` | |
94 | | -| `import utils.helpers as h` | `using h = Utils.Helpers.Exports;` | |
95 | | -| `from utils.helpers import format_text` | `using static Utils.Helpers.Exports;` | |
96 | | -| `from utils import *` | `using static Utils.Exports;` | |
| 93 | +**Changes Required:** Similar to Phase 1 - add project namespace and use module class name |
97 | 94 |
|
98 | 95 | --- |
99 | 96 |
|
100 | | -## Tests Added/Updated |
101 | | - |
102 | | -### Updated Existing Tests |
103 | | -All existing import tests in `RoslynEmitterModuleTests.cs` were updated to reflect the new behavior: |
| 97 | +### Phase 5: Handle Relative Imports (May Defer) |
| 98 | +**Problem:** `from .helpers import utility_func` fails with parser error - relative imports not supported |
104 | 99 |
|
105 | | -1. `GenerateCompilationUnit_WithImportStatement_GeneratesUsing` - Now expects .NET imports without `.Exports` |
106 | | -2. `GenerateCompilationUnit_WithImportAlias_GeneratesUsingAlias` - Now expects .NET imports without `.Exports` |
107 | | -3. `GenerateCompilationUnit_WithFromImport_GeneratesUsing` - Now expects .NET imports without `using static` |
108 | | -4. `GenerateCompilationUnit_WithFromImportAll_GeneratesUsing` - Now expects .NET imports normally |
109 | | -5. `GenerateCompilationUnit_WithMultipleImports_GeneratesAllUsings` - Updated assertions for .NET framework |
110 | | -6. `ConvertModuleNameToNamespace_SnakeCase_ConvertsToPascalCase` - Correctly tests Sharpy module with `.Exports` |
| 100 | +**Files:** `src/Sharpy.Compiler/Parser/Parser.cs` |
111 | 101 |
|
112 | | -### New Tests Added |
| 102 | +**Scope:** Parser doesn't recognize `.` prefix for relative imports. This is a parser feature, not code generation. |
113 | 103 |
|
114 | | -1. **`GenerateCompilationUnit_WithImportModule_GeneratesAliasToExports`** |
115 | | - - Tests: `import utils.helpers` → `using utils_helpers = Utils.Helpers.Exports;` |
| 104 | +**Recommendation:** Mark these tests as `[Fact(Skip = "...")]` for now if parser changes are complex. |
116 | 105 |
|
117 | | -2. **`GenerateCompilationUnit_WithImportModuleAsAlias_GeneratesCorrectAlias`** |
118 | | - - Tests: `import utils.helpers as h` → `using h = Utils.Helpers.Exports;` |
| 106 | +**Tests Affected:** |
| 107 | +- PackageInit_WithReExports_ExportsModuleMembers |
| 108 | +- ComplexScenario_PackageWithMultipleModulesAndReExports_Works |
| 109 | +- ComplexScenario_PackageImportingFromParentPackage_Works |
119 | 110 |
|
120 | 111 | --- |
121 | 112 |
|
122 | | -## Test Results |
| 113 | +## Key Files to Modify |
123 | 114 |
|
124 | | -✅ **All tests passing:** |
125 | | -- **RoslynEmitterModuleTests:** 30/30 tests passed |
126 | | -- **All CodeGen tests:** 378/378 tests passed |
127 | | -- **Full test suite:** 2942/2942 tests passed (81 skipped) |
| 115 | +| File | Changes | |
| 116 | +|------|---------| |
| 117 | +| `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` | Fix `GenerateImportUsings()` namespace resolution | |
| 118 | +| `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` | Ensure empty modules generate valid class | |
| 119 | +| `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` | Review entry point Main() generation | |
| 120 | +| `src/Sharpy.Compiler.Tests/Integration/Phase0110IntegrationTests.cs` | Mark relative import tests as skipped | |
128 | 121 |
|
129 | 122 | --- |
130 | 123 |
|
131 | | -## Key Design Decisions |
132 | | - |
133 | | -### 1. .NET Framework vs Sharpy Module Detection |
134 | | -- Used simple prefix matching on the first namespace component |
135 | | -- This avoids needing access to the full module resolution system during code generation |
136 | | -- Covers all common .NET framework namespaces |
137 | | - |
138 | | -### 2. Module Name Sanitization |
139 | | -- For Sharpy modules without an alias, dots are replaced with underscores |
140 | | -- Example: `utils.helpers` becomes `utils_helpers` for a valid C# identifier |
141 | | - |
142 | | -### 3. Backwards Compatibility |
143 | | -- .NET framework imports work exactly as they did before |
144 | | -- Only Sharpy module imports use the new `.Exports` pattern |
145 | | -- This ensures existing code continues to compile |
| 124 | +## Tests to Verify (Priority Order) |
| 125 | + |
| 126 | +### Must Pass (Core Import System) |
| 127 | +1. BasicImport_SimpleModule_Works |
| 128 | +2. BasicImport_ImportFromSubdirectory_Works |
| 129 | +3. BasicImport_MultipleImports_Works |
| 130 | +4. BasicImport_ImportVariable_Works |
| 131 | + |
| 132 | +### Must Pass (Multi-File) |
| 133 | +5. MultiFile_TwoFilesWithDependency_CompilesInCorrectOrder |
| 134 | +6. MultiFile_ThreeFilesChainedDependency_CompilesCorrectly |
| 135 | +7. MultiFile_SharedDependency_CompilesOnce |
| 136 | +8. MultiFile_ComplexDependencyGraph_ResolvesCorrectly |
| 137 | +9. MultiFile_FunctionCallAcrossModules_TypeChecksCorrectly |
| 138 | + |
| 139 | +### Should Pass (Packages) |
| 140 | +10. PackageInit_EmptyInit_MarksAsPackage |
| 141 | +11. PackageInit_WithVariables_DefinesPackageLevelVariables |
| 142 | +12. PackageInit_WithFunctions_DefinesPackageLevelFunctions |
| 143 | +13. PackageInit_NestedPackages_Works |
| 144 | +14. PackageInit_ImportFromPackage_Works |
| 145 | + |
| 146 | +### Should Pass (Project Files) |
| 147 | +15. ProjectFile_BasicConfiguration_CompilesSuccessfully |
| 148 | +16. ProjectFile_LibraryOutputType_CompilesWithoutEntryPoint |
| 149 | +17. ProjectFile_MultipleSourceFiles_CompilesAll |
| 150 | +18. ProjectFile_CustomSourceDirectory_FindsSourceFiles |
| 151 | + |
| 152 | +### Should Pass (Edge Cases) |
| 153 | +19. EdgeCase_EmptyModule_CompilesSuccessfully |
| 154 | +20. EdgeCase_ModuleWithOnlyComments_CompilesSuccessfully |
| 155 | +21. EdgeCase_ImportSameName_FromDifferentPackages_Works |
| 156 | +22. EdgeCase_DeepNesting_Works |
| 157 | + |
| 158 | +### Defer (Relative Imports - Parser Change Required) |
| 159 | +23. PackageInit_WithReExports_ExportsModuleMembers |
| 160 | +24. ComplexScenario_PackageWithMultipleModulesAndReExports_Works |
| 161 | +25. ComplexScenario_PackageImportingFromParentPackage_Works |
146 | 162 |
|
147 | 163 | --- |
148 | 164 |
|
149 | | -## Files Modified |
| 165 | +## Potential Risks |
150 | 166 |
|
151 | | -| File | Changes | Lines | |
152 | | -|------|---------|-------| |
153 | | -| `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs` | Updated import generation methods, added helper | 245-301 | |
154 | | -| `src/Sharpy.Compiler.Tests/CodeGen/RoslynEmitterModuleTests.cs` | Updated existing tests, added 2 new tests | Multiple | |
| 167 | +1. **Cascading Changes:** Fixing namespace resolution may reveal other issues in semantic analysis |
| 168 | +2. **Test Infrastructure:** Some failures may be test setup issues rather than compiler bugs |
| 169 | +3. **Backward Compatibility:** Changes to code generation could break existing compilations |
| 170 | +4. **Parser Complexity:** Relative imports require parser changes that may be out of scope |
155 | 171 |
|
156 | 172 | --- |
157 | 173 |
|
158 | | -## Example Usage |
159 | | - |
160 | | -### Complete Sharpy Example |
161 | | - |
162 | | -```python |
163 | | -# Import .NET framework namespace |
164 | | -import system.io |
165 | | -import system.collections.generic as Collections |
166 | | - |
167 | | -# Import Sharpy modules |
168 | | -import utils.helpers |
169 | | -import utils.formatters as fmt |
| 174 | +## Questions to Clarify |
170 | 175 |
|
171 | | -# From imports |
172 | | -from system.text import StringBuilder |
173 | | -from utils.validators import is_valid, check_format |
| 176 | +1. **Relative Imports:** Should `from .module import X` be implemented in this task or deferred? |
| 177 | + - Recommendation: Defer to separate task, skip affected tests |
174 | 178 |
|
175 | | -# Usage |
176 | | -file = system.io.File.ReadAllText("test.txt") |
177 | | -result = utils_helpers.FormatText(file) |
178 | | -validated = is_valid(result) |
179 | | -``` |
180 | | - |
181 | | -### Generated C# Code |
| 179 | +2. **Empty Modules:** Should `import empty_module` be valid when module has no exports? |
| 180 | + - Current tests expect this to work |
182 | 181 |
|
183 | | -```csharp |
184 | | -#nullable enable |
185 | | - |
186 | | -using System; |
187 | | -using System.Collections.Generic; |
188 | | -using System.Linq; |
189 | | -using global::Sharpy.Core; |
190 | | -using System.IO; |
191 | | -using Collections = System.Collections.Generic; |
192 | | -using utils_helpers = Utils.Helpers.Exports; |
193 | | -using fmt = Utils.Formatters.Exports; |
194 | | -using System.Text; |
195 | | -using static Utils.Validators.Exports; |
196 | | - |
197 | | -namespace Sharpy.MyModule |
198 | | -{ |
199 | | - public static class Exports |
200 | | - { |
201 | | - // Module code here... |
202 | | - } |
203 | | -} |
204 | | -``` |
| 182 | +3. **Variable Declarations as Entry Point:** Should a file with only `x: int = 42` generate a Main()? |
| 183 | + - Current logic requires executable statements, not just declarations |
205 | 184 |
|
206 | 185 | --- |
207 | 186 |
|
208 | | -## Conclusion |
209 | | - |
210 | | -The import code generation system now correctly handles both: |
211 | | -1. **.NET framework namespaces** - imported normally without `.Exports` |
212 | | -2. **Sharpy modules** - imported with `.Exports` for proper access to module-level functions |
213 | | - |
214 | | -This implementation provides a clean interop between Sharpy modules and .NET framework libraries while maintaining proper C# semantics. |
| 187 | +## Execution Order |
215 | 188 |
|
216 | | -**Status:** ✅ Task 0.1.10.7 Complete |
| 189 | +1. **Phase 1 First** - Fixing using directive generation will fix the majority of tests |
| 190 | +2. **Phase 2-3** - Fix edge cases for empty modules and entry points |
| 191 | +3. **Phase 4** - Fix aliased imports |
| 192 | +4. **Phase 5** - Defer or implement relative imports based on complexity assessment |
| 193 | +5. **Run all tests** to verify improvements |
| 194 | +6. **Skip remaining relative import tests** if not implementing parser changes |
0 commit comments