Change Request: Remove deprecated context pass-through SourceCode methods

[nzakas]

ESLint version

HEAD

What problem do you want to solve?

We have a bunch of deprecated methods on context that pass through to SourceCode:

• getComments()
• • getSource
• getAllComments
• getNodeByRangeIndex
• getComments
• getCommentsBefore
• getCommentsAfter
• getCommentsInside
• getJSDocComment
• getFirstToken
• getFirstTokens
• getLastToken
• getLastTokens
• getTokenAfter
• getTokenBefore
• getTokenByRangeStart
• getTokens
• getTokensAfter
• getTokensBefore
• getTokensBetween

What do you think is the correct solution?

I think it's time to remove these methods to clean up the codebase.

Participation

• I am willing to submit a pull request for this change.

Additional comments

No response

[mdjermanovic]

Sounds good to me.

We could add deprecation warnings in RuleTester in v8.x, and then remove these methods in v9.0.0?

[nzakas]

We don't actually have a way to do that because the context object isn't exposed outside of Linter in any way. So, I think a blog post is probably the best way to let people know this is coming.

[mdjermanovic]

RuleTester already wraps create(context) here:

eslint/lib/rule-tester/rule-tester.js

Lines 561 to 571 in 926a286

	linter.defineRule(ruleName, Object.assign({}, rule, {
	// Create a wrapper rule that freezes the `context` properties.
	create(context) {
	freezeDeeply(context.options);
	freezeDeeply(context.settings);
	freezeDeeply(context.parserOptions);
	return (typeof rule === "function" ? rule : rule.create)(context);
	}
	}));

Perhaps we could insert an object with methods that shadow deprecated ones:

        linter.defineRule(ruleName, Object.assign({}, rule, {

            // Create a wrapper rule that freezes the `context` properties.
            create(context) {
                freezeDeeply(context.options);
                freezeDeeply(context.settings);
                freezeDeeply(context.parserOptions);

                const newContext = Object.freeze(
                    Object.create(
                        context,
                        Object.fromEntries(Object.keys(DEPRECATED_SOURCECODE_PASSTHROUGHS).map(name => [
                            name,
                            {
                                value(...args) {

                                    // emit deprecation warning here

                                    // call the original method                                   
                                    return context[name].call(this, ...args);
                                },
                                enumerable: true
                            }
                        ]))
                    )
                );

                return (typeof rule === "function" ? rule : rule.create)(newContext);
            }
        }));

[nzakas]

Oh nice! I didn't realize we were doing that already (yet another part of the codebase I'm not familiar with). Then we can definitely do that.

[nzakas]

In looking at this, I discovered that the tests for flat-rule-tester.js don't include tests for legacy rule format or missing schemas, and I can't recall if this is intentional. Any idea @bmish?

[mdjermanovic]

It's intentional per #16063 (comment).

I think these warnings can also be implemented just in rule-tester.js.

[nzakas]

In that case, the PR is ready! 🎉

[JoshuaKGoldberg]

Looks like #17527 closed this out?

[mdjermanovic]

Looks like #17527 closed this out?

The remaining task is to remove these methods in v9.

[nzakas]

Working on this.

[sam3k]

2023-11-30 tsc-meeting note: This one will be ready along with the language plugins issue because it's all in the same PR