sscpac / statick

Making code quality easier.
https://sscpac.github.io/statick/
Creative Commons Zero v1.0 Universal
74 stars 14 forks source link

Add check for an empty issue file before trying to read the line for NOLINT. #467

Closed tdenewiler closed 1 year ago

tdenewiler commented 1 year ago

To see how this fixes an issue run the new unit tests against the main branch. Here are the steps I used.

git checkout exceptions-nolint-empty-log
git checkout main
git checkout -b x
git checkout exceptions-nolint-empty-log -- tests/exceptions/test_exceptions.py tests/exceptions/config/rsc/empty.log
python3 -m pytest tests/exceptions/test_exceptions.py

That should give you errors like the following.

collected 19 items                                                                                                                   

tests/exceptions/test_exceptions.py ................F..                                                                        [100%]

============================================================== FAILURES ==============================================================
________________________________________________ test_filter_issues_nolint_empty_log _________________________________________________

    def test_filter_issues_nolint_empty_log():
        """
        Test that NOLINT excpetions to issues do not fail with an empty issue log file.

        Expected result: same number of original issues in filtered issues
        """
        exceptions = Exceptions(
            os.path.join(os.path.dirname(__file__), "valid_exceptions.yaml")
        )

        filename = os.path.join(os.path.dirname(__file__), "config") + "/rsc" + "/empty.log"
        line_number = "0"
        tool = "dummy_tool"
        issue_type = "dummy_issue_type"
        severity = "0"
        message = "dummy_message"
        tool_issue = Issue(filename, line_number, tool, issue_type, severity, message, None)
        issues = {}
        issues["dummy_tool"] = [tool_issue]

>       filtered_issues = exceptions.filter_nolint(issues)

tests/exceptions/test_exceptions.py:362: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <statick_tool.exceptions.Exceptions object at 0x7fd5e9056df0>
issues = {'dummy_tool': [Issue(filename='/home/thomas/src/statick/statick/tests/exceptions/config/rsc/empty.log', line_number='0', tool='dummy_tool', issue_type='dummy_issue_type', severity='0', message='dummy_message', cert_reference=None)]}

    def filter_nolint(self, issues: Dict[str, List[Issue]]) -> Dict[str, List[Issue]]:
        """Filter out lines that have an explicit NOLINT on them.

        Sometimes the tools themselves don't properly filter these out if there is a
        complex macro or something.
        """
        for tool, tool_issues in list(issues.items()):
            warning_printed: bool = False
            to_remove: List[Issue] = []
            for issue in tool_issues:
                if not os.path.isabs(issue.filename):
                    if not warning_printed:
                        self.print_exception_warning(tool)
                        warning_printed = True
                    continue
                with open(issue.filename, encoding="utf-8") as fid:
                    lines = fid.readlines()
                line_number = int(issue.line_number) - 1
>               if line_number < len(lines) and "NOLINT" in lines[line_number]:
E               IndexError: list index out of range

statick_tool/exceptions.py:201: IndexError

At the end of this local test you can stash the changes and delete the temporary x branch.

codecov[bot] commented 1 year ago

Codecov Report

Merging #467 (a12db75) into main (7596ce9) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #467   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           58        58           
  Lines         3337      3339    +2     
=========================================
+ Hits          3337      3339    +2     
Impacted Files Coverage Δ
statick_tool/exceptions.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more