diff --git a/src/refactor/CacheAndTyperContext.hx b/src/refactor/CacheAndTyperContext.hx index 21e745a..c8ec85c 100644 --- a/src/refactor/CacheAndTyperContext.hx +++ b/src/refactor/CacheAndTyperContext.hx @@ -2,6 +2,7 @@ package refactor; import refactor.VerboseLogger; import refactor.discover.FileList; +import refactor.discover.FileReaderFunc; import refactor.discover.NameMap; import refactor.discover.TypeList; import refactor.typing.ITyper; @@ -12,4 +13,8 @@ typedef CacheAndTyperContext = { var typeList:TypeList; var verboseLog:VerboseLogger; var typer:Null; + var fileReader:FileReaderFunc; + var converter:ByteToCharConverterFunc; } + +typedef ByteToCharConverterFunc = (string:String, byteOffset:Int) -> Int; diff --git a/src/refactor/Cli.hx b/src/refactor/Cli.hx index ff77d07..f81b042 100644 --- a/src/refactor/Cli.hx +++ b/src/refactor/Cli.hx @@ -118,7 +118,9 @@ class Cli { forRealExecute: execute && forReal, docFactory: EditableDocument.new, verboseLog: verboseLog, - typer: null + typer: null, + fileReader: null, + converter: null, }); result.then(function(result:RefactorResult) { switch (result) { diff --git a/src/refactor/refactor/CanRefactorContext.hx b/src/refactor/refactor/CanRefactorContext.hx index 9a26ce3..ef7ff83 100644 --- a/src/refactor/refactor/CanRefactorContext.hx +++ b/src/refactor/refactor/CanRefactorContext.hx @@ -1,11 +1,5 @@ package refactor.refactor; -import refactor.discover.FileReaderFunc; - typedef CanRefactorContext = CacheAndTyperContext & { var what:RefactorWhat; - var fileReader:FileReaderFunc; - var converter:ByteToCharConverterFunc; } - -typedef ByteToCharConverterFunc = (string:String, byteOffset:Int) -> Int; diff --git a/src/refactor/refactor/ExtractMethod.hx b/src/refactor/refactor/ExtractMethod.hx index 3ca7419..0acd015 100644 --- a/src/refactor/refactor/ExtractMethod.hx +++ b/src/refactor/refactor/ExtractMethod.hx @@ -372,6 +372,8 @@ class ExtractMethod { switch (child.tok) { case Binop(OpAssign) | Binop(OpAssignOp(_)): assignedVars.push(s); + case Unop(OpIncrement) | Unop(OpDecrement): + assignedVars.push(s); default: } } @@ -382,7 +384,7 @@ class ExtractMethod { if (allReturns.length > 0) { var lastReturn = allReturns[allReturns.length - 1]; if (isSingleExpression(lastReturn, extractData.endToken)) { - return new CodeGenReturnIsLast(extractData, context, neededIdentifiers, lastReturn); + return new CodeGenReturnIsLast(extractData, context, neededIdentifiers); } } @@ -467,7 +469,11 @@ class ExtractMethod { continue; } } - scopedVarUses.push(varsValidAfterSelection.get(part)); + final scopedVar = varsValidAfterSelection.get(part); + if (scopedVarUses.contains(scopedVar)) { + continue; + } + scopedVarUses.push(scopedVar); trace("leaking " + varsValidAfterSelection.get(part)); } return scopedVarUses; diff --git a/src/refactor/refactor/RefactorHelper.hx b/src/refactor/refactor/RefactorHelper.hx index 2685734..ce8c8eb 100644 --- a/src/refactor/refactor/RefactorHelper.hx +++ b/src/refactor/refactor/RefactorHelper.hx @@ -1,7 +1,7 @@ package refactor.refactor; +import refactor.CacheAndTyperContext.ByteToCharConverterFunc; import refactor.discover.File; -import refactor.refactor.CanRefactorContext.ByteToCharConverterFunc; class RefactorHelper { public static function findTokensAtPos(root:TokenTree, searchPos:Int):TokensAtPos { diff --git a/src/refactor/refactor/extractmethod/CodeGenBase.hx b/src/refactor/refactor/extractmethod/CodeGenBase.hx index 62f28ab..639330f 100644 --- a/src/refactor/refactor/extractmethod/CodeGenBase.hx +++ b/src/refactor/refactor/extractmethod/CodeGenBase.hx @@ -37,6 +37,49 @@ abstract class CodeGenBase implements ICodeGen { } return snippet; } + + function parentTypeHint():Promise { + var func:Null = findParentFunction(); + if (func == null) { + return Promise.reject("failed to find return type of selected code"); + } + return TypingHelper.findTypeWithTyper(context, context.what.fileName, func.pos.max - 1).then(function(typeHint) { + return switch (typeHint) { + case null | ClasspathType(_) | LibType(_) | StructType(_) | UnknownType(_): + Promise.resolve(typeHint); + case FunctionType(args, retVal): + Promise.resolve(retVal); + } + }); + } + + function findParentFunction():Null { + var token = extractData.startToken.parent; + while (true) { + if (token == null) { + return null; + } + switch (token.tok) { + case Kwd(KwdFunction): + var child = token.getFirstChild(); + if (child == null) { + return null; + } + switch (child.tok) { + case Const(_): + return child; + default: + return token; + } + case Arrow: + return token; + case Root: + return null; + default: + token = token.parent; + } + } + } } typedef ReturnValueCallback = (value:String) -> String; diff --git a/src/refactor/refactor/extractmethod/CodeGenEmptyReturn.hx b/src/refactor/refactor/extractmethod/CodeGenEmptyReturn.hx index 30bb6d5..5d20b28 100644 --- a/src/refactor/refactor/extractmethod/CodeGenEmptyReturn.hx +++ b/src/refactor/refactor/extractmethod/CodeGenEmptyReturn.hx @@ -39,15 +39,8 @@ class CodeGenEmptyReturn extends CodeGenBase { + '${vars[0].name} = data;\n}\n'; case [_, _]: final dataVars = vars.map(v -> 'var ${v.name};').join("\n"); - final assignData = assignments.map(a -> '${a.name} = data.${a.name};').join("\n"); - final varsData = vars.map(v -> '${v.name} = data.${v.name};').join("\n"); - dataVars - + 'switch ($call) {\n' - + "case None:\n" - + "return;\n" - + "case Some(data):\n" - + '${assignData}${varsData}' - + "}\n"; + final assignData = assignments.concat(vars).map(a -> '${a.name} = data.${a.name};').join("\n"); + dataVars + 'switch ($call) {\n' + "case None:\n" + "return;\n" + "case Some(data):\n" + '${assignData}\n' + "}\n"; } } @@ -111,9 +104,8 @@ class CodeGenEmptyReturn extends CodeGenBase { " {\n" + snippet + returnLocalVar + "\n}\n"; case [_, _]: final snippet = replaceReturnValues(returnTokens, value -> 'None'); - final assignData = assignments.map(a -> '${a.name}: ${a.name},\n'); - final varsData = vars.map(v -> '${v.name}: ${v.name},\n'); - final returnAssigments = "\nreturn Some({\n" + assignData + varsData + "});"; + final assignData = assignments.concat(vars).map(a -> '${a.name}: ${a.name},\n'); + final returnAssigments = "\nreturn Some({\n" + assignData + "\n});"; " {\n" + snippet + returnAssigments + "\n}\n"; } } diff --git a/src/refactor/refactor/extractmethod/CodeGenNoReturn.hx b/src/refactor/refactor/extractmethod/CodeGenNoReturn.hx index 20c4752..92c1840 100644 --- a/src/refactor/refactor/extractmethod/CodeGenNoReturn.hx +++ b/src/refactor/refactor/extractmethod/CodeGenNoReturn.hx @@ -27,9 +27,8 @@ class CodeGenNoReturn extends CodeGenBase { '${assignments[0].name} = $call;\n'; case [_, _]: final dataVars = vars.map(v -> 'var ${v.name};').join("\n"); - final assignData = assignments.map(a -> '${a.name} = data.${a.name};').join("\n"); - final assignVars = vars.map(v -> '${v.name} = data.${v.name};').join("\n"); - '$dataVars\n{\nfinal data = $call;\n' + assignData + assignVars + "}\n"; + final assignData = assignments.concat(vars).map(a -> '${a.name} = data.${a.name};').join("\n"); + '$dataVars\n{\nfinal data = $call;\n' + assignData + "\n}\n"; } } @@ -88,9 +87,8 @@ class CodeGenNoReturn extends CodeGenBase { final returnAssigmentVar = '\nreturn ${assignments[0].name};'; " {\n" + selectedSnippet + returnAssigmentVar + "\n}\n"; case [_, _]: - final assignData = assignments.map(a -> '${a.name}: ${a.name},').join("\n"); - final assignVars = vars.map(v -> '${v.name}: ${v.name},').join("\n"); - final returnAssigments = "\nreturn {\n" + assignData + assignVars + "};"; + final assignData = assignments.concat(vars).map(a -> '${a.name}: ${a.name},').join("\n"); + final returnAssigments = "\nreturn {\n" + assignData + "\n};"; " {\n" + selectedSnippet + returnAssigments + "\n}\n"; } } diff --git a/src/refactor/refactor/extractmethod/CodeGenOpenEnded.hx b/src/refactor/refactor/extractmethod/CodeGenOpenEnded.hx index b762668..0213c9e 100644 --- a/src/refactor/refactor/extractmethod/CodeGenOpenEnded.hx +++ b/src/refactor/refactor/extractmethod/CodeGenOpenEnded.hx @@ -44,15 +44,14 @@ class CodeGenOpenEnded extends CodeGenBase { + "}\n"; case [_, _]: final dataVars = vars.map(v -> 'var ${v.name};').join("\n"); - final assignData = assignments.map(a -> '${a.name} = result.data.${a.name};').join("\n"); - final varsData = vars.map(v -> '${v.name} = result.data.${v.name};').join("\n"); + final assignData = assignments.concat(vars).map(a -> '${a.name} = result.data.${a.name};').join("\n"); '$dataVars' + '{\nfinal result = $call;\n' + "switch (result.ret) {\n" + "case Some(data):\n" + "return data;\n" + "case None:\n" - + '${assignData}${varsData}' + + '${assignData}\n' + "}\n" + "}\n"; } @@ -134,46 +133,6 @@ class CodeGenOpenEnded extends CodeGenBase { } } - function parentTypeHint():Promise { - var func:Null = findParentFunction(); - if (func == null) { - return Promise.reject("failed to find return type of selected code"); - } - return TypingHelper.findTypeWithTyper(context, context.what.fileName, func.pos.max - 1).then(function(typeHint) { - return switch (typeHint) { - case null | ClasspathType(_) | LibType(_) | StructType(_) | UnknownType(_): - Promise.resolve(typeHint); - case FunctionType(args, retVal): - Promise.resolve(retVal); - } - }); - } - - function findParentFunction():Null { - var token = extractData.startToken.parent; - while (true) { - switch (token.tok) { - case Kwd(KwdFunction): - var child = token.getFirstChild(); - if (child == null) { - return null; - } - switch (child.tok) { - case Const(_): - return child; - default: - return token; - } - case Arrow: - return token; - case Root: - return null; - default: - token = token.parent; - } - } - } - public function makeBody():String { return switch [assignments.length, vars.length] { case [0, 0]: @@ -189,9 +148,8 @@ class CodeGenOpenEnded extends CodeGenBase { return " {\n" + snippet + '\nreturn $returnData;\n}\n'; case [_, _]: final snippet = replaceReturnValues(returnTokens, value -> '{ret: Some($value)}'); - final assignData = assignments.map(a -> '${a.name}: ${a.name},\n'); - final varsData = vars.map(v -> '${v.name}: ${v.name},\n'); - final returnData = '{ret: None, data: {$assignData$varsData}}'; + final assignData = assignments.concat(vars).map(a -> '${a.name}: ${a.name},\n'); + final returnData = '{ret: None, data: {\n$assignData\n}}'; return " {\n" + snippet + '\nreturn $returnData;\n}\n'; } } diff --git a/src/refactor/refactor/extractmethod/CodeGenReturnIsLast.hx b/src/refactor/refactor/extractmethod/CodeGenReturnIsLast.hx index 88a2e90..65f06c1 100644 --- a/src/refactor/refactor/extractmethod/CodeGenReturnIsLast.hx +++ b/src/refactor/refactor/extractmethod/CodeGenReturnIsLast.hx @@ -3,12 +3,8 @@ package refactor.refactor.extractmethod; import refactor.discover.Identifier; class CodeGenReturnIsLast extends CodeGenBase { - final lastReturnToken:Null; - - public function new(extractData:ExtractMethodData, context:RefactorContext, neededIdentifiers:Array, lastReturnToken:TokenTree) { + public function new(extractData:ExtractMethodData, context:RefactorContext, neededIdentifiers:Array) { super(extractData, context, neededIdentifiers); - - this.lastReturnToken = lastReturnToken; } public function makeCallSite():String { @@ -19,11 +15,7 @@ class CodeGenReturnIsLast extends CodeGenBase { } public function makeReturnTypeHint():Promise { - if (lastReturnToken == null) { - return Promise.resolve(""); - } - final pos = lastReturnToken.getPos(); - return TypingHelper.findTypeWithTyper(context, context.what.fileName, pos.max - 2).then(function(typeHint):Promise { + return parentTypeHint().then(function(typeHint):Promise { if (typeHint == null) { return Promise.resolve(""); } diff --git a/src/refactor/typing/TypingHelper.hx b/src/refactor/typing/TypingHelper.hx index 4a3e0d9..5d5d28a 100644 --- a/src/refactor/typing/TypingHelper.hx +++ b/src/refactor/typing/TypingHelper.hx @@ -138,6 +138,12 @@ class TypingHelper { if (context.typer == null) { return Promise.reject("no typer for " + fileName + "@" + pos); } + switch (context.fileReader(fileName)) { + case Text(text): + pos = context.converter(text, pos); + case Token(root, text): + pos = context.converter(text, pos); + } return context.typer.resolveType(fileName, pos); } diff --git a/test/refactor/TestBase.hx b/test/refactor/TestBase.hx index e2cc622..2876aa3 100644 --- a/test/refactor/TestBase.hx +++ b/test/refactor/TestBase.hx @@ -1,9 +1,16 @@ package refactor; +import haxe.Exception; import haxe.PosInfos; import js.lib.Promise; +import byte.ByteData; +import haxeparser.HaxeLexer; +import hxparse.ParserError; +import tokentree.TokenTree; +import tokentree.TokenTreeBuilder; import refactor.RefactorResult; import refactor.TestEditableDocument; +import refactor.discover.FileContentType; import refactor.discover.FileList; import refactor.discover.NameMap; import refactor.discover.TraverseSources; @@ -137,4 +144,28 @@ class TestBase implements ITest { } return Promise.resolve(success); } + + function fileReader(path:String):FileContentType { + final text = sys.io.File.getContent(path); + var root:Null = null; + try { + final content = ByteData.ofString(text); + final lexer = new HaxeLexer(content, path); + var t:haxeparser.Data.Token = lexer.token(haxeparser.HaxeLexer.tok); + + final tokens:Array = []; + while (t.tok != Eof) { + tokens.push(t); + t = lexer.token(haxeparser.HaxeLexer.tok); + } + root = TokenTreeBuilder.buildTokenTree(tokens, content, TypeLevel); + return Token(root, text); + } catch (e:ParserError) { + throw 'failed to parse ${path} - ParserError: $e (${e.pos})'; + } catch (e:LexerError) { + throw 'failed to parse ${path} - LexerError: ${e.msg} (${e.pos})'; + } catch (e:Exception) { + throw 'failed to parse ${path} - ${e.details()}'; + } + } } diff --git a/test/refactor/refactor/RefactorExtractMethodTest.hx b/test/refactor/refactor/RefactorExtractMethodTest.hx index 1376f7f..4403fb6 100644 --- a/test/refactor/refactor/RefactorExtractMethodTest.hx +++ b/test/refactor/refactor/RefactorExtractMethodTest.hx @@ -67,6 +67,33 @@ class RefactorExtractMethodTest extends RefactorTestBase { checkRefactor(RefactorExtractMethod, {fileName: "testcases/methods/Main.hx", posStart: 568, posEnd: 720}, edits, async); } + function testCalculateTotalJustVars(async:Async) { + var edits:Array = [ + makeReplaceTestEdit("testcases/methods/Main.hx", + "var price;\n" + + "var quantity;\n" + + "{\nfinal data = calculateTotalExtract(item);\n" + + "price = data.price;\n" + + "quantity = data.quantity;\n" + + "}\n", + 630, 686, true), + makeInsertTestEdit("testcases/methods/Main.hx", + "function calculateTotalExtract(item:Item):{price:Float, quantity:Float} {\n" + + "var price = item.price;\n" + + " var quantity = item.quantity;\n" + + "return {\n" + + "price: price,\n" + + "quantity: quantity,\n" + + "};\n" + + "}\n", + 741, true), + ]; + addTypeHint("testcases/methods/Main.hx", 613, LibType("Item", "Item", [])); + addTypeHint("testcases/methods/Main.hx", 638, LibType("Float", "Float", [])); + addTypeHint("testcases/methods/Main.hx", 668, LibType("Float", "Float", [])); + checkRefactor(RefactorExtractMethod, {fileName: "testcases/methods/Main.hx", posStart: 629, posEnd: 686}, edits, async); + } + function testCalculateTotalWithLastReturn(async:Async) { var edits:Array = [ makeReplaceTestEdit("testcases/methods/Main.hx", "return calculateTotalExtract(items, total);\n", 569, 737, true), @@ -83,6 +110,7 @@ class RefactorExtractMethodTest extends RefactorTestBase { + "}\n", 741, true), ]; + addTypeHint("testcases/methods/Main.hx", 514, LibType("Float", "Float", [])); addTypeHint("testcases/methods/Main.hx", 735, LibType("Float", "Float", [])); checkRefactor(RefactorExtractMethod, {fileName: "testcases/methods/Main.hx", posStart: 568, posEnd: 737}, edits, async); } @@ -285,6 +313,7 @@ class RefactorExtractMethodTest extends RefactorTestBase { + "}\n", 357, true), ]; + addTypeHint("testcases/methods/MacroTools.hx", 133, LibType("Array", "Array", [LibType("Field", "Field", [])])); addTypeHint("testcases/methods/MacroTools.hx", 163, LibType("Array", "Array", [LibType("Field", "Field", [])])); addTypeHint("testcases/methods/MacroTools.hx", 351, LibType("Array", "Array", [LibType("Field", "Field", [])])); checkRefactor(RefactorExtractMethod, {fileName: "testcases/methods/MacroTools.hx", posStart: 195, posEnd: 353}, edits, async); diff --git a/test/refactor/refactor/RefactorTestBase.hx b/test/refactor/refactor/RefactorTestBase.hx index 0791970..83aca25 100644 --- a/test/refactor/refactor/RefactorTestBase.hx +++ b/test/refactor/refactor/RefactorTestBase.hx @@ -3,15 +3,9 @@ package refactor.refactor; import haxe.Exception; import haxe.PosInfos; import js.lib.Promise; -import byte.ByteData; -import haxeparser.HaxeLexer; -import hxparse.ParserError; -import tokentree.TokenTree; -import tokentree.TokenTreeBuilder; import utest.Async; import refactor.RefactorResult; import refactor.TestEditableDocument; -import refactor.discover.FileContentType; class RefactorTestBase extends TestBase { function checkRefactor(refactorType:RefactorType, what:RefactorWhat, edits:Array, async:Async, ?pos:PosInfos) { @@ -66,7 +60,7 @@ class RefactorTestBase extends TestBase { }, typer: null, converter: (string, byteOffset) -> byteOffset, - fileReader: testFileReader, + fileReader: fileReader, }); return switch (result) { case Unsupported: @@ -90,33 +84,9 @@ class RefactorTestBase extends TestBase { }, typer: typer, converter: (string, byteOffset) -> byteOffset, - fileReader: testFileReader, + fileReader: fileReader, }).then(function(success:RefactorResult) { return assertEdits(success, editList, edits, pos); }); } } - -function testFileReader(path:String):FileContentType { - final text = sys.io.File.getContent(path); - var root:Null = null; - try { - final content = ByteData.ofString(text); - final lexer = new HaxeLexer(content, path); - var t:haxeparser.Data.Token = lexer.token(haxeparser.HaxeLexer.tok); - - final tokens:Array = []; - while (t.tok != Eof) { - tokens.push(t); - t = lexer.token(haxeparser.HaxeLexer.tok); - } - root = TokenTreeBuilder.buildTokenTree(tokens, content, TypeLevel); - return Token(root, text); - } catch (e:ParserError) { - throw 'failed to parse ${path} - ParserError: $e (${e.pos})'; - } catch (e:LexerError) { - throw 'failed to parse ${path} - LexerError: ${e.msg} (${e.pos})'; - } catch (e:Exception) { - throw 'failed to parse ${path} - ${e.details()}'; - } -} diff --git a/test/refactor/rename/RenameTestBase.hx b/test/refactor/rename/RenameTestBase.hx index 0c9c2d0..7085f6d 100644 --- a/test/refactor/rename/RenameTestBase.hx +++ b/test/refactor/rename/RenameTestBase.hx @@ -60,6 +60,8 @@ class RenameTestBase extends TestBase { Sys.println('${pos.fileName}:${pos.lineNumber}: $text'); }, typer: withTyper ? typer : null, + converter: (string, byteOffset) -> byteOffset, + fileReader: fileReader, }).then(function(success:CanRenameResult) { return doRename(what, edits, withTyper, pos); }); @@ -78,6 +80,8 @@ class RenameTestBase extends TestBase { Sys.println('${pos.fileName}:${pos.lineNumber}: $text'); }, typer: withTyper ? typer : null, + converter: (string, byteOffset) -> byteOffset, + fileReader: fileReader, }).then(function(success:RefactorResult) { return assertEdits(success, editList, edits, pos); });