Extract procedure does not handling Exit correctly

[PeterBizz]

When using the extract procedure function, in some cases the exit handling is not handled properly. The follwing situation is a over simplified example (pseudo code)

procedure x (y:integer;z:integer):boolean;
begin
  _if y>z then exit(true);_
  exit(false);
end;

replace the if line with procedure results in :

procedure x (y:integer;z:integer):boolean;
begin
     newProcedure(y, z);
      exit(false);
end;

Solutioin: Since the line(s) selected for extraction do not all end in a return value the extract procedure request should fail

[DavidFeldhoff]

Hi Peter, great catch. Thanks for notifying! I'll look into it

[DavidFeldhoff]

Not made it to the current release as I'm not 100% sure how to solve it (and haven't found more time this weekend). Of course it's one solution to not show the code action then, but that might confuse people as well. Showing it and raising a meaningful error would also be an option (I think that's also your suggestion as I understood you) Another option I thought of is if it's not also helpful if the following would happen:

1. return variable would be created
2. return variable would be handed over to the extracted procedure
3. extracted procedure would return if exit was called or not and in case it was, return variable is returned.

So

procedure x (y:integer;z:integer):boolean;
begin
  _if y>z then exit(true);_
  exit(false);
end;

becomes

procedure x(y: Integer; z: Integer) returnVar: boolean;
begin
    if newProcedure(y, z, returnVar) then
        exit(returnVar);
    exit(false);
end;

procedure newProcedure(y: Integer; z: Integer, var returnVar: boolean): boolean
begin
    if y>z then begin
        returnVar := true;
        exit(true);
    end;
end;

If the code is really big that might be a big help. And that piece of code would be a valid solution in 95% of the cases (record return variables might be an issue). But I'm also not sure if people would like to have the code modified in that way or not. So I tend to go with the option to show the codeAction, but throw a meaningful error first. Implementing the last explained option might still be implemented then later. But anyway, I have to find again a bit time to sit down to implement either solutions of them.

[PeterBizz]

I agree a warning (/error) is enough at the moment. So the user is aware of possible introduction of breaking changes