(INSP) Missing Else inspection (intellij-rust#716)

kumbayo · Oct 7, 2016 · 938f195 · 938f195
1 parent 763c30b
commit 938f195
Show file tree

Hide file tree

Showing 4 changed files with 164 additions and 0 deletions.
diff --git a/src/main/kotlin/org/rust/ide/inspections/RustMissingElseInspection.kt b/src/main/kotlin/org/rust/ide/inspections/RustMissingElseInspection.kt
@@ -0,0 +1,47 @@
+package org.rust.ide.inspections
+
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.openapi.util.TextRange
+import com.intellij.psi.PsiComment
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiWhiteSpace
+import org.rust.ide.inspections.fixes.SubstituteTextFix
+import org.rust.lang.core.psi.RustElementVisitor
+import org.rust.lang.core.psi.RustExprStmtElement
+import org.rust.lang.core.psi.RustIfExprElement
+
+/**
+ * Checks for potentially missing `else`s.
+ * A partial analogue of Clippy's suspicious_else_formatting.
+ * QuickFix: Change to `else if`
+ */
+class RustMissingElseInspection : RustLocalInspectionTool() {
+    override fun getDisplayName(): String = "Missing else"
+
+    override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean) =
+        object : RustElementVisitor() {
+            override fun visitExprStmt(expr: RustExprStmtElement) {
+                val firstIf = expr.extractIf() ?: return
+                val nextIf = expr.rightSiblings
+                    .dropWhile { (it is PsiWhiteSpace || it is PsiComment) && '\n' !in it.text }
+                    .firstOrNull()
+                    .extractIf() ?: return
+                val rangeStart = expr.startOffsetInParent + firstIf.textLength
+                val rangeLen = nextIf.expr.textRange.startOffset - firstIf.textRange.startOffset - firstIf.textLength
+                val fixRange = TextRange(nextIf.textRange.startOffset, nextIf.textRange.startOffset)
+                holder.registerProblem(
+                    expr.parent,
+                    TextRange(rangeStart, rangeStart + rangeLen),
+                    "Suspicious if. Did you mean `else if`?",
+                    SubstituteTextFix(expr.containingFile, fixRange, "else ", "Change to `else if`"))
+            }
+        }
+
+    private fun PsiElement?.extractIf(): RustIfExprElement? = when(this) {
+        is RustIfExprElement -> this
+        is RustExprStmtElement -> firstChild.extractIf()
+        else -> null
+    }
+
+    private val PsiElement.rightSiblings: Sequence<PsiElement> get() = generateSequence(this.nextSibling) { it.nextSibling }
+}
diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml
@@ -154,6 +154,11 @@
                          enabledByDefault="true" level="WARNING"
                          implementationClass="org.rust.ide.inspections.RustSuspiciousAssignmentInspection"/>
 
+        <localInspection language="Rust" groupName="Rust"
+                         displayName="Missing else"
+                         enabledByDefault="true" level="WARNING"
+                         implementationClass="org.rust.ide.inspections.RustMissingElseInspection"/>
+
         <localInspection language="Rust" groupName="Redeclared symbols"
                          displayName="Duplicate field"
                          enabledByDefault="true" level="ERROR"

diff --git a/src/main/resources/inspectionDescriptions/RustMissingElse.html b/src/main/resources/inspectionDescriptions/RustMissingElse.html
@@ -0,0 +1,6 @@
+<html>
+<body>
+Detects suspiciously looking <code>if</code>-statements with potentially missing <code>else</code>s.<br>
+Partially corresponds to <a href="https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting">suspicious_else_formatting</a> lint from Rust Clippy.
+</body>
+</html>
diff --git a/src/test/kotlin/org/rust/ide/inspections/RustMissingElseInspectionTest.kt b/src/test/kotlin/org/rust/ide/inspections/RustMissingElseInspectionTest.kt
@@ -0,0 +1,106 @@
+package org.rust.ide.inspections
+
+/**
+ * Tests for Missing Else inspection.
+ */
+class RustMissingElseInspectionTest : RustInspectionsTestBase() {
+
+    override val dataPath = ""
+
+    fun testSimple() = checkByText<RustMissingElseInspection>("""
+        fn main() {
+            if true {
+            }<warning descr="Suspicious if. Did you mean `else if`?"> if </warning>true {
+            }
+        }
+    """)
+
+    fun testNoSpaces() = checkByText<RustMissingElseInspection>("""
+        fn main() {
+            let a = 10;
+            if true {
+            }<warning descr="Suspicious if. Did you mean `else if`?">if</warning>(a > 10){
+            }
+        }
+    """)
+
+    fun testWideSpaces() = checkByText<RustMissingElseInspection>("""
+        fn main() {
+            let a = 10;
+            if true {
+            }<warning descr="Suspicious if. Did you mean `else if`?">   if    </warning>(a > 10) {
+            }
+        }
+    """)
+
+    fun testComments() = checkByText<RustMissingElseInspection>("""
+        fn main() {
+            let a = 10;
+            if true {
+            }<warning descr="Suspicious if. Did you mean `else if`?"> /* commented  */ /* else */ if </warning>a > 10 {
+            }
+        }
+    """)
+
+    fun testNotLastExpr() = checkByText<RustMissingElseInspection>("""
+        fn main() {
+            let a = 10;
+            if a > 5 {
+            }<warning descr="Suspicious if. Did you mean `else if`?"> if </warning>a > 10{
+            }
+            let b = 20;
+        }
+    """)
+
+    fun testHandlesBlocksWithNoSiblingsCorrectly() = checkByText<RustMissingElseInspection>("""
+        fn main() {if true {}}
+    """)
+
+    fun testNotAppliedWhenLineBreakExists() = checkByText<RustMissingElseInspection>("""
+        fn main() {
+            if true {}
+            if true {}
+        }
+    """)
+
+    fun testNotAppliedWhenTheresNoSecondIf() = checkByText<RustMissingElseInspection>("""
+        fn main() {
+            if {
+                92;
+            }
+            {}
+        }
+    """)
+
+    fun testFix() = checkFixByText<RustMissingElseInspection>("Change to `else if`", """
+        fn main() {
+            let a = 10;
+            if a > 7 {
+            }<warning descr="Suspicious if. Did you mean `else if`?"> i<caret>f </warning>a > 14 {
+            }
+        }
+    """, """
+        fn main() {
+            let a = 10;
+            if a > 7 {
+            } else if a > 14 {
+            }
+        }
+    """)
+
+    fun testFixPreservesComments() = checkFixByText<RustMissingElseInspection>("Change to `else if`", """
+        fn main() {
+            let a = 10;
+            if a > 7 {
+            }<warning descr="Suspicious if. Did you mean `else if`?"> /* comment */<caret> if /* ! */ </warning>a > 14 {
+            }
+        }
+    """, """
+        fn main() {
+            let a = 10;
+            if a > 7 {
+            } /* comment */ else if /* ! */ a > 14 {
+            }
+        }
+    """)
+}