dosisod / refurb

A tool for refurbishing and modernizing Python codebases
GNU General Public License v3.0
2.49k stars 54 forks source link

[Bug]: Ignore via amend doesn't work if multiple errors selected #337

Closed ppiasek closed 1 week ago

ppiasek commented 7 months ago

Has your issue already been fixed?

The Bug

I found the issue where only the first error is taken into consideration from the amend section.

In my case, I want to disable all checks for a single path, so I specified a list of all possible checks as ignore values for a single directory.

I checked the code (the same code is on master) and probably found the issue:

def is_ignored_via_amend(error: Error, settings: Settings) -> bool:
    assert error.filename

    path = Path(error.filename).resolve()
    error_code = ErrorCode.from_error(type(error))
    config_root = Path(settings.config_file).parent if settings.config_file else Path()

    for ignore in settings.ignore:
        if ignore.path:
            ignore_path = (config_root / ignore.path).resolve()

            if path.is_relative_to(ignore_path):
                if isinstance(ignore, ErrorCode):
                    return str(ignore) == str(error_code)

                return ignore.value in error.categories

    return False

The loop always ends on the first iteration if the path is relative to the ignore path, the error code is never checked there. There are not UTs for that function, probably that's why it was missed.

The solution below worked for me, so I'm creating MR for that. Please review.

def is_ignored_via_amend(error: Error, settings: Settings) -> bool:
    assert error.filename

    path = Path(error.filename).resolve()
    error_code = str(ErrorCode.from_error(type(error)))
    config_root = Path(settings.config_file).parent if settings.config_file else Path()

    errors_to_ignore = []
    categories_to_ignore = []
    for ignore in settings.ignore:
        if ignore.path:
            ignore_path = (config_root / ignore.path).resolve()

            if path.is_relative_to(ignore_path):
                if isinstance(ignore, ErrorCode):
                    errors_to_ignore.append(str(ignore))
                else:
                    categories_to_ignore.append(ignore.value)

    return error_code in errors_to_ignore or any(category in categories_to_ignore for category in error.categories)

Version Info

Refurb: v2.0.0
Mypy: v1.10.0

Python Version

3.11

Config File

[tool.refurb]
enable = ["FURB100", "FURB101", "FURB102", "FURB103", "FURB104", "FURB105", "FURB106", "FURB107", "FURB108", "FURB109", "FURB110", "FURB111", "FURB112", "FURB113", "FURB114", "FURB115", "FURB116", "FURB117", "FURB118", "FURB119", "FURB120", "FURB121", "FURB122", "FURB123", "FURB124", "FURB125", "FURB126", "FURB127", "FURB128", "FURB129", "FURB130", "FURB131", "FURB132", "FURB133", "FURB134", "FURB135", "FURB136", "FURB137", "FURB138", "FURB139", "FURB140", "FURB141", "FURB142", "FURB143", "FURB144", "FURB145", "FURB146", "FURB147", "FURB148", "FURB149", "FURB150", "FURB151", "FURB152", "FURB153", "FURB154", "FURB155", "FURB156", "FURB157", "FURB158", "FURB159", "FURB160", "FURB161", "FURB162", "FURB163", "FURB164", "FURB165", "FURB166", "FURB167", "FURB168", "FURB169", "FURB170", "FURB171", "FURB172", "FURB173", "FURB174", "FURB175", "FURB176", "FURB177", "FURB178", "FURB179", "FURB180", "FURB181", "FURB182", "FURB183", "FURB184", "FURB185", "FURB186", "FURB187", "FURB188", "FURB189", "FURB190", "FURB191", "FURB192"]
python_version = "3.11"

[[tool.refurb.amend]]
path = "old"
ignore = ["FURB100", "FURB101", "FURB102", "FURB103", "FURB104", "FURB105", "FURB106", "FURB107", "FURB108", "FURB109", "FURB110", "FURB111", "FURB112", "FURB113", "FURB114", "FURB115", "FURB116", "FURB117", "FURB118", "FURB119", "FURB120", "FURB121", "FURB122", "FURB123", "FURB124", "FURB125", "FURB126", "FURB127", "FURB128", "FURB129", "FURB130", "FURB131", "FURB132", "FURB133", "FURB134", "FURB135", "FURB136", "FURB137", "FURB138", "FURB139", "FURB140", "FURB141", "FURB142", "FURB143", "FURB144", "FURB145", "FURB146", "FURB147", "FURB148", "FURB149", "FURB150", "FURB151", "FURB152", "FURB153", "FURB154", "FURB155", "FURB156", "FURB157", "FURB158", "FURB159", "FURB160", "FURB161", "FURB162", "FURB163", "FURB164", "FURB165", "FURB166", "FURB167", "FURB168", "FURB169", "FURB170", "FURB171", "FURB172", "FURB173", "FURB174", "FURB175", "FURB176", "FURB177", "FURB178", "FURB179", "FURB180", "FURB181", "FURB182", "FURB183", "FURB184", "FURB185", "FURB186", "FURB187", "FURB188", "FURB189", "FURB190", "FURB191", "FURB192"]

Extra Info

None

ppiasek commented 5 months ago

@dosisod any chances to look at that issue?