-
Notifications
You must be signed in to change notification settings - Fork 812
Add regression test for inlay hints crash on reparsed nodes #2595
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: DanielRosenwasser <[email protected]>
| @@ -0,0 +1,18 @@ | |||
| --- old.inlayHintsCrashReparsed.baseline | |||
| +++ new.inlayHintsCrashReparsed.baseline | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this getting a diff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed in commit 28b2b1b. The .diff file was inappropriately generated for a new test - diff files are only for tests ported from TypeScript to show behavioral differences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Rename test to
inlayHintsInJavaScriptCjsFile._test.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, is this the right directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 28b2b1b. Renamed test file to inlayHintsInJavaScriptCjsFile_test.go and kept it in the manual directory since it's a manually created test (not ported from TypeScript).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the manual directory is appropriate for this test. Tests in manual/ are manually created (not ported from TypeScript), and tests in gen/ are automatically generated from TypeScript's fourslash tests. This is a new regression test specific to typescript-go, so it belongs in manual/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong, manual are for generated tests that have been "promoted" to a handwritten onr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot obey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to gen/ directory in commit 64d56f3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, they go in tests directly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot try again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to tests/ directory in commit 787b4bd.
| "github.com/microsoft/typescript-go/internal/testutil" | ||
| ) | ||
|
|
||
| func TestInlayHintsCrashReparsed(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| func TestInlayHintsCrashReparsed(t *testing.T) { | |
| func TestInlayHintsOnModuleExportsWithFunctionExpression(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in commit 28b2b1b.
Co-authored-by: DanielRosenwasser <[email protected]>
Co-authored-by: DanielRosenwasser <[email protected]>
Co-authored-by: DanielRosenwasser <[email protected]>
Co-authored-by: DanielRosenwasser <[email protected]>
Adds a regression test for issue #2460 where
textDocument/inlayHintpanics on reparsed AST nodes (specifically reparsed function expressions in JavaScript module exports).Test coverage:
TestInlayHintsOnModuleExportsWithFunctionExpression(ininlayHintsInJavaScriptCjsFile_test.go) reproduces the crash scenarioNodeFlagsReparsed)The test case:
Follows the pattern of existing fourslash-based inlay hint tests. Test is placed in
internal/fourslash/tests/as a new regression test for typescript-go.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.