Skip to content

Commit

Permalink
Convert return statement to use Optional.ofNullable(xxx)/Optional.of(…
Browse files Browse the repository at this point in the history 
…xxx) in methods with Optional return type #56
  • Loading branch information
@markiewb
markiewb committed Apr 1, 2015
1 parent 69fab8a commit 2cac5a6
Show file tree
Hide file tree
Showing 8 changed files with 276 additions and 116 deletions.
8 changes: 5 additions & 3 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

<groupId>de.markiewb.netbeans.plugins</groupId>
<artifactId>AdditionalHints</artifactId>
<version>1.6.0</version>
<version>1.6.0.1</version>
<packaging>nbm</packaging>

<name>Additional Java hints</name>
Expand All @@ -14,7 +14,7 @@
<endorsed.dir>${project.build.directory}/endorsed</endorsed.dir>
<netbeans.version>RELEASE74</netbeans.version>
<netbeans.run.params.ide/>
<netbeans.run.params>-J-javaagent:"${current.jrebel.agent.path}" -J-Drebel.log=true ${netbeans.run.params.ide}</netbeans.run.params>
<!--<netbeans.run.params>-J-javaagent:"${current.jrebel.agent.path}" -J-Drebel.log=true ${netbeans.run.params.ide}</netbeans.run.params>-->
</properties>
<organization>
<name>Benno Markiewicz ([email protected])</name>
Expand Down Expand Up @@ -238,6 +238,7 @@
&lt;li&gt;"Detect dead instanceof-expressions" (since 1.4, no transformation)&lt;/li&gt;
&lt;li&gt;"Replace with Optional.isPresent()/Convert return null to return Optional.empty()" (since 1.5)&lt;/li&gt;
&lt;li&gt;Replace with null-assignment to Optional with Optional.empty() (since 1.6)&lt;/li&gt;
&lt;li&gt;Convert return xxx to return Optional.ofNullable(xxx)/Optional.of(xxx)/Optional.empty() (since 1.6)&lt;/li&gt;
&lt;/ul&gt;

&lt;h2&gt;Example:&lt;/h2&gt;
Expand All @@ -246,8 +247,9 @@
&lt;h2&gt;Updates&lt;/h2&gt;
&lt;h3&gt;1.6.0:&lt;/h3&gt;
&lt;ul&gt;
&lt;li&gt;[&lt;a href="https://github.com/markiewb/nb-additional-hints/issues/54"&gt;Updated Fix&lt;/a&gt;]: "Replace +..." works for more expressions&lt;/li&gt;
&lt;li&gt;[&lt;a href="https://github.com/markiewb/nb-additional-hints/issues/55"&gt;New Fix&lt;/a&gt;]: Replace with null-assignment to Optional with Optional.empty()&lt;/li&gt;
&lt;li&gt;[&lt;a href="https://github.com/markiewb/nb-additional-hints/issues/56"&gt;New Fix&lt;/a&gt;]: Convert return xxx to return Optional.ofNullable(xxx)/Optional.of(xxx)/Optional.empty()&lt;/li&gt;
&lt;li&gt;[&lt;a href="https://github.com/markiewb/nb-additional-hints/issues/54"&gt;Updated Fix&lt;/a&gt;]: "Replace +..." works for more expressions&lt;/li&gt;
&lt;/ul&gt;
&lt;h3&gt;1.5.0:&lt;/h3&gt;
&lt;ul&gt;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/de/markiewb/netbeans/plugins/hints/optional/AssignNullToOptional.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public class AssignNullToOptional {
})
@Hint(displayName = "#DN_AssignNull", description = "#DESC_AssignNull", category = "bugs", hintKind = Hint.Kind.INSPECTION, severity = Severity.ERROR)
@NbBundle.Messages("ERR_AssignNull=Replace with Optional.empty()")
public static ErrorDescription convertToOptional(HintContext ctx) {
public static ErrorDescription toFix(HintContext ctx) {
String result = null;
if (ctx.getVariables().containsKey("$var1")) {
result = "$var1 = Optional.empty()";
Expand Down
4 changes: 2 additions & 2 deletions ...lugins/hints/optional/AccessOptional.java → ...ugins/hints/optional/CompareOptional.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
@NbBundle.Messages({
"DN_AccessOptional=Replace with Optional.isPresent()",
"DESC_AccessOptional=<tt>java.util.Optional</tt> should not be null, so replace comparisons using <tt>null</tt> with <tt>isPresent()</tt>. <p>For example: <tt>Optional<T> var=...; if (null!=var){...}</tt> will be transformed to <tt>Optional<T> var=...; if (var.isPresent()){...}</tt></p><p>Provided by <a href=\"https://github.com/markiewb/nb-additional-hints\">nb-additional-hints</a> plugin</p>",})
public class AccessOptional {
public class CompareOptional {

@TriggerPatterns({
@TriggerPattern(value = "null!=$var1", constraints = @ConstraintVariableType(variable = "$var1", type = "java.util.Optional")),
Expand All @@ -70,7 +70,7 @@ public class AccessOptional {
})
@Hint(displayName = "#DN_AccessOptional", description = "#DESC_AccessOptional", category = "bugs", hintKind = Hint.Kind.INSPECTION, severity = Severity.ERROR)
@NbBundle.Messages("ERR_AccessOptional=Replace with Optional.isPresent()")
public static ErrorDescription convertToOptional(HintContext ctx) {
public static ErrorDescription toFix(HintContext ctx) {
String result = null;
if (ctx.getVariables().containsKey("$var1")) {
result = "$var1.isPresent()";
Expand Down
56 changes: 38 additions & 18 deletions ...hints/optional/ReturnNullForOptional.java → ...ins/hints/optional/ReturnForOptional.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@
package de.markiewb.netbeans.plugins.hints.optional;

import com.sun.source.tree.Tree;
import com.sun.source.tree.Tree.Kind;
import com.sun.source.util.TreePath;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.type.TypeMirror;
import org.netbeans.api.java.source.CompilationInfo;
import org.netbeans.spi.editor.hints.ErrorDescription;
import org.netbeans.spi.editor.hints.Fix;
import org.netbeans.spi.editor.hints.Severity;
Expand All @@ -54,38 +56,56 @@
import org.netbeans.spi.java.hints.HintContext;
import static org.netbeans.spi.java.hints.JavaFixUtilities.rewriteFix;
import org.netbeans.spi.java.hints.TriggerPattern;
import org.netbeans.spi.java.hints.TriggerTreeKind;
import org.openide.util.NbBundle.Messages;

/**
*
* @author markiewb
*/
@Messages({
"ERR_ReturnNullForOptional=Convert to Optional.empty(). The return value null looks suspicious for methods with return value java.util.Optional.",
"DN_ReturnNullForOptional=Convert return null to return Optional.empty()",
"DESC_ReturnNullForOptional=The return value <tt>null</tt> looks suspicious for methods with return value <tt>java.util.Optional</tt>, so replace it with <tt>Optional.empty()</tt>. For example <tt>return null</tt> will be transformed to <tt>return Optional.empty()</tt><p>Provided by <a href=\"https://github.com/markiewb/nb-additional-hints\">nb-additional-hints</a> plugin</p>"})
"ERR_ReturnForOptionalEmpty=Convert to Optional.empty(). The return value null looks suspicious for methods with return value java.util.Optional.",
"ERR_ReturnForOptionalNullable=Convert to Optional.ofNullable()/Optional.of(). The return value should be an Optional instance for a method with a return value java.util.Optional.",
"DN_ReturnForOptionalOfNullable=Convert to return Optional.ofNullable()",
"DN_ReturnForOptionalOf=Convert to return Optional.of()",
"DN_ReturnForOptionalEmpty=Convert to return Optional.empty()",
"DN_ReturnForOptional=Introduce java.util.Optional return values",
"DESC_ReturnForOptional=" + "Provides some fixes for return statements within methods with the return type <tt>java.util.Optional</tt>."
+ "<p>For example <ul><li><tt>return null</tt> will be transformed to <tt>return Optional.empty()</tt></li><li><tt>return xxx</tt> will be transformed to <tt>return Optional.of(xxx)</tt> or <tt>return Optional.ofNullable(xxx)</tt></li></ul></p><p>Provided by <a href=\"https://github.com/markiewb/nb-additional-hints\">nb-additional-hints</a> plugin</p>"})

public class ReturnNullForOptional {
public class ReturnForOptional {

@Hint(displayName = "#DN_ReturnNullForOptional", description = "#DESC_ReturnNullForOptional", category = "bugs", hintKind = Hint.Kind.INSPECTION, severity = Severity.ERROR)
@TriggerTreeKind(Tree.Kind.NULL_LITERAL)
public static ErrorDescription nullForOpt(HintContext ctx) {
@Hint(displayName = "#DN_ReturnForOptional", description = "#DESC_ReturnForOptional", category = "bugs", hintKind = Hint.Kind.INSPECTION, severity = Severity.ERROR)
@TriggerPattern("return $a")
public static ErrorDescription toFix(HintContext ctx) {

final TreePath nullTP = ctx.getPath();
final TreePath returnTP = nullTP.getParentPath();
if (null == returnTP || Tree.Kind.RETURN != returnTP.getLeaf().getKind()) {
return null;
}
final TreePath returnTP = ctx.getPath();
TreePath methodTP = getSurroundingMethod(returnTP);
if (null == methodTP) {
return null;
}
ExecutableElement method = (ExecutableElement) ctx.getInfo().getTrees().getElement(methodTP);
final String returnTyp = method.getReturnType().toString();
if (returnTyp.startsWith("java.util.Optional<") || returnTyp.equals("java.util.Optional")) {
Fix fix = rewriteFix(ctx, Bundle.DN_ReturnNullForOptional(), ctx.getPath(), "java.util.Optional.empty()");
return forName(ctx, ctx.getPath(), Bundle.ERR_ReturnNullForOptional(), fix);
final CompilationInfo ci = ctx.getInfo();
ExecutableElement method = (ExecutableElement) ci.getTrees().getElement(methodTP);
TypeMirror methodReturnType = method.getReturnType();
final String returnType = ci.getTypes().erasure(methodReturnType).toString();
if ("java.util.Optional".equals(returnType)) {

TreePath path = ctx.getVariables().get("$a");
//ignore existing "return Optional..."/"return o"
if (path.getLeaf().getKind()== Kind.IDENTIFIER || path.getLeaf().getKind()== Kind.METHOD_INVOCATION){
TypeMirror typeMirror = ci.getTypes().erasure(ci.getTrees().getTypeMirror(path));
if ("java.util.Optional".equals(typeMirror.toString())){
return null;
}
}

if (path.getLeaf().getKind() == Kind.NULL_LITERAL) {
Fix fix = rewriteFix(ctx, Bundle.DN_ReturnForOptionalEmpty(), returnTP, "return java.util.Optional.empty()");
return forName(ctx, returnTP, Bundle.ERR_ReturnForOptionalEmpty(), fix);
} else {
Fix fixA = rewriteFix(ctx, Bundle.DN_ReturnForOptionalOfNullable(), returnTP, "return java.util.Optional.ofNullable($a)");
Fix fixB = rewriteFix(ctx, Bundle.DN_ReturnForOptionalOf(), returnTP, "return java.util.Optional.of($a)");
return forName(ctx, returnTP, Bundle.ERR_ReturnForOptionalNullable(), fixA, fixB);
}

}
return null;
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/de/markiewb/netbeans/plugins/hints/optional/AccessOptionalTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public void testCaseA() throws Exception {
+ " }\n"
+ "}\n")
.sourceLevel("1.8")
.run(AccessOptional.class)
.run(CompareOptional.class)
.findWarning("4:12-4:19:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_AccessOptional())
.applyFix()
.assertCompilable()
Expand All @@ -51,7 +51,7 @@ public void testCaseB() throws Exception {
+ " }\n"
+ "}\n")
.sourceLevel("1.8")
.run(AccessOptional.class)
.run(CompareOptional.class)
.findWarning("4:12-4:19:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_AccessOptional())
.applyFix()
.assertCompilable()
Expand All @@ -76,7 +76,7 @@ public void testCaseC() throws Exception {
+ " }\n"
+ "}\n")
.sourceLevel("1.8")
.run(AccessOptional.class)
.run(CompareOptional.class)
.findWarning("4:12-4:19:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_AccessOptional())
.applyFix()
.assertCompilable()
Expand All @@ -101,7 +101,7 @@ public void testCaseD() throws Exception {
+ " }\n"
+ "}\n")
.sourceLevel("1.8")
.run(AccessOptional.class)
.run(CompareOptional.class)
.findWarning("4:12-4:19:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_AccessOptional())
.applyFix()
.assertCompilable()
Expand Down
Loading

0 comments on commit 2cac5a6

Please sign in to comment.