added testcases

fixed discovery of identifiers in if conditions
HaxeCheckstyle · Nov 24, 2024 · 7e16139 · 7e16139
1 parent 5bc1166
commit 7e16139
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 5 deletions.
diff --git a/src/refactor/discover/UsageCollector.hx b/src/refactor/discover/UsageCollector.hx
@@ -932,6 +932,8 @@ class UsageCollector {
 					case Binop(OpLt):
 						if (TokenTreeCheckUtils.isTypeParameter(child)) {
 							typeParamLt = child;
+						} else {
+							pOpenToken.push(child);
 						}
 					case Arrow:
 						var scopePos = child.getPos();

diff --git a/src/refactor/refactor/ExtractMethod.hx b/src/refactor/refactor/ExtractMethod.hx
@@ -285,17 +285,17 @@ class ExtractMethod {
 			if (identifier.name.contains(".")) {
 				return false;
 			}
-			switch (identifier.type) {
+			return switch (identifier.type) {
 				case ScopedLocal(scopeStart, scopeEnd, _):
 					if (scopeEnd <= extractData.startToken.pos.min) {
 						return false;
 					}
 					if (scopeStart > extractData.endToken.pos.max) {
 						return false;
 					}
-					return true;
+					true;
 				default:
-					return false;
+					false;
 			}
 		});
 	}
@@ -388,12 +388,13 @@ class ExtractMethod {
 			}
 		}
 
-		final modifiedIdentifiers:Array<Identifier> = [];
+		var modifiedCandidates:Map<String, Identifier> = new Map<String, Identifier>();
 		for (identifier in neededIdentifiers) {
 			if (assignedVars.contains(identifier.name)) {
-				modifiedIdentifiers.push(identifier);
+				modifiedCandidates.set(identifier.name, identifier);
 			}
 		}
+		final modifiedIdentifiers = findIdentifiersUsedAfterSelection(extractData, functionIdentifier, modifiedCandidates);
 
 		if (allReturns.length == 0) {
 			return new CodeGenNoReturn(extractData, context, neededIdentifiers, modifiedIdentifiers, leakingVars);
@@ -428,6 +429,11 @@ class ExtractMethod {
 			}
 		}
 
+		return findIdentifiersUsedAfterSelection(extractData, functionIdentifier, varsValidAfterSelection);
+	}
+
+	static function findIdentifiersUsedAfterSelection(extractData:ExtractMethodData, functionIdentifier:Identifier,
+			varsValidAfterSelection:Map<String, Identifier>):Array<Identifier> {
 		// find all identifier uses after user's selection that share same name
 		final varShadows:Map<String, Identifier> = new Map<String, Identifier>();
 		final allIdentifierUses:Array<Identifier> = functionIdentifier.findAllIdentifiers(identifier -> {

diff --git a/test/refactor/refactor/RefactorExtractMethodTest.hx b/test/refactor/refactor/RefactorExtractMethodTest.hx
@@ -168,6 +168,47 @@ class RefactorExtractMethodTest extends RefactorTestBase {
 		checkRefactor(RefactorExtractMethod, {fileName: "testcases/methods/Main.hx", posStart: 1080, posEnd: 1278}, edits, async);
 	}
 
+	function testCalcConditionalLevel(async:Async) {
+		var edits:Array<TestEdit> = [
+			makeReplaceTestEdit("testcases/methods/Main.hx", "count = calcConditionalLevelExtract(token, count);\n", 1388, 1543, true),
+			makeInsertTestEdit("testcases/methods/Main.hx",
+				"function calcConditionalLevelExtract(token:tokentree.TokenTree, count:Int):Int {\n"
+				+ "while ((token != null) && (token.tok != Root)) {\n"
+				+ "			switch (token.tok) {\n"
+				+ "				case Sharp(\"if\"):\n"
+				+ "					count++;\n"
+				+ "				default:\n"
+				+ "			}\n"
+				+ "			token = token.parent;\n"
+				+ "		}\n"
+				+ "return count;\n"
+				+ "}\n",
+				1600, true),
+		];
+		checkRefactor(RefactorExtractMethod, {fileName: "testcases/methods/Main.hx", posStart: 1387, posEnd: 1543}, edits, async);
+	}
+
+	function testCalcConditionalLevelWithVar(async:Async) {
+		var edits:Array<TestEdit> = [
+			makeReplaceTestEdit("testcases/methods/Main.hx", "var count = calcConditionalLevelExtract(token);\n", 1366, 1543, true),
+			makeInsertTestEdit("testcases/methods/Main.hx",
+				"function calcConditionalLevelExtract(token:tokentree.TokenTree):Int {\n"
+				+ "var count:Int = -1;\n"
+				+ "		while ((token != null) && (token.tok != Root)) {\n"
+				+ "			switch (token.tok) {\n"
+				+ "				case Sharp(\"if\"):\n"
+				+ "					count++;\n"
+				+ "				default:\n"
+				+ "			}\n"
+				+ "			token = token.parent;\n"
+				+ "		}\n"
+				+ "return count;\n"
+				+ "}\n",
+				1600, true),
+		];
+		checkRefactor(RefactorExtractMethod, {fileName: "testcases/methods/Main.hx", posStart: 1365, posEnd: 1543}, edits, async);
+	}
+
 	function testCalculateMath(async:Async) {
 		var edits:Array<TestEdit> = [
 			makeReplaceTestEdit("testcases/methods/Math.hx", "calculateExtract(a, b);\n", 106, 122, true),

diff --git a/test/refactor/rename/RenameClassTest.hx b/test/refactor/rename/RenameClassTest.hx
@@ -103,6 +103,14 @@ class RenameClassTest extends RenameTestBase {
 		checkRename({fileName: "testcases/classes/ChildClass.hx", toName: "ChildList", pos: 872}, edits, async);
 	}
 
+	public function testRenameScopeStart(async:Async) {
+		var edits:Array<TestEdit> = [
+			makeReplaceTestEdit("testcases/classes/ChildClass.hx", "scopeStartRenamed", 255, 265),
+			makeReplaceTestEdit("testcases/classes/ChildClass.hx", "scopeStartRenamed", 404, 414),
+		];
+		checkRename({fileName: "testcases/classes/ChildClass.hx", toName: "scopeStartRenamed", pos: 259}, edits, async);
+	}
+
 	public function testRenameStaticExtentionSum(async:Async) {
 		var edits:Array<TestEdit> = [
 			makeReplaceTestEdit("testcases/classes/ChildHelper.hx", "sumChilds", 62, 65),

diff --git a/testcases/methods/Main.hx b/testcases/methods/Main.hx
@@ -70,6 +70,22 @@ class Main {
 
 		return total;
 	}
+
+	function calcConditionalLevel(token:tokentree.TokenTree):Int {
+		var count:Int = -1;
+		while ((token != null) && (token.tok != Root)) {
+			switch (token.tok) {
+				case Sharp("if"):
+					count++;
+				default:
+			}
+			token = token.parent;
+		}
+		if (count <= 0) {
+			return 0;
+		}
+		return count;
+	}
 }
 
 typedef Item = {