refactor(path): path_has_wildcard() #40893

Problem:
Redundant code.

Solution:
Combine path_has_wildcard() and path_has_exp_wildcard(). They have similar logic.
Also, the latter operates on whatever wildcards remain after shell/env/backtick
expansion, so the two felt somewhat related.

Also drop some unnecessary MB_PTR_ADV calls.

src/nvim/charset.c

@@ -882,7 +882,7 @@ bool vim_isfilec_or_wc(int c)
   char buf[2];
   buf[0] = (char)c;
   buf[1] = NUL;
-  return vim_isfilec(c) || c == ']' || path_has_wildcard(buf);
+  return vim_isfilec(c) || c == ']' || path_has_wildcard(buf, true);
 }
 
 /// Check that "c" is a printable character.

src/nvim/ex_docmd.c

@@ -4006,7 +4006,7 @@ int expand_filename(exarg_T *eap, char **cmdlinep, const char **errormsgp)
   // Decide to expand wildcards *before* replacing '%', '#', etc.  If
   // the file name contains a wildcard it should not cause expanding.
   // (it will be expanded anyway if there is a wildcard before replacing).
-  bool has_wildcards = path_has_wildcard(p);
+  bool has_wildcards = path_has_wildcard(p, true);
   while (*p != NUL) {
     // Skip over `=expr`, wildcards in it are not expanded.
     if (p[0] == '`' && p[1] == '=') {
@@ -4109,7 +4109,7 @@ int expand_filename(exarg_T *eap, char **cmdlinep, const char **errormsgp)
           || vim_strchr(eap->arg, '~') != NULL) {
         expand_env_esc(eap->arg, NameBuff, MAXPATHL, (char *)(" \t" PATH_ESC_WILDCARDS), true,
                        NULL);
-        has_wildcards = path_has_wildcard(NameBuff);
+        has_wildcards = path_has_wildcard(NameBuff, true);
         p = NameBuff;
       } else {
         p = NULL;

src/nvim/os/shell.c

@@ -72,7 +72,7 @@ static void save_patterns(int num_pat, char **pat, int *num_file, char ***file)
 static bool have_wildcard(int num, char **file)
 {
   for (int i = 0; i < num; i++) {
-    if (path_has_wildcard(file[i])) {
+    if (path_has_wildcard(file[i], true)) {
       return true;
     }
   }

src/nvim/path.c

@@ -107,13 +107,11 @@ char *path_tail(const char *fname)
   }
 
   const char *tail = get_past_head(fname);
-  const char *p = tail;
   // Find last part of path.
-  while (*p != NUL) {
+  for (const char *p = tail; *p != NUL; p++) {
     if (vim_ispathsep_nocolon(*p)) {
       tail = p + 1;
     }
-    MB_PTR_ADV(p);
   }
   return (char *)tail;
 }
@@ -197,7 +195,7 @@ const char *path_next_component(const char *fname)
   FUNC_ATTR_NONNULL_ALL
 {
   while (*fname != NUL && !vim_ispathsep(*fname)) {
-    MB_PTR_ADV(fname);
+    fname++;
   }
   if (*fname != NUL) {
     fname++;
@@ -588,27 +586,28 @@ char *save_abs_path(const char *name)
   return TO_SLASH_SAVE(name);
 }
 
-/// Checks if a path has a wildcard character including '~', unless at the end.
-/// @param p  The path to expand.
-/// @returns Unix: True if it contains one of "?[{`'$".
-/// @returns Windows: True if it contains one of "*?$[".
-bool path_has_wildcard(const char *p)
+/// Checks if a path has an unescaped wildcard requiring expansion (path_expand).
+/// @param p The path to expand.
+/// @param all Also detect chars requiring special handling:
+///            - "`": backtick expansion
+///            - "'": shell quoting (Only Unix)
+///            - "$": environment variable expansion
+///            - "~": home directory expansion (historically treated as a wildcard
+///                   unless at the end)
+/// @return True if it has an unescaped wildcard
+bool path_has_wildcard(const char *p, bool all)
   FUNC_ATTR_NONNULL_ALL
 {
-  for (; *p; MB_PTR_ADV(p)) {
+  char *wildcards = all ? PATH_ALL_WILDCARDS : PATH_ESC_WILDCARDS;
+  for (; *p; p++) {
 #ifdef UNIX
     if (p[0] == '\\' && p[1] != NUL) {
       p++;
       continue;
     }
-
-    const char *wildcards = "*?[{`'$";
-#else
-    // Windows:
-    const char *wildcards = "?*$[`";
 #endif
     if (vim_strchr(wildcards, (uint8_t)(*p)) != NULL
-        || (p[0] == '~' && p[1] != NUL)) {
+        || (all && p[0] == '~' && p[1] != NUL)) {
       return true;
     }
   }
@@ -620,31 +619,6 @@ static int pstrcmp(const void *a, const void *b)
   return pathcmp(*(char **)a, *(char **)b, -1);
 }
 
-/// Checks if a path has a character path_expand can expand.
-/// @param p  The path to expand.
-/// @returns Unix: True if it contains one of *?[{.
-/// @returns Windows: True if it contains one of *?[.
-bool path_has_exp_wildcard(const char *p)
-  FUNC_ATTR_NONNULL_ALL
-{
-  for (; *p != NUL; MB_PTR_ADV(p)) {
-#ifdef UNIX
-    if (p[0] == '\\' && p[1] != NUL) {
-      p++;
-      continue;
-    }
-
-    const char *wildcards = "*?[{";
-#else
-    const char *wildcards = "*?[";  // Windows.
-#endif
-    if (vim_strchr(wildcards, (uint8_t)(*p)) != NULL) {
-      return true;
-    }
-  }
-  return false;
-}
-
 /// Recursively expands one path component into all matching files and/or
 /// directories. Handles "*", "?", "[a-z]", "**", etc.
 /// @remark "**" in `path` requests recursive expansion.
@@ -718,14 +692,13 @@ static size_t do_path_expand(garray_T *gap, const char *path, size_t wildoff, in
       }
       s = p + 1;
     } else if (path_end >= path + wildoff
-#ifdef MSWIN
                // "~" not included here, we want to treat it as literal.
-               // The "~/" case is already handled in `gen_expand_wildcards`.
-               && vim_strchr("*?[", (uint8_t)(*path_end)) != NULL
-#else
-               && (vim_strchr("*?[{~$", (uint8_t)(*path_end)) != NULL
-                   || (!p_fic && (flags & EW_ICASE) && mb_isalpha(utf_ptr2char(path_end))))
+               // "~/", "~user/" and env expansion are already handled in `gen_expand_wildcards`.
+               && (vim_strchr(PATH_ESC_WILDCARDS, (uint8_t)(*path_end)) != NULL
+#ifndef MSWIN
+                   || (!p_fic && (flags & EW_ICASE) && mb_isalpha(utf_ptr2char(path_end)))
 #endif
+                   )
                ) {
       e = p;
     }
@@ -829,7 +802,7 @@ static size_t do_path_expand(garray_T *gap, const char *path, size_t wildoff, in
         }
 
         vim_snprintf(buf + len, buflen - len, "%s", path_end);
-        if (path_has_exp_wildcard(path_end)) {      // handle more wildcards
+        if (path_has_wildcard(path_end, false)) {      // handle more wildcards
           if (stardepth < 100) {
             stardepth++;
             // need to expand another component of the path
@@ -1186,7 +1159,7 @@ const char *gettail_dir(const char *const fname)
   const char *next_dir_end = fname;
   bool look_for_sep = true;
 
-  for (const char *p = fname; *p != NUL;) {
+  for (const char *p = fname; *p != NUL; p++) {
     if (vim_ispathsep(*p)) {
       if (look_for_sep) {
         next_dir_end = p;
@@ -1198,7 +1171,6 @@ const char *gettail_dir(const char *const fname)
       }
       look_for_sep = true;
     }
-    MB_PTR_ADV(p);
   }
   return dir_end;
 }
@@ -1249,7 +1221,7 @@ static int expand_in_path(garray_T *const gap, char *const pattern, const int fl
 static bool has_env_var(char *p)
   FUNC_ATTR_NONNULL_ALL
 {
-  for (; *p; MB_PTR_ADV(p)) {
+  for (; *p; p++) {
     if (*p == '\\' && p[1] != NUL) {
       p++;
     } else if (vim_strchr("$", (uint8_t)(*p)) != NULL) {
@@ -1266,7 +1238,7 @@ static bool has_env_var(char *p)
 static bool has_special_wildchar(char *p, int flags)
   FUNC_ATTR_NONNULL_ALL
 {
-  for (; *p; MB_PTR_ADV(p)) {
+  for (; *p; p++) {
     // Disallow line break characters.
     if (*p == '\r' || *p == '\n') {
       break;
@@ -1391,7 +1363,7 @@ int gen_expand_wildcards(int num_pat, char **pat, int *num_file, char ***file, i
       // there is no match, and EW_NOTFOUND is given, add the pattern.
       // Otherwise: Add the file name if it exists or when EW_NOTFOUND is
       // given.
-      if (path_has_exp_wildcard(p) || (flags & EW_ICASE)) {
+      if (path_has_wildcard(p, false) || (flags & EW_ICASE)) {
         if ((flags & (EW_PATH | EW_CDPATH))
             && !path_is_absolute(p)
             && !(p[0] == '.'
@@ -1547,12 +1519,7 @@ void slash_adjust(char *p)
 
   char from = p_ssl ? '\\' : PATHSEP;
   char to = p_ssl ? PATHSEP : '\\';
-  while (*p) {
-    if (*p == from) {
-      *p = to;
-    }
-    MB_PTR_ADV(p);
-  }
+  strchrsub(p, from, to);
 }
 #endif
 

src/nvim/path.h

@@ -53,8 +53,10 @@ typedef enum file_comparison {
 
 #ifdef MSWIN
 # define PATH_ESC_WILDCARDS "*?["
+# define PATH_ALL_WILDCARDS "*?[`$"
 #else
 # define PATH_ESC_WILDCARDS "*?[{"
+# define PATH_ALL_WILDCARDS "*?[{`'$"
 #endif
 
 #include "path.h.generated.h"

src/nvim/tag.c

@@ -2938,7 +2938,7 @@ static char *expand_tag_fname(char *fname, char *const tag_fname, const bool exp
   // Expand file name (for environment variables) when needed.
   // Disallow backticks, they could execute arbitrary shell
   // commands.  This is not needed for tag filenames.
-  if (expand && path_has_wildcard(fname) && vim_strchr(fname, '`') == NULL) {
+  if (expand && path_has_wildcard(fname, true) && vim_strchr(fname, '`') == NULL) {
     ExpandInit(&xpc);
     xpc.xp_context = EXPAND_FILES;
     expanded_fname = ExpandOne(&xpc, fname, NULL,

test/functional/vimscript/glob_spec.lua

@@ -4,6 +4,7 @@ local n = require('test.functional.testnvim')()
 local describe, it, before_each, after_each = t.describe, t.it, t.before_each, t.after_each
 local clear, command, eval, eq = n.clear, n.command, n.eval, t.eq
 local mkdir = t.mkdir
+local fn = n.fn
 
 before_each(function()
   clear()
@@ -29,3 +30,33 @@ describe('glob()', function()
     eq({}, eval("glob('*', 0, 1)"))
   end)
 end)
+
+describe('glob() with $ and ~ mixed with wildcards', function()
+  before_each(function()
+    clear()
+    mkdir('test-wild')
+    command('silent cd test-wild')
+    -- Names containing literal '$' and '~'.
+    command([[call writefile([], 'a$b.txt')]])
+    command([[call writefile([], 'a$c.txt')]])
+    mkdir('test-wild/d~e')
+    command([[call writefile([], 'd~e/f.txt')]])
+  end)
+
+  after_each(function()
+    n.rmdir('test-wild')
+  end)
+
+  it('$VAR is expanded before the trailing wildcard is globbed', function()
+    command('let $WILDDIR = "d~e"')
+    eq({ 'd~e/f.txt' }, eval([[glob('$WILDDIR/*', 0, 1)]]))
+  end)
+
+  it("wildcard descends through a component containing '~'", function()
+    eq({ 'd~e/f.txt' }, eval([[glob('d~e/*', 0, 1)]]))
+  end)
+
+  it("'~' mid-component combined with a wildcard", function()
+    eq({ 'd~e' }, eval([[glob('d~*', 0, 1)]]))
+  end)
+end)