feat: Implement structured CodeCall fields (Issue #41 Part E)#47
feat: Implement structured CodeCall fields (Issue #41 Part E)#47
Conversation
Add TopLevelScope structure to replace the legacy `default` node pattern for representing file-level/module-level declarations across languages. New types: - TopLevelScope: container for top-level functions, fields, exports, type aliases - CodeTypeAlias: represents type alias declarations (TypeScript, Rust, Go, C++) CodeContainer additions: - TopLevel field for structured top-level declarations - getTopLevelFunctions(): combines TopLevel and legacy default nodes - getTopLevelFields(): combines multiple sources - getTopLevelExports(): returns exports from TopLevel - getActualDataStructures(): excludes default nodes - hasTopLevelDeclarations(): checks for any top-level content - migrateDefaultNodesToTopLevel(): migration helper for parsers This implements issue #41 section B (统一 top-level) while maintaining full backward compatibility with existing code using default nodes. Refs #41
- Fix isEmpty() to also check Extension map - Fix KDoc for isEmpty() - now correctly says "has no declarations" - Fix KDoc for isNotEmpty() - now says "has any declarations (i.e., is not empty)" - Add isLegacyTopLevelContainer() method to CodeDataStruct for better identification of legacy top-level containers - Update CodeContainer methods to use isLegacyTopLevelContainer() - Add tests for Extension in isEmpty() check - Add tests for isLegacyTopLevelContainer() method
…sers Implements Issue #41 part E: Call semantics structuring - Add structured fields to CodeCall: * ReceiverExpr: original receiver expression text * ReceiverType: resolved receiver type (CodeTypeRef) * Callee: function/method name being called * IsOptionalChain: optional chaining flag (TypeScript ?.) * Chain: list of method names in chained calls - Adapt all language parsers to populate new fields: * TypeScript: full support with chain extraction and optional chaining * Go: receiver expression and type resolution * Rust: receiver expression extraction * Java: method calls, field calls, and method references * Kotlin: navigation and call suffix handling * Python: function call receiver extraction * C#: member access and object creation * Scala: stable ID, infix expressions, and constructor calls - Add comprehensive tests for new CodeCall fields - Maintain backward compatibility with legacy fields Closes #41
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds structured Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR implements structured call semantics by adding five new fields to CodeCall (ReceiverExpr, ReceiverType, Callee, IsOptionalChain, Chain) and updating all eight language parsers to populate these fields. Additionally, it introduces TopLevelScope to replace the legacy "default" node pattern for representing file-level declarations.
Changes:
- Added structured fields to
CodeCallfor better representation of method calls, receivers, and call chains - Introduced
TopLevelScopedata class andCodeTypeAliasfor top-level declarations - Updated all language parsers (TypeScript, Go, Rust, Java, Kotlin, Python, C#, Scala) to populate new structured fields
- Added comprehensive tests for new domain classes and backward compatibility
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| chapi-domain/src/main/kotlin/chapi/domain/core/CodeCall.kt | Added 5 new structured fields (ReceiverExpr, ReceiverType, Callee, IsOptionalChain, Chain) with detailed documentation |
| chapi-domain/src/main/kotlin/chapi/domain/core/TopLevelScope.kt | New data class representing file-level declarations with helper methods |
| chapi-domain/src/main/kotlin/chapi/domain/core/CodeDataStruct.kt | Added isLegacyTopLevelContainer() method to detect legacy top-level patterns |
| chapi-domain/src/main/kotlin/chapi/domain/core/CodeContainer.kt | Added TopLevel field and helper methods for backward compatibility |
| chapi-domain/src/test/kotlin/chapi/domain/core/CodeCallTest.kt | Added tests for new structured CodeCall fields and backward compatibility |
| chapi-domain/src/test/kotlin/chapi/domain/core/TopLevelScopeTest.kt | Comprehensive tests for TopLevelScope across multiple languages |
| chapi-ast-typescript/src/main/kotlin/chapi/ast/typescriptast/TypeScriptFullIdentListener.kt | Updated to populate structured call fields with chain extraction and optional chaining support |
| chapi-ast-go/src/main/kotlin/chapi/ast/goast/GoFullIdentListener.kt | Updated to extract receiver expressions and resolve receiver types |
| chapi-ast-rust/src/main/kotlin/chapi/ast/rustast/RustFullIdentListener.kt | Updated to populate ReceiverExpr and Callee fields |
| chapi-ast-java/src/main/kotlin/chapi/ast/javaast/JavaFullIdentListener.kt | Updated to set structured fields for method calls and field accesses |
| chapi-ast-kotlin/src/main/kotlin/chapi/ast/kotlinast/KotlinFullIdentListener.kt | Updated to populate ReceiverExpr and Callee for navigation expressions |
| chapi-ast-python/src/main/kotlin/chapi/ast/pythonast/PythonAstBaseListener.kt | Updated to extract receiver information from function calls |
| chapi-ast-csharp/src/main/kotlin/chapi/ast/csharpast/CSharpFullIdentListener.kt | Updated to populate structured fields for member access and object creation |
| chapi-ast-scala/src/main/kotlin/chapi/ast/scalaast/ScalaFullIdentListener.kt | Updated to set ReceiverExpr and Callee for various call types |
| chapi-ast-go/src/test/kotlin/chapi/ast/goast/GoAnalyserTest.kt | Updated test to include Callee field verification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -521,15 +541,21 @@ class GoFullIdentListener(var fileName: String) : GoAstListener() { | |||
| val nodeName = handleForPrimary(child).orEmpty() | |||
|
|
|||
| // if nodeName ends with $.functionName, the functionName should be remove | |||
There was a problem hiding this comment.
The comment says "if nodeName ends with $.functionName" but the code checks for ".$funcName". The dollar sign in the comment should be a period to match the actual code logic.
| // if nodeName ends with $.functionName, the functionName should be remove | |
| // if nodeName ends with .${funcName}, the functionName should be removed |
| // Heuristic: Rust/Go module files often start with lowercase (e.g., "main", "file_mod") | ||
| // while actual classes/structs start with uppercase | ||
| val firstChar = NodeName.firstOrNull() | ||
| if (firstChar != null && firstChar.isLowerCase() && Type == DataStructType.OBJECT) { | ||
| return true | ||
| } |
There was a problem hiding this comment.
The heuristic for detecting legacy top-level containers based on lowercase first character may incorrectly flag legitimate lowercase-named types in languages like Rust where primitive types or type aliases can start with lowercase (e.g., "u32", "i64", "str"). Consider refining this heuristic to be more specific or adding additional checks to reduce false positives.
|
|
||
| // Create a CodeCall with IsOptionalChain = true | ||
| val callInfo = extractCallInfo(targetExpr) | ||
| val callee = targetExpr?.text?.substringAfterLast(".")?.substringBefore("(") ?: "" | ||
|
|
There was a problem hiding this comment.
In the OptionalCallExpressionContext handling, the callee extraction logic may not correctly handle all cases. The expression targetExpr?.text?.substringAfterLast(".")?.substringBefore("(") assumes there's always a dot before the method name, but optional calls can also be simple function calls like foo?.(). Consider adding a fallback when no dot is present to use the full expression text or the last identifier.
| // Create a CodeCall with IsOptionalChain = true | |
| val callInfo = extractCallInfo(targetExpr) | |
| val callee = targetExpr?.text?.substringAfterLast(".")?.substringBefore("(") ?: "" | |
| // Create a CodeCall with IsOptionalChain = true | |
| val callInfo = extractCallInfo(targetExpr) | |
| val callee = targetExpr?.text?.let { text -> | |
| // Strip off any argument list, then take the last identifier if dotted | |
| val base = text.substringBefore("(") | |
| val lastDotIndex = base.lastIndexOf('.') | |
| if (lastDotIndex >= 0 && lastDotIndex < base.length - 1) { | |
| base.substring(lastDotIndex + 1) | |
| } else { | |
| base | |
| } | |
| } ?: "" |
| if (e.singleExpression().size >= 2) { | ||
| collectChain(e.singleExpression(0)) | ||
| // The second singleExpression is the property being accessed | ||
| val propExpr = e.singleExpression(1) | ||
| if (propExpr is IdentifierExpressionContext) { | ||
| chain.add(propExpr.identifierName().text) | ||
| } | ||
| } |
There was a problem hiding this comment.
The extractCallInfo function's collectChain logic for OptionalChainExpressionContext checks if e.singleExpression().size >= 2, but this may not correctly handle all optional chaining scenarios. The grammar structure for optional chaining should be verified to ensure this indexing approach is always safe and correct.
| // Set ReceiverType if we can resolve it | ||
| currentCall.ReceiverType = CodeTypeRef.simple(resolvedFirst) |
There was a problem hiding this comment.
The ReceiverType is being set to CodeTypeRef.simple(resolvedFirst) where resolvedFirst is the type string from local variable lookup. However, if the variable is not found in localVars, resolvedFirst will be null and this will create a CodeTypeRef with a null name. This could lead to unexpected behavior. Consider adding a null check before setting ReceiverType.
- Merge master branch changes into feat/toplevel-scope - Use IsOptional instead of IsOptionalChain (aligned with master) - Add ChainArguments and ReturnType fields from master - Keep Callee field from our implementation - Update TypeScript parser to use IsOptional field - Update tests to use IsOptional instead of IsOptionalChain - Preserve all helper methods from master (allMethodNames, isChainedCall, etc.)
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #47 +/- ##
============================================
+ Coverage 67.39% 67.61% +0.21%
- Complexity 1386 1436 +50
============================================
Files 77 78 +1
Lines 6548 6731 +183
Branches 1248 1275 +27
============================================
+ Hits 4413 4551 +138
- Misses 1402 1430 +28
- Partials 733 750 +17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
chapi-ast-kotlin/src/main/kotlin/chapi/ast/kotlinast/KotlinFullIdentListener.kt (1)
199-210: UpdateCalleefield inrefineWithClass()to maintain consistency withFunctionName.When
refineWithClass()detects a creator and setsFunctionName = PRIMARY_CONSTRUCTOR(line 122), theCalleefield is not updated. SinceCalleeis the preferred field in newer code (seeallMethodNames()which usesCallee.ifEmpty { FunctionName }), this creates an inconsistency where:
FunctionName="PrimaryConstructor"Callee= original function name (unchanged)Update
refineWithClass()to also setCallee:Suggested fix
private fun CodeCall.refineWithClass(it: CodeDataStruct): CodeCall? { if (FunctionName[0].isUpperCase()) { if (it.NodeName == FunctionName) { Type = CallType.CREATOR Package = it.Package NodeName = it.NodeName this.FunctionName = PRIMARY_CONSTRUCTOR this.Callee = PRIMARY_CONSTRUCTOR // Add this return null } } return this }
🤖 Fix all issues with AI agents
In `@chapi-ast-go/src/test/kotlin/chapi/ast/goast/GoAnalyserTest.kt`:
- Around line 30-36: The test's CodeCall instance is missing the ReceiverExpr
field which the Go parser sets; update the CodeCall (in the GoAnalyserTest
CodeCall with NodeName="fmt", FunctionName="Println", Callee="Println") to
include ReceiverExpr = "fmt" so the expected object matches the parser output.
♻️ Duplicate comments (3)
chapi-ast-typescript/src/main/kotlin/chapi/ast/typescriptast/TypeScriptFullIdentListener.kt (1)
875-890: Verify callee extraction for edge cases in optional calls.The callee extraction at line 882 uses
substringAfterLast(".")?.substringBefore("("), which assumes there's a dot in the expression. For simple optional calls likefoo?.()(calling a nullable function directly), this would return the full expression text rather than just the function name.Consider using the
callInfo.calleeif available, or falling back to a more robust extraction:// Create a CodeCall with IsOptional = true val callInfo = extractCallInfo(targetExpr) - val callee = targetExpr?.text?.substringAfterLast(".")?.substringBefore("(") ?: "" + val callee = callInfo.callee.ifBlank { + targetExpr?.text?.let { text -> + val base = text.substringBefore("(") + if (base.contains('.')) base.substringAfterLast('.') else base + } ?: "" + }chapi-ast-go/src/main/kotlin/chapi/ast/goast/GoFullIdentListener.kt (2)
496-498: ReceiverType is safely set within null check.The
resolvedFirstvariable is guaranteed non-null at line 498 because it's inside theif (resolvedFirst != null)block (line 486). TheCodeTypeRef.simple(resolvedFirst)call is safe.
543-558: Comment typo:$should be.to match the code logic.The comment at line 543 says "if nodeName ends with $.functionName" but the code checks for ".$funcName". This was flagged in a previous review.
| CodeCall( | ||
| NodeName = "fmt", | ||
| FunctionName = "Println", | ||
| Parameters = listOf(CodeProperty(TypeValue = "hello world", TypeType = "string")) | ||
| Parameters = listOf(CodeProperty(TypeValue = "hello world", TypeType = "string")), | ||
| // New structured field | ||
| Callee = "Println" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the CodeCall class definition
fd -e kt -e java | xargs grep -l "class CodeCall" | head -5Repository: phodal/chapi
Length of output: 177
🏁 Script executed:
# Search for ReceiverExpr in the codebase
rg "ReceiverExpr" --type=kt -B2 -A2Repository: phodal/chapi
Length of output: 82
🏁 Script executed:
# Look at the test file to understand context better
cat -n chapi-ast-go/src/test/kotlin/chapi/ast/goast/GoAnalyserTest.kt | head -50Repository: phodal/chapi
Length of output: 1834
🏁 Script executed:
# Check if there are other test cases in the same file that show ReceiverExpr usage
rg "CodeCall" chapi-ast-go/src/test/kotlin/chapi/ast/goast/GoAnalyserTest.kt -B1 -A6Repository: phodal/chapi
Length of output: 469
🏁 Script executed:
# Read the CodeCall class definition
cat chapi-domain/src/main/kotlin/chapi/domain/core/CodeCall.ktRepository: phodal/chapi
Length of output: 8600
🏁 Script executed:
# Search for ReceiverExpr in all files (without type filter)
rg "ReceiverExpr" -B2 -A2Repository: phodal/chapi
Length of output: 26265
🏁 Script executed:
# Check for other CodeCall instances in Go tests
rg "CodeCall(" chapi-ast-go/src/test/kotlin -B1 -A8 | head -100Repository: phodal/chapi
Length of output: 124
Test is incomplete - add ReceiverExpr field to match Go parser output.
The CodeCall should include ReceiverExpr = "fmt" alongside the existing Callee = "Println". The Go parser implementation populates ReceiverExpr with the receiver expression (the package/object the method is called on), and the CodeCall documentation for Go shows this pattern: ReceiverExpr="v1" for v1.Group(...). Without it, the test expectation won't match the actual parser output.
🤖 Prompt for AI Agents
In `@chapi-ast-go/src/test/kotlin/chapi/ast/goast/GoAnalyserTest.kt` around lines
30 - 36, The test's CodeCall instance is missing the ReceiverExpr field which
the Go parser sets; update the CodeCall (in the GoAnalyserTest CodeCall with
NodeName="fmt", FunctionName="Println", Callee="Println") to include
ReceiverExpr = "fmt" so the expected object matches the parser output.
1 participant
Description
This PR implements Issue #41 Part E: Call semantics structuring - adding structured fields to
CodeCalland adapting all language parsers to populate them.Changes
Core Domain Changes
CodeCall:ReceiverExpr: Original receiver expression textReceiverType: Resolved receiver type (CodeTypeRef)Callee: Function/method name being calledIsOptionalChain: Optional chaining flag (TypeScript?.)Chain: List of method names in chained callsParser Adaptations
All language parsers have been updated to populate the new structured fields:
Testing
CodeCallfieldsRelated Issue
Closes #41 (Part E)
Checklist
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.