From 9a7e58bb5905656b7bf0837424913d69cc451982 Mon Sep 17 00:00:00 2001 From: qvalentin Date: Thu, 5 Sep 2024 18:39:47 +0200 Subject: [PATCH] feat: refactor document --- internal/adapter/yamlls/diagnostics.go | 4 +- internal/adapter/yamlls/document_sync.go | 12 ++-- .../generic_document_usecase.go | 2 +- internal/lsp/ast.go | 6 +- internal/lsp/ast_diagnostics_test.go | 2 +- internal/lsp/ast_test.go | 2 +- internal/lsp/document.go | 66 ++----------------- internal/lsp/document_store.go | 46 +++++++------ internal/lsp/lint.go | 2 +- internal/lsp/lint_test.go | 10 ++- .../lsp/symbol_table_template_context_test.go | 2 +- ...l_table_template_context_variables_test.go | 2 +- internal/lsp/symbol_table_test.go | 10 +-- internal/lsp/symbol_table_variables_test.go | 2 +- internal/lsp/variables_visitor_test.go | 4 +- internal/lsp/yaml_ast.go | 2 +- internal/lsp/yaml_ast_test.go | 6 +- 17 files changed, 69 insertions(+), 111 deletions(-) diff --git a/internal/adapter/yamlls/diagnostics.go b/internal/adapter/yamlls/diagnostics.go index 7a600a44..ebc6306d 100644 --- a/internal/adapter/yamlls/diagnostics.go +++ b/internal/adapter/yamlls/diagnostics.go @@ -32,7 +32,7 @@ func (c Connector) PublishDiagnostics(ctx context.Context, params *protocol.Publ return nil } -func filterDiagnostics(diagnostics []lsp.Diagnostic, ast *sitter.Tree, content string) (filtered []lsp.Diagnostic) { +func filterDiagnostics(diagnostics []lsp.Diagnostic, ast *sitter.Tree, content []byte) (filtered []lsp.Diagnostic) { filtered = []lsp.Diagnostic{} for _, diagnostic := range diagnostics { @@ -41,7 +41,7 @@ func filterDiagnostics(diagnostics []lsp.Diagnostic, ast *sitter.Tree, content s if node.Type() == "text" && childNode.Type() == "text" { logger.Debug("Diagnostic", diagnostic) - logger.Debug("Node", node.Content([]byte(content))) + logger.Debug("Node", node.Content(content)) if diagnisticIsRelevant(diagnostic, childNode) { diagnostic.Message = "Yamlls: " + diagnostic.Message diff --git a/internal/adapter/yamlls/document_sync.go b/internal/adapter/yamlls/document_sync.go index 45b0e6cc..f18ccb7f 100644 --- a/internal/adapter/yamlls/document_sync.go +++ b/internal/adapter/yamlls/document_sync.go @@ -9,7 +9,7 @@ import ( lsp "go.lsp.dev/protocol" ) -func (yamllsConnector Connector) InitiallySyncOpenDocuments(docs []*lsplocal.Document) { +func (yamllsConnector Connector) InitiallySyncOpenDocuments(docs []*lsplocal.TemplateDocument) { if yamllsConnector.server == nil { return } @@ -27,7 +27,7 @@ func (yamllsConnector Connector) InitiallySyncOpenDocuments(docs []*lsplocal.Doc yamllsConnector.DocumentDidOpen(doc.Ast, lsp.DidOpenTextDocumentParams{ TextDocument: lsp.TextDocumentItem{ URI: doc.URI, - Text: doc.Content, + Text: string(doc.Content), }, }) } @@ -39,7 +39,7 @@ func (yamllsConnector Connector) DocumentDidOpen(ast *sitter.Tree, params lsp.Di if !yamllsConnector.shouldRun(params.TextDocument.URI) { return } - params.TextDocument.Text = lsplocal.TrimTemplate(ast, params.TextDocument.Text) + params.TextDocument.Text = lsplocal.TrimTemplate(ast, []byte(params.TextDocument.Text)) err := yamllsConnector.server.DidOpen(context.Background(), ¶ms) if err != nil { @@ -47,7 +47,7 @@ func (yamllsConnector Connector) DocumentDidOpen(ast *sitter.Tree, params lsp.Di } } -func (yamllsConnector Connector) DocumentDidSave(doc *lsplocal.Document, params lsp.DidSaveTextDocumentParams) { +func (yamllsConnector Connector) DocumentDidSave(doc *lsplocal.TemplateDocument, params lsp.DidSaveTextDocumentParams) { if !yamllsConnector.shouldRun(doc.URI) { return } @@ -66,7 +66,7 @@ func (yamllsConnector Connector) DocumentDidSave(doc *lsplocal.Document, params }) } -func (yamllsConnector Connector) DocumentDidChange(doc *lsplocal.Document, params lsp.DidChangeTextDocumentParams) { +func (yamllsConnector Connector) DocumentDidChange(doc *lsplocal.TemplateDocument, params lsp.DidChangeTextDocumentParams) { if !yamllsConnector.shouldRun(doc.URI) { return } @@ -95,7 +95,7 @@ func (yamllsConnector Connector) DocumentDidChange(doc *lsplocal.Document, param } } -func (yamllsConnector Connector) DocumentDidChangeFullSync(doc *lsplocal.Document, params lsp.DidChangeTextDocumentParams) { +func (yamllsConnector Connector) DocumentDidChangeFullSync(doc *lsplocal.TemplateDocument, params lsp.DidChangeTextDocumentParams) { if !yamllsConnector.shouldRun(doc.URI) { return } diff --git a/internal/language_features/generic_document_usecase.go b/internal/language_features/generic_document_usecase.go index 23afbbf9..3943cd98 100644 --- a/internal/language_features/generic_document_usecase.go +++ b/internal/language_features/generic_document_usecase.go @@ -7,7 +7,7 @@ import ( ) type GenericDocumentUseCase struct { - Document *lsplocal.Document + Document *lsplocal.TemplateDocument DocumentStore *lsplocal.DocumentStore Chart *charts.Chart Node *sitter.Node diff --git a/internal/lsp/ast.go b/internal/lsp/ast.go index e82b8c81..b32f5122 100644 --- a/internal/lsp/ast.go +++ b/internal/lsp/ast.go @@ -8,10 +8,10 @@ import ( lsp "go.lsp.dev/protocol" ) -func ParseAst(oldTree *sitter.Tree, content string) *sitter.Tree { +func ParseAst(oldTree *sitter.Tree, content []byte) *sitter.Tree { parser := sitter.NewParser() parser.SetLanguage(gotemplate.GetLanguage()) - tree, _ := parser.ParseCtx(context.Background(), oldTree, []byte(content)) + tree, _ := parser.ParseCtx(context.Background(), oldTree, content) return tree } @@ -69,7 +69,7 @@ func isPointLargerOrEq(a sitter.Point, b sitter.Point) bool { return a.Row > b.Row } -func (d *Document) ApplyChangesToAst(newContent string) { +func (d *TemplateDocument) ApplyChangesToAst(newContent []byte) { d.Ast = ParseAst(nil, newContent) } diff --git a/internal/lsp/ast_diagnostics_test.go b/internal/lsp/ast_diagnostics_test.go index c3354598..c9183518 100644 --- a/internal/lsp/ast_diagnostics_test.go +++ b/internal/lsp/ast_diagnostics_test.go @@ -8,7 +8,7 @@ import ( func TestIsInElseBranch(t *testing.T) { template := `{{if pipeline}} t1 {{ else if pipeline }} t2 {{ else if pipeline2 }} t3 {{ else }} t4 {{ end }}` - ast := ParseAst(nil, template) + ast := ParseAst(nil, []byte(template)) // (template [0, 0] - [1, 0] // (if_action [0, 0] - [0, 95] // condition: (function_call [0, 5] - [0, 13] diff --git a/internal/lsp/ast_test.go b/internal/lsp/ast_test.go index de22a9fa..87718612 100644 --- a/internal/lsp/ast_test.go +++ b/internal/lsp/ast_test.go @@ -16,7 +16,7 @@ func TestFindRelevantChildNodeCompletio(t *testing.T) { {{ .Chart.N }} {{ . }} ` - ast := ParseAst(nil, template) + ast := ParseAst(nil, []byte(template)) logger.Println("RootNode:", ast.RootNode().String()) diff --git a/internal/lsp/document.go b/internal/lsp/document.go index 00fb9a57..f3dd529c 100644 --- a/internal/lsp/document.go +++ b/internal/lsp/document.go @@ -3,28 +3,19 @@ package lsp import ( "bytes" "fmt" - "strings" "github.com/mrjosh/helm-ls/internal/util" - sitter "github.com/smacker/go-tree-sitter" lsp "go.lsp.dev/protocol" ) -// Document represents an opened file. type Document struct { - URI lsp.DocumentURI - Path string - NeedsRefreshDiagnostics bool - Content string - lines []string - Ast *sitter.Tree - DiagnosticsCache DiagnosticsCache - IsOpen bool - SymbolTable *SymbolTable - IsYaml bool + URI lsp.DocumentURI + Path string + Content []byte + lines []string + IsOpen bool } -// ApplyChanges updates the content of the document from LSP textDocument/didChange events. func (d *Document) ApplyChanges(changes []lsp.TextDocumentContentChangeEvent) { defer func() { if r := recover(); r != nil { @@ -32,7 +23,7 @@ func (d *Document) ApplyChanges(changes []lsp.TextDocumentContentChangeEvent) { } }() - content := []byte(d.Content) + content := d.Content for _, change := range changes { start, end := util.PositionToIndex(change.Range.Start, content), util.PositionToIndex(change.Range.End, content) @@ -42,49 +33,6 @@ func (d *Document) ApplyChanges(changes []lsp.TextDocumentContentChangeEvent) { buf.Write(content[end:]) content = buf.Bytes() } - d.Content = string(content) - - d.ApplyChangesToAst(d.Content) - d.SymbolTable = NewSymbolTable(d.Ast, content) - + d.Content = content d.lines = nil } - -// WordAt returns the word found at the given location. -func (d *Document) WordAt(pos lsp.Position) string { - logger.Debug(pos) - - line, ok := d.getLine(int(pos.Line)) - if !ok { - return "" - } - return util.WordAt(line, int(pos.Character)) -} - -// getLine returns the line at the given index. -func (d *Document) getLine(index int) (string, bool) { - lines := d.getLines() - if index < 0 || index > len(lines) { - return "", false - } - return lines[index], true -} - -// getLines returns all the lines in the document. -func (d *Document) getLines() []string { - if d.lines == nil { - // We keep \r on purpose, to avoid messing up position conversions. - d.lines = strings.Split(d.Content, "\n") - } - return d.lines -} - -// GetContent implements PossibleDependencyFile. -func (d *Document) GetContent() []byte { - return []byte(d.Content) -} - -// GetPath implements PossibleDependencyFile. -func (d *Document) GetPath() string { - return d.Path -} diff --git a/internal/lsp/document_store.go b/internal/lsp/document_store.go index b7cb0a41..e7f36af3 100644 --- a/internal/lsp/document_store.go +++ b/internal/lsp/document_store.go @@ -20,28 +20,30 @@ func NewDocumentStore() *DocumentStore { } } -func (s *DocumentStore) GetAllDocs() []*Document { - var docs []*Document +func (s *DocumentStore) GetAllDocs() []*TemplateDocument { + var docs []*TemplateDocument s.documents.Range(func(_, v interface{}) bool { - docs = append(docs, v.(*Document)) + docs = append(docs, v.(*TemplateDocument)) return true }) return docs } -func (s *DocumentStore) DidOpen(params *lsp.DidOpenTextDocumentParams, helmlsConfig util.HelmlsConfiguration) (*Document, error) { +func (s *DocumentStore) DidOpen(params *lsp.DidOpenTextDocumentParams, helmlsConfig util.HelmlsConfiguration) (*TemplateDocument, error) { logger.Debug(fmt.Sprintf("Opening document %s with langID %s", params.TextDocument.URI, params.TextDocument.LanguageID)) uri := params.TextDocument.URI path := uri.Filename() - ast := ParseAst(nil, params.TextDocument.Text) - doc := &Document{ - URI: uri, - Path: path, - Content: params.TextDocument.Text, + ast := ParseAst(nil, []byte(params.TextDocument.Text)) + doc := &TemplateDocument{ + Document: Document{ + URI: uri, + Path: path, + Content: []byte(params.TextDocument.Text), + IsOpen: true, + }, Ast: ast, DiagnosticsCache: NewDiagnosticsCache(helmlsConfig), - IsOpen: true, SymbolTable: NewSymbolTable(ast, []byte(params.TextDocument.Text)), IsYaml: IsYamlDocument(uri, helmlsConfig.YamllsConfiguration), } @@ -50,35 +52,37 @@ func (s *DocumentStore) DidOpen(params *lsp.DidOpenTextDocumentParams, helmlsCon return doc, nil } -func (s *DocumentStore) Store(filename string, content []byte, helmlsConfig util.HelmlsConfiguration) { - _, ok := s.documents.Load(filename) +func (s *DocumentStore) Store(path string, content []byte, helmlsConfig util.HelmlsConfiguration) { + _, ok := s.documents.Load(path) if ok { return } - ast := ParseAst(nil, string(content)) - fileURI := uri.File(filename) + ast := ParseAst(nil, content) + fileURI := uri.File(path) s.documents.Store(fileURI.Filename(), - &Document{ - URI: fileURI, - Path: filename, - Content: string(content), + &TemplateDocument{ + Document: Document{ + URI: fileURI, + Path: path, + Content: content, + IsOpen: false, + }, Ast: ast, DiagnosticsCache: NewDiagnosticsCache(helmlsConfig), - IsOpen: false, SymbolTable: NewSymbolTable(ast, content), IsYaml: IsYamlDocument(fileURI, helmlsConfig.YamllsConfiguration), }, ) } -func (s *DocumentStore) Get(docuri uri.URI) (*Document, bool) { +func (s *DocumentStore) Get(docuri uri.URI) (*TemplateDocument, bool) { path := docuri.Filename() d, ok := s.documents.Load(path) if !ok { return nil, false } - return d.(*Document), ok + return d.(*TemplateDocument), ok } func IsYamlDocument(uri lsp.URI, yamllsConfiguration util.YamllsConfiguration) bool { diff --git a/internal/lsp/lint.go b/internal/lsp/lint.go index c449286a..2aeb928e 100644 --- a/internal/lsp/lint.go +++ b/internal/lsp/lint.go @@ -21,7 +21,7 @@ import ( var logger = log.GetLogger() -func GetDiagnosticsNotifications(chart *charts.Chart, doc *Document) []lsp.PublishDiagnosticsParams { +func GetDiagnosticsNotifications(chart *charts.Chart, doc *TemplateDocument) []lsp.PublishDiagnosticsParams { vals := chart.ValuesFiles.MainValuesFile.Values if chart.ValuesFiles.OverlayValuesFile != nil { vals = chartutil.CoalesceTables(chart.ValuesFiles.OverlayValuesFile.Values, chart.ValuesFiles.MainValuesFile.Values) diff --git a/internal/lsp/lint_test.go b/internal/lsp/lint_test.go index ba8968ff..7d022033 100644 --- a/internal/lsp/lint_test.go +++ b/internal/lsp/lint_test.go @@ -25,7 +25,9 @@ func TestLintNotifications(t *testing.T) { AdditionalValuesFiles: []*charts.ValuesFile{}, }, } - diagnostics := GetDiagnosticsNotifications(&chart, &Document{URI: uri.File("../../testdata/example/templates/deployment-no-templates.yaml")}) + diagnostics := GetDiagnosticsNotifications(&chart, &TemplateDocument{ + Document: Document{URI: uri.File("../../testdata/example/templates/deployment-no-templates.yaml")}, + }) assert.NotEmpty(t, diagnostics) assert.Len(t, diagnostics, 3) @@ -50,7 +52,11 @@ func TestLintNotificationsIncludesEmptyDiagnosticsForFixedIssues(t *testing.T) { AdditionalValuesFiles: []*charts.ValuesFile{}, }, } - diagnostics := GetDiagnosticsNotifications(&chart, &Document{URI: uri.File("../../testdata/example/templates/deployment-no-templates.yaml")}) + diagnostics := GetDiagnosticsNotifications(&chart, &TemplateDocument{ + Document: Document{ + URI: uri.File("../../testdata/example/templates/deployment-no-templates.yaml"), + }, + }) uris := []string{} for _, notification := range diagnostics { diff --git a/internal/lsp/symbol_table_template_context_test.go b/internal/lsp/symbol_table_template_context_test.go index e9e12312..06b241cb 100644 --- a/internal/lsp/symbol_table_template_context_test.go +++ b/internal/lsp/symbol_table_template_context_test.go @@ -52,7 +52,7 @@ func TestGetContextForSelectorExpression(t *testing.T) { } for _, tC := range testCases { t.Run(tC.desc, func(t *testing.T) { - ast := ParseAst(nil, tC.template) + ast := ParseAst(nil, []byte(tC.template)) node := ast.RootNode().Child(1) assert.Equal(t, tC.nodeContent, node.Content([]byte(tC.template))) diff --git a/internal/lsp/symbol_table_template_context_variables_test.go b/internal/lsp/symbol_table_template_context_variables_test.go index b5e6c584..ce2f7518 100644 --- a/internal/lsp/symbol_table_template_context_variables_test.go +++ b/internal/lsp/symbol_table_template_context_variables_test.go @@ -24,7 +24,7 @@ func TestResolveVariablesInTemplateContext(t *testing.T) { t.Run(tt.template, func(t *testing.T) { col := strings.Index(tt.template, "^") buf := strings.Replace(tt.template, "^", "", 1) - ast := ParseAst(nil, tt.template) + ast := ParseAst(nil, []byte(tt.template)) symbolTable := NewSymbolTable(ast, []byte(buf)) result, err := symbolTable.ResolveVariablesInTemplateContext( diff --git a/internal/lsp/symbol_table_test.go b/internal/lsp/symbol_table_test.go index 96d30fa5..e53d9480 100644 --- a/internal/lsp/symbol_table_test.go +++ b/internal/lsp/symbol_table_test.go @@ -19,7 +19,7 @@ func TestSymbolTableForIncludeDefinitions(t *testing.T) { {{ end }} ` - ast := ParseAst(nil, content) + ast := ParseAst(nil, []byte(content)) symbolTable := NewSymbolTable(ast, []byte(content)) @@ -68,7 +68,7 @@ func TestSymbolTableForValues(t *testing.T) { {{ end }} ` - ast := ParseAst(nil, content) + ast := ParseAst(nil, []byte(content)) symbolTable := NewSymbolTable(ast, []byte(content)) type expectedValue struct { @@ -187,9 +187,9 @@ func TestSymbolTableForValuesTestFile(t *testing.T) { if err != nil { t.Fatal("Could not read test file", err) } - ast := ParseAst(nil, string(content)) + ast := ParseAst(nil, content) - symbolTable := NewSymbolTable(ast, []byte(content)) + symbolTable := NewSymbolTable(ast, content) type expectedValue struct { path []string startPoint sitter.Point @@ -324,7 +324,7 @@ func TestSymbolTableForValuesSingleTests(t *testing.T) { for _, v := range testCases { t.Run(v.template, func(t *testing.T) { - ast := ParseAst(nil, v.template) + ast := ParseAst(nil, []byte(v.template)) symbolTable := NewSymbolTable(ast, []byte(v.template)) values := symbolTable.GetTemplateContextRanges(v.path) points := []sitter.Point{} diff --git a/internal/lsp/symbol_table_variables_test.go b/internal/lsp/symbol_table_variables_test.go index 405ceea2..de14034d 100644 --- a/internal/lsp/symbol_table_variables_test.go +++ b/internal/lsp/symbol_table_variables_test.go @@ -77,7 +77,7 @@ func TestGetVariableDefinition(t *testing.T) { } for _, tC := range testCases { t.Run(tC.template, func(t *testing.T) { - ast := ParseAst(nil, tC.template) + ast := ParseAst(nil, []byte(tC.template)) symbolTable := NewSymbolTable(ast, []byte(tC.template)) result, err := symbolTable.getVariableDefinition(tC.variableName, tC.accessRange) assert.Equal(t, tC.expectedError, err) diff --git a/internal/lsp/variables_visitor_test.go b/internal/lsp/variables_visitor_test.go index 30e88af2..f21f69ed 100644 --- a/internal/lsp/variables_visitor_test.go +++ b/internal/lsp/variables_visitor_test.go @@ -70,7 +70,7 @@ func TestSymbolTableForVariableDefinitions(t *testing.T) { for _, v := range testCases { t.Run(v.template, func(t *testing.T) { - ast := ParseAst(nil, v.template) + ast := ParseAst(nil, []byte(v.template)) symbolTable := NewSymbolTable(ast, []byte(v.template)) assert.Equal(t, v.expectedVariableDefinitions, symbolTable.variableDefinitions, fmt.Sprintf("Ast was %s", ast.RootNode())) @@ -97,7 +97,7 @@ func TestSymbolTableForVariableUsages(t *testing.T) { for _, v := range testCases { t.Run(v.template, func(t *testing.T) { - ast := ParseAst(nil, v.template) + ast := ParseAst(nil, []byte(v.template)) symbolTable := NewSymbolTable(ast, []byte(v.template)) assert.Equal(t, v.expectedVariableUsages, symbolTable.variableUsages, fmt.Sprintf("Ast was %s", ast.RootNode())) diff --git a/internal/lsp/yaml_ast.go b/internal/lsp/yaml_ast.go index 8d3f03d1..9b0f54a3 100644 --- a/internal/lsp/yaml_ast.go +++ b/internal/lsp/yaml_ast.go @@ -32,7 +32,7 @@ func getTextNodeRanges(gotemplateNode *sitter.Node) []sitter.Range { // This is done by keeping only the text nodes // which is easier then removing the template nodes // since template nodes could contain other nodes -func TrimTemplate(gotemplateTree *sitter.Tree, content string) string { +func TrimTemplate(gotemplateTree *sitter.Tree, content []byte) string { ranges := getTextNodeRanges(gotemplateTree.RootNode()) result := make([]byte, len(content)) for i := range result { diff --git a/internal/lsp/yaml_ast_test.go b/internal/lsp/yaml_ast_test.go index 05b4bded..02d13e3f 100644 --- a/internal/lsp/yaml_ast_test.go +++ b/internal/lsp/yaml_ast_test.go @@ -78,7 +78,7 @@ b: not`, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := getTextNodeRanges(ParseAst(nil, tt.args.gotemplateString).RootNode()) + got := getTextNodeRanges(ParseAst(nil, []byte(tt.args.gotemplateString)).RootNode()) assert.Equal(t, tt.want, got) }) } @@ -121,8 +121,8 @@ yaml: test } for _, tt := range tests { t.Run(tt.documentText, func(t *testing.T) { - gotemplateTree := ParseAst(nil, tt.documentText) - got := TrimTemplate(gotemplateTree, tt.documentText) + gotemplateTree := ParseAst(nil, []byte(tt.documentText)) + got := TrimTemplate(gotemplateTree, []byte(tt.documentText)) assert.Equal(t, tt.trimmedText, got) }) }