[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r210187332
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -172,18 +172,19 @@ private static boolean preConditionChecker(HintContext
ctx) {
}
TreePath treePath = ctx.getPath();
-EnhancedForLoopTree efl = null;
-TypeMirror expressionType = null;
if (treePath.getLeaf().getKind() == Tree.Kind.ENHANCED_FOR_LOOP) {
-efl = (EnhancedForLoopTree) treePath.getLeaf();
-expressionType = ctx.getInfo().getTrees().getTypeMirror(new
TreePath(treePath, efl.getExpression()));
-}
-
-Element treePathElement = info.getTrees().getElement(treePath);
-// variable should have local scope
-//No need to DeclaredType check as primitive types can also be
replaced with var in enhanced-for-loop
-if ((treePathElement != null && treePathElement.getKind() !=
ElementKind.LOCAL_VARIABLE) && expressionType == null) {
-return false;
+EnhancedForLoopTree efl = (EnhancedForLoopTree) treePath.getLeaf();
+TypeMirror expressionType =
ctx.getInfo().getTrees().getTypeMirror(new TreePath(treePath,
efl.getExpression()));
+//No need to DeclaredType check as primitive types can also be
replaced with var in enhanced-for-loop
+if (expressionType == null) {
Review comment:
I would prefer `Utilities.isValidType()`; unless the code is prepared to
deal with incomplete and erroneous types later.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r210187094
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertVarToExplicitType.java
##
@@ -152,16 +153,18 @@ private static boolean isLocalVarType(HintContext ctx) {
}
TreePath treePath = ctx.getPath();
-EnhancedForLoopTree efl = null;
-TypeMirror expressionType = null;
-if (treePath.getLeaf() instanceof EnhancedForLoopTree) {
-efl = (EnhancedForLoopTree) treePath.getLeaf();
-expressionType = ctx.getInfo().getTrees().getTypeMirror(new
TreePath(treePath, efl.getExpression()));
-}
-// should be local variable
-// Can either be a local variable or iterator in enhanced-for-loop
-if ((info.getTrees().getElement(treePath) != null &&
info.getTrees().getElement(treePath).getKind() != ElementKind.LOCAL_VARIABLE)
&& expressionType == null) {
-return false;
+if (treePath.getLeaf().getKind() == Tree.Kind.ENHANCED_FOR_LOOP) {
Review comment:
Lines 156-167 seem identical to `ConvertToVarHint.java:175-187`. The classes
are in the same package; could the code be shared (I think no instance vars are
used).
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r209255399
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -159,8 +191,9 @@ private static boolean preConditionChecker(HintContext
ctx) {
}
// hint is not applicable for compound variable declaration.
-if
(info.getTreeUtilities().isPartOfCompoundVariableDeclaration(treePath.getLeaf()))
+if
(info.getTreeUtilities().isPartOfCompoundVariableDeclaration(treePath.getLeaf()))
{
return false;
+}
// hint is not applicable for variable declaration where type is
already 'var'
return !info.getTreeUtilities().isVarType(treePath);
Review comment:
for enhanced `for` loop, this helper loops until the nearest textually
following assignment, definitely not good ! It should also watch for
`JavaTokenId.COLON`. Perhaps also `JavaTokenId.SEMICOLON`, for the case a
client executes it on a declaration without an assignment. or
`JavaTokenId.RPAREN` (end of enhanced-loop header).
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r209255399
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -159,8 +191,9 @@ private static boolean preConditionChecker(HintContext
ctx) {
}
// hint is not applicable for compound variable declaration.
-if
(info.getTreeUtilities().isPartOfCompoundVariableDeclaration(treePath.getLeaf()))
+if
(info.getTreeUtilities().isPartOfCompoundVariableDeclaration(treePath.getLeaf()))
{
return false;
+}
// hint is not applicable for variable declaration where type is
already 'var'
return !info.getTreeUtilities().isVarType(treePath);
Review comment:
for enhanced `for` loop, this helper loops until the nearest textually
following assignment, definitely not good ! It should also watch for
`JavaTokenId.COLON`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r209255399
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -159,8 +191,9 @@ private static boolean preConditionChecker(HintContext
ctx) {
}
// hint is not applicable for compound variable declaration.
-if
(info.getTreeUtilities().isPartOfCompoundVariableDeclaration(treePath.getLeaf()))
+if
(info.getTreeUtilities().isPartOfCompoundVariableDeclaration(treePath.getLeaf()))
{
return false;
+}
// hint is not applicable for variable declaration where type is
already 'var'
return !info.getTreeUtilities().isVarType(treePath);
Review comment:
for enhanced `for` loop, this helper loops back to the nearest textually
following assignment, definitely not good ! It should also watch for
`JavaTokenId.COLON`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r209255399
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -159,8 +191,9 @@ private static boolean preConditionChecker(HintContext
ctx) {
}
// hint is not applicable for compound variable declaration.
-if
(info.getTreeUtilities().isPartOfCompoundVariableDeclaration(treePath.getLeaf()))
+if
(info.getTreeUtilities().isPartOfCompoundVariableDeclaration(treePath.getLeaf()))
{
return false;
+}
// hint is not applicable for variable declaration where type is
already 'var'
return !info.getTreeUtilities().isVarType(treePath);
Review comment:
for enhanced `for` loop, this helper loops back to the nearest textually
preceding assignment, definitely not good ! it seems that the helper need to
stop at the boundary of some Tree parent, e.g. statements, variable decl,
block, lambda, ...
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r208543306
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -209,17 +243,15 @@ private static boolean isValidVarType(HintContext ctx) {
default:
break;
}
+// variable initializer type should be same as variable type.
+TypeMirror initTypeMirror =
ctx.getInfo().getTrees().getTypeMirror(initTreePath);
+TypeMirror variableTypeMirror =
ctx.getInfo().getTrees().getElement(treePath).asType();
+return !((!Utilities.isValidType(initTypeMirror)) ||
(!ctx.getInfo().getTypes().isSameType(variableTypeMirror,
Utilities.resolveCapturedType(ctx.getInfo(), initTypeMirror;
Review comment:
Could be simplified.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r208539954
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -129,6 +138,22 @@ protected void performRewrite(TransformationContext tc)
throws Exception {
initializerTree
);
wc.rewrite(oldVariableTree, newVariableTree);
+} else if (statementPath.getLeaf().getKind() ==
Tree.Kind.ENHANCED_FOR_LOOP) {
+EnhancedForLoopTree elfTree = (EnhancedForLoopTree)
statementPath.getLeaf();
+ExpressionTree expTree = elfTree.getExpression();
+VariableTree vtt = elfTree.getVariable();
+if (expTree == null) {
Review comment:
maybe also check for `vtt == null`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r208539692
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -65,8 +72,10 @@
"compiler.err.generic.array.creation" //NOI18N
)));
-@TriggerPattern("$mods$ $type $var = $init") //NOI18N
-
+@TriggerPatterns({
+@TriggerPattern("$mods$ $type $var = $init"), //NOI18N
+@TriggerPattern("for ($type $var : $expression { $stmts$; }") //NOI18N
Review comment:
Seems that closing `)` is missing in pattern. It works at runtime, but still
worth to correct.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r208540876
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -209,17 +243,15 @@ private static boolean isValidVarType(HintContext ctx) {
default:
break;
}
+// variable initializer type should be same as variable type.
+TypeMirror initTypeMirror =
ctx.getInfo().getTrees().getTypeMirror(initTreePath);
+TypeMirror variableTypeMirror =
ctx.getInfo().getTrees().getElement(treePath).asType();
+return !((!Utilities.isValidType(initTypeMirror)) ||
(!ctx.getInfo().getTypes().isSameType(variableTypeMirror,
Utilities.resolveCapturedType(ctx.getInfo(), initTypeMirror;
+} else if (expressionTreePath != null) {
+ExecutableElement iterator =
HintsUtils.findIterable(ctx.getInfo());
+return !(iterator == null);
Review comment:
`iterator != null`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r208540492
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -148,9 +173,16 @@ private static boolean preConditionChecker(HintContext
ctx) {
}
TreePath treePath = ctx.getPath();
+EnhancedForLoopTree efl = null;
+TypeMirror expressionType = null;
+if (treePath.getLeaf() instanceof EnhancedForLoopTree) {
+efl = (EnhancedForLoopTree) treePath.getLeaf();
+expressionType = ctx.getInfo().getTrees().getTypeMirror(new
TreePath(treePath, efl.getExpression()));
+}
// variable should have local scope
-if (info.getTrees().getElement(treePath).getKind() !=
ElementKind.LOCAL_VARIABLE) {
+//No need to DeclaredType check as primitive types can also be
replaced with var in enhanced-for-loop
+if ((info.getTrees().getElement(treePath) != null &&
info.getTrees().getElement(treePath).getKind() != ElementKind.LOCAL_VARIABLE)
&& expressionType == null) {
Review comment:
cache `info.getTrees().getElement(treePath)` in a local variable. I didn't
get the exact meaning of the condition - is it intended, that the processing
continues if e.g. `.getKind() != ELementKind.LOCAL_VARIABLE` provided that
`expressionType != null` ?
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r208513018 ## File path: java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java ## @@ -44,10 +46,15 @@ import org.openide.util.NbBundle.Messages; import javax.lang.model.SourceVersion; import javax.lang.model.element.ElementKind; +import javax.lang.model.element.ExecutableElement; +import javax.lang.model.element.TypeElement; Review comment: Seems unused This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org For additional commands, e-mail: notifications-h...@netbeans.apache.org For further information about the NetBeans mailing lists, visit: https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r208540274
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertToVarHint.java
##
@@ -148,9 +173,16 @@ private static boolean preConditionChecker(HintContext
ctx) {
}
TreePath treePath = ctx.getPath();
+EnhancedForLoopTree efl = null;
+TypeMirror expressionType = null;
+if (treePath.getLeaf() instanceof EnhancedForLoopTree) {
Review comment:
`.getKind()` instead of `instanceof`
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var hints for enhanced-for-loop
sdedic commented on a change in pull request #650: [NETBEANS-1075]-Adding var
hints for enhanced-for-loop
URL: https://github.com/apache/incubator-netbeans/pull/650#discussion_r208499266
##
File path:
java.hints/src/org/netbeans/modules/java/hints/jdk/ConvertVarToExplicitType.java
##
@@ -119,15 +148,21 @@ protected void performRewrite(TransformationContext tc)
throws Exception {
private static boolean isLocalVarType(HintContext ctx) {
CompilationInfo info = ctx.getInfo();
-
+
if (info.getSourceVersion().compareTo(SourceVersion.RELEASE_9) < 1) {
return false;
-}
+}
TreePath treePath = ctx.getPath();
-
+EnhancedForLoopTree efl = null;
+TypeMirror expressionType = null;
+if (treePath.getLeaf() instanceof EnhancedForLoopTree) {
Review comment:
In fact an implementation class (i.e. a proxy, or some kind of adapter)
could implement a number of interfaces, which can confuse `instanceof` checks.
As a general rule, if an API defines an enum / tag whose values correspond to
interfaces, the tag should be used in preference to `instanceof`, just to avoid
possible future problems. It's true that `instanceof interface-type` costs more
cycles than `instanceof class-type`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: notifications-unsubscr...@netbeans.apache.org
For additional commands, e-mail: notifications-h...@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
