Merge key validation into one method (#2508)

.gitignore

@@ -116,3 +116,6 @@ venv.bak/
 
 # osx
 .DS_Store
+
+# Pycharm
+.idea/

src/cfnlint/rules/functions/FindInMap.py

@@ -83,41 +83,20 @@ def map_name(self, map_name, mappings, tree):
 
         return matches
 
-    def first_key(self, first_key, tree):
-        """Check the validity of the first key"""
+    def match_key(self, key, tree, key_name, key_index):
+        """Check the validity of a key"""
         matches = []
-        if isinstance(first_key, (str, int)):
+        if isinstance(key, (str, int)):
             return matches
-        if isinstance(first_key, (dict)):
-            matches.extend(self.check_dict(first_key, tree[:] + [1]))
+        if isinstance(key, dict):
+            matches.extend(self.check_dict(key, tree[:] + [key_index]))
         else:
-            message = 'FindInMap first key should be a {0}, string, or int at {1}'
+            message = 'FindInMap {0} should be a {1}, string, or int at {2}'
             matches.append(
                 RuleMatch(
-                    tree[:] + [1],
-                    message.format(
-                        ', '.join(map(str, self.supported_functions)),
-                        '/'.join(map(str, tree)),
-                    ),
-                )
-            )
-
-        return matches
-
-    def second_key(self, second_key, tree):
-        """Check the validity of the second key"""
-        matches = []
-        if isinstance(second_key, (str, int)):
-            return matches
-
-        if isinstance(second_key, (dict)):
-            matches.extend(self.check_dict(second_key, tree[:] + [2]))
-        else:
-            message = 'FindInMap second key should be a {0}, string, or int at {1}'
-            matches.append(
-                RuleMatch(
-                    tree[:] + [2],
+                    tree[:] + [key_index],
                     message.format(
+                        key_name,
                         ', '.join(map(str, self.supported_functions)),
                         '/'.join(map(str, tree)),
                     ),
@@ -144,8 +123,8 @@ def match(self, cfn):
                 second_key = map_obj[2]
 
                 matches.extend(self.map_name(map_name, mappings, tree))
-                matches.extend(self.first_key(first_key, tree))
-                matches.extend(self.second_key(second_key, tree))
+                matches.extend(self.match_key(first_key, tree, 'first key', 1))
+                matches.extend(self.match_key(second_key, tree, 'second key', 2))
 
             else:
                 message = 'FindInMap is a list with 3 values for {0}'

test/fixtures/templates/bad/functions_findinmap.yaml

@@ -34,3 +34,8 @@ Resources:
           Fn::FindInMap: [ amimap, !Ref 'AWS::Region', !Ref 'AWS::Region' ]
         - TopLevelKey
         - SecondLevelKey
+      SubnetId:
+        Fn::FindInMap:
+          - amimap
+          - [Key, Value]
+          - [Key, Value]

test/unit/rules/functions/test_find_in_map.py

@@ -23,4 +23,4 @@ def test_file_positive(self):
 
     def test_file_negative(self):
         """Test failure"""
-        self.helper_file_negative('test/fixtures/templates/bad/functions_findinmap.yaml', 5)
+        self.helper_file_negative('test/fixtures/templates/bad/functions_findinmap.yaml', 7)