You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2020/05/02 22:55:19 UTC
[GitHub] [netbeans] errael opened a new pull request #2126: [NETBEANS-4299] find endPos for synthetic annotation attribute
errael opened a new pull request #2126:
URL: https://github.com/apache/netbeans/pull/2126
Fixes 12.0 regression from 11.3. (might be nb-javac to jdk14 issue)
This fixes "Use @NbBundle.messages" adding a message to
```
@Messages("OptionsCategory_Name_OptionsPlayHack=any old")
```
and also adding a message to
```
@Messages({"OptionsCategory_Name_OptionsPlayHack=any old", "OptionsCategory_Keywords_OptionsPlayHack=keyw1"})
```
Not sure it's the best fix, it seems safe; needs review.
Questions about the fix:
- I don't understand getKind() vs instanceof (if/when it matters)
- Not sure if this check fully qualifies "assignment as annotation", but don't think that matters.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on a change in pull request #2126: [NETBEANS-4299] find endPos for synthetic annotation attribute
Posted by GitBox <gi...@apache.org>.
errael commented on a change in pull request #2126:
URL: https://github.com/apache/netbeans/pull/2126#discussion_r419178065
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java
##########
@@ -535,8 +535,18 @@ public int endPos(JCTree t) {
VariableTree vt = fgt.getVariables().get(fgt.getVariables().size() - 1);
return TreeInfo.getEndPos((JCTree)vt, oldTopLevel.endPositions);
}
- return TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+ int endPos = TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+
+ // [NETBEANS-4299], catch a synthetic annotation attribute and try rhs
+ if (endPos == Position.NOPOS && t instanceof JCAssign) {
+ JCAssign assign = (JCAssign)t;
+ if (assign.lhs instanceof JCIdent
+ && ((JCIdent)assign.lhs).sym instanceof Symbol.MethodSymbol) {
Review comment:
Simplified. Since first check is NOPOS, probably doesn't matter how much stuff is done; but I like simple. My manual tests passed.
Do you believe this is safe for 12.0? There's that milestone thingy... And/or might add @ebarboni as reviewer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jlahoda commented on a change in pull request #2126: [NETBEANS-4299] find endPos for synthetic annotation attribute
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #2126:
URL: https://github.com/apache/netbeans/pull/2126#discussion_r419047946
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java
##########
@@ -535,8 +535,18 @@ public int endPos(JCTree t) {
VariableTree vt = fgt.getVariables().get(fgt.getVariables().size() - 1);
return TreeInfo.getEndPos((JCTree)vt, oldTopLevel.endPositions);
}
- return TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+ int endPos = TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+
+ // [NETBEANS-4299], catch a synthetic annotation attribute and try rhs
+ if (endPos == Position.NOPOS && t instanceof JCAssign) {
+ JCAssign assign = (JCAssign)t;
+ if (assign.lhs instanceof JCIdent
+ && ((JCIdent)assign.lhs).sym instanceof Symbol.MethodSymbol) {
Review comment:
I think the "&& ... instanceof MethodSymbol" is probably unnecessary.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on a change in pull request #2126: [NETBEANS-4299] find endPos for synthetic annotation attribute
Posted by GitBox <gi...@apache.org>.
errael commented on a change in pull request #2126:
URL: https://github.com/apache/netbeans/pull/2126#discussion_r419052793
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java
##########
@@ -535,8 +535,18 @@ public int endPos(JCTree t) {
VariableTree vt = fgt.getVariables().get(fgt.getVariables().size() - 1);
return TreeInfo.getEndPos((JCTree)vt, oldTopLevel.endPositions);
}
- return TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+ int endPos = TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+
+ // [NETBEANS-4299], catch a synthetic annotation attribute and try rhs
+ if (endPos == Position.NOPOS && t instanceof JCAssign) {
+ JCAssign assign = (JCAssign)t;
+ if (assign.lhs instanceof JCIdent
+ && ((JCIdent)assign.lhs).sym instanceof Symbol.MethodSymbol) {
Review comment:
I guess a regular/real assignment would have been in oldTopLevel.endPositions. So since it failed to get endPos as an assignment, all that's needed is to simply check the rhs (the end should be after that). Checking the lhs shouldn't be needed at all. I was being most conservative, but any rhs match should be good. I guess it could just be
```
// [NETBEANS-4299], catch a synthetic annotation attribute and try rhs
if (endPos == Position.NOPOS && t instanceof JCAssign) {
endPos = TreeInfo.getEndPos(((JCAssign)t).rhs, oldTopLevel.endPositions);
}
```
That feels less conservative and I don't know anything about the quirks in the trees.
@jlahoda your call. Do you want either change or as is?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] errael commented on a change in pull request #2126: [NETBEANS-4299] find endPos for synthetic annotation attribute
Posted by GitBox <gi...@apache.org>.
errael commented on a change in pull request #2126:
URL: https://github.com/apache/netbeans/pull/2126#discussion_r419178065
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java
##########
@@ -535,8 +535,18 @@ public int endPos(JCTree t) {
VariableTree vt = fgt.getVariables().get(fgt.getVariables().size() - 1);
return TreeInfo.getEndPos((JCTree)vt, oldTopLevel.endPositions);
}
- return TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+ int endPos = TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+
+ // [NETBEANS-4299], catch a synthetic annotation attribute and try rhs
+ if (endPos == Position.NOPOS && t instanceof JCAssign) {
+ JCAssign assign = (JCAssign)t;
+ if (assign.lhs instanceof JCIdent
+ && ((JCIdent)assign.lhs).sym instanceof Symbol.MethodSymbol) {
Review comment:
Simplified. Since first check is NOPOS, probably doesn't matter how much stuff is done; but I like simple. My manual tests passed.
BTW, I'm assuming endPos of assignment, is endPos of assignment.rhs in general.
Do you believe this is safe for 12.0? There's that milestone thingy... And/or might add @ebarboni as reviewer.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] jlahoda commented on a change in pull request #2126: [NETBEANS-4299] find endPos for synthetic annotation attribute
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #2126:
URL: https://github.com/apache/netbeans/pull/2126#discussion_r419155131
##########
File path: java/java.source.base/src/org/netbeans/modules/java/source/save/CasualDiff.java
##########
@@ -535,8 +535,18 @@ public int endPos(JCTree t) {
VariableTree vt = fgt.getVariables().get(fgt.getVariables().size() - 1);
return TreeInfo.getEndPos((JCTree)vt, oldTopLevel.endPositions);
}
- return TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+ int endPos = TreeInfo.getEndPos(t, oldTopLevel.endPositions);
+
+ // [NETBEANS-4299], catch a synthetic annotation attribute and try rhs
+ if (endPos == Position.NOPOS && t instanceof JCAssign) {
+ JCAssign assign = (JCAssign)t;
+ if (assign.lhs instanceof JCIdent
+ && ((JCIdent)assign.lhs).sym instanceof Symbol.MethodSymbol) {
Review comment:
I'd say - simplify the code. (We will see if some tests fail.) Overall, touching symbols too much inside CausalDiff is not very good, although in this particular case it probably does not matter much.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists