Skip to content

Commit

Permalink
[return-await] Support explicit resource management
Browse files Browse the repository at this point in the history
  • Loading branch information
kirkwaiblinger committed May 4, 2024
1 parent 1777ac9 commit 62d0950
Show file tree
Hide file tree
Showing 2 changed files with 263 additions and 1 deletion.
42 changes: 41 additions & 1 deletion packages/eslint-plugin/src/rules/return-await.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,44 @@ export default createRule({
scopeInfoStack.pop();
}

function affectsExplicitResourceManagement(node: TSESTree.Node): boolean {
// just need to determine if there is a `using` declaration in scope.
let scope = context.sourceCode.getScope(node);
const functionScope = scope.variableScope;
WHILE: while (true) {
FOR: for (const variable of scope.variables) {
if (variable.defs.length !== 1) {
continue FOR;
}
const [declaration] = variable.defs;
const declaratorNode = declaration.node;
const declarationNode =
declaratorNode.parent as TSESTree.VariableDeclaration;
if (declarationNode.kind !== 'using') {
continue FOR;
}
// deduce whether the variable is declared already or not.
// if it comes, after, we don't care
if (declaratorNode.range[1] < node.range[0]) {
return true;
}
}
if (scope === functionScope) {
// We've checked all the relevant scopes.
break WHILE;
}

if (scope.upper == null) {
// we've hit the top level of the program.
// This shouldn't happen, since
break WHILE;
}
scope = scope.upper;
}

return false;
}

/**
* Tests whether a node is inside of an explicit error handling context
* (try/catch/finally) in a way that throwing an exception will have an
Expand Down Expand Up @@ -242,7 +280,9 @@ export default createRule({
return;
}

const affectsErrorHandling = affectsExplicitErrorHandling(expression);
const affectsErrorHandling =
affectsExplicitErrorHandling(expression) ||
affectsExplicitResourceManagement(node);
const useAutoFix = !affectsErrorHandling;

if (option === 'always') {
Expand Down
222 changes: 222 additions & 0 deletions packages/eslint-plugin/tests/rules/return-await.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,92 @@ async function f() {
}
`,
},
{
code: `
declare const bleh: any;
async function f() {
using something = bleh;
return await Promise.resolve(2);
}
`,
},
{
code: `
declare const bleh: any;
async function f() {
using something = bleh;
{
return await Promise.resolve(2);
}
}
`,
},
{
code: `
declare const bleh: any;
async function f() {
return Promise.resolve(2);
using something = bleh;
}
`,
},
{
code: `
declare const bleh: any;
async function f() {
return await Promise.resolve(2);
using something = bleh;
}
`,
options: ['always'],
},
{
code: `
declare function asyncFn(): Promise<unknown>;
async function returnAwait() {
using _ = {
[Symbol.dispose]: () => {
console.log('dispose');
},
};
return await asyncFn();
}
`,
options: ['in-try-catch'],
},
{
code: `
declare function asyncFn(): Promise<unknown>;
async function outerFunction() {
using _ = {
[Symbol.dispose]: () => {
console.log('dispose');
},
};
async function innerFunction() {
return asyncFn();
}
}
`,
options: ['in-try-catch'],
},
{
code: `
declare function asyncFn(): Promise<unknown>;
async function outerFunction() {
using _ = {
[Symbol.dispose]: () => {
console.log('dispose');
},
};
const innerFunction = async () => asyncFn();
}
`,
options: ['in-try-catch'],
},
],
invalid: [
{
Expand Down Expand Up @@ -1399,5 +1485,141 @@ async function f() {
},
],
},
{
code: `
declare const bleh: any;
async function f() {
if (cond) {
using something = bleh;
if (anotherCondition) {
return Promise.resolve(2);
}
}
}
`,
options: ['always'],
output: null,
errors: [
{
line: 7,
messageId: 'requiredPromiseAwait',
suggestions: [
{
messageId: 'requiredPromiseAwaitSuggestion',
output: `
declare const bleh: any;
async function f() {
if (cond) {
using something = bleh;
if (anotherCondition) {
return await Promise.resolve(2);
}
}
}
`,
},
],
},
],
},
{
code: `
declare const bleh: any;
async function f() {
if (cond) {
using something = bleh;
} else if (anotherCondition) {
return Promise.resolve(2);
}
}
`,
options: ['always'],
output: `
declare const bleh: any;
async function f() {
if (cond) {
using something = bleh;
} else if (anotherCondition) {
return await Promise.resolve(2);
}
}
`,
errors: [
{
line: 7,
messageId: 'requiredPromiseAwait',
},
],
},
{
code: `
declare function asyncFn(): Promise<unknown>;
async function outerFunction() {
using _ = {
[Symbol.dispose]: () => {
console.log('dispose');
},
};
async function innerFunction() {
return await asyncFn();
}
}
`,
options: ['in-try-catch'],
output: `
declare function asyncFn(): Promise<unknown>;
async function outerFunction() {
using _ = {
[Symbol.dispose]: () => {
console.log('dispose');
},
};
async function innerFunction() {
return asyncFn();
}
}
`,
errors: [
{
line: 11,
messageId: 'disallowedPromiseAwait',
},
],
},
{
code: `
declare function asyncFn(): Promise<unknown>;
async function outerFunction() {
using _ = {
[Symbol.dispose]: () => {
console.log('dispose');
},
};
const innerFunction = async () => await asyncFn();
}
`,
options: ['in-try-catch'],
output: `
declare function asyncFn(): Promise<unknown>;
async function outerFunction() {
using _ = {
[Symbol.dispose]: () => {
console.log('dispose');
},
};
const innerFunction = async () => asyncFn();
}
`,
errors: [
{
line: 10,
messageId: 'disallowedPromiseAwait',
},
],
},
],
});

0 comments on commit 62d0950

Please sign in to comment.