feat: C# language #41
Conversation
WalkthroughThis pull request introduces a new C# language package into the ast-grep framework. It adds a set of files including a README with installation and usage instructions, type definitions, implementation code registering the C# parser, a testing setup using the nursery package, and a post-install script for setup tasks. The changes define exported entities for language registration and configure package metadata, dependencies, and scripts for building and testing. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Dev
participant Postinstall as Postinstall Hook
participant Parser as C# Parser Module
participant Nursery as Nursery Test Runner
User->>Postinstall: Trigger package post-installation
Postinstall->>Parser: Execute setup with current directory
User->>Nursery: Run language tests
Nursery->>Parser: Register language and parse sample code
Parser-->>Nursery: Return parsed syntax tree
Possibly related PRs
Poem
✨ 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. 🪧 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: 3
🧹 Nitpick comments (3)
packages/csharp/index.js (1)
1-9: LGTM, but consider adding additional file extensions and documentation.The code correctly sets up C# language integration for ast-grep. A few suggestions for improvement:
- Consider adding
.csxextension to the extensions array for C# script files- Adding a JSDoc comment at the top of the file would help document the module's purpose
- A brief comment explaining what
expandoCharis used for would improve clarityconst path = require('node:path') const libPath = path.join(__dirname, 'parser.so') +// C# language configuration for ast-grep module.exports = { libraryPath: libPath, - extensions: ["cs"], + extensions: ["cs", "csx"], languageSymbol: 'tree_sitter_CSharp', + // Character used for AST node expansion in patterns expandoChar: 'µ', }packages/csharp/package.json (2)
22-22: Add author information.The author field is empty. Adding proper attribution is important for maintainability and recognition.
- "author": "", + "author": "The ast-grep team",
8-9: Consider consolidating duplicate scripts.The
sourceandprepublishOnlyscripts are identical. Consider using a shared script or documenting why both are needed."scripts": { "build": "tree-sitter build -o parser.so", - "source": "node nursery.js source", - "prepublishOnly": "node nursery.js source", + "source": "node nursery.js source", + "prepublishOnly": "npm run source", "postinstall": "node postinstall.js", "test": "node nursery.js test" },
📜 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 (6)
packages/csharp/README.md(1 hunks)packages/csharp/index.d.ts(1 hunks)packages/csharp/index.js(1 hunks)packages/csharp/nursery.js(1 hunks)packages/csharp/package.json(1 hunks)packages/csharp/postinstall.js(1 hunks)
🔇 Additional comments (7)
packages/csharp/postinstall.js (1)
1-4: LGTM: Well-structured postinstall script.The script correctly imports and uses the
postinstallfunction from the@ast-grep/setup-langpackage, providing the current directory as needed for proper language setup.packages/csharp/index.d.ts (1)
1-7: LGTM: Clear type definition for language registration.The
LanguageRegistrationtype is well-defined with appropriate properties for language integration.packages/csharp/nursery.js (1)
1-3: LGTM: Good setup for testing environment.The imports correctly set up the nursery testing framework and language registration.
packages/csharp/README.md (1)
1-25: LGTM: Well-structured documentation.The README provides clear installation and usage instructions, covering all necessary steps and dependencies. The example code is concise and demonstrates the core functionality effectively.
packages/csharp/package.json (3)
13-20: LGTM for package files configuration.The files list includes all necessary components for the package to function correctly.
24-47: LGTM for dependency configuration and publishing setup.The dependency configuration is appropriate, with proper versioning and optional peer dependencies. The publishing configuration is also properly set up for npm.
23-23:✅ Verification successful
Verify license compatibility.
Ensure the ISC license is consistent with the licensing approach used in the rest of the ast-grep project.
🏁 Script executed:
#!/bin/bash # Check license consistency across ast-grep packages echo "Checking licenses in other ast-grep packages..." find packages -name package.json -not -path "*/node_modules/*" | xargs grep -l "license" | xargs grep "license"Length of output: 450
License Consistency Confirmed
The ISC license is used uniformly across all ast-grep packages, including inpackages/csharp/package.json(line 23). No changes are needed.
| declare const registratoin: LanguageRegistration | ||
| export default registratoin |
There was a problem hiding this comment.
Fix typo in variable name.
There's a typo in the variable name registratoin (should be registration). This impacts not only this file but also any imports that use this package.
-declare const registratoin: LanguageRegistration
-export default registratoin
+declare const registration: LanguageRegistration
+export default registration📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| declare const registratoin: LanguageRegistration | |
| export default registratoin | |
| declare const registration: LanguageRegistration | |
| export default registration |
| setup({ | ||
| dirname: __dirname, | ||
| name: 'CSharp', | ||
| treeSitterPackage: 'tree-sitter-c-sharp', | ||
| languageRegistration, | ||
| testRunner: (parse) => { | ||
| const sg = parse('var a = 123;') | ||
| const root = sg.root() | ||
| const node = root.find('var $A = 123') | ||
| assert.equal(node.kind(), 'variable_declaration') | ||
| } | ||
| }) |
There was a problem hiding this comment.
Missing import for assert.
The test runner uses assert but doesn't import it. Add the import statement to fix this issue.
const { setup } = require('@ast-grep/nursery')
const languageRegistration = require('./index')
+const assert = require('assert')
setup({
dirname: __dirname,
name: 'CSharp',
treeSitterPackage: 'tree-sitter-c-sharp',
languageRegistration,
testRunner: (parse) => {
const sg = parse('var a = 123;')
const root = sg.root()
const node = root.find('var $A = 123')
assert.equal(node.kind(), 'variable_declaration')
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setup({ | |
| dirname: __dirname, | |
| name: 'CSharp', | |
| treeSitterPackage: 'tree-sitter-c-sharp', | |
| languageRegistration, | |
| testRunner: (parse) => { | |
| const sg = parse('var a = 123;') | |
| const root = sg.root() | |
| const node = root.find('var $A = 123') | |
| assert.equal(node.kind(), 'variable_declaration') | |
| } | |
| }) | |
| const { setup } = require('@ast-grep/nursery') | |
| const languageRegistration = require('./index') | |
| const assert = require('assert') | |
| setup({ | |
| dirname: __dirname, | |
| name: 'CSharp', | |
| treeSitterPackage: 'tree-sitter-c-sharp', | |
| languageRegistration, | |
| testRunner: (parse) => { | |
| const sg = parse('var a = 123;') | |
| const root = sg.root() | |
| const node = root.find('var $A = 123') | |
| assert.equal(node.kind(), 'variable_declaration') | |
| } | |
| }) |
| { | ||
| "name": "@ast-grep/lang-csharp", | ||
| "version": "0.0.1", | ||
| "description": "", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add a meaningful package description.
The description field is empty. Adding a clear description will improve package discoverability and help users understand its purpose.
- "description": "",
+ "description": "C# language support for ast-grep",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "description": "", | |
| "description": "C# language support for ast-grep", |
|
Csharp released |
Summary by CodeRabbit
Documentation
New Features
Tests
Chores