Preserve @as decorator on record fields when creating interface #7779
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the @as decorator on record fields was not being preserved when creating interface files. The change ensures that type declarations with attributes are copied verbatim to maintain their decorators.
- Added logic to detect and preserve attributes (like
@as) on type declarations - Enhanced the AttributesUtils module with an
isEmptyfunction - Updated the interface creation process to copy type declarations with attributes verbatim
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| analysis/src/CreateInterface.ml | Core logic to detect attributes and preserve type declarations verbatim |
| tests/analysis_tests/tests/src/CreateInterface.res | Test case with external function and record type using @as decorator |
| tests/analysis_tests/tests/src/expected/CreateInterface.res.txt | Expected output showing preserved @as decorators |
| CHANGELOG.md | Documentation of the bug fix |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
analysis/src/CreateInterface.ml
Outdated
| sigItemToString | ||
| (Printtyp.tree_of_type_declaration id typeDecl resStatus) | ||
| in | ||
| Buffer.add_string buf (indent ^ newItemStr ^ "\n")); |
There was a problem hiding this comment.
[nitpick] The conditional logic should be inverted to reduce nesting. Consider using early return pattern: if AttributesUtils.isEmpty attributes then (* normal processing *) else (* verbatim copy *).
Copilot uses AI. Check for mistakes.
analysis/src/CreateInterface.ml
Outdated
|
|
||
| (if not (AttributesUtils.isEmpty attributes) then | ||
| (* Copy the type declaration verbatim to preserve attributes *) | ||
| Buffer.add_string buf ((lines |> String.concat "\n") ^ "\n") |
There was a problem hiding this comment.
String concatenation with ^ creates intermediate strings. Consider using Buffer.add_string for each line separately or String.concat "\n" without the additional concatenation.
| Buffer.add_string buf ((lines |> String.concat "\n") ^ "\n") | |
| Array.iter (fun line -> Buffer.add_string buf line; Buffer.add_char buf '\n') lines |
Copilot uses AI. Check for mistakes.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
analysis/src/CreateInterface.ml
Outdated
| let attributes = AttributesUtils.make lines in | ||
|
|
||
| (if not (AttributesUtils.isEmpty attributes) then | ||
| (* Copy the type declaration verbatim to preserve attributes *) |
There was a problem hiding this comment.
Does this work in the else branch too? Can we always copy verbatim? Or what's the tradeoff here?
There was a problem hiding this comment.
I've removed the else branch since every type declaration should be valid in an interface file when copied verbatim 👍 The only tradeoff I can think of is that we'll be copying any non-doc comments within the declaration
Closes #7611