[DOM] NaiveASTFlattener should be sync with ASTFlattener of the JDT.UI #3268
Conversation
|
@mpalat could you please review this PR and share your insights on this ? |
Will take a look. Meanwhile, please have a look and follow the process at https://github.com/eclipse-platform/eclipse.platform/blob/master/docs/Eclipse_API_Central_Deprecation_Policy.md when you are looking at deprecating APIs |
subyssurendran666
force-pushed
the
NaiveASTFlattener-should-be-sync-with-ASTFlattener-of-jdt-ui-3261
branch
from
79aaaaf to
1de89f4
Compare
subyssurendran666
force-pushed
the
NaiveASTFlattener-should-be-sync-with-ASTFlattener-of-jdt-ui-3261
branch
from
1de89f4 to
6c23547
Compare
| @@ -2072,7 +2072,12 @@ public boolean visit(TypeParameter node) { | |||
| @Override | |||
| public boolean visit(TypePattern node) { | |||
| if (DOMASTUtil.isPatternSupported(node.getAST())) { | |||
| node.getPatternVariable().accept(this); | |||
| if (node.getAST().apiLevel() < AST.JLS22) { | |||
| node.getPatternVariable().accept(this); | |||
There was a problem hiding this comment.
Should not use a deprecated API here
| * @exception UnsupportedOperationException if this expression is used with previewEnabled flag as false | ||
| * @since 3.39 | ||
| */ | ||
| public VariableDeclaration getPatternVariable2() { | ||
| supportedOnlyIn20(); | ||
| supportedOnlyIn22(); |
There was a problem hiding this comment.
Requesting for a cleanup here: Please change this name supportedOnlyIn22() to unSupportedBelow22()
There was a problem hiding this comment.
I believe the supportedOnlyIn22 method can be handle the logic. Please see the method
final void supportedOnlyIn22() {
if (this.ast.apiLevel < AST.JLS22_INTERNAL) {
throw new UnsupportedOperationException("Operation only supported in JLS22 AST"); //$NON-NLS-1$
}
}
Therefore should I create a new method called unSupportedBelow22 ?
There was a problem hiding this comment.
you can rename the current method if it's used only here
| @@ -276,7 +277,11 @@ boolean subtreeMatch0(ASTMatcher matcher, Object other) { | |||
| ASTNode clone0(AST target) { | |||
| TypePattern result = new TypePattern(target); | |||
| result.setSourceRange(getStartPosition(), getLength()); | |||
| result.setPatternVariable((VariableDeclaration) getPatternVariable().clone(target)); | |||
| if (this.ast.apiLevel < AST.JLS22_INTERNAL) { | |||
| result.setPatternVariable((VariableDeclaration) getPatternVariable().clone(target)); | |||
There was a problem hiding this comment.
Look at MethodDeclaration.clone0() - see how it has internalSetModifiers() method - this avoids deprecated warnings
| if (this.ast.apiLevel < AST.JLS22_INTERNAL) { | ||
| result.setPatternVariable((VariableDeclaration) getPatternVariable().clone(target)); | ||
| } else { | ||
| result.setPatternVariable((VariableDeclaration) getPatternVariable2().clone(target)); |
There was a problem hiding this comment.
cleanup: refer to MethodDeclaration.modifiers - see how it got changed from int to List. Can we think of using a similar approach here
What it does
The unnamed variables and pattern's DOM and ASTRewrite implemented through #2623. Following items should be added along with that.
The Problem has reported here: eclipse-jdt/eclipse.jdt.ui#1764 (comment)
How to test
This PR closes #3261
Author checklist