Merge remote-tracking branch 'jamesls/fix-error-messages' into develop

* jamesls/fix-error-messages:
  Remove redundant error check in jp
  Raise IncompleteExpressionError where appropriate
  Use singular argument when expected arity is 1
  Add test for empty array with sort_by
  Fix invalid-type error messages

Author: jamesls

bin/jp

@@ -51,9 +51,6 @@ def main():
     except exceptions.UnknownFunctionError as e:
         sys.stderr.write("unknown-function: %s\n" % e)
         return 1
-    except exceptions.LexerError as e:
-        sys.stderr.write("syntax-error: %s\n" % e)
-        return 1
     except exceptions.ParseError as e:
         sys.stderr.write("syntax-error: %s\n" % e)
         return 1

jmespath/exceptions.py

@@ -22,8 +22,8 @@ def __str__(self):
         # self.lex_position +1 to account for the starting double quote char.
         underline = ' ' * (self.lex_position + 1) + '^'
         return (
-            '%s: Parse error at column %s near '
-            'token "%s" (%s) for expression:\n"%s"\n%s' % (
+            '%s: Parse error at column %s, '
+            'token "%s" (%s), for expression:\n"%s"\n%s' % (
                 self.msg, self.lex_position, self.token_value, self.token_type,
                 self.expression, underline))
 
@@ -71,19 +71,29 @@ def __init__(self, expected, actual, name):
         self.expression = None
 
     def __str__(self):
-        return ("Expected %s arguments for function %s(), "
-                "received %s" % (self.expected_arity,
-                                 self.function_name,
-                                 self.actual_arity))
+        return ("Expected %s %s for function %s(), "
+                "received %s" % (
+                    self.expected_arity,
+                    self._pluralize('argument', self.expected_arity),
+                    self.function_name,
+                    self.actual_arity))
+
+    def _pluralize(self, word, count):
+        if count == 1:
+            return word
+        else:
+            return word + 's'
 
 
 @with_str_method
 class VariadictArityError(ArityError):
     def __str__(self):
-        return ("Expected at least %s arguments for function %s, "
-                "received %s" % (self.expected_arity,
-                                 self.function_name,
-                                 self.actual_arity))
+        return ("Expected at least %s %s for function %s(), "
+                "received %s" % (
+                    self.expected_arity,
+                    self._pluralize('argument', self.expected_arity),
+                    self.function_name,
+                    self.actual_arity))
 
 
 @with_str_method

jmespath/functions.py

@@ -317,7 +317,7 @@ def _func_sort_by(self, array, expref):
         # that validates that type, which requires that remaining array
         # elements resolve to the same type as the first element.
         required_type = self._convert_to_jmespath_type(
-            self.interpreter.visit(expref.expression, array[0]))
+            type(self.interpreter.visit(expref.expression, array[0])).__name__)
         if required_type not in ['number', 'string']:
             raise exceptions.JMESPathTypeError(
                 'sort_by', array[0], required_type, ['string', 'number'])
@@ -345,12 +345,14 @@ def _create_key_func(self, expr_node, allowed_types, function_name):
 
         def keyfunc(x):
             result = interpreter.visit(expr_node, x)
-            jmespath_type = self._convert_to_jmespath_type(result)
+            actual_typename = type(result).__name__
+            jmespath_type = self._convert_to_jmespath_type(actual_typename)
+            # allowed_types is in term of jmespath types, not python types.
             if jmespath_type not in allowed_types:
                 raise exceptions.JMESPathTypeError(
                     function_name, result, jmespath_type, allowed_types)
             return result
         return keyfunc
 
     def _convert_to_jmespath_type(self, pyobject):
-        return TYPES_MAP.get(type(pyobject).__name__, 'unknown')
+        return TYPES_MAP.get(pyobject, 'unknown')

jmespath/parser.py

@@ -427,6 +427,9 @@ def _assert_not_token(self, *token_types):
                 "Token %s not allowed to be: %s" % (actual_type, token_types))
 
     def _error_nud_token(self, token):
+        if token['type'] == 'eof':
+            raise exceptions.IncompleteExpressionError(
+                token['start'], token['value'], token['type'])
         raise exceptions.ParseError(token['start'], token['value'],
                                     token['type'], 'Invalid token.')
 

tests/compliance/functions.json

@@ -609,7 +609,7 @@
   },
   "cases": [
     {
-      "description": "function projection on variadic function",
+      "description": "sort by field expression",
       "expression": "sort_by(people, &age)",
       "result": [
          {"age": 10, "age_str": "10", "bool": true, "name": 3},
@@ -620,7 +620,7 @@
       ]
     },
     {
-      "description": "function projection on variadic function",
+      "description": "sort by function expression",
       "expression": "sort_by(people, &to_number(age_str))",
       "result": [
          {"age": 10, "age_str": "10", "bool": true, "name": 3},
@@ -631,7 +631,7 @@
       ]
     },
     {
-      "description": "function projection on variadic function",
+      "description": "function projection on sort_by function",
       "expression": "sort_by(people, &age)[].name",
       "result": [3, "a", "c", "b", "d"]
     },
@@ -655,6 +655,10 @@
       "expression": "sort_by(people, &age)[].extra",
       "result": ["foo", "bar"]
     },
+    {
+      "expression": "sort_by(`[]`, &age)",
+      "result": []
+    },
     {
       "expression": "max_by(people, &age)",
       "result": {"age": 50, "age_str": "50", "bool": false, "name": "d"}

tests/test_functions.py

@@ -4,6 +4,7 @@
 import json
 
 import jmespath
+from jmespath import exceptions
 
 
 class TestFunctions(unittest.TestCase):
@@ -16,3 +17,41 @@ def test_can_max_datetimes(self):
         data = [datetime.now(), datetime.now() + timedelta(seconds=1)]
         result = jmespath.search('max([*].to_string(@))', data)
         self.assertEqual(json.loads(result), str(data[-1]))
+
+    def test_type_error_messages(self):
+        with self.assertRaises(exceptions.JMESPathTypeError) as e:
+            jmespath.search('length(@)', 2)
+        exception = e.exception
+        # 1. Function name should be in error message
+        self.assertIn('length()', str(exception))
+        # 2. Mention it's an invalid type
+        self.assertIn('invalid type for value: 2', str(exception))
+        # 3. Mention the valid types:
+        self.assertIn("expected one of: ['string', 'array', 'object']",
+                      str(exception))
+        # 4. Mention the actual type.
+        self.assertIn('received: "number"', str(exception))
+
+    def test_singular_in_error_message(self):
+        with self.assertRaises(exceptions.ArityError) as e:
+            jmespath.search('length(@, @)', [0, 1])
+        exception = e.exception
+        self.assertEqual(
+            str(exception),
+            'Expected 1 argument for function length(), received 2')
+
+    def test_error_message_is_pluralized(self):
+        with self.assertRaises(exceptions.ArityError) as e:
+            jmespath.search('sort_by(@)', [0, 1])
+        exception = e.exception
+        self.assertEqual(
+            str(exception),
+            'Expected 2 arguments for function sort_by(), received 1')
+
+    def test_variadic_is_pluralized(self):
+        with self.assertRaises(exceptions.VariadictArityError) as e:
+            jmespath.search('not_null()', 'foo')
+        exception = e.exception
+        self.assertEqual(
+            str(exception),
+            'Expected at least 1 argument for function not_null(), received 0')

tests/test_parser.py

@@ -121,8 +121,8 @@ def test_bad_parse(self):
 
     def test_bad_parse_error_message(self):
         error_message = (
-            'Unexpected token: ]: Parse error at column 3 '
-            'near token "]" (RBRACKET) for expression:\n'
+            'Unexpected token: ]: Parse error at column 3, '
+            'token "]" (RBRACKET), for expression:\n'
             '"foo]baz"\n'
             '    ^')
         self.assert_error_message('foo]baz', error_message)
@@ -134,6 +134,13 @@ def test_bad_parse_error_message_with_multiselect(self):
             '                       ^')
         self.assert_error_message('foo.{bar: baz,bar: bar', error_message)
 
+    def test_incomplete_expression_with_missing_paren(self):
+        error_message = (
+            'Invalid jmespath expression: Incomplete expression:\n'
+            '"length(@,"\n'
+            '          ^')
+        self.assert_error_message('length(@,', error_message)
+
     def test_bad_lexer_values(self):
         error_message = (
             'Bad jmespath expression: '