Skip to content

Use correct type for async refactoring diagnostics #26749

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

Merged
merged 8 commits into from
Aug 31, 2018

Conversation

uniqueiniquity
Copy link
Contributor

Fixes #26374, supersedes #26720 .

This PR aims to fix a few issues:

  • Currently, we look to report suggestion diagnostics for the async code fix by checking the signatures of the return type annotation of a FunctionLikeDeclaration. Here, we change it to inspect the signatures of the entire declaration. Attempting this without changing the checker used to a non-diagnostics-producing checker caused issues elsewhere, but this change fixed those issues.
  • Currently, if a FunctionExpression that is fixable is the direct child of a VariableDeclaration, then it reports a diagnostic on the variable name. However, triggering the code fix on the variable name will not successfully perform the change. This PR specifically handles this scenario when performing the code fix.
  • We also add test coverage for FunctionExpressions, as well as FunctionLikeDeclarations without a return type annotation.
  • Lastly, we now validate spans that are reported as suggestion diagnostics against what is expected by the test.

@uniqueiniquity uniqueiniquity assigned RyanCavanaugh, amcasey and ghost and unassigned RyanCavanaugh, amcasey and ghost Aug 29, 2018
@uniqueiniquity uniqueiniquity requested review from RyanCavanaugh, amcasey and a user August 29, 2018 23:06
@@ -1178,11 +1188,17 @@ function [#|f|]() {
}
`);

_testConvertToAsyncFunction("convertToAsyncFunction_simpleFunctionExpression", `
const [#|foo|] = function () {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[#|foo|] [](start = 6, length = 8)

I realize you didn't change this, but that seems like a weird place for the squiggle. What if the function had a name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked at #26801.

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@@ -42,7 +42,18 @@ namespace ts.codefix {

function convertToAsyncFunction(changes: textChanges.ChangeTracker, sourceFile: SourceFile, position: number, checker: TypeChecker, context: CodeFixContextBase): void {
// get the function declaration - returns a promise
const functionToConvert: FunctionLikeDeclaration = getContainingFunction(getTokenAtPosition(sourceFile, position)) as FunctionLikeDeclaration;
const tokenAtPosition = getTokenAtPosition(sourceFile, position);
let functionToConvert: FunctionLikeDeclaration;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let functionToConvert: FunctionLikeDeclaration | undefined;, since getContainingFunction may return undefined.

functionToConvert = tokenAtPosition.parent.initializer;
}
else {
functionToConvert = getContainingFunction(getTokenAtPosition(sourceFile, position)) as FunctionLikeDeclaration;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use tryCast(..., isFunctionLikeDeclaration)

@uniqueiniquity uniqueiniquity merged commit 6ddf752 into microsoft:master Aug 31, 2018
@uniqueiniquity uniqueiniquity deleted the getWholeType branch August 31, 2018 16:09
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

Successfully merging this pull request may close these issues.

Async / Await Refactoring - Not offered on promise returning functions without an explicit return type
3 participants