Merge pull request #69 from testable-eu/repair-pattern

repair pattern

docs/How-to-run-CLI-Usage.md

@@ -22,7 +22,8 @@ The following main commands are currently implemented:
   - [`discovery`](./How-to-run-discover-measured-patterns.md): discover measured patterns within a project source code
   - [`manual-discovery`](./How-to-run-manual-discovery.md): execute discovery rules (normally associated to patterns) within a project source code
 - reporting: create reports about SAST measurement and/or pattern discovery (**CONTINUE**)
-  - [`sastreport`](./How-to-run-sastreport.md): fetch last SAST measurements for tools against patterns and aggregate in a common csv file 
+  - [`sastreport`](./How-to-run-sastreport.md): fetch last SAST measurements for tools against patterns and aggregate in a common csv file
+- [`patternrepair`](./How-to-run-patternrepair.md): Can repair a pattern in your pattern library, i.e. checks the JSON file, creates a README file etc.
 
 The following are under-investigation:
 

docs/How-to-run-patternrepair.md

@@ -0,0 +1,70 @@
+# How to run: Pattern repair
+
+## Overview
+
+This command can be used to repair a pattern in your library. At the moment this is only supported for PHP.
+
+## Command line
+
+To repair a pattern use:
+
+```text
+usage: tpframework [OPTIONS] COMMAND patternrepair [-h] -l LANGUAGE (-p PATTERN_ID [PATTERN_ID ...] | --pattern-range RANGE_START-RANGE_END | -a) [--tp-lib TP_LIB_DIR]
+                                                   [--output-dir OUTPUT_DIR] [--masking-file MASKING_FILE] [--measurement-results MEASUREMENT_DIR]
+                                                   [--checkdiscoveryrules-results CHECKDISCOVERYRULES_FILE] [--skip-readme]
+
+options:
+  -h, --help            show this help message and exit
+  -l LANGUAGE, --language LANGUAGE
+                        Programming language targeted
+  -p PATTERN_ID [PATTERN_ID ...], --patterns PATTERN_ID [PATTERN_ID ...]
+                        Specify pattern(s) ID(s) to test for discovery
+  --pattern-range RANGE_START-RANGE_END
+                        Specify pattern ID range separated by`-` (ex. 10-50)
+  -a, --all-patterns    Test discovery for all available patterns
+  --tp-lib TP_LIB_DIR   Absolute path to alternative pattern library, default resolves to `./testability_patterns`
+  --output-dir OUTPUT_DIR
+                        Absolute path to the folder where outcomes (e.g., log file, export file if any) will be stored, default resolves to `./out`
+  --masking-file MASKING_FILE
+                        Absolute path to a json file, that contains a mapping, if the name for some measurement tools should be kept secret, default is None
+  --measurement-results MEASUREMENT_DIR
+                        Absolute path to the folder where measurement results are stored, default resolves to `./measurements`
+  --checkdiscoveryrules-results CHECKDISCOVERYRULES_FILE
+                        Absolute path to the csv file, where the results of the `checkdiscoveryrules` command are stored, default resolves to `./checkdiscoveryrules.csv`
+  --skip-readme         If set, the README generation is skipped.
+```
+
+By default, the `patternrepair` will create a README file for a pattern, where an overview of the pattern is presented together with some measurement results, if available.
+For the generation of the REAMDE, there are a few files mandatory:
+First of all, there has to be a csv file, that contains the results of the `checkdiscoveryrules` command for the patterns, that should be repaired.
+Second, the results of the `measurement` command in a directory, structured similary to the pattern library.
+Additionally you can provide a masking file, that can be used to mask the names of tools used for `measurement`.
+The masking file should be a JSON file of the format `{<real_tool_name>: <masked_tool_name>}`.
+
+If `--skip-readme` is set, None of the files is required and no new README file will be generated.
+
+## Example
+
+`tpframework patternrepair -l php -p 1 --skip-readme`
+
+This command will take a look at PHP pattern 1 and tries to repair it, without generating a new README file.
+During that process it might provide you some feedback about files, that need manual review.
+The tool checks for the following things:
+
+- make sure, a pattern JSON file exists
+- ensure all relative links are correct
+- collect all instances within the pattern path (an instance is identified by a directory, that contains a JSON file in the instance format)
+- make sure the pattern name is correct (therefor the pattern name is derived from the directory name)
+- check the description field and warn if there is no description
+- check the given tags
+- validates the pattern json against the pattern json scheme
+- for each instance, repairing means:
+  - ensuring a instance JSON file with the required keys is available
+  - ensures all relative links exist
+  - check the scala rule if exists and iff necessary adjust the variable names
+  - check the description and again warn if there is no description provided
+  - checks that the field `expectation:expectation` is the opposite of `properties:negative_test_case`
+  - validates the instance json against the instance json scheme
+  - for PHP patterns:
+    - generates new opcode for each php file
+    - changes source line and sink line in the pattern JSON, according to the comments `// source`, `// sink` in the php file

qualitytests/cli/test_interface.py

@@ -1,5 +1,6 @@
 from pathlib import Path
 from typing import Dict
+from unittest.mock import patch, call
 import json
 import sys
 
@@ -13,10 +14,9 @@
 
 from qualitytests.qualitytests_utils import join_resources_path, create_mock_cpg, \
     get_result_output_dir, get_logfile_path, in_logfile, init_measure_test, \
-    init_sastreport_test, init_test
+    init_sastreport_test, init_test, create_pattern
 
 
-@pytest.mark.asyncio
 class TestInterface:
 
 
@@ -253,3 +253,45 @@ def test_check_discovery_rules_3(self, tmp_path, capsys, mocker):
         logfile = get_logfile_path(captured_out_lines)
         assert logfile and logfile.is_file()
 
+
+    def test_repair_patterns_not_including_readme(self):
+        sample_tp_lib = join_resources_path("sample_patlib")
+        test_pattern = create_pattern()
+        with patch("core.pattern.Pattern.init_from_id_and_language") as init_pattern_mock, \
+            patch("core.pattern.Pattern.repair") as patternrepair_mock, \
+            patch("core.utils.check_file_exist") as check_file_exists_mock, \
+            patch("core.utils.check_measurement_results_exist") as measurement_result_exist_mock, \
+            patch("pathlib.Path.mkdir") as mkdir_mock:
+            init_pattern_mock.return_value = test_pattern
+            interface.repair_patterns("JS", [1,2,3], None, True, Path("measurements"), Path("dr_results.csv"), Path("out"), sample_tp_lib)
+
+        patternrepair_mock.assert_called_with(False, 
+                                              discovery_rule_results=Path("dr_results.csv"), 
+                                              measurement_results=Path("measurements"), 
+                                              masking_file=None)
+        expected_calls = [call(1, "JS", sample_tp_lib), call(2, "JS", sample_tp_lib), call(3, "JS", sample_tp_lib)]
+        init_pattern_mock.assert_has_calls(expected_calls)
+        check_file_exists_mock.assert_not_called()
+        measurement_result_exist_mock.assert_not_called()
+        mkdir_mock.assert_called()
+
+    def test_repair_patterns_not_including_readme(self):
+        sample_tp_lib = join_resources_path("sample_patlib")
+        test_pattern = create_pattern()
+        with patch("core.pattern.Pattern.init_from_id_and_language") as init_pattern_mock, \
+            patch("core.pattern.Pattern.repair") as patternrepair_mock, \
+            patch("core.utils.check_file_exist") as check_file_exists_mock, \
+            patch("core.utils.check_measurement_results_exist") as measurement_result_exist_mock, \
+            patch("pathlib.Path.mkdir") as mkdir_mock:
+            init_pattern_mock.return_value = test_pattern
+            interface.repair_patterns("JS", [1,2,3], None, False, Path("measurements"), Path("dr_results.csv"), Path("out"), sample_tp_lib)
+
+        patternrepair_mock.assert_called_with(True, 
+                                              discovery_rule_results=Path("dr_results.csv"), 
+                                              measurement_results=Path("measurements"), 
+                                              masking_file=None)
+        expected_calls = [call(1, "JS", sample_tp_lib), call(2, "JS", sample_tp_lib), call(3, "JS", sample_tp_lib)]
+        init_pattern_mock.assert_has_calls(expected_calls)
+        check_file_exists_mock.assert_called()
+        measurement_result_exist_mock.assert_called_once()
+        mkdir_mock.assert_called()

qualitytests/cli/test_main.py

@@ -7,16 +7,19 @@
 from qualitytests.qualitytests_utils import pyexe, join_resources_path
 from cli import main
 
+from pathlib import Path
+
 
 class TestMain:
     testdir = Path(__file__).parent.parent.resolve()
     tpf = testdir.parent / "tp_framework/cli/main.py"
+    sample_tp_lib = str(join_resources_path("sample_patlib"))
 
 
     def test_cli_help_1(self):
         # process call
         cmd = pyexe + " {0} -h".format(self.tpf)
-        pr = subprocess.Popen(cmd, shell=False, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+        pr = subprocess.Popen(cmd.split(" "), shell=False, stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
         (output, errdata) = pr.communicate()
         output = output.decode("utf-8")
         print(output)
@@ -122,7 +125,7 @@ def test_cli_measure_4(self, tmp_path, mocker):
             main.main(['measure',
                        '-p', self.tp1, self.tp2,
                        '--tools', self.tool1, 'whatever', '-l', self.test_lang,
-                       '--tp-lib', str(tmp_path)])
+                       '--tp-lib', TestMain.sample_tp_lib])
 
 
     def test_cli_measure_5(self, tmp_path, mocker):
@@ -131,7 +134,7 @@ def test_cli_measure_5(self, tmp_path, mocker):
         main.main(['measure',
                    '-p', self.tp1, self.tp2,
                    '--tools', self.tool1, self.tool2, '-l', self.test_lang,
-                   '--tp-lib', str(tmp_path)])
+                   '--tp-lib', TestMain.sample_tp_lib])
 
 
     def _init_cli_report(self, mocker):
@@ -156,7 +159,7 @@ def test_cli_report_2(self, tmp_path, mocker):
                    '--print',
                    '-p', self.tp1, self.tp2,
                    '--tools', self.tool1, self.tool2, '-l', self.test_lang,
-                   '--tp-lib', str(tmp_path)])
+                   '--tp-lib', TestMain.sample_tp_lib])
 
 
     def test_cli_report_3(self, tmp_path, mocker):
@@ -188,11 +191,13 @@ def test_cli_report_4(self, tmp_path, mocker):
     def test_cli_report_5(self, tmp_path, mocker):
         self._init_cli_report(mocker)
         # Test: valid params, no tools i.e., get all measurements
+        test_tp_lib_path = join_resources_path("sample_patlib")
         main.main(['sastreport',
                    '--export', 'whatever.csv',
                    '-a',
                    '-l', self.test_lang,
-                   '--output-dir', str(tmp_path)
+                   '--output-dir', str(tmp_path),
+                   '--tp-lib', str(test_tp_lib_path)
                    # '--output-dir', str(tmp_path),
                    # '--only-last-measurement'
                    ])
@@ -206,9 +211,11 @@ def _init_cli_check_discovery_rules_1(self, mocker):
     def test_cli_check_discovery_rules_1(self, tmp_path, mocker):
         self._init_cli_check_discovery_rules_1(mocker)
         # Test: valid params
+        test_tp_lib_path = join_resources_path("sample_patlib")
         main.main(['checkdiscoveryrules',
                    '--export', 'whatever.csv',
                    '-a',
                    '-l', self.test_lang,
-                   '--output-dir', str(tmp_path)
+                   '--output-dir', str(tmp_path),
+                   '--tp-lib', str(test_tp_lib_path)
                    ])

qualitytests/cli/test_tpf_commands.py

@@ -51,9 +51,9 @@ def test_parse_patterns(self):
         tp_ids = tpf_commands.parse_patterns(False, tp_range, [], test_tp_lib_path, test_lang)
         assert tp_ids == [2, 3]
         # one and only one mutual exclusion params: pattern ids
-        itp_ids = [1,2,5,10]
+        itp_ids = [1,3]
         tp_ids = tpf_commands.parse_patterns(False, "", itp_ids, test_tp_lib_path, test_lang)
         assert tp_ids == itp_ids
         # one and only one mutual exclusion params: all
         tp_ids = tpf_commands.parse_patterns(True, "", [], test_tp_lib_path, test_lang)
-        assert tp_ids == [1,2,3]
+        assert tp_ids == [1,2,3,4]

qualitytests/core/test_discovery.py

@@ -7,9 +7,9 @@
 from pytest_mock import MockerFixture
 
 import config
-from core import utils, discovery, instance, pattern
-from core.exceptions import MeasurementNotFound, CPGGenerationError
-from qualitytests.qualitytests_utils import join_resources_path, get_result_output_dir
+from core import utils, discovery
+from core.exceptions import CPGGenerationError
+from qualitytests.qualitytests_utils import join_resources_path, create_instance
 
 
 class TestDiscovery:
@@ -253,12 +253,8 @@ def test_patch_PHP_discovery_rule_2(self, tmp_path):
         assert str(tmp_path) in str(pdr)
 
     def test_dicovery_with_empty_rule(self):
-        with open(join_resources_path("sample_patlib/PHP/4_empty_pattern/4_empty_pattern.json"), "r") as json_file:
-            pattern_dict = json.load(json_file)
-        test_pattern = pattern.pattern_from_dict(pattern_dict, "PHP", 4)
-        with open(join_resources_path("sample_patlib/PHP/4_empty_pattern/1_instance_4_empty_pattern/1_instance_4_empty_pattern.json"), "r") as json_file:
-            instance_dict = json.load(json_file)
-        tpi_instance = instance.instance_from_dict(instance_dict, test_pattern, "PHP", 1)
+        tpi_instance = create_instance()
+        tpi_instance.discovery_rule = None
         assert not tpi_instance.discovery_rule, "The test case is broken, instance 1 of PHP pattern 4 is not supposed to have a discovery rule"
         expected = dict.fromkeys(["rule_path", "method", "rule_name", "rule_accuracy", "rule_hash", "rule_name", "results", "rule_already_executed"], None)
         actual = discovery.discovery_for_tpi(tpi_instance, None, None, None)