mirror of
https://github.com/vector-im/element-call.git
synced 2026-08-29 21:15:19 +00:00
Fix some false positives flagged by ObservableScope leak lint rule
The rule should only care about enclosing function/class scopes. For example if an ObservableScope is received as a parameter to a function and then simply used inside an 'if' block (technically a different scope), that's not a problem.
This commit is contained in:
@@ -9,7 +9,21 @@ import { ESLintUtils } from "@typescript-eslint/utils";
|
|||||||
|
|
||||||
// These ObservableScope methods will not generally cause resource leaks even if
|
// These ObservableScope methods will not generally cause resource leaks even if
|
||||||
// called from a callback
|
// called from a callback
|
||||||
const safeScopeMethods = ["bind", "end"];
|
const safeScopeMethods = new Set(["bind", "end"]);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Determines whether the variable with the given name is local to
|
||||||
|
* the enclosing function or class scope.
|
||||||
|
*/
|
||||||
|
function isLocal(name, scope) {
|
||||||
|
// If it is nowhere to be found in the "through" scope, it is local.
|
||||||
|
if (!scope.through.some(({ identifier }) => identifier.name === name))
|
||||||
|
return true;
|
||||||
|
if (scope.type === "function" || scope.type === "class") return false;
|
||||||
|
// If this is something other than a function or class scope, check its outer
|
||||||
|
// scope.
|
||||||
|
return !scope.upper || isLocal(name, scope.upper);
|
||||||
|
}
|
||||||
|
|
||||||
const rule = ESLintUtils.RuleCreator(
|
const rule = ESLintUtils.RuleCreator(
|
||||||
() => "https://github.com/element-hq/element-call",
|
() => "https://github.com/element-hq/element-call",
|
||||||
@@ -32,15 +46,13 @@ const rule = ESLintUtils.RuleCreator(
|
|||||||
Identifier(node) {
|
Identifier(node) {
|
||||||
const scope = context.sourceCode.getScope(node);
|
const scope = context.sourceCode.getScope(node);
|
||||||
if (
|
if (
|
||||||
// Is this a reference to a variable defined in an outer ("through") scope?
|
// Is this a reference to a variable defined in an outer scope?
|
||||||
scope.through.some(
|
!isLocal(node.name, scope) &&
|
||||||
({ identifier }) => identifier.name === node.name,
|
|
||||||
) &&
|
|
||||||
// Exclude calls to "safe" ObservableScope methods
|
// Exclude calls to "safe" ObservableScope methods
|
||||||
node.parent?.type === "MemberExpression" &&
|
node.parent?.type === "MemberExpression" &&
|
||||||
node.parent.object === node &&
|
node.parent.object === node &&
|
||||||
node.parent.property.type === "Identifier" &&
|
node.parent.property.type === "Identifier" &&
|
||||||
!safeScopeMethods.includes(node.parent.property.name) &&
|
!safeScopeMethods.has(node.parent.property.name) &&
|
||||||
/(^s|S)cope$/.test(node.name)
|
/(^s|S)cope$/.test(node.name)
|
||||||
) {
|
) {
|
||||||
// TODO: Once oxlint supports lint rules that rely on TypeScript type-awareness,
|
// TODO: Once oxlint supports lint rules that rely on TypeScript type-awareness,
|
||||||
|
|||||||
Reference in New Issue
Block a user