chore: Add basic mypy check (no behavior change) (#2488)

Author: aahung

Makefile

@@ -26,6 +26,8 @@ black-check:
 lint:
 	# Linter performs static analysis to catch latent bugs
 	pylint --rcfile .pylintrc samtranslator
+	# mypy performs type check
+	mypy samtranslator
 
 prepare-companion-stack:
 	pytest -v --no-cov integration/setup -m setup

mypy.ini

@@ -0,0 +1,13 @@
+
+# https://mypy.readthedocs.io/en/stable/config_file.html#config-file-format
+
+[mypy]
+warn_return_any=True
+warn_unused_configs=True
+no_implicit_optional=True
+warn_redundant_casts=True
+warn_unused_ignores=True
+warn_unreachable=True
+
+[mypy-jsonschema,jsonschema.*]
+ignore_missing_imports=True

requirements/dev.txt

@@ -25,3 +25,11 @@ docopt~=0.6.2
 
 # formatter
 black==20.8b1
+
+# type check
+mypy==0.971
+
+# types
+boto3-stubs[appconfig,serverlessrepo]>=1.19.5,==1.*
+types-PyYAML~=5.4
+types-jsonschema~=3.2

samtranslator/intrinsics/actions.py

@@ -1,6 +1,5 @@
 import re
 
-from samtranslator.utils.py27hash_fix import Py27UniStr
 from samtranslator.model.exceptions import InvalidTemplateException, InvalidDocumentException
 
 
@@ -14,7 +13,11 @@ class Action(object):
     """
 
     _resource_ref_separator = "."
-    intrinsic_name = None
+
+    # Note(xinhol): `Action` should have been an abstract class. Disabling the type check for the next
+    # line to avoid any potential behavior change.
+    # TODO: Make `Action` an abstract class and not giving `intrinsic_name` initial value.
+    intrinsic_name: str = None  # type: ignore
 
     def __init__(self):
         if not self.intrinsic_name:

samtranslator/model/__init__.py

@@ -1,6 +1,8 @@
 """ CloudFormation Resource serialization, deserialization, and validation """
 import re
 import inspect
+from typing import Any, Callable, Dict
+
 from samtranslator.model.exceptions import InvalidResourceException
 from samtranslator.plugins import LifeCycleEvents
 from samtranslator.model.tags.resource_tagging import get_tag_list
@@ -37,8 +39,11 @@ class Resource(object):
     be considered invalid.
     """
 
-    resource_type = None
-    property_types = None
+    # Note(xinhol): `Resource` should have been an abstract class. Disabling the type check for the next
+    # two lines to avoid any potential behavior change.
+    # TODO: Make `Resource` an abstract class and not giving `resource_type`/`property_types` initial value.
+    resource_type: str = None  # type: ignore
+    property_types: Dict[str, PropertyType] = None  # type: ignore
     _keywords = ["logical_id", "relative_id", "depends_on", "resource_attributes"]
 
     # For attributes in this list, they will be passed into the translated template for the same resource itself.
@@ -54,7 +59,7 @@ class Resource(object):
     # attrs = {
     #   "arn": fnGetAtt(self.logical_id, "Arn")
     # }
-    runtime_attrs = {}
+    runtime_attrs: Dict[str, Callable[["Resource"], Any]] = {}  # TODO: replace Any with something more explicit
 
     def __init__(self, logical_id, relative_id=None, depends_on=None, attributes=None):
         """Initializes a Resource object with the given logical id.
@@ -387,7 +392,7 @@ class SamResourceMacro(ResourceMacro):
     # resources, there is a separate process that associates this property with LogicalId of the generated CFN resource
     # of the given type.
 
-    referable_properties = {}
+    referable_properties: Dict[str, str] = {}
 
     # Each resource can optionally override this tag:
     _SAM_KEY = "lambda:createdBy"

samtranslator/model/api/api_generator.py

@@ -31,13 +31,13 @@
 
 _CORS_WILDCARD = "'*'"
 CorsProperties = namedtuple(
-    "_CorsProperties", ["AllowMethods", "AllowHeaders", "AllowOrigin", "MaxAge", "AllowCredentials"]
+    "CorsProperties", ["AllowMethods", "AllowHeaders", "AllowOrigin", "MaxAge", "AllowCredentials"]
 )
 # Default the Cors Properties to '*' wildcard and False AllowCredentials. Other properties are actually Optional
 CorsProperties.__new__.__defaults__ = (None, None, _CORS_WILDCARD, None, False)
 
 AuthProperties = namedtuple(
-    "_AuthProperties",
+    "AuthProperties",
     [
         "Authorizers",
         "DefaultAuthorizer",
@@ -50,7 +50,7 @@
 )
 AuthProperties.__new__.__defaults__ = (None, None, None, True, None, None, None)
 UsagePlanProperties = namedtuple(
-    "_UsagePlanProperties", ["CreateUsagePlan", "Description", "Quota", "Tags", "Throttle", "UsagePlanName"]
+    "UsagePlanProperties", ["CreateUsagePlan", "Description", "Quota", "Tags", "Throttle", "UsagePlanName"]
 )
 UsagePlanProperties.__new__.__defaults__ = (None, None, None, None, None, None)
 

samtranslator/model/api/http_api_generator.py

@@ -20,11 +20,11 @@
 
 _CORS_WILDCARD = "*"
 CorsProperties = namedtuple(
-    "_CorsProperties", ["AllowMethods", "AllowHeaders", "AllowOrigins", "MaxAge", "ExposeHeaders", "AllowCredentials"]
+    "CorsProperties", ["AllowMethods", "AllowHeaders", "AllowOrigins", "MaxAge", "ExposeHeaders", "AllowCredentials"]
 )
 CorsProperties.__new__.__defaults__ = (None, None, None, None, None, False)
 
-AuthProperties = namedtuple("_AuthProperties", ["Authorizers", "DefaultAuthorizer", "EnableIamAuthorizer"])
+AuthProperties = namedtuple("AuthProperties", ["Authorizers", "DefaultAuthorizer", "EnableIamAuthorizer"])
 AuthProperties.__new__.__defaults__ = (None, None, False)
 DefaultStageName = "$default"
 HttpApiTagName = "httpapi:createdBy"

samtranslator/model/eventsources/pull.py

@@ -23,7 +23,10 @@ class PullEventSource(ResourceMacro):
     # Event types that support `FilterCriteria`, stored as a list to keep the alphabetical order
     RESOURCE_TYPES_WITH_EVENT_FILTERING = ["DynamoDB", "Kinesis", "SQS"]
 
-    resource_type = None
+    # Note(xinhol): `PullEventSource` should have been an abstract class. Disabling the type check for the next
+    # line to avoid any potential behavior change.
+    # TODO: Make `PullEventSource` an abstract class and not giving `resource_type` initial value.
+    resource_type: str = None  # type: ignore
     requires_stream_queue_broker = True
     property_types = {
         "Stream": PropertyType(False, is_str()),

samtranslator/model/eventsources/push.py

@@ -50,7 +50,10 @@ class PushEventSource(ResourceMacro):
     :cvar str principal: The AWS service principal of the source service.
     """
 
-    principal = None
+    # Note(xinhol): `PushEventSource` should have been an abstract class. Disabling the type check for the next
+    # line to avoid any potential behavior change.
+    # TODO: Make `PushEventSource` an abstract class and not giving `principal` initial value.
+    principal: str = None  # type: ignore
 
     def _construct_permission(
         self, function, source_arn=None, source_account=None, suffix="", event_source_token=None, prefix=None

samtranslator/model/sam_resources.py

@@ -1,6 +1,6 @@
 """ SAM macro definitions """
 import copy
-from typing import Union
+from typing import Any, Dict, Union
 from samtranslator.intrinsics.resolver import IntrinsicsResolver
 
 import samtranslator.model.eventsources
@@ -111,7 +111,7 @@ class SamFunction(SamResourceMacro):
     #
 
     # Conditions
-    conditions = {}
+    conditions: Dict[str, Any] = {}  # TODO: Replace `Any` with something more specific
 
     # Customers can refer to the following properties of SAM function
     referable_properties = {
@@ -607,7 +607,7 @@ def _validate_dlq(self):
         if not self.DeadLetterQueue.get("Type") or not self.DeadLetterQueue.get("TargetArn"):
             raise InvalidResourceException(
                 self.logical_id,
-                "'DeadLetterQueue' requires Type and TargetArn properties to be specified.".format(valid_dlq_types),
+                "'DeadLetterQueue' requires Type and TargetArn properties to be specified.",
             )
 
         if not (isinstance(self.DeadLetterQueue.get("Type"), str)):

samtranslator/model/sqs.py

@@ -1,11 +1,13 @@
+from typing import Dict
+
 from samtranslator.model import PropertyType, Resource
 from samtranslator.model.types import is_type, list_of
 from samtranslator.model.intrinsics import fnGetAtt, ref
 
 
 class SQSQueue(Resource):
     resource_type = "AWS::SQS::Queue"
-    property_types = {}
+    property_types: Dict[str, PropertyType] = {}
     runtime_attrs = {
         "queue_url": lambda self: ref(self.logical_id),
         "arn": lambda self: fnGetAtt(self.logical_id, "Arn"),

samtranslator/model/stepfunctions/events.py

@@ -22,7 +22,10 @@ class EventSource(ResourceMacro):
     :cvar str principal: The AWS service principal of the source service.
     """
 
-    principal = None
+    # Note(xinhol): `EventSource` should have been an abstract class. Disabling the type check for the next
+    # line to avoid any potential behavior change.
+    # TODO: Make `EventSource` an abstract class and not giving `principal` initial value.
+    principal: str = None  # type: ignore
 
     def _generate_logical_id(self, prefix, suffix, resource_type):
         """Helper utility to generate a logicial ID for a new resource

samtranslator/plugins/globals/globals.py

@@ -1,4 +1,6 @@
-from samtranslator.public.sdk.resource import SamResourceType
+from typing import Dict, List
+
+from samtranslator.public.sdk.resource import SamResourceType
 from samtranslator.public.intrinsics import is_intrinsics
 from samtranslator.swagger.swagger import SwaggerEditor
 
@@ -82,7 +84,7 @@ class Globals(object):
         SamResourceType.SimpleTable.value: ["SSESpecification"],
     }
     # unreleased_properties *must be* part of supported_properties too
-    unreleased_properties = {}
+    unreleased_properties: Dict[str, List[str]] = {}
 
     def __init__(self, template):
         """
@@ -179,9 +181,7 @@ def _parse(self, globals_dict):
 
         globals = {}
         if not isinstance(globals_dict, dict):
-            raise InvalidGlobalsSectionException(
-                self._KEYWORD, "It must be a non-empty dictionary".format(self._KEYWORD)
-            )
+            raise InvalidGlobalsSectionException(self._KEYWORD, "It must be a non-empty dictionary")
 
         for section_name, properties in globals_dict.items():
             resource_type = self._make_resource_type(section_name)

samtranslator/public/helpers.py

@@ -1,4 +1,6 @@
 from enum import Enum
+from typing import Any, Dict
+
 from samtranslator.model.exceptions import InvalidDocumentException, InvalidTemplateException
 from samtranslator.model.types import is_str
 
@@ -11,7 +13,7 @@ class SamResource(object):
     """
 
     type = None
-    properties = {}
+    properties: Dict[str, Any] = {}  # TODO: Replace `Any` with something more specific
 
     def __init__(self, resource_dict):
         """

samtranslator/sdk/resource.py

@@ -17,8 +17,8 @@
 MINSIZE = 8
 PERTURB_SHIFT = 5
 
-unicode_string_type = str if sys.version_info.major >= 3 else unicode
-long_int_type = int if sys.version_info.major >= 3 else long
+unicode_string_type = str  # TODO: remove it, python 2 legacy code
+long_int_type = int  # TODO: remove it, python 2 legacy code
 
 
 def to_py27_compatible_template(template, parameter_values=None):