feat: discourage monkey patching

[saberzero1]

Summary

closes #156

Adds a no-monkey-patching rule that discourages directly modifying prototypes, and bans the monkey-around package via no-restricted-imports.

Problem

Monkey patching Obsidian internals is a common source of breakage between plugins and across Obsidian updates. Plugins use the monkey-around package or directly assign to class prototypes to intercept core behavior. There's currently no lint rule to flag this.

Changes

New rule: no-monkey-patching (lib/rules/noMonkeyPatching.ts)

• Flags direct prototype assignment: Workspace.prototype.method = ...
• Flags Object.defineProperty(X.prototype, ...) and Object.defineProperties(X.prototype, ...)
• Flags Object.assign(X.prototype, ...)
• Does not flag non-prototype usage: Object.assign(target, source), Object.defineProperty(obj, ...), Object.create(Parent.prototype)

Banned dependency (lib/ruleOptions.ts)

• Added monkey-around to restrictedImportsOptions, handled by the existing no-restricted-imports rule alongside axios, got, etc.

Registration (lib/index.ts)

• Added import and rule registration under no-monkey-patching
• Added to recommendedPluginRulesConfig at error

Documentation

• Added docs/rules/no-monkey-patching.md

Tests

• Added tests/noMonkeyPatching.test.ts; 8 valid and 7 invalid cases covering all prototype mutation patterns

What changes for consumers

The no-monkey-patching rule is now included in recommended and recommendedWithLocalesEn at error severity. Plugins directly modifying prototypes will see errors. Plugins importing monkey-around will see an error from no-restricted-imports.

[liamcain]

Suggested change

	"Monkey patching Obsidian internals is discouraged.",
	"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.",

[tgrosinger]

What do you think about these other prototyping patterns that Claude suggested? Glancing over them quickly they seem reasonable, especially number 2.

High value — common patterns currently missed

1. Patching via Object.getPrototypeOf(...)

This is arguably the most common hand-rolled technique in Obsidian, because plugins usually hold an instance (this.app.workspace) rather than a class reference, so they reach the prototype dynamically:
Object.getPrototypeOf(this.app.workspace).getLeaf = function () { … };

Detect: AssignmentExpression where left.object is a CallExpression to Object.getPrototypeOf (or Reflect.getPrototypeOf). The current rule catches none of this.

2. Instance method patching (the big one — needs type info)

The dominant monkey-patch style isn't on prototypes at all; it's overwriting a method on a live Obsidian object:
this.app.workspace.getLeaf = function () { … };

Banning the monkey-around import covers the library path, but hand-rolled instance patching slips through entirely. You can't catch this safely without type information — flagging every obj.method = fn would be far too noisy. The principled version is to move the rule into recommendedTypedRulesConfig and use parser services to flag assignment to a method only when the receiver's type resolves to an obsidian export (Workspace, App, MetadataCache, etc.). That's the highest-impact change but also the largest; worth treating as a follow-up rather than folding into this PR.

Quick wins — close trivial bypasses cheaply

3. Reflect.* equivalents of the Object.* checks

Reflect.defineProperty(X.prototype, …) and Reflect.set(X.prototype, …) bypass the Object.defineProperty/assign branches. Adding Reflect as an accepted callee object in the same check is nearly free.

4. Computed prototype access on the object side

X['prototype'].foo = … is missed because isPrototypeAccess only matches an Identifier named prototype, not a string Literal. (Note: X.prototype['foo'] = … is already caught — only the ['prototype'] side leaks.) One-line fix: also accept Literal with value "prototype".

5. Object.setPrototypeOf(X.prototype, …) and delete X.prototype.method

Replacing or deleting prototype members is the same anti-pattern as adding to it. delete is a UnaryExpression (operator === "delete") over a prototype member; setPrototypeOf fits the existing Object.* call check. Lower frequency, but completes the "no prototype mutation" story coherently.

[tgrosinger]

isPrototypeAccess and isPrototypeMemberExpression are very similar (one is a subset of the other. Do we need both? Could we consolidate them?

[tgrosinger]

These three expressions are nearly identical. Can we combine them? Claude has this suggestion:

They collapse cleanly into one:

const PROTO_METHODS = { defineProperty: "definePropertyOnPrototype",
                        defineProperties: "definePropertyOnPrototype",
                        assign: "assignToPrototype" } as const;

then a single guard checking node.callee.property.name in PROTO_METHODS. Cuts ~50 lines and removes the inconsistent arguments.length >= 2 vs >= 1 checks (those bounds are loose anyway and don't affect correctness).

[liamcain]

The dominant monkey-patch style isn't on prototypes at all; it's overwriting a method on a live Obsidian object:

I see the value, I think that should be a separate PR and a separate rule entirely. If we have good signal on it, we should make it an error, not a warning. no-obsidian-internals-assignment or something

[tgrosinger]

npm run check:eslint-docs is broken on this branch currently.

[zsviczian]

I agree that unmanaged monkey patching should be discouraged.

One distinction I think is worth preserving is lifecycle-managed, reversible patching. Excalidraw uses monkey-around in a few cases, but registers every returned disposer with the plugin lifecycle, preserves the original function call, and applies only narrow, targeted extensions.

this.plugin.register(
  around(Workspace.prototype, {
    ...
  }),
);

This means the original behavior is restored automatically when the plugin unloads, and behavior is altered only for narrowly defined plugin features.

The proposed blanket monkey-around import ban at error severity treats this the same as an unmanaged prototype mutation.

Could the rule remain strict for direct or untracked patching, while allowing a narrowly documented exception, or possibly recognizing the plugin.register(around(...)) lifecycle-managed pattern?

As I also commented in #156, if there are legitimate, lifecycle-safe use cases, and I am confident Excalidraw's uses fall into that category, I think the rule should provide guardrails that allow those cases while still strongly discouraging unsafe monkey patching, rather than treating all uses as equivalent.