codetools: fixed node cache for nested proc, RenameIdentifier: do not check for conflict

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

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('=========================))))))))))))))))))))))))))))))))');

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

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