fixed wrong position for hover requests

fixed duplicated identifiers for return values
modified codegens

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<ITyper>;
+	var fileReader:FileReaderFunc;
+	var converter:ByteToCharConverterFunc;
 }
+
+typedef ByteToCharConverterFunc = (string:String, byteOffset:Int) -> Int;

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) {

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;

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;

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 {

src/refactor/refactor/extractmethod/CodeGenBase.hx

@@ -37,6 +37,49 @@ abstract class CodeGenBase implements ICodeGen {
 		}
 		return snippet;
 	}
+
+	function parentTypeHint():Promise<TypeHintType> {
+		var func:Null<TokenTree> = 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<TokenTree> {
+		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;

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";
 		}
 	}

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";
 		}
 	}

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<TypeHintType> {
-		var func:Null<TokenTree> = 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<TokenTree> {
-		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';
 		}
 	}

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<TokenTree>;
-
-	public function new(extractData:ExtractMethodData, context:RefactorContext, neededIdentifiers:Array<Identifier>, lastReturnToken:TokenTree) {
+	public function new(extractData:ExtractMethodData, context:RefactorContext, neededIdentifiers:Array<Identifier>) {
 		super(extractData, context, neededIdentifiers);
-
-		this.lastReturnToken = lastReturnToken;
 	}
 
 	public function makeCallSite():String {
@@ -19,11 +15,7 @@ class CodeGenReturnIsLast extends CodeGenBase {
 	}
 
 	public function makeReturnTypeHint():Promise<String> {
-		if (lastReturnToken == null) {
-			return Promise.resolve("");
-		}
-		final pos = lastReturnToken.getPos();
-		return TypingHelper.findTypeWithTyper(context, context.what.fileName, pos.max - 2).then(function(typeHint):Promise<String> {
+		return parentTypeHint().then(function(typeHint):Promise<String> {
 			if (typeHint == null) {
 				return Promise.resolve("");
 			}

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);
 	}
 

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<TokenTree> = 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<haxeparser.Data.Token> = [];
+			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()}';
+		}
+	}
 }

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<TestEdit> = [
+			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<TestEdit> = [
 			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);