✨ Kotlin #55
✨ Kotlin #55
Conversation
WalkthroughThis pull request adds Kotlin language support by introducing a new package, Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
🪧 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 (
|
NatoBoram
force-pushed
the
feature/kotlin
branch
2 times, most recently
from
4bbb114 to
e4551ad
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/kotlin/index.js (1)
1-9: Consider adding the metaVarChar property for consistency with type definition.The implementation exports the necessary properties for Kotlin language support, but doesn't include
metaVarCharwhich is defined in theLanguageRegistrationtype. While it's marked as optional in the type, you might want to include it for consistency if Kotlin has a specific meta-variable character.module.exports = { libraryPath: libPath, extensions: ['kt', 'ktm', 'kts'], languageSymbol: 'tree_sitter_kotlin', + metaVarChar: '$', // Add the appropriate meta-variable character for Kotlin expandoChar: 'µ', }packages/kotlin/README.md (1)
1-24: Good documentation start, but could be more comprehensive.The README provides basic installation and usage instructions for the Kotlin language support, which is helpful. However, it would benefit from additional information such as:
- Brief explanation of what ast-grep is and what this Kotlin package specifically enables
- Installation instructions for npm and yarn users (not just pnpm)
- Examples of actual Kotlin code patterns that can be recognized/parsed
- Information about supported Kotlin syntax features or limitations
Consider expanding the documentation to make it more accessible to users who might be unfamiliar with ast-grep or Kotlin integration.
packages/kotlin/nursery.js (1)
1-3: Consider module consistency between code and documentation.The nursery.js file uses CommonJS module syntax (require), while the README example uses ESM import statements. This inconsistency might confuse users.
Consider aligning the module system used in the tests with what's shown in the documentation, or note in the README if both module systems are supported.
packages/kotlin/package.json (3)
4-4: Add a meaningful package description.The description field is empty. Adding a clear description would help users understand the purpose of this package when viewing it on npm.
- "description": "", + "description": "Kotlin language support for ast-grep",
22-22: Add author information.The author field is empty. Including author information helps with package attribution and provides a point of contact.
- "author": "", + "author": "NatoBoram",
45-45: Recursive dependency in onlyBuiltDependencies.The onlyBuiltDependencies array includes "@ast-grep/lang-kotlin", which is recursive since this is the package itself.
- "onlyBuiltDependencies": ["@ast-grep/lang-kotlin", "tree-sitter-cli"] + "onlyBuiltDependencies": ["tree-sitter-cli"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
.changeset/khaki-badgers-hammer.md(1 hunks)packages/kotlin/README.md(1 hunks)packages/kotlin/index.d.ts(1 hunks)packages/kotlin/index.js(1 hunks)packages/kotlin/nursery.js(1 hunks)packages/kotlin/package.json(1 hunks)packages/kotlin/postinstall.js(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
packages/kotlin/postinstall.js (1)
packages/kotlin/nursery.js (1)
require(1-1)
🔇 Additional comments (4)
.changeset/khaki-badgers-hammer.md (1)
1-6: LGTM! Changeset file correctly declares the new package.The file properly indicates the addition of the
@ast-grep/lang-kotlinpackage with a patch version bump, following the project's changeset conventions.packages/kotlin/postinstall.js (1)
1-4: LGTM! Post-installation script follows project conventions.The script correctly imports the
postinstallfunction from the@ast-grep/setup-langpackage and invokes it with the current directory, consistent with other language packages in the project.packages/kotlin/index.d.ts (1)
1-10: LGTM! Type definitions are well-structured.The
LanguageRegistrationtype correctly defines all the necessary properties for Kotlin language support, and the default export is properly declared.packages/kotlin/package.json (1)
10-10:✅ Verification successful
Verify the existence of postinstall.js.
The package.json references a postinstall.js file in both the scripts and files sections, but this file wasn't included in the files for review.
🏁 Script executed:
#!/bin/bash # Check if postinstall.js exists in the repository fd --type file postinstall.jsLength of output: 412
Resolved: Verified that postinstall.js exists in packages/kotlin
The file referenced in
packages/kotlin/package.jsonis indeed present (located atpackages/kotlin/postinstall.js). No further action is needed.
| testRunner: parse => { | ||
| const sg = parse('println(123)') | ||
| const root = sg.root() | ||
| const node = root.find('println($A)') | ||
| assert.equal(node.kind(), 'call_expression') | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Test coverage is minimal.
The test only verifies a simple println(123) statement. While this is a good starting point, it would be more robust to test additional Kotlin language constructs like:
- Class and function declarations
- Control flow structures (if/else, loops)
- Lambda expressions
- Type declarations and generics
- String interpolation
Expand the test suite to cover more Kotlin syntax patterns to ensure the parser is robust across various language features.
| { | ||
| "name": "@ast-grep/lang-kotlin", | ||
| "version": "0.0.1", | ||
| "description": "", | ||
| "main": "index.js", | ||
| "scripts": { | ||
| "build": "tree-sitter build -o parser.so", | ||
| "source": "node nursery.js source", | ||
| "prepublishOnly": "node nursery.js source", | ||
| "postinstall": "node postinstall.js", | ||
| "test": "node nursery.js test" | ||
| }, | ||
| "files": [ | ||
| "index.js", | ||
| "index.d.ts", | ||
| "type.d.ts", | ||
| "postinstall.js", | ||
| "src", | ||
| "prebuilds" | ||
| ], | ||
| "keywords": ["ast-grep"], | ||
| "author": "", | ||
| "license": "ISC", | ||
| "dependencies": { | ||
| "@ast-grep/setup-lang": "0.0.3" | ||
| }, | ||
| "peerDependencies": { | ||
| "tree-sitter-cli": "0.24.6" | ||
| }, | ||
| "peerDependenciesMeta": { | ||
| "tree-sitter-cli": { | ||
| "optional": true | ||
| } | ||
| }, | ||
| "devDependencies": { | ||
| "@ast-grep/nursery": "0.0.2", | ||
| "tree-sitter-cli": "0.24.6", | ||
| "tree-sitter-kotlin": "0.3.8" | ||
| }, | ||
| "publishConfig": { | ||
| "access": "public", | ||
| "registry": "https://registry.npmjs.org/" | ||
| }, | ||
| "pnpm": { | ||
| "onlyBuiltDependencies": ["@ast-grep/lang-kotlin", "tree-sitter-cli"] | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Missing index.js and other referenced files.
The package.json references several important files that weren't included in this review:
- index.js (main entry point)
- index.d.ts (TypeScript definitions)
- type.d.ts
- postinstall.js
🏁 Script executed:
#!/bin/bash
# Check for the existence of important files
echo "Checking for required files in the packages/kotlin directory:"
for file in index.js index.d.ts type.d.ts postinstall.js
do
if [ -f "packages/kotlin/$file" ]; then
echo "✅ $file exists"
else
echo "❌ $file is missing"
fi
doneLength of output: 735
Action Required: Missing File Discrepancy in Package
After verification, only type.d.ts is missing, while index.js, index.d.ts, and postinstall.js are all present. Please add the missing type.d.ts file to the packages/kotlin directory, or update the files field in package.json if this file is not needed.
NatoBoram
force-pushed
the
feature/kotlin
branch
2 times, most recently
from
f5694e6 to
ed988d1
Compare
NatoBoram
force-pushed
the
feature/kotlin
branch
from
ed988d1 to
bcae3b8
Compare
Halfway there!
Summary by CodeRabbit
New Features
Documentation