Skip to content

Allow to extract code containing return statement if it's at the end of a function #18256

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

Open
ghost opened this issue Sep 5, 2017 · 3 comments
Labels
Domain: Refactorings e.g. extract to constant or function, rename symbol Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Milestone

Comments

@ghost
Copy link

ghost commented Sep 5, 2017

TypeScript Version: nightly (2.6.0-dev.20170904)

Code

export function f() {
    if (1) {
        return 1;
    }
    
    ...do stuff...
}

export function g() {
    ...do stuff...

    if (1) {
        return 1;
    }
}

Expected behavior:

Able to extract the if statement out of g.
Not able to extract from f because that would change the control flow.

Actual behavior:

Not able to extract in either case. (But somehow it works in #18091?)

@ghost ghost added Bug A bug in TypeScript Domain: Refactorings e.g. extract to constant or function, rename symbol labels Sep 5, 2017
@mhegazy mhegazy added this to the TypeScript 2.6 milestone Sep 5, 2017
@amcasey
Copy link
Member

amcasey commented Sep 7, 2017

I'm not sure I understand. Are you taking advantage of the fact that the range of the if in g implicitly contains the implicit return at the end of g? If so, I would say that is more of a suggestion than a bug, insofar as we don't guarantee that every meaningfully extractable range will offer the refactoring.

@ghost
Copy link
Author

ghost commented Sep 7, 2017

I think is should be as simple as saying that any selection inside a function that is not followed by anything else should be extractable. No matter where (or if) returns appear in it, it is equivalent to say return newFunction(); and move whatever control flow exists inside of newFunction instead.

@amcasey
Copy link
Member

amcasey commented Sep 7, 2017

From our offline suggestion, I understand this to be a suggestion that we create an exception to our "no extracting conditional returns" in the case where the extracted range is at the end of the method (easy: syntactically; harder: via control flow).

@amcasey amcasey modified the milestones: Future, TypeScript 2.6 Sep 7, 2017
@amcasey amcasey added Suggestion An idea for TypeScript and removed Bug A bug in TypeScript labels Sep 7, 2017
@RyanCavanaugh RyanCavanaugh added the Experience Enhancement Noncontroversial enhancements label Aug 15, 2018
@weswigham weswigham added the In Discussion Not yet reached consensus label Nov 6, 2018
@RyanCavanaugh RyanCavanaugh removed the In Discussion Not yet reached consensus label Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Refactorings e.g. extract to constant or function, rename symbol Experience Enhancement Noncontroversial enhancements Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants