Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[rule-tester] support multipass fixes #8554

Open
4 tasks done
bradzacher opened this issue Feb 26, 2024 · 2 comments · May be fixed by #8883
Open
4 tasks done

[rule-tester] support multipass fixes #8554

bradzacher opened this issue Feb 26, 2024 · 2 comments · May be fixed by #8883
Labels
accepting prs Go ahead, send a pull request that resolves this issue breaking change This change will require a new major version to be released enhancement New feature or request package: rule-tester Issues related to the @typescript-eslint/rule-tester package
Milestone
8.0.0

Comments

@bradzacher
Copy link
Member

Before You File a Proposal Please Confirm You Have Done The Following...

Relevant Package

utils

My proposal is suitable for this project

  • I believe my proposal would be useful to the broader TypeScript community (meaning it is not a niche proposal).

Description

eslint/eslint#18007 was rejected as a breaking change enhancement.

Given this impacts us directly (eg #8251) we should build this into our fork.

I propose the following changes:

  1. if the test case has output and the output value is a string and the test case requires more than 1 pass to fix then the test case fails with a message like "test requires multiple fixes to resolve due to overlapping fix ranges - please use the array form of output to declare all of the expected fix passes".
  2. accept an array for output which declares the expected fix passes for a test case.
    • An array of size 0 is disallowed.
    • An array of size 1 is equivalent to the string form.
    • An array of size n asserts that there are exactly n fix passes
    • The rule tester asserts that after each fix pass the output matches the nth element of the array.

Additional Info

No response
@bradzacher bradzacher added enhancement New feature or request breaking change This change will require a new major version to be released accepting prs Go ahead, send a pull request that resolves this issue package: rule-tester Issues related to the @typescript-eslint/rule-tester package labels Feb 26, 2024
@bradzacher bradzacher added this to the 8.0.0 milestone Feb 26, 2024
@auvred auvred mentioned this issue Mar 9, 2024
Closed
3 tasks
@StyleShit
Copy link
Contributor

Why would a rule developer care whether there should be multiple passes or a single pass?
I would assume that a developer would only care about the final output, no?

I mean... knowing the passes count is more of an implementation detail in this case IMO

WDYT?

@bradzacher
Copy link
Member Author

For me it's a debugging thing.
I remember we had some bugs with one of our rules (array-type) around how multipass fixes worked and we had to setup separate tests to help us debug and validate it.

It's about being explicit and acknowledging that the user will have two fixes applied because your rule defines overlapping ranges. This may be an important thing for perf reasons.

For example imagine switching out Array<Array<T>> for T[][]. One fix solution would be:

// fix one
  Array<Array<T>>
//^^^^^^^^^^^^^^^ replace with Array<T>[]

// fix two
  Array<Array<T>>
//      ^^^^^^^^ replace with T[]

But that solution is heavy-handed and has overlapping fix ranges so will always require two fix passes.

One might not consider this problem if they don't think about the fact that the test has overlapping fixes.

@auvred auvred mentioned this issue Mar 30, 2024
Draft
3 tasks
@auvred auvred linked a pull request Apr 8, 2024 that will close this issue
Open
3 tasks
@kirkwaiblinger kirkwaiblinger mentioned this issue May 1, 2024
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
accepting prs Go ahead, send a pull request that resolves this issue breaking change This change will require a new major version to be released enhancement New feature or request package: rule-tester Issues related to the @typescript-eslint/rule-tester package
Projects
None yet
Milestone
8.0.0
2 participants
@bradzacher @StyleShit