Skip to content

Conversation

@mshafer-NI
Copy link
Collaborator

Adding a "--aggressive" flag to acknowledge-existing-violations that will attempt to suppres, then format, then move suppressions back to the correct lines, and format, and repeat until formatting no longer changes the file (max of 10 tries).

  1. Extracted running the linter to _lint.lint
  2. Extracted config file consts from _cli.py to _config_constants.py
  3. Create _format.formt helper method for running black (decided to leave adding potential "format" command to future PR)
  4. Split out the per-file code from _acknowledge_existing_errors.acknowledge_lint_errors to _acknowledge_existing_errors._suppress_errors_in_file
  5. Add conditional handle_lines_that_are_now_too_long (this re-uses _suppress_errors_in_file for subsequent runs).
  6. Added test that uses "--agressive" flag and set output files to not be ignored by black (by changing the suffix so it didn't match the existing filter).

Example run:
(starting code)

def method_withBadName_andParams(my_normal_param, myBadlyNamedParam, my_other_Bad_param): 
    """Provide example where black will want to split out result."""
    return 5 + 7

This contains several errors, so round 1 will suppress those:

def method_withBadName_andParams(my_normal_param, myBadlyNamedParam, my_other_Bad_param):  # noqa N802: function name 'method_withBadName_andParams' should be lowercase (auto-generated noqa)  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
    """Provide example where black will want to split out result."""
    return 5 + 7

Now the line is too long, so when black is called, it will break up the line:

def method_withBadName_andParams(
    my_normal_param, myBadlyNamedParam, my_other_Bad_param
):  # noqa N802: function name 'method_withBadName_andParams' should be lowercase (auto-generated noqa)  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
    """Provide example where black will want to split out result."""
    return 5 + 7

However, now the suppressions are on the wrong line.
Because the file was reformatted - we attempt again.

We remove the suppressions and re-lint and suppress:

def method_withBadName_andParams(  # noqa N802: function name 'method_withBadName_andParams' should be lowercase (auto-generated noqa)
    my_normal_param, myBadlyNamedParam, my_other_Bad_param  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
): 
    """Provide example where black will want to split out result."""
    return 5 + 7

The args line is still too long, so black will again format

def method_withBadName_andParams(  # noqa N802: function name 'method_withBadName_andParams' should be lowercase (auto-generated noqa)
    my_normal_param, 
    myBadlyNamedParam, 
    my_other_Bad_param,  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
): 
    """Provide example where black will want to split out result."""
    return 5 + 7

Again, now the suppressions are on the wrong line, but we know it was reformatted, so we attempt again.

def method_withBadName_andParams(  # noqa N802: function name 'method_withBadName_andParams' should be lowercase (auto-generated noqa)
    my_normal_param, 
    myBadlyNamedParam,  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
    my_other_Bad_param,
): 
    """Provide example where black will want to split out result."""
    return 5 + 7

And this time, black is happy with the output, and so doesn't format and leaves the suppressions on the correct lines.

@mshafer-NI
Copy link
Collaborator Author

Note: I attempted to move the "--aggressive" logic up a level to the CLI (such that if --aggressive is passed, it would find all included .py files, call down to remove suppressions, call down to suppress, then do the loop.) however, this yielded a lot of difficulties. I will probably re-attempt when I next get a chance work on this (unless you disagree)

Copy link
Collaborator

@irwand irwand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code easier to understand IMO. Thx!

@mshafer-NI
Copy link
Collaborator Author

Note: I attempted to move the "--aggressive" logic up a level to the CLI (such that if --aggressive is passed, it would find all included .py files, call down to remove suppressions, call down to suppress, then do the loop.) however, this yielded a lot of difficulties. I will probably re-attempt when I next get a chance work on this (unless you disagree)

This is already a pretty big diff. I am opting to leave it as is and will come in later to add a "--remove-current-acknowledgements" arg

@mshafer-NI
Copy link
Collaborator Author

Blocked on #73

@mshafer-NI mshafer-NI merged commit 9c95e59 into ni:main Nov 15, 2021
@mshafer-NI mshafer-NI deleted the better_handle_when_noqa_will_cause_reformat branch November 15, 2021 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants