From 9f40a707e0ff61522bb93beb998888b8dbfe4ee4 Mon Sep 17 00:00:00 2001
From: Andreas Dangel <adangel@users.sourceforge.net>
Date: Fri, 20 May 2016 19:42:32 +0200
Subject: [PATCH] Closes #1486 Add rule for no-else-return - New Javascript
 rule

---
 .../main/resources/rulesets/releases/550.xml  |  1 +
 .../rulesets/ecmascript/unnecessary.xml       | 33 ++++++++++++++
 .../unnecessary/UnnecessaryRulesTest.java     |  1 +
 .../rule/unnecessary/xml/NoElseReturn.xml     | 45 +++++++++++++++++++
 src/site/markdown/overview/changelog.md       |  6 +++
 5 files changed, 86 insertions(+)
 create mode 100644 pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/rule/unnecessary/xml/NoElseReturn.xml

diff --git a/pmd-core/src/main/resources/rulesets/releases/550.xml b/pmd-core/src/main/resources/rulesets/releases/550.xml
index bc8befdd96..16d30f6256 100644
--- a/pmd-core/src/main/resources/rulesets/releases/550.xml
+++ b/pmd-core/src/main/resources/rulesets/releases/550.xml
@@ -27,5 +27,6 @@ This ruleset contains links to rules that are new in PMD v5.5.0
     <rule ref="rulesets/apex/style.xml/AvoidLogicInTrigger" />
     <rule ref="rulesets/apex/style.xml/AvoidGlobalModifier" />
 
+    <rule ref="rulesets/ecmascript/unnecessary.xml/NoElseReturn" />
 </ruleset>
 
diff --git a/pmd-javascript/src/main/resources/rulesets/ecmascript/unnecessary.xml b/pmd-javascript/src/main/resources/rulesets/ecmascript/unnecessary.xml
index da600ffda4..473f9827b3 100644
--- a/pmd-javascript/src/main/resources/rulesets/ecmascript/unnecessary.xml
+++ b/pmd-javascript/src/main/resources/rulesets/ecmascript/unnecessary.xml
@@ -71,4 +71,37 @@ if (bar) {
     </example>
   </rule>
 
+  <rule name="NoElseReturn"
+        language="ecmascript"
+        since="5.5.0"
+        message="The else block is unnecessary"
+        class="net.sourceforge.pmd.lang.rule.XPathRule"
+        externalInfoUrl="${pmd.website.baseurl}/rules/ecmascript/unnecessary.html#NoElseReturn">
+    <description>The else block in a if-else-construct is unnecessary if the `if` block contains a return.
+    Then the content of the else block can be put outside.
+    
+    See also: http://eslint.org/docs/rules/no-else-return
+    </description>
+    <priority>3</priority>
+    <properties>
+        <property name="xpath"><value><![CDATA[
+//IfStatement[@Else="true"][Scope[1]/ReturnStatement]
+        ]]></value></property>
+    </properties>
+    <example><![CDATA[
+// Bad:
+if (x) {
+  return y;
+} else {
+  return z;
+}
+
+// Good:
+if (x) {
+  return y;
+}
+return z;
+    ]]></example>
+  </rule>
+
 </ruleset>
\ No newline at end of file
diff --git a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/rule/unnecessary/UnnecessaryRulesTest.java b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/rule/unnecessary/UnnecessaryRulesTest.java
index f1d3653b3e..cc8dc00354 100644
--- a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/rule/unnecessary/UnnecessaryRulesTest.java
+++ b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/rule/unnecessary/UnnecessaryRulesTest.java
@@ -13,5 +13,6 @@ public class UnnecessaryRulesTest extends SimpleAggregatorTst {
     public void setUp() {
         addRule(RULESET, "UnnecessaryBlock");
         addRule(RULESET, "UnnecessaryParentheses");
+        addRule(RULESET, "NoElseReturn");
     }
 }
diff --git a/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/rule/unnecessary/xml/NoElseReturn.xml b/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/rule/unnecessary/xml/NoElseReturn.xml
new file mode 100644
index 0000000000..d0a5fe7e7d
--- /dev/null
+++ b/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/rule/unnecessary/xml/NoElseReturn.xml
@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<test-data>
+    <test-code>
+        <description>Simple violation</description>
+        <expected-problems>1</expected-problems>
+        <code><![CDATA[
+function foo() {
+    if (x) {
+        return y;
+    } else {
+        return z;
+    }
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>Simple fix</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+function foo() {
+    if (x) {
+        return y;
+    }
+    return z;
+}
+        ]]></code>
+    </test-code>
+
+    <test-code>
+        <description>No return in if</description>
+        <expected-problems>0</expected-problems>
+        <code><![CDATA[
+function foo() {
+    if (x) {
+        x++;
+    } else {
+        return z;
+    }
+    return x;
+}
+        ]]></code>
+    </test-code>
+
+</test-data>
diff --git a/src/site/markdown/overview/changelog.md b/src/site/markdown/overview/changelog.md
index 20b0347e8c..5f49d991fb 100644
--- a/src/site/markdown/overview/changelog.md
+++ b/src/site/markdown/overview/changelog.md
@@ -19,6 +19,7 @@ you'll need a java8 runtime environment.
 *   CPD: New command line parameter `--ignore-usings`: Ignore using directives in C# when comparing text.
 *   A JSON-renderer for PMD which is compatible with CodeClimate. See [PR#83](https://github.com/pmd/pmd/pull/83).
 *   [#1360](https://sourceforge.net/p/pmd/bugs/1360/): Provide backwards compatibility for PMD configuration file
+*   [#1486](https://sourceforge.net/p/pmd/bugs/1486/): Add rule for no-else-return
 
 **New/Modified/Deprecated Rules:**
 
@@ -38,6 +39,11 @@ you'll need a java8 runtime environment.
     *   apex-style: VariableNamingConventions, MethodNamingConventions, ClassNamingConventions,
         MethodWithSameNameAsEnclosingClass, AvoidLogicInTrigger, AvoidGlobalModifier
 
+*   Javascript
+    *   New Rule: ecmascript-unnecessary/NoElseReturn: The else block in a if-else-construct is
+        unnecessary if the `if` block contains a return. Then the content of the else block can be
+        put outside.
+
 **Pull Requests:**
 
 *   [#25](https://github.com/adangel/pmd/pull/25): Added option to exclude C# using directives from CPD analysis