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 2021/10/12 21:31:00 UTC
[GitHub] [netbeans] mbien opened a new pull request #3240: [jackpot] Updates to autoboxing inspections
mbien opened a new pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240
* repurpose a dusty perf rule to suggest `Boxed.valueOf() -> Boxed.parseType()` refactoring where applicable
* extract some functionality into a new rule which has the lone purpose to get rid of deprecated boxed primitive construction
example:
```java
private static void a(Integer i) {}
private static void b(String foo, int i) {}
private static void c(String foo, double a, long i) {}
public static void test() {
a(Integer.valueOf("")); // should do nothing, since destination type is not primitive
b("abcd", Integer.valueOf("1")); // -> parseInt
b("abcd", Integer.valueOf(1)); // -> parseInt
c("asdf", Float.valueOf("1"), Long.valueOf("1")); // -> parseFloat, parseLong
float five1 = Float.valueOf("5"); // -> parseFloat
Object five2 = Float.valueOf("5"); // does nothing
new ArrayList<>(Integer.valueOf("6")); // constructors work too
if(5 < Integer.valueOf("6").intValue()) {} // -> remove .intValue() -> parseInt
....
}
```
todo
* tests not updated
* getDestinationType() might not cover enough cases
* would be a great utility method if made accessible from hint files ;)
* I don't actually know what I am doing, feedback appreciated.
--
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 change in pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien commented on a change in pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#discussion_r785159077
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
+ srcType = ctx.getInfo().getTypes().unboxedType(srcType);
+ String[] replacement = PARSE_METHODS.get(srcType.getKind());
+ type = replacement[0];
+ method = replacement[1];
+ } else if (!(destType instanceof PrimitiveType) && srcType instanceof PrimitiveType) {
+ type = PARSE_METHODS.get(srcType.getKind())[0];
+ method = "valueOf"; // NOI18N
+ } else {
+ return null; // nothing to do, a different rule handles .intValue() boxing problems
}
- if (resType.getKind() == TypeKind.BOOLEAN) {
- if (ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
- // might alter new Boolean($v) to Boolean.valueOf($v), but that's all we can do. JDK < 5 has no
- // primitive-valued pasre* method for booleans.
- return null;
- }
+
+ if (srcType.getKind() == TypeKind.BOOLEAN && ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
+ return null; // JDK < 5 has no primitive-valued pasre* method for booleans.
}
- String[] arr = PARSE_METHODS.get(resType.getKind());
- if (arr == null) {
- return null; // just in case
+
+ Fix fix = JavaFixUtilities.rewriteFix(ctx, Bundle.FIX_UnnecessaryTempFromString1(type, method), ctx.getPath(), type + "." + method + "($v)"); // NOI18N
+ return ErrorDescriptionFactory.forTree(ctx, ctx.getPath(), Bundle.TEXT_UnnecessaryTempFromString(), fix); // NOI18N
+ }
+
+ private static TypeMirror getDestinationType(HintContext ctx, TreePath path) {
+
+ TreePath parent = path.getParentPath();
+ Tree parentLeaf = parent.getLeaf();
+
+ Types types = ctx.getInfo().getTypes();
+ Trees trees = ctx.getInfo().getTrees();
+
+ if (parentLeaf instanceof MethodInvocationTree) {
+
+ MethodInvocationTree met = (MethodInvocationTree) parentLeaf;
+
+ int index = met.getArguments().indexOf(path.getLeaf());
+
+ TypeMirror method = trees.getElement(new TreePath(path, met.getMethodSelect())).asType();
Review comment:
@jlahoda you think it is OK to leave the NEW_CLASS if-branch as is? I still don't know how to get a handle on a constructor, with getTypeMirror I would only get the class type, not the constructor type with its parameters.
--
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 change in pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien commented on a change in pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#discussion_r777225566
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
+ srcType = ctx.getInfo().getTypes().unboxedType(srcType);
+ String[] replacement = PARSE_METHODS.get(srcType.getKind());
+ type = replacement[0];
+ method = replacement[1];
+ } else if (!(destType instanceof PrimitiveType) && srcType instanceof PrimitiveType) {
+ type = PARSE_METHODS.get(srcType.getKind())[0];
+ method = "valueOf"; // NOI18N
+ } else {
+ return null; // nothing to do, a different rule handles .intValue() boxing problems
}
- if (resType.getKind() == TypeKind.BOOLEAN) {
- if (ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
- // might alter new Boolean($v) to Boolean.valueOf($v), but that's all we can do. JDK < 5 has no
- // primitive-valued pasre* method for booleans.
- return null;
- }
+
+ if (srcType.getKind() == TypeKind.BOOLEAN && ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
+ return null; // JDK < 5 has no primitive-valued pasre* method for booleans.
}
- String[] arr = PARSE_METHODS.get(resType.getKind());
- if (arr == null) {
- return null; // just in case
+
+ Fix fix = JavaFixUtilities.rewriteFix(ctx, Bundle.FIX_UnnecessaryTempFromString1(type, method), ctx.getPath(), type + "." + method + "($v)"); // NOI18N
+ return ErrorDescriptionFactory.forTree(ctx, ctx.getPath(), Bundle.TEXT_UnnecessaryTempFromString(), fix); // NOI18N
+ }
+
+ private static TypeMirror getDestinationType(HintContext ctx, TreePath path) {
+
+ TreePath parent = path.getParentPath();
+ Tree parentLeaf = parent.getLeaf();
+
+ Types types = ctx.getInfo().getTypes();
+ Trees trees = ctx.getInfo().getTrees();
+
+ if (parentLeaf instanceof MethodInvocationTree) {
+
+ MethodInvocationTree met = (MethodInvocationTree) parentLeaf;
+
+ int index = met.getArguments().indexOf(path.getLeaf());
+
+ TypeMirror method = trees.getElement(new TreePath(path, met.getMethodSelect())).asType();
Review comment:
the new code:
https://github.com/apache/netbeans/blob/96f3355749c8500a53cae56a98abe2288887521a/java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java#L521-L531
--
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 #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien edited a comment on pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#issuecomment-1003738718
> My main comment to this patch as such is that I think it is preferred to not have hints/fixes that immediately lead to another hint/fix. I think I would prefer if the changes would be done in one step, unless it is very difficult to do so.
I thought about this a lot while writing jackpot scripts in past (I uploaded some general purpose ones here: https://github.com/mbien/jackpot-inspections). At first I tried to maintain "shortcuts" which attempted to refactor everything in one step, but quickly saw that this would let the rule count explode. A shortcut is also only a single path from a specific bad code pattern to clean code, there are more creative ways of bad code than there can be shortcut rules - finding them all is a bit hopeless.
Additionally, experience shows from refactoring multiple projects from old java to new java that rules interact in fairly unpredictable ways. I always apply the same set of rules twice, since the second time will usually find something new if the project is large enough - its not really possible to account for that in a shortcut rule.
(Maybe its worth investigating if there would be a way to automatically merge rules. If a rule triggers, it could check if the refactoring would trigger another one.)
But back to this concrete PR. The problem here was that the rules were all over the place. There are autoboxing rules in Tiny and also several in the jdk package. There were gaps, some overlapped and some were not correct (as noticed in https://github.com/apache/netbeans/pull/3221). Its probably worth to move everything into one package and see after that if some steps could be left out somehow by merging rules (but this would be a different PR).
>
> Thanks for working on this!
no problem
--
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 change in pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien commented on a change in pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#discussion_r777224793
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
+ srcType = ctx.getInfo().getTypes().unboxedType(srcType);
+ String[] replacement = PARSE_METHODS.get(srcType.getKind());
+ type = replacement[0];
+ method = replacement[1];
+ } else if (!(destType instanceof PrimitiveType) && srcType instanceof PrimitiveType) {
+ type = PARSE_METHODS.get(srcType.getKind())[0];
+ method = "valueOf"; // NOI18N
+ } else {
+ return null; // nothing to do, a different rule handles .intValue() boxing problems
}
- if (resType.getKind() == TypeKind.BOOLEAN) {
- if (ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
- // might alter new Boolean($v) to Boolean.valueOf($v), but that's all we can do. JDK < 5 has no
- // primitive-valued pasre* method for booleans.
- return null;
- }
+
+ if (srcType.getKind() == TypeKind.BOOLEAN && ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
+ return null; // JDK < 5 has no primitive-valued pasre* method for booleans.
}
- String[] arr = PARSE_METHODS.get(resType.getKind());
- if (arr == null) {
- return null; // just in case
+
+ Fix fix = JavaFixUtilities.rewriteFix(ctx, Bundle.FIX_UnnecessaryTempFromString1(type, method), ctx.getPath(), type + "." + method + "($v)"); // NOI18N
+ return ErrorDescriptionFactory.forTree(ctx, ctx.getPath(), Bundle.TEXT_UnnecessaryTempFromString(), fix); // NOI18N
+ }
+
+ private static TypeMirror getDestinationType(HintContext ctx, TreePath path) {
Review comment:
awesome! I tried to find a utility like this but must have overlooked it. Replacement was easy.
--
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 pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien commented on pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#issuecomment-1003738718
> My main comment to this patch as such is that I think it is preferred to not have hints/fixes that immediately lead to another hint/fix. I think I would prefer if the changes would be done in one step, unless it is very difficult to do so.
I thought about this a lot while writing jackpot scripts in past (I uploaded some general purpose ones here: https://github.com/mbien/jackpot-inspections). At first I tried to maintain "shortcuts" which attempted to refactor everything in one step, but quickly saw that this would let the rule count explode. A shortcut is also only a single path from a specific bad code pattern to clean code, there are more creative ways of bad code than there can be shortcut rules - finding them all is a bit hopeless.
Additionally, experience shows from refactoring multiple projects from old java to new java that rules interact in fairly unpredictable ways. I always apply the same set of rules twice, since the second time will usually find something new if the project is large enough - its not really possible to account for that in a shortcut rule.
(Maybe its worth investigating if there would be a way to automatically merge rules. If a rule triggers, it could check if the refactoring would trigger another one.)
But back to this concrete PR. The problem here was that the rules were all over the place. There are autoboxing rules in Tiny and also several in the jdk package. There were gaps, some overlapped and some were not correct (as noticed in https://github.com/apache/netbeans/pull/3221). Its probably worth to move everything into one package and see after that if some steps could be left out somehow by merging rules.
>
> Thanks for working on this!
no problem
--
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 change in pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien commented on a change in pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#discussion_r817669270
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
+ srcType = ctx.getInfo().getTypes().unboxedType(srcType);
+ String[] replacement = PARSE_METHODS.get(srcType.getKind());
+ type = replacement[0];
+ method = replacement[1];
+ } else if (!(destType instanceof PrimitiveType) && srcType instanceof PrimitiveType) {
+ type = PARSE_METHODS.get(srcType.getKind())[0];
+ method = "valueOf"; // NOI18N
+ } else {
+ return null; // nothing to do, a different rule handles .intValue() boxing problems
}
- if (resType.getKind() == TypeKind.BOOLEAN) {
- if (ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
- // might alter new Boolean($v) to Boolean.valueOf($v), but that's all we can do. JDK < 5 has no
- // primitive-valued pasre* method for booleans.
- return null;
- }
+
+ if (srcType.getKind() == TypeKind.BOOLEAN && ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
+ return null; // JDK < 5 has no primitive-valued pasre* method for booleans.
}
- String[] arr = PARSE_METHODS.get(resType.getKind());
- if (arr == null) {
- return null; // just in case
+
+ Fix fix = JavaFixUtilities.rewriteFix(ctx, Bundle.FIX_UnnecessaryTempFromString1(type, method), ctx.getPath(), type + "." + method + "($v)"); // NOI18N
+ return ErrorDescriptionFactory.forTree(ctx, ctx.getPath(), Bundle.TEXT_UnnecessaryTempFromString(), fix); // NOI18N
+ }
+
+ private static TypeMirror getDestinationType(HintContext ctx, TreePath path) {
+
+ TreePath parent = path.getParentPath();
+ Tree parentLeaf = parent.getLeaf();
+
+ Types types = ctx.getInfo().getTypes();
+ Trees trees = ctx.getInfo().getTrees();
+
+ if (parentLeaf instanceof MethodInvocationTree) {
+
+ MethodInvocationTree met = (MethodInvocationTree) parentLeaf;
+
+ int index = met.getArguments().indexOf(path.getLeaf());
+
+ TypeMirror method = trees.getElement(new TreePath(path, met.getMethodSelect())).asType();
Review comment:
@jlahoda ping :)
--
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 pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien commented on pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#issuecomment-1014112103
my mistake. I forgot to press the re-request review button two weeks ago.
--
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 #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#issuecomment-1002769915
@jlahoda could you please give this a look?
--
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 change in pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien commented on a change in pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#discussion_r777225444
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
+ srcType = ctx.getInfo().getTypes().unboxedType(srcType);
+ String[] replacement = PARSE_METHODS.get(srcType.getKind());
+ type = replacement[0];
+ method = replacement[1];
+ } else if (!(destType instanceof PrimitiveType) && srcType instanceof PrimitiveType) {
+ type = PARSE_METHODS.get(srcType.getKind())[0];
+ method = "valueOf"; // NOI18N
+ } else {
+ return null; // nothing to do, a different rule handles .intValue() boxing problems
}
- if (resType.getKind() == TypeKind.BOOLEAN) {
- if (ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
- // might alter new Boolean($v) to Boolean.valueOf($v), but that's all we can do. JDK < 5 has no
- // primitive-valued pasre* method for booleans.
- return null;
- }
+
+ if (srcType.getKind() == TypeKind.BOOLEAN && ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
+ return null; // JDK < 5 has no primitive-valued pasre* method for booleans.
}
- String[] arr = PARSE_METHODS.get(resType.getKind());
- if (arr == null) {
- return null; // just in case
+
+ Fix fix = JavaFixUtilities.rewriteFix(ctx, Bundle.FIX_UnnecessaryTempFromString1(type, method), ctx.getPath(), type + "." + method + "($v)"); // NOI18N
+ return ErrorDescriptionFactory.forTree(ctx, ctx.getPath(), Bundle.TEXT_UnnecessaryTempFromString(), fix); // NOI18N
+ }
+
+ private static TypeMirror getDestinationType(HintContext ctx, TreePath path) {
+
+ TreePath parent = path.getParentPath();
+ Tree parentLeaf = parent.getLeaf();
+
+ Types types = ctx.getInfo().getTypes();
+ Trees trees = ctx.getInfo().getTrees();
+
+ if (parentLeaf instanceof MethodInvocationTree) {
+
+ MethodInvocationTree met = (MethodInvocationTree) parentLeaf;
+
+ int index = met.getArguments().indexOf(path.getLeaf());
+
+ TypeMirror method = trees.getElement(new TreePath(path, met.getMethodSelect())).asType();
Review comment:
I tried getTypeMirror first actually and it worked for methods, but I could not figure out how to make the NewClassTree work too. So instead of using two ways of handling very similar branches of an if, i decided to solve both the same.
The current version I just committed uses getTypeMirror for the method branch and Element.asType for the class branch. I would need some help to make both use getTypeMirror (i am missing the getMethodSelect() equivalent for constructors) - since i could not figure out how.
--
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] jlahoda commented on a change in pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
jlahoda commented on a change in pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#discussion_r777135856
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
+ srcType = ctx.getInfo().getTypes().unboxedType(srcType);
+ String[] replacement = PARSE_METHODS.get(srcType.getKind());
+ type = replacement[0];
+ method = replacement[1];
+ } else if (!(destType instanceof PrimitiveType) && srcType instanceof PrimitiveType) {
+ type = PARSE_METHODS.get(srcType.getKind())[0];
+ method = "valueOf"; // NOI18N
+ } else {
+ return null; // nothing to do, a different rule handles .intValue() boxing problems
}
- if (resType.getKind() == TypeKind.BOOLEAN) {
- if (ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
- // might alter new Boolean($v) to Boolean.valueOf($v), but that's all we can do. JDK < 5 has no
- // primitive-valued pasre* method for booleans.
- return null;
- }
+
+ if (srcType.getKind() == TypeKind.BOOLEAN && ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
+ return null; // JDK < 5 has no primitive-valued pasre* method for booleans.
}
- String[] arr = PARSE_METHODS.get(resType.getKind());
- if (arr == null) {
- return null; // just in case
+
+ Fix fix = JavaFixUtilities.rewriteFix(ctx, Bundle.FIX_UnnecessaryTempFromString1(type, method), ctx.getPath(), type + "." + method + "($v)"); // NOI18N
+ return ErrorDescriptionFactory.forTree(ctx, ctx.getPath(), Bundle.TEXT_UnnecessaryTempFromString(), fix); // NOI18N
+ }
+
+ private static TypeMirror getDestinationType(HintContext ctx, TreePath path) {
Review comment:
There is `java/java.hints/src/org/netbeans/modules/java/hints/errors/CreateElementUtilities.java#resolveType` which presumably does mostly what is needed here. There are some tricks played there, but I hope that could work.
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
Review comment:
Generally, the javac APIs discourage `instanceof` and encourage use of `.getKind() == <type>` (or probably `.getKind().isPrimitive()` in this case), even though that obviously lacks the implicit null check. The reason is that one implementation class may implement many of these interfaces (applies to all `javax.lang.model.**` and `com.sun.source.**` that have kinds). As a real-world example, the `com.sun.tools.javac.code.Type.ClassType` implements both `DeclaredType` and `ErrorType`, so doing `instanceof DeclaredType` or `instanceof ErrorType` would not work properly. The `.getKind()` returns the correct kind for the given type.
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
+ srcType = ctx.getInfo().getTypes().unboxedType(srcType);
+ String[] replacement = PARSE_METHODS.get(srcType.getKind());
+ type = replacement[0];
+ method = replacement[1];
+ } else if (!(destType instanceof PrimitiveType) && srcType instanceof PrimitiveType) {
+ type = PARSE_METHODS.get(srcType.getKind())[0];
+ method = "valueOf"; // NOI18N
+ } else {
+ return null; // nothing to do, a different rule handles .intValue() boxing problems
}
- if (resType.getKind() == TypeKind.BOOLEAN) {
- if (ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
- // might alter new Boolean($v) to Boolean.valueOf($v), but that's all we can do. JDK < 5 has no
- // primitive-valued pasre* method for booleans.
- return null;
- }
+
+ if (srcType.getKind() == TypeKind.BOOLEAN && ctx.getInfo().getSourceVersion().compareTo(SourceVersion.RELEASE_5) < 0) {
+ return null; // JDK < 5 has no primitive-valued pasre* method for booleans.
}
- String[] arr = PARSE_METHODS.get(resType.getKind());
- if (arr == null) {
- return null; // just in case
+
+ Fix fix = JavaFixUtilities.rewriteFix(ctx, Bundle.FIX_UnnecessaryTempFromString1(type, method), ctx.getPath(), type + "." + method + "($v)"); // NOI18N
+ return ErrorDescriptionFactory.forTree(ctx, ctx.getPath(), Bundle.TEXT_UnnecessaryTempFromString(), fix); // NOI18N
+ }
+
+ private static TypeMirror getDestinationType(HintContext ctx, TreePath path) {
+
+ TreePath parent = path.getParentPath();
+ Tree parentLeaf = parent.getLeaf();
+
+ Types types = ctx.getInfo().getTypes();
+ Trees trees = ctx.getInfo().getTrees();
+
+ if (parentLeaf instanceof MethodInvocationTree) {
+
+ MethodInvocationTree met = (MethodInvocationTree) parentLeaf;
+
+ int index = met.getArguments().indexOf(path.getLeaf());
+
+ TypeMirror method = trees.getElement(new TreePath(path, met.getMethodSelect())).asType();
Review comment:
As a general note - doing `Element.asType()` will return the type of the generic declaration (i.e. a type with the generic type parameters, like e.g. `(E)boolean` for `List.add`). What is likely needed here is the actual types used for the method invocation, with generic types (if any) resolved (e.g. `(Integer)boolean` for `List<Integer> l; l.add(...)`). `trees.getTypeMirror(<method-select>)` should return that, AFAIK.
(Note there's a slight inconsistency/difference between the `getElement` and `getTypeMirror` methods:
-if the method select is passed into the methods, they will return the `ExecutableElement` and `ExecutableType` for the invocation
-if the method invocation is passed into the methods, `getElement` will still return the `ExecutableElement`, but `getTypeMirror` will return the actual return type (with all generics resolved as needed)
)
--
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 change in pull request #3240: [jackpot] Updates to autoboxing inspections
Posted by GitBox <gi...@apache.org>.
mbien commented on a change in pull request #3240:
URL: https://github.com/apache/netbeans/pull/3240#discussion_r777224573
##########
File path: java/java.hints/src/org/netbeans/modules/java/hints/perf/Tiny.java
##########
@@ -462,36 +485,94 @@ public static ErrorDescription redundantToString(HintContext ctx) {
suppressWarnings = "UnnecessaryTemporaryOnConversionFromString"
)
public static ErrorDescription unnecessaryTempFromString(HintContext ctx) {
- TypeMirror resType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
- if (resType == null) {
- return null;
+
+ String type;
+ String method;
+
+ // determine if the destination is primitive
+ TypeMirror destType = getDestinationType(ctx, ctx.getPath());
+ TypeMirror srcType = ctx.getInfo().getTrees().getTypeMirror(ctx.getPath());
+
+ if (destType instanceof PrimitiveType && !(srcType instanceof PrimitiveType)) {
Review comment:
good to know -> switched everything to getKind()
--
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