Conversation
…moval Implement custom Go language comment processor that preserves important //go: directives (build tags, generate commands) while removing regular comments. This addresses the issue where strip-comments library was removing all comments including critical Go compiler directives. Features: - Preserve //go:build, //go:generate and other //go: directives - Remove regular // and /* */ comments appropriately - Handle nested block comments (Go-specific feature) - Protect comment-like text within strings, raw strings, and rune literals - Comprehensive test coverage for edge cases The GoManipulator uses a state machine to accurately parse Go syntax and distinguish between directives that must be preserved and comments that should be removed.
|
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. Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces a Go-specific comment remover via a new GoManipulator class and updates the .go file handling to use it. Adds tests covering Go directives, nested block comments, mixed directives/comments, and string literals containing //, verifying correct preservation and removal behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as FileProcessor
participant Map as manipulators[.ext]
participant GoMan as GoManipulator
participant Trim as rtrimLines
Caller->>Map: select manipulator for ".go"
Map-->>Caller: GoManipulator instance
Caller->>GoMan: removeComments(content)
Note over GoMan: State machine parses:<br/>- code<br/>- inline/block comments<br/>- strings/runes<br/>- //go: directives
GoMan->>Trim: rtrimLines(processed)
Trim-->>GoMan: trimmed
GoMan-->>Caller: result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Deploying repomix with
|
| Latest commit: |
575ae2b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://08837508.repomix.pages.dev |
| Branch Preview URL: | https://fix-go-strip-comments.repomix.pages.dev |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a specialized comment manipulation logic for Go language files. Its primary goal is to accurately remove standard comments while ensuring that critical Go compiler directives, such as //go:build and //go:generate, are preserved, which were previously incorrectly stripped by the generic comment remover.
Highlights
- Go-specific Comment Handling: Implemented a new GoManipulator class designed specifically for processing comments in Go source files.
- Preservation of Go Directives: Ensures that essential //go: compiler directives (e.g., //go:build, //go:generate, //go:embed) are identified and retained during comment removal.
- Accurate Comment Stripping: Correctly removes standard single-line (//) and multi-line (/* */) comments, including handling Go's unique nested block comment syntax.
- Context-Aware Parsing: Utilizes a state machine to differentiate between actual comments and comment-like text appearing within string literals, raw strings, and rune literals, preventing unintended modifications.
- Integration: Replaces the generic comment stripper with the new GoManipulator for .go files in the core file manipulation utility.
- Enhanced Test Coverage: Added comprehensive unit tests to validate the GoManipulator's behavior across various Go comment and directive scenarios.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Pull Request Overview
This PR adds a specialized GoManipulator class to handle Go file comment processing that preserves essential Go compiler directives (like //go:build, //go:generate, //go:embed) while removing regular comments.
- Implements state-machine-based parsing to accurately distinguish between Go directives and regular comments
- Adds proper handling for Go-specific syntax including nested block comments and string literals
- Replaces the generic strip-comments library with Go-specific logic for
.gofiles
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/core/file/fileManipulate.ts | Adds GoManipulator class and updates file extension mapping to use it for .go files |
| tests/core/file/fileManipulate.test.ts | Adds comprehensive test cases covering Go directive preservation, nested comments, and string literals |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #814 +/- ##
==========================================
+ Coverage 87.41% 87.59% +0.17%
==========================================
Files 113 113
Lines 6493 6603 +110
Branches 1331 1372 +41
==========================================
+ Hits 5676 5784 +108
- Misses 817 819 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces a GoManipulator to properly handle comment removal in Go files while preserving compiler directives like //go:build. The implementation uses a state machine, which is a robust approach for this kind of parsing. The new functionality is well-supported by a comprehensive set of test cases.
My review includes a couple of suggestions to improve the code's structure and maintainability by moving an enum definition and reducing some code duplication. Overall, this is a solid contribution that addresses an important issue for Go projects.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
tests/core/file/fileManipulate.test.ts (4)
736-782: Good coverage for directive preservation; consider adding embed/whitespace/EOF cases.
- Add a case for
//go:embedsince it is common and sensitive to removal.- Add a case where directives are indented with spaces/tabs (spec allows leading whitespace).
- Add a case where a directive is the last line without a trailing newline to validate the
lineEnd === -1path.I can append these as additional test vectors if you want.
784-806: Clarify nested block comment semantics; test intent is fine.Go block comments do not nest per spec; you’re intentionally handling nested sequences for robustness. A short comment in the test name or description would avoid confusion for readers.
808-820: Add a legacy build-tag case (// +build) to prevent regressions.Some repos still use legacy
// +buildtags. Consider a test ensuring such lines are preserved (or explicitly document that they’re removed).Happy to add a test variant preserving
// +build.
822-832: Nice string/rune coverage; add a raw-string edge and trailing-comment variant.
- Add a raw string ending at EOF to ensure state closes without newline.
- Add a line with code followed by
//go:to confirm it’s treated as a normal comment (not a directive).src/core/file/fileManipulate.ts (3)
71-78: Avoid recreating enum per call; hoist or use const enum.Defining
enum StateinsideremoveCommentsallocates at runtime on each call. Hoist it to module scope or use aconst enumto inline values.Apply (hoist to module scope):
- removeComments(content: string): string { - if (!content) return ''; - - enum State { +enum GoState { Normal = 0, InLineComment = 1, InBlockComment = 2, InDoubleQuoteString = 3, InRawString = 4, InRuneLiteral = 5, } +class GoManipulator extends BaseManipulator { + removeComments(content: string): string { + if (!content) return ''; - let state: State = State.Normal; + let state: GoState = GoState.Normal;If you prefer not to change scope, switch to
const enum Statefor zero runtime overhead (subject to TS config).
94-113: Directive detection is correct; optionally preserve legacy// +build.Current logic preserves
//go:when the line start (or only preceding whitespace) matches. Consider also preserving legacy// +buildtags to avoid breaking older codebases.Apply minimal change:
- if (restOfLine.startsWith('//go:')) { + if (restOfLine.startsWith('//go:') || restOfLine.startsWith('// +build')) {If you intentionally don't support legacy tags, add a brief comment stating that choice.
146-166: Fix misleading comment about nesting.Go block comments do not nest; you’re adding nesting support for resilience. Update the comment to avoid confusion.
- // Handle nested block comments (Go supports them) + // Handle nested block comment sequences for robustness (Go block comments do not nest per spec)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/core/file/fileManipulate.ts(2 hunks)tests/core/file/fileManipulate.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Build and run (windows-latest, 24.x)
- GitHub Check: Build and run (windows-latest, 23.x)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Build and run (macos-latest, 21.x)
- GitHub Check: Build and run (macos-latest, 24.x)
- GitHub Check: Build and run (windows-latest, 21.x)
- GitHub Check: Test (macos-latest, 22.x)
- GitHub Check: Build and run (ubuntu-latest, 21.x)
- GitHub Check: Test (macos-latest, 23.x)
- GitHub Check: Test (ubuntu-latest, 23.x)
- GitHub Check: Build and run with Bun (windows-latest, latest)
🔇 Additional comments (3)
src/core/file/fileManipulate.ts (3)
168-199: String and rune handling LGTM; minor newline note.Double-quoted strings can’t contain raw newlines, so not updating
lineStarthere is fine. Raw strings do hit the bottom\ncheck—good.
202-208: Return trimming is consistent with other manipulators.
rtrimLinesaligns outputs with tests and prior behavior.
344-344: Mapping switch to GoManipulator looks good.
.gonow uses the Go-specific parser as intended.
Address code review feedback: - Move GoParserState enum to module level to avoid re-declaration overhead - Optimize string operations by using content.startsWith() instead of substring - Improve code readability by hoisting duplicate result += char statements - Fix comment capitalization for consistency These changes improve performance by reducing unnecessary string operations and memory allocations while maintaining the same functionality.
Remove the unused lineStart variable and its assignments from the GoManipulator class. After optimization with hasNonWhitespaceOnLine tracking, lineStart is no longer needed for functionality. This resolves the final lint warning about unused variables.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new GoManipulator to handle comment removal in Go files while preserving compiler directives like //go:build. The implementation uses a state machine to parse the file content, which is a good approach. However, there is a critical issue in the handling of block comments: the code incorrectly implements support for nested block comments, which is not a feature of the Go language. This can lead to incorrect comment stripping. I've provided a detailed comment with a suggested fix to align the behavior with the Go specification. The rest of the implementation, including the handling of directives and string literals, appears correct, and the added tests are comprehensive.
Go block comments do not nest according to the language specification. The first */ sequence should close the comment, regardless of any /* sequences within it. This change removes the blockCommentDepth tracking and ensures correct parsing behavior for Go code containing sequences like /* comment with /* nested */ part */. Updated test expectations to reflect the correct Go language behavior.
Fixed formatting issue identified by Biome linter to ensure consistent code style across the project.
Removed the Go nested block comments test case as it was unnecessary and potentially misleading. Go block comments do not nest according to the language specification, so testing this behavior is not needed and could cause confusion about the expected behavior. The remaining tests adequately cover Go comment parsing functionality.
yamadashy
changed the title
This PR resolves a critical parser hang issue in the
strip-commentslibrary and implements a custom Go language comment processor that correctly preserves Go compiler directives while removing regular comments.Problem Analysis
Strip-comments Library Issues
The
strip-commentslibrary (v2.0.1) has several fundamental issues when processing Go code:Parser Hang Issue:
/^(['"])((?:\\1|[^\1])+?)(\1)/`[^\1]pattern doesn't work as expected in character classes, being interpreted as[^1]Language Support Mismatch:
exports.c = exports.javascriptProblematic Go Code Pattern:
Go Directives Issue
Additionally,
strip-commentsremoves critical Go compiler directives://go:build(build constraints/tags)//go:generate(code generation commands)//go:embed(file embedding)//go:directivesThese are compiler instructions, not regular comments, and must be preserved.
Solution
Implemented
GoManipulatorclass that addresses both issues:Parser Stability
Go Language Compliance
//go:prefixed comments intact//and/* */comments appropriately*/closes the comment)Implementation Details
//go:at line start or after whitespace-only contentBefore/After Comparison
Problematic Input (Previously Caused Hang):
Output (With GoManipulator):
Technical Benefits
Test Coverage
Added comprehensive test cases covering:
Checklist
npm run testnpm run lint