Skip to content

Commit

Permalink
Add more typing into template (#3260)
Browse files Browse the repository at this point in the history
  • Loading branch information
kddejong committed May 22, 2024
1 parent 08c0bbc commit 1d86f9d
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 46 deletions.
17 changes: 17 additions & 0 deletions src/cfnlint/_typing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
"""
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
SPDX-License-Identifier: MIT-0
"""

from __future__ import annotations

from typing import TYPE_CHECKING, Any, List, Protocol, Union

if TYPE_CHECKING:
from cfnlint.rules._Rule import RuleMatch

Path = List[Union[str, int]]


class CheckValueFn(Protocol):
def __call__(self, value: Any, path: Path, **kwargs: Any) -> list[RuleMatch]: ...
6 changes: 4 additions & 2 deletions src/cfnlint/conditions/conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
SPDX-License-Identifier: MIT-0
"""

from __future__ import annotations

import itertools
import logging
import traceback
Expand Down Expand Up @@ -154,7 +156,7 @@ def _build_cnf(
return (cnf, equal_vars)

def build_scenarios(
self, conditions: Dict[str, Set[bool]], region: None = None
self, conditions: Dict[str, Set[bool]], region: str | None = None
) -> Iterator[Dict[str, bool]]:
"""Given a list of condition names this function will
yield scenarios that represent those conditions and
Expand Down Expand Up @@ -203,7 +205,7 @@ def build_scenarios(
itertools.product(condition_names, [region]),
)
else:
products = itertools.product([True, False], repeat=len(condition_names))
products = itertools.product([True, False], repeat=len(condition_names)) # type: ignore

for p in products:
cnf = c_cnf.copy()
Expand Down
11 changes: 6 additions & 5 deletions src/cfnlint/rules/_Rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@

import logging
from datetime import datetime
from typing import Any, Dict, Iterator, List, Sequence, Tuple, Union
from typing import Any, Dict, Iterator, List, Tuple, Union

import cfnlint.helpers
import cfnlint.rules.custom
from cfnlint._typing import Path
from cfnlint.match import Match
from cfnlint.template import Template

Expand Down Expand Up @@ -75,18 +76,18 @@ class RuleMatch:
be used as keys in a dictionary.
"""

def __init__(self, path: Sequence[str | int], message: str, **kwargs):
def __init__(self, path: Path, message: str, **kwargs):
"""
Initialize a new RuleMatch instance.
Args:
path (Sequence[str | int]): The path to the element
path (Path): The path to the element
that triggered the rule match.
message (str): The message associated with the rule match.
**kwargs: Additional keyword arguments to be stored
as attributes on the RuleMatch instance.
"""
self.path: Sequence[str | int] = path
self.path: Path = path
self.path_string: str = "/".join(map(str, path))
self.message: str = message
self.context: List[RuleMatch] = []
Expand Down Expand Up @@ -307,7 +308,7 @@ def match_resource_properties(
self,
properties: dict[str, Any],
resourcetype: str,
path: Sequence[str | int],
path: Path,
cfn: Template,
) -> list[RuleMatch]:
return []
Expand Down
84 changes: 48 additions & 36 deletions src/cfnlint/template/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import cfnlint.conditions
import cfnlint.helpers
from cfnlint._typing import CheckValueFn, Path
from cfnlint.context import create_context_for_template
from cfnlint.decode.node import dict_node, list_node
from cfnlint.graph import Graph
Expand Down Expand Up @@ -357,7 +358,7 @@ def get_directives(self) -> dict[str, list[str]]:
return results

# pylint: disable=dangerous-default-value
def _search_deep_keys(self, searchText: str | re.Pattern, cfndict, path):
def _search_deep_keys(self, searchText: str | re.Pattern, cfndict, path: Path):
"""Search deep for keys and get their values.
Args:
Expand All @@ -371,7 +372,7 @@ def _search_deep_keys(self, searchText: str | re.Pattern, cfndict, path):
keys = []
if isinstance(cfndict, dict):
for key in cfndict:
pathprop = path[:]
pathprop: Path = path[:]
pathprop.append(key)
if isinstance(searchText, str):
if key == searchText:
Expand Down Expand Up @@ -434,7 +435,7 @@ def search_deep_keys(
results.append(["Globals"] + pre_result)
return results

def get_condition_values(self, template, path=[]) -> list[dict[str, Any]]:
def get_condition_values(self, template, path: Path | None) -> list[dict[str, Any]]:
"""
Evaluates conditions in the provided CloudFormation template and returns the values.
Expand All @@ -447,14 +448,15 @@ def get_condition_values(self, template, path=[]) -> list[dict[str, Any]]:
- "Path": The path to the condition value in the template.
- "Value": The value of the condition.
"""
path = path or []
matches: list[dict[str, Any]] = []
if not isinstance(template, list):
return matches
if not len(template) == 3:
return matches

for index, item in enumerate(template[1:]):
result = {}
result: dict[str, Any] = {}
result["Path"] = path[:] + [index + 1]
if not isinstance(item, (dict, list)):
# Just straight values and pass them through
Expand Down Expand Up @@ -490,7 +492,7 @@ def get_condition_values(self, template, path=[]) -> list[dict[str, Any]]:

return matches

def get_values(self, obj, key, path=[]):
def get_values(self, obj, key, path: Path | None = None):
"""
Logic for getting the value of a key in the provided object.
Expand All @@ -510,6 +512,7 @@ def get_values(self, obj, key, path=[]):
- Returns all the values as a list if condition
- Returns the value if its just a string, int, boolean, etc.
"""
path = path or []
matches = []

if not isinstance(obj, dict):
Expand Down Expand Up @@ -585,7 +588,7 @@ def get_values(self, obj, key, path=[]):

return matches

def _loc(self, obj):
def _loc(self, obj: Any) -> tuple[int, int, int, int]:
"""Return location of object"""
return (
obj.start_mark.line,
Expand Down Expand Up @@ -615,7 +618,7 @@ def get_sub_parameters(self, sub_string):

return results

def get_location_yaml(self, text, path):
def get_location_yaml(self, text: Any, path: Path):
"""
Get the location information for the given YAML text and path.
Expand Down Expand Up @@ -669,19 +672,19 @@ def get_location_yaml(self, text, path):
# pylint: disable=W0613,too-many-locals
def check_value(
self,
obj,
key,
path,
check_value=None,
check_ref=None,
check_get_att=None,
check_find_in_map=None,
check_split=None,
check_join=None,
check_import_value=None,
check_sub=None,
pass_if_null=False,
**kwargs,
obj: dict[str, Any],
key: str,
path: Path,
check_value: CheckValueFn | None = None,
check_ref: CheckValueFn | None = None,
check_get_att: CheckValueFn | None = None,
check_find_in_map: CheckValueFn | None = None,
check_split: CheckValueFn | None = None,
check_join: CheckValueFn | None = None,
check_import_value: CheckValueFn | None = None,
check_sub: CheckValueFn | None = None,
pass_if_null: bool = False,
**kwargs: dict[str, Any],
) -> list[RuleMatch]:
"""
Check the value of a key in the provided object.
Expand Down Expand Up @@ -780,7 +783,7 @@ def check_value(

return matches

def is_resource_available(self, path, resource):
def is_resource_available(self, path: Path, resource: str) -> list[dict[str, bool]]:
"""
Compares a path to a resource to see if it is available.
Expand All @@ -797,7 +800,7 @@ def is_resource_available(self, path, resource):
The dictionary keys are the condition names, and the values indicate whether
the condition is True or False.
"""
results = []
results: list[dict[str, bool]] = []
path_conditions = self.get_conditions_from_path(self.template, path)
resource_condition = (
self.template.get("Resources", {}).get(resource, {}).get("Condition")
Expand All @@ -815,7 +818,7 @@ def is_resource_available(self, path, resource):
# resource conditions are always true. If the same resource condition
# exists in the path with the False then nothing else matters
if False in path_conditions.get(resource_condition, {True}):
return [path_conditions]
return [{resource_condition: False}]

# if any condition paths loop back on themselves with the opposite
# then its unreachable code
Expand All @@ -831,7 +834,9 @@ def is_resource_available(self, path, resource):
# if resource condition isn't available then the resource is available
return results

def get_object_without_nested_conditions(self, obj, path, region=None):
def get_object_without_nested_conditions(
self, obj: dict | list, path: Path, region: str | None = None
):
"""
Get a list of object values without conditions included.
Expand Down Expand Up @@ -1054,8 +1059,11 @@ def get_object_without_conditions(self, obj, property_names=None, region=None):
return results

def get_condition_scenarios_below_path(
self, path, include_if_in_function=False, region=None
):
self,
path: Path,
include_if_in_function: bool = False,
region: str | None = None,
) -> list[dict[str, bool]]:
"""
Get all possible scenarios for the conditions in the provided object.
Expand All @@ -1073,7 +1081,7 @@ def get_condition_scenarios_below_path(
values are boolean values indicating whether the condition is True or False.
"""
fn_ifs = self.search_deep_keys("Fn::If")
results = {}
results: dict[str, set] = {}
for fn_if in fn_ifs:
if len(fn_if) >= len(path):
if path == fn_if[0 : len(path)]:
Expand Down Expand Up @@ -1153,12 +1161,12 @@ def get_conditions_from_property(value):

def get_conditions_from_path(
self,
text,
path,
include_resource_conditions=True,
include_if_in_function=True,
only_last=False,
):
text: Any,
path: Path,
include_resource_conditions: bool = True,
include_if_in_function: bool = True,
only_last: bool = False,
) -> dict[str, set[bool]]:
"""
Parent function to handle resources with conditions.
Expand Down Expand Up @@ -1196,8 +1204,12 @@ def get_conditions_from_path(
return results

def _get_conditions_from_path(
self, text, path, include_if_in_function=True, only_last=False
):
self,
text: Any,
path: Path,
include_if_in_function: bool = True,
only_last: bool = False,
) -> dict[str, set[bool]]:
"""
Get the conditions and their True/False value for the path provided
Input:
Expand All @@ -1208,7 +1220,7 @@ def _get_conditions_from_path(
if its in the True or False part of the path.
{'condition': {True}}
"""
results = {}
results: dict[str, set[bool]] = {}

def get_condition_name(value, num=None):
"""Test conditions for validity before providing the name"""
Expand Down
2 changes: 1 addition & 1 deletion test/unit/module/template/test_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ def test_is_resource_available(self):
],
"LambdaExecutionRole",
),
[{"isPrimary": {False}}],
[{"isPrimary": False}],
)
# No error when the condition has both true/false use cases
self.assertEqual(
Expand Down
4 changes: 2 additions & 2 deletions test/unit/rules/jsonschema/test_json_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
from __future__ import annotations

from test.unit.rules import BaseRuleTestCase
from typing import List

from cfnlint._typing import Path
from cfnlint.decode.cfn_json import Mark
from cfnlint.decode.node import dict_node, str_node
from cfnlint.rules import CloudFormationLintRule, RuleMatch
Expand Down Expand Up @@ -66,7 +66,7 @@ def setUp(self):
self.template = build_dict({})
self.cfn = Template("", self.template, ["us-east-1"])

def build_result(self, message: str, path: List[str]) -> RuleMatch:
def build_result(self, message: str, path: Path) -> RuleMatch:
if len(path) > 0:
return RuleMatch(
path[:],
Expand Down

0 comments on commit 1d86f9d

Please sign in to comment.