fix(ssh_config): Improvements

handlers/ssh_config/analyzer/analyzer.go

@@ -28,6 +28,7 @@ func Analyze(
 	}
 
 	errors = append(errors, analyzeDependents(d)...)
+	errors = append(errors, analyzeMatchBlocks(d)...)
 
 	return common.ErrsToDiagnostics(errors)
 }

handlers/ssh_config/analyzer/match.go

@@ -0,0 +1,93 @@
+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"
+)
+
+func analyzeMatchBlocks(
+	d *sshconfig.SSHDocument,
+) []common.LSPError {
+	errs := make([]common.LSPError, 0)
+
+	for _, matchBlock := range d.GetAllMatchBlocks() {
+		structureErrs := isMatchStructureValid(matchBlock.MatchValue)
+		errs = append(errs, structureErrs...)
+
+		if len(structureErrs) > 0 {
+			continue
+		}
+
+		errs = append(errs, checkMatch(matchBlock.MatchValue)...)
+	}
+
+	return errs
+}
+
+func isMatchStructureValid(
+	m *matchparser.Match,
+) []common.LSPError {
+	errs := make([]common.LSPError, 0)
+
+	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)),
+			})
+		}
+	}
+
+	return errs
+}
+
+func checkMatch(
+	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"),
+		})
+	}
+
+	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"),
+		})
+	}
+
+	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"),
+		})
+	}
+
+	// Check the `all` argument
+	if len(allEntries) == 1 {
+		allEntry := allEntries[0]
+		previousEntry := m.GetPreviousEntry(allEntry)
+
+		if previousEntry != nil && !utils.KeyExists(fields.MatchAllOptionAllowedPreviousOptions, 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'"),
+			})
+		}
+	}
+
+	return errs
+}

handlers/ssh_config/analyzer/match_test.go

@@ -0,0 +1,20 @@
+package analyzer
+
+import (
+	testutils_test "config-lsp/handlers/ssh_config/test_utils"
+	"testing"
+)
+
+func TestMatchInvalidAllArgument(
+	t *testing.T,
+) {
+	d := testutils_test.DocumentFromInput(t, `
+Match user lena all
+`)
+
+	errors := analyzeMatchBlocks(d)
+
+	if !(len(errors) == 1 && errors[0].Range.Start.Line == 0) {
+		t.Fatalf("Expected one error, got %v", errors)
+	}
+}

handlers/ssh_config/analyzer/options.go

@@ -25,10 +25,10 @@ func analyzeStructureIsValid(
 			errs = append(errs, checkOption(d, entry.(*ast.SSHOption), nil)...)
 		case *ast.SSHMatchBlock:
 			matchBlock := entry.(*ast.SSHMatchBlock)
-			errs = append(errs, checkMatchBlock(d, matchBlock)...)
+			errs = append(errs, checkBlock(d, matchBlock)...)
 		case *ast.SSHHostBlock:
 			hostBlock := entry.(*ast.SSHHostBlock)
-			errs = append(errs, checkHostBlock(d, hostBlock)...)
+			errs = append(errs, checkBlock(d, hostBlock)...)
 		}
 
 	}
@@ -106,33 +106,19 @@ func checkOption(
 	return errs
 }
 
-func checkMatchBlock(
+func checkBlock(
 	d *sshconfig.SSHDocument,
-	matchBlock *ast.SSHMatchBlock,
+	block ast.SSHBlock,
 ) []common.LSPError {
 	errs := make([]common.LSPError, 0)
 
-	it := matchBlock.Options.Iterator()
+	errs = append(errs, checkOption(d, block.GetEntryOption(), block)...)
+
+	it := block.GetOptions().Iterator()
 	for it.Next() {
 		option := it.Value().(*ast.SSHOption)
 
-		errs = append(errs, checkOption(d, option, matchBlock)...)
+		errs = append(errs, checkOption(d, option, block)...)
-	}
-
-	return errs
-}
-
-func checkHostBlock(
-	d *sshconfig.SSHDocument,
-	hostBlock *ast.SSHHostBlock,
-) []common.LSPError {
-	errs := make([]common.LSPError, 0)
-
-	it := hostBlock.Options.Iterator()
-	for it.Next() {
-		option := it.Value().(*ast.SSHOption)
-
-		errs = append(errs, checkOption(d, option, hostBlock)...)
 	}
 
 	return errs

handlers/ssh_config/analyzer/options_test.go

@@ -50,3 +50,39 @@ User root
 		t.Fatalf("Expected 1 error, got %v", errors)
 	}
 }
+
+func TestEmptyMatch(
+	t *testing.T,
+) {
+	d := testutils_test.DocumentFromInput(t, `
+User root
+
+Host example.com
+	User test
+
+Match
+`)
+	errors := analyzeStructureIsValid(d)
+
+	if len(errors) != 2 {
+		t.Fatalf("Expected 1 error, got %v", errors)
+	}
+}
+
+func TestEmptyWithSeparatorMatch(
+	t *testing.T,
+) {
+	d := testutils_test.DocumentFromInput(t, `
+User root
+
+Host example.com
+	User test
+
+Match 
+`)
+	errors := analyzeStructureIsValid(d)
+
+	if len(errors) != 1 {
+		t.Fatalf("Expected 1 error, got %v", errors)
+	}
+}

handlers/ssh_config/ast/parser_test.go

@@ -1,6 +1,7 @@
 package ast
 
 import (
+	matchparser "config-lsp/handlers/ssh_config/match-parser"
 	"config-lsp/utils"
 	"testing"
 )
@@ -323,6 +324,57 @@ Match
 	}
 }
 
+func TestMatchWithIncompleteEntry(
+	t *testing.T,
+) {
+	input := utils.Dedent(`
+Match user 
+`)
+	p := NewSSHConfig()
+
+	errors := p.Parse(input)
+
+	if len(errors) > 0 {
+		t.Fatalf("Expected no errors, got %v", errors)
+	}
+
+	if !(p.Options.Size() == 1) {
+		t.Errorf("Expected 1 option, but got: %v", p.Options.Size())
+	}
+
+	if !(len(utils.KeysOfMap(p.CommentLines)) == 0) {
+		t.Errorf("Expected no comment lines, but got: %v", len(p.CommentLines))
+	}
+
+	rawFirstEntry, _ := p.Options.Get(uint32(0))
+	firstEntry := rawFirstEntry.(*SSHMatchBlock)
+	if !(firstEntry.MatchOption.Key.Value.Raw == "Match") {
+		t.Errorf("Expected first entry to be User, but got: %v", firstEntry)
+	}
+
+	if !(firstEntry.MatchOption.OptionValue != nil && firstEntry.MatchOption.OptionValue.Value.Raw == "user ") {
+		t.Errorf("Expected first entry to have an empty value, but got: %v", firstEntry)
+	}
+
+	if !(firstEntry.MatchValue.Entries[0].Criteria.Type == matchparser.MatchCriteriaTypeUser) {
+		t.Errorf("Expected first entry to have a user criteria, but got: %v", firstEntry)
+	}
+}
+
+func TestInvalidMatchExample(
+	t *testing.T,
+) {
+	input := utils.Dedent(`
+Match us
+`)
+	p := NewSSHConfig()
+	errors := p.Parse(input)
+
+	if len(errors) == 0 {
+		t.Fatalf("Expected errors, got none")
+	}
+}
+
 func TestComplexBigExample(
 	t *testing.T,
 ) {

handlers/ssh_config/document_fields.go

@@ -1,6 +1,9 @@
 package sshconfig
 
-import "config-lsp/handlers/ssh_config/ast"
+import (
+	"config-lsp/handlers/ssh_config/ast"
+	"config-lsp/utils"
+)
 
 func (d SSHDocument) FindOptionByNameAndBlock(
 	name string,
@@ -35,3 +38,19 @@ func (d SSHDocument) DoesOptionExist(
 ) bool {
 	return d.FindOptionByNameAndBlock(name, block) != nil
 }
+
+func (d SSHDocument) GetAllMatchBlocks() []*ast.SSHMatchBlock {
+	matchBlocks := make([]*ast.SSHMatchBlock, 0, 5)
+
+	options := d.Indexes.AllOptionsPerName["Match"]
+	blocks := utils.KeysOfMap(options)
+
+	for _, block := range blocks {
+		switch block.GetBlockType() {
+		case ast.SSHBlockTypeMatch:
+			matchBlocks = append(matchBlocks, block.(*ast.SSHMatchBlock))
+		}
+	}
+
+	return matchBlocks
+}

handlers/ssh_config/document_fields_test.go

@@ -56,4 +56,9 @@ Match originalhost laptop exec "[[ $(/usr/bin/dig +short laptop.lan) == '' ]]"
 	if !(d.FindOptionByNameAndBlock("ProxyCommand", nil).Option.Start.Line == 0) {
 		t.Errorf("Expected 0, got %v", d.FindOptionByNameAndBlock("ProxyCommand", nil).Option.Start.Line)
 	}
+
+	matchBlocks := d.GetAllMatchBlocks()
+	if !(len(matchBlocks) == 1 && matchBlocks[0].Start.Line == 6) {
+		t.Errorf("Expected 1 match block, got %v", matchBlocks)
+	}
 }

handlers/ssh_config/fields/match.go

@@ -26,3 +26,14 @@ var MatchValueFieldMap = map[matchparser.MatchCriteriaType]docvalues.DeprecatedV
 	matchparser.MatchCriteriaTypeUser:         MatchUserField,
 	matchparser.MatchCriteriaTypeLocalUser:    MatchTypeLocalUserField,
 }
+
+var MatchAllOptionAllowedPreviousOptions = map[matchparser.MatchCriteriaType]struct{}{
+	matchparser.MatchCriteriaTypeCanonical: {},
+	matchparser.MatchCriteriaTypeFinal:     {},
+}
+
+var MatchSingleOptionCriterias = map[matchparser.MatchCriteriaType]struct{}{
+	matchparser.MatchCriteriaTypeAll:       {},
+	matchparser.MatchCriteriaTypeCanonical: {},
+	matchparser.MatchCriteriaTypeFinal:     {},
+}

handlers/ssh_config/match-parser/match_fields.go

@@ -31,6 +31,28 @@ func (m Match) GetEntryAtPosition(position common.Position) *MatchEntry {
 	return entry
 }
 
+func (m Match) FindEntries(name string) []*MatchEntry {
+	entries := make([]*MatchEntry, 0, 5)
+
+	for _, entry := range m.Entries {
+		if entry.Value.Value == name {
+			entries = append(entries, entry)
+		}
+	}
+
+	return entries
+}
+
+func (m Match) GetPreviousEntry(e *MatchEntry) *MatchEntry {
+	index := slices.Index(m.Entries, e)
+
+	if index == 0 || index == -1 {
+		return nil
+	}
+
+	return m.Entries[index-1]
+}
+
 func (e MatchEntry) GetValueAtPosition(position common.Position) *MatchValue {
 	if e.Values == nil {
 		return nil