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
[java] Deprecate Dimensionable #1436
Conversation
Generated by 🚫 Danger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Just some minor questions/comments - which we don't need to solve now.
@@ -20,6 +20,9 @@ public ASTResource(JavaParser p, int id) { | |||
public Object jjtAccept(JavaParserVisitor visitor, Object data) { | |||
return visitor.visit(this, data); | |||
} | |||
|
|||
// TODO Should we deprecate all methods from ASTFormalParameter? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean with this? Won't the deprecation be inherited?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean override all methods that ASTResource inherits from ASTFormalParameter in ASTResource (just calling super), and deprecate the overrides. AFAIK that wouldn't deprecate the methods in ASTFormalParameter, just in ASTResource
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean, because ASTResource should not have inherited from ASTFormalParameter in the first place? (https://docs.oracle.com/javase/specs/jls/se11/html/jls-14.html#jls-14.20.3)
The method isArray()
and getArrayDepth()
are deprecated, even if used via ASTResource
- but other methods of course are not deprecated.
We can tackle this in a later change/PR.
Deprecates Dimensionable so that we can remove it with 7.0.0. Adding a node for the dimensions to tackle #997 will replace its functionality