Properly track first Quick Info declaration + fix issue in fourslash#2641
Properly track first Quick Info declaration + fix issue in fourslash#2641ahejlsberg merged 7 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes two issues related to Quick Info functionality: properly tracking the first declaration when displaying Quick Info, and correcting baseline verification for multi-file fourslash tests.
Changes:
- Refactored declaration tracking in Quick Info to use a guard pattern that captures the first declaration encountered
- Fixed fourslash test infrastructure to use
marker.fileNameinstead off.activeFilenamefor hover requests in multi-file tests - Added a new test case for merged alias Quick Info scenarios
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/ls/hover.go | Refactored writeSymbol to not return a declaration directly; introduced setDeclaration helper that only sets firstDeclaration if it's nil, ensuring the first declaration is properly tracked across recursive calls |
| internal/fourslash/fourslash.go | Fixed bug where hover requests used active filename instead of marker's actual filename, causing incorrect behavior in multi-file tests |
| internal/fourslash/tests/quickInfoMergedAlias_test.go | Added test case covering merged alias scenarios across multiple files |
| testdata/baselines/reference/fourslash/quickInfo/*.baseline | Updated baselines showing fixes - markers that previously had null Quick Info now display proper content |
| flags = symbol.Flags & (ast.SymbolFlagsValue | ast.SymbolFlagsSignature | ast.SymbolFlagsType | ast.SymbolFlagsNamespace) | ||
| } | ||
| if flags == 0 { | ||
| if aliasLevel != 0 || b.Len() != 0 { |
There was a problem hiding this comment.
I get checking the alias level, but why do we need to check if b.Len() != 0 here?
There was a problem hiding this comment.
The check is here such that we reach the subsequent logic only when aliasLevel is zero and nothing has been output yet. In that case we want to do a final sanity check where we output any meaning of the symbol, even if getMeaningFromLocation doesn't indicate it. There are cases where getMeaningFromLocation returns "wrong" results because of errors in the source file, for example during completion (and completion actually invokes Quick Info).
|
You fixed four generated tests, so you should be able to do |
This is a follow-up PR to #2627 that fixes the issue reported here.
The PR also fixes an issue in fourslash that caused baseline verification of Quick Info to not work in multi-file tests.