Consolidate complete cycle detection and always report error on cycle

Detect recursive calls to builtin complete and the internal completion in
the same place.

In 0a0149cc2 (Prevent infinite recursion when completion wraps variable assignment)
we don't print an error when completing certain aliases like:

	alias vim "A=B vim"

But we also gave no completions.
We could make this case work, but I think that trying to salvage situations
like this one is way too complex. Instead, let the user know by printing an
error. Not sure if the style of the error fits.

We could add some heuristic to alias to not add --wraps in some cyclic cases.

src/builtin_complete.cpp

@@ -359,14 +359,8 @@ maybe_t<int> builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t *
         parser.libdata().transient_commandlines.push_back(do_complete_param);
         cleanup_t remove_transient([&] { parser.libdata().transient_commandlines.pop_back(); });
 
-        if (parser.libdata().builtin_complete_current_commandline) {
-            // Prevent accidental recursion (see #6171).
-        } else if (parser.libdata().builtin_complete_recursion_level >= 24) {
-            // Allow a limited number of recursive calls to complete (#3474).
-            streams.err.append_format(L"%ls: maximum recursion depth reached\n", cmd);
-        } else {
-            parser.libdata().builtin_complete_recursion_level++;
-            assert(!parser.libdata().builtin_complete_current_commandline);
+        // Prevent accidental recursion (see #6171).
+        if (!parser.libdata().builtin_complete_current_commandline) {
             if (!have_do_complete_param)
                 parser.libdata().builtin_complete_current_commandline = true;
 
@@ -403,7 +397,6 @@ maybe_t<int> builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t *
                 streams.out.push_back(L'\n');
             }
 
-            parser.libdata().builtin_complete_recursion_level--;
             parser.libdata().builtin_complete_current_commandline = false;
         }
     } else if (path.empty() && gnu_opt.empty()

src/complete.cpp

@@ -1379,13 +1379,7 @@ void completer_t::complete_custom(const wcstring &cmd, const wcstring &cmdline,
         ctx.parser->libdata().transient_commandlines.push_back(unaliased_cmd);
         cleanup_t remove_transient(
             [&] { ctx.parser->libdata().transient_commandlines.pop_back(); });
-        // Prevent infinite recursion when the completion for x wraps "A=B x" (#7344).
-        // Don't report an error since this could be a legitimate alias.
-        static uint32_t complete_assignment_recursion_count;
-        if (complete_assignment_recursion_count++ < 24) {
-            run_on_command(std::move(unaliased_cmd));
-        }
-        complete_assignment_recursion_count--;
+        perform_for_command(std::move(unaliased_cmd));
         *do_file = false;
     } else if (!complete_param(cmd, previous_argument, current_argument,
                                !had_ddash)) {  // Invoke any custom completions for this command.
@@ -1511,6 +1505,21 @@ void completer_t::mark_completions_duplicating_arguments(const wcstring &cmd,
 }
 
 void completer_t::perform_for_command(wcstring cmd) {
+    // Limit recursion, in case a user-defined completion has cycles, or the completion for "x"
+    // wraps "A=B x" (#3474, #7344).  No need to do that when there is no parser: this happens only
+    // for autosuggestions where we don't evaluate command substitutions or variable assignments.
+    if (ctx.parser) {
+        if (ctx.parser->libdata().complete_recursion_level >= 24) {
+            debug(0, _(L"completion reached maximum recursion depth, possible cycle?"),
+                  cmd.c_str());
+            return;
+        }
+        ++ctx.parser->libdata().complete_recursion_level;
+    };
+    cleanup_t decrement{[this]() {
+        if (ctx.parser) --ctx.parser->libdata().complete_recursion_level;
+    }};
+
     const size_t cursor_pos = cmd.size();
 
     // Find the plain statement to operate on. The cursor may be past it (#1261), so backtrack

src/parser.h

@@ -148,8 +148,8 @@ struct library_data_t {
     /// Last reader run count.
     uint64_t last_exec_run_counter{UINT64_MAX};
 
-    /// Number of recursive calls to builtin_complete().
-    uint32_t builtin_complete_recursion_level{0};
+    /// Number of recursive calls to the internal completion function.
+    uint32_t complete_recursion_level{0};
 
     /// If we're currently repainting the commandline.
     /// Useful to stop infinite loops.

tests/checks/complete.fish

@@ -65,7 +65,7 @@ complete
 
 # Recursive calls to complete (see #3474)
 complete -c complete_test_recurse1 -xa '(echo recursing 1>&2; complete -C"complete_test_recurse1 ")'
-complete -C'complete_test_recurse1 '
+LANG=C complete -C'complete_test_recurse1 '
 # CHECKERR: recursing
 # CHECKERR: recursing
 # CHECKERR: recursing
@@ -90,7 +90,7 @@ complete -C'complete_test_recurse1 '
 # CHECKERR: recursing
 # CHECKERR: recursing
 # CHECKERR: recursing
-# CHECKERR: complete: maximum recursion depth reached
+# CHECKERR: <E> fish: completion reached maximum recursion depth, possible cycle?
 
 # short options
 complete -c foo -f -a non-option-argument

tests/checks/wraps.fish

@@ -1,6 +1,8 @@
 #RUN: %fish %s
 # Validate some things about command wrapping.
 
+set -g LANG C # For predictable error messages.
+
 # This tests that we do not trigger a combinatorial explosion - see #5638.
 # Ensure it completes successfully.
 complete -c testcommand --wraps "testcommand x "
@@ -30,3 +32,4 @@ complete -C'testcommand2 explicit '
 
 complete -c recvar --wraps 'A=B recvar'
 complete -C 'recvar '
+# CHECKERR: <E> fish: completion reached maximum recursion depth, possible cycle?