Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recurse into if/try branches for explicit returns #52

Open
fredrikekre opened this issue Aug 28, 2024 · 1 comment
Open

Recurse into if/try branches for explicit returns #52

fredrikekre opened this issue Aug 28, 2024 · 1 comment

Comments

@fredrikekre
Copy link
Owner

Currently Runic will format something e.g.

function foo()
    if condition
        x
    else
        y
    end
end

into

function foo()
    return if condition
        x
    else
        y
    end
end

However, looking at some real world code examples of this it seems that about 50% of the time returning whatever value comes out of the if is not intended and adding return after the block is a better "fix". e.g.

function foo()
    if condition
        x
    else
        y
    end
    return
end

and around 50% or the time it is better to insert the returns inside the branches, e.g.

function foo()
    if condition
        return x
    else
        return y
    end
end

Some potential changes to the current setup:

  • If there are no return in any branch recurse and add returns in the branches instead of putting the return in front of the whole block. The question would then be if Runic should also add an explicit else branch when no such branch exist, e.g.
    function foo()
        if condition
            x
        end
    end
    would become
    function foo()
        if condition
            return x
        else
            return
        end
    end
  • If there are return in any branch, make sure there is a return in all branches.

The same discussion applies to try, but for try it can actually improve readability a bit too since it isn't obvious what the possible return values are in e.g.

function foo()
    try
        rand() < 0.5 && error("error")
        "try_value"
    catch err
        "catch_value"
    else
        "else_value"
    finally
        "finally_value"
    end
end

so making it explicit is a good idea, e.g.

function foo()
    try
        rand() < 0.5 && error("error")
        "try_value"
    catch err
        return "catch_value"
    else
        return "else_value"
    finally
        "finally_value"
    end
end
@fredrikekre
Copy link
Owner Author

For try I think the rules would be:

  1. If there is a finally branch with return this will override any other return so it is misleading to add return in any other branch (perhaps Runic should even remove existing ones?)
  2. If there is an else it can always have return except it is misleading to add if 1. or if the try block has a return.
  3. The catch block can always have it except misleading if 1.
  4. The try block can have it if there is no else and if not 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant