Quick fix for functions lacking return expressions #26434

Kingwl · 2018-08-14T03:55:31Z

this pr surmise return type and check the Assignable

the code fix handle follow 4 cases:

1. function like with return type annotation
1. call expression with a function like parameter, and argument is a function like expression
1. assignment with a type annotation, and initializer is a function like expression
1. jsx property assignment whitch can surmise the return type

and surmise follow 3 rules:

1. only one expression statement in Block body and the type is assignable
1. only one Block in Block body and the block only contains one labeled statement, make an object literal generated from label and statement, and that is assignable
  eg:

function A () {
    { a: 'b' }
}

a => {
    { a: 'b' }
}

1. declaration is arrow function and only one labeled statement in Block body, make an object literal generated from label and statement, and that is assignable
  eg:

a => {
    a: 'b'
}

for first case, i try to get the return type from type annotation and compare with type of expression statement
second case, i try to get the parameter from the signature and check the Assignable with the fixed type( after insert the return type)
third case, i try to get the type of the initializer and check the Assignable type of from the variable declaration type annotation ( after insert the return type)

some thing need todo:

1. add test case of fix all
1. add test case of actions
1. add more edge case for fix (eg. object literal or comma with the paren )
1. add more test for labeled statement fix
1. add jsx support
1. add property assign / class property declartion support

RyanCavanaugh · 2019-01-25T05:31:30Z

@DanielRosenwasser I'd like to talk about this one offline

Kingwl · 2019-01-30T15:36:42Z

any offline update here? 🤣

Kingwl · 2019-04-01T09:21:38Z

🙋🏻‍♂️

DanielRosenwasser · 2019-04-02T00:59:33Z

If I recall correctly, Ryan was just concerned that providing too many refactorings and quick fixes could become "noisy". I don't think that that should block this PR since these are legitimate fixes that can help users learn more about the language.

Kingwl · 2019-04-29T10:44:10Z

Maybe I could support more case:

function foo () {
  return // line feed
  {
    a: 1
  }
}

Kingwl · 2019-05-10T03:19:36Z

Could this get down on 3.5?

DanielRosenwasser · 2019-04-29T18:18:21Z

src/services/codefixes/returnValueSurmise.ts

+    }
+
+    function checkFixedAssignableTo(checker: TypeChecker, declaration: FunctionLikeDeclaration, expr: Expression, type: Type, isFunctionType: boolean) {
+        return checker.isTypeAssignableTo(checker.getTypeAtLocation(isFunctionType ? updateFunctionLikeBody(declaration, createBlock([createReturn(expr)])) : expr), type);


@rbuckton is this going to cause any trouble? Here, the type-checker is going to jump into synthetic nodes.

DanielRosenwasser · 2019-04-29T18:19:00Z

src/compiler/diagnosticMessages.json

+        "category": "Message",
+        "code": 95080
+    },
+    "Surmise all return value": {


I've got to admit: I don't know what the word surmise means, and I don't know if users will either. What's the intent here?

"Correct all return expressions" would be best here

"surmise" is not in basic English vocabulary and we need to avoid it entirely for clarity's sake.

DanielRosenwasser · 2019-04-29T18:19:20Z

src/services/codefixes/returnValueSurmise.ts

+        changes.replaceNode(sourceFile, declaration.body, createParen(expression));
+    }
+
+    function getActionForfixAddReturnStatement(context: CodeFixContext, declaration: FunctionLikeDeclaration, expression: Expression) {


Upper-case on fix, here and below

RyanCavanaugh

@Kingwl Overall looking pretty good; just need a couple fixes here before merge. Thanks!

RyanCavanaugh · 2020-01-08T21:22:31Z

src/compiler/diagnosticMessages.json

+        "category": "Message",
+        "code": 95080
+    },
+    "Surmise all return value": {


"Correct all return expressions" would be best here

"surmise" is not in basic English vocabulary and we need to avoid it entirely for clarity's sake.

Kingwl · 2020-01-09T09:44:13Z

@typescript-bot test this.

typescript-bot · 2020-01-09T09:44:17Z

Heya @Kingwl, I've started to run the extended test suite on this PR at 537e806. You can monitor the build here. It should now contribute to this PR's status checks.

DanielRosenwasser · 2020-01-09T09:54:07Z

did you mean

@typescript-bot pack this

?

The tests shouldn't uncover anything since this is an editor feature, right?

Kingwl · 2020-03-18T02:32:07Z

@typescript-bot pack this.

typescript-bot · 2020-03-18T02:32:14Z

Heya @Kingwl, I've started to run the tarball bundle task on this PR at 7fe9a82. You can monitor the build here.

typescript-bot · 2020-03-18T02:45:28Z

Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/68505/artifacts?artifactName=tgz&fileId=8C63B8D64E32F09B6D654098D56F19BF5BD7814BCE0D6A377468DAB51806CFA502&fileName=/typescript-3.9.0-insiders.20200318.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

Kingwl · 2020-03-24T16:54:45Z

Friendly ping @amcasey

amcasey · 2020-03-24T17:17:24Z

@Kingwl The commits kept coming, so I figured I'd just wait for your ping. 😄

amcasey

I made some nit-picky comments, but it looks good. Tomorrow, I'll build and play with it. Thanks!

src/compiler/diagnosticMessages.json

amcasey · 2020-03-25T02:18:25Z

src/services/codefixes/returnValueCorrect.ts

+    ];
+
+    enum FixKind {
+        MissingReturnStatement,


Arguably, these are problem kinds. The corresponding fix kinds would be "AddReturnStatement" and "AddParentheses".

Why are there only two kinds of fixes? Shouldn't there also be one for removing the braces?

And why not just use the fix IDs from above?

src/services/codefixes/returnValueCorrect.ts

tests/cases/fourslash/codeFixSurmiseReturnValue1.ts

amcasey · 2020-03-25T02:32:02Z

src/services/codefixes/returnValueCorrect.ts

+    }
+
+    function removeBlockBodyBrace(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: ArrowFunction, expression: Expression, withParen: boolean) {
+        changes.replaceNode(sourceFile, declaration.body, (withParen || needsParentheses(expression)) ? createParen(expression) : expression);


Is this going to delete comments? I'd be worried about anything between the end of the expression and the closing brace (and maybe things before the opening brace).

Looks like it might delete {/*this comment*/1}. Not the end of the world.

amcasey

Feels good when I use it locally. Still some minor comments open about code cleanliness, but I'd basically merge this as-is.

src/compiler/diagnosticMessages.json

amcasey · 2020-03-26T01:10:24Z

src/services/codefixes/returnValueCorrect.ts

+    }
+
+    function removeBlockBodyBrace(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: ArrowFunction, expression: Expression, withParen: boolean) {
+        changes.replaceNode(sourceFile, declaration.body, (withParen || needsParentheses(expression)) ? createParen(expression) : expression);


Looks like it might delete {/*this comment*/1}. Not the end of the world.

amcasey · 2020-03-26T01:20:49Z

tests/cases/fourslash/codeFixSurmiseReturnValue10.ts

+//// const a: ((() => number) | (() => undefined)) = () => { 1 }
+
+verify.codeFixAvailable([
+    { description: 'Add a return statement' },


Actually, I think @andrewbranch did a bunch of work to make semicolons match the surrounding style automatically. 😄

Kingwl · 2020-03-26T15:20:56Z

@typescript-bot pack this.

typescript-bot · 2020-03-26T15:21:02Z

Heya @Kingwl, I've started to run the tarball bundle task on this PR at 8b332fe. You can monitor the build here.

Kingwl · 2020-03-27T02:11:36Z

@DanielRosenwasser Is there any possibility to get this done in 3.9?

amcasey · 2020-04-02T00:56:13Z

@Kingwl Anything you're still working on (besides the merge conflict?) or shall I merge?

Kingwl · 2020-04-02T09:05:54Z

No, please feel free to merge

Kingwl added 3 commits August 10, 2018 10:34

stash

3f59aa9

add surmise for return type

7671221

add support for more case

0b1e6cc

Kingwl mentioned this pull request Aug 16, 2018

Quickfix for functions missing return statement #13589

Closed

Kingwl added 2 commits August 16, 2018 18:22

add more test case

1f9b9c0

add more testcase and fix all test

aca1722

Kingwl changed the title ~~[WIP] [Review need] Return value surmise~~ Return value surmise Aug 20, 2018

RyanCavanaugh added this to the TypeScript 3.2 milestone Sep 17, 2018

DanielRosenwasser modified the milestones: TypeScript 3.2, TypeScript 3.3.0 Nov 29, 2018

RyanCavanaugh modified the milestones: TypeScript 3.3, Community Jan 25, 2019

RyanCavanaugh assigned DanielRosenwasser Jan 25, 2019

Merge branch 'master' into returnValueSurmise

7cfd0fb

DanielRosenwasser changed the title ~~Return value surmise~~ Quick fix for functions lacking return expressions Apr 29, 2019

Kingwl added 4 commits April 30, 2019 15:17

fix changed diagnosis

cbe59bb

Merge branch 'master' into returnValueSurmise

48f1cca

fix broken test case

98d7eba

add more case

537e806

DanielRosenwasser reviewed May 14, 2019

View reviewed changes

RyanCavanaugh requested changes Jan 8, 2020

View reviewed changes

Kingwl added 3 commits March 18, 2020 09:51

Add return work in same line

94f845a

Merge branch 'master' into returnValueSurmise

a7f37a3

fix test cases

7fe9a82

amcasey reviewed Mar 25, 2020

View reviewed changes

amcasey approved these changes Mar 26, 2020

View reviewed changes

Kingwl added 3 commits March 26, 2020 21:53

rename baseline

af9552e

refactor and handle comment

601fc5e

Support semi

175cf4e

Kingwl force-pushed the returnValueSurmise branch from 16d90f0 to 175cf4e Compare March 26, 2020 15:03

Merge branch 'master' into returnValueSurmise

8b332fe

make helper internal

5d8355c

RyanCavanaugh approved these changes Apr 1, 2020

View reviewed changes

PR Backlog automation moved this from Waiting on author to Needs merge Apr 1, 2020

Merge branch 'master' into returnValueSurmise

522cac8

Kingwl force-pushed the returnValueSurmise branch from 31a0931 to 522cac8 Compare April 2, 2020 09:05

amcasey merged commit afc41f0 into microsoft:master Apr 2, 2020

PR Backlog automation moved this from Needs merge to Done Apr 2, 2020

Kingwl deleted the returnValueSurmise branch April 2, 2020 17:31

rbuckton mentioned this pull request May 8, 2020

Address false positive cases for return value codefix #38410

Closed

github-actions bot mentioned this pull request Aug 14, 2022

Remove span tag in title and unnecessary TOC in Ko microsoft/TypeScript-Website-Localizations#164

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Quick fix for functions lacking return expressions #26434

Quick fix for functions lacking return expressions #26434

Kingwl commented Aug 14, 2018 •

edited

Loading

RyanCavanaugh commented Jan 25, 2019

Kingwl commented Jan 30, 2019

Kingwl commented Apr 1, 2019

DanielRosenwasser commented Apr 2, 2019 •

edited

Loading

Kingwl commented Apr 29, 2019

Kingwl commented May 10, 2019

DanielRosenwasser Apr 29, 2019

DanielRosenwasser Apr 29, 2019

RyanCavanaugh Jan 8, 2020

DanielRosenwasser Apr 29, 2019

RyanCavanaugh left a comment

RyanCavanaugh Jan 8, 2020

Kingwl commented Jan 9, 2020

typescript-bot commented Jan 9, 2020 •

edited

Loading

DanielRosenwasser commented Jan 9, 2020

Kingwl commented Mar 18, 2020

typescript-bot commented Mar 18, 2020 •

edited

Loading

typescript-bot commented Mar 18, 2020 •

edited

Loading

Kingwl commented Mar 24, 2020

amcasey commented Mar 24, 2020

amcasey left a comment

amcasey Mar 25, 2020

amcasey Mar 25, 2020

amcasey Mar 25, 2020

amcasey Mar 25, 2020

amcasey Mar 26, 2020

amcasey left a comment

amcasey Mar 26, 2020

amcasey Mar 26, 2020

Kingwl commented Mar 26, 2020

typescript-bot commented Mar 26, 2020 •

edited

Loading

Kingwl commented Mar 27, 2020

amcasey commented Apr 2, 2020

Kingwl commented Apr 2, 2020

Quick fix for functions lacking return expressions #26434

Quick fix for functions lacking return expressions #26434

Conversation

Kingwl commented Aug 14, 2018 • edited Loading

RyanCavanaugh commented Jan 25, 2019

Kingwl commented Jan 30, 2019

Kingwl commented Apr 1, 2019

DanielRosenwasser commented Apr 2, 2019 • edited Loading

Kingwl commented Apr 29, 2019

Kingwl commented May 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyanCavanaugh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kingwl commented Jan 9, 2020

typescript-bot commented Jan 9, 2020 • edited Loading

DanielRosenwasser commented Jan 9, 2020

Kingwl commented Mar 18, 2020

typescript-bot commented Mar 18, 2020 • edited Loading

typescript-bot commented Mar 18, 2020 • edited Loading

Kingwl commented Mar 24, 2020

amcasey commented Mar 24, 2020

amcasey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amcasey left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kingwl commented Mar 26, 2020

typescript-bot commented Mar 26, 2020 • edited Loading

Kingwl commented Mar 27, 2020

amcasey commented Apr 2, 2020

Kingwl commented Apr 2, 2020

Kingwl commented Aug 14, 2018 •

edited

Loading

DanielRosenwasser commented Apr 2, 2019 •

edited

Loading

typescript-bot commented Jan 9, 2020 •

edited

Loading

typescript-bot commented Mar 18, 2020 •

edited

Loading

typescript-bot commented Mar 18, 2020 •

edited

Loading

typescript-bot commented Mar 26, 2020 •

edited

Loading