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 2022/06/23 14:11:17 UTC
[GitHub] [netbeans] dbalek opened a new pull request, #4278: Inline redundant variable hint added.
dbalek opened a new pull request, #4278:
URL: https://github.com/apache/netbeans/pull/4278
Hint for inlining redundant variables added.
--
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] sdedic commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906356968
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
@mbien I believe it can't be computed in general. If `$statements$` have some non-local side effect, then even a method call from `$init` may be affected through some field of another object.
There are at least two solutions:
- simple one: do not call it `redundant` variable, but simply suggest to inline the expression :)) It's the user responsibility to decide, and IDE to do the refactoring
- make the constraints tight:
-- does not reference any potentially non-final nonlocal symbols (that includes any invoked methods), or `statements` does not make any outbound method/constructor calls. We do not have machinery for nonlocal flow analysis.
-- `statements` does not *assign* to *any* of the symbols used in `init`
-- `init` should not read array members, otherwise the array could be aliased and it's item assigned through that alias. Maybe `Flow` in hints module could help here.
* or as Dusan did, limit to very special and safe cases.
--
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] sdedic commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906356968
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
@mbien I believe it can't be computed in general. If `$statements$` have some non-local side effect, then even a method call from `$init` may be affected through some field of another object.
There are at least two solutions:
- simple one: do not call it `redundant` variable, but simply suggest to inline the expression :)) It's the user responsibility to decide, and IDE to do the refactoring
- make the constraints tight:
-- does not reference any potentially non-final symbols nonlocal (that includes any invoked methods), or `statements` does not make any outbound method/constructor calls. We do not have machinery for nonlocal flow analysis.
-- `statements` does not *assign* to *any* of the symbols used in `init`
--
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 commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906306838
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
could this be solved by listing all local vars of $init (maybe there is a utility for that) and running `!referencedIn` on $statements$ for each of them?
--
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 commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906306838
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
could this be solved by listing all local vars of `$init` (maybe there is a utility for that) and running `!referencedIn` on `$statements$` for each of them?
--
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] sdedic commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r905892302
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
Q: what if `$statements` have some effects on the data used in the `$init` expression ? I.e.
```
int x = 5; // $before
int v = 10 + x; // hint here; $var
x++; // $statements, v is not referenced.
return v;
--
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] dbalek commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
dbalek commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906233067
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
Good point. Hint changed to a more restrictive version - limited only to
- local variables that are immediately returned
- local variables that are immediately assigned to another variable and then not used
--
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] sdedic commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906356968
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
@mbien I believe it can't be computed in general. If `$statements$` have some non-local side effect, then even a method call from `$init` may be affected through some field of another object.
There are at least two solutions:
- simple one: do not call it `redundant` variable, but simply suggest to inline the expression :)) It's the user responsibility to decide, and IDE to do the refactoring
- make the constraints tight:
-- does not reference any potentially non-final nonlocal symbols (that includes any invoked methods), or `statements` does not make any outbound method/constructor calls. We do not have machinery for nonlocal flow analysis.
-- `statements` does not *assign* to *any* of the symbols used in `init`
-- `init` should not read array members, otherwise the array could be aliased and it's item assigned through that alias. Maybe `Flow` in hints module could help here.
--
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] sdedic commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906356968
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
@mbien I believe it can't be computed in general. If `$statements$` have some non-local side effect, then even a method call from `$init` may be affected through some field of another object.
There are at least two solutions:
- simple one: do not call it `redundant` variable, but simply suggest to inline the expression :)) It's the user responsibility to decide, and IDE to do the refactoring
- make the constraints tight:
-- does not reference any potentially non-final nonlocal symbols (that includes any invoked methods), or `statements` does not make any outbound method/constructor calls. We do not have machinery for nonlocal flow analysis.
-- `statements` does not *assign* to *any* of the symbols used in `init`
-- `init` should not read array members, otherwise the array could be aliased and it's item assigned through that alias.
--
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] dbalek commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
dbalek commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r905884287
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,83 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", hintKind=Kind.ACTION, severity=Severity.HINT)
+ @TriggerPattern(value="$type $var = $init; $statements$ return $var;")
Review Comment:
Fixed. The light bulb should be on the variable declaration now.
--
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] dbalek merged pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
dbalek merged PR #4278:
URL: https://github.com/apache/netbeans/pull/4278
--
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] sdedic commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r905892302
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
Q: what if `$statements` have some effects on the data used in the `$init` expression ? I.e.
```
int x = 5; // $before
int v = 10 + x; // hint here; $var
x++; // $statements
return v;
--
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 commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r905408279
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,83 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", hintKind=Kind.ACTION, severity=Severity.HINT)
Review Comment:
nitpick: I believe it would improve readability (esp for future diffs) if this wouldn't be a single line, but this is consistent with the rest of the file so its not really a complaint - just my opinion
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,83 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", hintKind=Kind.ACTION, severity=Severity.HINT)
+ @TriggerPattern(value="$type $var = $init; $statements$ return $var;")
Review Comment:
the trigger seems to be a bit greedy. The hint is active even before the variable is declared.
--
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 commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906362247
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
I like your first solution. I maintain a list of jackpot rules I used myself over time (https://github.com/mbien/jackpot-inspections) and the first FAQ entry is:
Q: Can I apply suggested inspections without code review and testing?
A: No.
I don't think all hints have to be fool proof it it is communicated to the user that they can be risky. Like switching out StringBuffer with Builder will work in 99% of the time, but fail in the one case when someone actually shared it between threads. I believe this is completely OK.
Thats why I am somewhat for reverting it to the more aggressive version which uses `!referencedIn` as best effort attempt to suggest the right thing (and as you said, use less strong wording, don't call it "redundant").
--
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 commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906362247
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
I like your first solution. I maintain a list of jackpot rules I used myself over time (https://github.com/mbien/jackpot-inspections) and the first FAQ entry is:
Q: Can I apply suggested inspections without code review and testing?
A: No.
I don't think all hints have to be fool proof it it is communicated to the user that they can be risky. Like switching out StringBuffer with Builder will work in 99% of the time, but fail in the one case when someone actually shared it between threads. I believe this is completely OK.
Thats why I am somewhat for reverting it to the more aggressive version which uses `!referencedIn` as best effort attempt to suggest the right thing.
--
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] sdedic commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906356968
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
@mbien I believe it can't be computed in general. If `$statements$` have some non-local side effect, then even a method call from `$init` may be affected through some field of another object.
There are at least two solutions:
- simple one: do not call it `redundant` variable, but simply suggest to inline the expression :)) It's the user responsibility to decide, and IDE to do the refactoring
- make the constraints tight:
-- does not reference any potentially non-final nonlocal symbols (that includes any invoked methods), or `statements` does not make any outbound method/constructor calls. We do not have machinery for nonlocal flow analysis.
-- `statements` does not *assign* to *any* of the symbols used in `init`
--
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] sdedic commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r905892302
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
Q: what if `$statements` have some effects on the data used in the `$init` expression ?
--
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] dbalek commented on pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
dbalek commented on PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#issuecomment-1165388346
> I have to wonder, given how much code is needed to implement this, if this wouldn't be better solved by using the rule language
I don't know how to register hints using the rule language to be activated based on cursor position either.
However, inspired by your suggestion, I have simplified the hint registration to couple lines of code only.
--
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 commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
mbien commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r906362247
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,18 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", severity=Severity.HINT)
+ @TriggerPattern(value="$before$; $type $var = $init; $statements$; return $var;")
+ public static ErrorDescription inlineRedundantVar(HintContext ctx) {
+ TreePath var = ctx.getVariables().get("$var");
+ Collection<? extends TreePath> stats = ctx.getMultiVariables().get("$statements$");
+ if (var != null && stats != null) {
+ Element element = ctx.getInfo().getTrees().getElement(var);
+ if (element != null && element.getKind() == ElementKind.LOCAL_VARIABLE && !Utilities.isReferencedIn(ctx.getInfo(), var, stats)) {
Review Comment:
I like your first suggestion. I maintain a list of jackpot rules I used myself over time (https://github.com/mbien/jackpot-inspections) and the first FAQ entry is:
Q: Can I apply suggested inspections without code review and testing?
A: No.
I don't think all hints have to be fool proof it it is communicated to the user that they can be risky. Like switching out StringBuffer with Builder will work in 99% of the time, but fail in the one case when someone actually shared it between threads. I believe this is completely OK.
Thats why I am somewhat for reverting it to the more aggressive version which uses `!referencedIn` as best effort attempt to suggest the right thing (and as you said, use less strong wording, don't call it "redundant").
--
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] dbalek commented on a diff in pull request #4278: Inline redundant variable hint added.
Posted by GitBox <gi...@apache.org>.
dbalek commented on code in PR #4278:
URL: https://github.com/apache/netbeans/pull/4278#discussion_r905884894
##########
java/java.hints/src/org/netbeans/modules/java/hints/suggestions/Tiny.java:
##########
@@ -400,4 +402,83 @@ protected void performRewrite(final TransformationContext ctx) {
}
+ @Hint(displayName = "#DN_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", description = "#DESC_org.netbeans.modules.java.hints.suggestions.Tiny.inlineRedundantVar", category="suggestions", hintKind=Kind.ACTION, severity=Severity.HINT)
Review Comment:
Let it consistent with the rest of the file.
--
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