edgewall / genshi

Python toolkit for generation of output for the web
http://genshi.edgewall.org
Other
86 stars 35 forks source link

astutil: Add missing slice types #35

Closed rleigh-codelibre closed 3 years ago

rleigh-codelibre commented 3 years ago

While testing the latest master branch (b41e4d893f4406ebb22e4b0a9c2f98cc0964cfad) with the ome-model project (https://gitlab.com/codelibre/ome/ome-model/-/pipelines/214935602) I encountered a number of errors which all seemed to be down to missing slice types which weren't being handled and visited. The visitors already existed, and simply adding them in seemed to work.

It might be the case that there are additional types to add here.

With this change, my code worked correctly with Python 3.9. Without it, I got several Slice type not implemented errors for each of the types this PR adds back in.

FelixSchwarz commented 3 years ago

Thank you for the PR! Unfortunately the gitlab link you posted does not work for me. Do you think you could add some tests (or mention specific cases which did not work)? That way we could ensure these features will still work in future versions of Python/Genshi.

rleigh-codelibre commented 3 years ago

Ah, the branch got deleted automatically after the MR was merged. I've updated the link to the master pipeline which also links to the git commit (which is basically the commit in this PR, but in a copy of the codebase in this third-party repository).

We are using NewTextTemplate to generate C++ and Java code using some fairly complex templates: OMEXMLModelObject.template and OMEXMLMetadata.template are two examples which exercise most of the functionality and both of which triggered some of the slice errors.

Reducing this to a minimal testcase might be rather complex. I'm not sure which template features map onto the slice types to visit, but I can have more of a dig around.

rleigh-codelibre commented 3 years ago

I've had a dig through the templates to identify the bits which trigger the errors, and provided some pointers to where the error occurs plus some tracebacks to show what Genshi was doing at that point. Hopefully that might be useful to create a contrived testcase for each; I certainly wouldn't want to base the tests on my templates since they are far too complex.

  1. Call

See this line

{% for prop in klass.properties.values() %}\
{% choose %}\
{% when prop.name in customUpdatePropertyContent %}\
${customUpdatePropertyContent[prop.name]}

Where customUpdatePropertyContent is a dict which is set up here.

Traceback (most recent call last):
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 930, in <module>
    main(model, opts)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 152, in main
    template = NewTextTemplate(codecs.open(opts.lang.templatepath('CLASS'), encoding='utf-8').read(), encoding='utf-8')
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/text.py", line 140, in __init__
    Template.__init__(self, source, filepath=filepath, filename=filename,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/base.py", line 421, in __init__
    self._stream = self._parse(source, encoding)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/text.py", line 186, in _parse
    for kind, data, pos in interpolate(text, self.filepath, lineno,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/interpolation.py", line 77, in interpolate
    expr = Expression(chunk.strip(), pos[0], pos[1],
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/eval.py", line 71, in __init__
    self.code = _compile(node, self.source, mode=self.mode,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/eval.py", line 447, in _compile
    new_source = ASTCodeGenerator(tree).code
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 39, in __init__
    self.visit(tree)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 104, in visit_Expression
    return self.visit(node.body)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 748, in visit_Subscript
    _process_slice(node.slice)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 747, in _process_slice
    raise NotImplementedError('Slice type not implemented: {0}'.format(node.__class__.__name__) )
NotImplementedError: Slice type not implemented: Call
  1. Name

This is an odd one. The error is occurring on this line

{% python

At the start of a block of python code used later on during the code generation.

Traceback (most recent call last):
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 938, in <module>
    metadataMain(model, opts)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 664, in metadataMain
    processMetadataTemplate('METADATA_AGGREGATE',
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 593, in processMetadataTemplate
    template = NewTextTemplate(codecs.open(template, encoding='utf-8').read(), encoding='utf-8')
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/text.py", line 140, in __init__
    Template.__init__(self, source, filepath=filepath, filename=filename,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/base.py", line 421, in __init__
    self._stream = self._parse(source, encoding)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/text.py", line 209, in _parse
    suite = Suite(value, self.filepath, lineno,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/eval.py", line 71, in __init__
    self.code = _compile(node, self.source, mode=self.mode,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/eval.py", line 447, in _compile
    new_source = ASTCodeGenerator(tree).code
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 39, in __init__
    self.visit(tree)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 98, in visit_Module
    self.visit(n)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 176, in visit_FunctionDef
    self.visit(statement)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 496, in visit_Expr
    self.visit(node.value)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 661, in visit_Call
    self.visit(arg)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 589, in visit_Dict
    self.visit(value)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 750, in visit_Subscript
    _process_slice(node.slice)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 749, in _process_slice
    raise NotImplementedError('Slice type not implemented: {0}'.format(node.__class__.__name__) )
NotImplementedError: Slice type not implemented: Name
  1. Subscript

Similar to (2), at the start of a block of python code used later on during the code generation. Error is on this line.

Traceback (most recent call last):
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 939, in <module>
    omeXmlMetadataMain(model, opts)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 644, in omeXmlMetadataMain
    processMetadataTemplate('OMEXML_METADATA',
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 593, in processMetadataTemplate
    template = NewTextTemplate(codecs.open(template, encoding='utf-8').read(), encoding='utf-8')
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/text.py", line 140, in __init__
    Template.__init__(self, source, filepath=filepath, filename=filename,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/base.py", line 421, in __init__
    self._stream = self._parse(source, encoding)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/text.py", line 209, in _parse
    suite = Suite(value, self.filepath, lineno,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/eval.py", line 71, in __init__
    self.code = _compile(node, self.source, mode=self.mode,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/eval.py", line 447, in _compile
    new_source = ASTCodeGenerator(tree).code
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 39, in __init__
    self.visit(tree)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 98, in visit_Module
    self.visit(n)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 176, in visit_FunctionDef
    self.visit(statement)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 202, in visit_Return
    self.visit(node.value)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 752, in visit_Subscript
    _process_slice(node.slice)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 751, in _process_slice
    raise NotImplementedError('Slice type not implemented: {0}'.format(node.__class__.__name__) )
NotImplementedError: Slice type not implemented: Subscript
  1. Attribute

Exactly the same as (3). Trace indicates that's because the attribute follows the subscript (once I fix subscript handling, as in this PR):

Traceback (most recent call last):
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 939, in <module>
    omeXmlMetadataMain(model, opts)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 644, in omeXmlMetadataMain
    processMetadataTemplate('OMEXML_METADATA',
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/xsd-fu", line 593, in processMetadataTemplate
    template = NewTextTemplate(codecs.open(template, encoding='utf-8').read(), encoding='utf-8')
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/text.py", line 140, in __init__
    Template.__init__(self, source, filepath=filepath, filename=filename,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/base.py", line 421, in __init__
    self._stream = self._parse(source, encoding)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/text.py", line 209, in _parse
    suite = Suite(value, self.filepath, lineno,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/eval.py", line 71, in __init__
    self.code = _compile(node, self.source, mode=self.mode,
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/eval.py", line 447, in _compile
    new_source = ASTCodeGenerator(tree).code
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 39, in __init__
    self.visit(tree)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 98, in visit_Module
    self.visit(n)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 176, in visit_FunctionDef
    self.visit(statement)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 293, in visit_If
    self.visit(statement)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 496, in visit_Expr
    self.visit(node.value)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 661, in visit_Call
    self.visit(arg)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 589, in visit_Dict
    self.visit(value)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 91, in visit
    ret = visitor(node)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 754, in visit_Subscript
    _process_slice(node.slice)
  File "/Volumes/data/rleigh/code/ome-files/ome-model/xsd-fu/python/genshi/template/astutil.py", line 753, in _process_slice
    raise NotImplementedError('Slice type not implemented: {0}'.format(node.__class__.__name__) )
NotImplementedError: Slice type not implemented: Attribute
rleigh-codelibre commented 3 years ago

Updated to use a simple visit call, and it's all working fine for me.

hodgestar commented 3 years ago

@rleigh-codelibre Thanks!

I've pushed a few more tests but I'm not convinced I've covered all the cases yet. E.g. If I replace self.visit(node) with XXX some of the new tests still pass.

Do you perhaps have some smaller examples of the failure cases? Even if you just have one that causes a new test to fail, I can probably extrapolate the rest (things are a bit complicated because Genshi does some rewriting of top-level lookups).

hodgestar commented 3 years ago

P.S. Thank you very much for catching these issues.

rleigh-codelibre commented 3 years ago

@hodgestar I'm afraid I don't have anything simpler; I would need to try and contrive something which involves subscript, and I'm afraid I'm not too much of an expert (I've only used Genshi for this one purpose in this particular code generator). I can certainly try and see how the existing unit tests work and try to do something appropriate.

abhigns369 commented 3 years ago

I would like to work on it and I'm newbie so please guide me through it.Thanks

Anton-2 commented 3 years ago

Thanks, we had the same issue here when testing python 3.9 migration. Now our codebase (more than 380 genshi templates) seems to work fine with the self.visit(node) catch-all. Any idea regarding when this PR could be merged ?

hodgestar commented 3 years ago

@hodgestar I'm afraid I don't have anything simpler; I would need to try and contrive something which involves subscript, and I'm afraid I'm not too much of an expert (I've only used Genshi for this one purpose in this particular code generator). I can certainly try and see how the existing unit tests work and try to do something appropriate.

@rleigh-codelibre No problem. I found the missing test cases. Slices used inside a Suite (i.e. a code snippet with a list of statements) follow a slightly different code path to slices used in an Expression (i.e. a code snippet with just a single expression). I've added the tests for Suite cases and these fail without your patch as expected and pass with it.

hodgestar commented 3 years ago

Thanks, we had the same issue here when testing python 3.9 migration. Now our codebase (more than 380 genshi templates) seems to work fine with the self.visit(node) catch-all. Any idea regarding when this PR could be merged ?

@Anton-2 Now that I've found the missing tests cases, hopefully shortly.

rleigh-codelibre commented 3 years ago

@hodgestar That's much appreciated, thanks. Digging into the tests was on my todo list for later this week, but I have much more confidence in your understanding of the codebase!

hodgestar commented 3 years ago

0.7.5 has been released with this fix. Travis is still building the release for publishing but it should appear on PyPI shortly. In the mean time it's available at https://github.com/edgewall/genshi/releases/tag/0.7.5.

rleigh-codelibre commented 3 years ago

Many thanks for making the release, it's very much appreciated!