From b75f759c80a0754882426f04705fa4fa404383c4 Mon Sep 17 00:00:00 2001 From: i-keliukh <46344924+i-keliukh@users.noreply.github.com> Date: Tue, 4 Apr 2023 14:31:41 +0300 Subject: [PATCH 01/18] fix(test): new docker engine version compatibility After some update the docker engine started to require passing full paths to bind mounts when starting the contianer. The issue is reproduced on version 23.0.2. The error message from the docker engine is the following: "invalid volume specification: '.work:.work:rw': invalid mount config for type "volume": invalid mount path: '.work' mount path must be absolute". --- tests/deployment_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/deployment_utils.py b/tests/deployment_utils.py index 8d080315..32a67194 100644 --- a/tests/deployment_utils.py +++ b/tests/deployment_utils.py @@ -42,7 +42,8 @@ def add_bind_dirs(self, directories): if self._container: self.request.raiseerror("Container is already running, no dirs can be bound!") for directory in directories: - self._volumes[directory] = {'bind': directory, 'mode': 'rw'} + absolute_dir = str(pathlib.Path(directory).absolute()) + self._volumes[absolute_dir] = {'bind': absolute_dir, 'mode': 'rw'} def add_environment_variables(self, variables): if self._container: From b828b1d6dfbdf094dce4d5f153a335e75a0d8a38 Mon Sep 17 00:00:00 2001 From: i-keliukh <46344924+i-keliukh@users.noreply.github.com> Date: Wed, 5 Apr 2023 20:53:21 +0300 Subject: [PATCH 02/18] fix(analyzer): incorrect program name in help Before fix: $ python3.10 -m universum.analyzers.mypy --help usage: mypy.py [-h] ... After fix: $ python3.10 -m universum.analyzers.mypy --help usage: python3.10 -m universum.analyzers.mypy [-h] ... --- universum/analyzers/mypy.py | 7 ++----- universum/analyzers/pylint.py | 7 ++----- universum/analyzers/uncrustify.py | 11 ++++------- universum/analyzers/utils.py | 24 +++++++++++++++++++----- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/universum/analyzers/mypy.py b/universum/analyzers/mypy.py index d5c95e54..8536c835 100644 --- a/universum/analyzers/mypy.py +++ b/universum/analyzers/mypy.py @@ -1,19 +1,16 @@ import argparse - from typing import List from . import utils -def mypy_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Mypy analyzer") +def add_mypy_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument("--config-file", dest="config_file", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) - return parser @utils.sys_exit -@utils.analyzer(mypy_argument_parser()) +@utils.analyzer("Mypy analyzer", add_mypy_arguments) def main(settings: argparse.Namespace) -> List[utils.ReportData]: cmd = [f"python{settings.version}", '-m', 'mypy'] if settings.config_file: diff --git a/universum/analyzers/pylint.py b/universum/analyzers/pylint.py index 7b383f89..44207403 100644 --- a/universum/analyzers/pylint.py +++ b/universum/analyzers/pylint.py @@ -1,20 +1,17 @@ import argparse import json - from typing import List from . import utils -def pylint_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Pylint analyzer") +def add_pylint_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument("--rcfile", dest="rcfile", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) - return parser @utils.sys_exit -@utils.analyzer(pylint_argument_parser()) +@utils.analyzer("Pylint analyzer", add_pylint_arguments) def main(settings: argparse.Namespace) -> List[utils.ReportData]: cmd = [f"python{settings.version}", '-m', 'pylint', '-f', 'json'] if settings.rcfile: diff --git a/universum/analyzers/uncrustify.py b/universum/analyzers/uncrustify.py index 2f24ca2d..efe0af6c 100755 --- a/universum/analyzers/uncrustify.py +++ b/universum/analyzers/uncrustify.py @@ -1,31 +1,28 @@ import argparse import difflib import os -import shutil import pathlib import re - +import shutil from typing import Callable, List, Optional, Tuple from . import utils -def uncrustify_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Uncrustify analyzer") +def add_uncrustify_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument("--cfg-file", "-cf", dest="cfg_file", help="Name of the configuration file of Uncrustify; " "can also be set via 'UNCRUSTIFY_CONFIG' env. variable") parser.add_argument("--output-directory", "-od", dest="output_directory", default="uncrustify", help="Directory to store fixed files, generated by Uncrustify " - "and HTML files with diff; the default value is 'uncrustify'" + "and HTML files with diff; the default value is 'uncrustify'. " "Has to be distinct from source directory") parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, help="(optional) Set to generate html reports for each modified file") - return parser @utils.sys_exit -@utils.analyzer(uncrustify_argument_parser()) +@utils.analyzer("Uncrustify analyzer", add_uncrustify_arguments) def main(settings: argparse.Namespace) -> List[utils.ReportData]: if not shutil.which('uncrustify'): raise EnvironmentError("Please install uncrustify") diff --git a/universum/analyzers/utils.py b/universum/analyzers/utils.py index a6bfd452..214c6799 100644 --- a/universum/analyzers/utils.py +++ b/universum/analyzers/utils.py @@ -1,11 +1,12 @@ -import json -import sys import argparse import glob +import json +import os import pathlib import subprocess - +import sys from typing import Any, Callable, List, Optional, Tuple, Set + from typing_extensions import TypedDict from universum.lib.ci_exception import CiException @@ -19,18 +20,30 @@ def __init__(self, code: int = 2, message: Optional[str] = None): self.message: Optional[str] = message -def analyzer(parser: argparse.ArgumentParser): +def create_parser(description: str) -> argparse.ArgumentParser: + module_package = sys.modules["__main__"].__package__ + module_name, _ = os.path.splitext(os.path.basename(sys.argv[0])) + + prog = f"python{sys.version_info.major}.{sys.version_info.minor} -m {module_package}.{module_name}" + return argparse.ArgumentParser(prog=prog, description=description) + + +def analyzer(description: str, add_analyzer_arguments: Callable[[argparse.ArgumentParser], None]): """ Wraps the analyzer specific data and adds common protocol information: --files argument and its processing --result-file argument and its processing This function exists to define analyzer report interface - :param parser: Definition of analyzer custom arguments + :param description: Description of the analyzer, to be used in help + :param add_analyzer_arguments: Function that adds analyzer-specific arguments to parser :return: Wrapped analyzer with common reporting behaviour """ + def internal(func: Callable[[argparse.Namespace], List[ReportData]]) -> Callable[[], List[ReportData]]: def wrapper() -> List[ReportData]: + parser = create_parser(description) + add_analyzer_arguments(parser) add_files_argument(parser) add_result_file_argument(parser) settings: argparse.Namespace = parser.parse_args() @@ -75,6 +88,7 @@ def sys_exit(func: Callable[[], Any]) -> Callable[[], None]: >>> wrap_system_exit(sys_exit(_raise_custom)) 3 """ + def wrapper() -> None: exit_code: int try: From 1dd69773ecf3181f0bfe9f45a3bba64448ab0af1 Mon Sep 17 00:00:00 2001 From: i-keliukh <46344924+i-keliukh@users.noreply.github.com> Date: Fri, 7 Apr 2023 18:02:11 +0300 Subject: [PATCH 03/18] fix(analyzer): documentation generation broken by last change Partially reverted the last change to allow Sphinx argparse extension to work. --- doc/code_report.rst | 3 --- universum/analyzers/mypy.py | 6 ++++-- universum/analyzers/pylint.py | 6 ++++-- universum/analyzers/uncrustify.py | 6 ++++-- universum/analyzers/utils.py | 14 +++++--------- 5 files changed, 17 insertions(+), 18 deletions(-) diff --git a/doc/code_report.rst b/doc/code_report.rst index a9ca4b86..c16e9ec3 100644 --- a/doc/code_report.rst +++ b/doc/code_report.rst @@ -62,7 +62,6 @@ Pylint .. argparse:: :ref: universum.analyzers.pylint.pylint_argument_parser - :prog: {python} -m universum.analyzers.pylint Config example for ``universum.analyzers.pylint``: @@ -99,7 +98,6 @@ Mypy .. argparse:: :ref: universum.analyzers.mypy.mypy_argument_parser - :prog: {python} -m universum.analyzers.mypy Config example for ``universum.analyzers.mypy``: @@ -136,7 +134,6 @@ Uncrustify .. argparse:: :ref: universum.analyzers.uncrustify.uncrustify_argument_parser - :prog: {python} -m universum.analyzers.uncrustify :nodefault: Config example for ``universum.analyzers.uncrustify``: diff --git a/universum/analyzers/mypy.py b/universum/analyzers/mypy.py index 8536c835..a8b0544a 100644 --- a/universum/analyzers/mypy.py +++ b/universum/analyzers/mypy.py @@ -4,13 +4,15 @@ from . import utils -def add_mypy_arguments(parser: argparse.ArgumentParser) -> None: +def mypy_argument_parser() -> argparse.ArgumentParser: + parser = utils.create_parser("Mypy analyzer", __file__) parser.add_argument("--config-file", dest="config_file", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) + return parser @utils.sys_exit -@utils.analyzer("Mypy analyzer", add_mypy_arguments) +@utils.analyzer(mypy_argument_parser()) def main(settings: argparse.Namespace) -> List[utils.ReportData]: cmd = [f"python{settings.version}", '-m', 'mypy'] if settings.config_file: diff --git a/universum/analyzers/pylint.py b/universum/analyzers/pylint.py index 44207403..11cb8a67 100644 --- a/universum/analyzers/pylint.py +++ b/universum/analyzers/pylint.py @@ -5,13 +5,15 @@ from . import utils -def add_pylint_arguments(parser: argparse.ArgumentParser) -> None: +def pylint_argument_parser() -> argparse.ArgumentParser: + parser = utils.create_parser("Pylint analyzer", __file__) parser.add_argument("--rcfile", dest="rcfile", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) + return parser @utils.sys_exit -@utils.analyzer("Pylint analyzer", add_pylint_arguments) +@utils.analyzer(pylint_argument_parser()) def main(settings: argparse.Namespace) -> List[utils.ReportData]: cmd = [f"python{settings.version}", '-m', 'pylint', '-f', 'json'] if settings.rcfile: diff --git a/universum/analyzers/uncrustify.py b/universum/analyzers/uncrustify.py index efe0af6c..cfcc41df 100755 --- a/universum/analyzers/uncrustify.py +++ b/universum/analyzers/uncrustify.py @@ -9,7 +9,8 @@ from . import utils -def add_uncrustify_arguments(parser: argparse.ArgumentParser) -> None: +def uncrustify_argument_parser() -> argparse.ArgumentParser: + parser = utils.create_parser("Uncrustify analyzer", __file__) parser.add_argument("--cfg-file", "-cf", dest="cfg_file", help="Name of the configuration file of Uncrustify; " "can also be set via 'UNCRUSTIFY_CONFIG' env. variable") @@ -19,10 +20,11 @@ def add_uncrustify_arguments(parser: argparse.ArgumentParser) -> None: "Has to be distinct from source directory") parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, help="(optional) Set to generate html reports for each modified file") + return parser @utils.sys_exit -@utils.analyzer("Uncrustify analyzer", add_uncrustify_arguments) +@utils.analyzer(uncrustify_argument_parser()) def main(settings: argparse.Namespace) -> List[utils.ReportData]: if not shutil.which('uncrustify'): raise EnvironmentError("Please install uncrustify") diff --git a/universum/analyzers/utils.py b/universum/analyzers/utils.py index 214c6799..f754c42c 100644 --- a/universum/analyzers/utils.py +++ b/universum/analyzers/utils.py @@ -20,30 +20,26 @@ def __init__(self, code: int = 2, message: Optional[str] = None): self.message: Optional[str] = message -def create_parser(description: str) -> argparse.ArgumentParser: - module_package = sys.modules["__main__"].__package__ - module_name, _ = os.path.splitext(os.path.basename(sys.argv[0])) +def create_parser(description: str, module_path: str) -> argparse.ArgumentParser: + module_name, _ = os.path.splitext(os.path.basename(module_path)) - prog = f"python{sys.version_info.major}.{sys.version_info.minor} -m {module_package}.{module_name}" + prog = f"python{sys.version_info.major}.{sys.version_info.minor} -m {__package__}.{module_name}" return argparse.ArgumentParser(prog=prog, description=description) -def analyzer(description: str, add_analyzer_arguments: Callable[[argparse.ArgumentParser], None]): +def analyzer(parser: argparse.ArgumentParser): """ Wraps the analyzer specific data and adds common protocol information: --files argument and its processing --result-file argument and its processing This function exists to define analyzer report interface - :param description: Description of the analyzer, to be used in help - :param add_analyzer_arguments: Function that adds analyzer-specific arguments to parser + :param parser: Definition of analyzer custom arguments :return: Wrapped analyzer with common reporting behaviour """ def internal(func: Callable[[argparse.Namespace], List[ReportData]]) -> Callable[[], List[ReportData]]: def wrapper() -> List[ReportData]: - parser = create_parser(description) - add_analyzer_arguments(parser) add_files_argument(parser) add_result_file_argument(parser) settings: argparse.Namespace = parser.parse_args() From d06d6087ce12dd58f967dcd6d9d562b8499fb617 Mon Sep 17 00:00:00 2001 From: Oleksandr Andrieiev <40266373+o-andrieiev@users.noreply.github.com> Date: Mon, 10 Apr 2023 17:56:52 +0300 Subject: [PATCH 04/18] fix(configuration): allow False value for Step Replaced test of truthiness with an explicit check of None value Resolves #764 Tested by doctests --- universum/configuration_support.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/universum/configuration_support.py b/universum/configuration_support.py index f5706e06..75fafe40 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -344,7 +344,7 @@ def get(self, key: str, default: Any = None) -> Any: ... warnings.simplefilter("always") ... f() ... return w - >>> step = Step(name='foo', my_var='bar') + >>> step = Step(name='foo', my_var='bar', t1=None, t2=False) >>> do_and_get_warnings(lambda : step.get('name', 'test')) # doctest: +ELLIPSIS [] @@ -356,12 +356,16 @@ def get(self, key: str, default: Any = None) -> Any: 'test' >>> step.get('command', 'test') 'test' + >>> step.get('t1') is None + True + >>> step.get('t2') + False """ result = self._extras.get(key) - if result: + if result is not None: # for custom fields there is a distinction between None and falsy values return result result = self.__dict__.get(key) - if result: + if result: # non-custom fields initialized with falsy values warn("Using legacy API to access configuration values. Please use var." + key + " instead.") return result return default From e4db39b15cfe3a330ba915fa925e5e0c5379c0df Mon Sep 17 00:00:00 2001 From: Oleksandr Andrieiev <40266373+o-andrieiev@users.noreply.github.com> Date: Tue, 11 Apr 2023 11:47:30 +0300 Subject: [PATCH 05/18] fix(report): remove redundant file Static_analysis_report.json is not used for core report functionality, but is added to artifacts and creates misleading assumption that it has some deeper meaning. --- universum/modules/code_report_collector.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/universum/modules/code_report_collector.py b/universum/modules/code_report_collector.py index e558814c..edb2e795 100644 --- a/universum/modules/code_report_collector.py +++ b/universum/modules/code_report_collector.py @@ -92,9 +92,6 @@ def report_code_report_results(self) -> None: if text: report = json.loads(text) - json_file: TextIO = self.artifacts.create_text_file("Static_analysis_report.json") - json_file.write(json.dumps(report, indent=4)) - issue_count: int if not report and report != []: self.out.log_error("There are no results in code report file. Something went wrong.") From edd9c0c0c28d3ea4979cfc4b55cebc45856fe6d4 Mon Sep 17 00:00:00 2001 From: i-keliukh <46344924+i-keliukh@users.noreply.github.com> Date: Wed, 12 Apr 2023 10:35:47 +0300 Subject: [PATCH 06/18] feat(analyzer): code report based on clang-format Common code for clang_format and uncrustify analyzers is placed to utils. Also fixed some error messages of analyzers, fixed a test failure due to another docker engine update and added doc string for one unclear test. --- doc/code_report.rst | 39 +++++++- pylintrc | 5 +- setup.py | 6 +- tests/test_code_report.py | 63 +++++++++---- tests/test_nonci.py | 2 +- universum/analyzers/clang_format.py | 67 ++++++++++++++ universum/analyzers/diff_utils.py | 133 ++++++++++++++++++++++++++++ universum/analyzers/uncrustify.py | 131 +++------------------------ universum/analyzers/utils.py | 14 ++- 9 files changed, 318 insertions(+), 142 deletions(-) create mode 100755 universum/analyzers/clang_format.py create mode 100644 universum/analyzers/diff_utils.py diff --git a/doc/code_report.rst b/doc/code_report.rst index c16e9ec3..ecf06fcd 100644 --- a/doc/code_report.rst +++ b/doc/code_report.rst @@ -6,6 +6,7 @@ The following analysing modules (analysers) are currently added to Universum: * `pylint`_ * `mypy`_ * `uncrustify`_ + * `clang-format`_ Analysers are separate scripts, fully compatible with Universum. It is possible to use them as independent Python modules. @@ -143,7 +144,7 @@ Config example for ``universum.analyzers.uncrustify``: from universum.configuration_support import Configuration, Step configs = Configuration([Step(name="uncrustify", code_report=True, command=[ - "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp", + "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp/*.c", "--cfg-file", "file_name.cfg", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "uncrustify" ])]) @@ -161,4 +162,38 @@ will produce this list of configurations: .. testoutput:: $ ./.universum.py - [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] + [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp/*.c --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] + +Clang-format +------------ + +.. argparse:: + :ref: universum.analyzers.clang-format.clang_format_argument_parser + :nodefault: + +Config example for ``universum.analyzers.clang-format``: + +.. testcode:: + + from universum.configuration_support import Configuration, Step + + configs = Configuration([Step(name="clang-format", code_report=True, command=[ + "{python}", "-m", "universum.analyzers.clang-format", "--files", "/home/user/workspace/temp/*.c", + "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "clang-format", "-e", "clang-format-15", + ])]) + + if __name__ == '__main__': + print(configs.dump()) + +will produce this list of configurations: + +.. testcode:: + :hide: + + print("$ ./.universum.py") + print(configs.dump()) + +.. testoutput:: + + $ ./.universum.py + [{'name': 'clang-format', 'code_report': True, 'command': '{python} -m universum.analyzers.clang-format --files /home/user/workspace/temp/*.c --result-file ${CODE_REPORT_FILE} --output-directory clang-format -e clang-format-15'}] diff --git a/pylintrc b/pylintrc index 7b39d854..ee326fca 100644 --- a/pylintrc +++ b/pylintrc @@ -37,4 +37,7 @@ max-parents = 10 max-line-length = 130 [SIMILARITIES] -ignore-imports=yes \ No newline at end of file +ignore-imports=yes + +[TYPECHECK] +signature-mutators=universum.analyzers.utils.analyzer diff --git a/setup.py b/setup.py index 642aa26c..da23d2c1 100644 --- a/setup.py +++ b/setup.py @@ -35,7 +35,8 @@ def readme(): 'sh', 'lxml', 'typing-extensions', - 'ansi2html' + 'ansi2html', + 'pyyaml==6.0' ], extras_require={ 'p4': [p4], @@ -56,7 +57,8 @@ def readme(): 'coverage', 'mypy', 'types-requests', - 'selenium==3.141' + 'selenium==3.141', + 'types-PyYAML==6.0' ] }, package_data={'': ['*.css', '*.js']} diff --git a/tests/test_code_report.py b/tests/test_code_report.py index 9e57306c..f73cd2db 100644 --- a/tests/test_code_report.py +++ b/tests/test_code_report.py @@ -16,7 +16,7 @@ def fixture_runner_with_analyzers(docker_main: UniversumRunner): docker_main.environment.install_python_module("pylint") docker_main.environment.install_python_module("mypy") - docker_main.environment.assert_successful_execution("apt install uncrustify") + docker_main.environment.assert_successful_execution("apt install -y uncrustify clang-format") yield docker_main @@ -128,6 +128,11 @@ def finalize(self) -> str: input_tab_size = 2 """ +config_clang_format = """ +--- +AllowShortFunctionsOnASingleLine: Empty +""" + log_fail = r'Found [0-9]+ issues' log_success = r'Issues not found' @@ -167,7 +172,10 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c @pytest.mark.parametrize('analyzers, extra_args, tested_content, expected_success', [ [['uncrustify'], [], source_code_c, True], - [['uncrustify'], [], source_code_c.replace('\t', ' '), False], + [['uncrustify'], [], source_code_c.replace('\t', ' '), False], # by default uncrustify converts spaces to tabs + [['clang_format'], [], source_code_c.replace('\t', ' '), True], # by default clang-format expands tabs to 2 spaces + [['clang_format'], [], source_code_c.replace('\t', ' '), False], + [['clang_format', 'uncrustify'], [], source_code_c.replace('\t', ' '), False], [['pylint', 'mypy'], ["--python-version", python_version()], source_code_python, True], [['pylint'], ["--python-version", python_version()], source_code_python + '\n', False], [['mypy'], ["--python-version", python_version()], source_code_python.replace(': str', ': int'), False], @@ -177,6 +185,9 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c ], ids=[ 'uncrustify_no_issues', 'uncrustify_found_issues', + 'clang_format_no_issues', + 'clang_format_found_issues', + 'clang_format_and_uncrustify_found_issues', 'pylint_and_mypy_both_no_issues', 'pylint_found_issues', 'mypy_found_issues', @@ -194,6 +205,9 @@ def test_code_report_log(runner_with_analyzers: UniversumRunner, analyzers, extr if analyzer == 'uncrustify': args += ["--cfg-file", "cfg"] (runner_with_analyzers.local.root_directory / "cfg").write_text(config_uncrustify) + elif analyzer == 'clang_format': + (runner_with_analyzers.local.root_directory / ".clang-format").write_text(config_clang_format) + config.add_analyzer(analyzer, args) log = runner_with_analyzers.run(config.finalize()) @@ -210,7 +224,7 @@ def test_without_code_report_command(runner_with_analyzers: UniversumRunner): assert not pattern.findall(log) -@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify']) +@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify', 'clang_format']) @pytest.mark.parametrize('arg_set, expected_log', [ [["--files", "source_file.py"], "error: the following arguments are required: --result-file"], [["--files", "source_file.py", "--result-file"], "result-file: expected one argument"], @@ -235,10 +249,12 @@ def test_analyzer_python_version_params(runner_with_analyzers: UniversumRunner, "--result-file", "${CODE_REPORT_FILE}", '--rcfile'], "rcfile: expected one argument"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}"], - "Please specify the '--cfg_file' parameter or set an env. variable 'UNCRUSTIFY_CONFIG'"], + "Please specify the '--cfg-file' parameter or set 'UNCRUSTIFY_CONFIG' environment variable"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--cfg-file", "cfg", "--output-directory", "."], - "Target and source folders for uncrustify are not allowed to match"], + "Target folder must not be identical to source folder"], + ['clang_format', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "."], + "Target folder must not be identical to source folder"], ]) def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyzer, arg_set, expected_log): source_file = runner_with_analyzers.local.root_directory / "source_file" @@ -249,39 +265,54 @@ def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyz assert expected_log in log, f"'{expected_log}' is not found in '{log}'" -@pytest.mark.parametrize('extra_args, tested_content, expected_success, expected_artifact', [ - [[], source_code_c, True, False], - [["--report-html"], source_code_c.replace('\t', ' '), False, True], - [[], source_code_c.replace('\t', ' '), False, False], +@pytest.mark.parametrize('analyzer, extra_args, tested_content, expected_success, expected_artifact', [ + ['uncrustify', ["--report-html"], source_code_c, True, False], + ['uncrustify', ["--report-html"], source_code_c.replace('\t', ' '), False, True], + ['uncrustify', [], source_code_c.replace('\t', ' '), False, False], + ['clang_format', ["--report-html"], source_code_c.replace('\t', ' '), True, False], + ['clang_format', ["--report-html"], source_code_c, False, True], + ['clang_format', [], source_code_c, False, False], ], ids=[ "uncrustify_html_file_not_needed", "uncrustify_html_file_saved", "uncrustify_html_file_disabled", + "clang_format_html_file_not_needed", + "clang_format_html_file_saved", + "clang_format_html_file_disabled", ]) -def test_uncrustify_file_diff(runner_with_analyzers: UniversumRunner, - extra_args, tested_content, expected_success, expected_artifact): +def test_diff_html_file(runner_with_analyzers: UniversumRunner, analyzer, + extra_args, tested_content, expected_success, expected_artifact): + root = runner_with_analyzers.local.root_directory source_file = root / "source_file" source_file.write_text(tested_content) - (root / "cfg").write_text(config_uncrustify) common_args = [ "--result-file", "${CODE_REPORT_FILE}", "--files", "source_file", - "--cfg-file", "cfg", + "--output-directory", "diff_temp" ] + if analyzer == 'uncrustify': + (root / "cfg").write_text(config_uncrustify) + common_args.extend(["--cfg-file", "cfg"]) + elif analyzer == 'clang_format': + (root / ".clang-format").write_text(config_clang_format) + args = common_args + extra_args - extra_config = "artifacts='./uncrustify/source_file.html'" - log = runner_with_analyzers.run(ConfigData().add_analyzer('uncrustify', args, extra_config).finalize()) + extra_config = "artifacts='./diff_temp/source_file.html'" + log = runner_with_analyzers.run(ConfigData().add_analyzer(analyzer, args, extra_config).finalize()) expected_log = log_success if expected_success else log_fail assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'" expected_artifacts_state = "Success" if expected_artifact else "Failed" - expected_log = f"Collecting artifacts for the 'Run uncrustify' step - [^\n]*{expected_artifacts_state}" + expected_log = f"Collecting artifacts for the 'Run {analyzer}' step - [^\n]*{expected_artifacts_state}" assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'" def test_code_report_extended_arg_search(tmp_path: pathlib.Path, stdout_checker: FuzzyCallChecker): + """ + Test if ${CODE_REPORT_FILE} is replaced not only in --result-file argument of the Step + """ env = utils.LocalTestEnvironment(tmp_path, "main") env.settings.Vcs.type = "none" env.settings.LocalMainVcs.source_dir = str(tmp_path) diff --git a/tests/test_nonci.py b/tests/test_nonci.py index fe24168d..d03bb4f0 100644 --- a/tests/test_nonci.py +++ b/tests/test_nonci.py @@ -23,7 +23,7 @@ def test_launcher_output(docker_nonci: UniversumRunner): - version control and review system are not used - project root is set to current directory """ - cwd = str(docker_nonci.local.root_directory) + cwd = str(docker_nonci.local.root_directory.absolute()) artifacts = docker_nonci.artifact_dir file_output_expected = f"Adding file {artifacts}/test_step_log.txt to artifacts" pwd_string_in_logs = f"pwd:[{cwd}]" diff --git a/universum/analyzers/clang_format.py b/universum/analyzers/clang_format.py new file mode 100755 index 00000000..56a64f80 --- /dev/null +++ b/universum/analyzers/clang_format.py @@ -0,0 +1,67 @@ +import argparse +import pathlib +from typing import Callable, List, Optional, Tuple + +import yaml + +from . import utils, diff_utils + + +def clang_format_argument_parser() -> argparse.ArgumentParser: + parser = diff_utils.diff_analyzer_argument_parser("Clang-format analyzer", __file__, "clang-format") + parser.add_argument("--executable", "-e", dest="executable", default="clang-format", + help="The name of the clang-format executable, default: clang-format") + parser.add_argument("--style", dest="style", + help="The 'style' parameter of the clang-format. Can be literal 'file' string or " + "path to real file. See the clang-format documentation for details.") + return parser + + +def _add_style_param_if_present(cmd: List[str], settings: argparse.Namespace) -> None: + if settings.style: + cmd.extend(["-style", settings.style]) + + +@utils.sys_exit +@utils.analyzer(clang_format_argument_parser()) +def main(settings: argparse.Namespace) -> List[utils.ReportData]: + settings.name = "clang-format" + diff_utils.diff_analyzer_common_main(settings) + + html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None + if settings.write_html: + wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings) + html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) + + files: List[Tuple[pathlib.Path, pathlib.Path]] = [] + for src_file_absolute, target_file_absolute, _ in utils.get_files_with_absolute_paths(settings): + files.append((src_file_absolute, target_file_absolute)) + cmd = [settings.executable, src_file_absolute] + _add_style_param_if_present(cmd, settings) + output, _ = utils.run_for_output(cmd) + with open(target_file_absolute, "w", encoding="utf-8") as output_file: + output_file.write(output) + + return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) + + +def _get_wrapcolumn_tabsize(settings: argparse.Namespace) -> Tuple[int, int]: + cmd = [settings.executable, "--dump-config"] + _add_style_param_if_present(cmd, settings) + output, error = utils.run_for_output(cmd) + if error: + raise utils.AnalyzerException(message="clang-format --dump-config failed with the following error output: " + + error) + try: + clang_style = yaml.safe_load(output) + return clang_style["ColumnLimit"], clang_style["IndentWidth"] + except yaml.YAMLError as parse_error: + raise utils.AnalyzerException(message="Parsing of clang-format config produced the following error: " + + str(parse_error)) + except KeyError as key_error: + raise utils.AnalyzerException(message="Cannot find key in yaml during parsing of clang-format config: " + + str(key_error)) + + +if __name__ == "__main__": + main() diff --git a/universum/analyzers/diff_utils.py b/universum/analyzers/diff_utils.py new file mode 100644 index 00000000..493cf7de --- /dev/null +++ b/universum/analyzers/diff_utils.py @@ -0,0 +1,133 @@ +import argparse +import difflib +import pathlib +import shutil +from typing import Callable, List, Optional, Tuple + +from . import utils + + +class HtmlDiffFileWriter: + + def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: + self.target_folder = target_folder + self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) + + def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: + file_relative = file.relative_to(pathlib.Path.cwd()) + out_file_name: str = str(file_relative).replace('/', '_') + '.html' + with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: + out_file.write(self.differ.make_file(src, target, context=False)) + + +DiffWriter = Callable[[pathlib.Path, List[str], List[str]], None] + + +def diff_analyzer_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], + write_diff_file: Optional[DiffWriter] + ) -> List[utils.ReportData]: + result: List[utils.ReportData] = [] + for src_file, dst_file in files: + with open(src_file, encoding="utf-8") as src: + src_lines = src.readlines() + with open(dst_file, encoding="utf-8") as fixed: + fixed_lines = fixed.readlines() + + issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) + if issues and write_diff_file: + write_diff_file(src_file, src_lines, fixed_lines) + result.extend(issues) + return result + + +def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: + result = [] + matching_blocks: List[difflib.Match] = \ + difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() + previous_match = matching_blocks[0] + for match in matching_blocks[1:]: + block = _get_mismatching_block(previous_match, match, src, target) + previous_match = match + if not block: + continue + line, before, after = block + path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) + message = _get_issue_message(before, after) + result.append(utils.ReportData( + symbol="Code Style issue", + message=message, + path=str(path), + line=line + )) + + return result + + +def _get_issue_message(before: str, after: str) -> str: + # The maximum number of lines to write separate comments for + # If exceeded, summarized comment will be provided instead + max_lines = 11 + diff_size = len(before.splitlines()) + if diff_size > max_lines: + message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ + f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" + else: + # Message with before&after + message = f"\nOriginal code:\n```diff\n{before}```\n" + \ + f"Fixed code:\n```diff\n{after}```\n" + return message + + +def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] + match: difflib.Match, + src: List[str], target: List[str] + ) -> Optional[Tuple[int, str, str]]: + previous_match_end_in_src = previous_match.a + previous_match.size + previous_match_end_in_target = previous_match.b + previous_match.size + match_start_in_src = match.a + match_start_in_target = match.b + if previous_match_end_in_src == match_start_in_src: + return None + line = match_start_in_src + before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) + after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) + return line, before, after + + +def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: + return _replace_whitespace_characters(''.join(lines[start: end])) + + +_whitespace_character_mapping = { + " ": "\u00b7", + "\t": "\u2192\u2192\u2192\u2192", + "\n": "\u2193\u000a" +}.items() + + +def _replace_whitespace_characters(line: str) -> str: + for old_str, new_str in _whitespace_character_mapping: + line = line.replace(old_str, new_str) + return line + + +def diff_analyzer_argument_parser(description: str, module_path: str, output_directory: str) -> argparse.ArgumentParser: + parser = utils.create_parser(description, module_path) + parser.add_argument("--output-directory", "-od", dest="output_directory", default=output_directory, + help=f"Directory to store fixed files and HTML files with diff; the default " + f"value is '{output_directory}'. Has to be distinct from source directory") + parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, + help="(optional) Set to generate html reports for each modified file") + return parser + + +def diff_analyzer_common_main(settings: argparse.Namespace) -> None: + settings.target_folder = utils.normalize_path(settings.output_directory) + if settings.target_folder.exists() and settings.target_folder.samefile(pathlib.Path.cwd()): + raise EnvironmentError("Target folder must not be identical to source folder") + + settings.target_folder.mkdir(parents=True, exist_ok=True) + + if not shutil.which(settings.executable): + raise EnvironmentError(f"{settings.name} executable '{settings.executable}' is not found. " + f"Please install {settings.name} or fix the executable name.") diff --git a/universum/analyzers/uncrustify.py b/universum/analyzers/uncrustify.py index cfcc41df..7afc548d 100755 --- a/universum/analyzers/uncrustify.py +++ b/universum/analyzers/uncrustify.py @@ -1,85 +1,44 @@ import argparse -import difflib import os import pathlib import re -import shutil from typing import Callable, List, Optional, Tuple -from . import utils +from . import utils, diff_utils def uncrustify_argument_parser() -> argparse.ArgumentParser: - parser = utils.create_parser("Uncrustify analyzer", __file__) + parser = diff_utils.diff_analyzer_argument_parser("Uncrustify analyzer", __file__, "uncrustify") parser.add_argument("--cfg-file", "-cf", dest="cfg_file", help="Name of the configuration file of Uncrustify; " "can also be set via 'UNCRUSTIFY_CONFIG' env. variable") - parser.add_argument("--output-directory", "-od", dest="output_directory", default="uncrustify", - help="Directory to store fixed files, generated by Uncrustify " - "and HTML files with diff; the default value is 'uncrustify'. " - "Has to be distinct from source directory") - parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, - help="(optional) Set to generate html reports for each modified file") return parser @utils.sys_exit @utils.analyzer(uncrustify_argument_parser()) def main(settings: argparse.Namespace) -> List[utils.ReportData]: - if not shutil.which('uncrustify'): - raise EnvironmentError("Please install uncrustify") + settings.name = "uncrustify" + settings.executable = "uncrustify" + diff_utils.diff_analyzer_common_main(settings) + if not settings.cfg_file and 'UNCRUSTIFY_CONFIG' not in os.environ: - raise EnvironmentError("Please specify the '--cfg_file' parameter " - "or set an env. variable 'UNCRUSTIFY_CONFIG'") - target_folder: pathlib.Path = utils.normalize(settings.output_directory) - if target_folder.exists() and target_folder.samefile(pathlib.Path.cwd()): - raise EnvironmentError("Target and source folders for uncrustify are not allowed to match") + raise EnvironmentError("Please specify the '--cfg-file' parameter " + "or set 'UNCRUSTIFY_CONFIG' environment variable") + html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None if settings.write_html: wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings.cfg_file) - html_diff_file_writer = HtmlDiffFileWriter(target_folder, wrapcolumn, tabsize) + html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) cmd = ["uncrustify", "-q", "-c", settings.cfg_file, "--prefix", settings.output_directory] files: List[Tuple[pathlib.Path, pathlib.Path]] = [] - for src_file in settings.file_list: - src_file_absolute = utils.normalize(src_file) - src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) - target_file_absolute: pathlib.Path = target_folder.joinpath(src_file_relative) + for src_file_absolute, target_file_absolute, src_file_relative in utils.get_files_with_absolute_paths(settings): files.append((src_file_absolute, target_file_absolute)) cmd.append(src_file_relative) utils.run_for_output(cmd) - return uncrustify_output_parser(files, html_diff_file_writer) - - -class HtmlDiffFileWriter: - - def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: - self.target_folder = target_folder - self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) - - def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: - file_relative = file.relative_to(pathlib.Path.cwd()) - out_file_name: str = str(file_relative).replace('/', '_') + '.html' - with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: - out_file.write(self.differ.make_file(src, target, context=False)) - - -def uncrustify_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], - write_diff_file: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] - ) -> List[utils.ReportData]: - result: List[utils.ReportData] = [] - for src_file, uncrustify_file in files: - with open(src_file, encoding="utf-8") as src: - src_lines = src.readlines() - with open(uncrustify_file, encoding="utf-8") as fixed: - fixed_lines = fixed.readlines() - - issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) - if issues and write_diff_file: - write_diff_file(src_file, src_lines, fixed_lines) - result.extend(issues) - return result + return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: @@ -98,69 +57,5 @@ def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: return wrapcolumn, tabsize -def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: - result = [] - matching_blocks: List[difflib.Match] = \ - difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() - previous_match = matching_blocks[0] - for match in matching_blocks[1:]: - block = _get_mismatching_block(previous_match, match, src, target) - previous_match = match - if not block: - continue - line, before, after = block - path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) - message = _get_issue_message(before, after) - result.append(utils.ReportData( - symbol="Uncrustify Code Style issue", - message=message, - path=str(path), - line=line - )) - - return result - - -def _get_issue_message(before: str, after: str) -> str: - # The maximum number of lines to write separate comments for - # If exceeded, summarized comment will be provided instead - max_lines = 11 - diff_size = len(before.splitlines()) - if diff_size > max_lines: - message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ - f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" - else: - # Message with before&after - message = f"\nOriginal code:\n```diff\n{before}```\n" + \ - f"Uncrustify generated code:\n```diff\n{after}```\n" - return message - - -def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] - match: difflib.Match, - src: List[str], target: List[str] - ) -> Optional[Tuple[int, str, str]]: - previous_match_end_in_src = previous_match.a + previous_match.size - previous_match_end_in_target = previous_match.b + previous_match.size - match_start_in_src = match.a - match_start_in_target = match.b - if previous_match_end_in_src == match_start_in_src: - return None - line = match_start_in_src - before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) - after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) - return line, before, after - - -def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: - return _replace_invisible_symbols(''.join(lines[start: end])) - - -def _replace_invisible_symbols(line: str) -> str: - for old_str, new_str in zip([" ", "\t", "\n"], ["\u00b7", "\u2192\u2192\u2192\u2192", "\u2193\u000a"]): - line = line.replace(old_str, new_str) - return line - - if __name__ == "__main__": - main() # pylint: disable=no-value-for-parameter # see https://github.com/PyCQA/pylint/issues/259 + main() diff --git a/universum/analyzers/utils.py b/universum/analyzers/utils.py index f754c42c..7d921709 100644 --- a/universum/analyzers/utils.py +++ b/universum/analyzers/utils.py @@ -5,7 +5,7 @@ import pathlib import subprocess import sys -from typing import Any, Callable, List, Optional, Tuple, Set +from typing import Any, Callable, List, Optional, Tuple, Set, Iterable from typing_extensions import TypedDict @@ -156,6 +156,16 @@ def report_to_file(issues: List[ReportData], json_file: Optional[str] = None) -> sys.stdout.write(issues_json) -def normalize(file: str) -> pathlib.Path: +def normalize_path(file: str) -> pathlib.Path: file_path = pathlib.Path(file) return file_path if file_path.is_absolute() else pathlib.Path.cwd().joinpath(file_path) + + +def get_files_with_absolute_paths(settings: argparse.Namespace) -> Iterable[Tuple[pathlib.Path, + pathlib.Path, + pathlib.Path]]: + for src_file in settings.file_list: + src_file_absolute = normalize_path(src_file) + src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) + target_file_absolute: pathlib.Path = settings.target_folder.joinpath(src_file_relative) + yield src_file_absolute, target_file_absolute, src_file_relative From 1086bd8e69d7a77de0c533160901e054ffe971c6 Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Wed, 12 Apr 2023 19:14:25 +0300 Subject: [PATCH 07/18] fix(workflow): disable link previews in TG bot All previews refer to Universum main page anyway. --- .github/workflows/telegram-bot.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/telegram-bot.yml b/.github/workflows/telegram-bot.yml index 6d623abc..1fe265a6 100644 --- a/.github/workflows/telegram-bot.yml +++ b/.github/workflows/telegram-bot.yml @@ -63,7 +63,7 @@ jobs: fi if [[ ! -z $TEXT ]]; then - curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" \ + curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" --data-urlencode "disable_web_page_preview=True" \ --data-urlencode "text=$TEXT" --data-urlencode "parse_mode=HTML" $URL fi From b902d9d0dcd087908481a037cb35730f5c28c35c Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Mon, 24 Apr 2023 12:54:13 +0300 Subject: [PATCH 08/18] fix(workflow): add comment body for review comment --- .github/workflows/telegram-bot.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/telegram-bot.yml b/.github/workflows/telegram-bot.yml index 1fe265a6..c319b739 100644 --- a/.github/workflows/telegram-bot.yml +++ b/.github/workflows/telegram-bot.yml @@ -23,6 +23,7 @@ jobs: PR_MERGED: ${{ github.event.pull_request.merged_by.login }} REVIEW_STATE: ${{ github.event.review.state }} REVIEW_AUTHOR: ${{ github.event.review.user.login }} + REVIEW_COMMENT: ${{ github.event.review.body }} COMMENT_AUTHOR: ${{ github.event.comment.user.login }} COMMENT_URL: ${{ github.event.comment.html_url }} COMMENT_BODY: ${{ github.event.comment.body }} @@ -51,7 +52,10 @@ jobs: elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "changes_requested" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} requested changes for PR#${{ env.PR_NUMBER }}"` - elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" != "changes_requested" ]]; then + elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "commented" ]]; then + ESCAPED_TEXT=`echo -e "${{ env.REVIEW_COMMENT }}"| sed 's/\&/\&/g' | sed 's//\>/g'` + TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} posted the following comment to PR#${{ env.PR_NUMBER }}:\n$ESCAPED_TEXT"` + elif [[ ! -z "${{ github.event.review }}" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} ${{ env.REVIEW_STATE }} PR#${{ env.PR_NUMBER }}"` elif [[ -z "${{ github.event.review }}" && "${{ github.event.action }}" == "submitted" ]]; then TEXT=`echo -e "Due to GitHub Actions bug we cannot identify, who approved PR#${{ env.PR_NUMBER }}"` From 6c8d3f03fd54b8bbb961d0de4cd21c9922ffd868 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 4 May 2023 21:09:30 +0300 Subject: [PATCH 09/18] feat(configuration_support): change conditional branch steps type to Configuration --- tests/test_conditional_steps.py | 4 ++-- universum/configuration_support.py | 10 ++++----- universum/modules/artifact_collector.py | 27 +++++++++++-------------- universum/modules/structure_handler.py | 4 ++-- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/tests/test_conditional_steps.py b/tests/test_conditional_steps.py index a7316370..d27f1a0d 100644 --- a/tests/test_conditional_steps.py +++ b/tests/test_conditional_steps.py @@ -98,9 +98,9 @@ def _build_step_command(files_to_create, exit_code) -> List[str]: return ["bash", "-c", ";".join(commands)] def _write_config_file(self, steps_info) -> None: - true_branch_step: str = f"Step(**{str(steps_info.true_branch_step)})" \ + true_branch_step: str = f"Configuration([Step(**{str(steps_info.true_branch_step)})])" \ if steps_info.true_branch_step else "None" - false_branch_step: str = f"Step(**{str(steps_info.false_branch_step)})" \ + false_branch_step: str = f"Configuration([Step(**{str(steps_info.false_branch_step)})])" \ if steps_info.false_branch_step else "None" config_lines: List[str] = [ "from universum.configuration_support import Configuration, Step", diff --git a/universum/configuration_support.py b/universum/configuration_support.py index bf798af8..619d6782 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -182,8 +182,8 @@ def __init__(self, pass_tag: str = '', fail_tag: str = '', if_env_set: str = '', - if_succeeded = None, - if_failed = None, + if_succeeded: Optional['Configuration'] = None, + if_failed: Optional['Configuration'] = None, **kwargs) -> None: self.name: str = name self.directory: str = directory @@ -199,9 +199,9 @@ def __init__(self, self.pass_tag: str = pass_tag self.fail_tag: str = fail_tag self.if_env_set: str = if_env_set - self.if_succeeded = if_succeeded - self.if_failed = if_failed - self.is_conditional = self.if_succeeded or self.if_failed + self.if_succeeded: Optional['Configuration'] = if_succeeded + self.if_failed: Optional['Configuration'] = if_failed + self.is_conditional: bool = bool(self.if_succeeded or self.if_failed) self.children: Optional['Configuration'] = None self._extras: Dict[str, str] = {} for key, value in kwargs.items(): diff --git a/universum/modules/artifact_collector.py b/universum/modules/artifact_collector.py index c6748d23..1a1c5373 100644 --- a/universum/modules/artifact_collector.py +++ b/universum/modules/artifact_collector.py @@ -173,24 +173,21 @@ def set_and_clean_artifacts(self, project_configs: Configuration, ignore_existin with self.structure.block(block_name=name, pass_errors=True): self.preprocess_artifact_list(artifact_list, ignore_existing_artifacts) - def get_conditional_step_branches_artifacts(self, step: Step) -> List[ArtifactInfo]: - artifacts: List[Optional[ArtifactInfo]] = [ - self.get_config_artifact_if_exists(step.if_succeeded), - self.get_config_artifact_if_exists(step.if_failed), - self.get_config_artifact_if_exists(step.if_succeeded, is_report_artifact=True), - self.get_config_artifact_if_exists(step.if_failed, is_report_artifact=True) - ] + def get_conditional_step_branches_artifacts(self, conditional_step: Step) -> List[ArtifactInfo]: + steps_to_process: List[Step] = [] + if conditional_step.if_succeeded: + steps_to_process.extend(list(conditional_step.if_succeeded.all())) + if conditional_step.if_failed: + steps_to_process.extend(list(conditional_step.if_failed.all())) + + artifacts: List[Optional[ArtifactInfo]] = [] + for step in steps_to_process: + artifacts.append(self.get_config_artifact(step)) + artifacts.append(self.get_config_artifact(step, is_report_artifact=True)) + defined_artifacts: List[ArtifactInfo] = [artifact for artifact in artifacts if artifact] return defined_artifacts - def get_config_artifact_if_exists(self, step: Step, is_report_artifact: bool = False) -> Optional[ArtifactInfo]: - if not step: - return None - artifact: str = step.report_artifacts if is_report_artifact else step.artifacts - if not artifact: - return None - return self.get_config_artifact(step, is_report_artifact) - def get_config_artifact(self, step: Step, is_report_artifact: bool = False) -> ArtifactInfo: artifact: str = step.report_artifacts if is_report_artifact else step.artifacts path: str = utils.parse_path(artifact, self.settings.project_root) diff --git a/universum/modules/structure_handler.py b/universum/modules/structure_handler.py index 8a81cffb..733699a1 100644 --- a/universum/modules/structure_handler.py +++ b/universum/modules/structure_handler.py @@ -247,11 +247,11 @@ def execute_steps_recursively(self, parent: Step, elif child.is_conditional: conditional_step_succeeded: bool = self.process_one_step(merged_item, step_executor, skip_execution=False) - step_to_execute: Step = merged_item.if_succeeded if conditional_step_succeeded \ + step_to_execute: Optional[Configuration] = merged_item.if_succeeded if conditional_step_succeeded \ else merged_item.if_failed if step_to_execute: # branch step can be not set return self.execute_steps_recursively(parent=Step(), - children=Configuration([step_to_execute]), + children=step_to_execute, step_executor=step_executor, skip_execution=False) current_step_failed = False # conditional step should be always successful From 0f844186ea08e2dff4a622a64c00d662bff84a3f Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Sun, 7 May 2023 21:57:31 +0300 Subject: [PATCH 10/18] documentation update --- doc/configuring.rst | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/configuring.rst b/doc/configuring.rst index ce0b5962..551ebe8d 100644 --- a/doc/configuring.rst +++ b/doc/configuring.rst @@ -441,9 +441,9 @@ If any of them is missing or not set in current environment, the step will be ex Conditional steps --------------------- -Conditional step is a :class:`Step` object, that has ``if_succeeded`` or ``if_failed`` parameters with other steps assigned. -If the conditional step succeeds, then the step from the ``if_succeeded`` parameter will be executed. -If the conditional step fails, the step from the ``if_failed`` parameter will be executed instead. +Conditional step is a :class:`Step` object, that has ``if_succeeded`` or ``if_failed`` parameters with other :class:`Configuration` objects assigned. +If the conditional step succeeds, then the Configuration from the ``if_succeeded`` parameter will be executed. +If the conditional step fails, then the Configuration from the ``if_failed`` parameter will be executed instead. Configuration example: @@ -451,8 +451,8 @@ Configuration example: from universum.configuration_support import Configuration, Step - true_branch_step = Step(name="Positive branch step", command=["ls"]) - false_branch_step = Step(name="Negative branch step", command=["pwd"]) + true_branch_step = Configuration([Step(name="Positive branch step", command=["ls"])]) + false_branch_step = Configuration([Step(name="Negative branch step", command=["pwd"])]) conditional_step = Step(name="Conditional step", command=["./script.sh"], if_succeeded=true_branch_step, if_failed=false_branch_step) @@ -502,7 +502,7 @@ In general, conditional steps behave as any other regular steps, but here are so * Will always be marked as successful in the log * TeamCity tag will not be set for the conditional step * Branch steps - * Only one branch step will be executed + * Only one branch Configuration will be executed * Both branches' artifacts will be checked for existence before the steps execution * Artifacts collection or any other side-effects will not be triggered for non-executed branch step * If chosen branch step is not set, nothing will happen. From 075d3c0f2d710adf2cd905260b3e45513be6e33a Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Wed, 10 May 2023 06:55:19 +0300 Subject: [PATCH 11/18] update Step docstring --- universum/configuration_support.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/universum/configuration_support.py b/universum/configuration_support.py index 619d6782..7813c5eb 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -125,11 +125,11 @@ class Step: fail_tag A tag used to mark failed TeamCity builds. See `pass_tag` for details. Not applicable for conditional steps. if_succeeded - Another step, that will be executed in case of this step will succeed. Having this parameter non-None will - make the current step conditional. + Another Configuration, that will be executed in case of this step will succeed. + Having this parameter non-None will make the current step conditional. if_failed - Another step, that will be executed in case of this step will fail. Having this parameter non-None will - make the current step conditional. + Another Configuration, that will be executed in case of this step will fail. + Having this parameter non-None will make the current step conditional. Each parameter is optional, and is substituted with a falsy value, if omitted. From a3b80a5db4db2664edf0311870370988a84d2e41 Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Wed, 10 May 2023 12:55:00 +0300 Subject: [PATCH 12/18] fix(swarm): remove warning suppression that is no longer relevant --- universum/modules/vcs/swarm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/universum/modules/vcs/swarm.py b/universum/modules/vcs/swarm.py index 53eb8e46..ca671301 100644 --- a/universum/modules/vcs/swarm.py +++ b/universum/modules/vcs/swarm.py @@ -9,7 +9,7 @@ from ...lib.ci_exception import CiException from ...lib.gravity import Dependency -urllib3.disable_warnings((urllib3.exceptions.InsecurePlatformWarning, urllib3.exceptions.SNIMissingWarning)) # type: ignore +urllib3.disable_warnings(urllib3.exceptions.InsecurePlatformWarning) # type: ignore __all__ = [ "Swarm" From 9f0894a20cc8409a145465d8dba5d1f0520ce4cc Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Wed, 10 May 2023 17:54:22 +0300 Subject: [PATCH 13/18] update(docs): update changelog to Universum 0.19.15 --- CHANGELOG.rst | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0ea21a2d..0678da9b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,23 @@ Change log ========== +0.19.15 (2023-05-10) +-------------------- + +New features +~~~~~~~~~~~~ + +* **analyzer:** code report based on clang-format + +Bug fixes +~~~~~~~~~ + +* **config:** fixed returning "None" instead of "False" for Step custom keys set to "False" +* **artifact:** remove redundant Static_analysis_report.json from artifacts +* **analyzer:** incorrect program name in help +* **report:** set exit code even if reporting crashes + + 0.19.14 (2022-11-14) -------------------- From 1f26ff0d2a4f644261391df76255fbd812d9e35d Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Mon, 15 May 2023 17:01:47 +0300 Subject: [PATCH 14/18] update(doc): increase version --- universum/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/universum/__init__.py b/universum/__init__.py index 97c18b90..efd4d5e5 100644 --- a/universum/__init__.py +++ b/universum/__init__.py @@ -1,2 +1,2 @@ __title__ = "Universum" -__version__ = "0.19.15" +__version__ = "0.19.16" From d335a6baa2ae8188ccba7bb464c9a436da5c9304 Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Tue, 16 May 2023 09:35:28 +0300 Subject: [PATCH 15/18] fix(doc): fix failing RTD builds None is no longer supported as default language for RTD Fix typo in 'clang-format' module name --- doc/code_report.rst | 4 ++-- doc/conf.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/code_report.rst b/doc/code_report.rst index ecf06fcd..48317dcf 100644 --- a/doc/code_report.rst +++ b/doc/code_report.rst @@ -168,10 +168,10 @@ Clang-format ------------ .. argparse:: - :ref: universum.analyzers.clang-format.clang_format_argument_parser + :ref: universum.analyzers.clang_format.clang_format_argument_parser :nodefault: -Config example for ``universum.analyzers.clang-format``: +Config example for ``universum.analyzers.clang_format``: .. testcode:: diff --git a/doc/conf.py b/doc/conf.py index 229cdb94..d25e3408 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -50,7 +50,7 @@ # # This is also used if you do content translation via gettext catalogs. # Usually you set "language" from the command line for these cases. -language = None +language = 'en' # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. From 0112821a321dbe5ee74c025bd6656d532cb2855e Mon Sep 17 00:00:00 2001 From: Kateryna Dovgan <46348880+k-dovgan@users.noreply.github.com> Date: Tue, 16 May 2023 21:10:38 +0300 Subject: [PATCH 16/18] fix(test): outdated dependencies Latest selenium version is 4.0.9; latest urllib3 version is 2.0.2. However, used selenium version is 3.141, and latest non-failing urllib3 version for running FF with this selenium version is 1.26.15 --- setup.py | 1 + 1 file changed, 1 insertion(+) diff --git a/setup.py b/setup.py index da23d2c1..e5dc59d4 100644 --- a/setup.py +++ b/setup.py @@ -58,6 +58,7 @@ def readme(): 'mypy', 'types-requests', 'selenium==3.141', + 'urllib3==1.26.15', # This is required for selenium-3.141 to work correctly 'types-PyYAML==6.0' ] }, From 31281a656a976b5cb41635ce821dd48c2d1a6451 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 25 May 2023 06:54:00 +0300 Subject: [PATCH 17/18] chore: merge with master branch (#805) --- .github/workflows/telegram-bot.yml | 8 +- CHANGELOG.rst | 17 +++ doc/code_report.rst | 42 ++++++- doc/conf.py | 2 +- pylintrc | 5 +- setup.py | 7 +- tests/deployment_utils.py | 3 +- tests/test_code_report.py | 63 +++++++--- tests/test_nonci.py | 2 +- universum/__init__.py | 2 +- universum/analyzers/clang_format.py | 67 +++++++++++ universum/analyzers/diff_utils.py | 133 +++++++++++++++++++++ universum/analyzers/mypy.py | 3 +- universum/analyzers/pylint.py | 3 +- universum/analyzers/uncrustify.py | 132 ++------------------ universum/analyzers/utils.py | 28 ++++- universum/configuration_support.py | 10 +- universum/modules/code_report_collector.py | 3 - universum/modules/vcs/swarm.py | 2 +- 19 files changed, 368 insertions(+), 164 deletions(-) create mode 100755 universum/analyzers/clang_format.py create mode 100644 universum/analyzers/diff_utils.py diff --git a/.github/workflows/telegram-bot.yml b/.github/workflows/telegram-bot.yml index 6d623abc..c319b739 100644 --- a/.github/workflows/telegram-bot.yml +++ b/.github/workflows/telegram-bot.yml @@ -23,6 +23,7 @@ jobs: PR_MERGED: ${{ github.event.pull_request.merged_by.login }} REVIEW_STATE: ${{ github.event.review.state }} REVIEW_AUTHOR: ${{ github.event.review.user.login }} + REVIEW_COMMENT: ${{ github.event.review.body }} COMMENT_AUTHOR: ${{ github.event.comment.user.login }} COMMENT_URL: ${{ github.event.comment.html_url }} COMMENT_BODY: ${{ github.event.comment.body }} @@ -51,7 +52,10 @@ jobs: elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "changes_requested" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} requested changes for PR#${{ env.PR_NUMBER }}"` - elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" != "changes_requested" ]]; then + elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "commented" ]]; then + ESCAPED_TEXT=`echo -e "${{ env.REVIEW_COMMENT }}"| sed 's/\&/\&/g' | sed 's//\>/g'` + TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} posted the following comment to PR#${{ env.PR_NUMBER }}:\n$ESCAPED_TEXT"` + elif [[ ! -z "${{ github.event.review }}" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} ${{ env.REVIEW_STATE }} PR#${{ env.PR_NUMBER }}"` elif [[ -z "${{ github.event.review }}" && "${{ github.event.action }}" == "submitted" ]]; then TEXT=`echo -e "Due to GitHub Actions bug we cannot identify, who approved PR#${{ env.PR_NUMBER }}"` @@ -63,7 +67,7 @@ jobs: fi if [[ ! -z $TEXT ]]; then - curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" \ + curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" --data-urlencode "disable_web_page_preview=True" \ --data-urlencode "text=$TEXT" --data-urlencode "parse_mode=HTML" $URL fi diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0ea21a2d..0678da9b 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,23 @@ Change log ========== +0.19.15 (2023-05-10) +-------------------- + +New features +~~~~~~~~~~~~ + +* **analyzer:** code report based on clang-format + +Bug fixes +~~~~~~~~~ + +* **config:** fixed returning "None" instead of "False" for Step custom keys set to "False" +* **artifact:** remove redundant Static_analysis_report.json from artifacts +* **analyzer:** incorrect program name in help +* **report:** set exit code even if reporting crashes + + 0.19.14 (2022-11-14) -------------------- diff --git a/doc/code_report.rst b/doc/code_report.rst index a9ca4b86..48317dcf 100644 --- a/doc/code_report.rst +++ b/doc/code_report.rst @@ -6,6 +6,7 @@ The following analysing modules (analysers) are currently added to Universum: * `pylint`_ * `mypy`_ * `uncrustify`_ + * `clang-format`_ Analysers are separate scripts, fully compatible with Universum. It is possible to use them as independent Python modules. @@ -62,7 +63,6 @@ Pylint .. argparse:: :ref: universum.analyzers.pylint.pylint_argument_parser - :prog: {python} -m universum.analyzers.pylint Config example for ``universum.analyzers.pylint``: @@ -99,7 +99,6 @@ Mypy .. argparse:: :ref: universum.analyzers.mypy.mypy_argument_parser - :prog: {python} -m universum.analyzers.mypy Config example for ``universum.analyzers.mypy``: @@ -136,7 +135,6 @@ Uncrustify .. argparse:: :ref: universum.analyzers.uncrustify.uncrustify_argument_parser - :prog: {python} -m universum.analyzers.uncrustify :nodefault: Config example for ``universum.analyzers.uncrustify``: @@ -146,7 +144,7 @@ Config example for ``universum.analyzers.uncrustify``: from universum.configuration_support import Configuration, Step configs = Configuration([Step(name="uncrustify", code_report=True, command=[ - "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp", + "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp/*.c", "--cfg-file", "file_name.cfg", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "uncrustify" ])]) @@ -164,4 +162,38 @@ will produce this list of configurations: .. testoutput:: $ ./.universum.py - [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] + [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp/*.c --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] + +Clang-format +------------ + +.. argparse:: + :ref: universum.analyzers.clang_format.clang_format_argument_parser + :nodefault: + +Config example for ``universum.analyzers.clang_format``: + +.. testcode:: + + from universum.configuration_support import Configuration, Step + + configs = Configuration([Step(name="clang-format", code_report=True, command=[ + "{python}", "-m", "universum.analyzers.clang-format", "--files", "/home/user/workspace/temp/*.c", + "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "clang-format", "-e", "clang-format-15", + ])]) + + if __name__ == '__main__': + print(configs.dump()) + +will produce this list of configurations: + +.. testcode:: + :hide: + + print("$ ./.universum.py") + print(configs.dump()) + +.. testoutput:: + + $ ./.universum.py + [{'name': 'clang-format', 'code_report': True, 'command': '{python} -m universum.analyzers.clang-format --files /home/user/workspace/temp/*.c --result-file ${CODE_REPORT_FILE} --output-directory clang-format -e clang-format-15'}] diff --git a/doc/conf.py b/doc/conf.py index 229cdb94..d25e3408 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -50,7 +50,7 @@ # # This is also used if you do content translation via gettext catalogs. # Usually you set "language" from the command line for these cases. -language = None +language = 'en' # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. diff --git a/pylintrc b/pylintrc index 7b39d854..ee326fca 100644 --- a/pylintrc +++ b/pylintrc @@ -37,4 +37,7 @@ max-parents = 10 max-line-length = 130 [SIMILARITIES] -ignore-imports=yes \ No newline at end of file +ignore-imports=yes + +[TYPECHECK] +signature-mutators=universum.analyzers.utils.analyzer diff --git a/setup.py b/setup.py index 642aa26c..e5dc59d4 100644 --- a/setup.py +++ b/setup.py @@ -35,7 +35,8 @@ def readme(): 'sh', 'lxml', 'typing-extensions', - 'ansi2html' + 'ansi2html', + 'pyyaml==6.0' ], extras_require={ 'p4': [p4], @@ -56,7 +57,9 @@ def readme(): 'coverage', 'mypy', 'types-requests', - 'selenium==3.141' + 'selenium==3.141', + 'urllib3==1.26.15', # This is required for selenium-3.141 to work correctly + 'types-PyYAML==6.0' ] }, package_data={'': ['*.css', '*.js']} diff --git a/tests/deployment_utils.py b/tests/deployment_utils.py index 8d080315..32a67194 100644 --- a/tests/deployment_utils.py +++ b/tests/deployment_utils.py @@ -42,7 +42,8 @@ def add_bind_dirs(self, directories): if self._container: self.request.raiseerror("Container is already running, no dirs can be bound!") for directory in directories: - self._volumes[directory] = {'bind': directory, 'mode': 'rw'} + absolute_dir = str(pathlib.Path(directory).absolute()) + self._volumes[absolute_dir] = {'bind': absolute_dir, 'mode': 'rw'} def add_environment_variables(self, variables): if self._container: diff --git a/tests/test_code_report.py b/tests/test_code_report.py index 9e57306c..f73cd2db 100644 --- a/tests/test_code_report.py +++ b/tests/test_code_report.py @@ -16,7 +16,7 @@ def fixture_runner_with_analyzers(docker_main: UniversumRunner): docker_main.environment.install_python_module("pylint") docker_main.environment.install_python_module("mypy") - docker_main.environment.assert_successful_execution("apt install uncrustify") + docker_main.environment.assert_successful_execution("apt install -y uncrustify clang-format") yield docker_main @@ -128,6 +128,11 @@ def finalize(self) -> str: input_tab_size = 2 """ +config_clang_format = """ +--- +AllowShortFunctionsOnASingleLine: Empty +""" + log_fail = r'Found [0-9]+ issues' log_success = r'Issues not found' @@ -167,7 +172,10 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c @pytest.mark.parametrize('analyzers, extra_args, tested_content, expected_success', [ [['uncrustify'], [], source_code_c, True], - [['uncrustify'], [], source_code_c.replace('\t', ' '), False], + [['uncrustify'], [], source_code_c.replace('\t', ' '), False], # by default uncrustify converts spaces to tabs + [['clang_format'], [], source_code_c.replace('\t', ' '), True], # by default clang-format expands tabs to 2 spaces + [['clang_format'], [], source_code_c.replace('\t', ' '), False], + [['clang_format', 'uncrustify'], [], source_code_c.replace('\t', ' '), False], [['pylint', 'mypy'], ["--python-version", python_version()], source_code_python, True], [['pylint'], ["--python-version", python_version()], source_code_python + '\n', False], [['mypy'], ["--python-version", python_version()], source_code_python.replace(': str', ': int'), False], @@ -177,6 +185,9 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c ], ids=[ 'uncrustify_no_issues', 'uncrustify_found_issues', + 'clang_format_no_issues', + 'clang_format_found_issues', + 'clang_format_and_uncrustify_found_issues', 'pylint_and_mypy_both_no_issues', 'pylint_found_issues', 'mypy_found_issues', @@ -194,6 +205,9 @@ def test_code_report_log(runner_with_analyzers: UniversumRunner, analyzers, extr if analyzer == 'uncrustify': args += ["--cfg-file", "cfg"] (runner_with_analyzers.local.root_directory / "cfg").write_text(config_uncrustify) + elif analyzer == 'clang_format': + (runner_with_analyzers.local.root_directory / ".clang-format").write_text(config_clang_format) + config.add_analyzer(analyzer, args) log = runner_with_analyzers.run(config.finalize()) @@ -210,7 +224,7 @@ def test_without_code_report_command(runner_with_analyzers: UniversumRunner): assert not pattern.findall(log) -@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify']) +@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify', 'clang_format']) @pytest.mark.parametrize('arg_set, expected_log', [ [["--files", "source_file.py"], "error: the following arguments are required: --result-file"], [["--files", "source_file.py", "--result-file"], "result-file: expected one argument"], @@ -235,10 +249,12 @@ def test_analyzer_python_version_params(runner_with_analyzers: UniversumRunner, "--result-file", "${CODE_REPORT_FILE}", '--rcfile'], "rcfile: expected one argument"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}"], - "Please specify the '--cfg_file' parameter or set an env. variable 'UNCRUSTIFY_CONFIG'"], + "Please specify the '--cfg-file' parameter or set 'UNCRUSTIFY_CONFIG' environment variable"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--cfg-file", "cfg", "--output-directory", "."], - "Target and source folders for uncrustify are not allowed to match"], + "Target folder must not be identical to source folder"], + ['clang_format', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "."], + "Target folder must not be identical to source folder"], ]) def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyzer, arg_set, expected_log): source_file = runner_with_analyzers.local.root_directory / "source_file" @@ -249,39 +265,54 @@ def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyz assert expected_log in log, f"'{expected_log}' is not found in '{log}'" -@pytest.mark.parametrize('extra_args, tested_content, expected_success, expected_artifact', [ - [[], source_code_c, True, False], - [["--report-html"], source_code_c.replace('\t', ' '), False, True], - [[], source_code_c.replace('\t', ' '), False, False], +@pytest.mark.parametrize('analyzer, extra_args, tested_content, expected_success, expected_artifact', [ + ['uncrustify', ["--report-html"], source_code_c, True, False], + ['uncrustify', ["--report-html"], source_code_c.replace('\t', ' '), False, True], + ['uncrustify', [], source_code_c.replace('\t', ' '), False, False], + ['clang_format', ["--report-html"], source_code_c.replace('\t', ' '), True, False], + ['clang_format', ["--report-html"], source_code_c, False, True], + ['clang_format', [], source_code_c, False, False], ], ids=[ "uncrustify_html_file_not_needed", "uncrustify_html_file_saved", "uncrustify_html_file_disabled", + "clang_format_html_file_not_needed", + "clang_format_html_file_saved", + "clang_format_html_file_disabled", ]) -def test_uncrustify_file_diff(runner_with_analyzers: UniversumRunner, - extra_args, tested_content, expected_success, expected_artifact): +def test_diff_html_file(runner_with_analyzers: UniversumRunner, analyzer, + extra_args, tested_content, expected_success, expected_artifact): + root = runner_with_analyzers.local.root_directory source_file = root / "source_file" source_file.write_text(tested_content) - (root / "cfg").write_text(config_uncrustify) common_args = [ "--result-file", "${CODE_REPORT_FILE}", "--files", "source_file", - "--cfg-file", "cfg", + "--output-directory", "diff_temp" ] + if analyzer == 'uncrustify': + (root / "cfg").write_text(config_uncrustify) + common_args.extend(["--cfg-file", "cfg"]) + elif analyzer == 'clang_format': + (root / ".clang-format").write_text(config_clang_format) + args = common_args + extra_args - extra_config = "artifacts='./uncrustify/source_file.html'" - log = runner_with_analyzers.run(ConfigData().add_analyzer('uncrustify', args, extra_config).finalize()) + extra_config = "artifacts='./diff_temp/source_file.html'" + log = runner_with_analyzers.run(ConfigData().add_analyzer(analyzer, args, extra_config).finalize()) expected_log = log_success if expected_success else log_fail assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'" expected_artifacts_state = "Success" if expected_artifact else "Failed" - expected_log = f"Collecting artifacts for the 'Run uncrustify' step - [^\n]*{expected_artifacts_state}" + expected_log = f"Collecting artifacts for the 'Run {analyzer}' step - [^\n]*{expected_artifacts_state}" assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'" def test_code_report_extended_arg_search(tmp_path: pathlib.Path, stdout_checker: FuzzyCallChecker): + """ + Test if ${CODE_REPORT_FILE} is replaced not only in --result-file argument of the Step + """ env = utils.LocalTestEnvironment(tmp_path, "main") env.settings.Vcs.type = "none" env.settings.LocalMainVcs.source_dir = str(tmp_path) diff --git a/tests/test_nonci.py b/tests/test_nonci.py index fe24168d..d03bb4f0 100644 --- a/tests/test_nonci.py +++ b/tests/test_nonci.py @@ -23,7 +23,7 @@ def test_launcher_output(docker_nonci: UniversumRunner): - version control and review system are not used - project root is set to current directory """ - cwd = str(docker_nonci.local.root_directory) + cwd = str(docker_nonci.local.root_directory.absolute()) artifacts = docker_nonci.artifact_dir file_output_expected = f"Adding file {artifacts}/test_step_log.txt to artifacts" pwd_string_in_logs = f"pwd:[{cwd}]" diff --git a/universum/__init__.py b/universum/__init__.py index 97c18b90..efd4d5e5 100644 --- a/universum/__init__.py +++ b/universum/__init__.py @@ -1,2 +1,2 @@ __title__ = "Universum" -__version__ = "0.19.15" +__version__ = "0.19.16" diff --git a/universum/analyzers/clang_format.py b/universum/analyzers/clang_format.py new file mode 100755 index 00000000..56a64f80 --- /dev/null +++ b/universum/analyzers/clang_format.py @@ -0,0 +1,67 @@ +import argparse +import pathlib +from typing import Callable, List, Optional, Tuple + +import yaml + +from . import utils, diff_utils + + +def clang_format_argument_parser() -> argparse.ArgumentParser: + parser = diff_utils.diff_analyzer_argument_parser("Clang-format analyzer", __file__, "clang-format") + parser.add_argument("--executable", "-e", dest="executable", default="clang-format", + help="The name of the clang-format executable, default: clang-format") + parser.add_argument("--style", dest="style", + help="The 'style' parameter of the clang-format. Can be literal 'file' string or " + "path to real file. See the clang-format documentation for details.") + return parser + + +def _add_style_param_if_present(cmd: List[str], settings: argparse.Namespace) -> None: + if settings.style: + cmd.extend(["-style", settings.style]) + + +@utils.sys_exit +@utils.analyzer(clang_format_argument_parser()) +def main(settings: argparse.Namespace) -> List[utils.ReportData]: + settings.name = "clang-format" + diff_utils.diff_analyzer_common_main(settings) + + html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None + if settings.write_html: + wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings) + html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) + + files: List[Tuple[pathlib.Path, pathlib.Path]] = [] + for src_file_absolute, target_file_absolute, _ in utils.get_files_with_absolute_paths(settings): + files.append((src_file_absolute, target_file_absolute)) + cmd = [settings.executable, src_file_absolute] + _add_style_param_if_present(cmd, settings) + output, _ = utils.run_for_output(cmd) + with open(target_file_absolute, "w", encoding="utf-8") as output_file: + output_file.write(output) + + return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) + + +def _get_wrapcolumn_tabsize(settings: argparse.Namespace) -> Tuple[int, int]: + cmd = [settings.executable, "--dump-config"] + _add_style_param_if_present(cmd, settings) + output, error = utils.run_for_output(cmd) + if error: + raise utils.AnalyzerException(message="clang-format --dump-config failed with the following error output: " + + error) + try: + clang_style = yaml.safe_load(output) + return clang_style["ColumnLimit"], clang_style["IndentWidth"] + except yaml.YAMLError as parse_error: + raise utils.AnalyzerException(message="Parsing of clang-format config produced the following error: " + + str(parse_error)) + except KeyError as key_error: + raise utils.AnalyzerException(message="Cannot find key in yaml during parsing of clang-format config: " + + str(key_error)) + + +if __name__ == "__main__": + main() diff --git a/universum/analyzers/diff_utils.py b/universum/analyzers/diff_utils.py new file mode 100644 index 00000000..493cf7de --- /dev/null +++ b/universum/analyzers/diff_utils.py @@ -0,0 +1,133 @@ +import argparse +import difflib +import pathlib +import shutil +from typing import Callable, List, Optional, Tuple + +from . import utils + + +class HtmlDiffFileWriter: + + def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: + self.target_folder = target_folder + self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) + + def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: + file_relative = file.relative_to(pathlib.Path.cwd()) + out_file_name: str = str(file_relative).replace('/', '_') + '.html' + with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: + out_file.write(self.differ.make_file(src, target, context=False)) + + +DiffWriter = Callable[[pathlib.Path, List[str], List[str]], None] + + +def diff_analyzer_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], + write_diff_file: Optional[DiffWriter] + ) -> List[utils.ReportData]: + result: List[utils.ReportData] = [] + for src_file, dst_file in files: + with open(src_file, encoding="utf-8") as src: + src_lines = src.readlines() + with open(dst_file, encoding="utf-8") as fixed: + fixed_lines = fixed.readlines() + + issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) + if issues and write_diff_file: + write_diff_file(src_file, src_lines, fixed_lines) + result.extend(issues) + return result + + +def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: + result = [] + matching_blocks: List[difflib.Match] = \ + difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() + previous_match = matching_blocks[0] + for match in matching_blocks[1:]: + block = _get_mismatching_block(previous_match, match, src, target) + previous_match = match + if not block: + continue + line, before, after = block + path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) + message = _get_issue_message(before, after) + result.append(utils.ReportData( + symbol="Code Style issue", + message=message, + path=str(path), + line=line + )) + + return result + + +def _get_issue_message(before: str, after: str) -> str: + # The maximum number of lines to write separate comments for + # If exceeded, summarized comment will be provided instead + max_lines = 11 + diff_size = len(before.splitlines()) + if diff_size > max_lines: + message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ + f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" + else: + # Message with before&after + message = f"\nOriginal code:\n```diff\n{before}```\n" + \ + f"Fixed code:\n```diff\n{after}```\n" + return message + + +def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] + match: difflib.Match, + src: List[str], target: List[str] + ) -> Optional[Tuple[int, str, str]]: + previous_match_end_in_src = previous_match.a + previous_match.size + previous_match_end_in_target = previous_match.b + previous_match.size + match_start_in_src = match.a + match_start_in_target = match.b + if previous_match_end_in_src == match_start_in_src: + return None + line = match_start_in_src + before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) + after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) + return line, before, after + + +def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: + return _replace_whitespace_characters(''.join(lines[start: end])) + + +_whitespace_character_mapping = { + " ": "\u00b7", + "\t": "\u2192\u2192\u2192\u2192", + "\n": "\u2193\u000a" +}.items() + + +def _replace_whitespace_characters(line: str) -> str: + for old_str, new_str in _whitespace_character_mapping: + line = line.replace(old_str, new_str) + return line + + +def diff_analyzer_argument_parser(description: str, module_path: str, output_directory: str) -> argparse.ArgumentParser: + parser = utils.create_parser(description, module_path) + parser.add_argument("--output-directory", "-od", dest="output_directory", default=output_directory, + help=f"Directory to store fixed files and HTML files with diff; the default " + f"value is '{output_directory}'. Has to be distinct from source directory") + parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, + help="(optional) Set to generate html reports for each modified file") + return parser + + +def diff_analyzer_common_main(settings: argparse.Namespace) -> None: + settings.target_folder = utils.normalize_path(settings.output_directory) + if settings.target_folder.exists() and settings.target_folder.samefile(pathlib.Path.cwd()): + raise EnvironmentError("Target folder must not be identical to source folder") + + settings.target_folder.mkdir(parents=True, exist_ok=True) + + if not shutil.which(settings.executable): + raise EnvironmentError(f"{settings.name} executable '{settings.executable}' is not found. " + f"Please install {settings.name} or fix the executable name.") diff --git a/universum/analyzers/mypy.py b/universum/analyzers/mypy.py index d5c95e54..a8b0544a 100644 --- a/universum/analyzers/mypy.py +++ b/universum/analyzers/mypy.py @@ -1,12 +1,11 @@ import argparse - from typing import List from . import utils def mypy_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Mypy analyzer") + parser = utils.create_parser("Mypy analyzer", __file__) parser.add_argument("--config-file", dest="config_file", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) return parser diff --git a/universum/analyzers/pylint.py b/universum/analyzers/pylint.py index 7b383f89..11cb8a67 100644 --- a/universum/analyzers/pylint.py +++ b/universum/analyzers/pylint.py @@ -1,13 +1,12 @@ import argparse import json - from typing import List from . import utils def pylint_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Pylint analyzer") + parser = utils.create_parser("Pylint analyzer", __file__) parser.add_argument("--rcfile", dest="rcfile", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) return parser diff --git a/universum/analyzers/uncrustify.py b/universum/analyzers/uncrustify.py index 2f24ca2d..7afc548d 100755 --- a/universum/analyzers/uncrustify.py +++ b/universum/analyzers/uncrustify.py @@ -1,86 +1,44 @@ import argparse -import difflib import os -import shutil import pathlib import re - from typing import Callable, List, Optional, Tuple -from . import utils +from . import utils, diff_utils def uncrustify_argument_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser(description="Uncrustify analyzer") + parser = diff_utils.diff_analyzer_argument_parser("Uncrustify analyzer", __file__, "uncrustify") parser.add_argument("--cfg-file", "-cf", dest="cfg_file", help="Name of the configuration file of Uncrustify; " "can also be set via 'UNCRUSTIFY_CONFIG' env. variable") - parser.add_argument("--output-directory", "-od", dest="output_directory", default="uncrustify", - help="Directory to store fixed files, generated by Uncrustify " - "and HTML files with diff; the default value is 'uncrustify'" - "Has to be distinct from source directory") - parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, - help="(optional) Set to generate html reports for each modified file") return parser @utils.sys_exit @utils.analyzer(uncrustify_argument_parser()) def main(settings: argparse.Namespace) -> List[utils.ReportData]: - if not shutil.which('uncrustify'): - raise EnvironmentError("Please install uncrustify") + settings.name = "uncrustify" + settings.executable = "uncrustify" + diff_utils.diff_analyzer_common_main(settings) + if not settings.cfg_file and 'UNCRUSTIFY_CONFIG' not in os.environ: - raise EnvironmentError("Please specify the '--cfg_file' parameter " - "or set an env. variable 'UNCRUSTIFY_CONFIG'") - target_folder: pathlib.Path = utils.normalize(settings.output_directory) - if target_folder.exists() and target_folder.samefile(pathlib.Path.cwd()): - raise EnvironmentError("Target and source folders for uncrustify are not allowed to match") + raise EnvironmentError("Please specify the '--cfg-file' parameter " + "or set 'UNCRUSTIFY_CONFIG' environment variable") + html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None if settings.write_html: wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings.cfg_file) - html_diff_file_writer = HtmlDiffFileWriter(target_folder, wrapcolumn, tabsize) + html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) cmd = ["uncrustify", "-q", "-c", settings.cfg_file, "--prefix", settings.output_directory] files: List[Tuple[pathlib.Path, pathlib.Path]] = [] - for src_file in settings.file_list: - src_file_absolute = utils.normalize(src_file) - src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) - target_file_absolute: pathlib.Path = target_folder.joinpath(src_file_relative) + for src_file_absolute, target_file_absolute, src_file_relative in utils.get_files_with_absolute_paths(settings): files.append((src_file_absolute, target_file_absolute)) cmd.append(src_file_relative) utils.run_for_output(cmd) - return uncrustify_output_parser(files, html_diff_file_writer) - - -class HtmlDiffFileWriter: - - def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: - self.target_folder = target_folder - self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) - - def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: - file_relative = file.relative_to(pathlib.Path.cwd()) - out_file_name: str = str(file_relative).replace('/', '_') + '.html' - with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: - out_file.write(self.differ.make_file(src, target, context=False)) - - -def uncrustify_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], - write_diff_file: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] - ) -> List[utils.ReportData]: - result: List[utils.ReportData] = [] - for src_file, uncrustify_file in files: - with open(src_file, encoding="utf-8") as src: - src_lines = src.readlines() - with open(uncrustify_file, encoding="utf-8") as fixed: - fixed_lines = fixed.readlines() - - issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) - if issues and write_diff_file: - write_diff_file(src_file, src_lines, fixed_lines) - result.extend(issues) - return result + return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: @@ -99,69 +57,5 @@ def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: return wrapcolumn, tabsize -def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: - result = [] - matching_blocks: List[difflib.Match] = \ - difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() - previous_match = matching_blocks[0] - for match in matching_blocks[1:]: - block = _get_mismatching_block(previous_match, match, src, target) - previous_match = match - if not block: - continue - line, before, after = block - path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) - message = _get_issue_message(before, after) - result.append(utils.ReportData( - symbol="Uncrustify Code Style issue", - message=message, - path=str(path), - line=line - )) - - return result - - -def _get_issue_message(before: str, after: str) -> str: - # The maximum number of lines to write separate comments for - # If exceeded, summarized comment will be provided instead - max_lines = 11 - diff_size = len(before.splitlines()) - if diff_size > max_lines: - message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ - f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" - else: - # Message with before&after - message = f"\nOriginal code:\n```diff\n{before}```\n" + \ - f"Uncrustify generated code:\n```diff\n{after}```\n" - return message - - -def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] - match: difflib.Match, - src: List[str], target: List[str] - ) -> Optional[Tuple[int, str, str]]: - previous_match_end_in_src = previous_match.a + previous_match.size - previous_match_end_in_target = previous_match.b + previous_match.size - match_start_in_src = match.a - match_start_in_target = match.b - if previous_match_end_in_src == match_start_in_src: - return None - line = match_start_in_src - before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) - after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) - return line, before, after - - -def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: - return _replace_invisible_symbols(''.join(lines[start: end])) - - -def _replace_invisible_symbols(line: str) -> str: - for old_str, new_str in zip([" ", "\t", "\n"], ["\u00b7", "\u2192\u2192\u2192\u2192", "\u2193\u000a"]): - line = line.replace(old_str, new_str) - return line - - if __name__ == "__main__": - main() # pylint: disable=no-value-for-parameter # see https://github.com/PyCQA/pylint/issues/259 + main() diff --git a/universum/analyzers/utils.py b/universum/analyzers/utils.py index a6bfd452..7d921709 100644 --- a/universum/analyzers/utils.py +++ b/universum/analyzers/utils.py @@ -1,11 +1,12 @@ -import json -import sys import argparse import glob +import json +import os import pathlib import subprocess +import sys +from typing import Any, Callable, List, Optional, Tuple, Set, Iterable -from typing import Any, Callable, List, Optional, Tuple, Set from typing_extensions import TypedDict from universum.lib.ci_exception import CiException @@ -19,6 +20,13 @@ def __init__(self, code: int = 2, message: Optional[str] = None): self.message: Optional[str] = message +def create_parser(description: str, module_path: str) -> argparse.ArgumentParser: + module_name, _ = os.path.splitext(os.path.basename(module_path)) + + prog = f"python{sys.version_info.major}.{sys.version_info.minor} -m {__package__}.{module_name}" + return argparse.ArgumentParser(prog=prog, description=description) + + def analyzer(parser: argparse.ArgumentParser): """ Wraps the analyzer specific data and adds common protocol information: @@ -29,6 +37,7 @@ def analyzer(parser: argparse.ArgumentParser): :param parser: Definition of analyzer custom arguments :return: Wrapped analyzer with common reporting behaviour """ + def internal(func: Callable[[argparse.Namespace], List[ReportData]]) -> Callable[[], List[ReportData]]: def wrapper() -> List[ReportData]: add_files_argument(parser) @@ -75,6 +84,7 @@ def sys_exit(func: Callable[[], Any]) -> Callable[[], None]: >>> wrap_system_exit(sys_exit(_raise_custom)) 3 """ + def wrapper() -> None: exit_code: int try: @@ -146,6 +156,16 @@ def report_to_file(issues: List[ReportData], json_file: Optional[str] = None) -> sys.stdout.write(issues_json) -def normalize(file: str) -> pathlib.Path: +def normalize_path(file: str) -> pathlib.Path: file_path = pathlib.Path(file) return file_path if file_path.is_absolute() else pathlib.Path.cwd().joinpath(file_path) + + +def get_files_with_absolute_paths(settings: argparse.Namespace) -> Iterable[Tuple[pathlib.Path, + pathlib.Path, + pathlib.Path]]: + for src_file in settings.file_list: + src_file_absolute = normalize_path(src_file) + src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) + target_file_absolute: pathlib.Path = settings.target_folder.joinpath(src_file_relative) + yield src_file_absolute, target_file_absolute, src_file_relative diff --git a/universum/configuration_support.py b/universum/configuration_support.py index 7813c5eb..43831a8c 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -355,7 +355,7 @@ def get(self, key: str, default: Any = None) -> Any: ... warnings.simplefilter("always") ... f() ... return w - >>> step = Step(name='foo', my_var='bar') + >>> step = Step(name='foo', my_var='bar', t1=None, t2=False) >>> do_and_get_warnings(lambda : step.get('name', 'test')) # doctest: +ELLIPSIS [] @@ -367,12 +367,16 @@ def get(self, key: str, default: Any = None) -> Any: 'test' >>> step.get('command', 'test') 'test' + >>> step.get('t1') is None + True + >>> step.get('t2') + False """ result = self._extras.get(key) - if result: + if result is not None: # for custom fields there is a distinction between None and falsy values return result result = self.__dict__.get(key) - if result: + if result: # non-custom fields initialized with falsy values warn("Using legacy API to access configuration values. Please use var." + key + " instead.") return result return default diff --git a/universum/modules/code_report_collector.py b/universum/modules/code_report_collector.py index e558814c..edb2e795 100644 --- a/universum/modules/code_report_collector.py +++ b/universum/modules/code_report_collector.py @@ -92,9 +92,6 @@ def report_code_report_results(self) -> None: if text: report = json.loads(text) - json_file: TextIO = self.artifacts.create_text_file("Static_analysis_report.json") - json_file.write(json.dumps(report, indent=4)) - issue_count: int if not report and report != []: self.out.log_error("There are no results in code report file. Something went wrong.") diff --git a/universum/modules/vcs/swarm.py b/universum/modules/vcs/swarm.py index 53eb8e46..ca671301 100644 --- a/universum/modules/vcs/swarm.py +++ b/universum/modules/vcs/swarm.py @@ -9,7 +9,7 @@ from ...lib.ci_exception import CiException from ...lib.gravity import Dependency -urllib3.disable_warnings((urllib3.exceptions.InsecurePlatformWarning, urllib3.exceptions.SNIMissingWarning)) # type: ignore +urllib3.disable_warnings(urllib3.exceptions.InsecurePlatformWarning) # type: ignore __all__ = [ "Swarm" From 6919d68e3c4ba48827d29a8bfc040c9e7cef2024 Mon Sep 17 00:00:00 2001 From: miltolstoy Date: Thu, 25 May 2023 20:49:39 +0300 Subject: [PATCH 18/18] Revert "chore: merge with master branch (#805)" (#806) This reverts commit 31281a656a976b5cb41635ce821dd48c2d1a6451. --- .github/workflows/telegram-bot.yml | 8 +- CHANGELOG.rst | 17 --- doc/code_report.rst | 42 +------ doc/conf.py | 2 +- pylintrc | 5 +- setup.py | 7 +- tests/deployment_utils.py | 3 +- tests/test_code_report.py | 63 +++------- tests/test_nonci.py | 2 +- universum/__init__.py | 2 +- universum/analyzers/clang_format.py | 67 ----------- universum/analyzers/diff_utils.py | 133 --------------------- universum/analyzers/mypy.py | 3 +- universum/analyzers/pylint.py | 3 +- universum/analyzers/uncrustify.py | 132 ++++++++++++++++++-- universum/analyzers/utils.py | 28 +---- universum/configuration_support.py | 10 +- universum/modules/code_report_collector.py | 3 + universum/modules/vcs/swarm.py | 2 +- 19 files changed, 164 insertions(+), 368 deletions(-) delete mode 100755 universum/analyzers/clang_format.py delete mode 100644 universum/analyzers/diff_utils.py diff --git a/.github/workflows/telegram-bot.yml b/.github/workflows/telegram-bot.yml index c319b739..6d623abc 100644 --- a/.github/workflows/telegram-bot.yml +++ b/.github/workflows/telegram-bot.yml @@ -23,7 +23,6 @@ jobs: PR_MERGED: ${{ github.event.pull_request.merged_by.login }} REVIEW_STATE: ${{ github.event.review.state }} REVIEW_AUTHOR: ${{ github.event.review.user.login }} - REVIEW_COMMENT: ${{ github.event.review.body }} COMMENT_AUTHOR: ${{ github.event.comment.user.login }} COMMENT_URL: ${{ github.event.comment.html_url }} COMMENT_BODY: ${{ github.event.comment.body }} @@ -52,10 +51,7 @@ jobs: elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "changes_requested" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} requested changes for PR#${{ env.PR_NUMBER }}"` - elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" == "commented" ]]; then - ESCAPED_TEXT=`echo -e "${{ env.REVIEW_COMMENT }}"| sed 's/\&/\&/g' | sed 's//\>/g'` - TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} posted the following comment to PR#${{ env.PR_NUMBER }}:\n$ESCAPED_TEXT"` - elif [[ ! -z "${{ github.event.review }}" ]]; then + elif [[ ! -z "${{ github.event.review }}" && "${{ env.REVIEW_STATE }}" != "changes_requested" ]]; then TEXT=`echo -e "${{ env.REVIEW_AUTHOR }} ${{ env.REVIEW_STATE }} PR#${{ env.PR_NUMBER }}"` elif [[ -z "${{ github.event.review }}" && "${{ github.event.action }}" == "submitted" ]]; then TEXT=`echo -e "Due to GitHub Actions bug we cannot identify, who approved PR#${{ env.PR_NUMBER }}"` @@ -67,7 +63,7 @@ jobs: fi if [[ ! -z $TEXT ]]; then - curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" --data-urlencode "disable_web_page_preview=True" \ + curl --get --data-urlencode "chat_id=${{ secrets.TELEGRAM_CHAT_ID }}" \ --data-urlencode "text=$TEXT" --data-urlencode "parse_mode=HTML" $URL fi diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0678da9b..0ea21a2d 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,23 +1,6 @@ Change log ========== -0.19.15 (2023-05-10) --------------------- - -New features -~~~~~~~~~~~~ - -* **analyzer:** code report based on clang-format - -Bug fixes -~~~~~~~~~ - -* **config:** fixed returning "None" instead of "False" for Step custom keys set to "False" -* **artifact:** remove redundant Static_analysis_report.json from artifacts -* **analyzer:** incorrect program name in help -* **report:** set exit code even if reporting crashes - - 0.19.14 (2022-11-14) -------------------- diff --git a/doc/code_report.rst b/doc/code_report.rst index 48317dcf..a9ca4b86 100644 --- a/doc/code_report.rst +++ b/doc/code_report.rst @@ -6,7 +6,6 @@ The following analysing modules (analysers) are currently added to Universum: * `pylint`_ * `mypy`_ * `uncrustify`_ - * `clang-format`_ Analysers are separate scripts, fully compatible with Universum. It is possible to use them as independent Python modules. @@ -63,6 +62,7 @@ Pylint .. argparse:: :ref: universum.analyzers.pylint.pylint_argument_parser + :prog: {python} -m universum.analyzers.pylint Config example for ``universum.analyzers.pylint``: @@ -99,6 +99,7 @@ Mypy .. argparse:: :ref: universum.analyzers.mypy.mypy_argument_parser + :prog: {python} -m universum.analyzers.mypy Config example for ``universum.analyzers.mypy``: @@ -135,6 +136,7 @@ Uncrustify .. argparse:: :ref: universum.analyzers.uncrustify.uncrustify_argument_parser + :prog: {python} -m universum.analyzers.uncrustify :nodefault: Config example for ``universum.analyzers.uncrustify``: @@ -144,7 +146,7 @@ Config example for ``universum.analyzers.uncrustify``: from universum.configuration_support import Configuration, Step configs = Configuration([Step(name="uncrustify", code_report=True, command=[ - "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp/*.c", + "{python}", "-m", "universum.analyzers.uncrustify", "--files", "/home/user/workspace/temp", "--cfg-file", "file_name.cfg", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "uncrustify" ])]) @@ -162,38 +164,4 @@ will produce this list of configurations: .. testoutput:: $ ./.universum.py - [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp/*.c --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] - -Clang-format ------------- - -.. argparse:: - :ref: universum.analyzers.clang_format.clang_format_argument_parser - :nodefault: - -Config example for ``universum.analyzers.clang_format``: - -.. testcode:: - - from universum.configuration_support import Configuration, Step - - configs = Configuration([Step(name="clang-format", code_report=True, command=[ - "{python}", "-m", "universum.analyzers.clang-format", "--files", "/home/user/workspace/temp/*.c", - "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "clang-format", "-e", "clang-format-15", - ])]) - - if __name__ == '__main__': - print(configs.dump()) - -will produce this list of configurations: - -.. testcode:: - :hide: - - print("$ ./.universum.py") - print(configs.dump()) - -.. testoutput:: - - $ ./.universum.py - [{'name': 'clang-format', 'code_report': True, 'command': '{python} -m universum.analyzers.clang-format --files /home/user/workspace/temp/*.c --result-file ${CODE_REPORT_FILE} --output-directory clang-format -e clang-format-15'}] + [{'name': 'uncrustify', 'code_report': True, 'command': '{python} -m universum.analyzers.uncrustify --files /home/user/workspace/temp --cfg-file file_name.cfg --result-file ${CODE_REPORT_FILE} --output-directory uncrustify'}] diff --git a/doc/conf.py b/doc/conf.py index d25e3408..229cdb94 100644 --- a/doc/conf.py +++ b/doc/conf.py @@ -50,7 +50,7 @@ # # This is also used if you do content translation via gettext catalogs. # Usually you set "language" from the command line for these cases. -language = 'en' +language = None # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. diff --git a/pylintrc b/pylintrc index ee326fca..7b39d854 100644 --- a/pylintrc +++ b/pylintrc @@ -37,7 +37,4 @@ max-parents = 10 max-line-length = 130 [SIMILARITIES] -ignore-imports=yes - -[TYPECHECK] -signature-mutators=universum.analyzers.utils.analyzer +ignore-imports=yes \ No newline at end of file diff --git a/setup.py b/setup.py index e5dc59d4..642aa26c 100644 --- a/setup.py +++ b/setup.py @@ -35,8 +35,7 @@ def readme(): 'sh', 'lxml', 'typing-extensions', - 'ansi2html', - 'pyyaml==6.0' + 'ansi2html' ], extras_require={ 'p4': [p4], @@ -57,9 +56,7 @@ def readme(): 'coverage', 'mypy', 'types-requests', - 'selenium==3.141', - 'urllib3==1.26.15', # This is required for selenium-3.141 to work correctly - 'types-PyYAML==6.0' + 'selenium==3.141' ] }, package_data={'': ['*.css', '*.js']} diff --git a/tests/deployment_utils.py b/tests/deployment_utils.py index 32a67194..8d080315 100644 --- a/tests/deployment_utils.py +++ b/tests/deployment_utils.py @@ -42,8 +42,7 @@ def add_bind_dirs(self, directories): if self._container: self.request.raiseerror("Container is already running, no dirs can be bound!") for directory in directories: - absolute_dir = str(pathlib.Path(directory).absolute()) - self._volumes[absolute_dir] = {'bind': absolute_dir, 'mode': 'rw'} + self._volumes[directory] = {'bind': directory, 'mode': 'rw'} def add_environment_variables(self, variables): if self._container: diff --git a/tests/test_code_report.py b/tests/test_code_report.py index f73cd2db..9e57306c 100644 --- a/tests/test_code_report.py +++ b/tests/test_code_report.py @@ -16,7 +16,7 @@ def fixture_runner_with_analyzers(docker_main: UniversumRunner): docker_main.environment.install_python_module("pylint") docker_main.environment.install_python_module("mypy") - docker_main.environment.assert_successful_execution("apt install -y uncrustify clang-format") + docker_main.environment.assert_successful_execution("apt install uncrustify") yield docker_main @@ -128,11 +128,6 @@ def finalize(self) -> str: input_tab_size = 2 """ -config_clang_format = """ ---- -AllowShortFunctionsOnASingleLine: Empty -""" - log_fail = r'Found [0-9]+ issues' log_success = r'Issues not found' @@ -172,10 +167,7 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c @pytest.mark.parametrize('analyzers, extra_args, tested_content, expected_success', [ [['uncrustify'], [], source_code_c, True], - [['uncrustify'], [], source_code_c.replace('\t', ' '), False], # by default uncrustify converts spaces to tabs - [['clang_format'], [], source_code_c.replace('\t', ' '), True], # by default clang-format expands tabs to 2 spaces - [['clang_format'], [], source_code_c.replace('\t', ' '), False], - [['clang_format', 'uncrustify'], [], source_code_c.replace('\t', ' '), False], + [['uncrustify'], [], source_code_c.replace('\t', ' '), False], [['pylint', 'mypy'], ["--python-version", python_version()], source_code_python, True], [['pylint'], ["--python-version", python_version()], source_code_python + '\n', False], [['mypy'], ["--python-version", python_version()], source_code_python.replace(': str', ': int'), False], @@ -185,9 +177,6 @@ def test_code_report_direct_log(runner_with_analyzers: UniversumRunner, tested_c ], ids=[ 'uncrustify_no_issues', 'uncrustify_found_issues', - 'clang_format_no_issues', - 'clang_format_found_issues', - 'clang_format_and_uncrustify_found_issues', 'pylint_and_mypy_both_no_issues', 'pylint_found_issues', 'mypy_found_issues', @@ -205,9 +194,6 @@ def test_code_report_log(runner_with_analyzers: UniversumRunner, analyzers, extr if analyzer == 'uncrustify': args += ["--cfg-file", "cfg"] (runner_with_analyzers.local.root_directory / "cfg").write_text(config_uncrustify) - elif analyzer == 'clang_format': - (runner_with_analyzers.local.root_directory / ".clang-format").write_text(config_clang_format) - config.add_analyzer(analyzer, args) log = runner_with_analyzers.run(config.finalize()) @@ -224,7 +210,7 @@ def test_without_code_report_command(runner_with_analyzers: UniversumRunner): assert not pattern.findall(log) -@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify', 'clang_format']) +@pytest.mark.parametrize('analyzer', ['pylint', 'mypy', 'uncrustify']) @pytest.mark.parametrize('arg_set, expected_log', [ [["--files", "source_file.py"], "error: the following arguments are required: --result-file"], [["--files", "source_file.py", "--result-file"], "result-file: expected one argument"], @@ -249,12 +235,10 @@ def test_analyzer_python_version_params(runner_with_analyzers: UniversumRunner, "--result-file", "${CODE_REPORT_FILE}", '--rcfile'], "rcfile: expected one argument"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}"], - "Please specify the '--cfg-file' parameter or set 'UNCRUSTIFY_CONFIG' environment variable"], + "Please specify the '--cfg_file' parameter or set an env. variable 'UNCRUSTIFY_CONFIG'"], ['uncrustify', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--cfg-file", "cfg", "--output-directory", "."], - "Target folder must not be identical to source folder"], - ['clang_format', ["--files", "source_file", "--result-file", "${CODE_REPORT_FILE}", "--output-directory", "."], - "Target folder must not be identical to source folder"], + "Target and source folders for uncrustify are not allowed to match"], ]) def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyzer, arg_set, expected_log): source_file = runner_with_analyzers.local.root_directory / "source_file" @@ -265,54 +249,39 @@ def test_analyzer_specific_params(runner_with_analyzers: UniversumRunner, analyz assert expected_log in log, f"'{expected_log}' is not found in '{log}'" -@pytest.mark.parametrize('analyzer, extra_args, tested_content, expected_success, expected_artifact', [ - ['uncrustify', ["--report-html"], source_code_c, True, False], - ['uncrustify', ["--report-html"], source_code_c.replace('\t', ' '), False, True], - ['uncrustify', [], source_code_c.replace('\t', ' '), False, False], - ['clang_format', ["--report-html"], source_code_c.replace('\t', ' '), True, False], - ['clang_format', ["--report-html"], source_code_c, False, True], - ['clang_format', [], source_code_c, False, False], +@pytest.mark.parametrize('extra_args, tested_content, expected_success, expected_artifact', [ + [[], source_code_c, True, False], + [["--report-html"], source_code_c.replace('\t', ' '), False, True], + [[], source_code_c.replace('\t', ' '), False, False], ], ids=[ "uncrustify_html_file_not_needed", "uncrustify_html_file_saved", "uncrustify_html_file_disabled", - "clang_format_html_file_not_needed", - "clang_format_html_file_saved", - "clang_format_html_file_disabled", ]) -def test_diff_html_file(runner_with_analyzers: UniversumRunner, analyzer, - extra_args, tested_content, expected_success, expected_artifact): - +def test_uncrustify_file_diff(runner_with_analyzers: UniversumRunner, + extra_args, tested_content, expected_success, expected_artifact): root = runner_with_analyzers.local.root_directory source_file = root / "source_file" source_file.write_text(tested_content) + (root / "cfg").write_text(config_uncrustify) common_args = [ "--result-file", "${CODE_REPORT_FILE}", "--files", "source_file", - "--output-directory", "diff_temp" + "--cfg-file", "cfg", ] - if analyzer == 'uncrustify': - (root / "cfg").write_text(config_uncrustify) - common_args.extend(["--cfg-file", "cfg"]) - elif analyzer == 'clang_format': - (root / ".clang-format").write_text(config_clang_format) - args = common_args + extra_args - extra_config = "artifacts='./diff_temp/source_file.html'" - log = runner_with_analyzers.run(ConfigData().add_analyzer(analyzer, args, extra_config).finalize()) + extra_config = "artifacts='./uncrustify/source_file.html'" + log = runner_with_analyzers.run(ConfigData().add_analyzer('uncrustify', args, extra_config).finalize()) expected_log = log_success if expected_success else log_fail assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'" expected_artifacts_state = "Success" if expected_artifact else "Failed" - expected_log = f"Collecting artifacts for the 'Run {analyzer}' step - [^\n]*{expected_artifacts_state}" + expected_log = f"Collecting artifacts for the 'Run uncrustify' step - [^\n]*{expected_artifacts_state}" assert re.findall(expected_log, log), f"'{expected_log}' is not found in '{log}'" def test_code_report_extended_arg_search(tmp_path: pathlib.Path, stdout_checker: FuzzyCallChecker): - """ - Test if ${CODE_REPORT_FILE} is replaced not only in --result-file argument of the Step - """ env = utils.LocalTestEnvironment(tmp_path, "main") env.settings.Vcs.type = "none" env.settings.LocalMainVcs.source_dir = str(tmp_path) diff --git a/tests/test_nonci.py b/tests/test_nonci.py index d03bb4f0..fe24168d 100644 --- a/tests/test_nonci.py +++ b/tests/test_nonci.py @@ -23,7 +23,7 @@ def test_launcher_output(docker_nonci: UniversumRunner): - version control and review system are not used - project root is set to current directory """ - cwd = str(docker_nonci.local.root_directory.absolute()) + cwd = str(docker_nonci.local.root_directory) artifacts = docker_nonci.artifact_dir file_output_expected = f"Adding file {artifacts}/test_step_log.txt to artifacts" pwd_string_in_logs = f"pwd:[{cwd}]" diff --git a/universum/__init__.py b/universum/__init__.py index efd4d5e5..97c18b90 100644 --- a/universum/__init__.py +++ b/universum/__init__.py @@ -1,2 +1,2 @@ __title__ = "Universum" -__version__ = "0.19.16" +__version__ = "0.19.15" diff --git a/universum/analyzers/clang_format.py b/universum/analyzers/clang_format.py deleted file mode 100755 index 56a64f80..00000000 --- a/universum/analyzers/clang_format.py +++ /dev/null @@ -1,67 +0,0 @@ -import argparse -import pathlib -from typing import Callable, List, Optional, Tuple - -import yaml - -from . import utils, diff_utils - - -def clang_format_argument_parser() -> argparse.ArgumentParser: - parser = diff_utils.diff_analyzer_argument_parser("Clang-format analyzer", __file__, "clang-format") - parser.add_argument("--executable", "-e", dest="executable", default="clang-format", - help="The name of the clang-format executable, default: clang-format") - parser.add_argument("--style", dest="style", - help="The 'style' parameter of the clang-format. Can be literal 'file' string or " - "path to real file. See the clang-format documentation for details.") - return parser - - -def _add_style_param_if_present(cmd: List[str], settings: argparse.Namespace) -> None: - if settings.style: - cmd.extend(["-style", settings.style]) - - -@utils.sys_exit -@utils.analyzer(clang_format_argument_parser()) -def main(settings: argparse.Namespace) -> List[utils.ReportData]: - settings.name = "clang-format" - diff_utils.diff_analyzer_common_main(settings) - - html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None - if settings.write_html: - wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings) - html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) - - files: List[Tuple[pathlib.Path, pathlib.Path]] = [] - for src_file_absolute, target_file_absolute, _ in utils.get_files_with_absolute_paths(settings): - files.append((src_file_absolute, target_file_absolute)) - cmd = [settings.executable, src_file_absolute] - _add_style_param_if_present(cmd, settings) - output, _ = utils.run_for_output(cmd) - with open(target_file_absolute, "w", encoding="utf-8") as output_file: - output_file.write(output) - - return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) - - -def _get_wrapcolumn_tabsize(settings: argparse.Namespace) -> Tuple[int, int]: - cmd = [settings.executable, "--dump-config"] - _add_style_param_if_present(cmd, settings) - output, error = utils.run_for_output(cmd) - if error: - raise utils.AnalyzerException(message="clang-format --dump-config failed with the following error output: " + - error) - try: - clang_style = yaml.safe_load(output) - return clang_style["ColumnLimit"], clang_style["IndentWidth"] - except yaml.YAMLError as parse_error: - raise utils.AnalyzerException(message="Parsing of clang-format config produced the following error: " + - str(parse_error)) - except KeyError as key_error: - raise utils.AnalyzerException(message="Cannot find key in yaml during parsing of clang-format config: " + - str(key_error)) - - -if __name__ == "__main__": - main() diff --git a/universum/analyzers/diff_utils.py b/universum/analyzers/diff_utils.py deleted file mode 100644 index 493cf7de..00000000 --- a/universum/analyzers/diff_utils.py +++ /dev/null @@ -1,133 +0,0 @@ -import argparse -import difflib -import pathlib -import shutil -from typing import Callable, List, Optional, Tuple - -from . import utils - - -class HtmlDiffFileWriter: - - def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: - self.target_folder = target_folder - self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) - - def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: - file_relative = file.relative_to(pathlib.Path.cwd()) - out_file_name: str = str(file_relative).replace('/', '_') + '.html' - with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: - out_file.write(self.differ.make_file(src, target, context=False)) - - -DiffWriter = Callable[[pathlib.Path, List[str], List[str]], None] - - -def diff_analyzer_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], - write_diff_file: Optional[DiffWriter] - ) -> List[utils.ReportData]: - result: List[utils.ReportData] = [] - for src_file, dst_file in files: - with open(src_file, encoding="utf-8") as src: - src_lines = src.readlines() - with open(dst_file, encoding="utf-8") as fixed: - fixed_lines = fixed.readlines() - - issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) - if issues and write_diff_file: - write_diff_file(src_file, src_lines, fixed_lines) - result.extend(issues) - return result - - -def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: - result = [] - matching_blocks: List[difflib.Match] = \ - difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() - previous_match = matching_blocks[0] - for match in matching_blocks[1:]: - block = _get_mismatching_block(previous_match, match, src, target) - previous_match = match - if not block: - continue - line, before, after = block - path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) - message = _get_issue_message(before, after) - result.append(utils.ReportData( - symbol="Code Style issue", - message=message, - path=str(path), - line=line - )) - - return result - - -def _get_issue_message(before: str, after: str) -> str: - # The maximum number of lines to write separate comments for - # If exceeded, summarized comment will be provided instead - max_lines = 11 - diff_size = len(before.splitlines()) - if diff_size > max_lines: - message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ - f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" - else: - # Message with before&after - message = f"\nOriginal code:\n```diff\n{before}```\n" + \ - f"Fixed code:\n```diff\n{after}```\n" - return message - - -def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] - match: difflib.Match, - src: List[str], target: List[str] - ) -> Optional[Tuple[int, str, str]]: - previous_match_end_in_src = previous_match.a + previous_match.size - previous_match_end_in_target = previous_match.b + previous_match.size - match_start_in_src = match.a - match_start_in_target = match.b - if previous_match_end_in_src == match_start_in_src: - return None - line = match_start_in_src - before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) - after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) - return line, before, after - - -def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: - return _replace_whitespace_characters(''.join(lines[start: end])) - - -_whitespace_character_mapping = { - " ": "\u00b7", - "\t": "\u2192\u2192\u2192\u2192", - "\n": "\u2193\u000a" -}.items() - - -def _replace_whitespace_characters(line: str) -> str: - for old_str, new_str in _whitespace_character_mapping: - line = line.replace(old_str, new_str) - return line - - -def diff_analyzer_argument_parser(description: str, module_path: str, output_directory: str) -> argparse.ArgumentParser: - parser = utils.create_parser(description, module_path) - parser.add_argument("--output-directory", "-od", dest="output_directory", default=output_directory, - help=f"Directory to store fixed files and HTML files with diff; the default " - f"value is '{output_directory}'. Has to be distinct from source directory") - parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, - help="(optional) Set to generate html reports for each modified file") - return parser - - -def diff_analyzer_common_main(settings: argparse.Namespace) -> None: - settings.target_folder = utils.normalize_path(settings.output_directory) - if settings.target_folder.exists() and settings.target_folder.samefile(pathlib.Path.cwd()): - raise EnvironmentError("Target folder must not be identical to source folder") - - settings.target_folder.mkdir(parents=True, exist_ok=True) - - if not shutil.which(settings.executable): - raise EnvironmentError(f"{settings.name} executable '{settings.executable}' is not found. " - f"Please install {settings.name} or fix the executable name.") diff --git a/universum/analyzers/mypy.py b/universum/analyzers/mypy.py index a8b0544a..d5c95e54 100644 --- a/universum/analyzers/mypy.py +++ b/universum/analyzers/mypy.py @@ -1,11 +1,12 @@ import argparse + from typing import List from . import utils def mypy_argument_parser() -> argparse.ArgumentParser: - parser = utils.create_parser("Mypy analyzer", __file__) + parser = argparse.ArgumentParser(description="Mypy analyzer") parser.add_argument("--config-file", dest="config_file", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) return parser diff --git a/universum/analyzers/pylint.py b/universum/analyzers/pylint.py index 11cb8a67..7b383f89 100644 --- a/universum/analyzers/pylint.py +++ b/universum/analyzers/pylint.py @@ -1,12 +1,13 @@ import argparse import json + from typing import List from . import utils def pylint_argument_parser() -> argparse.ArgumentParser: - parser = utils.create_parser("Pylint analyzer", __file__) + parser = argparse.ArgumentParser(description="Pylint analyzer") parser.add_argument("--rcfile", dest="rcfile", type=str, help="Specify a configuration file.") utils.add_python_version_argument(parser) return parser diff --git a/universum/analyzers/uncrustify.py b/universum/analyzers/uncrustify.py index 7afc548d..2f24ca2d 100755 --- a/universum/analyzers/uncrustify.py +++ b/universum/analyzers/uncrustify.py @@ -1,44 +1,86 @@ import argparse +import difflib import os +import shutil import pathlib import re + from typing import Callable, List, Optional, Tuple -from . import utils, diff_utils +from . import utils def uncrustify_argument_parser() -> argparse.ArgumentParser: - parser = diff_utils.diff_analyzer_argument_parser("Uncrustify analyzer", __file__, "uncrustify") + parser = argparse.ArgumentParser(description="Uncrustify analyzer") parser.add_argument("--cfg-file", "-cf", dest="cfg_file", help="Name of the configuration file of Uncrustify; " "can also be set via 'UNCRUSTIFY_CONFIG' env. variable") + parser.add_argument("--output-directory", "-od", dest="output_directory", default="uncrustify", + help="Directory to store fixed files, generated by Uncrustify " + "and HTML files with diff; the default value is 'uncrustify'" + "Has to be distinct from source directory") + parser.add_argument("--report-html", dest="write_html", action="store_true", default=False, + help="(optional) Set to generate html reports for each modified file") return parser @utils.sys_exit @utils.analyzer(uncrustify_argument_parser()) def main(settings: argparse.Namespace) -> List[utils.ReportData]: - settings.name = "uncrustify" - settings.executable = "uncrustify" - diff_utils.diff_analyzer_common_main(settings) - + if not shutil.which('uncrustify'): + raise EnvironmentError("Please install uncrustify") if not settings.cfg_file and 'UNCRUSTIFY_CONFIG' not in os.environ: - raise EnvironmentError("Please specify the '--cfg-file' parameter " - "or set 'UNCRUSTIFY_CONFIG' environment variable") - + raise EnvironmentError("Please specify the '--cfg_file' parameter " + "or set an env. variable 'UNCRUSTIFY_CONFIG'") + target_folder: pathlib.Path = utils.normalize(settings.output_directory) + if target_folder.exists() and target_folder.samefile(pathlib.Path.cwd()): + raise EnvironmentError("Target and source folders for uncrustify are not allowed to match") html_diff_file_writer: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] = None if settings.write_html: wrapcolumn, tabsize = _get_wrapcolumn_tabsize(settings.cfg_file) - html_diff_file_writer = diff_utils.HtmlDiffFileWriter(settings.target_folder, wrapcolumn, tabsize) + html_diff_file_writer = HtmlDiffFileWriter(target_folder, wrapcolumn, tabsize) cmd = ["uncrustify", "-q", "-c", settings.cfg_file, "--prefix", settings.output_directory] files: List[Tuple[pathlib.Path, pathlib.Path]] = [] - for src_file_absolute, target_file_absolute, src_file_relative in utils.get_files_with_absolute_paths(settings): + for src_file in settings.file_list: + src_file_absolute = utils.normalize(src_file) + src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) + target_file_absolute: pathlib.Path = target_folder.joinpath(src_file_relative) files.append((src_file_absolute, target_file_absolute)) cmd.append(src_file_relative) utils.run_for_output(cmd) - return diff_utils.diff_analyzer_output_parser(files, html_diff_file_writer) + return uncrustify_output_parser(files, html_diff_file_writer) + + +class HtmlDiffFileWriter: + + def __init__(self, target_folder: pathlib.Path, wrapcolumn: int, tabsize: int) -> None: + self.target_folder = target_folder + self.differ = difflib.HtmlDiff(tabsize=tabsize, wrapcolumn=wrapcolumn) + + def __call__(self, file: pathlib.Path, src: List[str], target: List[str]) -> None: + file_relative = file.relative_to(pathlib.Path.cwd()) + out_file_name: str = str(file_relative).replace('/', '_') + '.html' + with open(self.target_folder.joinpath(out_file_name), 'w', encoding="utf-8") as out_file: + out_file.write(self.differ.make_file(src, target, context=False)) + + +def uncrustify_output_parser(files: List[Tuple[pathlib.Path, pathlib.Path]], + write_diff_file: Optional[Callable[[pathlib.Path, List[str], List[str]], None]] + ) -> List[utils.ReportData]: + result: List[utils.ReportData] = [] + for src_file, uncrustify_file in files: + with open(src_file, encoding="utf-8") as src: + src_lines = src.readlines() + with open(uncrustify_file, encoding="utf-8") as fixed: + fixed_lines = fixed.readlines() + + issues = _get_issues_from_diff(src_file, src_lines, fixed_lines) + if issues and write_diff_file: + write_diff_file(src_file, src_lines, fixed_lines) + result.extend(issues) + return result def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: @@ -57,5 +99,69 @@ def _get_wrapcolumn_tabsize(cfg_file: str) -> Tuple[int, int]: return wrapcolumn, tabsize +def _get_issues_from_diff(src_file: pathlib.Path, src: List[str], target: List[str]) -> List[utils.ReportData]: + result = [] + matching_blocks: List[difflib.Match] = \ + difflib.SequenceMatcher(a=src, b=target).get_matching_blocks() + previous_match = matching_blocks[0] + for match in matching_blocks[1:]: + block = _get_mismatching_block(previous_match, match, src, target) + previous_match = match + if not block: + continue + line, before, after = block + path: pathlib.Path = src_file.relative_to(pathlib.Path.cwd()) + message = _get_issue_message(before, after) + result.append(utils.ReportData( + symbol="Uncrustify Code Style issue", + message=message, + path=str(path), + line=line + )) + + return result + + +def _get_issue_message(before: str, after: str) -> str: + # The maximum number of lines to write separate comments for + # If exceeded, summarized comment will be provided instead + max_lines = 11 + diff_size = len(before.splitlines()) + if diff_size > max_lines: + message = f"\nLarge block of code ({diff_size} lines) has issues\n" + \ + f"Non-compliant code blocks exceeding {max_lines} lines are not reported\n" + else: + # Message with before&after + message = f"\nOriginal code:\n```diff\n{before}```\n" + \ + f"Uncrustify generated code:\n```diff\n{after}```\n" + return message + + +def _get_mismatching_block(previous_match: difflib.Match, # src[a:a+size] = target[b:b+size] + match: difflib.Match, + src: List[str], target: List[str] + ) -> Optional[Tuple[int, str, str]]: + previous_match_end_in_src = previous_match.a + previous_match.size + previous_match_end_in_target = previous_match.b + previous_match.size + match_start_in_src = match.a + match_start_in_target = match.b + if previous_match_end_in_src == match_start_in_src: + return None + line = match_start_in_src + before = _get_text_for_block(previous_match_end_in_src - 1, match_start_in_src, src) + after = _get_text_for_block(previous_match_end_in_target - 1, match_start_in_target, target) + return line, before, after + + +def _get_text_for_block(start: int, end: int, lines: List[str]) -> str: + return _replace_invisible_symbols(''.join(lines[start: end])) + + +def _replace_invisible_symbols(line: str) -> str: + for old_str, new_str in zip([" ", "\t", "\n"], ["\u00b7", "\u2192\u2192\u2192\u2192", "\u2193\u000a"]): + line = line.replace(old_str, new_str) + return line + + if __name__ == "__main__": - main() + main() # pylint: disable=no-value-for-parameter # see https://github.com/PyCQA/pylint/issues/259 diff --git a/universum/analyzers/utils.py b/universum/analyzers/utils.py index 7d921709..a6bfd452 100644 --- a/universum/analyzers/utils.py +++ b/universum/analyzers/utils.py @@ -1,12 +1,11 @@ +import json +import sys import argparse import glob -import json -import os import pathlib import subprocess -import sys -from typing import Any, Callable, List, Optional, Tuple, Set, Iterable +from typing import Any, Callable, List, Optional, Tuple, Set from typing_extensions import TypedDict from universum.lib.ci_exception import CiException @@ -20,13 +19,6 @@ def __init__(self, code: int = 2, message: Optional[str] = None): self.message: Optional[str] = message -def create_parser(description: str, module_path: str) -> argparse.ArgumentParser: - module_name, _ = os.path.splitext(os.path.basename(module_path)) - - prog = f"python{sys.version_info.major}.{sys.version_info.minor} -m {__package__}.{module_name}" - return argparse.ArgumentParser(prog=prog, description=description) - - def analyzer(parser: argparse.ArgumentParser): """ Wraps the analyzer specific data and adds common protocol information: @@ -37,7 +29,6 @@ def analyzer(parser: argparse.ArgumentParser): :param parser: Definition of analyzer custom arguments :return: Wrapped analyzer with common reporting behaviour """ - def internal(func: Callable[[argparse.Namespace], List[ReportData]]) -> Callable[[], List[ReportData]]: def wrapper() -> List[ReportData]: add_files_argument(parser) @@ -84,7 +75,6 @@ def sys_exit(func: Callable[[], Any]) -> Callable[[], None]: >>> wrap_system_exit(sys_exit(_raise_custom)) 3 """ - def wrapper() -> None: exit_code: int try: @@ -156,16 +146,6 @@ def report_to_file(issues: List[ReportData], json_file: Optional[str] = None) -> sys.stdout.write(issues_json) -def normalize_path(file: str) -> pathlib.Path: +def normalize(file: str) -> pathlib.Path: file_path = pathlib.Path(file) return file_path if file_path.is_absolute() else pathlib.Path.cwd().joinpath(file_path) - - -def get_files_with_absolute_paths(settings: argparse.Namespace) -> Iterable[Tuple[pathlib.Path, - pathlib.Path, - pathlib.Path]]: - for src_file in settings.file_list: - src_file_absolute = normalize_path(src_file) - src_file_relative = src_file_absolute.relative_to(pathlib.Path.cwd()) - target_file_absolute: pathlib.Path = settings.target_folder.joinpath(src_file_relative) - yield src_file_absolute, target_file_absolute, src_file_relative diff --git a/universum/configuration_support.py b/universum/configuration_support.py index 43831a8c..7813c5eb 100644 --- a/universum/configuration_support.py +++ b/universum/configuration_support.py @@ -355,7 +355,7 @@ def get(self, key: str, default: Any = None) -> Any: ... warnings.simplefilter("always") ... f() ... return w - >>> step = Step(name='foo', my_var='bar', t1=None, t2=False) + >>> step = Step(name='foo', my_var='bar') >>> do_and_get_warnings(lambda : step.get('name', 'test')) # doctest: +ELLIPSIS [] @@ -367,16 +367,12 @@ def get(self, key: str, default: Any = None) -> Any: 'test' >>> step.get('command', 'test') 'test' - >>> step.get('t1') is None - True - >>> step.get('t2') - False """ result = self._extras.get(key) - if result is not None: # for custom fields there is a distinction between None and falsy values + if result: return result result = self.__dict__.get(key) - if result: # non-custom fields initialized with falsy values + if result: warn("Using legacy API to access configuration values. Please use var." + key + " instead.") return result return default diff --git a/universum/modules/code_report_collector.py b/universum/modules/code_report_collector.py index edb2e795..e558814c 100644 --- a/universum/modules/code_report_collector.py +++ b/universum/modules/code_report_collector.py @@ -92,6 +92,9 @@ def report_code_report_results(self) -> None: if text: report = json.loads(text) + json_file: TextIO = self.artifacts.create_text_file("Static_analysis_report.json") + json_file.write(json.dumps(report, indent=4)) + issue_count: int if not report and report != []: self.out.log_error("There are no results in code report file. Something went wrong.") diff --git a/universum/modules/vcs/swarm.py b/universum/modules/vcs/swarm.py index ca671301..53eb8e46 100644 --- a/universum/modules/vcs/swarm.py +++ b/universum/modules/vcs/swarm.py @@ -9,7 +9,7 @@ from ...lib.ci_exception import CiException from ...lib.gravity import Dependency -urllib3.disable_warnings(urllib3.exceptions.InsecurePlatformWarning) # type: ignore +urllib3.disable_warnings((urllib3.exceptions.InsecurePlatformWarning, urllib3.exceptions.SNIMissingWarning)) # type: ignore __all__ = [ "Swarm"