PR#1355: build_aux: use pylint --output-format=json - copr/copr

copr / copr

#1355 build_aux: use pylint --output-format=json

Merged 3 years ago by praiskup. Opened 3 years ago by praiskup.

Unknown source linter-nicer-pylint-output into master

build_aux: linter: rewrite pylint wrapper to python

Pavel Raiskup • 3 years ago

4fa6149

build_aux: use pylint --output-format=json

Pavel Raiskup • 3 years ago

b5e9e5a

build_aux/copr-csdiff-pylint

file added

+46

		`@@ -0,0 +1,46 @@`
		`+ #! /usr/bin/python3`
		`+`
		`+ """`
		`+ The csdiff tool doesn't support the Pylint's JSON output yet. So this just a`
		`+ trivial wrapper which reads Pylint's report and transforms it to JSON which is`
		`+ supported by csdiff.`
		`+`
		`+ The script accepts the same parameters as pylint itself.`
		`+ """`
		`+`
		`+ import sys`
		`+ import json`
		`+ import subprocess`
		`+`
		`+ PYLINT = ["pylint", "-rn", "--score=no", "--output-format=json"]`
		`+`
		`+`
		`+ def _main():`
		`+ pylint_command = PYLINT + sys.argv[1:]`
		`+`
		`+ # pylint: disable=subprocess-run-check`
		`+ pylint_result = subprocess.run(pylint_command, capture_output=True)`
		`+ data = json.loads(pylint_result.stdout)`
		`+ csdiff_data = []`
		`+ for defect in data:`
		`+ message = defect["obj"] + ": " if defect["obj"] else ""`
		`+ message += defect["message"]`
		`+`
		`+ csdiff_data.append({`
		`+ "checker": "PYLINT",`
		`+ "events": [{`
		`+ "file_name": defect["path"],`
		`+ "line": defect["line"],`
		`+ "column": defect["column"],`
		`+ "event": "{}({})".format(defect["message-id"],`
		`+ defect["symbol"]),`
		`+ "message": message,`
		`+ }],`
		`+ })`
		`+`
		`+ print(json.dumps({"defects": csdiff_data}, indent=2))`
		`+ return pylint_result.returncode`
		`+`
		`+`
		`+ if __name__ == "__main__":`
		`+ sys.exit(_main())`

build_aux/csdiff-pylint

file removed

-12

		`@@ -1,12 +0,0 @@`
		`- #! /bin/bash`
		`-`
		`- # csdiff mis-parses the line if we don't produce absoulute filenames, hence`
		`- # the leading /.`
		`-`
		`- pylint -rn \`
		`- --score=no \`
		`- --msg-template='/{path}:{line}:{column}: {msg_id}[pylint]: {msg}' \`
		`- "$@" \`
		`- \| grep -F -v '*******'`
		`-`
		`- exit "${PIPESTATUS[0]}"`

build_aux/linter

file modified

+9 -16

		`@@ -18,17 +18,18 @@`
		`log = logging.getLogger() # pylint: disable=invalid-name`

		`CSDIFF_PYLINT = os.path.realpath(os.path.join(os.path.dirname(__file__),`
		`- 'csdiff-pylint'))`
		`+ 'copr-csdiff-pylint'))`
		`+`

		`def _run_csdiff(old, new, msg):`
		`- popen_diff = Popen(['csdiff', old, new],`
		`+ popen_diff = Popen(['csdiff', '-c', old, new],`
		`stdout=PIPE)`
		`diff = popen_diff.communicate()[0].decode('utf-8')`
		`if diff:`
		`- print("============")`
		`- print(msg)`
		`- print("============")`
		`- print()`
		`+ sep = "=" * (len(msg) + 2)`
		`+ print(sep)`
		`+ print(" {} ".format(msg))`
		`+ print(sep + "\n")`
		`sys.stdout.write(diff)`
		`return int(bool(diff))`

		`@@ -67,7 +68,7 @@`
		`rules = []`
		`for pair in self.renames:`
		`old, new = self.modify_rename(pair[0], pair[1])`
		`- rule = "s\|^{}\|{}\|".format(old, new)`
		`+ rule = "s\|: \"{}\"\|: \"{}\"\|".format(old, new)`
		`rules += ['-e', rule]`

		`return ['sed'] + rules`
		`@@ -122,15 +123,6 @@`
		`def is_compatible(self, file):`
		`return file.type == 'python'`

		`- @classmethod`
		`- def modify_rename(cls, old, new):`
		`- """`
		`- Git reports 'a -> b' rename, but we use '/a -> /b' here because`
		`- otherwise csdiff wouldn't parse the reports. That's why we have to`
		`- modify it here, too.`
		`- """`
		`- return '/' + old, '/' + new`
		`-`
		`def command(self, projectdir, filenames):`
		`abs_pylintrc = os.path.join(self.gitroot, projectdir, 'pylintrc')`
		`pylintrc = os.path.realpath(abs_pylintrc)`
		`@@ -284,6 +276,7 @@`

		`if self.options.print_fixed_errors:`
		`_run_csdiff(new_report, old_report, "Fixed warnings")`
		`+ print()`

		`sys.exit(_run_csdiff(old_report, new_report, "Added warnings"))`

praiskup commented 3 years ago

csdiff tool currently doesn't support the input in Pylint's JSON format
directly, so we pre-format it by custom filter for now.

I initially wanted to use '{obj}' in --msg-format, to have some messages
more specific (see [1] and [2]), but some {obj} instances would contain
csdiff incompatible output (pieces of code, e.g.).

So, the JSON format is well-generated by Pylint, well re-formatted by
python-json, and then well understood by csdiff => this is much reliable
variant.

While I'm on it, I'm making the added/fixed headers in output slightly
nicer and separated from the rest of the output.

See upstream docs for more info:
[1] https://docs.pylint.org/en/1.6.0/output.html
[2] https://github.com/kdudka/csdiff/issues/6

rebased onto 8a1096c2362b0f09960cac2dfe3dbf859985f207

3 years ago

frostyx commented on line 23 of build_aux/csdiff-pylint 3 years ago

I would go with main() without underscore

frostyx commented on line 12 of build_aux/csdiff-pylint 3 years ago

I think it's worth it to rewrite the whole scripts in python now. We have basically two bash lines executing 40 lines python code ... It doesn't make much sense. I know, I know, those two lines of bash code will be ~5-10 lines of python, but that's IMHO still better than mixing those languages like this.

frostyx commented 3 years ago

Otherwise LGTM

praiskup commented on line 23 of build_aux/csdiff-pylint 3 years ago

By the underscore I mean to say it is module private function. By doing
that, pylint doesn't complain we need to document it (which we really
shouldn't I believe).

This _ prefix is new to me, so I'm not yet fully decided if that is
correct. What I really like is that pylint "forces" us to write the
documentation for the (even copr internal) API.

That said, if you insist on that - I can add differnt hack/fix to silence the
pylint warning.

1 new commit added