Fix a bug caused by less args for map function (#257)

Kuniwak · web-flow · commit fba96acab2fe · 2018-01-04T21:09:33.000+09:00
* Reproduce #256 * Fix crash caused by invalid map functions that have less arguments * Add ProhibitInvalidMapCall * Remove debug code
diff --git a/test/fixture/ast/scope_plugin/fixture_to_scope_plugin_issue_256.vim b/test/fixture/ast/scope_plugin/fixture_to_scope_plugin_issue_256.vim
@@ -0,0 +1 @@
+call string(map(foo), 'v:val')
diff --git a/test/fixture/policy/prohibit_invalid_map_call_invalid.vim b/test/fixture/policy/prohibit_invalid_map_call_invalid.vim
@@ -0,0 +1 @@
+call string(map([]), 'v:val')
diff --git a/test/fixture/policy/prohibit_invalid_map_call_valid.vim b/test/fixture/policy/prohibit_invalid_map_call_valid.vim
@@ -0,0 +1 @@
+call string(map([], 'v:val'))
diff --git a/test/integration/vint/linting/policy/test_prohibit_invalid_map_call.py b/test/integration/vint/linting/policy/test_prohibit_invalid_map_call.py
@@ -0,0 +1,30 @@
+import unittest
+from test.asserting.policy import PolicyAssertion, get_fixture_path
+
+from vint.linting.level import Level
+from vint.linting.policy.prohibit_invalid_map_call import ProhibitInvalidMapCall
+
+PATH_VALID_VIM_SCRIPT = get_fixture_path('prohibit_invalid_map_call_valid.vim')
+PATH_INVALID_VIM_SCRIPT = get_fixture_path('prohibit_invalid_map_call_invalid.vim')
+
+
+class TestProhibitInvalidMapCall(PolicyAssertion, unittest.TestCase):
+    def test_get_violation_if_found_with_valid(self):
+        self.assertFoundNoViolations(PATH_VALID_VIM_SCRIPT, ProhibitInvalidMapCall)
+
+
+    def test_get_violation_if_found_with_invalid(self):
+        expected_violations = [
+            {
+                'name': 'ProhibitInvalidMapCall',
+                'level': Level.ERROR,
+                'position': {
+                    'line': 1,
+                    'column': 16,
+                    'path': PATH_INVALID_VIM_SCRIPT
+                }
+            }
+        ]
+        self.assertFoundViolationsEqual(PATH_INVALID_VIM_SCRIPT,
+                                        ProhibitInvalidMapCall,
+                                        expected_violations)
diff --git a/test/unit/vint/ast/plugin/scope_plugin/test_map_and_filter_parser.py b/test/unit/vint/ast/plugin/scope_plugin/test_map_and_filter_parser.py
@@ -16,8 +16,8 @@
 
 
 class Fixtures(enum.Enum):
-    MAP_AND_FILTER_VARIABLE = Path(FIXTURE_BASE_PATH,
-                                   'fixture_to_scope_plugin_map_and_filter.vim')
+    MAP_AND_FILTER_VARIABLE = Path(FIXTURE_BASE_PATH, 'fixture_to_scope_plugin_map_and_filter.vim')
+    ISSUE_256 = Path(FIXTURE_BASE_PATH, 'fixture_to_scope_plugin_issue_256.vim')
 
 
 class TestMapAndFilterParser(unittest.TestCase):
@@ -66,6 +66,13 @@ def enter_handler(node):
         self.assertTrue(all(is_map_and_filter_content_visited.values()))
 
 
+    def test_issue_256(self):
+        ast = self.create_ast(Fixtures.ISSUE_256)
+        parser = MapAndFilterParser()
+        got_ast = parser.process(ast)
+
+        self.assertIsNotNone(got_ast)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/vint/ast/plugin/scope_plugin/map_and_filter_parser.py b/vint/ast/plugin/scope_plugin/map_and_filter_parser.py
@@ -30,7 +30,13 @@ def enter_handler(node):
             if not is_map_or_function_call:
                 return
 
-            string_expr_node = node['rlist'][1]
+            args = node['rlist']
+
+            # Prevent crash. See https://github.com/Kuniwak/vint/issues/256.
+            if len(args) < 2:
+                return
+
+            string_expr_node = args[1]
 
             # We can analyze only STRING nodes by static analyzing.
             if NodeType(string_expr_node['type']) is not NodeType.STRING:
diff --git a/vint/linting/policy/prohibit_invalid_map_call.py b/vint/linting/policy/prohibit_invalid_map_call.py
@@ -0,0 +1,30 @@
+from vint.ast.node_type import NodeType
+from vint.linting.level import Level
+from vint.linting.policy.abstract_policy import AbstractPolicy
+from vint.linting.policy_registry import register_policy
+
+
+@register_policy
+class ProhibitInvalidMapCall(AbstractPolicy):
+    def __init__(self):
+        super(ProhibitInvalidMapCall, self).__init__()
+        self.description = 'Number of arguments for map() must be 2 (if not, it will throw E118 or E119)'
+        self.reference = ':help map()'
+        self.level = Level.ERROR
+
+
+    def listen_node_types(self):
+        return [NodeType.CALL]
+
+
+    def is_valid(self, node, lint_context):
+        left_node = node['left']
+
+        if NodeType(left_node['type']) != NodeType.IDENTIFIER:
+            return True
+
+        if left_node['value'] != 'map':
+            return True
+
+        args = node['rlist']
+        return len(args) == 2
