Add precedence and printing for JSDoc types #782
Conversation
This is needed to avoid a panic in a test once CommonJS import/exports are available. I've guessed at the right thing to do here but I don't have a solid idea. I haven't added tests because I'm not sure how: - Submodule tests only cover `*` - `*`, `!T` and `?T` are testable outside JSDoc. - `...T` and `T=` are only parsed in JSDoc.
There was a problem hiding this comment.
Pull Request Overview
This PR updates the handling of JSDoc types in the printer and AST precedence modules to avoid panics and enable correct printing behaviors as CommonJS import/exports become available.
- Implements dedicated printer functions for various JSDoc types.
- Introduces a new precedence level for JSDoc optional and variadic types and updates the precedence grouping for additional JSDoc types.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/printer/printer.go | Added new functions for printing JSDoc types and updated type logic. |
| internal/ast/precedence.go | Updated precedence rules for JSDoc types to support new printer logic. |
Comments suppressed due to low confidence (1)
internal/printer/printer.go:2323
- New JSDoc type printer functions are introduced without associated tests. Consider adding tests to verify correct punctuation and type emission to prevent regressions.
func (p *Printer) emitJSDocAllType(node *ast.Node) {
| switch n.Kind { | ||
| case KindConditionalType: | ||
| return TypePrecedenceConditional | ||
| case KindJSDocOptionalType, KindJSDocVariadicType: |
There was a problem hiding this comment.
The precedence grouping for JSDoc types appears inconsistent: some types use TypePrecedenceJSDoc while others fall into a different group. Review if all JSDoc types should share the same precedence level or if the differences are intentional and add clarifying comments.
| case KindJSDocOptionalType, KindJSDocVariadicType: | |
| case KindJSDocOptionalType, KindJSDocVariadicType, | |
| KindJSDocAllType, KindJSDocNullableType, KindJSDocNonNullableType: | |
| // All JSDoc-related types are grouped under TypePrecedenceJSDoc for consistency. |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Only Optional and Variadic are special-cased in parsing. The others are parsed just like any, unknown, etc. This is intentional.
3 participants
This is needed to avoid a panic in a test once CommonJS import/exports are available.
I've guessed at the right thing to do here but I don't have a solid idea.
I haven't added tests because I'm not sure how:
**,!Tand?Tare testable outside JSDoc....TandT=are only parsed in JSDoc.