Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new MCP server for Sails IDL and program development, including tools for IDL parsing, ABI encoding/decoding, and resource management. My review identified two high-severity issues: the attribute checking logic in abi-tools.ts is prone to false positives due to the use of includes(), and the type resolution logic in type-tools.ts is overly complex and potentially incorrect, which should be replaced by leveraging the existing TypeResolver.
| function isPayable(func: IServiceFunc): boolean { | ||
| return func.docs?.some((d) => d.includes('#[payable]')) ?? false; | ||
| } | ||
|
|
||
| function isReturnsValue(func: IServiceFunc): boolean { | ||
| return func.docs?.some((d) => d.includes('#[returns_value]')) ?? false; | ||
| } | ||
|
|
||
| function isIndexed(field: IStructField): boolean { | ||
| return field.docs?.some((d) => d.includes('#[indexed]')) ?? false; | ||
| } |
There was a problem hiding this comment.
The functions isPayable, isReturnsValue, and isIndexed use string.includes() to check for attributes like #[payable] within doc comments. This is not robust and can lead to false positives. For example, a comment // Note: this function is not #[payable] would cause isPayable to return true.
The check should be more specific by matching the trimmed doc string exactly against the attribute.
| function isPayable(func: IServiceFunc): boolean { | |
| return func.docs?.some((d) => d.includes('#[payable]')) ?? false; | |
| } | |
| function isReturnsValue(func: IServiceFunc): boolean { | |
| return func.docs?.some((d) => d.includes('#[returns_value]')) ?? false; | |
| } | |
| function isIndexed(field: IStructField): boolean { | |
| return field.docs?.some((d) => d.includes('#[indexed]')) ?? false; | |
| } | |
| function isPayable(func: IServiceFunc): boolean { | |
| return func.docs?.some((d) => d.trim() === '#[payable]') ?? false; | |
| } | |
| function isReturnsValue(func: IServiceFunc): boolean { | |
| return func.docs?.some((d) => d.trim() === '#[returns_value]') ?? false; | |
| } | |
| function isIndexed(field: IStructField): boolean { | |
| return field.docs?.some((d) => d.trim() === '#[indexed]') ?? false; | |
| } |
No description provided.