Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add redundant ref deref inspection with fix #264

Merged
merged 1 commit into from
Dec 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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()
}
9 changes: 9 additions & 0 deletions src/main/kotlin/org/move/lang/core/psi/ext/MvParensExpr.kt
Original file line number Diff line number Diff line change
@@ -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()
}
4 changes: 4 additions & 0 deletions src/main/resources/META-INF/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<html>
<body>
<p>Needless pair of `*` and `&amp;` operators</p>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -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
)
}
Loading