Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
3 tasks done
ppiasek opened this issue Apr 30, 2024 · 0 comments · May be fixed by #338
Open
3 tasks done

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

ppiasek opened this issue Apr 30, 2024 · 0 comments · May be fixed by #338
Assignees
@dosisod
Labels
bug Something isn't working

Comments

@ppiasek
Copy link

ppiasek commented Apr 30, 2024 •
edited

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?
  • If reporting a false positive/incorrect suggestion, have you double checked that the suggested fix changes the code semantics?

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 ppiasek added the bug Something isn't working label Apr 30, 2024
@ppiasek ppiasek linked a pull request Apr 30, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dosisod dosisod

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix is_ignored_via_amend and add UTs ppiasek/refurb
2 participants
@dosisod @ppiasek