Migrate LSP YAML parser to gotreesitter#302
Merged
Merged
Conversation
Reviewer's GuideMigrates the LSP YAML parsing logic from the CGO-based tree-sitter bindings to the pure-Go gotreesitter client and removes CGO/system toolchain requirements from builds, Docker image, and release workflows while updating dependencies and documentation accordingly. Class diagram for updated LSP YAML parsing with gotreesitterclassDiagram
class parser {
+getPositionType(document *string, position lsp.Position) PositionType
+inMixinsPosition(document *string, position lsp.Position) bool
+inDependsPosition(document *string, position lsp.Position) bool
+extractFilenameFromMixins(document *string, position lsp.Position) string
+getCommands(document *string) []Command
+getCurrentCommand(document *string, position lsp.Position) *Command
+findCommand(document *string, commandName string) *Command
+extractDependsValues(document *string) []string
}
class Command {
+name string
+position lsp.Position
}
class yamlLanguageVar {
+yamlLanguage ts.Language
}
class parseYAMLDocumentFn {
+parseYAMLDocument(document *string) (*ts.Tree, []byte, error)
}
class cursorHelpers {
+isCursorWithinNode(node *ts.Node, pos lsp.Position) bool
+isCursorWithinNodePoints(startPoint ts.Point, endPoint ts.Point, pos lsp.Position) bool
+isCursorAtLine(node *ts.Node, pos lsp.Position) bool
}
class getLineFn {
+getLine(document *string, line uint32) string
}
parser --> Command : uses
parser --> yamlLanguageVar : uses
parser --> parseYAMLDocumentFn : uses
parser --> cursorHelpers : uses
parser --> getLineFn : uses
yamlLanguageVar --> ts : uses
parseYAMLDocumentFn --> ts : uses
cursorHelpers --> ts : uses
Flow diagram for LSP YAML parsing with gotreesitterflowchart TD
A[Receive_document_and_optional_position] --> B[Call_parseYAMLDocument]
B --> C{Parse_error?}
C -- yes --> Z[Return_default_value_or_nil]
C -- no --> D[Obtain_tree_and_docBytes]
D --> E[Get_root_node_tree.RootNode]
E --> F{Root_is_nil?}
F -- yes --> Z
F -- no --> G[Create_query_ts.NewQuery_with_yamlLanguage]
G --> H{Query_error?}
H -- yes --> Z
H -- no --> I[Execute_query_query.Exec_with_root_and_docBytes]
I --> J[Iterate_matches_matches.NextMatch]
J --> K{More_matches?}
K -- no --> Z
K -- yes --> L[Iterate_captures_in_match]
L --> M{Capture_matches_target_name?}
M -- no --> J
M -- yes --> N[Use_cursor_helpers_isCursorWithinNode_or_isCursorAtLine]
N --> O{Cursor_or_line_matches?}
O -- no --> J
O -- yes --> P[Extract_Text_or_position_from_capture]
P --> Q{Caller_context}
Q -- inMixinsPosition --> R[Return_true_if_mixins_key_under_cursor]
Q -- inDependsPosition --> S[Return_true_if_depends_value_under_cursor]
Q -- extractFilenameFromMixins --> T[Return_filename_string]
Q -- getCommands --> U[Append_Command_to_slice]
Q -- getCurrentCommand --> V[Return_matching_Command]
Q -- findCommand --> W[Return_Command_matching_name]
Q -- extractDependsValues --> X[Append_depends_value_to_slice]
R --> J
S --> J
T --> J
U --> J
V --> J
W --> J
X --> J
Z[End_method_returning_bool_Command_or_slice]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="internal/lsp/treesitter_test.go" line_range="7" />
<code_context>
import (
"strings"
+ ts "github.com/odvcencio/gotreesitter"
+ "github.com/odvcencio/gotreesitter/grammars"
"github.com/tliron/commonlog"
</code_context>
<issue_to_address>
**suggestion (testing):** Add/extend tests to validate behaviour of YAML parsing helpers with gotreesitter
This change swaps the YAML parser and adds `parseYAMLDocument`, altering how nodes/queries are handled (points, text, types, named captures). To guard against regressions, please add or extend tests around the main helpers that rely on these behaviours:
- `inMixinsPosition`
- `inDependsPosition`
- `extractFilenameFromMixins`
- `getCommands` / `getCurrentCommand` / `findCommand`
- `extractDependsValues`
Ensure the tests cover both block and flow sequences, multiple items, and nested structures (e.g. several differently shaped commands). A small table-driven set of YAML snippets with expected positions/outputs should be enough to validate the new parser behaves like the old one.
Suggested implementation:
```golang
"reflect"
"testing"
ts "github.com/odvcencio/gotreesitter"
"github.com/odvcencio/gotreesitter/grammars"
"github.com/tliron/commonlog"
lsp "github.com/tliron/glsp/protocol_3_16"
)
var logger = commonlog.GetLogger("test")
// helper to parse a YAML document with the same parser implementation used in production
func mustParseYAMLDocument(t *testing.T, content string) *ts.Node {
t.Helper()
parser := ts.NewParser()
err := parser.SetLanguage(grammars.YAML())
if err != nil {
t.Fatalf("failed to set YAML grammar: %v", err)
}
tree, err := parser.Parse([]byte(content), nil)
if err != nil {
t.Fatalf("failed to parse YAML: %v", err)
}
root := tree.RootNode()
if root == nil {
t.Fatalf("nil root node for YAML content")
}
return root
}
// helper to create an LSP position
func pos(line, character uint32) lsp.Position {
return lsp.Position{
Line: line,
Character: character,
}
}
// Test inMixinsPosition with block and flow sequences and multiple items
func TestInMixinsPosition_BlockAndFlowSequences(t *testing.T) {
tests := []struct {
name string
content string
position lsp.Position
want bool
}{
{
name: "block sequence - inside first mixin",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
// position on "./common.yml"
position: pos(4, 10),
want: true,
},
{
name: "block sequence - inside second mixin",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
// position on "./other.yml"
position: pos(5, 10),
want: true,
},
{
name: "block sequence - outside mixins key",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
// somewhere on "tasks"
position: pos(1, 1),
want: false,
},
{
name: "flow sequence - inside first mixin",
content: `
tasks:
default:
mixins: [./common.yml, ./other.yml]
`,
position: pos(3, 15),
want: true,
},
{
name: "flow sequence - between mixin items",
content: `
tasks:
default:
mixins: [./common.yml, ./other.yml]
`,
position: pos(3, 25),
want: true,
},
{
name: "no mixins key",
content: `
tasks:
default:
cmds:
- echo "hello"
`,
position: pos(3, 5),
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root := mustParseYAMLDocument(t, tt.content)
got := inMixinsPosition(root, tt.position)
if got != tt.want {
t.Fatalf("inMixinsPosition() = %v, want %v", got, tt.want)
}
})
}
}
// Test inDependsPosition with block and flow sequences and nested tasks
func TestInDependsPosition_BlockFlowAndNested(t *testing.T) {
tests := []struct {
name string
content string
position lsp.Position
want bool
}{
{
name: "block sequence - inside depends value",
content: `
tasks:
build:
deps:
- clean
- compile
`,
position: pos(4, 10),
want: true,
},
{
name: "flow sequence - inside depends value",
content: `
tasks:
build:
deps: [clean, compile]
`,
position: pos(3, 15),
want: true,
},
{
name: "nested task - depends for nested task",
content: `
tasks:
build:
deps:
- clean
clean:
cmds:
- echo "clean"
`,
position: pos(4, 10),
want: true,
},
{
name: "outside depends key",
content: `
tasks:
build:
cmds:
- echo "build"
`,
position: pos(3, 5),
want: false,
},
{
name: "depends key but on key name, not value",
content: `
tasks:
build:
deps:
- clean
`,
// on "deps" identifier
position: pos(3, 4),
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root := mustParseYAMLDocument(t, tt.content)
got := inDependsPosition(root, tt.position)
if got != tt.want {
t.Fatalf("inDependsPosition() = %v, want %v", got, tt.want)
}
})
}
}
// Test extractFilenameFromMixins over multiple structures
func TestExtractFilenameFromMixins(t *testing.T) {
tests := []struct {
name string
content string
position lsp.Position
wantFile string
wantFound bool
}{
{
name: "block sequence - first element",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
position: pos(4, 12),
wantFile: "./common.yml",
wantFound: true,
},
{
name: "block sequence - second element",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
position: pos(5, 12),
wantFile: "./other.yml",
wantFound: true,
},
{
name: "flow sequence - first element",
content: `
tasks:
default:
mixins: [./common.yml, ./other.yml]
`,
position: pos(3, 16),
wantFile: "./common.yml",
wantFound: true,
},
{
name: "flow sequence - second element",
content: `
tasks:
default:
mixins: [./common.yml, ./other.yml]
`,
position: pos(3, 30),
wantFile: "./other.yml",
wantFound: true,
},
{
name: "outside mixins values",
content: `
tasks:
default:
mixins:
- ./common.yml
`,
position: pos(1, 1),
wantFile: "",
wantFound: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root := mustParseYAMLDocument(t, tt.content)
gotFile, gotFound := extractFilenameFromMixins(root, tt.position)
if gotFound != tt.wantFound {
t.Fatalf("extractFilenameFromMixins() found = %v, want %v", gotFound, tt.wantFound)
}
if gotFile != tt.wantFile {
t.Fatalf("extractFilenameFromMixins() file = %q, want %q", gotFile, tt.wantFile)
}
})
}
}
// commandInfo is the minimal shape we expect from getCommands/findCommand/getCurrentCommand.
// Adapt the concrete type if the actual implementation uses a different name/fields.
type commandInfo struct {
Name string
Depends []string
}
// Test getCommands, getCurrentCommand and findCommand with various command shapes
func TestCommandsHelpers_BlockAndFlowAndNested(t *testing.T) {
content := `
tasks:
default:
cmds:
- echo "default"
build:
deps:
- clean
- test
cmds:
- echo "build"
test:
deps: [lint]
cmds:
- echo "test"
lint:
cmds:
- echo "lint"
`
root := mustParseYAMLDocument(t, content)
// getCommands should return all commands with their names and depends
commands := getCommands(root)
// we expect at least the commands defined above; the exact type may differ
defaultCmd := findCommand(root, "default")
if defaultCmd == nil {
t.Fatalf("findCommand() did not return default command")
}
buildCmd := findCommand(root, "build")
if buildCmd == nil {
t.Fatalf("findCommand() did not return build command")
}
testCmd := findCommand(root, "test")
if testCmd == nil {
t.Fatalf("findCommand() did not return test command")
}
// getCurrentCommand: position within each command's body should map to that command
tests := []struct {
name string
position lsp.Position
wantName string
}{
{
name: "inside default command",
position: pos(4, 8),
wantName: "default",
},
{
name: "inside build command",
position: pos(8, 8),
wantName: "build",
},
{
name: "inside test command",
position: pos(13, 8),
wantName: "test",
},
{
name: "inside lint command",
position: pos(18, 8),
wantName: "lint",
},
}
if commands == nil {
t.Fatalf("getCommands() returned nil")
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd := getCurrentCommand(root, tt.position)
if cmd == nil {
t.Fatalf("getCurrentCommand() = nil, want command %q", tt.wantName)
}
name := cmd.Name
if name != tt.wantName {
t.Fatalf("getCurrentCommand().Name = %q, want %q", name, tt.wantName)
}
})
}
}
// Test extractDependsValues for both block and flow sequences
func TestExtractDependsValues_BlockAndFlow(t *testing.T) {
content := `
tasks:
blockDeps:
deps:
- clean
- build
cmds:
- echo "block"
flowDeps:
deps: [test, lint]
cmds:
- echo "flow"
mixedDeps:
deps:
- deploy
cmds:
- echo "mixed"
`
root := mustParseYAMLDocument(t, content)
blockCmd := findCommand(root, "blockDeps")
if blockCmd == nil {
t.Fatalf("findCommand() did not return blockDeps")
}
flowCmd := findCommand(root, "flowDeps")
if flowCmd == nil {
t.Fatalf("findCommand() did not return flowDeps")
}
mixedCmd := findCommand(root, "mixedDeps")
if mixedCmd == nil {
t.Fatalf("findCommand() did not return mixedDeps")
}
tests := []struct {
name string
command interface{}
wantDeps []string
}{
{
name: "block deps",
command: blockCmd,
wantDeps: []string{"clean", "build"},
},
{
name: "flow deps",
command: flowCmd,
wantDeps: []string{"test", "lint"},
},
{
name: "single dep",
command: mixedCmd,
wantDeps: []string{"deploy"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
deps := extractDependsValues(root, tt.command)
if !reflect.DeepEqual(deps, tt.wantDeps) {
t.Fatalf("extractDependsValues() = %#v, want %#v", deps, tt.wantDeps)
}
})
}
}
```
The new tests assume the following helper signatures and behaviours:
- `inMixinsPosition(root *ts.Node, position lsp.Position) bool`
- `inDependsPosition(root *ts.Node, position lsp.Position) bool`
- `extractFilenameFromMixins(root *ts.Node, position lsp.Position) (string, bool)`
- `getCommands(root *ts.Node) interface{}` (only checked for non-nil; adapt as needed)
- `getCurrentCommand(root *ts.Node, position lsp.Position) *commandInfo` (or another command type with a `Name` field)
- `findCommand(root *ts.Node, name string) *commandInfo` (or pointer to your concrete command type)
- `extractDependsValues(root *ts.Node, command interface{}) []string`
If your actual helpers have different signatures or types:
1. Adjust the tests to match the real types (e.g. replace `commandInfo` with your `command`/`task` type and update field accesses).
2. If `parseYAMLDocument` already exists as a shared helper, you can:
- Remove the local `mustParseYAMLDocument` helper and reuse the existing one, or
- Implement `mustParseYAMLDocument` as a thin wrapper around `parseYAMLDocument`.
3. The `position` values are based on the literal strings as written (0-based lines/columns). If your position mapping uses a different convention, tweak the `pos()` calls accordingly so the cursor falls on the intended nodes.
4. If `extractDependsValues` does not need the `root` node, drop that parameter from the tests and adjust the call site.
5. If `getCommands` returns a map or slice, update the tests to assert more detailed structure (e.g. command names, ranges) as appropriate for your implementation.
These tests are designed to validate consistent behaviour of the YAML helpers under the new `gotreesitter`-based parser, especially around block vs flow sequences, multiple items, and nested task structures. Adjust them to fit your concrete APIs while retaining the same coverage patterns.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "reflect" | ||
| "testing" | ||
|
|
||
| ts "github.com/odvcencio/gotreesitter" |
There was a problem hiding this comment.
suggestion (testing): Add/extend tests to validate behaviour of YAML parsing helpers with gotreesitter
This change swaps the YAML parser and adds parseYAMLDocument, altering how nodes/queries are handled (points, text, types, named captures). To guard against regressions, please add or extend tests around the main helpers that rely on these behaviours:
inMixinsPositioninDependsPositionextractFilenameFromMixinsgetCommands/getCurrentCommand/findCommandextractDependsValues
Ensure the tests cover both block and flow sequences, multiple items, and nested structures (e.g. several differently shaped commands). A small table-driven set of YAML snippets with expected positions/outputs should be enough to validate the new parser behaves like the old one.
Suggested implementation:
"reflect"
"testing"
ts "github.com/odvcencio/gotreesitter"
"github.com/odvcencio/gotreesitter/grammars"
"github.com/tliron/commonlog"
lsp "github.com/tliron/glsp/protocol_3_16"
)
var logger = commonlog.GetLogger("test")
// helper to parse a YAML document with the same parser implementation used in production
func mustParseYAMLDocument(t *testing.T, content string) *ts.Node {
t.Helper()
parser := ts.NewParser()
err := parser.SetLanguage(grammars.YAML())
if err != nil {
t.Fatalf("failed to set YAML grammar: %v", err)
}
tree, err := parser.Parse([]byte(content), nil)
if err != nil {
t.Fatalf("failed to parse YAML: %v", err)
}
root := tree.RootNode()
if root == nil {
t.Fatalf("nil root node for YAML content")
}
return root
}
// helper to create an LSP position
func pos(line, character uint32) lsp.Position {
return lsp.Position{
Line: line,
Character: character,
}
}
// Test inMixinsPosition with block and flow sequences and multiple items
func TestInMixinsPosition_BlockAndFlowSequences(t *testing.T) {
tests := []struct {
name string
content string
position lsp.Position
want bool
}{
{
name: "block sequence - inside first mixin",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
// position on "./common.yml"
position: pos(4, 10),
want: true,
},
{
name: "block sequence - inside second mixin",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
// position on "./other.yml"
position: pos(5, 10),
want: true,
},
{
name: "block sequence - outside mixins key",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
// somewhere on "tasks"
position: pos(1, 1),
want: false,
},
{
name: "flow sequence - inside first mixin",
content: `
tasks:
default:
mixins: [./common.yml, ./other.yml]
`,
position: pos(3, 15),
want: true,
},
{
name: "flow sequence - between mixin items",
content: `
tasks:
default:
mixins: [./common.yml, ./other.yml]
`,
position: pos(3, 25),
want: true,
},
{
name: "no mixins key",
content: `
tasks:
default:
cmds:
- echo "hello"
`,
position: pos(3, 5),
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root := mustParseYAMLDocument(t, tt.content)
got := inMixinsPosition(root, tt.position)
if got != tt.want {
t.Fatalf("inMixinsPosition() = %v, want %v", got, tt.want)
}
})
}
}
// Test inDependsPosition with block and flow sequences and nested tasks
func TestInDependsPosition_BlockFlowAndNested(t *testing.T) {
tests := []struct {
name string
content string
position lsp.Position
want bool
}{
{
name: "block sequence - inside depends value",
content: `
tasks:
build:
deps:
- clean
- compile
`,
position: pos(4, 10),
want: true,
},
{
name: "flow sequence - inside depends value",
content: `
tasks:
build:
deps: [clean, compile]
`,
position: pos(3, 15),
want: true,
},
{
name: "nested task - depends for nested task",
content: `
tasks:
build:
deps:
- clean
clean:
cmds:
- echo "clean"
`,
position: pos(4, 10),
want: true,
},
{
name: "outside depends key",
content: `
tasks:
build:
cmds:
- echo "build"
`,
position: pos(3, 5),
want: false,
},
{
name: "depends key but on key name, not value",
content: `
tasks:
build:
deps:
- clean
`,
// on "deps" identifier
position: pos(3, 4),
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root := mustParseYAMLDocument(t, tt.content)
got := inDependsPosition(root, tt.position)
if got != tt.want {
t.Fatalf("inDependsPosition() = %v, want %v", got, tt.want)
}
})
}
}
// Test extractFilenameFromMixins over multiple structures
func TestExtractFilenameFromMixins(t *testing.T) {
tests := []struct {
name string
content string
position lsp.Position
wantFile string
wantFound bool
}{
{
name: "block sequence - first element",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
position: pos(4, 12),
wantFile: "./common.yml",
wantFound: true,
},
{
name: "block sequence - second element",
content: `
tasks:
default:
mixins:
- ./common.yml
- ./other.yml
`,
position: pos(5, 12),
wantFile: "./other.yml",
wantFound: true,
},
{
name: "flow sequence - first element",
content: `
tasks:
default:
mixins: [./common.yml, ./other.yml]
`,
position: pos(3, 16),
wantFile: "./common.yml",
wantFound: true,
},
{
name: "flow sequence - second element",
content: `
tasks:
default:
mixins: [./common.yml, ./other.yml]
`,
position: pos(3, 30),
wantFile: "./other.yml",
wantFound: true,
},
{
name: "outside mixins values",
content: `
tasks:
default:
mixins:
- ./common.yml
`,
position: pos(1, 1),
wantFile: "",
wantFound: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
root := mustParseYAMLDocument(t, tt.content)
gotFile, gotFound := extractFilenameFromMixins(root, tt.position)
if gotFound != tt.wantFound {
t.Fatalf("extractFilenameFromMixins() found = %v, want %v", gotFound, tt.wantFound)
}
if gotFile != tt.wantFile {
t.Fatalf("extractFilenameFromMixins() file = %q, want %q", gotFile, tt.wantFile)
}
})
}
}
// commandInfo is the minimal shape we expect from getCommands/findCommand/getCurrentCommand.
// Adapt the concrete type if the actual implementation uses a different name/fields.
type commandInfo struct {
Name string
Depends []string
}
// Test getCommands, getCurrentCommand and findCommand with various command shapes
func TestCommandsHelpers_BlockAndFlowAndNested(t *testing.T) {
content := `
tasks:
default:
cmds:
- echo "default"
build:
deps:
- clean
- test
cmds:
- echo "build"
test:
deps: [lint]
cmds:
- echo "test"
lint:
cmds:
- echo "lint"
`
root := mustParseYAMLDocument(t, content)
// getCommands should return all commands with their names and depends
commands := getCommands(root)
// we expect at least the commands defined above; the exact type may differ
defaultCmd := findCommand(root, "default")
if defaultCmd == nil {
t.Fatalf("findCommand() did not return default command")
}
buildCmd := findCommand(root, "build")
if buildCmd == nil {
t.Fatalf("findCommand() did not return build command")
}
testCmd := findCommand(root, "test")
if testCmd == nil {
t.Fatalf("findCommand() did not return test command")
}
// getCurrentCommand: position within each command's body should map to that command
tests := []struct {
name string
position lsp.Position
wantName string
}{
{
name: "inside default command",
position: pos(4, 8),
wantName: "default",
},
{
name: "inside build command",
position: pos(8, 8),
wantName: "build",
},
{
name: "inside test command",
position: pos(13, 8),
wantName: "test",
},
{
name: "inside lint command",
position: pos(18, 8),
wantName: "lint",
},
}
if commands == nil {
t.Fatalf("getCommands() returned nil")
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cmd := getCurrentCommand(root, tt.position)
if cmd == nil {
t.Fatalf("getCurrentCommand() = nil, want command %q", tt.wantName)
}
name := cmd.Name
if name != tt.wantName {
t.Fatalf("getCurrentCommand().Name = %q, want %q", name, tt.wantName)
}
})
}
}
// Test extractDependsValues for both block and flow sequences
func TestExtractDependsValues_BlockAndFlow(t *testing.T) {
content := `
tasks:
blockDeps:
deps:
- clean
- build
cmds:
- echo "block"
flowDeps:
deps: [test, lint]
cmds:
- echo "flow"
mixedDeps:
deps:
- deploy
cmds:
- echo "mixed"
`
root := mustParseYAMLDocument(t, content)
blockCmd := findCommand(root, "blockDeps")
if blockCmd == nil {
t.Fatalf("findCommand() did not return blockDeps")
}
flowCmd := findCommand(root, "flowDeps")
if flowCmd == nil {
t.Fatalf("findCommand() did not return flowDeps")
}
mixedCmd := findCommand(root, "mixedDeps")
if mixedCmd == nil {
t.Fatalf("findCommand() did not return mixedDeps")
}
tests := []struct {
name string
command interface{}
wantDeps []string
}{
{
name: "block deps",
command: blockCmd,
wantDeps: []string{"clean", "build"},
},
{
name: "flow deps",
command: flowCmd,
wantDeps: []string{"test", "lint"},
},
{
name: "single dep",
command: mixedCmd,
wantDeps: []string{"deploy"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
deps := extractDependsValues(root, tt.command)
if !reflect.DeepEqual(deps, tt.wantDeps) {
t.Fatalf("extractDependsValues() = %#v, want %#v", deps, tt.wantDeps)
}
})
}
}The new tests assume the following helper signatures and behaviours:
inMixinsPosition(root *ts.Node, position lsp.Position) boolinDependsPosition(root *ts.Node, position lsp.Position) boolextractFilenameFromMixins(root *ts.Node, position lsp.Position) (string, bool)getCommands(root *ts.Node) interface{}(only checked for non-nil; adapt as needed)getCurrentCommand(root *ts.Node, position lsp.Position) *commandInfo(or another command type with aNamefield)findCommand(root *ts.Node, name string) *commandInfo(or pointer to your concrete command type)extractDependsValues(root *ts.Node, command interface{}) []string
If your actual helpers have different signatures or types:
- Adjust the tests to match the real types (e.g. replace
commandInfowith yourcommand/tasktype and update field accesses). - If
parseYAMLDocumentalready exists as a shared helper, you can:- Remove the local
mustParseYAMLDocumenthelper and reuse the existing one, or - Implement
mustParseYAMLDocumentas a thin wrapper aroundparseYAMLDocument.
- Remove the local
- The
positionvalues are based on the literal strings as written (0-based lines/columns). If your position mapping uses a different convention, tweak thepos()calls accordingly so the cursor falls on the intended nodes. - If
extractDependsValuesdoes not need therootnode, drop that parameter from the tests and adjust the call site. - If
getCommandsreturns a map or slice, update the tests to assert more detailed structure (e.g. command names, ranges) as appropriate for your implementation.
These tests are designed to validate consistent behaviour of the YAML helpers under the new gotreesitter-based parser, especially around block vs flow sequences, multiple items, and nested task structures. Adjust them to fit your concrete APIs while retaining the same coverage patterns.
Contributor
Greptile SummaryThis PR replaces the CGo-based Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant LSP as LSP Handler
participant P as parser
participant H as parseYAMLDocument()
participant GT as gotreesitter
participant G as grammars.YamlLanguage
Note over G,GT: Package init — yamlLanguage loaded once
G-->>GT: YamlLanguage()
LSP->>P: inMixinsPosition / inDependsPosition / getCommands / etc.
P->>H: parseYAMLDocument(document)
H->>GT: ts.NewParser(yamlLanguage).Parse(docBytes)
GT-->>H: *Tree, error
H-->>P: *Tree, []byte, error
P->>GT: ts.NewQuery(queryStr, yamlLanguage)
GT-->>P: *Query
P->>GT: query.Exec(root, yamlLanguage, docBytes)
GT-->>P: MatchIterator
loop NextMatch()
P->>GT: matches.NextMatch()
GT-->>P: Match, ok
P->>GT: capture.Node.Text / Type / Parent / StartPoint
GT-->>P: result
end
P->>GT: tree.Release()
P-->>LSP: bool / string / []Command
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+82
to
+83
| github.com/odvcencio/gotreesitter v0.9.2 h1:ZROpRS+bTcC1mwofBp53l66Jv00FH0ccViSwGVmaBBM= | ||
| github.com/odvcencio/gotreesitter v0.9.2/go.mod h1:Sx+iYJBfw5xSWkSttLSuFvguJctlH+ma1BTxZ0MPCqo= |
Contributor
There was a problem hiding this comment.
Replacing go-tree-sitter with gotreesitter has caused stretchr/testify to drop from v1.9.0 to v1.7.0 as a transitive dependency (via gotreesitter's own test dependencies). Since testify is not directly imported anywhere in this module, the behavioral impact is minimal. However, this is worth being aware of in case any of the module's own transitive dependencies require v1.9.0 or above, which could cause unexpected go mod tidy conflicts down the line.
kindermax
force-pushed
the
codex/adopt-gotreesitter-for-project
branch
3 times, most recently
from
a2d0763 to
3aedb47
Compare
kindermax
force-pushed
the
codex/adopt-gotreesitter-for-project
branch
from
3aedb47 to
2234be0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
gotreesitterclient, updating parser helpers and queries accordinglyTesting
Summary by Sourcery
Migrate the LSP YAML parsing logic to use the pure-Go gotreesitter client instead of CGO-based tree-sitter bindings, and remove associated CGO/toolchain requirements from builds and release workflows.
Enhancements:
Build:
Documentation: