Skip to content

Commit 8f30654

Browse files
antonsyndantonsynd
committed
tests: Fix some
1 parent 502b266 commit 8f30654

18 files changed

+1857
-124
lines changed

‎PLAN-0.1.10.CG3.md‎

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,217 @@
1+
# Implementation Plan: Fix Nested Package Namespace Generation (Task 0.1.10.CG3)
2+
3+
## Problem Analysis
4+
5+
### Current Behavior
6+
For a file at `level1/level2/level3/__init__.spy`:
7+
- **Current namespace**: `TestProject.Level1.Level2.Level3.Level3` (duplicated segment)
8+
- **Expected namespace**: `TestProject.Level1.Level2.Level3.Init`
9+
10+
The import alias also has issues:
11+
- **Current**: `using level1_level2_level3 = ...` (incomplete/incorrect)
12+
- **Expected**: Properly formed alias pointing to correct namespace
13+
14+
### Root Cause
15+
In `RoslynEmitter.cs:203-230`, the `GenerateProjectNamespace()` method:
16+
17+
1. Adds directory parts to namespace (correct): `Level1.Level2.Level3`
18+
2. Adds filename as final component (problematic): For `__init__.spy`, `Path.GetFileNameWithoutExtension()` returns `__init__`, then `SimpleToPascalCase("__init__")` tries to convert it
19+
20+
The issue is that:
21+
- `__init__` should become `Init` (not be skipped, and not cause duplication)
22+
- The directory name `level3` and `__init__` are being conflated somehow
23+
24+
Looking at the code more carefully:
25+
26+
```csharp
27+
// Line 218-220: Add directory parts (Level1, Level2, Level3)
28+
var dirParts = relativeDir.Split(...)
29+
.Select(p => SimpleToPascalCase(p));
30+
namespaceParts.AddRange(dirParts);
31+
32+
// Line 224-227: Add filename (__init__ -> Init or Level3?)
33+
if (!string.IsNullOrEmpty(fileName))
34+
{
35+
namespaceParts.Add(SimpleToPascalCase(fileName));
36+
}
37+
```
38+
39+
For `level1/level2/level3/__init__.spy`:
40+
- `relativePath` = `level1/level2/level3/__init__.spy`
41+
- `relativeDir` = `level1/level2/level3`
42+
- `fileName` = `__init__`
43+
44+
So the namespace becomes: `TestProject.Level1.Level2.Level3.__init__` where `SimpleToPascalCase("__init__")` is called.
45+
46+
The `SimpleToPascalCase` function (lines 438-474) splits on underscores, so `__init__` becomes `Init` (after splitting and capitalizing).
47+
48+
**Wait - the bug description says it produces `Level3.Level3`, not `Level3.Init`**. Let me re-examine the actual bug.
49+
50+
If the result is `Level3.Level3`, the directory is being included twice. This could happen if:
51+
1. The file is being treated as if it's named `level3.spy` instead of `__init__.spy`
52+
2. Or there's special handling for `__init__` that's incorrectly using the parent directory name
53+
54+
## Implementation Approach
55+
56+
### Step 1: Understand the actual bug flow
57+
Create a test that reproduces the exact issue to understand the data flow.
58+
59+
### Step 2: Fix `GenerateProjectNamespace()` in `RoslynEmitter.cs`
60+
61+
Location: `src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs:203-230`
62+
63+
The fix should:
64+
1. Detect when the file is `__init__.spy`
65+
2. For `__init__.spy` files, the namespace should be:
66+
- The project namespace + directory path + `Init`
67+
- NOT directory path + directory name (which causes duplication)
68+
69+
**Proposed change:**
70+
71+
```csharp
72+
private NameSyntax GenerateProjectNamespace()
73+
{
74+
// Start with project root namespace
75+
var namespaceParts = new List<string> { _context.ProjectNamespace! };
76+
77+
// Get relative path from project src directory to source file
78+
var relativePath = Path.GetRelativePath(_context.ProjectRootPath!, _context.SourceFilePath!);
79+
80+
// Extract directory path (without filename)
81+
var relativeDir = Path.GetDirectoryName(relativePath) ?? "";
82+
var fileName = Path.GetFileNameWithoutExtension(_context.SourceFilePath);
83+
84+
// Add directory parts to namespace (if not at root)
85+
if (!string.IsNullOrEmpty(relativeDir) && relativeDir != ".")
86+
{
87+
var dirParts = relativeDir.Split(new[] { '/', '\\' }, StringSplitOptions.RemoveEmptyEntries)
88+
.Select(p => SimpleToPascalCase(p));
89+
namespaceParts.AddRange(dirParts);
90+
}
91+
92+
// Add file name as final namespace component
93+
// Special case: __init__.spy should become "Init", not cause duplication
94+
if (!string.IsNullOrEmpty(fileName))
95+
{
96+
var pascalFileName = SimpleToPascalCase(fileName);
97+
// __init__ converts to "Init" after SimpleToPascalCase strips underscores
98+
// This is the correct behavior
99+
namespaceParts.Add(pascalFileName);
100+
}
101+
102+
return ParseName(string.Join(".", namespaceParts));
103+
}
104+
```
105+
106+
Actually, looking at `SimpleToPascalCase`:
107+
- Input: `__init__`
108+
- After sanitizing: `__init__`
109+
- After splitting on `_`: `["init"]` (empty parts removed)
110+
- After capitalizing: `Init`
111+
112+
So the current code SHOULD produce `Init`. The bug must be elsewhere.
113+
114+
### Step 3: Investigate potential issue in `SimpleToPascalCase`
115+
116+
Looking at line 469: `var parts = sanitized.ToString().Split('_', StringSplitOptions.RemoveEmptyEntries);`
117+
118+
For `__init__`:
119+
- sanitized = `__init__`
120+
- parts = `["init"]` (leading/trailing underscores create empty parts that are removed)
121+
- Result should be `Init`
122+
123+
This seems correct. The bug might be:
124+
1. In how `_context.SourceFilePath` is set
125+
2. In project compilation code that sets up the context
126+
3. A different code path being taken
127+
128+
### Step 4: Check `ProjectCompiler.cs`
129+
130+
The task mentions `src/Sharpy.Compiler/ProjectCompiler.cs` but this file doesn't exist at that path. The actual file is:
131+
`src/Sharpy.Compiler/Project/ProjectCompiler.cs`
132+
133+
Need to check how it sets up `CodeGenContext` for `__init__.spy` files.
134+
135+
### Step 5: Fix import alias generation
136+
137+
Related to lines 271-350 in `RoslynEmitter.cs` (`GenerateImportUsings`).
138+
139+
For nested package imports like `import level1.level2.level3`:
140+
- The alias should be `level1_level2_level3`
141+
- The target should be `TestProject.Level1.Level2.Level3.Init.Exports`
142+
143+
## Key Files to Modify
144+
145+
1. **`src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs`** (primary)
146+
- Lines 203-230: `GenerateProjectNamespace()` - fix namespace generation for `__init__.spy`
147+
- Lines 271-350: `GenerateImportUsings()` - fix import alias generation for nested packages
148+
149+
2. **`src/Sharpy.Compiler/Project/ProjectCompiler.cs`** (if needed)
150+
- Check how `CodeGenContext` is configured for package init files
151+
152+
## Tests to Verify
153+
154+
### New Tests Needed
155+
156+
1. **Namespace generation for `__init__.spy`**:
157+
```csharp
158+
[Fact]
159+
public void GenerateProjectNamespace_NestedPackageInit_ProducesCorrectNamespace()
160+
{
161+
// Setup: project root at /project with file at /project/src/level1/level2/__init__.spy
162+
// Expected: TestProject.Level1.Level2.Init
163+
}
164+
```
165+
166+
2. **Deeply nested `__init__.spy`**:
167+
```csharp
168+
[Fact]
169+
public void GenerateProjectNamespace_DeeplyNestedPackageInit_NoDuplication()
170+
{
171+
// Setup: /project/src/level1/level2/level3/__init__.spy
172+
// Expected: TestProject.Level1.Level2.Level3.Init
173+
// NOT: TestProject.Level1.Level2.Level3.Level3
174+
}
175+
```
176+
177+
3. **Import alias for nested packages**:
178+
```csharp
179+
[Fact]
180+
public void GenerateImportUsings_NestedPackageImport_CorrectAlias()
181+
{
182+
// import level1.level2.level3
183+
// Expected alias: level1_level2_level3
184+
// Expected target: TestProject.Level1.Level2.Level3.Init.Exports
185+
}
186+
```
187+
188+
### Existing Tests to Run
189+
190+
- `RoslynEmitterIntegrationTests.cs` - ensure no regressions
191+
- `ProjectCompilationTests.cs` - verify package imports still work
192+
- Full test suite with `dotnet test`
193+
194+
## Potential Risks
195+
196+
1. **Breaking existing package imports**: Changes to namespace generation could break imports that rely on current (buggy) behavior
197+
2. **Class name collision with "Init"**: If a package has both `__init__.spy` and `init.spy`, there could be a collision
198+
3. **Build path differences**: Windows vs Unix path separators in test environments
199+
200+
## Questions to Clarify
201+
202+
1. **Should `__init__.spy` always become `Init` class?** Or should it use the package directory name?
203+
- Python convention: `__init__.py` makes the directory a package
204+
- Current Sharpy convention seems to use filename for class name
205+
206+
2. **What about the `Exports` class?** Is there an `Exports` class generated for `__init__.spy` files like for regular modules?
207+
208+
3. **Import alias completeness**: The task description truncates the current alias output. What is the complete expected form?
209+
210+
## Implementation Order
211+
212+
1. Write failing test that reproduces the exact bug
213+
2. Debug to find exact cause of duplication
214+
3. Fix `GenerateProjectNamespace()`
215+
4. Fix `GenerateImportUsings()` if needed
216+
5. Run full test suite
217+
6. Add comprehensive tests for edge cases

‎PLAN-0.1.10.CG4.md‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
# Implementation Plan: Fix `__init__.spy` Class Name for Packages (Task 0.1.10.CG4)
2+
3+
## Problem Analysis
4+
5+
### Current Behavior
6+
For a package `__init__.spy` file (e.g., `mypackage/__init__.spy`):
7+
- **Current class name**: `Init` (from converting `__init__` → `Init`)
8+
- **Import expects**: `Exports`
9+
10+
Example of the mismatch:
11+
```csharp
12+
// Generated C# for mypackage/__init__.spy
13+
namespace TestProject.Mypackage {
14+
public static class Init { ... } // ← BUG: Class named "Init"
15+
}
16+
17+
// Import system generates
18+
using mypackage = TestProject.Mypackage.Exports; // ← Expects "Exports"
19+
```
20+
21+
### Root Cause
22+
The `GetModuleClassName()` method (lines 717-729) already returns `"Exports"` for non-entry-point modules. However, looking at the existing code, it seems the issue is elsewhere - possibly in the previous CG3 task that may have changed behavior.
23+
24+
Wait - re-reading the task description and examining the code more carefully:
25+
26+
The current `GetModuleClassName()` implementation at line 728 **already returns `"Exports"`**:
27+
```csharp
28+
private string GetModuleClassName(bool willGenerateMainMethod = false, HashSet<string>? functionNames = null)
29+
{
30+
// All modules use "Exports" as the class name
31+
if (willGenerateMainMethod)
32+
{
33+
return "Program";
34+
}
35+
return "Exports";
36+
}
37+
```
38+
39+
This means either:
40+
1. The bug was already fixed in a previous task (CG3), or
41+
2. The task description's understanding is incorrect, or
42+
3. There's a different code path generating the `Init` class name
43+
44+
Let me trace where `Init` could be coming from:
45+
- `SimpleToPascalCase("__init__")` → `Init` (splitting on underscores)
46+
- But this is for namespace generation, not class name
47+
48+
**After further analysis**: The task description may be referring to an **older state** of the code. The current `GetModuleClassName()` already returns `"Exports"`. However, the task is assigned and we should verify this works correctly.
49+
50+
## Verification Approach
51+
52+
Since the code appears to already return `"Exports"`, the plan is:
53+
54+
### Step 1: Verify Current Behavior
55+
Write a test that specifically checks `__init__.spy` files generate a class named `Exports`.
56+
57+
### Step 2: Check for Edge Cases
58+
Ensure the fix works for:
59+
- Root-level `__init__.spy`
60+
- Nested package `__init__.spy` (e.g., `level1/level2/__init__.spy`)
61+
- Deeply nested packages
62+
63+
### Step 3: Verify Import Resolution
64+
Ensure imports can find the `Exports` class in `__init__.spy` compiled modules.
65+
66+
## Key File
67+
68+
**`src/Sharpy.Compiler/CodeGen/RoslynEmitter.cs`**
69+
- Lines 717-729: `GetModuleClassName()` - Already returns `"Exports"` for non-entry-point modules
70+
71+
## Implementation Steps
72+
73+
### Step 1: Add Unit Test for Class Name Generation
74+
75+
Add to `RoslynEmitterNamespaceTests.cs` (or create `RoslynEmitterClassNameTests.cs`):
76+
77+
```csharp
78+
[Fact]
79+
public void GetModuleClassName_InitFile_ReturnsExports()
80+
{
81+
// Arrange - Create emitter for __init__.spy file
82+
var emitter = CreateEmitterWithProjectContext(
83+
projectNamespace: "TestProject",
84+
projectRootPath: "/project/src",
85+
sourceFilePath: "/project/src/mypackage/__init__.spy"
86+
);
87+
88+
// Act - Generate a module (which uses GetModuleClassName internally)
89+
// Need to verify the class name is "Exports"
90+
91+
// Assert
92+
// Verify generated C# has "public static class Exports"
93+
}
94+
```
95+
96+
### Step 2: Add Integration Test
97+
98+
Add to `Phase0110IntegrationTests.cs`:
99+
100+
```csharp
101+
[Fact]
102+
public void PackageInit_GeneratesExportsClass_NotInitClass()
103+
{
104+
var helper = CreateHelper();
105+
106+
helper.AddPackage("mypackage", @"
107+
def my_func() -> int:
108+
return 42
109+
");
110+
111+
helper.AddSourceFile("main.spy", @"
112+
import mypackage
113+
result = mypackage.my_func()
114+
");
115+
116+
helper.WithEntryPoint("main.spy");
117+
var result = helper.Compile();
118+
119+
Assert.True(result.Success);
120+
121+
// Verify the generated C# for mypackage/__init__.spy contains "Exports" class
122+
var initFile = result.GeneratedCSharpFiles
123+
.FirstOrDefault(f => f.Key.Contains("__init__"));
124+
Assert.Contains("public static class Exports", initFile.Value);
125+
Assert.DoesNotContain("public static class Init", initFile.Value);
126+
}
127+
```
128+
129+
### Step 3: Verify No Regression
130+
131+
Run the full test suite to ensure all existing package import tests pass.
132+
133+
## Tests to Verify
134+
135+
### New Tests
136+
1. `GetModuleClassName_InitFile_ReturnsExports` - Unit test for class name
137+
2. `PackageInit_GeneratesExportsClass_NotInitClass` - Integration test verifying generated C#
138+
139+
### Existing Tests to Run
140+
```bash
141+
dotnet test --filter "FullyQualifiedName~Package"
142+
dotnet test --filter "FullyQualifiedName~RoslynEmitter"
143+
dotnet test # Full suite
144+
```
145+
146+
## Potential Risks
147+
148+
1. **Already Fixed**: The code appears to already return `"Exports"`. If tests pass, this task may be complete.
149+
2. **Other Code Paths**: There might be an alternative code path that generates `Init` that we haven't found.
150+
3. **Test Coverage Gap**: Existing tests may not explicitly verify the class name.
151+
152+
## Questions/Clarifications
153+
154+
1. **Is this task still needed?** The current `GetModuleClassName()` returns `"Exports"`. Need to verify with tests.
155+
2. **Was this fixed in CG3?** The previous task (CG3) dealt with namespace generation, which may have inadvertently affected this.
156+
157+
## Implementation Order
158+
159+
1. ✅ Read and understand current `GetModuleClassName()` implementation
160+
2. Write unit test to verify `__init__.spy` generates `Exports` class
161+
3. Write integration test to verify package imports work
162+
4. Run existing tests to verify no regressions
163+
5. If tests fail, implement the fix (though code appears correct)
164+
6. Mark task complete if all tests pass
165+
166+
## Expected Result
167+
168+
After verification/implementation:
169+
- `mypackage/__init__.spy` generates: `namespace TestProject.Mypackage { public static class Exports { ... } }`
170+
- `import mypackage` generates: `using mypackage = TestProject.Mypackage.Exports;`
171+
- Both align correctly.

0 commit comments

Comments
 (0)