feat: diagnostics tool implementation (#34)

* feat: complete implementation of getDiagnostics tool

* refactor(getDiagnostics): remove pcall from logger import

* refactor(getDiagnostics): use alternate json encode

We're also passing nil explicitly to get diagnostics from all buffers

* refactor(getDiagnostics): tweak file scheme handling, assume URI input

* fix(getDiagnostics): remove Neovim context from description

Author: krmcbride

lua/claudecode/tools/get_diagnostics.lua

@@ -1,45 +1,103 @@
 --- Tool implementation for getting diagnostics.
 
+-- NOTE: Its important we don't tip off Claude that we're dealing with Neovim LSP diagnostics because it may adjust
+-- line and col numbers by 1 on its own (since it knows nvim LSP diagnostics are 0-indexed). By calling these
+-- "editor diagnostics" and converting to 1-indexed ourselves we (hopefully) avoid incorrect line and column numbers
+-- in Claude's responses.
+local schema = {
+  description = "Get language diagnostics (errors, warnings) from the editor",
+  inputSchema = {
+    type = "object",
+    properties = {
+      uri = {
+        type = "string",
+        description = "Optional file URI to get diagnostics for. If not provided, gets diagnostics for all open files.",
+      },
+    },
+    additionalProperties = false,
+    ["$schema"] = "http://json-schema.org/draft-07/schema#",
+  },
+}
+
 --- Handles the getDiagnostics tool invocation.
 -- Retrieves diagnostics from Neovim's diagnostic system.
--- @param _params table The input parameters for the tool (currently unused).
+-- @param params table The input parameters for the tool.
+-- @field params.uri string|nil Optional file URI to get diagnostics for.
 -- @return table A table containing the list of diagnostics.
 -- @error table A table with code, message, and data for JSON-RPC error if failed.
-local function handler(_params) -- Prefix unused params with underscore
+local function handler(params)
   if not vim.lsp or not vim.diagnostic or not vim.diagnostic.get then
-    -- This tool is internal, so returning an error might be too strong.
     -- Returning an empty list or a specific status could be an alternative.
     -- For now, let's align with the error pattern for consistency if the feature is unavailable.
     error({
       code = -32000,
       message = "Feature unavailable",
-      data = "LSP or vim.diagnostic.get not available in this Neovim version/configuration.",
+      data = "Diagnostics not available in this editor version/configuration.",
     })
   end
 
-  local all_diagnostics = vim.diagnostic.get(0) -- Get for all buffers
+  local logger = require("claudecode.logger")
+
+  logger.debug("getDiagnostics handler called with params: " .. vim.inspect(params))
+
+  -- Extract the uri parameter
+  local diagnostics
+
+  if not params.uri then
+    -- Get diagnostics for all buffers
+    logger.debug("Getting diagnostics for all open buffers")
+    diagnostics = vim.diagnostic.get(nil)
+  else
+    local uri = params.uri
+    -- Strips the file:// scheme
+    local filepath = vim.uri_to_fname(uri)
+
+    -- Get buffer number for the specific file
+    local bufnr = vim.fn.bufnr(filepath)
+    if bufnr == -1 then
+      -- File is not open in any buffer, throw an error
+      logger.debug("File buffer must be open to get diagnostics: " .. filepath)
+      error({
+        code = -32001,
+        message = "File not open",
+        data = "File must be open to retrieve diagnostics: " .. filepath,
+      })
+    else
+      -- Get diagnostics for the specific buffer
+      logger.debug("Getting diagnostics for bufnr: " .. bufnr)
+      diagnostics = vim.diagnostic.get(bufnr)
+    end
+  end
 
   local formatted_diagnostics = {}
-  for _, diagnostic in ipairs(all_diagnostics) do
+  for _, diagnostic in ipairs(diagnostics) do
     local file_path = vim.api.nvim_buf_get_name(diagnostic.bufnr)
     -- Ensure we only include diagnostics with valid file paths
     if file_path and file_path ~= "" then
       table.insert(formatted_diagnostics, {
-        file = file_path,
-        line = diagnostic.lnum, -- 0-indexed from vim.diagnostic.get
-        character = diagnostic.col, -- 0-indexed from vim.diagnostic.get
-        severity = diagnostic.severity, -- e.g., vim.diagnostic.severity.ERROR
-        message = diagnostic.message,
-        source = diagnostic.source,
+        type = "text",
+        -- json encode this
+        text = vim.json.encode({
+          -- Use the file path and diagnostic information
+          filePath = file_path,
+          -- Convert line and column to 1-indexed
+          line = diagnostic.lnum + 1,
+          character = diagnostic.col + 1,
+          severity = diagnostic.severity, -- e.g., vim.diagnostic.severity.ERROR
+          message = diagnostic.message,
+          source = diagnostic.source,
+        }),
       })
     end
   end
 
-  return { diagnostics = formatted_diagnostics }
+  return {
+    content = formatted_diagnostics,
+  }
 end
 
 return {
   name = "getDiagnostics",
-  schema = nil, -- Internal tool
+  schema = schema,
   handler = handler,
 }

tests/unit/tools/get_diagnostics_spec.lua

@@ -5,12 +5,23 @@ describe("Tool: get_diagnostics", function()
 
   before_each(function()
     package.loaded["claudecode.tools.get_diagnostics"] = nil
+    package.loaded["claudecode.logger"] = nil
+
+    -- Mock the logger module
+    package.loaded["claudecode.logger"] = {
+      debug = function() end,
+      error = function() end,
+      info = function() end,
+      warn = function() end,
+    }
+
     get_diagnostics_handler = require("claudecode.tools.get_diagnostics").handler
 
     _G.vim = _G.vim or {}
     _G.vim.lsp = _G.vim.lsp or {} -- Ensure vim.lsp exists for the check
     _G.vim.diagnostic = _G.vim.diagnostic or {}
     _G.vim.api = _G.vim.api or {}
+    _G.vim.fn = _G.vim.fn or {}
 
     -- Default mocks
     _G.vim.diagnostic.get = spy.new(function()
@@ -19,12 +30,34 @@ describe("Tool: get_diagnostics", function()
     _G.vim.api.nvim_buf_get_name = spy.new(function(bufnr)
       return "/path/to/file_for_buf_" .. tostring(bufnr) .. ".lua"
     end)
+    _G.vim.json.encode = spy.new(function(obj)
+      return vim.inspect(obj) -- Use vim.inspect as a simple serialization
+    end)
+    _G.vim.fn.bufnr = spy.new(function(filepath)
+      -- Mock buffer lookup
+      if filepath == "/test/file.lua" then
+        return 1
+      end
+      return -1 -- File not open
+    end)
+    _G.vim.uri_to_fname = spy.new(function(uri)
+      -- Realistic mock that matches vim.uri_to_fname behavior
+      if uri:sub(1, 7) == "file://" then
+        return uri:sub(8)
+      end
+      -- Real vim.uri_to_fname throws an error for URIs without proper scheme
+      error("URI must contain a scheme: " .. uri)
+    end)
   end)
 
   after_each(function()
     package.loaded["claudecode.tools.get_diagnostics"] = nil
+    package.loaded["claudecode.logger"] = nil
     _G.vim.diagnostic.get = nil
     _G.vim.api.nvim_buf_get_name = nil
+    _G.vim.json.encode = nil
+    _G.vim.fn.bufnr = nil
+    _G.vim.uri_to_fname = nil
     -- Note: We don't nullify _G.vim.lsp or _G.vim.diagnostic entirely
     -- as they are checked for existence.
   end)
@@ -33,9 +66,9 @@ describe("Tool: get_diagnostics", function()
     local success, result = pcall(get_diagnostics_handler, {})
     expect(success).to_be_true()
     expect(result).to_be_table()
-    expect(result.diagnostics).to_be_table()
-    expect(#result.diagnostics).to_be(0)
-    assert.spy(_G.vim.diagnostic.get).was_called_with(0)
+    expect(result.content).to_be_table()
+    expect(#result.content).to_be(0)
+    assert.spy(_G.vim.diagnostic.get).was_called_with(nil)
   end)
 
   it("should return formatted diagnostics if available", function()
@@ -49,19 +82,24 @@ describe("Tool: get_diagnostics", function()
 
     local success, result = pcall(get_diagnostics_handler, {})
     expect(success).to_be_true()
-    expect(result.diagnostics).to_be_table()
-    expect(#result.diagnostics).to_be(2)
+    expect(result.content).to_be_table()
+    expect(#result.content).to_be(2)
+
+    -- Check that results are MCP content items
+    expect(result.content[1].type).to_be("text")
+    expect(result.content[2].type).to_be("text")
 
-    expect(result.diagnostics[1].file).to_be("/path/to/file_for_buf_1.lua")
-    expect(result.diagnostics[1].line).to_be(10)
-    expect(result.diagnostics[1].character).to_be(5)
-    expect(result.diagnostics[1].severity).to_be(1)
-    expect(result.diagnostics[1].message).to_be("Error message 1")
-    expect(result.diagnostics[1].source).to_be("linter1")
+    -- Verify JSON encoding was called with correct structure
+    assert.spy(_G.vim.json.encode).was_called(2)
 
-    expect(result.diagnostics[2].file).to_be("/path/to/file_for_buf_2.lua")
-    expect(result.diagnostics[2].severity).to_be(2)
-    expect(result.diagnostics[2].message).to_be("Warning message 2")
+    -- Check the first diagnostic was encoded with 1-indexed values
+    local first_call_args = _G.vim.json.encode.calls[1].vals[1]
+    expect(first_call_args.filePath).to_be("/path/to/file_for_buf_1.lua")
+    expect(first_call_args.line).to_be(11) -- 10 + 1 for 1-indexing
+    expect(first_call_args.character).to_be(6) -- 5 + 1 for 1-indexing
+    expect(first_call_args.severity).to_be(1)
+    expect(first_call_args.message).to_be("Error message 1")
+    expect(first_call_args.source).to_be("linter1")
 
     assert.spy(_G.vim.api.nvim_buf_get_name).was_called_with(1)
     assert.spy(_G.vim.api.nvim_buf_get_name).was_called_with(2)
@@ -87,8 +125,12 @@ describe("Tool: get_diagnostics", function()
 
     local success, result = pcall(get_diagnostics_handler, {})
     expect(success).to_be_true()
-    expect(#result.diagnostics).to_be(1)
-    expect(result.diagnostics[1].file).to_be("/path/to/file1.lua")
+    expect(#result.content).to_be(1)
+
+    -- Verify only the diagnostic with a file path was included
+    assert.spy(_G.vim.json.encode).was_called(1)
+    local encoded_args = _G.vim.json.encode.calls[1].vals[1]
+    expect(encoded_args.filePath).to_be("/path/to/file1.lua")
   end)
 
   it("should error if vim.diagnostic.get is not available", function()
@@ -98,7 +140,7 @@ describe("Tool: get_diagnostics", function()
     expect(err).to_be_table()
     expect(err.code).to_be(-32000)
     assert_contains(err.message, "Feature unavailable")
-    assert_contains(err.data, "LSP or vim.diagnostic.get not available")
+    assert_contains(err.data, "Diagnostics not available in this editor version/configuration.")
   end)
 
   it("should error if vim.diagnostic is not available", function()
@@ -120,4 +162,49 @@ describe("Tool: get_diagnostics", function()
     expect(success).to_be_false()
     expect(err.code).to_be(-32000)
   end)
+
+  it("should filter diagnostics by URI when provided", function()
+    local mock_diagnostics = {
+      { bufnr = 1, lnum = 10, col = 5, severity = 1, message = "Error in file1", source = "linter1" },
+    }
+    _G.vim.diagnostic.get = spy.new(function(bufnr)
+      if bufnr == 1 then
+        return mock_diagnostics
+      end
+      return {}
+    end)
+    _G.vim.api.nvim_buf_get_name = spy.new(function(bufnr)
+      if bufnr == 1 then
+        return "/test/file.lua"
+      end
+      return ""
+    end)
+
+    local success, result = pcall(get_diagnostics_handler, { uri = "file:///test/file.lua" })
+    expect(success).to_be_true()
+    expect(#result.content).to_be(1)
+
+    -- Should have used vim.uri_to_fname to convert URI to file path
+    assert.spy(_G.vim.uri_to_fname).was_called_with("file:///test/file.lua")
+    assert.spy(_G.vim.diagnostic.get).was_called_with(1)
+    assert.spy(_G.vim.fn.bufnr).was_called_with("/test/file.lua")
+  end)
+
+  it("should error for URI of unopened file", function()
+    _G.vim.fn.bufnr = spy.new(function()
+      return -1 -- File not open
+    end)
+
+    local success, err = pcall(get_diagnostics_handler, { uri = "file:///unknown/file.lua" })
+    expect(success).to_be_false()
+    expect(err).to_be_table()
+    expect(err.code).to_be(-32001)
+    expect(err.message).to_be("File not open")
+    assert_contains(err.data, "File must be open to retrieve diagnostics: /unknown/file.lua")
+
+    -- Should have used vim.uri_to_fname and checked for buffer but not called vim.diagnostic.get
+    assert.spy(_G.vim.uri_to_fname).was_called_with("file:///unknown/file.lua")
+    assert.spy(_G.vim.fn.bufnr).was_called_with("/unknown/file.lua")
+    assert.spy(_G.vim.diagnostic.get).was_not_called()
+  end)
 end)