Repository: xonsh/xonsh
Pull request: feat: Ability to call the program by name from callable alias with the same name without the infinite loop error
Closes #3159

Now we have transparent way to wrap commands into callable aliases.

Example 1:
```python
def _echo(args):
    echo @(args)
aliases['echo'] = _echo
echo ok
# Before: freezed on infinite loop
# After: ok
```

Example 2 - elegant ExecAlias (it will be awesome with #4201):
```python
aliases['echo'] = "echo @('ok')"  # ExecAlias
echo
# Before: freezed on infinite loop
# After: ok
```

Example 3 - show error for the user without freezing
```python
aliases['first'] = "second @(1)"  # ExecAlias just for demo
aliases['second'] = "first @(1)"  # ExecAlias just for demo
first
# Before: freezed on infinite loop
# After: Exception: Infinite loop of calls for "first" alias.
```

## For community
⬇️  **Please click the 👍 reaction instead of leaving a `+1` or 👍  comment**


Problem statement:
Infinite alias call
I'm trying to override 'ls' command to display dotfiles in my $DOTFILES directory.
This code goes into an endless loop because _ls function calls ls command and it calls _ls function.

```python
def _ls():
    if $(pwd).rstrip(os.linesep) == $DOTFILES:
        ls -Ga
    else:
        ls -G
aliases['ls'] = _ls
```

## xonfig

<details>

```
+------------------+---------------------+
| xonsh            | 0.8.10              |
| Git SHA          | 2cb42bdb            |
| Commit Date      | Feb 6 16:49:16 2019 |
| Python           | 3.6.4               |
| PLY              | 3.11                |
| have readline    | True                |
| prompt toolkit   | 2.0.7               |
| shell type       | prompt_toolkit2     |
| pygments         | 2.3.1               |
| on posix         | True                |
| on linux         | False               |
| on darwin        | True                |
| on windows       | False               |
| on cygwin        | False               |
| on msys2         | False               |
| is superuser     | False               |
| default encoding | utf-8               |
| xonsh encoding   | utf-8               |
| encoding errors  | surrogateescape     |
+------------------+---------------------+
```

</details>

## Current Behavior
<!--- Tell us what happens instead of the expected behavior -->
<!--- If part of your bug report is a traceback, please first enter debug mode before triggering the error
To enter debug mode, set the environment variableNSH_DEBUG=1` _before_ starting `xonsh`.  `XO
On Linux and OSX, an easy way to to do this is to run `env XONSH_DEBUG=1 xonsh` -->

When I input "ls" command, the terminal freezes.

Hints from issue discussion:
Hi @miyashiiii, you can replace the inner `ls` calls with `@$(which -s ls)` which should resolve to your actual `ls` executable.
Hi @lacoch. Thank you for your answer.
I knew the way to avoid this issue, but I think the infinite alias call shouldn't happen.
cf. https://stackoverflow.com/questions/56492258/how-to-override-built-in-command-in-xonsh
I should have written it.
Oh, I see! Thanks for the pointer. I don't really think this is a bug, but if @gforsyth thinks so...
> I should have written it.

Yes, perhaps ;)
I asked @miyashiiii to open the issue.  It might not be a bug, but it does seem like a slightly inconsistent behavior to me.  It's a common pattern to alias commands + default arguments to the built-in command name and that definitely works when we define it directly, e.g.

```
gil@badcatredux ~ $ aliases['ls'] = ['ls', '--color=auto', '-v']                              
gil@badcatredux ~ $ ls                                                                        
aur.archlinux.org  github.com
```

Commit message: Merge branch 'main' of github.com:xonsh/xonsh into fix_aliases_infinite_loop

# Conflicts:
#	tests/test_commands_cache.py

Patch to review:
diff --git a/news/fix_aliases_infinite_loop.rst b/news/fix_aliases_infinite_loop.rst
new file mode 100644
index 0000000000..60a5ab3232
--- /dev/null
+++ b/news/fix_aliases_infinite_loop.rst
@@ -0,0 +1,23 @@
+**Added:**
+
+* Ability to call the tool by the name from callable alias with the same name without the infinite loop error.
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>
diff --git a/tests/test_integrations.py b/tests/test_integrations.py
index 9ade1fa802..65dbc1eef1 100644
--- a/tests/test_integrations.py
+++ b/tests/test_integrations.py
@@ -102,6 +102,40 @@ def _f():
         "hello\n",
         0,
     ),
+    # testing alias stack: lambda function
+    (
+        """
+# skip_if_on_windows
+def _echo():
+    echo hello
+
+aliases['echo'] = _echo
+echo
+""",
+        "hello\n",
+        0,
+    ),
+    # testing alias stack: ExecAlias
+    (
+        """
+# skip_if_on_windows        
+aliases['echo'] = "echo @('hello')"
+echo
+""",
+        "hello\n",
+        0,
+    ),
+    # testing alias stack: callable alias (ExecAlias) + no binary location + infinite loop
+    (
+        """
+# skip_if_on_windows        
+aliases['first'] = "second @(1)"
+aliases['second'] = "first @(1)"
+first
+""",
+        lambda out: 'Infinite loop of calls for "first" alias.' in out,
+        0,
+    ),
     # test redirecting a function alias to a file
     (
         """
@@ -477,8 +511,13 @@ def _echo(args):
 @pytest.mark.parametrize("case", ALL_PLATFORMS)
 def test_script(case):
     script, exp_out, exp_rtn = case
+    if ON_WINDOWS and 'skip_if_on_windows' in script:
+        return
     out, err, rtn = run_xonsh(script)
-    assert exp_out == out
+    if callable(exp_out):
+        assert exp_out(out)
+    else:
+        assert exp_out == out
     assert exp_rtn == rtn
 
 
diff --git a/xonsh/procs/proxies.py b/xonsh/procs/proxies.py
index 4e9b100b8c..316ed9270a 100644
--- a/xonsh/procs/proxies.py
+++ b/xonsh/procs/proxies.py
@@ -500,10 +500,16 @@ def run(self):
             sp_stderr = sys.stderr
         # run the function itself
         try:
+            alias_stack = builtins.__xonsh__.env.get("ALIAS_STACK", "")
+            if self.env["ALIAS_NAME"]:
+                alias_stack += ":" + self.env["ALIAS_NAME"]
+
             with STDOUT_DISPATCHER.register(sp_stdout), STDERR_DISPATCHER.register(
                 sp_stderr
             ), xt.redirect_stdout(STDOUT_DISPATCHER), xt.redirect_stderr(
                 STDERR_DISPATCHER
+            ), builtins.__xonsh__.env.swap(
+                ALIAS_STACK=alias_stack
             ):
                 r = self.f(self.args, sp_stdin, sp_stdout, sp_stderr, spec, spec.stack)
         except SystemExit as e:
diff --git a/xonsh/procs/specs.py b/xonsh/procs/specs.py
index 845c4e5297..5c82663531 100644
--- a/xonsh/procs/specs.py
+++ b/xonsh/procs/specs.py
@@ -355,6 +355,8 @@ def __init__(
         # pure attrs
         self.args = list(cmd)
         self.alias = None
+        self.alias_name = None
+        self.alias_stack = builtins.__xonsh__.env.get("ALIAS_STACK", "").split(":")
         self.binary_loc = None
         self.is_proxy = False
         self.background = False
@@ -442,6 +444,7 @@ def run(self, *, pipeline_group=None):
         kwargs = {n: getattr(self, n) for n in self.kwnames}
         self.prep_env(kwargs)
         if callable(self.alias):
+            kwargs["env"]["ALIAS_NAME"] = self.alias_name
             p = self.cls(self.alias, self.cmd, **kwargs)
         else:
             self.prep_preexec_fn(kwargs, pipeline_group=pipeline_group)
@@ -589,17 +592,29 @@ def redirect_trailing(self):
     def resolve_alias(self):
         """Sets alias in command, if applicable."""
         cmd0 = self.cmd[0]
+
+        if cmd0 in self.alias_stack:
+            # Disabling the alias resolving to prevent infinite loop in call stack
+            # and futher using binary_loc to resolve the alias name.
+            self.alias = None
+            return
+
         if callable(cmd0):
             alias = cmd0
         else:
             alias = builtins.aliases.get(cmd0, None)
+            if alias is not None:
+                self.alias_name = cmd0
         self.alias = alias
 
     def resolve_binary_loc(self):
         """Sets the binary location"""
         alias = self.alias
         if alias is None:
-            binary_loc = xenv.locate_binary(self.cmd[0])
+            cmd0 = self.cmd[0]
+            binary_loc = xenv.locate_binary(cmd0)
+            if binary_loc == cmd0 and cmd0 in self.alias_stack:
+                raise Exception(f'Infinite loop of calls for "{cmd0}" alias.')
         elif callable(alias):
             binary_loc = None
         else:


Code review comments:
diff_hunk: @@ -477,8 +511,13 @@ def _echo(args):
 @pytest.mark.parametrize("case", ALL_PLATFORMS)
 def test_script(case):
     script, exp_out, exp_rtn = case
+    if ON_WINDOWS and 'skip_if_on_windows' in script:
path: tests/test_integrations.py
text: @user1:
Use `pytest.param` with `skip_if_on_windows` instead

diff_hunk: @@ -500,10 +500,16 @@ def run(self):
             sp_stderr = sys.stderr
         # run the function itself
         try:
+            alias_stack = builtins.__xonsh__.env.get("ALIAS_STACK", "")
+            if self.env["ALIAS_NAME"]:
+                alias_stack += ":" + self.env["ALIAS_NAME"]
+
             with STDOUT_DISPATCHER.register(sp_stdout), STDERR_DISPATCHER.register(
                 sp_stderr
             ), xt.redirect_stdout(STDOUT_DISPATCHER), xt.redirect_stderr(
                 STDERR_DISPATCHER
+            ), builtins.__xonsh__.env.swap(
+                ALIAS_STACK=alias_stack
path: xonsh/procs/proxies.py
text: @user1:
Don't we need to add `ALIAS_NAME=""` here as well? 

@author:
Nope because we need only stack. If we will have an alias in further calls it will be just checked and added to stack.

diff_hunk: @@ -442,6 +444,7 @@ def run(self, *, pipeline_group=None):
         kwargs = {n: getattr(self, n) for n in self.kwnames}
         self.prep_env(kwargs)
         if callable(self.alias):
+            kwargs["env"]["ALIAS_NAME"] = self.alias_name
path: xonsh/procs/specs.py
text: @user1:
I'm fine with setting env vars for this, as long as we use names that are obviously internal like `__XONSH_ALIAS_NAME` and `__XONSH_ALIAS_STACK`

diff_hunk: @@ -589,17 +592,29 @@ def redirect_trailing(self):
     def resolve_alias(self):
         """Sets alias in command, if applicable."""
         cmd0 = self.cmd[0]
+
+        if cmd0 in self.alias_stack:
+            # Disabling the alias resolving to prevent infinite loop in call stack
+            # and futher using binary_loc to resolve the alias name.
+            self.alias = None
+            return
+
         if callable(cmd0):
             alias = cmd0
         else:
             alias = builtins.aliases.get(cmd0, None)
+            if alias is not None:
+                self.alias_name = cmd0
         self.alias = alias
 
     def resolve_binary_loc(self):
         """Sets the binary location"""
         alias = self.alias
         if alias is None:
-            binary_loc = xenv.locate_binary(self.cmd[0])
+            cmd0 = self.cmd[0]
+            binary_loc = xenv.locate_binary(cmd0)
+            if binary_loc == cmd0 and cmd0 in self.alias_stack:
+                raise Exception(f'Infinite loop of calls for "{cmd0}" alias.')
path: xonsh/procs/specs.py
text: @user1:
```suggestion
                raise Exception(f'Recursive calls to "{cmd0}" alias.')
```

We don't know it's infinite, only that it's recursive :)

diff_hunk: @@ -500,10 +500,16 @@ def run(self):
             sp_stderr = sys.stderr
         # run the function itself
         try:
+            alias_stack = builtins.__xonsh__.env.get("ALIAS_STACK", "")
+            if self.env["ALIAS_NAME"]:
path: xonsh/procs/proxies.py
text: @user1:
```suggestion
            if self.get("ALIAS_NAME"):
```