feat: code review fixes and LSP enhancements

cmd/gosqlx/cmd/watch.go

@@ -43,6 +43,7 @@ type FileWatcher struct {
 	debounceMu   sync.Mutex
 	watchedFiles map[string]bool
 	watchedDirs  map[string]bool
+	watchedMu    sync.RWMutex // Protects watchedFiles and watchedDirs
 }
 
 // NewFileWatcher creates a new file watcher instance
@@ -75,7 +76,10 @@ func (fw *FileWatcher) Watch(args []string) error {
 		return err
 	}
 
-	if len(fw.watchedFiles) == 0 && len(fw.watchedDirs) == 0 {
+	fw.watchedMu.RLock()
+	noFiles := len(fw.watchedFiles) == 0 && len(fw.watchedDirs) == 0
+	fw.watchedMu.RUnlock()
+	if noFiles {
 		return fmt.Errorf("no files or directories to watch")
 	}
 
@@ -119,7 +123,9 @@ func (fw *FileWatcher) watchLoop(ctx context.Context) {
 			if event.Has(fsnotify.Create) {
 				info, err := os.Stat(event.Name)
 				if err == nil && !info.IsDir() && fw.shouldProcessFile(event.Name) {
+					fw.watchedMu.Lock()
 					fw.watchedFiles[event.Name] = true
+					fw.watchedMu.Unlock()
 				}
 			}
 
@@ -228,7 +234,15 @@ func (fw *FileWatcher) formatFile(filename string, timestamp string) {
 func (fw *FileWatcher) processAllFiles() error {
 	timestamp := time.Now().Format("15:04:05")
 
+	// Copy files under lock to avoid race conditions
+	fw.watchedMu.RLock()
+	files := make([]string, 0, len(fw.watchedFiles))
 	for file := range fw.watchedFiles {
+		files = append(files, file)
+	}
+	fw.watchedMu.RUnlock()
+
+	for _, file := range files {
 		switch fw.opts.Mode {
 		case WatchModeValidate:
 			fw.validateFile(file, timestamp)
@@ -291,6 +305,9 @@ func (fw *FileWatcher) addSinglePath(path string) error {
 		return fmt.Errorf("cannot get absolute path for '%s': %w", path, err)
 	}
 
+	fw.watchedMu.Lock()
+	defer fw.watchedMu.Unlock()
+
 	if info.IsDir() {
 		if !fw.watchedDirs[absPath] {
 			if err := fw.watcher.Add(absPath); err != nil {
@@ -328,6 +345,9 @@ func (fw *FileWatcher) addDirectoryRecursive(root string) error {
 			return err
 		}
 
+		fw.watchedMu.Lock()
+		defer fw.watchedMu.Unlock()
+
 		if info.IsDir() {
 			if !fw.watchedDirs[absPath] {
 				if err := fw.watcher.Add(absPath); err != nil {
@@ -358,15 +378,27 @@ func (fw *FileWatcher) printWatchStatus() {
 		modeStr = "formatting"
 	}
 
+	fw.watchedMu.RLock()
+	numFiles := len(fw.watchedFiles)
+	numDirs := len(fw.watchedDirs)
+	var files []string
+	if fw.opts.Verbose {
+		files = make([]string, 0, numFiles)
+		for file := range fw.watchedFiles {
+			files = append(files, file)
+		}
+	}
+	fw.watchedMu.RUnlock()
+
 	fmt.Fprintf(fw.opts.Out, "%s Watching %d file(s) in %d director(y/ies) for %s\n",
 		colorCyan("👁"),
-		len(fw.watchedFiles),
-		len(fw.watchedDirs),
+		numFiles,
+		numDirs,
 		modeStr)
 
 	if fw.opts.Verbose {
 		fmt.Fprintf(fw.opts.Out, "Watched files:\n")
-		for file := range fw.watchedFiles {
+		for _, file := range files {
 			fmt.Fprintf(fw.opts.Out, "  - %s\n", file)
 		}
 	}

pkg/errors/builders.go

@@ -202,3 +202,67 @@ func InvalidSetOperationError(operation, description string, location models.Loc
 		location,
 	).WithContext(sql, len(operation)).WithHint("Ensure both queries have the same number and compatible types of columns")
 }
+
+// Semantic Errors (E3001-E3004)
+
+// UndefinedTableError creates an error for referencing an undefined table
+func UndefinedTableError(tableName string, location models.Location, sql string) *Error {
+	return NewError(
+		ErrCodeUndefinedTable,
+		fmt.Sprintf("table '%s' does not exist", tableName),
+		location,
+	).WithContext(sql, len(tableName)).WithHint(fmt.Sprintf("Check the table name '%s' for typos or ensure it exists in the schema", tableName))
+}
+
+// UndefinedColumnError creates an error for referencing an undefined column
+func UndefinedColumnError(columnName, tableName string, location models.Location, sql string) *Error {
+	message := fmt.Sprintf("column '%s' does not exist", columnName)
+	hint := fmt.Sprintf("Check the column name '%s' for typos or ensure it exists in the table", columnName)
+	if tableName != "" {
+		message = fmt.Sprintf("column '%s' does not exist in table '%s'", columnName, tableName)
+		hint = fmt.Sprintf("Check that column '%s' exists in table '%s'", columnName, tableName)
+	}
+	return NewError(
+		ErrCodeUndefinedColumn,
+		message,
+		location,
+	).WithContext(sql, len(columnName)).WithHint(hint)
+}
+
+// TypeMismatchError creates an error for type mismatch in expressions
+func TypeMismatchError(leftType, rightType, context string, location models.Location, sql string) *Error {
+	message := fmt.Sprintf("type mismatch: cannot compare %s with %s", leftType, rightType)
+	if context != "" {
+		message = fmt.Sprintf("type mismatch in %s: cannot compare %s with %s", context, leftType, rightType)
+	}
+	return NewError(
+		ErrCodeTypeMismatch,
+		message,
+		location,
+	).WithContext(sql, 1).WithHint(fmt.Sprintf("Ensure compatible types or use explicit CAST to convert %s to %s", leftType, rightType))
+}
+
+// AmbiguousColumnError creates an error for ambiguous column reference
+func AmbiguousColumnError(columnName string, tables []string, location models.Location, sql string) *Error {
+	tableList := "multiple tables"
+	if len(tables) > 0 {
+		tableList = fmt.Sprintf("tables: %s", joinStrings(tables, ", "))
+	}
+	return NewError(
+		ErrCodeAmbiguousColumn,
+		fmt.Sprintf("column '%s' is ambiguous (appears in %s)", columnName, tableList),
+		location,
+	).WithContext(sql, len(columnName)).WithHint(fmt.Sprintf("Qualify the column with a table name or alias, e.g., 'table_name.%s'", columnName))
+}
+
+// joinStrings is a helper to join strings with a separator
+func joinStrings(strs []string, sep string) string {
+	if len(strs) == 0 {
+		return ""
+	}
+	result := strs[0]
+	for i := 1; i < len(strs); i++ {
+		result += sep + strs[i]
+	}
+	return result
+}

pkg/lsp/documents.go

@@ -72,12 +72,25 @@ func (dm *DocumentManager) Close(uri string) {
 	delete(dm.documents, uri)
 }
 
-// Get retrieves a document
+// Get retrieves a copy of a document to avoid race conditions
+// The returned document is a snapshot and modifications won't affect the original
 func (dm *DocumentManager) Get(uri string) (*Document, bool) {
 	dm.mu.RLock()
 	defer dm.mu.RUnlock()
 	doc, ok := dm.documents[uri]
-	return doc, ok
+	if !ok {
+		return nil, false
+	}
+	// Return a copy to prevent race conditions when accessing fields after lock release
+	docCopy := &Document{
+		URI:        doc.URI,
+		LanguageID: doc.LanguageID,
+		Version:    doc.Version,
+		Content:    doc.Content,
+		Lines:      make([]string, len(doc.Lines)),
+	}
+	copy(docCopy.Lines, doc.Lines)
+	return docCopy, true
 }
 
 // GetContent retrieves a document's content
@@ -138,35 +151,38 @@ func positionToOffset(lines []string, pos Position) int {
 }
 
 // GetWordAtPosition returns the word at the given position
+// Uses rune-based indexing for proper UTF-8 handling
 func (doc *Document) GetWordAtPosition(pos Position) string {
 	if pos.Line >= len(doc.Lines) {
 		return ""
 	}
 
 	line := doc.Lines[pos.Line]
-	if pos.Character >= len(line) {
+	runes := []rune(line)
+
+	if pos.Character >= len(runes) {
 		return ""
 	}
 
-	// Find word boundaries
+	// Find word boundaries using rune indexing for UTF-8 safety
 	start := pos.Character
 	end := pos.Character
 
 	// Move start backwards to find word start
-	for start > 0 && isWordChar(rune(line[start-1])) {
+	for start > 0 && isWordChar(runes[start-1]) {
 		start--
 	}
 
 	// Move end forwards to find word end
-	for end < len(line) && isWordChar(rune(line[end])) {
+	for end < len(runes) && isWordChar(runes[end]) {
 		end++
 	}
 
 	if start == end {
 		return ""
 	}
 
-	return line[start:end]
+	return string(runes[start:end])
 }
 
 // isWordChar returns true if c is a valid word character