Skip to content

feat(linter): add support for vitest/valid-title rule #12085

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

taearls
Copy link
Contributor

@taearls taearls commented Jul 5, 2025

I noticed that there's already a fully working implementation of jest/valid-title, and the logic is very similar. However, the vitest rule has slightly different configuration options, so it's not quite the same. I made the vitest/valid-title rule work with their specific configuration options and unit tests.

As part of this, I added support for the vitest typecheck option so that all the vitest-specific test scenarios would pass for this rule. This meant adding a VitestPluginSettings module with typecheck available as a boolean, defaulting to false.

I considered trying to reuse some of the logic from the jest rule equivalent, but since there are subtle differences I didn't think the refactor was worth it on a first pass. The only tradeoff is that there's some duplicate logic between the jest / vitest rules for this.

Future consideration: The only other rule that uses typecheck as a config setting in eslint-plugin-vitest is the expect-expect rule, which we currently have listed in the VITEST_COMPATIBLE_JEST_RULES. It might be worth adding support for typecheck in that rule for 100% compatibility.


Reference:

Vitest Config Options from vitest/valid_title.rs):

interface Options {
    ignoreTypeOfDescribeName?: boolean;
    allowArguments?: boolean;
    disallowedWords?: string[];
    mustNotMatch?: Partial<Record<'describe' | 'test' | 'it', string>> | string;
    mustMatch?: Partial<Record<'describe' | 'test' | 'it', string>> | string;
}

Jest Config Options (from jest/valid_title.rs):

interface Options {
    ignoreSpaces?: boolean;
    ignoreTypeOfTestName?: boolean;
    ignoreTypeOfDescribeName?: boolean;
    disallowedWords?: string[];
    mustNotMatch?: Partial<Record<'describe' | 'test' | 'it', string>> | string;
    mustMatch?: Partial<Record<'describe' | 'test' | 'it', string>> | string;
}

Vitest valid-title Rule Docs
Jest valid-title Rule Docs

@taearls taearls requested a review from camc314 as a code owner July 5, 2025 18:18
@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI C-enhancement Category - New feature or request labels Jul 5, 2025
Copy link

codspeed-hq bot commented Jul 5, 2025

CodSpeed Instrumentation Performance Report

Merging #12085 will not alter performance

Comparing taearls:linter/vitest-valid-title (461e23f) with main (d732e85)

Summary

✅ 38 untouched benchmarks

@@ -0,0 +1,929 @@
use std::hash::Hash;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR, jest/vitest rules are mapped together as theyua re often so similar. Assuming there isn't a lot of extra work, could you move the test cases from this file into the jest.valid_title.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Would it also make sense to combine these rules into one file, merging the configurations? Or should I just move the tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cli Area - CLI A-linter Area - Linter C-enhancement Category - New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants