Improve testing of code fixes, and improve diagnostic messages #18742
Conversation
ghost
src/harness/fourslash.ts
Outdated
| this.applyEdits(change.fileName, change.textChanges, /*isFormattingEdit*/ false); | ||
| } | ||
|
|
||
| if (this.getRanges().length === 0) { |
There was a problem hiding this comment.
It might be easier to understand the tests if we are uniform in requiring a range to be specified, rather than switching the behavior in a way that forces the user to navigate back to this line of the source.
We could alternately come up with a different name for newContent that does capture this switching behavior.
I personally favor the first option, but I'm open to alternatives.
| @@ -2337,6 +2337,38 @@ namespace FourSlash { | |||
| } | |||
| } | |||
|
|
|||
| public verifyCodeFix(options: FourSlashInterface.VerifyCodeFixOptions) { | |||
There was a problem hiding this comment.
Consider merging this into rangeAfterCodeFix (possibly in a separate PR) to keep the 4/ API simple.
There was a problem hiding this comment.
I'm planning to remove all rangeAfterCodeFix in another PR.
| @@ -4359,4 +4395,11 @@ namespace FourSlashInterface { | |||
| export interface CompletionsAtOptions { | |||
| isNewIdentifierLocation?: boolean; | |||
| } | |||
|
|
|||
| export interface VerifyCodeFixOptions { | |||
| description: string; | |||
There was a problem hiding this comment.
Should description and newContent be optional as well?
There was a problem hiding this comment.
Not if we want to enforce that these are tested.
ghost
mentioned this pull request
We were reporting fixes to add instance methods as static, and vice versa.
Also added a diagnostic message for adding static properties.
Since this resulted in no test changes, I added functionality to assert the description of code fixes; haven't applied it to every test yet but I think that would be a good idea since we've caught some bugs already.
The new tester no longer ignores whitespace, which uncovered #18741 and #18743 (and also more #18445 errors).