Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setParseAction not returning expected exception #251

Closed
zyp-rgb opened this issue Nov 16, 2020 · 5 comments
Closed

setParseAction not returning expected exception #251

zyp-rgb opened this issue Nov 16, 2020 · 5 comments

Comments

@zyp-rgb
Copy link

zyp-rgb commented Nov 16, 2020

pyparsing verison: 3.0.0b1
python version: 3.8.2

I am using setParseAction to do some validation and changing to the parsed result.
In validation, I might raise ParseSyntaxException('some customized info') and expect to catch the exception in parsing outside with customized info

It worked as expected when I was using pyparsing==2.4.5.
But in 3.0.0b1, the exception I catch is constantly ParseException with my customized info being replaced by normal loc,ori_sql,msg info. Is that a expected feature or a bug?

@ptmcg
Copy link
Member

ptmcg commented Nov 16, 2020

I tried to reproduce this with the following code:

import pyparsing as pp
print("Pyparsing version", pp.__version__)

def pa(s, l, t):
    print("in pa", t.asList(), flush=True)
    raise pp.ParseSyntaxException("wut?")

grammar = pp.Word("ABC").addParseAction(pa)
grammar.parseString("ABCABCABC")

And got this output:

Pyparsing version 3.0.0b1
in pa ['ABCABCABC']
Traceback (most recent call last):
  File "C:/Users/ptmcg/dev/pyparsing/gh/pyparsing/scratch/bad_parse_action_exception.py", line 12, in <module>
    grammar.parseString("ABCABCABC")
  File "C:\Users\ptmcg\dev\pyparsing\gh\pyparsing\pyparsing\core.py", line 869, in parseString
    raise exc.with_traceback(None)
pyparsing.exceptions.ParseSyntaxException: wut?  (at char 0), (line:1, col:1)

Can you post some code that illustrates this issue for you?

@zyp-rgb
Copy link
Author

zyp-rgb commented Nov 17, 2020

Took some time to find out where the problem is.
So in the old version, infixNotation would raise the SyntaxException.
I guess the new version checks all the calls and return the last unmatch call exception, which is ParseException

import pyparsing as pp
from pyparsing import CaselessLiteral
from pyparsing import Suppress
from pyparsing import pyparsing_common
from pyparsing import Group
from pyparsing import Optional
from pyparsing import delimitedList
from pyparsing import ParseSyntaxException

print(pp.__version__)
left_parenthesis, right_parenthesis = map(Suppress, '()')
left_bracket, right_bracket = map(Suppress, '[]')


def mode_call_action(tokens):
  """check fraction range & organize structure"""
  token = tokens[0]
  result = {'mode': token['mode']}
  if token.get('fraction'):
    raise ParseSyntaxException(f'Function MODE requires no input.')

mode_call = Group(
    CaselessLiteral('MODE').setResultsName('mode')
    + (left_parenthesis
       + Optional(pyparsing_common.number.setResultsName('fraction') |
                  (CaselessLiteral('ARRAY').suppress()
                   + left_bracket
                   + Group(delimitedList(pyparsing_common.number))(
                          'args').setResultsName('fraction')
                   + right_bracket))
       + right_parenthesis)).setParseAction(mode_call_action)

other_call = Group(pyparsing_common.identifier('other') +
                  left_parenthesis +
                  Group(delimitedList(pyparsing_common.number))('args') +
                  right_parenthesis)

join_condition = pp.Forward()
join_condition <<= pp.infixNotation((
    mode_call |
    other_call),
    [(pp.CaselessLiteral('+'), 2, pp.opAssoc.LEFT, )])

# raise the parse exception
join_condition.parseString("MODE(2) WITHIN GROUP (ORDER BY a.name)", parseAll=True)

# if remove the other call, the exception is what I expected
join_condition <<= pp.infixNotation((
    mode_call ),
    [(pp.CaselessLiteral('+'), 2, pp.opAssoc.LEFT, )])
join_condition.parseString("MODE(2) WITHIN GROUP (ORDER BY a.name)", parseAll=True)

@ptmcg
Copy link
Member

ptmcg commented Nov 25, 2020

I looked a little further at this, and tried to simplify your grammar just down to the expression you are passing to infixNotation, and I think this is more an issue with MatchFirst ('|' operator):

mode_call.setName("mode_call")
other_call.setName("other_call")
for expr in (mode_call, other_call, mode_call | other_call):
    print(expr)
    try:
        result = expr.parseString("MODE(2)", parseAll=True)
        print(result.dump())
    except pp.ParseBaseException as pbe:
        print(pbe.explain())
    print()

Prints:

mode_call

^
ParseSyntaxException: Function MODE requires no input.  (at char 0), (line:1, col:1)

other_call
[['MODE', [2]]]
[0]:
  ['MODE', [2]]
  - args: [2]
  - other: 'MODE'
  [0]:
    MODE
  [1]:
    [2]

{mode_call | other_call}
[['MODE', [2]]]
[0]:
  ['MODE', [2]]
  - args: [2]
  - other: 'MODE'
  [0]:
    MODE
  [1]:
    [2]

I tracked this change down to a fix made in August 2019, but was not rolled into the 2.4.7 release. The issue the fix was for was releated to the Each class, but I replicated it to the MatchFirst and Or classes also. Now rethinking this...

@zyp-rgb
Copy link
Author

zyp-rgb commented Apr 14, 2021

U R right. Took some time looking into the source code and got a few thoughts.

Now the MatchFirst behave as treating "post-parse action" as part of a parser, which will not handle FatalException raised in action (though already matched) until all the possibility are exausted.

Problem is, if the latter pattern of a "MatchFirst" is a superset of a part of a before pattern, the expected Fatal message will not be raised. Because the latter pattern will return successfully with only matching a part of the string, bypassing the original expected info, while rest of the string produce a "Expected end of text ...." error.

import pyparsing as pp

def raise_exception(tokens):
  raise pp.ParseSyntaxException('should raise here')

test = pp.MatchFirst(
    (pp.pyparsing_common.integer + pp.pyparsing_common.identifier).setParseAction(raise_exception) |
    pp.pyparsing_common.number)
test.parseString('1s', parseAll=True)

Maybe just use the old fashion and raise Fatal immediately? Otherwise the Fatal Exception and Normal Exception do not have any difference.

@ptmcg
Copy link
Member

ptmcg commented Oct 2, 2021

Released in 3.0.0rc2

@ptmcg ptmcg closed this as completed Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants