add redundant ref deref inspection with fix (#264)

pontem-network · Dec 24, 2024 · f60e335 · f60e335
1 parent e9e6329
commit f60e335
Show file tree

Hide file tree

Showing 5 changed files with 187 additions and 0 deletions.
diff --git a/src/main/kotlin/org/move/ide/inspections/MvRedundantRefDerefInspection.kt b/src/main/kotlin/org/move/ide/inspections/MvRedundantRefDerefInspection.kt
@@ -0,0 +1,49 @@
+package org.move.ide.inspections
+
+import com.intellij.codeInspection.ProblemHighlightType
+import com.intellij.codeInspection.ProblemsHolder
+import com.intellij.openapi.project.Project
+import com.intellij.psi.PsiFile
+import org.move.lang.core.psi.MvBorrowExpr
+import org.move.lang.core.psi.MvDerefExpr
+import org.move.lang.core.psi.MvExpr
+import org.move.lang.core.psi.MvParensExpr
+import org.move.lang.core.psi.MvVisitor
+import org.move.lang.core.psi.ext.unwrap
+
+class MvRedundantRefDerefInspection: MvLocalInspectionTool() {
+    override fun buildMvVisitor(
+        holder: ProblemsHolder,
+        isOnTheFly: Boolean
+    ): MvVisitor = object: MvVisitor() {
+
+        override fun visitDerefExpr(o: MvDerefExpr) {
+            val innerExpr = o.innerExpr
+            if (innerExpr is MvBorrowExpr) {
+                if (innerExpr.expr == null) return
+                holder.registerProblem(
+                    o,
+                    "Needless pair of `*` and `&` operators: consider removing them",
+                    ProblemHighlightType.WEAK_WARNING,
+                    RemoveRefDerefFix(o)
+                )
+            }
+        }
+    }
+
+    private class RemoveRefDerefFix(element: MvDerefExpr): DiagnosticFix<MvDerefExpr>(element) {
+
+        override fun getText(): String = "Remove needless `*`, `&` operators"
+
+        override fun invoke(project: Project, file: PsiFile, element: MvDerefExpr) {
+            val borrowExpr = element.innerExpr as? MvBorrowExpr ?: return
+            val itemExpr = borrowExpr.expr ?: return
+            element.replace(itemExpr)
+        }
+    }
+}
+
+private val MvDerefExpr.innerExpr: MvExpr? get() {
+    val expr = this.expr
+    return if (expr !is MvParensExpr) expr else expr.unwrap()
+}
diff --git a/src/main/kotlin/org/move/lang/core/psi/ext/MvParensExpr.kt b/src/main/kotlin/org/move/lang/core/psi/ext/MvParensExpr.kt
@@ -0,0 +1,9 @@
+package org.move.lang.core.psi.ext
+
+import org.move.lang.core.psi.MvExpr
+import org.move.lang.core.psi.MvParensExpr
+
+fun MvParensExpr.unwrap(): MvExpr? {
+    val expr = this.expr
+    return if (expr !is MvParensExpr) expr else expr.unwrap()
+}
diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml
@@ -305,6 +305,10 @@
                      displayName="Convert to compound expr"
                      enabledByDefault="true" level="WEAK WARNING"
                      implementationClass="org.move.ide.inspections.compilerV2.MvReplaceWithCompoundAssignmentInspection" />
+    <localInspection language="Move" groupName="Move"
+                     displayName="Needless pair of `*` and `&amp;` operators"
+                     enabledByDefault="true" level="WEAK WARNING"
+                     implementationClass="org.move.ide.inspections.MvRedundantRefDerefInspection" />
 
     <!--  cannot be run on-the-fly, therefore enabled  -->
     <globalInspection language="Move" groupName="Move"

diff --git a/src/main/resources/inspectionDescriptions/MvRedundantRefDeref.html b/src/main/resources/inspectionDescriptions/MvRedundantRefDeref.html
@@ -0,0 +1,5 @@
+<html>
+<body>
+<p>Needless pair of `*` and `&amp;` operators</p>
+</body>
+</html>
diff --git a/src/test/kotlin/org/move/ide/inspections/MvRedundantRefDerefInspectionTest.kt b/src/test/kotlin/org/move/ide/inspections/MvRedundantRefDerefInspectionTest.kt
@@ -0,0 +1,120 @@
+package org.move.ide.inspections
+
+import org.intellij.lang.annotations.Language
+import org.move.utils.tests.annotation.InspectionTestBase
+
+class MvRedundantRefDerefInspectionTest: InspectionTestBase(MvRedundantRefDerefInspection::class) {
+    fun `test no error`() = checkWarnings(
+        """
+        module 0x1::m {
+            fun main() {
+                &1;
+                *1;
+                **1;
+                &&1;
+            }
+        }        
+    """
+    )
+
+    fun `test no error for deref and then borrow as it can be a copy op`() = checkWarnings(
+        """
+        module 0x1::m {
+            fun main() {
+                &*1;
+            }
+        }        
+    """
+    )
+
+    fun `test error for borrow and then deref`() = checkFixByText(
+        """
+        module 0x1::m {
+            fun main() {
+                <weak_warning descr="Needless pair of `*` and `&` operators: consider removing them">*/*caret*/&1</weak_warning>;
+            }
+        }        
+    """, """
+        module 0x1::m {
+            fun main() {
+                1;
+            }
+        }        
+    """
+    )
+
+    fun `test error for borrow and then deref with parens`() = checkFixByText(
+        """
+        module 0x1::m {
+            fun main() {
+                <weak_warning descr="Needless pair of `*` and `&` operators: consider removing them">*/*caret*/(&1)</weak_warning>;
+            }
+        }        
+    """, """
+        module 0x1::m {
+            fun main() {
+                1;
+            }
+        }        
+    """
+    )
+
+    fun `test error for mutable borrow and then deref`() = checkFixByText(
+        """
+        module 0x1::m {
+            fun main() {
+                <weak_warning descr="Needless pair of `*` and `&` operators: consider removing them">*&mut /*caret*/1</weak_warning>;
+            }
+        }        
+    ""","""
+        module 0x1::m {
+            fun main() {
+                1;
+            }
+        }        
+    """,
+    )
+
+    fun `test error for mutable borrow and then deref with parens`() = checkFixByText(
+        """
+        module 0x1::m {
+            fun main() {
+                <weak_warning descr="Needless pair of `*` and `&` operators: consider removing them">*(&mut /*caret*/1)</weak_warning>;
+            }
+        }        
+    ""","""
+        module 0x1::m {
+            fun main() {
+                1;
+            }
+        }        
+    """,
+    )
+
+    fun `test error for mutable borrow and then deref with parens on item`() = checkFixByText(
+        """
+        module 0x1::m {
+            fun main() {
+                <weak_warning descr="Needless pair of `*` and `&` operators: consider removing them">*&mut (/*caret*/1)</weak_warning>;
+            }
+        }        
+    ""","""
+        module 0x1::m {
+            fun main() {
+                (1);
+            }
+        }        
+    """,
+    )
+
+    private fun checkFixByText(
+        @Language("Move") before: String,
+        @Language("Move") after: String,
+    ) = checkFixByText(
+        "Remove needless `*`, `&` operators",
+        before,
+        after,
+        checkWarn = false,
+        checkWeakWarn = true
+    )
+}