[Bug]: Non-default settings file values are not respected

[maxrake]

Has your issue already been fixed?

• Have you checked to see if your issue still exists on the master branch? See the docs for instructions on how to setup a local build of Refurb.
• Have you looked at the open/closed issues to see if anyone has already reported your issue?

The Bug

Non-default values for format, python_version, or sort_by used in a settings file are not respected. Instead, they get overridden by the default values from parsing command line arguments. The documentation states:

...the values in the config file will be merged with the values specified via the command line.

...but even when there are no values specified for these specific options on the command line, the defaults are used anyway and stomp over the values provided in the config file. This can be seen by updating an existing test, test_command_line_args_merge_config_file(), to include the three affected options:

def test_command_line_args_merge_config_file() -> None:
    contents = """\
[tool.refurb]
load = ["some", "folders"]
ignore = [100, "FURB101"]
enable = ["FURB111", "FURB222"]
quiet = true
format = "github"
python_version = "3.10"
sort_by = "error"
"""

    command_line_args = parse_args(
        ["--load", "x", "--ignore", "123", "--enable", "FURB200"]
    )
    config_file = parse_config_file(contents)

    merged = Settings.merge(config_file, command_line_args)

    assert merged == Settings(
        load=["some", "folders", "x"],
        ignore={ErrorCode(100), ErrorCode(101), ErrorCode(123)},
        enable={ErrorCode(111), ErrorCode(222), ErrorCode(200)},
        quiet=True,
        format="github",
        python_version=(3, 10),
        sort_by="error"
    )

Emits the following error (when run with a Python 3.11 interpreter and with very verbose pytest output):

___________________________________________________________ test_command_line_args_merge_config_file ___________________________________________________________

    def test_command_line_args_merge_config_file() -> None:
        contents = """\
    [tool.refurb]
    load = ["some", "folders"]
    ignore = [100, "FURB101"]
    enable = ["FURB111", "FURB222"]
    quiet = true
    format = "github"
    python_version = "3.10"
    sort_by = "error"
    """

        command_line_args = parse_args(
            ["--load", "x", "--ignore", "123", "--enable", "FURB200"]
        )
        config_file = parse_config_file(contents)

        merged = Settings.merge(config_file, command_line_args)

>       assert merged == Settings(
            load=["some", "folders", "x"],
            ignore={ErrorCode(100), ErrorCode(101), ErrorCode(123)},
            enable={ErrorCode(111), ErrorCode(222), ErrorCode(200)},
            quiet=True,
            format="github",
            python_version=(3, 10),
            sort_by="error"
        )
E       AssertionError: assert Settings(files=[], explain=None, ignore={ErrorCode(id=100, prefix='FURB', path=None), ErrorCode(id=123, prefix='FURB', path=None), ErrorCode(id=101, prefix='FURB', path=None)}, load=['some', 'folders', 'x'], enable={ErrorCode(id=111, prefix='FURB', path=None), ErrorCode(id=222, prefix='FURB', path=None), ErrorCode(id=200, prefix='FURB', path=None)}, disable=set(), debug=False, generate=False, help=False, version=False, quiet=True, enable_all=False, disable_all=False, config_file=None, python_version=(3, 11), mypy_args=[], format='text', sort_by='filename') == Settings(files=[], explain=None, ignore={ErrorCode(id=100, prefix='FURB', path=None), ErrorCode(id=123, prefix='FURB', path=None), ErrorCode(id=101, prefix='FURB', path=None)}, load=['some', 'folders', 'x'], enable={ErrorCode(id=111, prefix='FURB', path=None), ErrorCode(id=222, prefix='FURB', path=None), ErrorCode(id=200, prefix='FURB', path=None)}, disable=set(), debug=False, generate=False, help=False, version=False, quiet=True, enable_all=False, disable_all=False, config_file=None, python_version=(3, 10), mypy_args=[], format='github', sort_by='error')
E
E         Matching attributes:
E         ['files',
E          'explain',
E          'ignore',
E          'load',
E          'enable',
E          'disable',
E          'debug',
E          'generate',
E          'help',
E          'version',
E          'quiet',
E          'enable_all',
E          'disable_all',
E          'config_file',
E          'mypy_args']
E         Differing attributes:
E         ['python_version', 'format', 'sort_by']
E
E         Drill down into differing attribute python_version:
E           python_version: (3, 11) != (3, 10)
E           At index 1 diff: 11 != 10
E           Full diff:
E           - (3, 10)
E           ?      ^
E           + (3, 11)
E           ?      ^
E
E         Drill down into differing attribute format:
E           format: 'text' != 'github'
E           - github
E           + text
E
E         Drill down into differing attribute sort_by:
E           sort_by: 'filename' != 'error'
E           - error
E           + filename

test/test_arg_parsing.py:198: AssertionError

But it should not be emitting an error instance because values that are provided in a config file and not on the command line should be used.

Version Info

Refurb: v1.15.0
Mypy: v1.2.0

Python Version

Python 3.11.3

Config File

# The bug manifests when any of these three lines are in the `pyproject.toml`
# file and also NOT provided as options on the command line:

[tool.refurb]
format = "github"
python_version = "3.10"  # Assuming the python version used to run `refurb` is NOT 3.10
sort_by = "error"

Extra Info

Just curious...but why is argument parsing done manually instead of with a library, like argparse?

[dosisod]

Thank you for reporting this!

To answer your question about argparse: I've never really used argparse before, and so rolling my own argument parsers is just easier for me, especially for smaller applications. Refurb used to be small, but has grown a lot since I started it, so it might be worth while to switch to something like argparse.

[maxrake]

Thank you for the quick fix!