fix: proper semantic highlighting and formatting for type declarations #1883
Conversation
📝 WalkthroughWalkthroughThe pull request includes updates to the changelog for the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/schema/src/language-server/zmodel-formatter.ts (1)
Line range hint
114-119: Improve error case handlingThe fallback case returning 1 could be improved with:
- A more descriptive comment explaining when this case occurs
- Proper error logging for debugging
} else { - // we shouldn't get here - length = 1; + // Fallback case for unknown field type structure + console.warn(`Unexpected field type structure in ${field.$type}:`, field); + length = 1; // Minimum length to prevent formatting issues }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/ide/jetbrains/CHANGELOG.md(1 hunks)packages/schema/src/language-server/zmodel-formatter.ts(2 hunks)packages/schema/src/language-server/zmodel-semantic.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ide/jetbrains/CHANGELOG.md
🔇 Additional comments (5)
packages/schema/src/language-server/zmodel-semantic.ts (3)
21-22: LGTM: Import additions are well-organized
The new imports follow the established pattern and are correctly placed in alphabetical order.
41-41: LGTM: Semantic highlighting extensions are consistent
The additions of isTypeDef and isTypeDefField to their respective conditional blocks maintain consistency with the existing semantic highlighting patterns:
- Type definitions are highlighted as types (like other declarations)
- Type definition fields are highlighted as variables (like other fields)
Also applies to: 49-49
41-41: Verify complete coverage of type definition nodes
Let's ensure we haven't missed any type definition related nodes that might need semantic highlighting.
Also applies to: 49-49
✅ Verification successful
Based on the search results, I can now generate the final response:
Type definition nodes are comprehensively covered
The semantic highlighting implementation correctly handles all type definition related nodes in the codebase. The search results show:
TypeDefis properly defined as a core AST node type alongside other declaration types likeDataModel,Enum, etc.TypeDefFieldandTypeDefFieldTypeare properly defined for type definition fields and their types- The semantic highlighting code at line 41 includes
isTypeDef(node)in its condition, which matches the AST structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any type definition related nodes in the AST definitions
# that might need semantic highlighting
# Look for type definition related interfaces/types in AST definitions
ast-grep --pattern 'interface $_TypeDef {
$$$
}'
# Look for type definition related visitor methods
ast-grep --pattern 'visit$_TypeDef($_) {
$$$
}'
Length of output: 104
Script:
#!/bin/bash
# Let's try a different approach to find type definition related nodes
# Search for type definition related terms in the codebase
rg -i "typedef|typedefinition" -A 2 -B 2
# Look for AST node type definitions
rg "type.*Node.*=" -A 2
# Search for visitor pattern implementations related to types
rg "visit.*Type" -A 2
Length of output: 99103
packages/schema/src/language-server/zmodel-formatter.ts (2)
29-30: LGTM: Clean extension of formatting logic to TypeDef nodes
The changes symmetrically extend the formatting logic to handle both DataModel and TypeDef nodes, maintaining consistent spacing and alignment across both types.
107-107: Verify TypeDefField structure matches DataModelField
The method now handles both field types uniformly. Let's verify that TypeDefField has the same structure for type information.
✅ Verification successful
Based on the search results, I can now generate the final response. The code shows that TypeDefField has a similar structure to DataModelField, particularly in terms of type information:
TypeDefField structure matches DataModelField for type handling
Both field types share the same type structure with:
- A
typeproperty of typeTypeDefFieldType - Support for array notation with
arrayboolean - Support for optional fields with
optionalboolean - Support for both builtin types and references
- Proper handling of unsupported fields
The implementation in getFieldTypeLength correctly handles both field types by accessing common properties. The error case (returning 1) is appropriate as a fallback for field type length calculation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for TypeDefField definition to verify its structure
ast-grep --pattern 'interface TypeDefField {
$$$
type: $_
$$$
}'
Length of output: 72
Script:
#!/bin/bash
# Let's try a different approach to find TypeDefField definition
rg "TypeDefField" -A 10 -B 2
Length of output: 60490
No description provided.