chore: Make methods that do not need to be instance methods static (#…

…2525)

* chore: Make instance methods that do not need to be instance methods static

* Add type hints to methods touched

* Merge develop

.pylintrc

@@ -63,7 +63,6 @@ confidence=
 # no Warning level messages displayed, use"--disable=all --enable=classes
 # --disable=W"
 disable=
-    R0201, # Method could be a function
     W0613, # Unused argument %r
     W0640, # Cell variable %s defined in loop A variable used in a closure is defined in a loop
     R0902, # Too many instance attributes (%s/%s)

samcli/cli/context.py

@@ -4,6 +4,7 @@
 
 import logging
 import uuid
+from typing import Optional, cast
 
 import boto3
 import botocore
@@ -33,6 +34,8 @@ class Context:
     properties used by every CLI command.
     """
 
+    _session_id: str
+
     def __init__(self):
         """
         Initialize the context with default values
@@ -87,7 +90,7 @@ def profile(self, value):
         self._refresh_session()
 
     @property
-    def session_id(self):
+    def session_id(self) -> str:
         """
         Returns the ID of this command session. This is a randomly generated UUIDv4 which will not change until the
         command terminates.
@@ -131,7 +134,7 @@ def template_dict(self):
         return None
 
     @staticmethod
-    def get_current_context():
+    def get_current_context() -> Optional["Context"]:
         """
         Get the current Context object from Click's context stacks. This method is safe to run within the
         actual command's handler that has a ``@pass_context`` annotation. Outside of the handler, you run
@@ -159,7 +162,7 @@ def my_command_handler(ctx):
 
         click_core_ctx = click.get_current_context()
         if click_core_ctx:
-            return click_core_ctx.find_object(Context) or click_core_ctx.ensure_object(Context)
+            return cast("Context", click_core_ctx.find_object(Context) or click_core_ctx.ensure_object(Context))
 
         return None
 

samcli/cli/global_config.py

@@ -7,6 +7,7 @@
 import uuid
 import os
 from pathlib import Path
+from typing import Optional, Dict, Any
 
 import click
 
@@ -39,7 +40,7 @@ def __init__(self, config_dir=None, installation_id=None, telemetry_enabled=None
         self._telemetry_enabled = telemetry_enabled
 
     @property
-    def config_dir(self):
+    def config_dir(self) -> Path:
         if not self._config_dir:
             # Internal Environment variable to customize SAM CLI App Dir. Currently used only by integ tests.
             app_dir = os.getenv("__SAM_CLI_APP_DIR")
@@ -137,7 +138,7 @@ def telemetry_enabled(self, value):
         self._set_value("telemetryEnabled", value)
         self._telemetry_enabled = value
 
-    def _get_value(self, key):
+    def _get_value(self, key: str) -> Optional[Any]:
         cfg_path = self._get_config_file_path(CONFIG_FILENAME)
         if not cfg_path.exists():
             return None
@@ -146,7 +147,7 @@ def _get_value(self, key):
             json_body = json.loads(body)
             return json_body.get(key)
 
-    def _set_value(self, key, value):
+    def _set_value(self, key: str, value: Any) -> Any:
         cfg_path = self._get_config_file_path(CONFIG_FILENAME)
         if not cfg_path.exists():
             return self._set_json_cfg(cfg_path, key, value)
@@ -186,7 +187,8 @@ def _get_or_set_uuid(self, key):
             return cfg_value
         return self._set_value(key, str(uuid.uuid4()))
 
-    def _set_json_cfg(self, filepath, key, value, json_body=None):
+    @staticmethod
+    def _set_json_cfg(filepath: Path, key: str, value: Any, json_body: Optional[Dict] = None) -> Any:
         """
         Special logic method to add a value to a JSON configuration file. This
         method will write a new version of the file in question, so it will

samcli/commands/_utils/table_print.py

@@ -4,6 +4,7 @@
 from itertools import count, zip_longest
 import textwrap
 from functools import wraps
+from typing import Sized
 
 import click
 
@@ -116,7 +117,7 @@ def pprint_columns(columns, width, margin, format_string, format_args, columns_d
         click.secho(format_string.format(*format_args, **columns_dict), fg=color)
 
 
-def newline_per_item(iterable, counter):
+def newline_per_item(iterable: Sized, counter: int) -> None:
     """
     Adds a new line based on the index of a given iterable
     Parameters

samcli/commands/deploy/deploy_context.py

@@ -17,6 +17,7 @@
 
 import logging
 import os
+from typing import Dict, List
 
 import boto3
 import click
@@ -208,18 +209,20 @@ def deploy(
                 raise
             click.echo(str(ex))
 
-    def merge_parameters(self, template_dict, parameter_overrides):
+    @staticmethod
+    def merge_parameters(template_dict: Dict, parameter_overrides: Dict) -> List[Dict]:
         """
         CloudFormation CreateChangeset requires a value for every parameter
         from the template, either specifying a new value or use previous value.
         For convenience, this method will accept new parameter values and
         generates a dict of all parameters in a format that ChangeSet API
         will accept
 
+        :param template_dict:
         :param parameter_overrides:
         :return:
         """
-        parameter_values = []
+        parameter_values: List[Dict] = []
 
         if not isinstance(template_dict.get("Parameters", None), dict):
             return parameter_values

samcli/commands/deploy/guided_config.py

@@ -1,6 +1,7 @@
 """
 Set of Utilities to deal with reading/writing to configuration file during sam deploy
 """
+from typing import Any
 
 import click
 
@@ -105,5 +106,6 @@ def _save_image_repositories(self, cmd_names, config_env, samconfig, image_repos
             _image_repositories = [f"{key}={value}" for key, value in image_repositories.items()]
             samconfig.put(cmd_names, self.section, "image_repositories", _image_repositories, env=config_env)
 
-    def quote_parameter_values(self, parameter_value):
+    @staticmethod
+    def quote_parameter_values(parameter_value: Any) -> str:
         return '"{}"'.format(parameter_value)

samcli/commands/deploy/guided_context.py

@@ -3,6 +3,7 @@
 """
 
 import logging
+from typing import Dict, Any
 
 import click
 from click.types import FuncParamType
@@ -332,7 +333,10 @@ def run(self):
                 signing_profiles=self.signing_profiles,
             )
 
-    def _get_parameter_value(self, parameter_key, parameter_properties, parameter_override_from_cmdline):
+    @staticmethod
+    def _get_parameter_value(
+        parameter_key: str, parameter_properties: Dict, parameter_override_from_cmdline: Dict
+    ) -> Any:
         """
         This function provide the value of a parameter. If the command line/config file have "override_parameter"
         whose key exist in the template file parameters, it will use the corresponding value.

samcli/commands/init/init_templates.py

@@ -10,7 +10,8 @@
 import shutil
 import subprocess
 
-from pathlib import Path  # must come after Py2.7 deprecation
+from pathlib import Path
+from typing import Dict
 
 import click
 
@@ -83,8 +84,11 @@ def location_from_app_template(self, package_type, runtime, base_image, dependen
             msg = "Can't find application template " + app_template + " - check valid values in interactive init."
             raise InvalidInitTemplateError(msg) from ex
 
-    def _check_app_template(self, entry, app_template):
-        return entry["appTemplate"] == app_template
+    @staticmethod
+    def _check_app_template(entry: Dict, app_template: str) -> bool:
+        # we need to cast it to bool because entry["appTemplate"] can be Any, and Any's __eq__ can return Any
+        # detail: https://github.com/python/mypy/issues/5697
+        return bool(entry["appTemplate"] == app_template)
 
     def init_options(self, package_type, runtime, base_image, dependency_manager):
         if not self.clone_attempted:
@@ -113,7 +117,8 @@ def _init_options_from_manifest(self, package_type, runtime, base_image, depende
                 return list(templates_by_dep)
             return list(templates)
 
-    def _init_options_from_bundle(self, package_type, runtime, dependency_manager):
+    @staticmethod
+    def _init_options_from_bundle(package_type, runtime, dependency_manager):
         for mapping in list(itertools.chain(*(RUNTIME_DEP_TEMPLATE_MAPPING.values()))):
             if runtime in mapping["runtimes"] or any([r.startswith(runtime) for r in mapping["runtimes"]]):
                 if not dependency_manager or dependency_manager == mapping["dependency_manager"]:
@@ -128,7 +133,8 @@ def _init_options_from_bundle(self, package_type, runtime, dependency_manager):
         )
         raise InvalidInitTemplateError(msg)
 
-    def _shared_dir_check(self, shared_dir):
+    @staticmethod
+    def _shared_dir_check(shared_dir: Path) -> bool:
         try:
             shared_dir.mkdir(mode=0o700, parents=True, exist_ok=True)
             return True
@@ -146,13 +152,13 @@ def _clone_repo(self):
             return
         expected_path = os.path.normpath(os.path.join(shared_dir, self._repo_name))
         if self._template_directory_exists(expected_path):
-            self._overwrite_existing_templates(shared_dir, expected_path)
+            self._overwrite_existing_templates(expected_path)
         else:
             # simply create the app templates repo
             self._clone_new_app_templates(shared_dir, expected_path)
         self.clone_attempted = True
 
-    def _overwrite_existing_templates(self, shared_dir, expected_path):
+    def _overwrite_existing_templates(self, expected_path: str):
         self.repo_path = expected_path
         # workflow to clone a copy to a new directory and overwrite
         with osutils.mkdir_temp(ignore_errors=True) as tempdir:
@@ -174,11 +180,12 @@ def _overwrite_existing_templates(self, shared_dir, expected_path):
                 if "not found" in output.lower():
                     click.echo("WARN: Could not clone app template repo.")
 
-    def _replace_app_templates(self, temp_path, dest_path):
+    @staticmethod
+    def _replace_app_templates(temp_path: str, dest_path: str) -> None:
         try:
-            LOG.debug("Removing old templates from %s", str(dest_path))
+            LOG.debug("Removing old templates from %s", dest_path)
             shutil.rmtree(dest_path, onerror=rmtree_callback)
-            LOG.debug("Copying templates from %s to %s", str(temp_path), str(dest_path))
+            LOG.debug("Copying templates from %s to %s", temp_path, dest_path)
             shutil.copytree(temp_path, dest_path, ignore=shutil.ignore_patterns("*.git"))
         except (OSError, shutil.Error) as ex:
             # UNSTABLE STATE
@@ -208,11 +215,13 @@ def _clone_new_app_templates(self, shared_dir, expected_path):
                 if "not found" in output.lower():
                     click.echo("WARN: Could not clone app template repo.")
 
-    def _template_directory_exists(self, expected_path):
+    @staticmethod
+    def _template_directory_exists(expected_path: str) -> bool:
         path = Path(expected_path)
         return path.exists()
 
-    def _git_executable(self):
+    @staticmethod
+    def _git_executable() -> str:
         execname = "git"
         if platform.system().lower() == "windows":
             options = [execname, "{}.cmd".format(execname), "{}.exe".format(execname), "{}.bat".format(execname)]

samcli/commands/local/generate_event/event_generation.py

@@ -24,7 +24,7 @@ class ServiceCommand(click.MultiCommand):
         List all of the subcommands
     """
 
-    def __init__(self, events_lib, *args, **kwargs):
+    def __init__(self, events_lib: events.Events, *args, **kwargs):
         """
         Constructor for the ServiceCommand class
 
@@ -96,7 +96,7 @@ class EventTypeSubCommand(click.MultiCommand):
 
     TAGS = "tags"
 
-    def __init__(self, events_lib, top_level_cmd_name, subcmd_definition, *args, **kwargs):
+    def __init__(self, events_lib: events.Events, top_level_cmd_name, subcmd_definition, *args, **kwargs):
         """
         constructor for the EventTypeSubCommand class
 
@@ -177,8 +177,11 @@ def list_commands(self, ctx):
         """
         return sorted(self.subcmd_definition.keys())
 
+    @staticmethod
     @track_command
-    def cmd_implementation(self, events_lib, top_level_cmd_name, subcmd_name, *args, **kwargs):
+    def cmd_implementation(
+        events_lib: events.Events, top_level_cmd_name: str, subcmd_name: str, *args, **kwargs
+    ) -> str:
         """
         calls for value substitution in the event json and returns the
         customized json as a string

samcli/commands/package/package_context.py

@@ -18,6 +18,7 @@
 import json
 import logging
 import os
+from typing import Optional
 
 import boto3
 import click
@@ -139,7 +140,8 @@ def _export(self, template_path, use_json):
 
         return exported_str
 
-    def write_output(self, output_file_name, data):
+    @staticmethod
+    def write_output(output_file_name: Optional[str], data: str) -> None:
         if output_file_name is None:
             click.echo(data)
             return

samcli/lib/build/app_builder.py

@@ -7,6 +7,7 @@
 import json
 import logging
 import pathlib
+from typing import Dict
 
 import docker
 from aws_lambda_builders import RPC_PROTOCOL_VERSION as lambda_builders_protocol_version
@@ -172,7 +173,8 @@ def _get_build_graph(self):
         build_graph.clean_redundant_definitions_and_update(not self._is_building_specific_resource)
         return build_graph
 
-    def update_template(self, template_dict, original_template_path, built_artifacts):
+    @staticmethod
+    def update_template(template_dict: Dict, original_template_path: str, built_artifacts: Dict[str, str]) -> Dict:
         """
         Given the path to built artifacts, update the template to point appropriate resource CodeUris to the artifacts
         folder

samcli/lib/build/build_strategy.py

@@ -4,6 +4,7 @@
 import logging
 import pathlib
 import shutil
+from abc import abstractmethod, ABC
 
 from samcli.commands.build.exceptions import MissingBuildMethodException
 from samcli.lib.utils import osutils
@@ -14,7 +15,7 @@
 LOG = logging.getLogger(__name__)
 
 
-class BuildStrategy:
+class BuildStrategy(ABC):
     """
     Base class for BuildStrategy
     Keeps basic implementation of build, build_functions and build_layers
@@ -50,12 +51,12 @@ def _build_functions(self, build_graph):
 
         return function_build_results
 
+    @abstractmethod
     def build_single_function_definition(self, build_definition):
         """
         Builds single function definition and returns dictionary which contains function name as key,
         build location as value
         """
-        return {}
 
     def _build_layers(self, build_graph):
         """
@@ -67,12 +68,12 @@ def _build_layers(self, build_graph):
 
         return layer_build_results
 
+    @abstractmethod
     def build_single_layer_definition(self, layer_definition):
         """
         Builds single layer definition and returns dictionary which contains layer name as key,
         build location as value
         """
-        return {}
 
 
 class DefaultBuildStrategy(BuildStrategy):