🐛 When we are using a RPC client that is already defined handle empty list

[shawn-hurley]

Summary by CodeRabbit

• Bug Fixes
  • Fixed a potential crash that could occur when evaluating references with an empty workspace folder list. The application now safely handles scenarios where no workspace folders are configured.

[coderabbitai]

Walkthrough

A guard clause was added to check that the WorkspaceFolders list is non-empty before accessing its first element in the reference evaluation loop. This prevents out-of-bounds access when filtering references against workspace folder URIs.

Changes

Cohort / File(s)	Summary
Bounds check fix lsp/base_service_client/base_capabilities.go	Added length guard before accessing WorkspaceFolders[0] to prevent out-of-bounds access when the workspace folders list is empty

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

This is a straightforward defensive programming fix that adds a simple length check before array access. No complex logic or state management involved.

Poem

🐰 A guard stands watch at the list's front door,
Ensuring we never index where none explore,
Empty or full, we check before we go—
Safe bounds and clean logic steal the show! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title references handling an empty list with RPC clients, which directly aligns with the changeset that adds a guard to prevent indexing when WorkspaceFolders list is empty.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

lsp/base_service_client/base_capabilities.go (1)

68-70: Consider checking all workspace folders, not just the first.

The current implementation only checks whether ref.URI contains WorkspaceFolders[0]. If the LSP server supports multi-root workspaces, this could miss valid references in other workspace folders.

Consider iterating over all workspace folders:

-			if len(sc.BaseConfig.WorkspaceFolders) > 0 && !strings.Contains(ref.URI, sc.BaseConfig.WorkspaceFolders[0]) {
-				continue
-			}
+			inWorkspace := false
+			for _, folder := range sc.BaseConfig.WorkspaceFolders {
+				if strings.Contains(ref.URI, folder) {
+					inWorkspace = true
+					break
+				}
+			}
+			if !inWorkspace {
+				continue
+			}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2770737 and 62de805.

📒 Files selected for processing (1)

• lsp/base_service_client/base_capabilities.go (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

Learnt from: shawn-hurley
Repo: konveyor/analyzer-lsp PR: 860
File: lsp/base_service_client/base_service_client.go:212-214
Timestamp: 2025-10-21T16:25:40.903Z
Learning: In `lsp/base_service_client/base_service_client.go`, when an RPC connection is provided via `c.RPC` (not nil), it is treated as already initialized and no handler setup is performed. This is known technical debt that needs to be addressed in the future to ensure LSP notifications like `textDocument/publishDiagnostics` are properly processed.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

• GitHub Check: test
• GitHub Check: test (macos-latest, darwin, arm64)
• GitHub Check: test (ubuntu-latest, linux, arm64)
• GitHub Check: test (windows-latest, windows, amd64)
• GitHub Check: test (ubuntu-latest, linux, amd64)
• GitHub Check: benchmark (ubuntu-latest, linux)
• GitHub Check: test (macos-latest, darwin, amd64)
• GitHub Check: benchmark (macos-latest, mac)
• GitHub Check: benchmark (windows-latest, windows)

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Semantic change: empty WorkspaceFolders now processes all references.

The guard prevents the out-of-bounds panic, but when WorkspaceFolders is empty, the condition short-circuits to false, so the continue statement doesn't execute. This means all references will be processed rather than being filtered to the workspace.

Given the intent (per the comment above) is to filter references to the loaded location, consider returning early or explicitly handling the empty case:

+	if len(sc.BaseConfig.WorkspaceFolders) == 0 {
+		return resp{Matched: false}, nil
+	}
+
 	for _, s := range symbols {
 		references := sc.GetAllReferences(ctx, s.Location.Value.(protocol.Location))
 
 		breakEarly := false
 		for _, ref := range references {
 			// Look for things that are in the location loaded,
 			// Note may need to filter out vendor at some point
-			if len(sc.BaseConfig.WorkspaceFolders) > 0 && !strings.Contains(ref.URI, sc.BaseConfig.WorkspaceFolders[0]) {
+			if !strings.Contains(ref.URI, sc.BaseConfig.WorkspaceFolders[0]) {
 				continue
 			}

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In lsp/base_service_client/base_capabilities.go around lines 68 to 70, the
current guard uses sc.BaseConfig.WorkspaceFolders[0] without explicitly handling
the empty-slice case which causes the condition to evaluate to false and thus
processes all references; update the code to explicitly handle
len(sc.BaseConfig.WorkspaceFolders) == 0 (either return early or skip processing
references by continuing) and only access WorkspaceFolders[0] after confirming
length > 0 so references are correctly filtered to the loaded workspace.