eval: writefile: Give more adequate IO errors and do not call putc()

src/nvim/eval.c

@@ -30,6 +30,7 @@
 #include "nvim/ex_eval.h"
 #include "nvim/ex_getln.h"
 #include "nvim/fileio.h"
+#include "nvim/os/fileio.h"
 #include "nvim/func_attr.h"
 #include "nvim/fold.h"
 #include "nvim/getchar.h"
@@ -18021,29 +18022,53 @@ static void f_winsaveview(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 }
 
 /// Writes list of strings to file
-static bool write_list(FILE *fd, list_T *list, bool binary)
+///
+/// @param  fp  File to write to.
+/// @param[in]  list  List to write.
+/// @param[in]  binary  Whether to write in binary mode.
+///
+/// @return true in case of success, false otherwise.
+static bool write_list(FileDescriptor *const fp, const list_T *const list,
+                       const bool binary)
 {
-  int ret = true;
+  for (const listitem_T *li = list->lv_first; li != NULL; li = li->li_next) {
-
+    int error = 0;
-  for (listitem_T *li = list->lv_first; li != NULL; li = li->li_next) {
+    const char *const s = (const char *)get_tv_string((typval_T *)&li->li_tv);
-    for (char_u *s = get_tv_string(&li->li_tv); *s != NUL; ++s) {
+    const char *hunk_start = s;
-      if (putc(*s == '\n' ? NUL : *s, fd) == EOF) {
+    for (const char *p = hunk_start;; p++) {
-        ret = false;
+      if (*p == NUL || *p == NL) {
-        break;
+        if (p != hunk_start) {
+          const ptrdiff_t written = file_write(fp, hunk_start,
+                                               (size_t)(p - hunk_start));
+          if (written < 0) {
+            error = (int)written;
+            break;
+          }
+        }
+        if (*p == NUL) {
+          break;
+        } else {
+          hunk_start = p + 1;
+          const ptrdiff_t written = file_write(fp, (char []){ NUL }, 1);
+          if (written < 0) {
+            error = (int)written;
+            break;
+          }
+        }
       }
     }
     if (!binary || li->li_next != NULL) {
-      if (putc('\n', fd) == EOF) {
+      const ptrdiff_t written = file_write(fp, "\n", 1);
-        ret = false;
+      if (written < 0) {
-        break;
+        error = (int)written;
       }
     }
-    if (ret == false) {
+    if (error != 0) {
-      EMSG(_(e_write));
+      emsgf(_("E80: Error while writing: %s"), os_strerror(error));
-      break;
+      return false;
     }
   }
-  return ret;
+  return true;
 }
 
 /// Saves a typval_T as a string.
@@ -18153,17 +18178,25 @@ static void f_writefile(typval_T *argvars, typval_T *rettv, FunPtr fptr)
 
   // Always open the file in binary mode, library functions have a mind of
   // their own about CR-LF conversion.
-  char_u *fname = get_tv_string(&argvars[1]);
+  const char *const fname = (const char *)get_tv_string(&argvars[1]);
-  FILE *fd;
+  FileDescriptor *fp;
-  if (*fname == NUL || (fd = mch_fopen((char *)fname,
+  int error;
-       append ? APPENDBIN : WRITEBIN)) == NULL) {
+  rettv->vval.v_number = -1;
-    EMSG2(_(e_notcreate), *fname == NUL ? (char_u *)_("<empty>") : fname);
+  if (*fname == NUL) {
-    rettv->vval.v_number = -1;
+    EMSG(_("E482: Can't open file with an empty name"));
+  } else if ((fp = file_open_new(&error, fname,
+                                 ((append ? kFileAppend : kFileTruncate)
+                                  | kFileCreate), 0666)) == NULL) {
+    emsgf(_("E482: Can't open file %s for writing: %s"),
+          fname, os_strerror(error));
   } else {
-    if (write_list(fd, argvars[0].vval.v_list, binary) == false) {
+    if (write_list(fp, argvars[0].vval.v_list, binary)) {
-      rettv->vval.v_number = -1;
+      rettv->vval.v_number = 0;
+    }
+    if ((error = file_free(fp)) != 0) {
+      emsgf(_("E80: Error when closing file %s: %s"),
+            fname, os_strerror(error));
     }
-    fclose(fd);
   }
 }
 /*

src/nvim/ex_cmds.c

@@ -1131,11 +1131,12 @@ static void do_filter(
    */
   ++no_wait_return;             /* don't call wait_return() while busy */
   if (itmp != NULL && buf_write(curbuf, itmp, NULL, line1, line2, eap,
-          FALSE, FALSE, FALSE, TRUE) == FAIL) {
+                                false, false, false, true) == FAIL) {
-    msg_putchar('\n');                  /* keep message from buf_write() */
+    msg_putchar('\n');  // Keep message from buf_write().
-    --no_wait_return;
+    no_wait_return--;
-    if (!aborting())
+    if (!aborting()) {
-      (void)EMSG2(_(e_notcreate), itmp);        /* will call wait_return */
+      EMSG2(_("E482: Can't create file %s"), itmp);  // Will call wait_return.
+    }
     goto filterend;
   }
   if (curbuf != old_curbuf)

src/nvim/globals.h

@@ -1145,7 +1145,6 @@ EXTERN char_u e_noprev[] INIT(= N_("E34: No previous command"));
 EXTERN char_u e_noprevre[] INIT(= N_("E35: No previous regular expression"));
 EXTERN char_u e_norange[] INIT(= N_("E481: No range allowed"));
 EXTERN char_u e_noroom[] INIT(= N_("E36: Not enough room"));
-EXTERN char_u e_notcreate[] INIT(= N_("E482: Can't create file %s"));
 EXTERN char_u e_notmp[] INIT(= N_("E483: Can't get temp file name"));
 EXTERN char_u e_notopen[] INIT(= N_("E484: Can't open file %s"));
 EXTERN char_u e_notread[] INIT(= N_("E485: Can't read file %s"));

test/functional/eval/writefile_spec.lua

@@ -0,0 +1,93 @@
+local helpers = require('test.functional.helpers')(after_each)
+
+local clear = helpers.clear
+local eq = helpers.eq
+local funcs = helpers.funcs
+local exc_exec = helpers.exc_exec
+local read_file = helpers.read_file
+
+local fname = 'Xtest-functional-eval-writefile'
+
+before_each(clear)
+after_each(function() os.remove(fname) end)
+
+describe('writefile()', function()
+  it('writes empty list to a file', function()
+    eq(nil, read_file(fname))
+    eq(0, funcs.writefile({}, fname))
+    eq('', read_file(fname))
+    os.remove(fname)
+    eq(nil, read_file(fname))
+    eq(0, funcs.writefile({}, fname, 'b'))
+    eq('', read_file(fname))
+    os.remove(fname)
+    eq(nil, read_file(fname))
+    eq(0, funcs.writefile({}, fname, 'ab'))
+    eq('', read_file(fname))
+    os.remove(fname)
+    eq(nil, read_file(fname))
+    eq(0, funcs.writefile({}, fname, 'a'))
+    eq('', read_file(fname))
+  end)
+
+  it('writes list with an empty string to a file', function()
+    eq(0, exc_exec(
+       ('call writefile([$XXX_NONEXISTENT_VAR_XXX], "%s", "b")'):format(
+          fname)))
+    eq('', read_file(fname))
+    eq(0, exc_exec(('call writefile([$XXX_NONEXISTENT_VAR_XXX], "%s")'):format(
+        fname)))
+    eq('\n', read_file(fname))
+  end)
+
+  it('appends to a file', function()
+    eq(nil, read_file(fname))
+    eq(0, funcs.writefile({'abc', 'def', 'ghi'}, fname))
+    eq('abc\ndef\nghi\n', read_file(fname))
+    eq(0, funcs.writefile({'jkl'}, fname, 'a'))
+    eq('abc\ndef\nghi\njkl\n', read_file(fname))
+    os.remove(fname)
+    eq(nil, read_file(fname))
+    eq(0, funcs.writefile({'abc', 'def', 'ghi'}, fname, 'b'))
+    eq('abc\ndef\nghi', read_file(fname))
+    eq(0, funcs.writefile({'jkl'}, fname, 'ab'))
+    eq('abc\ndef\nghijkl', read_file(fname))
+  end)
+
+  it('correctly treats NLs', function()
+    eq(0, funcs.writefile({'\na\nb\n'}, fname, 'b'))
+    eq('\0a\0b\0', read_file(fname))
+    eq(0, funcs.writefile({'a\n\n\nb'}, fname, 'b'))
+    eq('a\0\0\0b', read_file(fname))
+  end)
+
+  it('correctly overwrites file', function()
+    eq(0, funcs.writefile({'\na\nb\n'}, fname, 'b'))
+    eq('\0a\0b\0', read_file(fname))
+    eq(0, funcs.writefile({'a\n'}, fname, 'b'))
+    eq('a\0', read_file(fname))
+  end)
+
+  it('errors out with invalid arguments', function()
+    eq('Vim(call):E119: Not enough arguments for function: writefile',
+       exc_exec('call writefile()'))
+    eq('Vim(call):E119: Not enough arguments for function: writefile',
+       exc_exec('call writefile([])'))
+    eq('Vim(call):E118: Too many arguments for function: writefile',
+       exc_exec(('call writefile([], "%s", "b", 1)'):format(fname)))
+    for _, arg in ipairs({'0', '0.0', 'function("tr")', '{}', '"test"'}) do
+      eq('Vim(call):E686: Argument of writefile() must be a List',
+         exc_exec(('call writefile(%s, "%s", "b")'):format(arg, fname)))
+    end
+    for _, args in ipairs({'%s, "b"', '"' .. fname .. '", %s'}) do
+      eq('Vim(call):E806: using Float as a String',
+         exc_exec(('call writefile([], %s)'):format(args:format('0.0'))))
+      eq('Vim(call):E730: using List as a String',
+         exc_exec(('call writefile([], %s)'):format(args:format('[]'))))
+      eq('Vim(call):E731: using Dictionary as a String',
+         exc_exec(('call writefile([], %s)'):format(args:format('{}'))))
+      eq('Vim(call):E729: using Funcref as a String',
+         exc_exec(('call writefile([], %s)'):format(args:format('function("tr")'))))
+    end
+  end)
+end)

test/functional/helpers.lua

@@ -343,6 +343,16 @@ local function write_file(name, text, dont_dedent)
   file:close()
 end
 
+local function read_file(name)
+  local file = io.open(name, 'r')
+  if not file then
+    return nil
+  end
+  local ret = file:read('*a')
+  file:close()
+  return ret
+end
+
 local function source(code)
   local fname = tmpname()
   write_file(fname, code)
@@ -584,6 +594,7 @@ local M = {
   sleep = sleep,
   set_session = set_session,
   write_file = write_file,
+  read_file = read_file,
   os_name = os_name,
   rmdir = rmdir,
   mkdir = lfs.mkdir,