diff --git a/pom.xml b/pom.xml index 8e8be60..bbe0e50 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ de.markiewb.netbeans.plugins AdditionalHints - 1.6.0 + 1.6.0.1 nbm Additional Java hints @@ -14,7 +14,7 @@ ${project.build.directory}/endorsed RELEASE74 - -J-javaagent:"${current.jrebel.agent.path}" -J-Drebel.log=true ${netbeans.run.params.ide} + Benno Markiewicz (benno.markiewicz@googlemail.com) @@ -238,6 +238,7 @@ <li>"Detect dead instanceof-expressions" (since 1.4, no transformation)</li> <li>"Replace with Optional.isPresent()/Convert return null to return Optional.empty()" (since 1.5)</li> <li>Replace with null-assignment to Optional with Optional.empty() (since 1.6)</li> +<li>Convert return xxx to return Optional.ofNullable(xxx)/Optional.of(xxx)/Optional.empty() (since 1.6)</li> </ul> <h2>Example:</h2> @@ -246,8 +247,9 @@ <h2>Updates</h2> <h3>1.6.0:</h3> <ul> -<li>[<a href="https://github.com/markiewb/nb-additional-hints/issues/54">Updated Fix</a>]: "Replace +..." works for more expressions</li> <li>[<a href="https://github.com/markiewb/nb-additional-hints/issues/55">New Fix</a>]: Replace with null-assignment to Optional with Optional.empty()</li> +<li>[<a href="https://github.com/markiewb/nb-additional-hints/issues/56">New Fix</a>]: Convert return xxx to return Optional.ofNullable(xxx)/Optional.of(xxx)/Optional.empty()</li> +<li>[<a href="https://github.com/markiewb/nb-additional-hints/issues/54">Updated Fix</a>]: "Replace +..." works for more expressions</li> </ul> <h3>1.5.0:</h3> <ul> diff --git a/src/main/java/de/markiewb/netbeans/plugins/hints/optional/AssignNullToOptional.java b/src/main/java/de/markiewb/netbeans/plugins/hints/optional/AssignNullToOptional.java index b763214..f9181a1 100644 --- a/src/main/java/de/markiewb/netbeans/plugins/hints/optional/AssignNullToOptional.java +++ b/src/main/java/de/markiewb/netbeans/plugins/hints/optional/AssignNullToOptional.java @@ -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()"; diff --git a/src/main/java/de/markiewb/netbeans/plugins/hints/optional/AccessOptional.java b/src/main/java/de/markiewb/netbeans/plugins/hints/optional/CompareOptional.java similarity index 96% rename from src/main/java/de/markiewb/netbeans/plugins/hints/optional/AccessOptional.java rename to src/main/java/de/markiewb/netbeans/plugins/hints/optional/CompareOptional.java index 2d93c8b..a38bf59 100644 --- a/src/main/java/de/markiewb/netbeans/plugins/hints/optional/AccessOptional.java +++ b/src/main/java/de/markiewb/netbeans/plugins/hints/optional/CompareOptional.java @@ -60,7 +60,7 @@ @NbBundle.Messages({ "DN_AccessOptional=Replace with Optional.isPresent()", "DESC_AccessOptional=java.util.Optional should not be null, so replace comparisons using null with isPresent().

For example: Optional var=...; if (null!=var){...} will be transformed to Optional var=...; if (var.isPresent()){...}

Provided by nb-additional-hints plugin

",}) -public class AccessOptional { +public class CompareOptional { @TriggerPatterns({ @TriggerPattern(value = "null!=$var1", constraints = @ConstraintVariableType(variable = "$var1", type = "java.util.Optional")), @@ -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()"; diff --git a/src/main/java/de/markiewb/netbeans/plugins/hints/optional/ReturnNullForOptional.java b/src/main/java/de/markiewb/netbeans/plugins/hints/optional/ReturnForOptional.java similarity index 51% rename from src/main/java/de/markiewb/netbeans/plugins/hints/optional/ReturnNullForOptional.java rename to src/main/java/de/markiewb/netbeans/plugins/hints/optional/ReturnForOptional.java index cd2df1c..274526a 100644 --- a/src/main/java/de/markiewb/netbeans/plugins/hints/optional/ReturnNullForOptional.java +++ b/src/main/java/de/markiewb/netbeans/plugins/hints/optional/ReturnForOptional.java @@ -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; @@ -54,7 +56,6 @@ 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; /** @@ -62,30 +63,49 @@ * @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 null looks suspicious for methods with return value java.util.Optional, so replace it with Optional.empty(). For example return null will be transformed to return Optional.empty()

Provided by nb-additional-hints plugin

"}) + "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 java.util.Optional." + + "

For example

Provided by nb-additional-hints plugin

"}) -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; diff --git a/src/test/java/de/markiewb/netbeans/plugins/hints/optional/AccessOptionalTest.java b/src/test/java/de/markiewb/netbeans/plugins/hints/optional/AccessOptionalTest.java index 4afec71..9c04325 100644 --- a/src/test/java/de/markiewb/netbeans/plugins/hints/optional/AccessOptionalTest.java +++ b/src/test/java/de/markiewb/netbeans/plugins/hints/optional/AccessOptionalTest.java @@ -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() @@ -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() @@ -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() @@ -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() diff --git a/src/test/java/de/markiewb/netbeans/plugins/hints/optional/ReturnForOptionalTest.java b/src/test/java/de/markiewb/netbeans/plugins/hints/optional/ReturnForOptionalTest.java new file mode 100644 index 0000000..678f646 --- /dev/null +++ b/src/test/java/de/markiewb/netbeans/plugins/hints/optional/ReturnForOptionalTest.java @@ -0,0 +1,225 @@ +/* + * To change this license header, choose License Headers in Project Properties. + * To change this template file, choose Tools | Templates + * and open the template in the editor. + */ +package de.markiewb.netbeans.plugins.hints.optional; + +import java.util.Optional; +import org.junit.Test; +import org.netbeans.modules.java.hints.test.api.HintTest; + +/** + * + * @author markiewb + */ +public class ReturnForOptionalTest { + + @Test + public void testCaseSimpleCase_FullQualifiedName_null() throws Exception { + HintTest.create() + .input("package test;\n" + + "public class Test {\n" + + " public java.util.Optional method() {\n" + + " return null;\n" + + " }\n" + + "}") + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .findWarning("3:8-3:14:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnForOptionalEmpty()) + .applyFix() + .assertOutput("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public java.util.Optional method() {\n" + + " return Optional.empty();\n" + + " }\n" + + "}"); + + } + + @Test + public void testCaseSimpleCase_Nested_null() throws Exception { + HintTest.create() + .input("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " {{return null;}}\n" + + " }\n" + + "}") + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .findWarning("4:10-4:16:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnForOptionalEmpty()) + .applyFix() + .assertOutput("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " {{return Optional.empty();}}\n" + + " }\n" + + "}"); + + } + @Test + public void testCaseSimpleCase_ignoreExistingFQNOptional() throws Exception { + HintTest.create() + .input("package test;\n" + + "public class Test {\n" + + " public java.util.Optional method() {\n" + + " return java.util.Optional.empty();\n" + + " }\n" + + "}") + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .assertWarnings(); + + } + @Test + public void testCaseSimpleCase_ignoreExistingOptional() throws Exception { + HintTest.create() + .input("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return Optional.empty();\n" + + " }\n" + + "}") + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .assertWarnings(); + + } + + @Test + public void testCaseSimpleCase_ignoreExistingOptionalVariable() throws Exception { + HintTest.create() + .input("package test;\n" + + "public class Test {\n" + + " public java.util.Optional method(java.util.Optional o) {\n" + + " return o;\n" + + " }\n" + + "}") + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .assertWarnings(); + + } + @Test + public void testCaseSimpleCase_notNull_2_of() throws Exception { + HintTest.create() + .input("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return \"ABC\";\n" + + " }\n" + + "}", false) + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .findWarning("4:8-4:14:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnForOptionalNullable()) + .applyFix(Bundle.DN_ReturnForOptionalOf()) + .assertOutput("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return Optional.of(\"ABC\");\n" + + " }\n" + + "}"); + } + @Test + public void testCaseSimpleCase_notNull_2_ofNullable() throws Exception { + HintTest.create() + .input("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return \"ABC\";\n" + + " }\n" + + "}", false) + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .findWarning("4:8-4:14:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnForOptionalNullable()) + .applyFix(Bundle.DN_ReturnForOptionalOfNullable()) + .assertOutput("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return Optional.ofNullable(\"ABC\");\n" + + " }\n" + + "}"); + } + + @Test + public void testCaseSimpleCase_null() throws Exception { + HintTest.create() + .input("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return null;\n" + + " }\n" + + "}") + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .findWarning("4:8-4:14:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnForOptionalEmpty()) + .applyFix() + .assertOutput("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return Optional.empty();\n" + + " }\n" + + "}"); + + } + + @Test + public void testCaseSimpleCase_null_nontypedMethod() throws Exception { + HintTest.create() + .input("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return null;\n" + + " }\n" + + "}") + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .findWarning("4:8-4:14:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnForOptionalEmpty()) + .applyFix() + .assertOutput("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return Optional.empty();\n" + + " }\n" + + "}"); + + } + + @Test + public void testCaseSimpleCase_null_typedMethod() throws Exception { + HintTest.create() + .input("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return null;\n" + + " }\n" + + "}") + .sourceLevel("1.8") + .run(ReturnForOptional.class) + .findWarning("4:8-4:14:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnForOptionalEmpty()) + .applyFix() + .assertOutput("package test;\n" + + "import java.util.Optional;\n" + + "public class Test {\n" + + " public Optional method() {\n" + + " return Optional.empty();\n" + + " }\n" + + "}"); + + } + +} diff --git a/src/test/java/de/markiewb/netbeans/plugins/hints/optional/ReturnNullForOptionalTest.java b/src/test/java/de/markiewb/netbeans/plugins/hints/optional/ReturnNullForOptionalTest.java deleted file mode 100644 index 49660e5..0000000 --- a/src/test/java/de/markiewb/netbeans/plugins/hints/optional/ReturnNullForOptionalTest.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * To change this license header, choose License Headers in Project Properties. - * To change this template file, choose Tools | Templates - * and open the template in the editor. - */ -package de.markiewb.netbeans.plugins.hints.optional; - -import org.junit.Test; -import org.netbeans.modules.java.hints.test.api.HintTest; - -/** - * - * @author markiewb - */ -public class ReturnNullForOptionalTest { - - @Test - public void testCaseSimpleCase_FullQualifiedName() throws Exception { - HintTest.create() - .input("package test;\n" - + "public class Test {\n" - + " public java.util.Optional method() {\n" - + " return null;\n" - + " }\n" - + "}") - .sourceLevel("1.8") - .run(ReturnNullForOptional.class) - .findWarning("3:15-3:19:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnNullForOptional()) - .applyFix() - .assertOutput("package test;\n" - + "import java.util.Optional;\n" - + "public class Test {\n" - + " public java.util.Optional method() {\n" - + " return Optional.empty();\n" - + " }\n" - + "}"); - - } - - @Test - public void testCaseSimpleCase() throws Exception { - HintTest.create() - .input("package test;\n" - + "import java.util.Optional;\n" - + "public class Test {\n" - + " public Optional method() {\n" - + " return null;\n" - + " }\n" - + "}") - .sourceLevel("1.8") - .run(ReturnNullForOptional.class) - .findWarning("4:15-4:19:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnNullForOptional()) - .applyFix() - .assertOutput("package test;\n" - + "import java.util.Optional;\n" - + "public class Test {\n" - + " public Optional method() {\n" - + " return Optional.empty();\n" - + " }\n" - + "}"); - - } - - @Test - public void testCaseSimpleCase_Nested() throws Exception { - HintTest.create() - .input("package test;\n" - + "import java.util.Optional;\n" - + "public class Test {\n" - + " public Optional method() {\n" - + " {{return null;}}\n" - + " }\n" - + "}") - .sourceLevel("1.8") - .run(ReturnNullForOptional.class) - .findWarning("4:17-4:21:error:" + de.markiewb.netbeans.plugins.hints.optional.Bundle.ERR_ReturnNullForOptional()) - .applyFix() - .assertOutput("package test;\n" - + "import java.util.Optional;\n" - + "public class Test {\n" - + " public Optional method() {\n" - + " {{return Optional.empty();}}\n" - + " }\n" - + "}"); - - } - -} diff --git a/src/test/java/example/Optional.java b/src/test/java/example/Optional.java index e1504a1..6d683d5 100644 --- a/src/test/java/example/Optional.java +++ b/src/test/java/example/Optional.java @@ -25,6 +25,7 @@ public static Optional main(String[] args) { // } // // } +// return "String"; return null; } }