Skip to content

Commit

Permalink
eliminate unused lambda params when formatting
Browse files Browse the repository at this point in the history
  • Loading branch information
nighca committed Jan 14, 2025
1 parent d4b5214 commit c3daeae
Show file tree
Hide file tree
Showing 3 changed files with 258 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export const onCloned0: DefinitionDocumentationItem = {
overloadId: '0'
},
insertText: 'onCloned data => {\n\t${1}\n}',
overview: 'onCloned data? => {}',
overview: 'onCloned data => {}',
detail: makeBasicMarkdownString({
en: 'Listen to current sprite cloned, optionally receiving data',
zh: '当前精灵被复制时执行,并可选地接收数据'
Expand All @@ -69,7 +69,7 @@ export const onTouchStart0: DefinitionDocumentationItem = {
overloadId: '0'
},
insertText: 'onTouchStart sprite => {\n\t${1}\n}',
overview: 'onTouchStart sprite? => {}',
overview: 'onTouchStart sprite => {}',
detail: makeBasicMarkdownString({
en: 'Listen to current sprite starting to be touched by any other sprites, optionally receiving the sprite',
zh: '当前精灵与其他任意精灵开始接触时执行,并可选地接收精灵信息'
Expand All @@ -95,7 +95,7 @@ export const onTouchStart2: DefinitionDocumentationItem = {
overloadId: '2'
},
insertText: 'onTouchStart ${1:name}, sprite => {\n\t${2}\n}',
overview: 'onTouchStart name, sprite? => {}',
overview: 'onTouchStart name, sprite => {}',
detail: makeBasicMarkdownString({
en: 'Listen to current sprite starting to be touched by sprite of given name, optionally receiving the sprite',
zh: '当前精灵与指定名字的精灵开始接触时执行,并可选地接收精灵信息'
Expand All @@ -121,7 +121,7 @@ export const onTouchStart4: DefinitionDocumentationItem = {
overloadId: '4'
},
insertText: 'onTouchStart [${1:}], sprite => {\n\t${2}\n}',
overview: 'onTouchStart names, sprite? => {}',
overview: 'onTouchStart names, sprite => {}',
detail: makeBasicMarkdownString({
en: 'Listen to current sprite starting to be touched by any sprite of given names, optionally receiving the sprite',
zh: '当前精灵与任一指定名字的精灵开始接触时执行,并可选地接收精灵信息'
Expand All @@ -147,7 +147,7 @@ export const onMoving0: DefinitionDocumentationItem = {
overloadId: '0'
},
insertText: 'onMoving info => {\n\t${1}\n}',
overview: 'onMoving info? => {}',
overview: 'onMoving info => {}',
detail: makeBasicMarkdownString({
en: 'Listen to current sprite moving (position change), optionally receiving the moving info',
zh: '当前精灵移动(位置改变)时执行,并可选地接收移动信息'
Expand All @@ -173,7 +173,7 @@ export const onTurning0: DefinitionDocumentationItem = {
overloadId: '0'
},
insertText: 'onTurning info => {\n\t${1}\n}',
overview: 'onTurning info? => {}',
overview: 'onTurning info => {}',
detail: makeBasicMarkdownString({
en: 'Listen to current sprite turning (heading change), optionally receiving the turning info',
zh: '当前精灵转向(朝向改变)时执行,并可选地接收转向信息'
Expand Down Expand Up @@ -1483,7 +1483,7 @@ export const gameOnKey1: DefinitionDocumentationItem = {
overloadId: '1'
},
insertText: 'onKey [${1:}], key => {\n\t${2}\n}',
overview: 'onKey keys, key? => {}',
overview: 'onKey keys, key => {}',
detail: makeBasicMarkdownString({
en: 'Listen to given keys pressed, optionally receiving the key pressed, e.g., `onKey [KeyA, KeyB], key => {}`',
zh: '指定多个按键,任一被按下时执行,并可选地接收被按下的按键,如:`onKey [KeyA, KeyB], key => {}`'
Expand Down
175 changes: 166 additions & 9 deletions tools/spxls/internal/server/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,12 @@ package server
import (
"bytes"
"fmt"
"go/types"
"io/fs"
"path"

gopast "github.com/goplus/gop/ast"
gopfmt "github.com/goplus/gop/format"
gopparser "github.com/goplus/gop/parser"
"github.com/goplus/gop/parser/fsx/memfs"
goptoken "github.com/goplus/gop/token"
)

Expand All @@ -28,7 +27,7 @@ func (s *Server) textDocumentFormatting(params *DocumentFormattingParams) ([]Tex
return nil, err
}

formatted, err := formatSpx(content)
formatted, err := formatSpx(s, spxFile)
if err != nil {
return nil, fmt.Errorf("failed to format spx source file: %w", err)
}
Expand Down Expand Up @@ -56,18 +55,19 @@ func (s *Server) textDocumentFormatting(params *DocumentFormattingParams) ([]Tex
}

// formatSpx formats a spx source file.
func formatSpx(src []byte) ([]byte, error) {
func formatSpx(s *Server, spxFileName string) ([]byte, error) {
// Parse the source into AST.
fset := goptoken.NewFileSet()
astFile, err := gopparser.ParseFSEntry(fset, memfs.SingleFile("", "main.spx", string(src)), "main.spx", nil, gopparser.Config{
Mode: gopparser.ParseComments | gopparser.ParseGoPlusClass,
})
compileResult, _ := s.compile()
astFile := compileResult.mainASTPkg.Files[spxFileName]
if astFile == nil {
// Return error only if parsing completely failed. For partial parsing
// failures, we proceed with formatting.
return nil, err
return nil, fmt.Errorf("failed to parse spx source file")
}

eliminateUnusedLambdaParams(compileResult, astFile)

fset := compileResult.fset
// Sort import statements first.
gopast.SortImports(fset, astFile)

Expand Down Expand Up @@ -136,3 +136,160 @@ func formatSpx(src []byte) ([]byte, error) {
}
return buf.Bytes(), nil
}

// eliminateUnusedLambdaParams eliminates useless lambda parameter declarations.
// A lambda parameter is considered "useless" if:
// 1. The parameter is not used.
// 2. The lambda is passed to a function that has a overload which receives the lambda without the parameter.
// Then we can omit its declaration safely.
//
// NOTE: There are limitations with current implementation:
// 1. Only `LambdaExpr2` (not `LambdaExpr`) is supported.
// 2. Only the last parameter of the lambda is checked.
// We may complete it in the future, if needed.
func eliminateUnusedLambdaParams(compileResult *compileResult, astFile *gopast.File) {
gopast.Inspect(astFile, func(n gopast.Node) bool {
callExpr, ok := n.(*gopast.CallExpr)
if !ok {
return true
}
funIdent, ok := callExpr.Fun.(*gopast.Ident)
if !ok {
return true
}
funType, funTypeOverloads := getFuncAndOverloadsType(compileResult, funIdent)
if funType == nil || funTypeOverloads == nil {
return true
}
paramsType := funType.Signature().Params()
for argIdx, argExpr := range callExpr.Args {
lambdaExpr, ok := argExpr.(*gopast.LambdaExpr2)
if !ok {
continue
}
if argIdx >= paramsType.Len() {
break
}
lambdaSig, ok := paramsType.At(argIdx).Type().(*types.Signature)
if !ok {
continue
}
if len(lambdaExpr.Lhs) == 0 {
continue
}
// To simplify the implementation, we only check & process the last parameter,
// which is enough to cover known cases.
lastParamIdx := len(lambdaExpr.Lhs) - 1
if used := isIdentUsed(compileResult, lambdaExpr.Lhs[lastParamIdx]); used {
continue
}

newParamTypes := make([]*types.Var, lambdaSig.Params().Len()-1)
for i := 0; i < lambdaSig.Params().Len()-1; i++ {
newParamTypes[i] = lambdaSig.Params().At(i)
}
newLambdaSig := types.NewSignatureType(
lambdaSig.Recv(),
getTypeParamSlice(lambdaSig.RecvTypeParams()),
getTypeParamSlice(lambdaSig.TypeParams()),
types.NewTuple(newParamTypes...),
lambdaSig.Results(),
lambdaSig.Variadic(),
)
hasMatchedOverload := false
for _, overloadType := range funTypeOverloads {
if overloadType == funType {
continue
}
overloadParamsType := overloadType.Signature().Params()
if overloadParamsType.Len() != paramsType.Len() {
continue
}
overloadLambdaSig, ok := overloadParamsType.At(argIdx).Type().(*types.Signature)
if !ok {
continue
}
if types.AssignableTo(newLambdaSig, overloadLambdaSig) {
hasMatchedOverload = true
break
}
}
if hasMatchedOverload {
lambdaExpr.Lhs = lambdaExpr.Lhs[:lastParamIdx]
if len(lambdaExpr.Lhs) == 0 {
// Avoid `index out of range [0] with length 0` when printing lambda expression.
lambdaExpr.Lhs = nil
}
}
}
return true
})
}

// getFuncAndOverloadsType returns the function type and all its overloads.
func getFuncAndOverloadsType(compileResult *compileResult, funIdent *gopast.Ident) (fun *types.Func, overloads []*types.Func) {
funTypeObj := compileResult.typeInfo.ObjectOf(funIdent)
if funTypeObj == nil {
return
}
funType, ok := funTypeObj.(*types.Func)
if !ok {
return
}
pkg := funType.Pkg()
if pkg == nil {
return
}
recvTypeName := compileResult.selectorTypeNameForIdent(funIdent)
if recvTypeName == "" {
return
}
if isSpxPkgObject(funTypeObj) && recvTypeName == "Sprite" {
recvTypeName = "SpriteImpl"
}

recvType := funType.Pkg().Scope().Lookup(recvTypeName).Type()
if recvType == nil {
return
}
recvNamed, ok := recvType.(*types.Named)
if !ok || !isNamedStructType(recvNamed) {
return
}
var underlineFunType *types.Func
walkStruct(recvNamed, func(member types.Object, selector *types.Named) bool {
method, ok := member.(*types.Func)
if !ok {
return true
}
if pn, overloadId := parseGopFuncName(method.Name()); pn == funIdent.Name && overloadId == nil {
underlineFunType = method
}
return true
})
if underlineFunType == nil {
return
}
return funType, expandGopOverloadableFunc(underlineFunType)
}

func isIdentUsed(compileResult *compileResult, ident *gopast.Ident) bool {
callbackParamTypeObj := compileResult.typeInfo.ObjectOf(ident)
for _, typeObj := range compileResult.typeInfo.Uses {
if typeObj == callbackParamTypeObj {
return true
}
}
return false
}

func getTypeParamSlice(list *types.TypeParamList) []*types.TypeParam {
if list == nil {
return nil
}
slice := make([]*types.TypeParam, list.Len())
for i := 0; i < list.Len(); i++ {
slice[i] = list.At(i)
}
return slice
}
85 changes: 85 additions & 0 deletions tools/spxls/internal/server/format_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,4 +226,89 @@ run "assets", {Title: "My Game"}
require.NoError(t, err)
require.Nil(t, edits)
})

t.Run("WithUnusedLambdaParams", func(t *testing.T) {
s := New(newMapFSWithoutModTime(map[string][]byte{
"main.spx": []byte(`// A spx game.
onKey [KeyLeft, KeyRight], (key) => {
println "key"
}
onKey [KeyLeft, KeyRight], (key) => {
println key
}
`),
}), nil)
params := &DocumentFormattingParams{
TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"},
}

edits, err := s.textDocumentFormatting(params)
require.NoError(t, err)
require.Len(t, edits, 1)
assert.Contains(t, edits, TextEdit{
Range: Range{
Start: Position{Line: 0, Character: 0},
End: Position{Line: 8, Character: 0},
},
NewText: `// A spx game.
onKey [KeyLeft, KeyRight], () => {
println "key"
}
onKey [KeyLeft, KeyRight], (key) => {
println key
}
`,
})
})

t.Run("WithUnusedLambdaParamsForSprite", func(t *testing.T) {
s := New(newMapFSWithoutModTime(map[string][]byte{
"main.spx": {},
"MySprite.spx": []byte(`// A spx game.
onKey [KeyLeft, KeyRight], (key) => {
println "key"
}
onTouchStart s => {}
onTouchStart s => {
println "touched", s
}
onTouchStart => {}
onTouchStart (s, t) => { // type mismatch
}
onTouchStart 123, (s) => { // type mismatch
}
`),
}), nil)
params := &DocumentFormattingParams{
TextDocument: TextDocumentIdentifier{URI: "file:///MySprite.spx"},
}

edits, err := s.textDocumentFormatting(params)
require.NoError(t, err)
require.Len(t, edits, 1)
assert.Contains(t, edits, TextEdit{
Range: Range{
Start: Position{Line: 0, Character: 0},
End: Position{Line: 13, Character: 0},
},
NewText: `// A spx game.
onKey [KeyLeft, KeyRight], () => {
println "key"
}
onTouchStart => {
}
onTouchStart s => {
println "touched", s
}
onTouchStart => {
}
onTouchStart (s, t) => { // type mismatch
}
onTouchStart 123, (s) => { // type mismatch
}
`,
})
})
}

0 comments on commit c3daeae

Please sign in to comment.