From f6b3be28f74a03664b12455a75c7b96f35bb8af5 Mon Sep 17 00:00:00 2001 From: AlexHaxe Date: Fri, 20 May 2022 00:54:41 +0200 Subject: [PATCH] fixed local var scope --- .haxerc | 2 +- CHANGELOG.md | 4 ++ haxelib.json | 4 +- package-lock.json | 4 +- package.json | 2 +- src/refactor/PrintHelper.hx | 8 ++-- src/refactor/Refactor.hx | 12 ++--- src/refactor/discover/IdentifierType.hx | 4 +- src/refactor/discover/UsageCollector.hx | 39 +++++++++------- src/refactor/rename/RenameField.hx | 13 ++++-- src/refactor/rename/RenameHelper.hx | 15 +++--- src/refactor/rename/RenameScopedLocal.hx | 22 +++++---- test/refactor/ScopedLocalTest.hx | 59 ++++++++++++++++++++++++ testcases/scopedlocal/Refactor.hx | 17 +++++++ 14 files changed, 150 insertions(+), 55 deletions(-) diff --git a/.haxerc b/.haxerc index 5ee0dc9..64dc84b 100644 --- a/.haxerc +++ b/.haxerc @@ -1,4 +1,4 @@ { - "version": "bd1a05d", + "version": "a8f2911", "resolveLibs": "scoped" } \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 37a42b4..475b2bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## dev branch / next version (2.x.x) +## 2.2.1 (2022-05-20) + +- fixed local var scope + ## 2.2.0 (2022-05-03) - added canRename API call diff --git a/haxelib.json b/haxelib.json index a5e109f..e68d05c 100644 --- a/haxelib.json +++ b/haxelib.json @@ -8,8 +8,8 @@ "refactor" ], "description": "A code renaming tool for Haxe", - "version": "2.2.0", - "releasenote": "added canRename API call - see CHANGELOG", + "version": "2.2.1", + "releasenote": "fixed local var scope - see CHANGELOG", "contributors": [ "AlexHaxe" ], diff --git a/package-lock.json b/package-lock.json index 000e0e4..2b23521 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@haxecheckstyle/haxe-rename", - "version": "2.2.0", + "version": "2.2.1", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@haxecheckstyle/haxe-rename", - "version": "2.2.0", + "version": "2.2.1", "license": "MIT", "bin": { "haxe-rename": "bin/rename.js" diff --git a/package.json b/package.json index cf9cb90..67fb9a6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@haxecheckstyle/haxe-rename", - "version": "2.2.0", + "version": "2.2.1", "description": "Renaming tool for Haxe", "repository": { "type": "git", diff --git a/src/refactor/PrintHelper.hx b/src/refactor/PrintHelper.hx index 166de2d..39afd83 100644 --- a/src/refactor/PrintHelper.hx +++ b/src/refactor/PrintHelper.hx @@ -18,8 +18,8 @@ class PrintHelper { 'EnumField(${params.map((p) -> '"${p.name}"')})'; case CaseLabel(switchIdentifier): 'CaseLabel(${switchIdentifier.name})'; - case ScopedLocal(scopeEnd, scopeType): - 'ScopedLocal($scopeEnd, ${scopeType.scopeTypeToString()})'; + case ScopedLocal(scopeStart, scopeEnd, scopeType): + 'ScopedLocal($scopeStart - $scopeEnd, ${scopeType.scopeTypeToString()})'; default: '$identType'; } @@ -29,8 +29,8 @@ class PrintHelper { return switch (scopeType) { case Parameter(params): 'Parameter(${params.map((i) -> '"${i.name}"')})'; - case ForLoop(scopeStart, loopIdentifiers): - 'ForLoop(${loopIdentifiers.map((i) -> '"${i.name}"')}) - scopeStart: $scopeStart'; + case ForLoop(loopIdentifiers): + 'ForLoop(${loopIdentifiers.map((i) -> '"${i.name}"')})'; default: '$scopeType'; } diff --git a/src/refactor/Refactor.hx b/src/refactor/Refactor.hx index 9169771..6f05d83 100644 --- a/src/refactor/Refactor.hx +++ b/src/refactor/Refactor.hx @@ -119,9 +119,9 @@ class Refactor { rename(context); case CaseLabel(_): Promise.reject(RefactorResult.Unsupported(identifier.toString()).printRefactorResult()); - case ScopedLocal(scopeEnd, type): - context.verboseLog('rename scoped local "${identifier.name}" (${type.scopeTypeToString()}) to "${context.what.toName}"'); - RenameScopedLocal.refactorScopedLocal(context, file, identifier, scopeEnd); + case ScopedLocal(scopeStart, scopeEnd, type): + context.verboseLog('rename scoped local "${identifier.name} [$scopeStart - $scopeEnd]" (${type.scopeTypeToString()}) to "${context.what.toName}"'); + RenameScopedLocal.refactorScopedLocal(context, file, identifier, scopeStart, scopeEnd); } } @@ -153,12 +153,12 @@ class Refactor { if (candidate == null) { candidate = use; } - case ScopedLocal(scopeEnd, ForLoop(scopeStart, _)) if (!onlyFields): + case ScopedLocal(scopeStart, scopeEnd, ForLoop(_)) if (!onlyFields): if ((scopeStart < identifier.pos.start) && (identifier.pos.start < scopeEnd)) { candidate = use; } - case ScopedLocal(scopeEnd, _) if (!onlyFields): - if ((use.pos.start < identifier.pos.start) && (identifier.pos.start < scopeEnd)) { + case ScopedLocal(scopeStart, scopeEnd, _) if (!onlyFields): + if ((scopeStart < identifier.pos.start) && (identifier.pos.start < scopeEnd)) { candidate = use; } default: diff --git a/src/refactor/discover/IdentifierType.hx b/src/refactor/discover/IdentifierType.hx index e9ea9e1..6318673 100644 --- a/src/refactor/discover/IdentifierType.hx +++ b/src/refactor/discover/IdentifierType.hx @@ -36,7 +36,7 @@ enum IdentifierType { ArrayAccess(posClosing:Int); Access; ForIterator; - ScopedLocal(scopeEnd:Int, scopeType:ScopedLocalType); + ScopedLocal(scopeStart:Int, scopeEnd:Int, scopeType:ScopedLocalType); StringConst; } @@ -44,7 +44,7 @@ enum ScopedLocalType { Parameter(params:Array); Var; CaseCapture; - ForLoop(scopeStart:Int, loopIdentifiers:Array); + ForLoop(loopIdentifiers:Array); } enum TypedefFieldType { diff --git a/src/refactor/discover/UsageCollector.hx b/src/refactor/discover/UsageCollector.hx index f358be0..a6953ff 100644 --- a/src/refactor/discover/UsageCollector.hx +++ b/src/refactor/discover/UsageCollector.hx @@ -560,11 +560,14 @@ class UsageCollector { for (child in token.children) { switch (child.tok) { case Kwd(KwdVar): - makeIdentifier(context, child.getFirstChild(), ScopedLocal(scopeEnd, Var), identifier); + child = child.getFirstChild(); + makeIdentifier(context, child, ScopedLocal(child.getPos().max, scopeEnd, Var), identifier); case Kwd(KwdFinal): - makeIdentifier(context, child.getFirstChild(), ScopedLocal(scopeEnd, Var), identifier); + child = child.getFirstChild(); + makeIdentifier(context, child, ScopedLocal(child.getPos().max, scopeEnd, Var), identifier); case Kwd(KwdFunction): - var method:Identifier = makeIdentifier(context, child.getFirstChild(), ScopedLocal(scopeEnd, Var), identifier); + child = child.getFirstChild(); + var method:Identifier = makeIdentifier(context, child, ScopedLocal(child.pos.min, scopeEnd, Var), identifier); readMethod(context, method, child.getFirstChild()); case Dot: case Semicolon: @@ -599,17 +602,19 @@ class UsageCollector { case Kwd(KwdVar): var fullPos:Position = token.parent.getPos(); var scopeEnd:Int = fullPos.max; - var variable:Identifier = makeIdentifier(context, token.getFirstChild(), ScopedLocal(scopeEnd, Var), identifier); - readVarInit(context, variable, token.getFirstChild()); + var token:TokenTree = token.getFirstChild(); + var variable:Identifier = makeIdentifier(context, token, ScopedLocal(token.getPos().max, scopeEnd, Var), identifier); + readVarInit(context, variable, token); return; case Kwd(KwdFunction): var fullPos:Position = token.parent.getPos(); var scopeEnd:Int = fullPos.max; - var method:Null = makeIdentifier(context, token.getFirstChild(), ScopedLocal(scopeEnd, Var), identifier); + var child:TokenTree = token.getFirstChild(); + var method:Null = makeIdentifier(context, child, ScopedLocal(child.pos.min, scopeEnd, Var), identifier); if (method == null) { readMethod(context, identifier, token); } else { - readMethod(context, method, token.getFirstChild()); + readMethod(context, method, child); } return; case Kwd(KwdThis): @@ -706,7 +711,7 @@ class UsageCollector { if (pClose != null) { scopeStart = pClose.pos.max; } - var ident:Identifier = makeIdentifier(context, token, ScopedLocal(scopeEnd, ForLoop(scopeStart, loopIdentifiers)), identifier); + var ident:Identifier = makeIdentifier(context, token, ScopedLocal(scopeStart, scopeEnd, ForLoop(loopIdentifiers)), identifier); loopIdentifiers.push(ident); if (!token.hasChildren()) { return; @@ -714,7 +719,7 @@ class UsageCollector { for (child in token.children) { switch (child.tok) { case Binop(OpArrow): - ident = makeIdentifier(context, child.getFirstChild(), ScopedLocal(scopeEnd, ForLoop(scopeStart, loopIdentifiers)), identifier); + ident = makeIdentifier(context, child.getFirstChild(), ScopedLocal(scopeStart, scopeEnd, ForLoop(loopIdentifiers)), identifier); loopIdentifiers.push(ident); default: readExpression(context, ident, child); @@ -744,7 +749,8 @@ class UsageCollector { case Const(CIdent(_)): readCaseConst(context, identifier, child, scopeEnd); case Kwd(KwdVar): - makeIdentifier(context, child.getFirstChild(), ScopedLocal(scopeEnd, CaseCapture), identifier); + child = child.getFirstChild(); + makeIdentifier(context, child, ScopedLocal(child.pos.min, scopeEnd, CaseCapture), identifier); case BkOpen: readCaseArray(context, identifier, child, scopeEnd); case BrOpen: @@ -780,7 +786,7 @@ class UsageCollector { return; } for (child in token.children) { - makeIdentifier(context, child, ScopedLocal(scopeEnd, CaseCapture), identifier); + makeIdentifier(context, child, ScopedLocal(child.pos.max, scopeEnd, CaseCapture), identifier); } } @@ -809,7 +815,7 @@ class UsageCollector { } if (field.uses != null) { for (use in field.uses) { - use.type = ScopedLocal(scopeEnd, CaseCapture); + use.type = ScopedLocal(use.pos.start, scopeEnd, CaseCapture); } } default: @@ -823,10 +829,11 @@ class UsageCollector { for (child in token.children) { switch (child.tok) { case Question: - var paramIdent:Identifier = makeIdentifier(context, child.getFirstChild(), ScopedLocal(scopeEnd, Parameter(params)), identifier); + child = child.getFirstChild(); + var paramIdent:Identifier = makeIdentifier(context, child, ScopedLocal(child.pos.min, scopeEnd, Parameter(params)), identifier); params.push(paramIdent); - case Const(CIdent(s)): - var paramIdent:Identifier = makeIdentifier(context, child, ScopedLocal(scopeEnd, Parameter(params)), identifier); + case Const(CIdent(_)): + var paramIdent:Identifier = makeIdentifier(context, child, ScopedLocal(child.pos.min, scopeEnd, Parameter(params)), identifier); params.push(paramIdent); default: } @@ -912,7 +919,7 @@ class UsageCollector { case Binop(OpIn): case Binop(OpArrow): switch (type) { - case ScopedLocal(_, ForLoop(_)): + case ScopedLocal(_, _, ForLoop(_)): default: pOpenToken = child; } diff --git a/src/refactor/rename/RenameField.hx b/src/refactor/rename/RenameField.hx index 23fc9a7..8ef6c1f 100644 --- a/src/refactor/rename/RenameField.hx +++ b/src/refactor/rename/RenameField.hx @@ -65,14 +65,17 @@ class RenameField { static function replaceInType(changelist:Changelist, type:Type, prefix:String, from:String, to:String) { var allUses:Array = type.getIdentifiers(prefix + from); - var scopeEnd:Int = 0; + var innerScopeStart:Int = 0; + var innerScopeEnd:Int = -1; for (use in allUses) { - if (use.pos.start <= scopeEnd) { + if ((innerScopeStart < use.pos.start) && (use.pos.start < innerScopeEnd)) { continue; } + switch (use.type) { - case ScopedLocal(end, _): - scopeEnd = end; + case ScopedLocal(start, end, _): + innerScopeStart = start; + innerScopeEnd = end; continue; case StructureField(_): continue; @@ -92,7 +95,7 @@ class RenameField { break; } switch (use.type) { - case ScopedLocal(end, _): + case ScopedLocal(_, end, _): if (end > access.pos.start) { shadowed = true; break; diff --git a/src/refactor/rename/RenameHelper.hx b/src/refactor/rename/RenameHelper.hx index fa3e33a..abe4eb8 100644 --- a/src/refactor/rename/RenameHelper.hx +++ b/src/refactor/rename/RenameHelper.hx @@ -170,8 +170,11 @@ class RenameHelper { fieldCandidate = use; case EnumField(_): return Promise.resolve(KnownType(use.defineType, [])); - case ScopedLocal(scopeEnd, _): - if ((pos >= use.pos.start) && (pos <= scopeEnd)) { + case ScopedLocal(scopeStart, scopeEnd, _): + if ((pos >= scopeStart) && (pos <= scopeEnd)) { + candidate = use; + } + if (pos == use.pos.start) { candidate = use; } case CaseLabel(switchIdentifier): @@ -190,12 +193,12 @@ class RenameHelper { var typeHint:Null = candidate.getTypeHint(); switch (candidate.type) { - case ScopedLocal(_, ForLoop(_, loopIdent)): + case ScopedLocal(_, _, ForLoop(loopIdent)): var index:Int = loopIdent.indexOf(candidate); var changes:Array> = []; for (child in loopIdent) { switch (child.type) { - case ScopedLocal(_, ForLoop(_, _)): + case ScopedLocal(_, _, ForLoop(_)): continue; default: changes.push(findTypeOfIdentifier(context, { @@ -223,7 +226,7 @@ class RenameHelper { } return Promise.reject("type not found"); }); - case ScopedLocal(_, Parameter(params)): + case ScopedLocal(_, _, Parameter(params)): if (typeHint != null) { return typeFromTypeHint(context, typeHint); } @@ -376,7 +379,7 @@ class RenameHelper { var firstParam:Null = null; for (use in identifier.uses) { switch (use.type) { - case ScopedLocal(_, Parameter(_)): + case ScopedLocal(_, _, Parameter(_)): firstParam = use; break; default: diff --git a/src/refactor/rename/RenameScopedLocal.hx b/src/refactor/rename/RenameScopedLocal.hx index 7ac7cc3..f7a827c 100644 --- a/src/refactor/rename/RenameScopedLocal.hx +++ b/src/refactor/rename/RenameScopedLocal.hx @@ -8,11 +8,13 @@ import refactor.discover.IdentifierPos; import refactor.edits.Changelist; class RenameScopedLocal { - public static function refactorScopedLocal(context:RefactorContext, file:File, identifier:Identifier, scopeEnd:Int):Promise { + public static function refactorScopedLocal(context:RefactorContext, file:File, identifier:Identifier, scopeStart:Int, + scopeEnd:Int):Promise { var changelist:Changelist = new Changelist(context); var identifierDot:String = identifier.name + "."; var toNameDot:String = context.what.toName + "."; - var scopeStart:Int = identifier.pos.start; + changelist.addChange(identifier.pos.fileName, ReplaceText(context.what.toName, identifier.pos), identifier); + var allUses:Array = identifier.defineType.findAllIdentifiers(function(ident:Identifier) { if (ident.pos.start < scopeStart) { return false; @@ -60,20 +62,20 @@ class RenameScopedLocal { } var skipForIterator:Bool = false; + var innerScopeStart:Int = 0; + var innerScopeEnd:Int = -1; for (use in allUses) { + if ((innerScopeStart < use.pos.start) && (use.pos.start < innerScopeEnd)) { + continue; + } switch (use.type) { - case ScopedLocal(scopeEnd, ForLoop(start, _)): + case ScopedLocal(start, scopeEnd, _): if (use.pos.start == identifier.pos.start) { scopeStart = start; skipForIterator = true; } else { - scopeStart = scopeEnd; - continue; - } - case ScopedLocal(scopeEnd, _): - if (use.pos.start != identifier.pos.start) { - // new parameter with identical name, so we skip its scope - scopeStart = scopeEnd; + innerScopeStart = start; + innerScopeEnd = scopeEnd; continue; } case StructureField(_): diff --git a/test/refactor/ScopedLocalTest.hx b/test/refactor/ScopedLocalTest.hx index 7f05c98..b247b7c 100644 --- a/test/refactor/ScopedLocalTest.hx +++ b/test/refactor/ScopedLocalTest.hx @@ -115,4 +115,63 @@ class ScopedLocalTest extends TestBase { ]; refactorAndCheck({fileName: "testcases/scopedlocal/Refactor.hx", toName: "file", pos: 2181}, edits, async); } + + public function testRenameParameterValue(async:Async) { + var edits:Array = [ + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2271, 2276), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2300, 2305), + ]; + refactorAndCheck({fileName: "testcases/scopedlocal/Refactor.hx", toName: "data", pos: 2274}, edits, async); + } + + public function testRenameParameterValue2(async:Async) { + var edits:Array = [ + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2426, 2431), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2453, 2458), + ]; + refactorAndCheck({fileName: "testcases/scopedlocal/Refactor.hx", toName: "data", pos: 2429}, edits, async); + } + + public function testRenameLocalValue(async:Async) { + var edits:Array = [ + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2290, 2295), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2323, 2328), + ]; + refactorAndCheck({fileName: "testcases/scopedlocal/Refactor.hx", toName: "data", pos: 2291}, edits, async); + } + + public function testRenameLocalValue2(async:Async) { + var edits:Array = [ + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2290, 2295), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2323, 2328), + ]; + refactorAndCheck({fileName: "testcases/scopedlocal/Refactor.hx", toName: "data", pos: 2326}, edits, async); + } + + public function testRenameLocalValue3(async:Async) { + var edits:Array = [ + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2445, 2450), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2477, 2482), + ]; + refactorAndCheck({fileName: "testcases/scopedlocal/Refactor.hx", toName: "data", pos: 2447}, edits, async); + } + + public function testRenameLocalValue4(async:Async) { + var edits:Array = [ + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2445, 2450), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2477, 2482), + ]; + refactorAndCheck({fileName: "testcases/scopedlocal/Refactor.hx", toName: "data", pos: 2479}, edits, async); + } + + public function testRenameFieldValue(async:Async) { + var edits:Array = [ + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2237, 2242), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2315, 2320), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2375, 2380), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2390, 2395), + makeReplaceTestEdit("testcases/scopedlocal/Refactor.hx", "data", 2467, 2472), + ]; + refactorAndCheck({fileName: "testcases/scopedlocal/Refactor.hx", toName: "data", pos: 2239}, edits, async); + } } diff --git a/testcases/scopedlocal/Refactor.hx b/testcases/scopedlocal/Refactor.hx index 7cd01a2..21528f1 100644 --- a/testcases/scopedlocal/Refactor.hx +++ b/testcases/scopedlocal/Refactor.hx @@ -78,4 +78,21 @@ class Refactor { trace(val); } } + + var value:String; + + function setValue(value:Int) { + var value = '$value'; + this.value = value; + } + + function clearValue() { + var value = '$value'; + this.value = ""; + } + + function setValue2(value:Int) { + var value = value; + this.value = '$value'; + } }