ronantakizawa/codereview-bench huggingface dataset:
Repository: nvbn/thefuck
File: thefuck/rules/php_s.py (Python)

Code before:
from thefuck.utils import replace_argument


def match(command):
    return "php -s" in command.script


def get_new_command(command):
    return replace_argument(command.script, "-s", "-S")


Review comment (suggestion):
`-s` (lowercase) is a valid command and will timeout when ran by TheFuck. Meaning output will be empty. The rule must tell it doesn't require output. So, add this to the end of the rule:

```python
requires_output = False
```

Code after:
from thefuck.utils import replace_argument, for_app


@for_app('php')
def match(command):
    return "php -s" in command.script


def get_new_command(command):
    return replace_argument(command.script, "-s", "-S")


requires_output = False
Repository: nvbn/thefuck
File: thefuck/rules/git_push_different_branch_names.py (Python)

Code before:
import re
from thefuck.utils import replace_argument
from thefuck.specific.git import git_support


@git_support
def match(command):
    return "git push" in command.script and "The upstream branch of your current branch does not match" in command.output


@git_support
def _parse_git_output(output):
    regex = re.compile(r'^ +(git push [^\s]+ [^\s]+)', re.MULTILINE)
    return regex.findall(output)[0]


@git_support
def get_new_command(command):
    return replace_argument(command.script, "delete", "remove")


Review comment (suggestion):
These 2 lines can be replaced by one:

```python
return re.findall(r'^ +(git push [^\s]+ [^\s]+)', command.output, re.MULTILINE)
```

And maybe move it inside `get_new_command`, while **fixing** the latter too.

Code after:
import re
from thefuck.specific.git import git_support


@git_support
def match(command):
    return "push" in command.script and "The upstream branch of your current branch does not match" in command.output


@git_support
def get_new_command(command):
    return re.findall(r'^ +(git push [^\s]+ [^\s]+)', command.output, re.MULTILINE)[0]
Repository: nvbn/thefuck
File: thefuck/rules/git_push.py (Python)

Code before:
import re
from thefuck.utils import replace_argument
from thefuck.specific.git import git_support


@git_support
def match(command):
    return ('push' in command.script_parts
            and 'set-upstream' in command.output)


def _get_upstream_option_index(command_parts):
    if '--set-upstream' in command_parts:
        return command_parts.index('--set-upstream')
    elif '-u' in command_parts:
        return command_parts.index('-u')
    else:
        return None


@git_support
def get_new_command(command):
    # If --set-upstream or -u are passed, remove it and its argument. This is
    # because the remaining arguments are concatenated onto the command suggested
    # by git, which includes --set-upstream and its argument
    command_parts = command.script_parts[:]
    upstream_option_index = _get_upstream_option_index(command_parts)

    if upstream_option_index is not None:
        command_parts.pop(upstream_option_index)

        # In case of `git push -u` we don't have next argument:
        if len(command_parts) > upstream_option_index:
            command_parts.pop(upstream_option_index)

Review comment (suggestion):
<strike>
I'm thinking we should just make this line be:

```python
            and '--set-upstream' in command.script_parts)
```

in order to allow `--set-upstream` to occur elsewhere than right after `push`. Unless someone names a branch `--set-upstream`, this should be fine.
</strike>

EDIT: Nevermind, I wasn't paying attention to the fact that the *output* was being tested.

Code after:
import re
from thefuck.utils import replace_argument
from thefuck.specific.git import git_support


@git_support
def match(command):
    return ('push' in command.script_parts
            and 'git push --set-upstream' in command.output)


def _get_upstream_option_index(command_parts):
    if '--set-upstream' in command_parts:
        return command_parts.index('--set-upstream')
    elif '-u' in command_parts:
        return command_parts.index('-u')
    else:
        return None


@git_support
def get_new_command(command):
    # If --set-upstream or -u are passed, remove it and its argument. This is
    # because the remaining arguments are concatenated onto the command suggested
    # by git, which includes --set-upstream and its argument
    command_parts = command.script_parts[:]
    upstream_option_index = _get_upstream_option_index(command_parts)

    if upstream_option_index is not None:
        command_parts.pop(upstream_option_index)

        # In case of `git push -u` we don't have next argument:
        if len(command_parts) > upstream_option_index:
            command_parts.pop(upstream_option_index)
Repository: nvbn/thefuck
File: tests/rules/test_cat_dir.py (Python)

Code before:
import pytest
from thefuck.rules.cat_dir import match, get_new_command
from thefuck.types import Command


@pytest.mark.parametrize('command', [
    Command('cat foo', 'cat: foo: Is a directory'),
    Command('cat /foo/bar/', 'cat: /foo/bar/: Is a directory'),
])
def test_match(command):
    assert match(command)


@pytest.mark.parametrize('command', [
    Command('cat foo', 'foo bar baz'),
    Command('cat foo bar', 'foo bar baz'),
    Command('', ''),
])
def test_not_match(command):
    assert not match(command)


@pytest.mark.parametrize('command, new_command', [
    (Command('cat foo', 'cat: foo: Is a directory'), 'ls foo'),
    (Command('cat /foo/bar/', 'cat: /foo/bar/: Is a directory'), 'ls /foo/bar/'),
])
def test_get_new_command(command, new_command):
    assert get_new_command(command) == new_command


Review comment (suggestion):
The actual output contains a LINEFEED (`\n`) at the end, so should the test output: `'cat: foo: Is a directory\n'` and `'cat: /foo/bar/: Is a directory\n'`

Code after:
import pytest
from thefuck.rules.cat_dir import match, get_new_command
from thefuck.types import Command


@pytest.mark.parametrize('command', [
    Command('cat foo', 'cat: foo: Is a directory\n'),
    Command('cat /foo/bar/', 'cat: /foo/bar/: Is a directory\n'),
    Command('cat cat/', 'cat: cat/: Is a directory\n'),
])
def test_match(command):
    assert match(command)


@pytest.mark.parametrize('command', [
    Command('cat foo', 'foo bar baz'),
    Command('cat foo bar', 'foo bar baz'),
])
def test_not_match(command):
    assert not match(command)


@pytest.mark.parametrize('command, new_command', [
    (Command('cat foo', 'cat: foo: Is a directory\n'), 'ls foo'),
    (Command('cat /foo/bar/', 'cat: /foo/bar/: Is a directory\n'), 'ls /foo/bar/'),
    (Command('cat cat', 'cat: cat: Is a directory\n'), 'ls cat'),
])
def test_get_new_command(command, new_command):
    assert get_new_command(command) == new_command
Repository: nvbn/thefuck
File: thefuck/rules/cat_dir.py (Python)

Code before:
import re


def match(command):
    return (
        command.script.startswith('cat') and
        re.match(r'cat: .+: Is a directory', command.output)
    )


def get_new_command(command):
    return re.sub(r'^cat', 'ls', command.script)


Review comment (suggestion):
`re.match` could be replaced with `str.startswith`, `str.rstrip` and `str.endswith` used in conjunction. `str.rstrip` is needed here because `command.output` ends with `\n` (see previous line comment)

Code after:
def match(command):
    return (
        command.script.startswith('cat') and
        command.output.startswith('cat: ') and
        command.output.rstrip().endswith(': Is a directory')
    )


def get_new_command(command):
    return command.script.replace('cat', 'ls', 1)
Repository: nvbn/thefuck
File: thefuck/rules/cat_dir.py (Python)

Code before:
import re


def match(command):
    return (
        command.script.startswith('cat') and
        re.match(r'cat: .+: Is a directory', command.output)
    )


def get_new_command(command):
    return re.sub(r'^cat', 'ls', command.script)


Review comment (suggestion):
This can be easily replaced with [`str.replace`](https://docs.python.org/3/library/stdtypes.html?highlight=replace#str.replace). Just make sure that only the first occurrence of `cat` is replaced.

Code after:
def match(command):
    return (
        command.script.startswith('cat') and
        command.output.startswith('cat: ') and
        command.output.rstrip().endswith(': Is a directory')
    )


def get_new_command(command):
    return command.script.replace('cat', 'ls', 1)
Repository: nvbn/thefuck
File: thefuck/rules/docker_image_being_used_by_container.py (Python)

Code before:
from thefuck.utils import for_app
from thefuck.shells import shell


@for_app('docker')
def match(command):
    '''
    Matches a command's output with docker's output
    warning you that you need to remove a container before removing an image.
    '''
    return 'image is being used by running container' in command.output


def get_new_command(command):
    '''
    Prepends docker container rm -f {container ID} to
    the previous docker image rm {image ID} command
    '''
    container_id = command.output.strip().split(' ')
    return shell.and_('docker container rm -f {} && {}').format(container_id[-1], command.script)


Review comment (suggestion):
The thing is, some shells – such as Fish<sup>1</sup> – do not support the `&&` operator. They use things like `; and` instead, for example. That's why each of the shells in TheFuck implements its own `AND` operator (i.e. `shell.and_()`. It takes a sequence of commands and joins them by the supported operator:

```python
oneliner = shell.and_('cmd1', 'cmd2', 'cmd3')
```

So, please change it like the following:

```suggestion
    return shell.and_('docker container rm -f {}', '{}').format(container_id[-1], command.script)
```  
 
> * <sup>1</sup> Recent versions of Fish support `&&` but we must keep backward compatibility

Code after:
from thefuck.utils import for_app
from thefuck.shells import shell


@for_app('docker')
def match(command):
    '''
    Matches a command's output with docker's output
    warning you that you need to remove a container before removing an image.
    '''
    return 'image is being used by running container' in command.output


def get_new_command(command):
    '''
    Prepends docker container rm -f {container ID} to
    the previous docker image rm {image ID} command
    '''
    container_id = command.output.strip().split(' ')
    return shell.and_('docker container rm -f {}', '{}').format(container_id[-1], command.script)
Repository: nvbn/thefuck
File: thefuck/rules/go_unknown_command.py (Python)

Code before:
from thefuck.utils import get_closest, replace_argument

_GOLANG_COMMANDS = (
    "bug", "build", "clean", "doc", "env", "fix", "fmt", "generate", "get",
    "install", "list", "mod", "run", "test", "tool", "version", "vet")


def match(command):
    return (command.script_parts
            and command.script_parts[0] == "go"
            and 'unknown command' in command.output)


def get_new_command(command):
    closest_subcommand = get_closest(command.script_parts[1],
                                     _GOLANG_COMMANDS)
    return replace_argument(command.script, command.script_parts[1],
                            closest_subcommand)


Review comment (suggestion):
It will be better to replace that check with `for_app` decorator, it helps with things like `sudo`:
```python
from thefuck.utils import for_app

@for_app('go')
def match(command):
    ...
```

Code after:
from itertools import dropwhile, islice, takewhile
import subprocess

from thefuck.utils import get_closest, replace_argument, for_app, which, cache


def get_golang_commands():
    proc = subprocess.Popen('go', stderr=subprocess.PIPE)
    lines = [line.decode('utf-8').strip() for line in proc.stderr.readlines()]
    lines = dropwhile(lambda line: line != 'The commands are:', lines)
    lines = islice(lines, 2, None)
    lines = takewhile(lambda line: line, lines)
    return [line.split(' ')[0] for line in lines]


if which('go'):
    get_docker_commands = cache(which('go'))(get_golang_commands)


@for_app('go')
def match(command):
    return 'unknown command' in command.output


def get_new_command(command):
    closest_subcommand = get_closest(command.script_parts[1], get_golang_commands())
    return replace_argument(command.script, command.script_parts[1],
                            closest_subcommand)
Repository: nvbn/thefuck
File: thefuck/rules/docker_not_command.py (Python)

Code before:


def get_docker_commands():
    proc = subprocess.Popen('docker', stdout=subprocess.PIPE, stderr=subprocess.PIPE)

    # Old version docker returns its output to stdout, while newer version returns to stderr.
    lines = proc.stdout.readlines() or proc.stderr.readlines()
    lines = [line.decode('utf-8') for line in lines]

    # Only newer versions of docker have management commands in the help text.
    if 'Management Commands:\n' in lines:
        management_commands = _parse_commands(lines, 'Management Commands:\n')
    else:
        management_commands = []
    regular_commands = _parse_commands(lines, 'Commands:\n')
    return management_commands + regular_commands


if which('docker'):
    get_docker_commands = cache(which('docker'))(get_docker_commands)


@sudo_support
def get_new_command(command):
    if 'Usage:' in command.output and len(command.script_parts) > 1:
        management_subcommands = _parse_commands(map(lambda x: x + '\n', command.output.split('\n')), 'Commands:\n')
        return replace_command(command, command.script_parts[2], management_subcommands)

    wrong_command = re.findall(
        r"docker: '(\w+)' is not a docker command.", command.output)[0]
    return replace_command(command, wrong_command, get_docker_commands())


Review comment (suggestion):
I think it might be better to replace `map(lambda x: x + '\n', command.output.split('\n'))` with just `command.output.split('\n')` and replace `lambda line: line != '\n'` in :17 with `lambda line: line.strip()`, so it will handle both `'\n'` and `''`.

Code after:


def get_docker_commands():
    proc = subprocess.Popen('docker', stdout=subprocess.PIPE, stderr=subprocess.PIPE)

    # Old version docker returns its output to stdout, while newer version returns to stderr.
    lines = proc.stdout.readlines() or proc.stderr.readlines()
    lines = [line.decode('utf-8') for line in lines]

    # Only newer versions of docker have management commands in the help text.
    if 'Management Commands:\n' in lines:
        management_commands = _parse_commands(lines, 'Management Commands:')
    else:
        management_commands = []
    regular_commands = _parse_commands(lines, 'Commands:')
    return management_commands + regular_commands


if which('docker'):
    get_docker_commands = cache(which('docker'))(get_docker_commands)


@sudo_support
def get_new_command(command):
    if 'Usage:' in command.output and len(command.script_parts) > 1:
        management_subcommands = _parse_commands(command.output.split('\n'), 'Commands:')
        return replace_command(command, command.script_parts[2], management_subcommands)

    wrong_command = re.findall(
        r"docker: '(\w+)' is not a docker command.", command.output)[0]
    return replace_command(command, wrong_command, get_docker_commands())
Repository: nvbn/thefuck
File: thefuck/rules/pacman_invalid_option.py (Python)

Code before:
from thefuck.specific.archlinux import archlinux_env
import re


def match(command):
    return "error: invalid option '-s'" in command.output


def get_new_command(command):
    opt = re.findall(r" -[dqrstuf]", command.script)[0]
    return re.sub(opt, opt.upper(), command.script)


enabled_by_default = true, pacman = archlinux_env()


Review comment (suggestion):
`enabled_by_default` is being declared as a `tuple(boo, str)`. That way, it's always evaluated to `True`. Please change it as such:
```suggestion
enabled_by_default = archlinux_env()
```

Code after:
from thefuck.specific.archlinux import archlinux_env
import re


def match(command):
    return "error: invalid option '-s'" in command.output


def get_new_command(command):
    opt = re.findall(r" -[dqrstuf]", command.script)[0]
    return re.sub(opt, opt.upper(), command.script)


enabled_by_default = archlinux_env()
Repository: nvbn/thefuck
File: thefuck/rules/wrong_hyphen_before_subcommand.py (Python)

Code before:
        return True

    if find_subcommand(manpage, cmd, subcmd):
        return True

    flags = get_flags(manpage)
    for flag in subcmd:
        if not find_flag(flags, flag):
            return False

    return True


@sudo_support
def get_new_command(command):
    cmd, subcmd = command.script_parts[0].split('-')[:2]
    manpage = get_manpage(cmd)
    synopsis = get_synopsis(manpage)

    if subcmd in synopsis or find_subcommand(manpage, cmd, subcmd):
        # We are dealing with an accidentally added hyphen
        return command.script.replace('-', ' ', 1)

    else:
        # We are dealing with a missing space
        return command.script.replace('-', ' -', 1)


priority = 2500


Review comment (bug):
Perhaps a simpler version of the entire rule would be better.

IMO, TheFuck should delegate that choice to the user.

Following the path the current rule is going, the failed command `apt-install foo` would get two suggestions, `apt -install foo` and `apt install foo`, among others.

Thing is that the former – `apt -install foo` – already gets suggested by the existing `missing_space_before_subcommand ` rule.

Looking at it that way, all the `wrong_hyphen_before_subcommand` – or even an *augmented* `missing_space_before_subcommand` – rule would have to do is replace the hyphen with a space.

What you think?

Code after:
from thefuck.utils import get_all_executables
from thefuck.specific.sudo import sudo_support


@sudo_support
def match(command):
    first_part = command.script_parts[0]
    if "-" not in first_part or first_part in get_all_executables():
        return False
    cmd, _ = first_part.split("-", 1)
    return cmd in get_all_executables()


@sudo_support
def get_new_command(command):
    return command.script.replace("-", " ", 1)


priority = 4500
requires_output = False
Repository: nvbn/thefuck
File: thefuck/rules/wrong_hyphen_before_subcommand.py (Python)

Code before:
from thefuck.utils import get_all_executables
from thefuck.specific.sudo import sudo_support


@sudo_support
def match(command):
    if '-' not in command.script_parts[0] or command.script_parts[0] in get_all_executables():
        return False

    cmd, subcmd = command.script_parts[0].split('-')[:2]
    if cmd not in get_all_executables():
        return False

    return True


@sudo_support
def get_new_command(command):
    return command.script.replace("-", " ", 1)


priority = 2900


Review comment (suggestion):
Now that `subcmd` isn't necessary anymore, this line could be changed to:
```suggestion
    cmd = command.script_parts[0].split('-', 1)[0]
```

Code after:
from thefuck.utils import get_all_executables
from thefuck.specific.sudo import sudo_support


@sudo_support
def match(command):
    first_part = command.script_parts[0]
    if "-" not in first_part or first_part in get_all_executables():
        return False
    cmd, _ = first_part.split("-", 1)
    return cmd in get_all_executables()


@sudo_support
def get_new_command(command):
    return command.script.replace("-", " ", 1)


priority = 4500
requires_output = False
Repository: nvbn/thefuck
File: thefuck/rules/wrong_hyphen_before_subcommand.py (Python)

Code before:
from thefuck.utils import get_all_executables
from thefuck.specific.sudo import sudo_support


@sudo_support
def match(command):
    if '-' not in command.script_parts[0] or command.script_parts[0] in get_all_executables():
        return False

    cmd, subcmd = command.script_parts[0].split('-')[:2]
    if cmd not in get_all_executables():
        return False

    return True


@sudo_support
def get_new_command(command):
    return command.script.replace("-", " ", 1)


priority = 2900


Review comment (suggestion):
This could simply be :

```python
return cmd in get_all_executables()
```

Code after:
from thefuck.utils import get_all_executables
from thefuck.specific.sudo import sudo_support


@sudo_support
def match(command):
    first_part = command.script_parts[0]
    if "-" not in first_part or first_part in get_all_executables():
        return False
    cmd, _ = first_part.split("-", 1)
    return cmd in get_all_executables()


@sudo_support
def get_new_command(command):
    return command.script.replace("-", " ", 1)


priority = 4500
requires_output = False
Repository: nvbn/thefuck
File: thefuck/rules/switch_lang.py (Python)

Code before:
def _decompose_korean(command):
    def _change_double(ch):
        if ch in DOUBLE_LIST:
            return DOUBLE_MOD_LIST[DOUBLE_LIST.index(ch)]
        else:
            return ch

    hg_str = u''
    for ch in command.script:
        if u'가' <= ch <= u'힣':
            ord_ch = ord(ch) - ord(u'가')
            hd = ord_ch // 588
            bd = (ord_ch - 588 * hd) // 28
            tl = ord_ch - 588 * hd - 28 * bd
            for ch in [HEAD_LIST[hd], BODY_LIST[bd], TAIL_LIST[tl]]:
                if ch != ' ':
                    hg_str += _change_double(ch)
        else:
            hg_str += _change_double(ch)
    return hg_str


def match(command):
    if 'not found' not in command.output:
        return False
    if any([u'ㄱ' <= ch <= u'ㅎ' or u'ㅏ' <= ch <= u'ㅣ' or u'가' <= ch <= u'힣'
            for ch in command.script]):
        return True

    matched_layout = _get_matched_layout(command)
    return (matched_layout and
            _switch_command(command, matched_layout) != get_alias())


def get_new_command(command):
    if any([u'ㄱ' <= ch <= u'ㅎ' or u'ㅏ' <= ch <= u'ㅣ' or u'가' <= ch <= u'힣'
            for ch in command.script]):
        command.script = _decompose_korean(command)
    matched_layout = _get_matched_layout(command)
    return _switch_command(command, matched_layout)


Review comment (suggestion):
If you remove the square brackets here, you'll be passing a generator to `any` instead of a list. That way, the iteration stops at the very first element of `command.script` that meets the conditional expression, not having to iterate over all of them for every case. Generators FTW! ✨

Code after:
    def _change_double(ch):
        if ch in DOUBLE_LIST:
            return DOUBLE_MOD_LIST[DOUBLE_LIST.index(ch)]
        return ch

    hg_str = u''
    for ch in command.script:
        if u'가' <= ch <= u'힣':
            ord_ch = ord(ch) - ord(u'가')
            hd = ord_ch // 588
            bd = (ord_ch - 588 * hd) // 28
            tl = ord_ch - 588 * hd - 28 * bd
            for ch in [HEAD_LIST[hd], BODY_LIST[bd], TAIL_LIST[tl]]:
                if ch != ' ':
                    hg_str += _change_double(ch)
        else:
            hg_str += _change_double(ch)
    return hg_str


def match(command):
    if 'not found' not in command.output:
        return False
    if any(u'ㄱ' <= ch <= u'ㅎ' or u'ㅏ' <= ch <= u'ㅣ' or u'가' <= ch <= u'힣'
            for ch in command.script):
        return True

    matched_layout = _get_matched_layout(command)
    return (matched_layout and
            _switch_command(command, matched_layout) != get_alias())


def get_new_command(command):
    if any(u'ㄱ' <= ch <= u'ㅎ' or u'ㅏ' <= ch <= u'ㅣ' or u'가' <= ch <= u'힣'
            for ch in command.script):
        command.script = _decompose_korean(command)
    matched_layout = _get_matched_layout(command)
    return _switch_command(command, matched_layout)