groovy

AnnotationConstantExpression.visit should include a call to super.visit

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.7.5
  • Fix Version/s: 1.8-rc-4
  • Component/s: class generator
  • Labels:
    None
  • Number of attachments :
    0

Description

the visit method of AnnotationConstantExpression should include a call to super.visitConstantExpression(visitor). There is currently no way for a visitor to visit the actual AnnotationConstantExpression, but only its members.

Issue Links

is depended upon by

Bug - A problem which impairs or prevents the functions of the product. GRECLIPSE-600 [Parser] Organize import removes nested annotations import

  • Minor - Minor loss of function, or other problem where easy workaround is present.
  • Resolved - A resolution has been taken, and it is awaiting verification by reporter. From here issues are either reopened, or are closed.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Hamlet D'Arcy added a comment -

Just an FYI, This issue and GROOVY-3653 might create backwards compatibility issues for anyone who writes AST visitors. If the user applies a manual workaround to visit these nodes and then the visitor automatically visits the nodes as well then the node might be visited twice. We should still fix this, but we should document it in release notes or something. I know CodeNarc will need to update, not sure what others.

Show
Hamlet D'Arcy added a comment - Just an FYI, This issue and GROOVY-3653 might create backwards compatibility issues for anyone who writes AST visitors. If the user applies a manual workaround to visit these nodes and then the visitor automatically visits the nodes as well then the node might be visited twice. We should still fix this, but we should document it in release notes or something. I know CodeNarc will need to update, not sure what others.
Hide
Permalink
blackdrag blackdrag added a comment -

The question is if that should go in 1.8 or not. I would assume for 1.7.x it is a bit dangerous

Show
blackdrag blackdrag added a comment - The question is if that should go in 1.8 or not. I would assume for 1.7.x it is a bit dangerous
Hide
Permalink
Guillaume Laforge added a comment -

In 1.7.x, it's indeed a bit dangerous, but it should go in 1.8, yes.

Show
Guillaume Laforge added a comment - In 1.7.x, it's indeed a bit dangerous, but it should go in 1.8, yes.
Hide
Permalink
blackdrag blackdrag added a comment -

I added the call

Show
blackdrag blackdrag added a comment - I added the call

People

  • Assignee:
    blackdrag blackdrag
    Reporter:
    Andrew Eisenberg
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: