Repository: xonsh/xonsh
Pull request: Fix column offset of unary operator AST node
Fixes #3564 

Problem statement:
chaining other commands after `nix-store` causes "NameError: name 'nix' is not defined"
## xonfig

<details>

```
$ xonfig                                                                                
+------------------+-----------------+
| xonsh            | 0.9.17          |
| Python           | 3.7.7           |
| PLY              | 3.11            |
| have readline    | True            |
| prompt toolkit   | 3.0.4           |
| shell type       | prompt_toolkit2 |
| pygments         | 2.5.2           |
| on posix         | True            |
| on linux         | True            |
| distro           | unknown         |
| on darwin        | False           |
| on windows       | False           |
| on cygwin        | False           |
| on msys2         | False           |
| is superuser     | False           |
| default encoding | utf-8           |
| xonsh encoding   | utf-8           |
| encoding errors  | surrogateescape |
+------------------+-----------------+
```

</details>

## nix-info -m

<details>

 - system: `"x86_64-linux"`
 - host os: `Linux 5.4.35, NixOS, 20.09pre223023.fce7562cf46 (Nightingale)`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.3.4`
 - channels(root): `"nixos-20.09pre223023.fce7562cf46, nixos-unstable-20.09pre218613.ae6bdcc5358"`
 - channels(das-g): `""`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

</details>

## Expected Behavior
If `nix-store` is in `$PATH`, it can be used as part of longer command lines, too, with other commands chained after it.

## Current Behavior
<!--- Tell us what happens instead of the expected behavior -->
If the command line continues after `nix-store` and its arguments, xonsh won't run `nix-store` and instead reports `NameError: name 'nix' is not defined`.

<!--- 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 variable `XONSH_DEBUG=1` _before_ starting `xonsh`.
On Linux and OSX, an easy way to to do this is to run `env XONSH_DEBUG=1 xonsh` -->

### Traceback (if applicable)

<details>

```
xonsh: To log full traceback to a file set: $XONSH_TRACEBACK_LOGFILE = <filename>
Traceback (most recent call last):
  File "/nix/store/y22pwfk39i9r22ga10c3ifbp345gdl0p-xonsh-0.9.17/lib/python3.7/site-packages/xonsh/base_shell.py", line 362, in default
    run_compiled_code(code, self.ctx, None, "single")
  File "/nix/store/y22pwfk39i9r22ga10c3ifbp345gdl0p-xonsh-0.9.17/lib/python3.7/site-packages/xonsh/codecache.py", line 67, in run_compiled_code
    func(code, glb, loc)
  File "<xonsh-code>", line 1, in <module>
NameError: name 'nix' is not defined
```

</details>

## Steps to Reproduce
<!--- Please try to write out a minimal reproducible snippet to trigger the bug, it will help us fix it! -->

1. make sure `nix-store` is in `$PATH`
    ```xonsh
    nix-store --version
    ```
    You should get some output like
    ```
    nix-store (Nix) 2.3.4
    ```
2. call the following command line
    ```xonsh
    nix-store --version or true
    ```

Hints from issue discussion:
This also occurs with aliases that contain a dash (`-`) if you pass a long option and chain some other command with `and` after that:

```xonsh
$ aliases['foo-bar'] = 'true'
$ foo-bar
$ foo-bar --version
true (GNU coreutils) 8.31
[...]

$ foo-bar and true
$ foo-bar --version and true                                                            
xonsh: To log full traceback to a file set: $XONSH_TRACEBACK_LOGFILE = <filename>
Traceback (most recent call last):
  File "/nix/store/y22pwfk39i9r22ga10c3ifbp345gdl0p-xonsh-0.9.17/lib/python3.7/site-packages/xonsh/base_shell.py", line 362, in default
    run_compiled_code(code, self.ctx, None, "single")
  File "/nix/store/y22pwfk39i9r22ga10c3ifbp345gdl0p-xonsh-0.9.17/lib/python3.7/site-packages/xonsh/codecache.py", line 67, in run_compiled_code
    func(code, glb, loc)
  File "<xonsh-code>", line 1, in <module>
NameError: name 'foo' is not defined
```
Workaround: use `$[...]`:
```xonsh
$ aliases['foo-bar'] = 'true'
$ $[foo-bar --version] and true
true (GNU coreutils) 8.31
[...]
$ $[nix-store --version] and true
nix-store (Nix) 2.3.4
```

("Workaround", because I don't think `$[...]` should be necessary here.)
Wow! Thanks for reporting @das-g.
It insists for some reason on executing `foo-bar` in python mode. This is also interresting:
```
$ aliases["foo"]="true"
$ foo --abc and echo a
a
$ foo --abcd and echo a
NameError: name 'foo' is not defined
```
>  This is also interresting:
> 
> ```
> $ aliases["foo"]="true"
> $ foo --abc and echo a
> a
> $ foo --abcd and echo a
> NameError: name 'foo' is not defined
> ```

Oh, so it doesn't even require a dash in the executable to trigger this problem. :thinking:
Another symptom:
```
$ aliases["showargs"] = lambda args: print(args)
$ showargs -a and echo a
['-a']
a
$ showargs --a and echo a
['--a', 'and']
a
```
The ultimate cause of the issue seems to be, that unary operator ({'+', '-', '~'}) AST nodes have incorrect column offset.

For `showargs -a and echo a` the relevant part of the AST looks like:
```
BoolOp(op=And(), values=[
  BinOp(
    left=Name(id='showargs', ctx=Load(), lineno=1, col_offset=0), op=Sub(),
    right=Name(id='a', ctx=Load(), lineno=1, col_offset=10), lineno=1, col_offset=0)
```
The maximum col_offset here is the one of the "a" "Name" (that's what the parameter seems to be to the Python parser), which is indeed the position of the final token at the left side of the `and` operation.
For `showargs --a and echo a` the AST, however, is:
```
BoolOp(op=And(), values=[
  BinOp(
    left=Name(id='showargs', ctx=Load(), lineno=1, col_offset=0), op=Sub(),
    right=UnaryOp(op=USub(), operand=Name(id='a', ctx=Load(), lineno=1, col_offset=11), lineno=1, col_offset=13), lineno=1, col_offset=0),
```
The maximum col_offset here - the one of the unary `USub()` operator - is 13, which is way behind the whole parameter construct.

Commit message: Fix unary operator AST node column offset

This acually fixes a bug in subprocess mode. Logical subrocess
operators could not follow long subprocess arguments (e.g. --version).

Patch to review:
diff --git a/xonsh/parsers/base.py b/xonsh/parsers/base.py
index 8d4c48286e..266a393071 100644
--- a/xonsh/parsers/base.py
+++ b/xonsh/parsers/base.py
@@ -2153,12 +2153,13 @@ def p_factor_power(self, p):
         p[0] = p[1]
 
     def p_factor_unary(self, p):
-        """factor : PLUS factor
-                  | MINUS factor
-                  | TILDE factor
+        """factor : plus_tok factor
+                  | minus_tok factor
+                  | tilde_tok factor
         """
-        op = self._factor_ops[p[1]]()
-        p[0] = ast.UnaryOp(op=op, operand=p[2], lineno=self.lineno, col_offset=self.col)
+        p1 = p[1]
+        op = self._factor_ops[p1.value]()
+        p[0] = ast.UnaryOp(op=op, operand=p[2], lineno=self.lineno, col_offset=p1.lexpos)
 
     def p_power_atom(self, p):
         """power : atom_expr"""


Code review comments:
diff_hunk: @@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* Logical sobprocess operators now work after long arguments (e.g. --version).
path: news/fix-unary-position.rst
text: @user1:
```suggestion
* Logical subprocess operators now work after long arguments (e.g. `--version`).
```

@author:
Thanks! New system - no proofreader :wink: