Skip to content

Commit

Permalink
Fix the remaining pylint warnings for usable pre-commit hook
Browse files Browse the repository at this point in the history
Signed-off-by: Bernhard Kaindl <[email protected]>
  • Loading branch information
bernhardkaindl committed Jan 17, 2024
1 parent 069e1cc commit b510309
Show file tree
Hide file tree
Showing 14 changed files with 36 additions and 38 deletions.
3 changes: 1 addition & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ repos:
args:
[
-sn, # Don't display the score
--rcfile=pylintrc,
--load-plugins=pylint.extensions.eq_without_hash,
--ignore-imports=yes,
"--disable=duplicate-code,line-too-long",
Expand All @@ -94,7 +93,7 @@ repos:
hooks:
- id: run-pyre
name: run-pyre
entry: python run-pyre.py
entry: python pyre_runner.py
types: [python]
language: python
log_file: ".git/pre-commit-pyre.log"
Expand Down
5 changes: 3 additions & 2 deletions pylintrc → .pylintrc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# Python code to execute, usually for sys.path manipulation such as
# pygtk.require().
# For absolute sys.path, use this for getting our stubs path dynamically:
init-hook="from pylint.config import find_pylintrc; import os, sys; sys.path.append(os.path.dirname(find_pylintrc()) + '/stubs')"
init-hook="import os,sys;sys.path.append(os.path.dirname('stubs'))"

# Profiled execution.
profile=no
Expand Down Expand Up @@ -50,7 +50,7 @@ load-plugins=
#
disable=W0142,W0703,C0111,R0201,W0603,W0613,W0212,W0141,
bad-option-value, # Skip complaining about suppressions for older option values
unrecognized-option, # Skip complaining on pylintrc options only in pylint2/pylint3
unrecognized-option, # Skip complaining on .pylintrc options only in pylint2/pylint3
unknown-option-value, # Skip complaining about checkers only in pylint2/pylint3
useless-object-inheritance, # "object" is not obsolete for supporting Python2
super-with-arguments, # super() with arguments is a Python3-only feature
Expand All @@ -61,6 +61,7 @@ disable=W0142,W0703,C0111,R0201,W0603,W0613,W0212,W0141,
consider-using-dict-items,
consider-using-enumerate,
consider-using-ternary,
duplicate-code,
len-as-condition,
no-else-return,
raise-missing-from,
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ You may want to install `pytype` in your user environment to run it directly wit
# On Python != 3.8, pytype can't import xml.dom.minidom, use 3.8:
python3.8 -m pip install pytype
python -m pip install tabulate
./run-pytype.py
./pytype_runner.py
```

## Installation of dependencies using `pip`
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ $(MY_OUTPUT_DIR)/python-libs.inc: $(MY_OUTPUT_DIR)/.dirstamp Makefile

.PHONY: pylint
pylint:
run-pylint.sh $(PYTHON_LIBS_SOURCES)
pylint_runner.sh $(PYTHON_LIBS_SOURCES)

.PHONY: sources
sources: $(RPM_SRPMSDIR)/$(PYTHON_LIB_SRPM)
Expand Down
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ The Continuous Integration Tests feature:
- Automatic Upload of the combined coverage to CodeCov (from the GitHub Workflow)
- Checking of the combined coverage against the diff to master: Fails if changes are not covered!
- Pylint report in the GitHub Action Summary page, with Warning and Error annotatios, even in the code review.
- Check that changes don't generate pylint warnings (if warning classes which are enabled in pylintrc)
- Check that changes don't generate pylint warnings (if warning classes which are enabled in .pylintrc)
- Static analysis using `mypy`, `pyre` and `pytype`

This enforces that any change (besides whitespace):

- has code coverage and
- does not introduce a `pylint` warning which is not disabled in `pylintrc`
- does not introduce a `pylint` warning which is not disabled in `.pylintrc`
- does not introduce a type of static analysis warning which is currently suppressed.

## Status Summary
Expand All @@ -54,7 +54,7 @@ open a recent workflow run the latest and scroll down until you see the tables!
- `pytest.ini`: The defaults used by `pytest` unless overruled by command line options
- `.github/workflows/main.yml`: Configuration of the GitHub CI matrix jobs and coverage upload
- `.github/act-serial.yaml`: Configuration for the jobs run by the local GitHub actions runner `act`
- `pylintrc`: Configuration file of `Pylint`
- `.pylintrc`: Configuration file of `Pylint`

## Installation and Setup of a development environment

Expand All @@ -71,8 +71,8 @@ For the installation of the general development dependencies, visit [INSTALL.md]
issues by Python3.6
- Test with `python2.7 -m pytest`
- Run `mypy` (without any arguments - The configuration is in `pyproject.toml`)
- Run `./run-pytype.py`
- Run `./run-pyre.py`
- Run `./pytype_runner.py`
- Run `./pyre_runner.py`
- Run `tox -e py36-lint` and fix any `Pylint` warnings
- Run `tox -e py310-covcombine-check` and fix any missing diff-coverage.
- Run `tox` for the full CI test suite
Expand Down
17 changes: 7 additions & 10 deletions run-pylint.py → pylint_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,11 @@
from io import StringIO
from typing import List, TextIO

import pandas as pd # type: ignore[import]
from pylint.lint import Run # type: ignore[import]
from pylint.reporters import JSONReporter # type: ignore[import]
from toml import load

import pandas as pd # type: ignore[import]


def del_dict_keys(r, *args):
for arg in args:
Expand Down Expand Up @@ -108,11 +107,12 @@ def cleanup_results_dict(r, sym):
# https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/eq-without-hash.html
#
pylint_options: List[str] = [
"--load-plugins", "pylint.extensions.eq_without_hash",
"--load-plugins",
"pylint.extensions.eq_without_hash",
]

def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):

def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):
pylint_overview = []
pylint_results = []
pylint_paths = []
Expand All @@ -125,7 +125,7 @@ def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):
smells_total = 0
for path in pylint_paths:
filename = path.rsplit("/", maxsplit=1)[-1]
if filename in ["__init__.py", "pylintrc"]:
if filename in ["__init__.py", "..pylintrc"]:
continue
reporter_buffer = StringIO()
results = Run(
Expand Down Expand Up @@ -169,12 +169,9 @@ def pylint_project(check_dirs: List[str], errorlog: TextIO, branch_url: str):

message_ids[sym] = "" # Populate a dict of the pylint message symbols seen in this file
# https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-notice-message
endline = r['endLine']
endline = r["endLine"]
end = f",endLine={endline}" if endline and endline != lineno else ""
print(
f"::{cls} file={path},line={lineno}{end},"
f"title=pylint {msg_id}: {sym}::{msg}"
)
print(f"::{cls} file={path},line={lineno}{end}," f"title=pylint {msg_id}: {sym}::{msg}")
r["path"] = f"[{linktext}]({branch_url}/{path}#L{lineno})"
cleanup_results_dict(r, sym)
filtered_file_results.append(r)
Expand Down
File renamed without changes.
12 changes: 5 additions & 7 deletions run-pytype.py → pytype_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,12 @@ def run_pytype(command: List[str], branch_url: str, errorlog: TextIO, results):
error += filter_line(line, row)
continue
errorlog.write(
error
+ " (you should find an entry in the pytype results with links below)\n"
error + " (you should find an entry in the pytype results with links below)\n"
)
results.append(row)
row = {}
error = ""
match = re.match(
r'File ".*libs/([^"]+)", line (\S+), in ([^:]+): (.*) \[(\S+)\]', line
)
match = re.match(r'File ".*libs/([^"]+)", line (\S+), in ([^:]+): (.*) \[(\S+)\]', line)
if match:
error, row = generate_github_annotation(match, branch_url)
if popen.stdout:
Expand Down Expand Up @@ -137,6 +134,7 @@ def run_pytype_and_parse_annotations(xfail_files: List[str], branch_url: str):
print("No errors in", xfail_file)
return err_code or len(results), results


def to_markdown(me, fp, returncode, results, branch_url):
mylink = f"[{me}]({branch_url}/{me}.py)"
pytype_link = "[pytype](https://google.github.io/pytype)"
Expand All @@ -162,7 +160,7 @@ def setup_and_run_pytype_action(scriptname: str):
branch = os.environ.get("GITHUB_HEAD_REF", None) or os.environ.get("GITHUB_REF_NAME", None)
filelink_baseurl = f"{server_url}/{repository}/blob/{branch}"
retcode, results = run_pytype_and_parse_annotations(xfail_files, filelink_baseurl)
# Write the panda dable to a markdown output file:
# Write the panda dable to a markdown output file:
summary_file = os.environ.get("GITHUB_STEP_SUMMARY", None)
if summary_file:
with open(summary_file, "w", encoding="utf-8") as fp:
Expand All @@ -174,4 +172,4 @@ def setup_and_run_pytype_action(scriptname: str):
if __name__ == "__main__":
script_basename = os.path.basename(__file__).split(".")[0]
basicConfig(format=script_basename + ": %(message)s", level=INFO)
sys.exit(setup_and_run_pytype_action(scriptname = script_basename))
sys.exit(setup_and_run_pytype_action(scriptname=script_basename))
3 changes: 2 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
"""

# pyre-ignore-all-errors[21]
import pytest # pyre does not find the module when run by tox -e py311-pyre
import warnings

import pytest # pyre does not find the module when run by tox -e py311-pyre

@pytest.fixture(autouse=True)
def set_warnings():
"""
Expand Down
5 changes: 3 additions & 2 deletions tests/test_cpiofile.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ def test_cpiofile_modes(fs):
assert exc_info.type == TypeError
if sys.version_info < (3, 0):
# Test Python2 pattern from host-upgrade-plugin:/etc/xapi.d/plugins/prepare_host_upgrade.py
import StringIO
# Import the Python2-only StringIO.StringIO module, imported as Py2StringIO:
from StringIO import Py2StringIO # pylint: disable=import-outside-toplevel

stringio = StringIO.StringIO()
stringio = Py2StringIO()
archive = CpioFile.open(fileobj=stringio, mode="w|gz") # type: ignore[arg-type]
archive.hardlinks = False
archive.close()
Expand Down
6 changes: 3 additions & 3 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ deps = pylint
pandas
tabulate
commands =
python run-pylint.py xcp tests
python pylint_runner.py xcp tests
diff-quality --compare-branch=origin/master --violations=pylint \
--ignore-whitespace --fail-under 100 \
--html-report {envlogdir}/pylint-diff.html {envlogdir}/pylint.txt
Expand Down Expand Up @@ -202,7 +202,7 @@ max-line-length = 129
[pyre]
commands =
pyre: python3.11 --version -V # Needs py311-pyre, does not work with py310-pyre
python ./run-pyre.py
python pyre_runner.py
-pyright

[pytype]
Expand All @@ -212,4 +212,4 @@ commands =
python3.10 -V # Needs python <= 3.10, and 3.10 is needed to parse new "|" syntax
pytype --version
# Runs pytype -j auto -k --config .github/workflows/pytype.cfg and parses the output:
python3 ./run-pytype.py # When switching versions, update .github/workflows/pytype.cfg too!
python3 pytype_runner.py # When switching versions, update .github/workflows/pytype.cfg too!
6 changes: 3 additions & 3 deletions xcp/accessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def openAddress(self, address):
else:
self.lastError = mapError(e.errno)
return False
except Exception as e:
except Exception:
self.lastError = 500
return False
return filehandle
Expand Down Expand Up @@ -245,7 +245,7 @@ def openAddress(self, address):
else:
self.lastError = mapError(e.errno)
return False
except Exception as e:
except Exception:
self.lastError = 500
return False
return reader
Expand Down Expand Up @@ -332,7 +332,7 @@ def access(self, path): # pylint: disable=arguments-differ,arguments-renamed
else:
self.lastError = mapError(e.errno)
return False
except Exception as e:
except Exception:
self.lastError = 500
return False

Expand Down
1 change: 1 addition & 0 deletions xcp/pci.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ def read(cls):
for f in ['/usr/share/hwdata/pci.ids']:
if os.path.exists(f):
return cls(f)
# pylint: disable-next=broad-exception-raised
raise Exception('Failed to open PCI database')

def findVendor(self, vendor):
Expand Down
2 changes: 1 addition & 1 deletion xcp/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def __eq__(self, v):
# is implemented:
# https://docs.python.org/3/reference/datamodel.html#object.__hash__
# Example:https://github.com/swagger-api/swagger-codegen/issues/6475
# Python2 pylint --py3k warns about it, and Pylint3 with out pylintrc
# Python2 pylint --py3k warns about it, and Pylint3 without .pylintrc
# now too:
def __hash__(self): # type:() -> int
return hash(str(self.ver))
Expand Down

0 comments on commit b510309

Please sign in to comment.