diff --git a/README.md b/README.md index 51f6d36..92d6a00 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ You can also override or add rules: 🇬🇧 Set in the `recommendedWithLocalesEn` configuration.\ 🔧 Automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/user-guide/command-line-interface#--fix). -| Name                                          | Description | 💼 | ⚠️ | 🔧 | +| Name | Description | 💼 | ⚠️ | 🔧 | | :----------------------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------------------------------- | :----- | :----- | :- | | [commands/no-command-in-command-id](docs/rules/commands/no-command-in-command-id.md) | Disallow using the word 'command' in a command ID. | ✅ 🇬🇧 | | | | [commands/no-command-in-command-name](docs/rules/commands/no-command-in-command-name.md) | Disallow using the word 'command' in a command name. | ✅ 🇬🇧 | | | @@ -111,6 +111,7 @@ You can also override or add rules: | [hardcoded-config-path](docs/rules/hardcoded-config-path.md) | Disallow hardcoded `.obsidian` config paths. Use `Vault#configDir` instead. | ✅ 🇬🇧 | | | | [no-forbidden-elements](docs/rules/no-forbidden-elements.md) | Disallow attachment of forbidden elements to the DOM in Obsidian plugins. | ✅ 🇬🇧 | | | | [no-global-this](docs/rules/no-global-this.md) | Disallow `global` and `globalThis`. Use `window` or `activeWindow` for popout window compatibility. | ✅ 🇬🇧 | | 🔧 | +| [no-monkey-patching](docs/rules/no-monkey-patching.md) | Discourage directly modifying prototypes. | ✅ 🇬🇧 | | | | [no-nodejs-modules](docs/rules/no-nodejs-modules.md) | Disallow importing Node.js built-in modules unless guarded by Platform.isDesktop | ✅ 🇬🇧 | | | | [no-plugin-as-component](docs/rules/no-plugin-as-component.md) | Disallow anti-patterns when passing a component to MarkdownRenderer.render to prevent memory leaks. | ✅ 🇬🇧 | | | | [no-sample-code](docs/rules/no-sample-code.md) | Disallow sample code snippets from the Obsidian plugin template. | ✅ 🇬🇧 | | 🔧 | diff --git a/docs/rules/no-monkey-patching.md b/docs/rules/no-monkey-patching.md new file mode 100644 index 0000000..e15364b --- /dev/null +++ b/docs/rules/no-monkey-patching.md @@ -0,0 +1,7 @@ +# obsidianmd/no-monkey-patching + +📝 Discourage directly modifying prototypes. + +💼 This rule is enabled in the following configs: ✅ `recommended`, 🇬🇧 `recommendedWithLocalesEn`. + + diff --git a/lib/index.ts b/lib/index.ts index dd0cc3d..9640a0a 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -25,6 +25,7 @@ import sampleNames from "./rules/sampleNames.js"; import validateManifest from "./rules/validateManifest.js"; import validateLicense from "./rules/validateLicense.js"; import ruleCustomMessage from "./rules/ruleCustomMessage.js"; +import noMonkeyPatching from "./rules/noMonkeyPatching.js"; import noNodejsModules from "./rules/noNodejsModules.js"; import noUnsupportedApi from "./rules/noUnsupportedApi.js"; import { getManifest } from "./manifest.js"; @@ -85,6 +86,7 @@ const plugin = { "no-sample-code": noSampleCode, "no-tfile-tfolder-cast": noTFileTFolderCast, "no-view-references-in-plugin": noViewReferencesInPlugin, + "no-monkey-patching": noMonkeyPatching, "no-static-styles-assignment": noStaticStylesAssignment, "object-assign": objectAssign, platform: platform, @@ -142,6 +144,7 @@ const recommendedPluginRulesConfig: RulesConfig = { "obsidianmd/no-plugin-as-component": "error", "obsidianmd/no-sample-code": "error", "obsidianmd/no-tfile-tfolder-cast": "error", + "obsidianmd/no-monkey-patching": "error", "obsidianmd/no-static-styles-assignment": "error", "obsidianmd/object-assign": "error", "obsidianmd/platform": "error", diff --git a/lib/ruleOptions.ts b/lib/ruleOptions.ts index 76a2309..7af90a0 100644 --- a/lib/ruleOptions.ts +++ b/lib/ruleOptions.ts @@ -52,6 +52,11 @@ export const restrictedImportsOptions = [ message: "The 'moment' package is bundled with Obsidian. Please import it from 'obsidian' instead.", }, + { + name: "monkey-around", + message: + "This plugin may be modifying Obsidian internals. Plugins that do this have a higher likelihood to introduce unexpected behavior in the app. They are also more likely to break when Obsidian updates.", + }, ] as const; export const noUnusedExpressionsOptions = [{ allowShortCircuit: true, allowTernary: true }] as const; \ No newline at end of file diff --git a/lib/rules/noMonkeyPatching.ts b/lib/rules/noMonkeyPatching.ts new file mode 100644 index 0000000..56c4bde --- /dev/null +++ b/lib/rules/noMonkeyPatching.ts @@ -0,0 +1,175 @@ +import { TSESTree, ESLintUtils } from "@typescript-eslint/utils"; + +const ruleCreator = ESLintUtils.RuleCreator( + (name) => + `https://github.com/obsidianmd/eslint-plugin/blob/master/docs/rules/${name}.md`, +); + +function isPrototypeAccess(node: TSESTree.Node): node is TSESTree.MemberExpression { + if (node.type !== TSESTree.AST_NODE_TYPES.MemberExpression) { + return false; + } + if ( + node.property.type === TSESTree.AST_NODE_TYPES.Identifier && + node.property.name === "prototype" + ) { + return true; + } + if ( + node.computed && + node.property.type === TSESTree.AST_NODE_TYPES.Literal && + node.property.value === "prototype" + ) { + return true; + } + return false; +} + +function isPrototypeMemberAccess(node: TSESTree.MemberExpression): boolean { + return isPrototypeAccess(node.object); +} + +function getMemberName(node: TSESTree.MemberExpression): string { + const parts: string[] = []; + let current: TSESTree.Expression = node; + while (current.type === TSESTree.AST_NODE_TYPES.MemberExpression) { + const prop = current.property; + if (prop.type === TSESTree.AST_NODE_TYPES.Identifier) { + parts.unshift(prop.name); + } else if (prop.type === TSESTree.AST_NODE_TYPES.Literal) { + parts.unshift(String(prop.value)); + } else { + parts.unshift("[computed]"); + } + current = current.object; + } + if (current.type === TSESTree.AST_NODE_TYPES.Identifier) { + parts.unshift(current.name); + } + return parts.join("."); +} + +const PROTO_METHODS = { + defineProperty: "definePropertyOnPrototype", + defineProperties: "definePropertyOnPrototype", + assign: "assignToPrototype", + set: "assignToPrototype", +} as const; + +function isGetPrototypeOfCall(node: TSESTree.Expression): boolean { + return ( + node.type === TSESTree.AST_NODE_TYPES.CallExpression && + node.callee.type === TSESTree.AST_NODE_TYPES.MemberExpression && + node.callee.object.type === TSESTree.AST_NODE_TYPES.Identifier && + (node.callee.object.name === "Object" || node.callee.object.name === "Reflect") && + node.callee.property.type === TSESTree.AST_NODE_TYPES.Identifier && + node.callee.property.name === "getPrototypeOf" + ); +} + +export default ruleCreator({ + name: "no-monkey-patching", + meta: { + type: "problem" as const, + docs: { + description: + "Discourage directly modifying prototypes.", + }, + schema: [], + messages: { + directPrototypeAssignment: + "Do not assign to `{{name}}`. Directly modifying prototypes is unsafe and discouraged.", + definePropertyOnPrototype: + "Do not use `Object.defineProperty` on `{{name}}`. Directly modifying prototypes is unsafe and discouraged.", + assignToPrototype: + "Do not use `Object.assign` on `{{name}}`. Directly modifying prototypes is unsafe and discouraged.", + setPrototypeOf: + "Do not use `Object.setPrototypeOf` on a prototype. Directly modifying prototypes is unsafe and discouraged.", + deletePrototypeMember: + "Do not delete `{{name}}`. Directly modifying prototypes is unsafe and discouraged.", + getPrototypeOfAssignment: + "Do not assign to a member of `Object.getPrototypeOf(...)`. Directly modifying prototypes is unsafe and discouraged.", + }, + }, + defaultOptions: [], + create(context) { + return { + CallExpression(node: TSESTree.CallExpression) { + if ( + node.callee.type !== TSESTree.AST_NODE_TYPES.MemberExpression || + node.callee.object.type !== TSESTree.AST_NODE_TYPES.Identifier || + node.callee.property.type !== TSESTree.AST_NODE_TYPES.Identifier + ) { + return; + } + + const objectName = node.callee.object.name; + const methodName = node.callee.property.name; + + if ( + (objectName === "Object" || objectName === "Reflect") && + methodName === "setPrototypeOf" && + node.arguments.length >= 1 && + isPrototypeAccess(node.arguments[0]) + ) { + context.report({ + node, + messageId: "setPrototypeOf", + }); + return; + } + + if ( + (objectName === "Object" || objectName === "Reflect") && + methodName in PROTO_METHODS && + node.arguments.length >= 1 && + isPrototypeAccess(node.arguments[0]) + ) { + const target = node.arguments[0] as TSESTree.MemberExpression; + context.report({ + node, + messageId: PROTO_METHODS[methodName as keyof typeof PROTO_METHODS], + data: { name: getMemberName(target) }, + }); + return; + } + }, + + AssignmentExpression(node: TSESTree.AssignmentExpression) { + if (node.left.type !== TSESTree.AST_NODE_TYPES.MemberExpression) { + return; + } + + if (isGetPrototypeOfCall(node.left.object)) { + context.report({ + node, + messageId: "getPrototypeOfAssignment", + }); + return; + } + + if (isPrototypeMemberAccess(node.left) || isPrototypeAccess(node.left)) { + context.report({ + node, + messageId: "directPrototypeAssignment", + data: { name: getMemberName(node.left) }, + }); + } + }, + + UnaryExpression(node: TSESTree.UnaryExpression) { + if ( + node.operator === "delete" && + node.argument.type === TSESTree.AST_NODE_TYPES.MemberExpression && + isPrototypeMemberAccess(node.argument) + ) { + context.report({ + node, + messageId: "deletePrototypeMember", + data: { name: getMemberName(node.argument) }, + }); + } + }, + }; + }, +}); diff --git a/tests/noMonkeyPatching.test.ts b/tests/noMonkeyPatching.test.ts new file mode 100644 index 0000000..038357f --- /dev/null +++ b/tests/noMonkeyPatching.test.ts @@ -0,0 +1,139 @@ +import { RuleTester } from "@typescript-eslint/rule-tester"; +import noMonkeyPatchingRule from "../lib/rules/noMonkeyPatching.js"; + +const ruleTester = new RuleTester(); + +ruleTester.run("no-monkey-patching", noMonkeyPatchingRule, { + valid: [ + { + name: "normal import is allowed", + code: "import { Plugin } from 'obsidian';", + }, + { + name: "normal require is allowed", + code: "const obsidian = require('obsidian');", + }, + { + name: "assigning to own class prototype is allowed via normal property", + code: "const obj = {}; obj.foo = 'bar';", + }, + { + name: "Object.assign with plain objects is allowed", + code: "Object.assign(target, source);", + }, + { + name: "Object.defineProperty on plain object is allowed", + code: "Object.defineProperty(obj, 'key', { value: 42 });", + }, + { + name: "accessing prototype without assignment is allowed", + code: "const proto = Array.prototype;", + }, + { + name: "Object.getPrototypeOf without assignment is allowed", + code: "const x = Object.getPrototypeOf(obj);", + }, + { + name: "Object.create with prototype is allowed", + code: "const child = Object.create(Parent.prototype);", + }, + { + name: "Reflect.defineProperty on plain object is allowed", + code: "Reflect.defineProperty(obj, 'key', { value: 42 });", + }, + { + name: "Reflect.set on plain object is allowed", + code: "Reflect.set(target, 'key', value);", + }, + { + name: "delete on non-prototype member is allowed", + code: "delete obj.foo;", + }, + { + name: "Object.setPrototypeOf on non-prototype target is allowed", + code: "Object.setPrototypeOf(obj, proto);", + }, + ], + invalid: [ + { + name: "direct prototype method assignment is forbidden", + code: "Workspace.prototype.getActiveViewOfType = function() { return null; };", + errors: [{ messageId: "directPrototypeAssignment", data: { name: "Workspace.prototype.getActiveViewOfType" } }], + }, + { + name: "Array.prototype assignment is forbidden", + code: "Array.prototype.customMethod = function() {};", + errors: [{ messageId: "directPrototypeAssignment", data: { name: "Array.prototype.customMethod" } }], + }, + { + name: "Object.prototype assignment is forbidden", + code: "Object.prototype.foo = 'bar';", + errors: [{ messageId: "directPrototypeAssignment", data: { name: "Object.prototype.foo" } }], + }, + { + name: "assigning to .prototype itself is forbidden", + code: "MyClass.prototype = {};", + errors: [{ messageId: "directPrototypeAssignment", data: { name: "MyClass.prototype" } }], + }, + { + name: "Object.defineProperty on prototype is forbidden", + code: "Object.defineProperty(Workspace.prototype, 'method', { value: function() {} });", + errors: [{ messageId: "definePropertyOnPrototype", data: { name: "Workspace.prototype" } }], + }, + { + name: "Object.defineProperties on prototype is forbidden", + code: "Object.defineProperties(Array.prototype, { custom: { value: 1 } });", + errors: [{ messageId: "definePropertyOnPrototype", data: { name: "Array.prototype" } }], + }, + { + name: "Object.assign on prototype is forbidden", + code: "Object.assign(Element.prototype, { customMethod() {} });", + errors: [{ messageId: "assignToPrototype", data: { name: "Element.prototype" } }], + }, + { + name: "Reflect.defineProperty on prototype is forbidden", + code: "Reflect.defineProperty(Workspace.prototype, 'method', { value: function() {} });", + errors: [{ messageId: "definePropertyOnPrototype", data: { name: "Workspace.prototype" } }], + }, + { + name: "Reflect.set on prototype is forbidden", + code: "Reflect.set(Array.prototype, 'customMethod', function() {});", + errors: [{ messageId: "assignToPrototype", data: { name: "Array.prototype" } }], + }, + { + name: "computed prototype access via bracket notation is forbidden", + code: "Workspace['prototype'].getLeaf = function() {};", + errors: [{ messageId: "directPrototypeAssignment", data: { name: "Workspace.prototype.getLeaf" } }], + }, + { + name: "Object.defineProperty with computed prototype access is forbidden", + code: "Object.defineProperty(Workspace['prototype'], 'method', { value: function() {} });", + errors: [{ messageId: "definePropertyOnPrototype", data: { name: "Workspace.prototype" } }], + }, + { + name: "Object.getPrototypeOf assignment is forbidden", + code: "Object.getPrototypeOf(workspace).getLeaf = function() {};", + errors: [{ messageId: "getPrototypeOfAssignment" }], + }, + { + name: "Reflect.getPrototypeOf assignment is forbidden", + code: "Reflect.getPrototypeOf(workspace).getLeaf = function() {};", + errors: [{ messageId: "getPrototypeOfAssignment" }], + }, + { + name: "Object.setPrototypeOf on prototype is forbidden", + code: "Object.setPrototypeOf(MyClass.prototype, OtherClass.prototype);", + errors: [{ messageId: "setPrototypeOf" }], + }, + { + name: "Reflect.setPrototypeOf on prototype is forbidden", + code: "Reflect.setPrototypeOf(MyClass.prototype, OtherClass.prototype);", + errors: [{ messageId: "setPrototypeOf" }], + }, + { + name: "delete on prototype member is forbidden", + code: "delete Workspace.prototype.getLeaf;", + errors: [{ messageId: "deletePrototypeMember", data: { name: "Workspace.prototype.getLeaf" } }], + }, + ], +});