ronantakizawa/codereview-bench huggingface dataset:
Repository: neovim/neovim
File: runtime/lua/vim/lsp/diagnostic.lua (Lua)

Code before:
      return api.nvim_win_get_buf(window) ~= bufnr
    end)
  then
    return
  end

  local method = ms.textDocument_diagnostic
  local clients = vim.lsp.get_clients({ bufnr = bufnr, method = method, id = client_id })
  local bufstate = assert(bufstates[bufnr])

  util._cancel_requests({
    bufnr = bufnr,
    clients = clients,
    method = method,
    type = 'pending',
  })
  for _, client in ipairs(clients) do
    ---@type lsp.DocumentDiagnosticParams
    local params = {
      textDocument = util.make_text_document_params(bufnr),
      previousResultId = bufstate.client_result_id[client.id],
    }
    client:request(method, params, nil, bufnr)
  end
end

--- Enable pull diagnostics for a buffer
---@param bufnr (integer) Buffer handle, or 0 for current
---@private
function M._enable(bufnr)
  bufnr = vim._resolve_bufnr(bufnr)

  if not bufstates[bufnr] then
    bufstates[bufnr] = { enabled = true, client_result_id = {} }

    api.nvim_create_autocmd('LspNotify', {
      buffer = bufnr,
      callback = function(opts)
        if
          opts.data.method ~= ms.textDocument_didChange
          and opts.data.method ~= ms.textDocument_didOpen
        then
          return
        end
        if bufstates[bufnr] and bufstates[bufnr].enabled then
          local client_id = opts.data.client_id --- @type integer?
          _refresh(bufnr, client_id, true)
        end
      end,
      group = augroup,
    })

Review comment (suggestion):
We'll want to set `client_result_id = {}` when disabling diagnostics: https://github.com/neovim/neovim/blob/f1422a313fa57c8c7045163aa209a9fd9166fe5a/runtime/lua/vim/lsp/diagnostic.lua#L341

Code after:
  local bufstate = assert(bufstates[bufnr])

  util._cancel_requests({
    bufnr = bufnr,
    clients = clients,
    method = method,
    type = 'pending',
  })
  for _, client in ipairs(clients) do
    ---@type lsp.DocumentDiagnosticParams
    local params = {
      textDocument = util.make_text_document_params(bufnr),
      previousResultId = bufstate.client_result_id[client.id],
    }
    client:request(method, params, nil, bufnr)
  end
end

--- Enable pull diagnostics for a buffer
---@param bufnr (integer) Buffer handle, or 0 for current
---@private
function M._enable(bufnr)
  bufnr = vim._resolve_bufnr(bufnr)

  if not bufstates[bufnr] then
    bufstates[bufnr] = { enabled = true, client_result_id = {} }

    api.nvim_create_autocmd('LspNotify', {
      buffer = bufnr,
      callback = function(opts)
        if
          opts.data.method ~= ms.textDocument_didChange
          and opts.data.method ~= ms.textDocument_didOpen
        then
          return
        end
        if bufstates[bufnr] and bufstates[bufnr].enabled then
          local client_id = opts.data.client_id --- @type integer?
          _refresh(bufnr, client_id, true)
        end
      end,
      group = augroup,
    })

    api.nvim_buf_attach(bufnr, false, {
      on_reload = function()
        if bufstates[bufnr] and bufstates[bufnr].enabled then
          _refresh(bufnr)
        end
      end,
      on_detach = function()
Repository: neovim/neovim
File: src/nvim/strings.c (C)

Code before:
        return true;
      }
    }
  }
  return false;
}

/// Concatenate two strings and return the result in allocated memory.
char *concat_str(const char *restrict str1, const char *restrict str2)
  FUNC_ATTR_NONNULL_RET FUNC_ATTR_MALLOC FUNC_ATTR_NONNULL_ALL
{
  size_t l = strlen(str1);
  char *dest = xmalloc(l + strlen(str2) + 1);
  STRCPY(dest, str1);
  STRCPY(dest + l, str2);
  return dest;
}

/// Concatenate two strings with separator in middle and return the result in allocated memory.
char *join_str(const char *restrict str1, const char *restrict str2, const char *restrict sep)
  FUNC_ATTR_NONNULL_RET FUNC_ATTR_MALLOC FUNC_ATTR_NONNULL_ALL
{
  size_t l = strlen(str1);
  size_t s = strlen(sep);
  char *dest = xmalloc(l + s + strlen(str2) + 1);
  STRCPY(dest, str1);
  STRCPY(dest + l, sep);
  STRCPY(dest + l + s, str2);
  return dest;
}


static const char *const e_printf =
  N_("E766: Insufficient arguments for printf()");

/// Get number argument from idxp entry in tvs
///
/// Will give an error message for Vimscript entry with invalid type or for insufficient entries.
///
/// @param[in]  tvs  List of Vimscript values. List is terminated by VAR_UNKNOWN value.
/// @param[in,out]  idxp  Index in a list. Will be incremented. Indexing starts at 1.
///
/// @return Number value or 0 in case of error.
static varnumber_T tv_nr(typval_T *tvs, int *idxp)
  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
{
  int idx = *idxp - 1;
  varnumber_T n = 0;

  if (tvs[idx].v_type == VAR_UNKNOWN) {
    emsg(_(e_printf));

Review comment (suggestion):
can use memcpy since we already calculated the lengths. not sure why `concat_str` doesn't do that.

Code after:
        return true;
      }
    }
  }
  return false;
}

/// Concatenate two strings and return the result in allocated memory.
char *concat_str(const char *restrict str1, const char *restrict str2)
  FUNC_ATTR_NONNULL_RET FUNC_ATTR_MALLOC FUNC_ATTR_NONNULL_ALL
{
  size_t l = strlen(str1);
  char *dest = xmalloc(l + strlen(str2) + 1);
  STRCPY(dest, str1);
  STRCPY(dest + l, str2);
  return dest;
}

static const char *const e_printf =
  N_("E766: Insufficient arguments for printf()");

/// Get number argument from idxp entry in tvs
///
/// Will give an error message for Vimscript entry with invalid type or for insufficient entries.
///
/// @param[in]  tvs  List of Vimscript values. List is terminated by VAR_UNKNOWN value.
/// @param[in,out]  idxp  Index in a list. Will be incremented. Indexing starts at 1.
///
/// @return Number value or 0 in case of error.
static varnumber_T tv_nr(typval_T *tvs, int *idxp)
  FUNC_ATTR_NONNULL_ALL FUNC_ATTR_WARN_UNUSED_RESULT
{
  int idx = *idxp - 1;
  varnumber_T n = 0;

  if (tvs[idx].v_type == VAR_UNKNOWN) {
    emsg(_(e_printf));
  } else {
    (*idxp)++;
    bool err = false;
    n = tv_get_number_chk(&tvs[idx], &err);
    if (err) {
      n = 0;
    }
  }
  return n;
}

/// Get string argument from idxp entry in tvs
///
/// Will give an error message for Vimscript entry with invalid type or for
Repository: neovim/neovim
File: runtime/lua/vim/secure.lua (Lua)

Code before:
    hash = vim.fn.sha256(contents)
  end

  return contents, hash
end

--- Writes provided {trust} table to trust database at
--- $XDG_STATE_HOME/nvim/trust.
---
---@param trust table<string, string> Trust table to write
local function write_trust(trust)
  vim.validate('trust', trust, 'table')
  local f = assert(io.open(vim.fn.stdpath('state') .. '/trust', 'w'))

  local t = {} ---@type string[]
  for p, h in pairs(trust) do
    t[#t + 1] = string.format('%s %s\n', h, p)
  end
  f:write(table.concat(t))
  f:close()
end

--- If {path} is a file: attempt to read the file, prompting the user if the file should be
--- trusted.
---
--- If {path} is a directory: return true if the directory is trusted, prompting the user as
--- necessary.
---
--- The user's choice is persisted in a trust database at
--- $XDG_STATE_HOME/nvim/trust.
---
---@since 11
---@see |:trust|
---
---@param path (string) Path to a file or directory to read.
---
---@return (boolean|string|nil) If {path} is not trusted or does not exist, returns `nil`. Otherwise,
---        returns the contents of {path} if it is a file, or true if {path} is a directory.
function M.read(path)
  vim.validate('path', path, 'string')
  local fullpath = vim.uv.fs_realpath(vim.fs.normalize(path))
  if not fullpath then
    return nil
  end

  local trust = read_trust()

  if trust[fullpath] == '!' then
    -- File is denied
    return nil
  end

Review comment (suggestion):
```suggestion
--- If {path} is a directory: return true if the directory is trusted (non-recursive), prompting the user as
```

Code after:

  hash = vim.fn.sha256(contents)

  return contents, hash
end

--- Writes provided {trust} table to trust database at
--- $XDG_STATE_HOME/nvim/trust.
---
---@param trust table<string, string> Trust table to write
local function write_trust(trust)
  vim.validate('trust', trust, 'table')
  local f = assert(io.open(vim.fn.stdpath('state') .. '/trust', 'w'))

  local t = {} ---@type string[]
  for p, h in pairs(trust) do
    t[#t + 1] = string.format('%s %s\n', h, p)
  end
  f:write(table.concat(t))
  f:close()
end

--- If {path} is a file: attempt to read the file, prompting the user if the file should be
--- trusted.
---
--- If {path} is a directory: return true if the directory is trusted (non-recursive), prompting
--- the user as necessary.
---
--- The user's choice is persisted in a trust database at
--- $XDG_STATE_HOME/nvim/trust.
---
---@since 11
---@see |:trust|
---
---@param path (string) Path to a file or directory to read.
---
---@return (boolean|string|nil) If {path} is not trusted or does not exist, returns `nil`. Otherwise,
---        returns the contents of {path} if it is a file, or true if {path} is a directory.
function M.read(path)
  vim.validate('path', path, 'string')
  local fullpath = vim.uv.fs_realpath(vim.fs.normalize(path))
  if not fullpath then
    return nil
  end

  local trust = read_trust()

  if trust[fullpath] == '!' then
    -- File is denied
    return nil
  end
Repository: neovim/neovim
File: runtime/lua/vim/secure.lua (Lua)

Code before:
  local fullpath = vim.uv.fs_realpath(vim.fs.normalize(path))
  if not fullpath then
    return nil
  end

  local trust = read_trust()

  if trust[fullpath] == '!' then
    -- File is denied
    return nil
  end

  local contents, hash = compute_hash(fullpath, nil)
  if not contents then
    return nil
  end

  if trust[fullpath] == hash then
    -- File already exists in trust database
    return contents
  end

  local directory_note = ''
  if hash == 'directory' then
    directory_note =
      ' Note that directory trust is by name, if the contents change, the directory remains trusted.'
  end

  -- File either does not exist in trust database or the hash does not match
  local ok, result = pcall(
    vim.fn.confirm,
    string.format('%s is not trusted.%s', fullpath, directory_note),
    '&ignore\n&view\n&deny\n&allow',
    1
  )

  if not ok and result ~= 'Keyboard interrupt' then
    error(result)
  elseif not ok or result == 0 or result == 1 then
    -- Cancelled or ignored
    return nil
  elseif result == 2 then
    -- View
    vim.cmd('sview ' .. fullpath)
    return nil
  elseif result == 3 then
    -- Deny
    trust[fullpath] = '!'
    contents = nil
  elseif result == 4 then
    -- Allow

Review comment (suggestion):
```suggestion
      ' DIRECTORY trust is decided only by its name, not its contents.'
```

Code after:
  local fullpath = vim.uv.fs_realpath(vim.fs.normalize(path))
  if not fullpath then
    return nil
  end

  local trust = read_trust()

  if trust[fullpath] == '!' then
    -- File is denied
    return nil
  end

  local contents, hash = compute_hash(fullpath, nil)
  if not contents then
    return nil
  end

  if trust[fullpath] == hash then
    -- File already exists in trust database
    return contents
  end

  local dir_msg = ''
  if hash == 'directory' then
    dir_msg = ' DIRECTORY trust is decided only by its name, not its contents.'
  end

  -- File either does not exist in trust database or the hash does not match
  local ok, result = pcall(
    vim.fn.confirm,
    string.format('%s is not trusted.%s', fullpath, dir_msg),
    '&ignore\n&view\n&deny\n&allow',
    1
  )

  if not ok and result ~= 'Keyboard interrupt' then
    error(result)
  elseif not ok or result == 0 or result == 1 then
    -- Cancelled or ignored
    return nil
  elseif result == 2 then
    -- View
    vim.cmd('sview ' .. fullpath)
    return nil
  elseif result == 3 then
    -- Deny
    trust[fullpath] = '!'
    contents = nil
  elseif result == 4 then
    -- Allow
    trust[fullpath] = hash
Repository: neovim/neovim
File: runtime/lua/vim/_system.lua (Lua)

Code before:

  return renv
end

local is_win = vim.fn.has('win32') == 1

local M = {}

--- @param cmd string
--- @param opts uv.spawn.options
--- @param on_exit fun(code: integer, signal: integer)
--- @param on_error fun()
--- @return uv.uv_process_t, integer
local function spawn(cmd, opts, on_exit, on_error)
  if is_win then
    local cmd1 = vim.fn.exepath(cmd)
    if cmd1 ~= '' then
      cmd = cmd1
    end
  end

  local handle, pid_or_err = uv.spawn(cmd, opts, on_exit)
  if not handle then
    on_error()
    if opts.cwd and not uv.fs_stat(opts.cwd) then
      error(("%s: '%s'"):format(pid_or_err, opts.cwd))
    elseif vim.fn.executable(cmd) == 0 then
      error(('%s: "%s"'):format(pid_or_err, cmd))
    else
      error(pid_or_err)
    end
  end
  return handle, pid_or_err --[[@as integer]]
end

--- @param timeout integer
--- @param cb fun()
--- @return uv.uv_timer_t
local function timer_oneshot(timeout, cb)
  local timer = assert(uv.new_timer())
  timer:start(timeout, 0, function()
    timer:stop()
    timer:close()
    cb()
  end)
  return timer
end

--- @param state vim.SystemState
--- @param code integer
--- @param signal integer

Review comment (suggestion):
to match the one below, use " quotes in the message. also should we add a `cwd` hint in the message?
```suggestion
      error(('%s: cwd: "%s"'):format(pid_or_err, opts.cwd))
```

Code after:
  end

  return renv
end

local is_win = vim.fn.has('win32') == 1

local M = {}

--- @param cmd string
--- @param opts uv.spawn.options
--- @param on_exit fun(code: integer, signal: integer)
--- @param on_error fun()
--- @return uv.uv_process_t, integer
local function spawn(cmd, opts, on_exit, on_error)
  if is_win then
    local cmd1 = vim.fn.exepath(cmd)
    if cmd1 ~= '' then
      cmd = cmd1
    end
  end

  local handle, pid_or_err = uv.spawn(cmd, opts, on_exit)
  if not handle then
    on_error()
    if opts.cwd and not uv.fs_stat(opts.cwd) then
      error(("%s (cwd): '%s'"):format(pid_or_err, opts.cwd))
    elseif vim.fn.executable(cmd) == 0 then
      error(("%s (cmd): '%s'"):format(pid_or_err, cmd))
    else
      error(pid_or_err)
    end
  end
  return handle, pid_or_err --[[@as integer]]
end

--- @param timeout integer
--- @param cb fun()
--- @return uv.uv_timer_t
local function timer_oneshot(timeout, cb)
  local timer = assert(uv.new_timer())
  timer:start(timeout, 0, function()
    timer:stop()
    timer:close()
    cb()
  end)
  return timer
end

--- @param state vim.SystemState
--- @param code integer
Repository: neovim/neovim
File: test/functional/lua/system_spec.lua (Lua)

Code before:
    end)

    assert(ok, 'process did not exit')

    -- Check the process is no longer running
    assert(not vim.api.nvim_get_proc(obj.pid), 'process still exists')

    return res
  end)
end

describe('vim.system', function()
  before_each(function()
    clear()
  end)

  for name, system in pairs { sync = system_sync, async = system_async } do
    describe('(' .. name .. ')', function()
      it('failure modes', function()
        t.matches(
          "ENOENT%: no such file .* %(cmd%): 'non%-existent%-cmd'",
          t.pcall_err(system, { 'non-existent-cmd', 'arg1', 'arg2' }, { text = true })
        )

        t.matches(
          "ENOENT%: no such file .* %(cwd%): 'non%-existent%-cmd'",
          t.pcall_err(system, { 'echo', 'hello' }, { cwd = 'non-existent-cwd', text = true })
        )
      end)

      it('can run simple commands', function()
        eq('hello\n', system({ 'echo', 'hello' }, { text = true }).stdout)
      end)

      it('handle input', function()
        eq('hellocat', system({ 'cat' }, { stdin = 'hellocat', text = true }).stdout)
      end)

      it('supports timeout', function()
        eq({
          code = 124,
          signal = 15,
          stdout = '',
          stderr = '',
        }, system({ 'sleep', '10' }, { timeout = 1000 }))
      end)
    end)
  end

  it('kill processes', function()
    exec_lua(function()

Review comment (suggestion):
```suggestion
          "ENOENT%: no such file .* %(cwd%): 'non%-existent%-cwd'",
```

Code after:
    end)

    assert(ok, 'process did not exit')

    -- Check the process is no longer running
    assert(not vim.api.nvim_get_proc(obj.pid), 'process still exists')

    return res
  end)
end

describe('vim.system', function()
  before_each(function()
    clear()
  end)

  for name, system in pairs { sync = system_sync, async = system_async } do
    describe('(' .. name .. ')', function()
      it('failure modes', function()
        t.matches(
          "ENOENT%: no such file .* %(cmd%): 'non%-existent%-cmd'",
          t.pcall_err(system, { 'non-existent-cmd', 'arg1', 'arg2' }, { text = true })
        )

        t.matches(
          "ENOENT%: no such file .* %(cwd%): 'non%-existent%-cwd'",
          t.pcall_err(system, { 'echo', 'hello' }, { cwd = 'non-existent-cwd', text = true })
        )
      end)

      it('can run simple commands', function()
        eq('hello\n', system({ 'echo', 'hello' }, { text = true }).stdout)
      end)

      it('handle input', function()
        eq('hellocat', system({ 'cat' }, { stdin = 'hellocat', text = true }).stdout)
      end)

      it('supports timeout', function()
        eq({
          code = 124,
          signal = 15,
          stdout = '',
          stderr = '',
        }, system({ 'sleep', '10' }, { timeout = 1000 }))
      end)
    end)
  end

  it('kill processes', function()
    exec_lua(function()
Repository: neovim/neovim
File: runtime/lua/vim/diagnostic.lua (Lua)

Code before:

  local diagnostics = {} --- @type vim.Diagnostic[]
  for _, item in ipairs(list) do
    if item.valid == 1 then
      local lnum = math.max(0, item.lnum - 1)
      local col = math.max(0, item.col - 1)
      local end_lnum = item.end_lnum > 0 and (item.end_lnum - 1) or lnum
      local end_col = item.end_col > 0 and (item.end_col - 1) or col
      local severity = item.type ~= '' and M.severity[item.type] or M.severity.ERROR
      diagnostics[#diagnostics + 1] = {
        bufnr = item.bufnr,
        lnum = lnum,
        col = col,
        end_lnum = end_lnum,
        end_col = end_col,
        severity = severity,
        message = item.text,
      }
    end
  end
  return diagnostics
end

function M.status()
  local counts = M.count(0)
  local user_signs = vim.tbl_get(M.config(), 'signs', 'text') or {}
  local signs = vim.tbl_extend('keep', user_signs, { 'E', 'W', 'I', 'H' })
  local result_str = vim
    .iter(pairs(counts))
    :map(function(severity, count)
      return ('%s:%s'):format(signs[severity], count)
    end)
    :join(' ')

  return result_str
end

return M


Review comment (suggestion):
```suggestion
  local user_signs = vim.tbl_get(M.config() --[[@as vim.diagnostic.Opts]], 'signs', 'text') or {}
```
To satisfy CI. But M.config() should probably have an overload to handle both signatures `fun(vim.diagnostic.Opts): nil` and `fun(): vim.diagnostic.Opts`.

Code after:
        enable
          -- Enable everything by setting diagnostic_disabled to an empty table.
          and {}
        -- Disable everything (including as yet non-existing buffers and namespaces) by setting
        -- diagnostic_disabled to an empty table and set its metatable to always return true.
        or setmetatable({}, {
          __index = function()
            return true
          end,
        })
      )
    else
      local ns = M.get_namespace(ns_id)
      ns.disabled = not enable
    end
  else
    bufnr = vim._resolve_bufnr(bufnr)
    if not ns_id then
      diagnostic_disabled[bufnr] = (not enable) and true or nil
    else
      if type(diagnostic_disabled[bufnr]) ~= 'table' then
        if enable then
          return
        end
        diagnostic_disabled[bufnr] = {}
      end
      diagnostic_disabled[bufnr][ns_id] = (not enable) and true or nil
    end
  end

  if enable then
    M.show(ns_id, bufnr)
  else
    M.hide(ns_id, bufnr)
  end
end

--- Parse a diagnostic from a string.
---
--- For example, consider a line of output from a linter:
---
--- ```
--- WARNING filename:27:3: Variable 'foo' does not exist
--- ```
---
--- This can be parsed into |vim.Diagnostic| structure with:
---
--- ```lua
--- local s = "WARNING filename:27:3: Variable 'foo' does not exist"
--- local pattern = "^(%w+) %w+:(%d+):(%d+): (.+)$"
--- local groups = { "severity", "lnum", "col", "message" }
Repository: neovim/neovim
File: runtime/lua/vim/diagnostic.lua (Lua)

Code before:
--- The severities with 0 diagnostics are left out.
--- Example `E:2 W:3 I:4 H:5`
---
--- To customise appearance, set diagnostic signs text with
--- ```lua
--- vim.diagnostic.config({
---   signs = { text = { [vim.diagnostic.severity.ERROR] = 'Error', ... } }
--- })
--- ```
---
---@return string
function M.status()
  local counts = M.count(0)
  local user_signs = vim.tbl_get(M.config() --[[@as vim.diagnostic.Opts]], 'signs', 'text') or {}
  local signs = vim.tbl_extend('keep', user_signs, { 'E', 'W', 'I', 'H' })
  local result_str = vim
    .iter(pairs(counts))
    :map(function(severity, count)
      return ('%s:%s'):format(signs[severity], count)
    end)
    :join(' ')

  return result_str
end

if vim.o.statusline:find('vim.diagnostic.status') then
  vim.api.nvim_create_autocmd('DiagnosticChanged', {
    group = vim.api.nvim_create_augroup('nvim.diagnostic.status', {}),
    command = 'redrawstatus',
  })
end

return M


Review comment (suggestion):
checking this at module-load time is kind of random. I would just drop this check for now. Users can `:au! nvim.diagnostic.status` if it matters that much.

Code after:
        diagnostic_disabled[bufnr] = {}
      end
      diagnostic_disabled[bufnr][ns_id] = (not enable) and true or nil
    end
  end

  if enable then
    M.show(ns_id, bufnr)
  else
    M.hide(ns_id, bufnr)
  end
end

--- Parse a diagnostic from a string.
---
--- For example, consider a line of output from a linter:
---
--- ```
--- WARNING filename:27:3: Variable 'foo' does not exist
--- ```
---
--- This can be parsed into |vim.Diagnostic| structure with:
---
--- ```lua
--- local s = "WARNING filename:27:3: Variable 'foo' does not exist"
--- local pattern = "^(%w+) %w+:(%d+):(%d+): (.+)$"
--- local groups = { "severity", "lnum", "col", "message" }
--- vim.diagnostic.match(s, pattern, groups, { WARNING = vim.diagnostic.WARN })
--- ```
---
---@param str string String to parse diagnostics from.
---@param pat string Lua pattern with capture groups.
---@param groups string[] List of fields in a |vim.Diagnostic| structure to
---                    associate with captures from {pat}.
---@param severity_map table A table mapping the severity field from {groups}
---                          with an item from |vim.diagnostic.severity|.
---@param defaults table? Table of default values for any fields not listed in {groups}.
---                       When omitted, numeric values default to 0 and "severity" defaults to
---                       ERROR.
---@return vim.Diagnostic?: |vim.Diagnostic| structure or `nil` if {pat} fails to match {str}.
function M.match(str, pat, groups, severity_map, defaults)
  vim.validate('str', str, 'string')
  vim.validate('pat', pat, 'string')
  vim.validate('groups', groups, 'table')
  vim.validate('severity_map', severity_map, 'table', true)
  vim.validate('defaults', defaults, 'table', true)

  --- @type table<string,vim.diagnostic.Severity>
  severity_map = severity_map or M.severity

  local matches = { str:match(pat) } --- @type any[]