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

Repo: Make utility for static member access #8323

Open
kirkwaiblinger opened this issue Jan 30, 2024 · 5 comments · May be fixed by #9836
Open

Repo: Make utility for static member access #8323

kirkwaiblinger opened this issue Jan 30, 2024 · 5 comments · May be fixed by #9836
Labels
accepting prs Go ahead, send a pull request that resolves this issue package: utils Issues related to the @typescript-eslint/utils package repo maintenance things to do with maintenance of the repo, and not with code/docs

Comments

@kirkwaiblinger
Copy link
Member

Suggestion

This is a refactoring opportunity that comes from #8216 (comment).

In short, it may be a common pattern to want to check for either code that looks like

object.filter

or

const computedMember = "filter";
object[computedMember];

This could be extracted to a utility with approximate usage

declare const memberExpression: MemberExpressionComputedName | MemberExpressionNonComputedName,
const isFilterCall = isStaticMemberAccessOfValue(memberExpression, "filter");

An example implementation of this is 581e9ae#diff-5af06c156c2d34d6ea1d2b8f91d583d82d7e629cbd5226145c6c0b72449bcfb2R298, implemented in a way that did not aim to be more generic than the needs of that specific module. This might be possible to replace with a more general utility

@kirkwaiblinger kirkwaiblinger added repo maintenance things to do with maintenance of the repo, and not with code/docs triage Waiting for team members to take a look labels Jan 30, 2024
@auvred
Copy link
Member

auvred commented Jan 31, 2024

+1! It would be nice to make these checks consistent throughout the plugin codebase. There are a few examples that can be changed as part of this refactor:

@JoshuaKGoldberg
Copy link
Member

Whew, what a list! +1 to this from me 🙂 thanks for filing + looking!

@JoshuaKGoldberg JoshuaKGoldberg added accepting prs Go ahead, send a pull request that resolves this issue and removed triage Waiting for team members to take a look labels Jan 31, 2024
@kirkwaiblinger
Copy link
Member Author

kirkwaiblinger commented Feb 1, 2024

I believe these guys could similarly be replaced with a utility, as well as probably a few more places in that file

function getRejectionHandlerFromThenCall(
expression: TSESTree.CallExpression,
): TSESTree.CallExpressionArgument | undefined {
if (
expression.callee.type === AST_NODE_TYPES.MemberExpression &&
expression.callee.property.type === AST_NODE_TYPES.Identifier &&
expression.callee.property.name === 'then' &&
expression.arguments.length >= 2
) {
return expression.arguments[1];
}
return undefined;
}
function getObjectFromFinallyCall(
expression: TSESTree.CallExpression,
): TSESTree.Expression | undefined {
return expression.callee.type === AST_NODE_TYPES.MemberExpression &&
expression.callee.property.type === AST_NODE_TYPES.Identifier &&
expression.callee.property.name === 'finally'
? expression.callee.object
: undefined;
}

@JoshuaKGoldberg
Copy link
Member

So it doesn't get lost: #8412 has another one for class-literal-property-style. Would be nice to re-use in that rule too.

@auvred
Copy link
Member

auvred commented Apr 23, 2024

Noticed this while working on #8952:

unbound-method rule also needs more thorough handling of static member access: playground

@Josh-Cena Josh-Cena added the package: utils Issues related to the @typescript-eslint/utils package label Jun 2, 2024
@abrahamguo abrahamguo linked a pull request Aug 20, 2024 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepting prs Go ahead, send a pull request that resolves this issue package: utils Issues related to the @typescript-eslint/utils package repo maintenance things to do with maintenance of the repo, and not with code/docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants