Repository navigation
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 55 minutes. View limit detailsLimit details: You’ve used the included review currently available. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (8)
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (8)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds a parser for Bash, POSIX, and mksh syntax. It translates parsed input into a typed ESTree-style AST, exports the parser and types, adds tests, and documents node shapes and source-position behavior. ChangesBash parser
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant parseBash
participant mvdanSh as mvdan-sh parser
participant Translator as AST translator
Caller->>parseBash: Bash source and parse options
parseBash->>mvdanSh: Parse source with selected shell variant
mvdanSh-->>parseBash: Syntax tree and comments
parseBash->>Translator: Translate syntax tree and source positions
Translator-->>parseBash: AST and comments
parseBash-->>Caller: AST and comments
Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 3 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
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 |
1155c3a to
a100dd6
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Operator extraction mishandles comments, error paths are omitted, and parts of the public documentation are inaccurate.
Review effort: Balanced
Findings: 1
Open (5)
What changed in this PR
Adds a public Bash parser that translates mvdan-sh output into an ESTree-style AST.
Changes:
- Adds AST types, parser translation, errors, variants, and Unicode offsets.
- Adds comprehensive parser and entry-point tests.
- Documents the syntax tree and direct parser API.
| File | Description |
|---|---|
src/parser/parse.ts |
Implements parsing and AST translation. |
src/parser/parse.spec.ts |
Tests parser behavior and node types. |
src/parser/mvdan-sh.d.ts |
Declares the required dependency API. |
src/types.ts |
Defines public AST types. |
src/index.ts |
Exports the parser and types. |
src/index.spec.ts |
Tests public parser exports. |
README.md |
Documents direct parser usage. |
docs/syntax-tree.md |
Documents AST structure and semantics. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| #operatorBetween(left: MvdanNode, right: MvdanNode): string { | ||
| const slice = this.#text | ||
| .slice(this.#end(left), this.#start(right)) | ||
| .trim(); | ||
|
|
||
| return slice.split(/\s+/u)[0] as string; | ||
| } |
There was a problem hiding this comment.
Partly right. The arithmetic example isn't valid Bash (# doesn't start a comment inside (( )), and the parser rejects it), but the same helper is used for [[ ]], where [[ a # note && b ]] is valid and did yield "#" as the operator. Fixed in db341d1 by skipping leading comments as well as whitespace, with a regression test covering a comment on either side of the operator.
| throw new BashSyntaxError( | ||
| parseError.Text || parseError.Error(), | ||
| line, | ||
| column, | ||
| ); |
There was a problem hiding this comment.
Agreed that path had no observable effect. I didn't switch to Error() though: that string is prefixed with path:line:col:, which duplicates the location ESLint already prints from line/column. Instead db341d1 exposes the option as BashSyntaxError#path, updates the option's doc comment to say so, and adds a test.
| ## Traversal | ||
|
|
||
| The traversal order for each node type is defined by the exported | ||
| `visitorKeys` object. Rules receive nodes by type name and may use `:exit` | ||
| suffixes: | ||
|
|
||
| ```js | ||
| export default { | ||
| create(context) { | ||
| return { | ||
| Command(node) {}, | ||
| "Program:exit"(node) {}, | ||
| }; | ||
| }, | ||
| }; | ||
| ``` |
There was a problem hiding this comment.
Leaving this as is. This PR is the bottom of a stack that lands in order, and the visitorKeys export arrives in the very next PR (#3). The doc describes the tree format as a whole, so removing the section here only to re-add it one PR later would just be churn.
bbaef73 to
67b3785
Compare
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Skip comments on either side of a test operator, expose the path option as BashSyntaxError#path, and document the mksh ;| case terminator. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Renames parseBash, BashSyntaxError, BashNode, and the related types to their Shell equivalents (BashShellVariant becomes ShellVariant), and updates the docs to refer to @eslint/shell. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds a shell parser that produces an ESTree-style syntax tree. It wraps
mvdan-sh, a JavaScript build of mvdan/sh (the parser behindshfmt), and translates its output.What's included
src/parser/parse.ts:parseShell(text, { variant, path })returns{ ast, comments }.ShellSyntaxError, with 1-basedlineandcolumn.variantis"bash"(default),"posix", or"mksh". Unknown variants throw aTypeError.src/parser/mvdan-sh.d.ts: minimal ambient types for the parts ofmvdan-shused here.src/types.ts: the AST node types only. Rule and language types come in the next PR.docs/syntax-tree.md: documents every node type.src/index.ts: re-exportsparseShell,ShellSyntaxError, and the AST types.sourceCode.getRange()andgetLoc(), which the next PR adds.Tree format
type,start, andend(0-based character offsets,endexclusive). There are norangeorlocproperties; the next PR'sSourceCodederives them.mvdan-shreports UTF-8 byte offsets, which are converted to JavaScript string offsets. Tests cover multi-byte and astral characters.mvdan-shexposes operators as numeric enums, so operator text (&&,>>,:-,;;&, …) is read from the source between the relevant child nodes.Pipelinenode.&&/||chains are left-associativeLogicalExpressions.elifbecomes a nestedIfStatement, andelsebecomes anElseClause.!negation, and&background are properties of each statement.Program.comments. They are not traversed.Known limitations
mvdan-shwas deprecated upstream in April 2025 in favor ofsh-syntax(Deprecatemvdan-shnpm package in favor ofsh-syntaxmvdan/sh#1145), and 0.10.1 is its last release. It works for everything tested here.sh-syntaxis not a drop-in replacement for this plugin: its parse output stops at the statement level (each command is only positions, with no type, arguments, expansions, or nested statements), and its API is async while ESLint parsing must be synchronous.Redirect'sheredocword can lie outside its parent statement's range.[ ... ]: a plainCommandnamed[. Only[[ ... ]]is parsed as aTestCommand.Testing
63 parser tests cover every node type, all
${...}operator forms, redirects and heredocs, comments, Unicode offsets, error locations, and the posix/mksh variants. This PR also adds an entry-point test for the parser export. 82 tests total, including the existing metadata test; build, lint, and format checks pass.🤖 Generated with Claude Code
Stack created with GitHub Stacks CLI • Give Feedback 💬
Summary by CodeRabbit