fixed discovery of identifiers in callback functions, fixes #10

updated tokentree lib
HaxeCheckstyle · Oct 26, 2024 · b271617 · b271617
1 parent da4c46a
commit b271617
Show file tree

Hide file tree

Showing 9 changed files with 127 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## dev branch / next version (2.x.x)
 
+- fixed discovery of identifiers in callback functions, fixes [#10](https://github.com/HaxeCheckstyle/haxe-rename/issues/10)
+
 ## 2.2.3 (2024-09-22)
 
 - fixed package rename when identifier has multiple matches in folder structure, fixes [#3](https://github.com/HaxeCheckstyle/haxe-rename/issues/3)

diff --git a/haxe_libraries/tokentree.hxml b/haxe_libraries/tokentree.hxml
@@ -1,3 +1,3 @@
-# @install: lix --silent download "haxelib:/tokentree#1.2.14" into tokentree/1.2.14/haxelib
--cp ${HAXE_LIBCACHE}/tokentree/1.2.14/haxelib/src
--D tokentree=1.2.14
+# @install: lix --silent download "haxelib:/tokentree#1.2.17" into tokentree/1.2.17/haxelib
+-cp ${HAXE_LIBCACHE}/tokentree/1.2.17/haxelib/src
+-D tokentree=1.2.17
diff --git a/src/refactor/discover/File.hx b/src/refactor/discover/File.hx
@@ -20,6 +20,8 @@ class File {
 		var stat:FileStat = FileSystem.stat(name);
 		fileDate = stat.mtime.getTime();
 		fileSize = stat.size;
+		importList = [];
+		typeList = [];
 	}
 
 	public function init(packageIdent:Null<Identifier>, imports:Array<Import>, types:Array<Type>, posForImport:Int) {

diff --git a/src/refactor/discover/NameMap.hx b/src/refactor/discover/NameMap.hx
@@ -18,6 +18,22 @@ class NameMap {
 		return results;
 	}
 
+	public function getIdentifier(name:String, file:String, pos:Int):Null<Identifier> {
+		var results:Null<Array<Identifier>> = names.get(name);
+		if (results == null) {
+			return null;
+		}
+		for (ident in results) {
+			if (ident.file.name != file) {
+				continue;
+			}
+			if (ident.pos.start == pos) {
+				return ident;
+			}
+		}
+		return null;
+	}
+
 	public function addIdentifier(identifier:Identifier) {
 		function addToMap(map:IdentifierMap, key:String) {
 			var list:Null<Array<Identifier>> = map.get(key);

diff --git a/src/refactor/discover/UsageCollector.hx b/src/refactor/discover/UsageCollector.hx
@@ -595,14 +595,25 @@ class UsageCollector {
 			for (child in token.children) {
 				switch (child.tok) {
 					case Dot | Comma:
-					case POpen | Binop(OpAssign):
+					case POpen:
+						readCallParams(context, child);
+					case Binop(OpAssign):
 					default:
 						readExpression(context, identifier, child);
 				}
 			}
 		}
 	}
 
+	function readCallParams(context:UsageContext, token:Null<TokenTree>) {
+		if (!token.hasChildren()) {
+			return;
+		}
+		for (child in token.children) {
+			readExpression(context, null, child);
+		}
+	}
+
 	function readExpression(context:UsageContext, identifier:Identifier, token:Null<TokenTree>) {
 		if (token == null) {
 			return;
@@ -875,7 +886,7 @@ class UsageCollector {
 		var pack:Array<String> = [];
 		var typeParamLt:Null<TokenTree> = null;
 		var typeHintColon:Null<TokenTree> = null;
-		var pOpenToken:Null<TokenTree> = null;
+		var pOpenToken:Array<TokenTree> = [];
 		var parent:TokenTree = nameToken.parent;
 
 		var lastNamePart:TokenTree = nameToken;
@@ -923,21 +934,21 @@ class UsageCollector {
 							} else {
 								type = Call(false);
 							}
-							pOpenToken = child;
+							pOpenToken.push(child);
 						}
 					case Binop(OpAssign):
 						if (!parent.matches(Kwd(KwdTypedef))) {
-							pOpenToken = child;
+							pOpenToken.push(child);
 						}
 					case Binop(OpIn):
 					case Binop(OpArrow):
 						switch (type) {
 							case ScopedLocal(_, _, ForLoop(_)):
 							default:
-								pOpenToken = child;
+								pOpenToken.push(child);
 						}
 					case BkOpen | Binop(_):
-						pOpenToken = child;
+						pOpenToken.push(child);
 					case DblDot:
 						switch (TokenTreeCheckUtils.getColonType(child)) {
 							case SwitchCase:
@@ -950,6 +961,8 @@ class UsageCollector {
 							case At:
 							case Unknown:
 						}
+					case Dot:
+						pOpenToken.push(child);
 					default:
 				}
 			}
@@ -958,7 +971,11 @@ class UsageCollector {
 		if (pack.length <= 0) {
 			return null;
 		}
-		var identifier:Identifier = new Identifier(type, pack.join("."), pos, context.nameMap, context.file, context.type);
+		var name:String = pack.join(".");
+		var identifier:Null<Identifier> = context.nameMap.getIdentifier(name, context.file.name, pos.start);
+		if (identifier == null) {
+			identifier = new Identifier(type, name, pos, context.nameMap, context.file, context.type);
+		}
 
 		if (parentIdentifier != null) {
 			parentIdentifier.addUse(identifier);
@@ -969,10 +986,10 @@ class UsageCollector {
 		if (typeHintColon != null) {
 			readTypeHint(context, identifier, typeHintColon, TypeHint);
 		}
-		if (pOpenToken != null) {
-			readExpression(context, identifier, pOpenToken);
-			if (pOpenToken.nextSibling != null) {
-				readExpression(context, identifier, pOpenToken.nextSibling);
+		for (child in pOpenToken) {
+			readExpression(context, identifier, child);
+			if (child.nextSibling != null) {
+				readExpression(context, identifier, child.nextSibling);
 			}
 		}
 		return identifier;

diff --git a/test/refactor/ClassTest.hx b/test/refactor/ClassTest.hx
@@ -288,4 +288,18 @@ class ClassTest extends TestBase {
 		];
 		refactorAndCheck({fileName: "testcases/classes/DemoClassA.hx", toName: "wasRenamed", pos: 61}, edits, async);
 	}
+
+	public function testRenamePrinter(async:Async) {
+		var edits:Array<TestEdit> = [
+			makeMoveTestEdit("testcases/classes/Printer.hx", "testcases/classes/PrinterRenamed.hx"),
+			makeReplaceTestEdit("testcases/classes/Printer.hx", "PrinterRenamed", 383, 390),
+			makeReplaceTestEdit("testcases/classes/Printer.hx", "PrinterRenamed", 545, 552),
+			makeReplaceTestEdit("testcases/classes/Printer.hx", "PrinterRenamed", 699, 706),
+			makeReplaceTestEdit("testcases/classes/Printer.hx", "PrinterRenamed", 930, 937),
+			makeReplaceTestEdit("testcases/classes/Printer.hx", "PrinterRenamed", 1073, 1080),
+			makeReplaceTestEdit("testcases/classes/import.hx", "PrinterRenamed", 80, 87),
+			makeReplaceTestEdit("testcases/classes/pack/UsePrinter.hx", "PrinterRenamed", 58, 65),
+		];
+		refactorAndCheck({fileName: "testcases/classes/Printer.hx", toName: "PrinterRenamed", pos: 1076}, edits, async);
+	}
 }
diff --git a/testcases/classes/Printer.hx b/testcases/classes/Printer.hx
@@ -0,0 +1,54 @@
+package testcases.classes;
+
+import js.Browser;
+import tink.core.Future;
+import tink.core.Promise;
+
+@:expose class PrinterMain {
+	public static function main():Void {
+		// Setup stuff
+		final futureTrigger = Future.trigger();
+		futureTrigger.trigger("Future");
+
+		final promise:Promise<String> = Promise.resolve("Promise");
+
+		// Example 1 - "Normal" usage of `Printer` (WORKS)
+		new Printer("Normal");
+
+		// Example 2 - `Printer` used within promise handling (WORKS)
+		promise.handle(result -> {
+			switch (result) {
+				case Success(text): new Printer(text);
+				default:
+			}
+		});
+
+		// Example 3 - `Printer` used within future handling (FAILS)
+		futureTrigger.asFuture().handle(text -> {
+			new Printer(text);
+		});
+
+		// Example 4 - `Printer` used within promise handling but promise is created in `TextLoader` (FAILS)
+		new TextLoader().load("TextLoader").handle(result -> {
+			switch (result) {
+				case Success(text): new Printer(text);
+				default:
+			}
+		});
+	}
+}
+
+// Rename this class and observe that examples 1 and 2 are updated, but the others are not.
+class Printer {
+	public function new(text:String) {
+		Browser.console.log(text);
+	}
+}
+
+class TextLoader {
+	public function new() {}
+
+	public function load(text:String):Promise<String> {
+		return Promise.resolve(text);
+	}
+}
diff --git a/testcases/classes/import.hx b/testcases/classes/import.hx
@@ -1,4 +1,5 @@
 package classes;
 
 import refactor.discover.Identifier;
+import testcases.classes.Printer;
 import classes.*;
diff --git a/testcases/classes/pack/UsePrinter.hx b/testcases/classes/pack/UsePrinter.hx
@@ -0,0 +1,7 @@
+package testcases.classes.pack;
+
+import testcases.classes.Printer;
+
+function usingPrinter() {
+	new TextLoader();
+}