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/12/28 11:58:59 UTC
[GitHub] [netbeans] jherkel opened a new pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
jherkel opened a new pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612
see https://issues.apache.org/jira/browse/NETBEANS-4274
----------------------------------------------------------------
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 #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#discussion_r563497319
##########
File path: java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/RenameTransformer.java
##########
@@ -254,7 +254,7 @@ public boolean accept(Element e, TypeMirror type) {
String useThis = null;
String useSuper = null;
- if (elementToFind!=null && elementToFind.getKind().isField()) {
+ if (elementToFind!=null && elementToFind.getKind().isField() && tree.getKind() != Tree.Kind.MEMBER_SELECT) {
Review comment:
I think I agree with @jherkel - the code on lines 262-267 does not make much sense to me. If a reference to a field is qualified, it cannot clash with a local variable (AFAIK), even if the qualifier is different from `this`. I agree we should delete the whole `if` starting at 262. Maybe instead of `tree.getKind() != Tree.Kind.MEMBER_SELECT` do `&& tree.getKind() == Tree.Kind.IDENTIFIER`, because I believe that is the only case where the clash can occur.
----------------------------------------------------------------
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 #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#discussion_r563497319
##########
File path: java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/RenameTransformer.java
##########
@@ -254,7 +254,7 @@ public boolean accept(Element e, TypeMirror type) {
String useThis = null;
String useSuper = null;
- if (elementToFind!=null && elementToFind.getKind().isField()) {
+ if (elementToFind!=null && elementToFind.getKind().isField() && tree.getKind() != Tree.Kind.MEMBER_SELECT) {
Review comment:
I think I agree with @jherkel - the code on lines 262-267 does not make much sense to me. If a reference to a field is qualified, it cannot clash with a local variable (AFAIK), even if the qualifier is different from `this`. I agree we should delete the whole `if` starting at 262. Maybe instead of `tree.getKind() != Tree.Kind.MEMBER_SELECT` do `&& tree.getKind() == Tree.Kind.IDENTIFIER`, because I believe that is the only case where the clash can occur.
----------------------------------------------------------------
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] mbien commented on pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
mbien commented on pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#issuecomment-1012352876
added to NB13 milestone to put it back on the radar
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
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] mbien edited a comment on pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
mbien edited a comment on pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#issuecomment-1012352876
added to NB13 milestone to put it back on the radar (seems like reviewer remarks had been addressed)
edit: one job errors when i restart it. It might need a rebase on latest master to make the CI happy - not sure why this happens.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
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] jherkel commented on a change in pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
jherkel commented on a change in pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#discussion_r550867066
##########
File path: java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/RenameTransformer.java
##########
@@ -254,7 +254,7 @@ public boolean accept(Element e, TypeMirror type) {
String useThis = null;
String useSuper = null;
- if (elementToFind!=null && elementToFind.getKind().isField()) {
+ if (elementToFind!=null && elementToFind.getKind().isField() && tree.getKind() != Tree.Kind.MEMBER_SELECT) {
Review comment:
Good point. I think the whole if statement started from line 262 can be deleted. Because this part of code (started from line 257) checks if a new name for refactored field doesn't have the same name as some local variables do. For this case it is adding an outer scope (this or class name for static fields). But I think if field is a member i.e. tree.getKind() == Tree.Kind.MEMBER_SELECT we don't need to do anything because we have a scope.
----------------------------------------------------------------
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] matthiasblaesing commented on a change in pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on a change in pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#discussion_r550803414
##########
File path: java/refactoring.java/src/org/netbeans/modules/refactoring/java/plugins/RenameTransformer.java
##########
@@ -254,7 +254,7 @@ public boolean accept(Element e, TypeMirror type) {
String useThis = null;
String useSuper = null;
- if (elementToFind!=null && elementToFind.getKind().isField()) {
+ if (elementToFind!=null && elementToFind.getKind().isField() && tree.getKind() != Tree.Kind.MEMBER_SELECT) {
Review comment:
Please add reasoning why you propose a change. In this case, the change looks wrong, as it contradicts the check in line 262. That branch can't be reached anymore with your change in place.
----------------------------------------------------------------
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] neilcsmith-net commented on pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
neilcsmith-net commented on pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#issuecomment-1014661433
Thanks @mbien Let's merge before branch and check in rc for any issues.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
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] mbien edited a comment on pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
mbien edited a comment on pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#issuecomment-1012352876
added to NB13 milestone to put it back on the radar (seems like reviewer remarks had been addressed)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
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] matthiasblaesing commented on pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#issuecomment-753526226
@sarveshkesharwani @rtaneja1 @jlahoda could you please have a look at this?
----------------------------------------------------------------
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] geertjanw commented on pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
geertjanw commented on pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612#issuecomment-812538194
Any reason not to merge?
--
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] neilcsmith-net merged pull request #2612: [NETBEANS-4274] fix refactoring of class member when a new name is the same as the name of a local variable
Posted by GitBox <gi...@apache.org>.
neilcsmith-net merged pull request #2612:
URL: https://github.com/apache/netbeans/pull/2612
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
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