From f6b5cd0a9d166dd3e5747b2b4261bb1bf409ab75 Mon Sep 17 00:00:00 2001 From: mattias Date: Thu, 2 Jan 2025 10:10:40 +0100 Subject: [PATCH] codetools: fixed node cache for nested proc, RenameIdentifier: do not check for conflict --- components/codetools/codetoolmanager.pas | 48 ++----------------- components/codetools/finddeclarationtool.pas | 7 ++- .../codetools/tests/testfinddeclaration.pas | 32 ++++++++++++- .../codetools/tests/testrefactoring.pas | 13 ++--- 4 files changed, 45 insertions(+), 55 deletions(-) diff --git a/components/codetools/codetoolmanager.pas b/components/codetools/codetoolmanager.pas index 7b749ade74..3041d44446 100644 --- a/components/codetools/codetoolmanager.pas +++ b/components/codetools/codetoolmanager.pas @@ -572,8 +572,7 @@ type var TreeOfPCodeXYPosition: TAVLTree): boolean; function RenameIdentifier(TreeOfPCodeXYPosition: TAVLTree; const OldIdentifier, NewIdentifier: string; - DeclarationCode: TCodeBuffer; DeclarationCaretXY: PPoint; - out isConflicted: boolean): boolean; + DeclarationCode: TCodeBuffer; DeclarationCaretXY: PPoint): boolean; function ReplaceWord(Code: TCodeBuffer; const OldWord, NewWord: string; ChangeStrings: boolean): boolean; function RemoveIdentifierDefinition(Code: TCodeBuffer; X, Y: integer @@ -3038,20 +3037,17 @@ begin end; end; -function TCodeToolManager.RenameIdentifier(TreeOfPCodeXYPosition: TAVLTree; - const OldIdentifier, NewIdentifier: string; DeclarationCode: TCodeBuffer; - DeclarationCaretXY: PPoint; out isConflicted: boolean): boolean; +function TCodeToolManager.RenameIdentifier(TreeOfPCodeXYPosition: TAVLTree; const OldIdentifier, + NewIdentifier: string; DeclarationCode: TCodeBuffer; DeclarationCaretXY: PPoint): boolean; var ANode: TAVLTreeNode; CurCodePos: PCodeXYPosition; IdentStartPos, IdentEndPos,IdentAmdPos: integer; i,j: Integer; Code:TCodeBuffer; - DottedIdents, isOK: Boolean; - StrippedIdent:string; + DottedIdents: Boolean; + StrippedIdent, aComment:string; CodeTool:TCustomCodeTool; - anItem: TIdentifierListItem; - aComment: string; function GetIdent(Identifier: PChar): string; begin @@ -3063,7 +3059,6 @@ var begin Result:=false; - isConflicted:=false; {$IFDEF CTDEBUG} DebugLn('TCodeToolManager.RenameIdentifier A Old=',OldIdentifier,' New=',NewIdentifier,' ',dbgs(TreeOfPCodeXYPosition<>nil)); {$ENDIF} @@ -3083,39 +3078,6 @@ begin // -> go from end of source to start of source, so that replacing does not // change any CodeXYPosition not yet processed - if CompareIdentifiers(PChar(OldIdentifier),PChar(NewIdentifier))=0 then begin - // change in case or ampersands -> no check for conflict needed - end else begin - // check for conflicts - ANode:=TreeOfPCodeXYPosition.FindHighest; - isOK:=true; - while isOK and (ANode<>nil) do begin - CurCodePos:=PCodeXYPosition(ANode.Data); - Code:=CurCodePos^.Code; - Code.LineColToPosition(CurCodePos^.Y,CurCodePos^.X,IdentStartPos); - if IdentStartPos<1 then begin - ANode:=TreeOfPCodeXYPosition.FindPrecessor(ANode); - continue; - end; - // todo: replace GatherIdentifiers with something faster - GatherIdentifiers(Code,CurCodePos^.X,CurCodePos^.Y); - anItem:=IdentifierList.FindIdentifier(PChar(NewIdentifier)); - isOK:= not ((anItem<>nil) and - (CompareDottedIdentifiers(PChar(OldIdentifier), PChar(NewIdentifier))<>0)); - if not isOK then begin - {$IFDEF VerboseFindReferences} - debugln(['TCodeToolManager.RenameIdentifier conflict found for ',dbgs(CurCodePos),', conflicts with ',anItem.AsString]); - {$ENDIF} - break; - end; - ANode:=TreeOfPCodeXYPosition.FindPrecessor(ANode); - end; - if not isOK then begin - isConflicted:=true; - exit; - end; - end; - ANode:=TreeOfPCodeXYPosition.FindLowest; while ANode<>nil do begin // next position diff --git a/components/codetools/finddeclarationtool.pas b/components/codetools/finddeclarationtool.pas index 704ec04619..ba950f6ecb 100644 --- a/components/codetools/finddeclarationtool.pas +++ b/components/codetools/finddeclarationtool.pas @@ -13216,7 +13216,9 @@ begin LastNodeCache:=nil; // start with parent of deepest node and end parent of highest Node:=StartNode; - repeat + if Node.Desc in AllNodeCacheDescs then + Node:=Node.Parent; // e.g. if start was ctnProcedure, it was not searched inside + while Node<>nil do begin if (Node.Desc in AllNodeCacheDescs) then begin if (Node.Cache=nil) then CreateNewNodeCache(Node); @@ -13237,11 +13239,12 @@ begin end; {$ENDIF} LastNodeCache:=CurNodeCache; + if (EndNode=Node) or EndNode.HasAsParent(Node) then break; end; end; end; Node:=Node.Parent; - until (Node=nil) or (EndNode=Node) or EndNode.HasAsParent(Node); + end; {$IFDEF ShowNodeCache} if BeVerbose then begin DebugLn('=========================))))))))))))))))))))))))))))))))'); diff --git a/components/codetools/tests/testfinddeclaration.pas b/components/codetools/tests/testfinddeclaration.pas index 488f640835..2a9c53d4ca 100644 --- a/components/codetools/tests/testfinddeclaration.pas +++ b/components/codetools/tests/testfinddeclaration.pas @@ -120,6 +120,7 @@ type procedure TestFindDeclaration_Program; procedure TestFindDeclaration_Basic; procedure TestFindDeclaration_Proc_BaseTypes; + procedure TestFindDeclaration_ProcMested; procedure TestFindDeclaration_With; procedure TestFindDeclaration_ClassOf; procedure TestFindDeclaration_NestedClasses; @@ -400,9 +401,9 @@ begin //debugln(['TTestFindDeclaration.FindDeclarations Marker="',Marker,'" params: ',dbgstr(MainTool.Src,p,CommentP-p)]); if (Marker='declaration') or (Marker='declaration!') or (Marker='completion') then begin ExpectedPath:=copy(Src,PathPos,CommentP-1-PathPos); - {$IFDEF VerboseFindDeclarationTests} + { $IFDEF VerboseFindDeclarationTests} debugln(['TTestFindDeclaration.FindDeclarations searching "',Marker,'" at ',MainTool.CleanPosToStr(NameStartPos-1),' ExpectedPath=',ExpectedPath]); - {$ENDIF} + { $ENDIF} if (Marker='declaration') or (Marker='declaration!') then begin MainTool.CleanPosToCaret(IdentifierStartPos,CursorPos); @@ -767,6 +768,33 @@ begin FindDeclarations('moduletests/fdt_proc_basetypes.pas'); end; +procedure TTestFindDeclaration.TestFindDeclaration_ProcMested; +begin + StartProgram; + Add([ + '{$mode objfpc}', + 'procedure Fly(Size: word);', + '', + ' procedure Sub1;', + ' var Size: byte;', + ' begin', + ' Size{ declaration:Fly.Sub1.Size}:=3;', + ' end;', + '', + ' procedure Sub2(Size: word);', + ' begin', + ' Size{declaration:Fly.Sub2.Size}:=4;', + ' end;', + 'begin', + ' Size{declaration:Fly.Size}:=Size{ declaration:Fly.Size}+1;', + 'end;', + '', + 'begin', + 'end.', + '']); + FindDeclarations(Code); +end; + procedure TTestFindDeclaration.TestFindDeclaration_With; begin FindDeclarations('moduletests/fdt_with.pas'); diff --git a/components/codetools/tests/testrefactoring.pas b/components/codetools/tests/testrefactoring.pas index bc3b075d9a..9e558ff4b9 100644 --- a/components/codetools/tests/testrefactoring.pas +++ b/components/codetools/tests/testrefactoring.pas @@ -64,7 +64,6 @@ var DeclarationCaretXY: TPoint; PascalReferences: TAVLTree; OldIdentifier: string; - isConflicted: boolean; begin if not IsDottedIdentifier(NewIdentifier) then Fail('TCustomTestRefactoring.RenameReferences invalid NewName="'+NewIdentifier+'"'); @@ -124,14 +123,12 @@ begin Fail('CodeToolBoss.FindReferencesInFiles failed at '+dbgs(DeclarationCaretXY)+' File='+Code.Filename); end; + // todo: check for conflicts + if not CodeToolBoss.RenameIdentifier(PascalReferences, - OldIdentifier, NewIdentifier, DeclCode, @DeclarationCaretXY, isConflicted) - then begin - if isConflicted then - Fail('CodeToolBoss.RenameIdentifier failed as a conflict found') - else - Fail('CodeToolBoss.RenameIdentifier failed'); - end; + OldIdentifier, NewIdentifier, DeclCode, @DeclarationCaretXY) + then + Fail('CodeToolBoss.RenameIdentifier failed'); finally CodeToolBoss.FreeTreeOfPCodeXYPosition(PascalReferences);