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/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. 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_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/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 bf798af8..43831a8c 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. @@ -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(): @@ -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/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/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/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 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"