Skip to content

Commit 88e3d3f

Browse files
committed
refactor: remove wrapper types for named basic types
Signed-off-by: Christian Stewart <[email protected]>
1 parent a99a3bf commit 88e3d3f

File tree

15 files changed

+764
-46
lines changed

15 files changed

+764
-46
lines changed

compiler/analysis.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,10 @@ type Analysis struct {
115115

116116
// PackageMetadata holds package-level metadata
117117
PackageMetadata map[string]interface{}
118+
119+
// WrapperTypes tracks types that should be implemented as wrapper classes
120+
// This includes both local types with methods and imported types that are known wrapper types
121+
WrapperTypes map[types.Type]bool
118122
}
119123

120124
// PackageAnalysis holds cross-file analysis data for a package
@@ -147,6 +151,7 @@ func NewAnalysis() *Analysis {
147151
ReflectedFunctions: make(map[ast.Node]*ReflectedFunctionInfo),
148152
FunctionAssignments: make(map[types.Object]ast.Node),
149153
PackageMetadata: make(map[string]interface{}),
154+
WrapperTypes: make(map[types.Type]bool),
150155
}
151156
}
152157

@@ -1581,3 +1586,91 @@ func (v *analysisVisitor) findVariableUsageInExpr(expr ast.Expr, lhsVarNames map
15811586
// For now, we'll ignore them as they're less common in shadowing scenarios
15821587
}
15831588
}
1589+
1590+
// IsWrapperType returns whether the given type should be implemented as a wrapper class
1591+
// This includes both local types with methods and imported types that are known wrapper types
1592+
func (a *Analysis) IsWrapperType(t types.Type) bool {
1593+
if t == nil {
1594+
return false
1595+
}
1596+
1597+
// Check if we've already determined this type is a wrapper type
1598+
if isWrapper, exists := a.WrapperTypes[t]; exists {
1599+
return isWrapper
1600+
}
1601+
1602+
// For named types, check if they have methods
1603+
if namedType, ok := t.(*types.Named); ok {
1604+
// Exclude struct types - they should remain as classes, not wrapper types
1605+
if _, isStruct := namedType.Underlying().(*types.Struct); isStruct {
1606+
a.WrapperTypes[t] = false
1607+
return false
1608+
}
1609+
1610+
// Exclude interface types
1611+
if _, isInterface := namedType.Underlying().(*types.Interface); isInterface {
1612+
a.WrapperTypes[t] = false
1613+
return false
1614+
}
1615+
1616+
// Check for common wrapper type patterns from standard library FIRST
1617+
// This is important for imported types that we can't analyze methods for
1618+
if obj := namedType.Obj(); obj != nil && obj.Pkg() != nil {
1619+
pkgPath := obj.Pkg().Path()
1620+
typeName := obj.Name()
1621+
1622+
// Known wrapper types from standard library
1623+
isKnownWrapper := false
1624+
switch pkgPath {
1625+
case "os":
1626+
isKnownWrapper = typeName == "FileMode"
1627+
case "time":
1628+
isKnownWrapper = typeName == "Duration"
1629+
case "syscall":
1630+
// Many syscall types are wrappers
1631+
isKnownWrapper = true
1632+
}
1633+
1634+
if isKnownWrapper {
1635+
a.WrapperTypes[t] = true
1636+
return true
1637+
}
1638+
}
1639+
1640+
// Check if this type has methods defined on it (for local types)
1641+
if namedType.NumMethods() > 0 {
1642+
a.WrapperTypes[t] = true
1643+
return true
1644+
}
1645+
}
1646+
1647+
// Handle type aliases (Go 1.9+)
1648+
if aliasType, ok := t.(*types.Alias); ok {
1649+
// Get the underlying type and check if it's a known wrapper type
1650+
if obj := aliasType.Obj(); obj != nil && obj.Pkg() != nil {
1651+
pkgPath := obj.Pkg().Path()
1652+
typeName := obj.Name()
1653+
1654+
// Known wrapper types from standard library
1655+
isKnownWrapper := false
1656+
switch pkgPath {
1657+
case "os":
1658+
isKnownWrapper = typeName == "FileMode"
1659+
case "time":
1660+
isKnownWrapper = typeName == "Duration"
1661+
case "syscall":
1662+
// Many syscall types are wrappers
1663+
isKnownWrapper = true
1664+
}
1665+
1666+
if isKnownWrapper {
1667+
a.WrapperTypes[t] = true
1668+
return true
1669+
}
1670+
}
1671+
}
1672+
1673+
// Cache negative result
1674+
a.WrapperTypes[t] = false
1675+
return false
1676+
}

compiler/analysis_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"go/parser"
66
"go/token"
77
"go/types"
8+
"os"
89
"testing"
910

1011
"golang.org/x/tools/go/packages"
@@ -232,3 +233,81 @@ func main() {
232233
t.Log("")
233234
}
234235
}
236+
237+
// TestWrapperTypeDetection verifies that the analysis correctly identifies wrapper types
238+
func TestWrapperTypeDetection(t *testing.T) {
239+
// Use the actual compliance test case
240+
testPath := "../compliance/tests/wrapper_type_args"
241+
242+
// Load the package using the packages config like the main compiler does
243+
cfg := &packages.Config{
244+
Mode: packages.NeedName |
245+
packages.NeedFiles |
246+
packages.NeedCompiledGoFiles |
247+
packages.NeedImports |
248+
packages.NeedDeps |
249+
packages.NeedExportFile |
250+
packages.NeedTypes |
251+
packages.NeedSyntax |
252+
packages.NeedTypesInfo |
253+
packages.NeedTypesSizes,
254+
Tests: false,
255+
Env: append(os.Environ(), "GOOS=js", "GOARCH=wasm"),
256+
}
257+
258+
pkgs, err := packages.Load(cfg, testPath)
259+
if err != nil {
260+
t.Fatalf("Failed to load test package: %v", err)
261+
}
262+
263+
if len(pkgs) != 1 {
264+
t.Fatalf("Expected 1 package, got %d", len(pkgs))
265+
}
266+
267+
pkg := pkgs[0]
268+
if len(pkg.Errors) > 0 {
269+
t.Fatalf("Package has errors: %v", pkg.Errors[0])
270+
}
271+
272+
// Run analysis on the first file
273+
if len(pkg.Syntax) == 0 {
274+
t.Fatal("No syntax files found")
275+
}
276+
277+
analysis := NewAnalysis()
278+
cmap := ast.NewCommentMap(pkg.Fset, pkg.Syntax[0], pkg.Syntax[0].Comments)
279+
AnalyzeFile(pkg.Syntax[0], pkg, analysis, cmap)
280+
281+
// Verify the WrapperTypes map was initialized
282+
if analysis.WrapperTypes == nil {
283+
t.Error("WrapperTypes map was not initialized")
284+
}
285+
286+
// Test some type lookups to verify wrapper type detection works
287+
// We'll check if MyMode (which has methods) is detected as a wrapper type
288+
scope := pkg.Types.Scope()
289+
290+
// Check if MyMode is detected as a wrapper type
291+
if obj := scope.Lookup("MyMode"); obj != nil {
292+
if typeName, ok := obj.(*types.TypeName); ok {
293+
isWrapper := analysis.IsWrapperType(typeName.Type())
294+
if !isWrapper {
295+
t.Errorf("MyMode should be detected as wrapper type, got %v", isWrapper)
296+
}
297+
t.Logf("MyMode wrapper detection: %v (correct)", isWrapper)
298+
}
299+
}
300+
301+
// Test that regular struct types are not detected as wrapper types
302+
if obj := scope.Lookup("MyDir"); obj != nil {
303+
if typeName, ok := obj.(*types.TypeName); ok {
304+
isWrapper := analysis.IsWrapperType(typeName.Type())
305+
if isWrapper {
306+
t.Errorf("MyDir should not be detected as wrapper type, got %v", isWrapper)
307+
}
308+
t.Logf("MyDir wrapper detection: %v (correct)", isWrapper)
309+
}
310+
}
311+
312+
t.Logf("Analysis completed successfully with %d wrapper types tracked", len(analysis.WrapperTypes))
313+
}

compiler/expr-call-type-conversion.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,17 @@ func (c *GoToTSCompiler) writeTypeConversion(exp *ast.CallExpr, funIdent *ast.Id
256256
c.tsw.WriteLiterally("new ")
257257
c.tsw.WriteLiterally(funIdent.String())
258258
c.tsw.WriteLiterally("(")
259-
if err := c.WriteValueExpr(exp.Args[0]); err != nil {
259+
260+
// Use auto-wrapping for the constructor argument
261+
// The constructor parameter type is the underlying type of the named type
262+
// For MyMode (which is type MyMode os.FileMode), the constructor expects os.FileMode
263+
constructorParamType := typeName.Type()
264+
if namedType, ok := typeName.Type().(*types.Named); ok {
265+
// For named types, the constructor expects the underlying type
266+
constructorParamType = namedType.Underlying()
267+
}
268+
269+
if err := c.writeAutoWrappedArgument(exp.Args[0], constructorParamType); err != nil {
260270
return true, fmt.Errorf("failed to write argument for type constructor: %w", err)
261271
}
262272
c.tsw.WriteLiterally(")")

0 commit comments

Comments
 (0)