feat: Automatic generation of Required Annotations (#452)

GideonKoenig · GideonKoenig · web-flow · commit b9031b58627a · 2022-05-06T14:47:34.000+02:00
* Code should be working - Still need to write a test * Add tests and prepare for merge with issue #441 * Still testing * get required test funktioniert jetzt * Test done and working * Satisfy Linter * style: apply automatic fixes of linters * Bug fix * Implemented the new AnnotationStore object Improved auxiliary functions Added ParameterInfo Class to communicate the return values of functions better Migrated said ParameterInfo and ParameterType classes to annotation_model.py Test is now working - all is good * Update package-parser.iml * Renamed function to better communicate it's function * Delete package-parser.iml * Satisfy Linter -> Fixed typing * style: apply automatic fixes of linters * Remove unnecessary import * Further seperate the replaceable math part of determining if a parameter should be optional or required for more clarity * Function namechange Co-authored-by: GideonKoenig <ginosiassa.koenig@online.de> Co-authored-by: GideonKoenig <GideonKoenig@users.noreply.github.com>
diff --git a/package-parser/package_parser/commands/generate_annotations/generate_annotations.py b/package-parser/package_parser/commands/generate_annotations/generate_annotations.py
@@ -8,6 +8,9 @@
 from package_parser.models.annotation_models import (
     AnnotationStore,
     ConstantAnnotation,
+    ParameterInfo,
+    ParameterType,
+    RequiredAnnotation,
     UnusedAnnotation,
 )
 from package_parser.utils import parent_qname
@@ -22,10 +25,9 @@ def generate_annotations(
     :param api_file: API file
     :param usages_file: UsageStore file
     :param output_file: Output file
-    :return: None
     """
     if api_file is None or usages_file is None or output_file is None:
-        raise ValueError("api_file, usages_file, and output_file must be specified.")
+        raise ValueError("Api_file, usages_file, and output_file must be specified.")
 
     with api_file:
         api_json = json.load(api_file)
@@ -36,7 +38,11 @@ def generate_annotations(
         usages = UsageStore.from_json(usages_json)
 
     annotations = AnnotationStore()
-    annotation_functions = [__get_unused_annotations, __get_constant_annotations]
+    annotation_functions = [
+        __get_unused_annotations,
+        __get_constant_annotations,
+        __get_required_annotations,
+    ]
 
     __generate_annotation_dict(api, usages, annotations, annotation_functions)
 
@@ -60,29 +66,21 @@ def __get_constant_annotations(
     usages: UsageStore, api: API, annotations: AnnotationStore
 ) -> None:
     """
-    Returns all parameters that are only ever assigned a single value.
+    Collect all parameters that are only ever assigned a single value.
     :param usages: UsageStore object
     :param api: API object for usages
-    :return: {"constant": dict[str, dict[str, str]]}
+    :param annotations: AnnotationStore, that holds all annotations
     """
-    for parameter_qname in list(usages.parameter_usages.keys()):
-        if len(usages.value_usages[parameter_qname].values()) == 0:
-            continue
-
-        if len(usages.value_usages[parameter_qname].keys()) == 1:
-            if usages.most_common_value(parameter_qname) is None:
-                continue
-
-            target_name = __qname_to_target_name(api, parameter_qname)
-            default_type, default_value = __get_default_type_from_value(
-                str(usages.most_common_value(parameter_qname))
-            )
+    for qname in list(usages.parameter_usages.keys()):
+        parameter_info = __get_parameter_info(qname, usages)
 
+        if parameter_info.type == ParameterType.Constant:
+            formatted_name = __qname_to_target_name(api, qname)
             annotations.constant.append(
                 ConstantAnnotation(
-                    target=target_name,
-                    defaultType=default_type,
-                    defaultValue=default_value,
+                    target=formatted_name,
+                    defaultValue=parameter_info.value,
+                    defaultType=parameter_info.value_type,
                 )
             )
 
@@ -91,10 +89,10 @@ def __get_unused_annotations(
     usages: UsageStore, api: API, annotations: AnnotationStore
 ) -> None:
     """
-    Returns all parameters that are never used.
+    Collect all parameters, functions and classes that are never used.
     :param usages: UsageStore object
     :param api: API object for usages
-    :return: {"unused": dict[str, dict[str, str]]}
+    :param annotations: AnnotationStore, that holds all annotations
     """
     for parameter_name in list(api.parameters().keys()):
         if (
@@ -121,9 +119,31 @@ def __get_unused_annotations(
             annotations.unused.append(UnusedAnnotation(formatted_name))
 
 
+def __get_required_annotations(
+    usages: UsageStore, api: API, annotations: AnnotationStore
+) -> None:
+    """
+    Collects all parameters that are currently optional but should be required to be assign a value
+    :param usages: Usage store
+    :param api: Description of the API
+    :param annotations: AnnotationStore, that holds all annotations
+    """
+    parameters = api.parameters()
+    optional_parameter = [
+        (it, parameters[it])
+        for it in parameters
+        if parameters[it].default_value is not None
+    ]
+    for qname, _ in optional_parameter:
+
+        if __get_parameter_info(qname, usages).type is ParameterType.Required:
+            formatted_name = __qname_to_target_name(api, qname)
+            annotations.requireds.append(RequiredAnnotation(formatted_name))
+
+
 def __qname_to_target_name(api: API, qname: str) -> str:
     """
-    Formats the given name to the wanted format. This method is to be removed as soon as the UsageStore is updated to
+    Formats the given name to the output format. This method is to be removed as soon as the UsageStore is updated to
     use the new format.
     :param api: API object
     :param qname: Name pre-formatting
@@ -149,21 +169,17 @@ def __qname_to_target_name(api: API, qname: str) -> str:
     return package_name + module_name + class_name + function_name + parameter_name
 
 
-def __get_default_type_from_value(default_value: str) -> tuple[str, str]:
-    default_value = str(default_value)[1:-1]
-
+def __get_default_type_from_value(default_value: str) -> str:
     if default_value == "null":
         default_type = "none"
     elif default_value == "True" or default_value == "False":
         default_type = "boolean"
     elif default_value.isnumeric():
         default_type = "number"
-        default_value = default_value
     else:
         default_type = "string"
-        default_value = default_value
 
-    return default_type, default_value
+    return default_type
 
 
 def _preprocess_usages(usages: UsageStore, api: API) -> None:
@@ -255,3 +271,48 @@ def __add_implicit_usages_of_default_value(usages: UsageStore, api: API) -> None
 
         for location in locations_of_implicit_usages_of_default_value:
             usages.add_value_usage(parameter_qname, default_value, location)
+
+
+def __get_parameter_info(qname: str, usages: UsageStore) -> ParameterInfo:
+    """
+    Returns a ParameterInfo object, that contains the type of the parameter, the value that is associated with it, and the values type
+    :param qname: name of the parameter
+    :param usages: UsageStore
+    :return ParameterInfo
+    """
+    values = [(it[0], len(it[1])) for it in usages.value_usages[qname].items()]
+
+    if len(values) == 0:
+        return ParameterInfo(ParameterType.Unused)
+    elif len(values) == 1:
+        value = values[0][0]
+        if value[0] == "'":
+            value = value[1:-1]
+        return ParameterInfo(
+            ParameterType.Constant, value, __get_default_type_from_value(value)
+        )
+
+    if __is_required(values):
+        return ParameterInfo(ParameterType.Required)
+
+    value = max(values, key=lambda item: item[1])[0]
+    if value[0] == "'":
+        value = value[1:-1]
+    return ParameterInfo(
+        ParameterType.Optional, value, __get_default_type_from_value(value)
+    )
+
+
+def __is_required(values: list[tuple[str, int]]) -> bool:
+    """
+    This replaceable function determines how to differentiate between an optional and a required parameter
+    :param values: List of all associated values and the amount they get used with
+    :return True means the parameter should be required, False means it should be optional
+    """
+    n = len(values)
+    m = sum([count for value, count in values])
+
+    seconds_most_used_value_tupel, most_used_value_tupel = sorted(
+        values, key=lambda tup: tup[1]
+    )[-2:]
+    return most_used_value_tupel[1] - seconds_most_used_value_tupel[1] <= m / n
diff --git a/package-parser/package_parser/models/annotation_models.py b/package-parser/package_parser/models/annotation_models.py
@@ -1,4 +1,5 @@
 import dataclasses
+from enum import Enum
 
 
 @dataclasses.dataclass
@@ -103,3 +104,21 @@ def to_json(self) -> dict:
                 annotation.target: annotation.to_json() for annotation in self.enums
             },
         }
+
+
+class ParameterType(Enum):
+    Constant = 0
+    Optional = 1
+    Required = 2
+    Unused = 3
+
+
+class ParameterInfo:
+    type: ParameterType
+    value: str
+    value_type: str
+
+    def __init__(self, parameter_type, value="", value_type=""):
+        self.type = parameter_type
+        self.value = value
+        self.value_type = value_type
diff --git a/package-parser/tests/commands/generate_annotations/test_generate_annotations.py b/package-parser/tests/commands/generate_annotations/test_generate_annotations.py
@@ -6,6 +6,7 @@
 from package_parser.commands.find_usages import UsageStore
 from package_parser.commands.generate_annotations.generate_annotations import (
     __get_constant_annotations,
+    __get_required_annotations,
     __get_unused_annotations,
     __qname_to_target_name,
     _preprocess_usages,
@@ -28,6 +29,7 @@
     },
 }
 
+
 CONSTANT_EXPECTED: dict[str, dict[str, str]] = {
     "test/test/commonly_used_global_function/unused_optional_parameter": {
         "defaultType": "string",
@@ -44,9 +46,25 @@
         "defaultValue": "blup",
         "target": "test/test/commonly_used_global_function/useless_required_parameter",
     },
+    "test/test/commonly_used_global_required_and_optional_function/constant_parameter": {
+        "defaultType": "string",
+        "defaultValue": "bockwurst",
+        "target": "test/test/commonly_used_global_required_and_optional_function/constant_parameter",
+    },
+}
+
+REQUIREDS_EXPECTED: dict[str, dict[str, str]] = {
+    "test/test/commonly_used_global_required_and_optional_function/optional_that_should_be_required": {
+        "target": "test/test/commonly_used_global_required_and_optional_function/optional_that_should_be_required"
+    },
+    "test/test/commonly_used_global_required_and_optional_function/commonly_used_barely_required": {
+        "target": "test/test/commonly_used_global_required_and_optional_function/commonly_used_barely_required"
+    },
+    "test/test/commonly_used_global_function/useful_optional_parameter": {
+        "target": "test/test/commonly_used_global_function/useful_optional_parameter"
+    },
 }
 
-REQUIREDS_EXPECTED: dict[str, dict[str, str]] = {}
 OPTIONALS_EXPECTED: dict[str, dict[str, str]] = {}
 BOUNDARIES_EXPECTED: dict[str, dict[str, str]] = {}
 ENUMS_EXPECTED: dict[str, dict[str, str]] = {}
@@ -123,6 +141,16 @@ def test_get_constant():
     } == CONSTANT_EXPECTED
 
 
+def test_get_required():
+    usages, api, usages_file, api_file, usages_json_path, api_json_path = setup()
+    annotations = AnnotationStore()
+    _preprocess_usages(usages, api)
+    __get_required_annotations(usages, api, annotations)
+    assert {
+        annotation.target: annotation.to_json() for annotation in annotations.requireds
+    } == REQUIREDS_EXPECTED
+
+
 def test_generate():
     usages, api, usages_file, api_file, usages_json_path, api_json_path = setup()
     out_file_path = os.path.join(
diff --git a/package-parser/tests/data/api_data.json b/package-parser/tests/data/api_data.json
@@ -15,7 +15,8 @@
             "functions": [
                 "test.unused_global_function",
                 "test.rarely_used_global_function",
-                "test.commonly_used_global_function"
+                "test.commonly_used_global_function",
+                "test.commonly_used_global_required_and_optional_function"
             ]
         }
     ],
@@ -156,6 +157,90 @@
             "description": "",
             "docstring": "",
             "source_code": ""
+        },
+        {
+            "name": "commonly_used_global_required_and_optional_function",
+            "unique_name": "commonly_used_global_required_and_optional_function",
+            "qname": "test.commonly_used_global_required_and_optional_function",
+            "unique_qname": "test.commonly_used_global_required_and_optional_function",
+            "decorators": [],
+            "parameters": [
+                {
+                    "name": "optional_that_should_be_required",
+                    "default_value": "'brains'",
+                    "is_public": true,
+                    "assigned_by": "POSITION_OR_NAME",
+                    "docstring": {
+                        "type": "str",
+                        "description": ""
+                    }
+                },
+                {
+                    "name": "required_that_should_be_required",
+                    "default_value": null,
+                    "is_public": true,
+                    "assigned_by": "POSITION_OR_NAME",
+                    "docstring": {
+                        "type": "str",
+                        "description": ""
+                    }
+                },
+                {
+                    "name": "required_that_should_be_optional",
+                    "default_value": null,
+                    "is_public": true,
+                    "assigned_by": "POSITION_OR_NAME",
+                    "docstring": {
+                        "type": "str",
+                        "description": ""
+                    }
+                },
+                {
+                    "name": "optional_that_should_be_optional",
+                    "default_value": "'captain_morgan'",
+                    "is_public": true,
+                    "assigned_by": "POSITION_OR_NAME",
+                    "docstring": {
+                        "type": "str",
+                        "description": ""
+                    }
+                },
+                {
+                    "name": "commonly_used_almost_required",
+                    "default_value": "'marvel'",
+                    "is_public": true,
+                    "assigned_by": "POSITION_OR_NAME",
+                    "docstring": {
+                        "type": "str",
+                        "description": ""
+                    }
+                },
+                {
+                    "name": "commonly_used_barely_required",
+                    "default_value": "'otto'",
+                    "is_public": true,
+                    "assigned_by": "POSITION_OR_NAME",
+                    "docstring": {
+                        "type": "str",
+                        "description": ""
+                    }
+                },
+                {
+                    "name": "constant_parameter",
+                    "default_value": null,
+                    "is_public": true,
+                    "assigned_by": "POSITION_OR_NAME",
+                    "docstring": {
+                        "type": "str",
+                        "description": ""
+                    }
+                }
+            ],
+            "results": [],
+            "is_public": true,
+            "description": "",
+            "docstring": "",
+            "source_code": ""
         }
     ]
 }
diff --git a/package-parser/tests/data/usage_data.json b/package-parser/tests/data/usage_data.json
