From a0c0ccce2575fc0072c7f7f4521437a4d00a3cfa Mon Sep 17 00:00:00 2001 From: Myzel394 <50424412+Myzel394@users.noreply.github.com> Date: Tue, 1 Oct 2024 16:22:32 +0200 Subject: [PATCH] refactor(ssh_config): Use Diagnostics instead of LSPError --- handlers/ssh_config/analyzer/analyzer.go | 9 +-- handlers/ssh_config/analyzer/block.go | 24 +++---- handlers/ssh_config/analyzer/block_test.go | 12 +++- handlers/ssh_config/analyzer/dependents.go | 35 ++++------ handlers/ssh_config/analyzer/match.go | 72 ++++++++++----------- handlers/ssh_config/analyzer/match_test.go | 12 +++- handlers/ssh_config/analyzer/quotes_test.go | 22 +++++-- 7 files changed, 99 insertions(+), 87 deletions(-) diff --git a/handlers/ssh_config/analyzer/analyzer.go b/handlers/ssh_config/analyzer/analyzer.go index 5f05f1c..2167966 100644 --- a/handlers/ssh_config/analyzer/analyzer.go +++ b/handlers/ssh_config/analyzer/analyzer.go @@ -38,12 +38,9 @@ func Analyze( } analyzeValuesAreValid(ctx) + analyzeDependents(ctx) + analyzeBlocks(ctx) + analyzeMatchBlocks(ctx) return ctx.diagnostics - - errors = append(errors, analyzeDependents(d)...) - errors = append(errors, analyzeBlocks(d)...) - errors = append(errors, analyzeMatchBlocks(d)...) - - return common.ErrsToDiagnostics(errors) } diff --git a/handlers/ssh_config/analyzer/block.go b/handlers/ssh_config/analyzer/block.go index 704137e..1eb48f1 100644 --- a/handlers/ssh_config/analyzer/block.go +++ b/handlers/ssh_config/analyzer/block.go @@ -2,23 +2,23 @@ package analyzer import ( "config-lsp/common" - sshconfig "config-lsp/handlers/ssh_config" - "errors" + + protocol "github.com/tliron/glsp/protocol_3_16" ) func analyzeBlocks( - d *sshconfig.SSHDocument, -) []common.LSPError { - errs := make([]common.LSPError, 0) - - for _, block := range d.GetAllBlocks() { + ctx *analyzerContext, +) { + for _, block := range ctx.document.GetAllBlocks() { if block.GetOptions().Size() == 0 { - errs = append(errs, common.LSPError{ - Range: block.GetEntryOption().LocationRange, - Err: errors.New("This block is empty"), + ctx.diagnostics = append(ctx.diagnostics, protocol.Diagnostic{ + Range: block.GetEntryOption().LocationRange.ToLSPRange(), + Message: "This block is empty", + Severity: &common.SeverityHint, + Tags: []protocol.DiagnosticTag{ + protocol.DiagnosticTagUnnecessary, + }, }) } } - - return errs } diff --git a/handlers/ssh_config/analyzer/block_test.go b/handlers/ssh_config/analyzer/block_test.go index caddd53..35bbad9 100644 --- a/handlers/ssh_config/analyzer/block_test.go +++ b/handlers/ssh_config/analyzer/block_test.go @@ -3,6 +3,8 @@ package analyzer import ( testutils_test "config-lsp/handlers/ssh_config/test_utils" "testing" + + protocol "github.com/tliron/glsp/protocol_3_16" ) func TestBlockEmptyBlock( @@ -11,10 +13,14 @@ func TestBlockEmptyBlock( d := testutils_test.DocumentFromInput(t, ` Host * `) + ctx := &analyzerContext{ + document: *d, + diagnostics: make([]protocol.Diagnostic, 0), + } - errors := analyzeBlocks(d) + analyzeBlocks(ctx) - if !(len(errors) == 1) { - t.Errorf("Expected an error, but got %v", len(errors)) + if !(len(ctx.diagnostics) == 1) { + t.Errorf("Expected an error, but got %v", len(ctx.diagnostics)) } } diff --git a/handlers/ssh_config/analyzer/dependents.go b/handlers/ssh_config/analyzer/dependents.go index bb8d9ea..ad838da 100644 --- a/handlers/ssh_config/analyzer/dependents.go +++ b/handlers/ssh_config/analyzer/dependents.go @@ -2,40 +2,34 @@ package analyzer import ( "config-lsp/common" - sshconfig "config-lsp/handlers/ssh_config" "config-lsp/handlers/ssh_config/ast" "config-lsp/handlers/ssh_config/fields" - "errors" "fmt" + + protocol "github.com/tliron/glsp/protocol_3_16" ) func analyzeDependents( - d *sshconfig.SSHDocument, -) []common.LSPError { - errs := make([]common.LSPError, 0) - - for _, option := range d.Config.GetAllOptions() { - errs = append(errs, checkIsDependent(d, option.Option.Key, option.Block)...) + ctx *analyzerContext, +) { + for _, option := range ctx.document.Config.GetAllOptions() { + checkIsDependent(ctx, option.Option.Key, option.Block) } - - return errs } func checkIsDependent( - d *sshconfig.SSHDocument, + ctx *analyzerContext, key *ast.SSHKey, block ast.SSHBlock, -) []common.LSPError { - errs := make([]common.LSPError, 0) - +) { dependentOptions, found := fields.DependentFields[key.Key] if !found { - return errs + return } for _, dependentOption := range dependentOptions { - if opts, found := d.Indexes.AllOptionsPerName[dependentOption]; found { + if opts, found := ctx.document.Indexes.AllOptionsPerName[dependentOption]; found { _, existsInBlock := opts[block] _, existsInGlobal := opts[nil] @@ -44,11 +38,10 @@ func checkIsDependent( } } - errs = append(errs, common.LSPError{ - Range: key.LocationRange, - Err: errors.New(fmt.Sprintf("Option '%s' requires option '%s' to be present", key.Key, dependentOption)), + ctx.diagnostics = append(ctx.diagnostics, protocol.Diagnostic{ + Range: key.LocationRange.ToLSPRange(), + Message: fmt.Sprintf("Option '%s' requires option '%s' to be present", key.Key, dependentOption), + Severity: &common.SeverityError, }) } - - return errs } diff --git a/handlers/ssh_config/analyzer/match.go b/handlers/ssh_config/analyzer/match.go index 4f79493..9865201 100644 --- a/handlers/ssh_config/analyzer/match.go +++ b/handlers/ssh_config/analyzer/match.go @@ -2,77 +2,78 @@ package analyzer import ( "config-lsp/common" - sshconfig "config-lsp/handlers/ssh_config" "config-lsp/handlers/ssh_config/fields" matchparser "config-lsp/handlers/ssh_config/match-parser" "config-lsp/utils" - "errors" "fmt" + + protocol "github.com/tliron/glsp/protocol_3_16" ) func analyzeMatchBlocks( - d *sshconfig.SSHDocument, -) []common.LSPError { - errs := make([]common.LSPError, 0) + ctx *analyzerContext, +) { + for _, matchBlock := range ctx.document.GetAllMatchBlocks() { + isValid := isMatchStructureValid(ctx, matchBlock.MatchValue) - for _, matchBlock := range d.GetAllMatchBlocks() { - structureErrs := isMatchStructureValid(matchBlock.MatchValue) - errs = append(errs, structureErrs...) - - if len(structureErrs) > 0 { + if !isValid { continue } - errs = append(errs, checkMatch(matchBlock.MatchValue)...) + checkMatch(ctx, matchBlock.MatchValue) } - - return errs } func isMatchStructureValid( + ctx *analyzerContext, m *matchparser.Match, -) []common.LSPError { - errs := make([]common.LSPError, 0) +) bool { + isValid := true for _, entry := range m.Entries { if !utils.KeyExists(fields.MatchSingleOptionCriterias, entry.Criteria.Type) && entry.Value.Value == "" { - errs = append(errs, common.LSPError{ - Range: entry.LocationRange, - Err: errors.New(fmt.Sprintf("Argument '%s' requires a value", entry.Criteria.Type)), + ctx.diagnostics = append(ctx.diagnostics, protocol.Diagnostic{ + Range: entry.LocationRange.ToLSPRange(), + Message: fmt.Sprintf("Argument '%s' requires a value", entry.Criteria.Type), + Severity: &common.SeverityError, }) + + isValid = false } } - return errs + return isValid } func checkMatch( + ctx *analyzerContext, m *matchparser.Match, -) []common.LSPError { - errs := make([]common.LSPError, 0) - +) { // Check single options allEntries := m.FindEntries("all") if len(allEntries) > 1 { - errs = append(errs, common.LSPError{ - Range: allEntries[1].LocationRange, - Err: errors.New("'all' may only be used once"), + ctx.diagnostics = append(ctx.diagnostics, protocol.Diagnostic{ + Range: allEntries[1].LocationRange.ToLSPRange(), + Message: "'all' may only be used once", + Severity: &common.SeverityError, }) } canonicalEntries := m.FindEntries("canonical") if len(canonicalEntries) > 1 { - errs = append(errs, common.LSPError{ - Range: canonicalEntries[1].LocationRange, - Err: errors.New("'canonical' may only be used once"), + ctx.diagnostics = append(ctx.diagnostics, protocol.Diagnostic{ + Range: canonicalEntries[1].LocationRange.ToLSPRange(), + Message: "'canonical' may only be used once", + Severity: &common.SeverityError, }) } finalEntries := m.FindEntries("final") if len(finalEntries) > 1 { - errs = append(errs, common.LSPError{ - Range: finalEntries[1].LocationRange, - Err: errors.New("'final' may only be used once"), + ctx.diagnostics = append(ctx.diagnostics, protocol.Diagnostic{ + Range: finalEntries[1].LocationRange.ToLSPRange(), + Message: "'final' may only be used once", + Severity: &common.SeverityError, }) } @@ -82,12 +83,11 @@ func checkMatch( previousEntry := m.GetPreviousEntry(allEntry) if previousEntry != nil && !utils.KeyExists(fields.MatchAllArgumentAllowedPreviousOptions, previousEntry.Criteria.Type) { - errs = append(errs, common.LSPError{ - Range: allEntry.LocationRange, - Err: errors.New("'all' should either be the first entry or immediately follow 'final' or 'canonical'"), + ctx.diagnostics = append(ctx.diagnostics, protocol.Diagnostic{ + Range: allEntry.LocationRange.ToLSPRange(), + Message: "'all' should either be the first entry or immediately follow 'final' or 'canonical'", + Severity: &common.SeverityError, }) } } - - return errs } diff --git a/handlers/ssh_config/analyzer/match_test.go b/handlers/ssh_config/analyzer/match_test.go index cf58060..c642c5c 100644 --- a/handlers/ssh_config/analyzer/match_test.go +++ b/handlers/ssh_config/analyzer/match_test.go @@ -3,6 +3,8 @@ package analyzer import ( testutils_test "config-lsp/handlers/ssh_config/test_utils" "testing" + + protocol "github.com/tliron/glsp/protocol_3_16" ) func TestMatchInvalidAllArgument( @@ -11,10 +13,14 @@ func TestMatchInvalidAllArgument( d := testutils_test.DocumentFromInput(t, ` Match user lena all `) + ctx := &analyzerContext{ + document: *d, + diagnostics: make([]protocol.Diagnostic, 0), + } - errors := analyzeMatchBlocks(d) + analyzeMatchBlocks(ctx) - if !(len(errors) == 1 && errors[0].Range.Start.Line == 0) { - t.Fatalf("Expected one error, got %v", errors) + if !(len(ctx.diagnostics) == 1 && ctx.diagnostics[0].Range.Start.Line == 0) { + t.Fatalf("Expected one error, got %v", ctx.diagnostics) } } diff --git a/handlers/ssh_config/analyzer/quotes_test.go b/handlers/ssh_config/analyzer/quotes_test.go index 34af3bb..2914e32 100644 --- a/handlers/ssh_config/analyzer/quotes_test.go +++ b/handlers/ssh_config/analyzer/quotes_test.go @@ -3,6 +3,8 @@ package analyzer import ( testutils_test "config-lsp/handlers/ssh_config/test_utils" "testing" + + protocol "github.com/tliron/glsp/protocol_3_16" ) func TestSimpleInvalidQuotesExample( @@ -68,12 +70,16 @@ func TestDependentOptionsExample( Port 1234 CanonicalDomains example.com `) + ctx := &analyzerContext{ + document: *d, + diagnostics: make([]protocol.Diagnostic, 0), + } option := d.FindOptionsByName("canonicaldomains")[0] - errors := checkIsDependent(d, option.Option.Key, option.Block) + checkIsDependent(ctx, option.Option.Key, option.Block) - if !(len(errors) == 1) { - t.Errorf("Expected 1 error, got %v", len(errors)) + if !(len(ctx.diagnostics) == 1) { + t.Errorf("Expected 1 error, got %v", len(ctx.diagnostics)) } } @@ -85,11 +91,15 @@ Port 1234 CanonicalizeHostname yes CanonicalDomains example.com `) + ctx := &analyzerContext{ + document: *d, + diagnostics: make([]protocol.Diagnostic, 0), + } option := d.FindOptionsByName("canonicaldomains")[0] - errors := checkIsDependent(d, option.Option.Key, option.Block) + checkIsDependent(ctx, option.Option.Key, option.Block) - if len(errors) > 0 { - t.Errorf("Expected no errors, got %v", len(errors)) + if len(ctx.diagnostics) > 0 { + t.Errorf("Expected no errors, got %v", len(ctx.diagnostics)) } }