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

Compound assignment inspection #223

Merged
merged 2 commits into from
Oct 29, 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
25 changes: 19 additions & 6 deletions src/main/grammars/MoveLexer.flex
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,33 @@ QUOTE_IDENTIFIER=\'[_a-zA-Z][_a-zA-Z0-9]*
"!=" { return NOT_EQ; }

"!" { return EXCL; }

"<" { return LT; }
">" { return GT; }
"@" { return AT; }
"#" { return HASH; }
"=>" { return FAT_ARROW; }

"+=" { return PLUS_EQ; }
"-=" { return MINUS_EQ; }
"*=" { return MUL_EQ; }
"/=" { return DIV_EQ; }
"%=" { return MODULO_EQ; }

"+" { return PLUS; }
"-" { return MINUS; }
"*" { return MUL; }
"/" { return DIV; }
"%" { return MODULO; }
"^" { return XOR; }

"<" { return LT; }
">" { return GT; }
"&=" { return AND_EQ; }
"|=" { return OR_EQ; }
"^=" { return XOR_EQ; }

"&" { return AND; }
"|" { return OR; }
"@" { return AT; }
"#" { return HASH; }
"=>" { return FAT_ARROW; }
"^" { return XOR; }


// keywords
"script" { return SCRIPT_KW; }
Expand Down
11 changes: 11 additions & 0 deletions src/main/grammars/MoveParser.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
DOT = '.'

EXCL = '!'

PLUS = '+'
MINUS = '-'
XOR = '^'
Expand All @@ -101,6 +102,16 @@
DOT_DOT = '..'
FAT_ARROW = '=>'

PLUS_EQ = '+='
MINUS_EQ = '-='
MUL_EQ = '*='
DIV_EQ = '/='
MODULO_EQ = '%='

AND_EQ = '&='
OR_EQ = '|='
XOR_EQ = '^='

ADDRESS = 'address_kw'
HAS = 'has_kw'
IS = 'is_kw'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package org.move.ide.inspections.compilerV2

import com.intellij.codeInsight.PsiEquivalenceUtil
import com.intellij.codeInspection.ProblemHighlightType.WEAK_WARNING
import com.intellij.codeInspection.ProblemsHolder
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiFile
import org.move.ide.inspections.DiagnosticFix
import org.move.lang.core.MOVE_ARITHMETIC_BINARY_OPS
import org.move.lang.core.psi.MvAssignmentExpr
import org.move.lang.core.psi.MvBinaryExpr
import org.move.lang.core.psi.ext.elementType
import org.move.lang.core.psi.ext.operator
import org.move.lang.core.psi.psiFactory

class MvReplaceWithCompoundAssignmentInspection:
Move2OnlyInspectionBase<MvAssignmentExpr>(MvAssignmentExpr::class.java) {

override fun visitTargetElement(element: MvAssignmentExpr, holder: ProblemsHolder, isOnTheFly: Boolean) {
val lhsExpr = element.expr
val initializerExpr = element.initializer.expr ?: return
if (initializerExpr is MvBinaryExpr
&& initializerExpr.operator.elementType in MOVE_ARITHMETIC_BINARY_OPS
) {
// take lhs of binary plus expr
val argumentExpr = initializerExpr.left
if (PsiEquivalenceUtil.areElementsEquivalent(lhsExpr, argumentExpr)) {
val op = initializerExpr.operator.text
holder.registerProblem(
element,
"Can be replaced with compound assignment",
WEAK_WARNING,
ReplaceWithCompoundAssignmentFix(element, op)
)
}
}
}

class ReplaceWithCompoundAssignmentFix(assignmentExpr: MvAssignmentExpr, val op: String):
DiagnosticFix<MvAssignmentExpr>(assignmentExpr) {

override fun getText(): String = "Replace with compound assignment expr"

override fun invoke(project: Project, file: PsiFile, element: MvAssignmentExpr) {
val lhsExpr = element.expr
val rhsExpr = (element.initializer.expr as? MvBinaryExpr)?.right ?: return

val psiFactory = project.psiFactory
val assignBinExpr = psiFactory.expr<MvBinaryExpr>("x $op= 1")
assignBinExpr.left.replace(lhsExpr)
assignBinExpr.right?.replace(rhsExpr)

element.replace(assignBinExpr)
}
}
}
11 changes: 9 additions & 2 deletions src/main/kotlin/org/move/lang/core/MvTokenType.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import org.move.lang.MoveParserDefinition.Companion.EOL_COMMENT
import org.move.lang.MoveParserDefinition.Companion.EOL_DOC_COMMENT
import org.move.lang.MvElementTypes.*

class MvTokenType(debugName: String) : IElementType(debugName, MoveLanguage)
class MvTokenType(debugName: String): IElementType(debugName, MoveLanguage)

fun tokenSetOf(vararg tokens: IElementType) = TokenSet.create(*tokens)

Expand All @@ -28,6 +28,11 @@ val TYPES = tokenSetOf(PATH_TYPE, REF_TYPE, TUPLE_TYPE)

val MOVE_COMMENTS = tokenSetOf(BLOCK_COMMENT, EOL_COMMENT, EOL_DOC_COMMENT)

val MOVE_ARITHMETIC_BINARY_OPS = tokenSetOf(
PLUS, MINUS, MUL, DIV, MODULO,
AND, OR, XOR,
LT_LT, GT_GT,
)
val MOVE_BINARY_OPS = tokenSetOf(
OR_OR, AND_AND,
EQ_EQ_GT, LT_EQ_EQ_GT,
Expand All @@ -37,7 +42,9 @@ val MOVE_BINARY_OPS = tokenSetOf(
OR, AND, XOR,
MUL, DIV, MODULO,
PLUS, MINUS,
XOR, AND, OR
XOR, AND, OR,
PLUS_EQ, MINUS_EQ, MUL_EQ, DIV_EQ, MODULO_EQ,
AND_EQ, OR_EQ, XOR_EQ, GT_GT_EQ, LT_LT_EQ,
)

val LIST_OPEN_SYMBOLS = tokenSetOf(L_PAREN, LT)
Expand Down
3 changes: 3 additions & 0 deletions src/main/kotlin/org/move/lang/core/psi/ext/MvBinaryExpr.kt
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package org.move.lang.core.psi.ext

import com.intellij.lang.ASTNode
import com.intellij.psi.PsiElement
import org.move.lang.MvElementTypes.BINARY_OP
import org.move.lang.core.psi.MvBinaryExpr
import org.move.lang.core.psi.MvElementImpl

val MvBinaryExpr.operator: PsiElement get() = binaryOp.operator

abstract class MvBinaryExprMixin(node: ASTNode): MvElementImpl(node),
MvBinaryExpr {
override fun toString(): String {
Expand Down
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 @@ -295,6 +295,10 @@
displayName="Convert to index expr"
enabledByDefault="true" level="WEAK WARNING"
implementationClass="org.move.ide.inspections.compilerV2.MvReplaceWithIndexExprInspection" />
<localInspection language="Move" groupName="Move"
displayName="Convert to compound expr"
enabledByDefault="true" level="WEAK WARNING"
implementationClass="org.move.ide.inspections.compilerV2.MvReplaceWithCompoundAssignmentInspection" />

<!-- 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>
Replaces `x = x + 1` with `x += 1`
</body>
</html>
1 change: 1 addition & 0 deletions src/test/kotlin/org/move/ide/formatter/FormatterTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ class FormatterTest : MvFormatterTestBase() {
fun `test inner block`() = doTest()
fun `test expressions`() = doTest()
fun `test chop wraps`() = doTest()
fun `test compound assignments`() = doTest()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package org.move.ide.inspections.compilerV2

import org.intellij.lang.annotations.Language
import org.move.utils.tests.MoveV2
import org.move.utils.tests.annotation.InspectionTestBase

@MoveV2
class MvReplaceWithCompoundAssignmentInspectionTest:
InspectionTestBase(MvReplaceWithCompoundAssignmentInspection::class) {

fun `test replace variable assignment with plus`() = doFixTest(
"""
module 0x1::m {
fun main() {
let x = 1;
<weak_warning descr="Can be replaced with compound assignment">/*caret*/x = x + 1</weak_warning>;
}
}
""", """
module 0x1::m {
fun main() {
let x = 1;
x += 1;
}
}
"""
)

fun `test replace variable assignment with left shift`() = doFixTest(
"""
module 0x1::m {
fun main() {
let x = 1;
<weak_warning descr="Can be replaced with compound assignment">/*caret*/x = x << 1</weak_warning>;
}
}
""", """
module 0x1::m {
fun main() {
let x = 1;
x <<= 1;
}
}
"""
)

fun `test replace deref assignment with plus`() = doFixTest(
"""
module 0x1::m {
fun main(p: &u8) {
<weak_warning descr="Can be replaced with compound assignment">/*caret*/*p = *p + 1</weak_warning>;
}
}
""", """
module 0x1::m {
fun main(p: &u8) {
*p += 1;
}
}
"""
)

private fun doTest(@Language("Move") text: String) =
checkByText(text, checkWarn = false, checkWeakWarn = true)

private fun doFixTest(
@Language("Move") before: String,
@Language("Move") after: String,
) =
checkFixByText("Replace with compound assignment expr", before, after,
checkWarn = false, checkWeakWarn = true)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module 0x1::compound_assignments {
fun main() {
x += 1;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module 0x1::compound_assignments {
fun main() {
x += 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ FILE
PsiElement(IDENTIFIER)('x')
PsiWhiteSpace(' ')
MvBinaryOpImpl(BINARY_OP)
PsiElement(+)('+')
PsiElement(=)('=')
PsiElement(+=)('+=')
PsiWhiteSpace(' ')
MvLitExprImpl(LIT_EXPR)
PsiElement(INTEGER_LITERAL)('1')
Expand All @@ -41,8 +40,7 @@ FILE
PsiElement(IDENTIFIER)('x')
PsiWhiteSpace(' ')
MvBinaryOpImpl(BINARY_OP)
PsiElement(-)('-')
PsiElement(=)('=')
PsiElement(-=)('-=')
PsiWhiteSpace(' ')
MvLitExprImpl(LIT_EXPR)
PsiElement(INTEGER_LITERAL)('1')
Expand All @@ -55,8 +53,7 @@ FILE
PsiElement(IDENTIFIER)('x')
PsiWhiteSpace(' ')
MvBinaryOpImpl(BINARY_OP)
PsiElement(*)('*')
PsiElement(=)('=')
PsiElement(*=)('*=')
PsiWhiteSpace(' ')
MvLitExprImpl(LIT_EXPR)
PsiElement(INTEGER_LITERAL)('1')
Expand All @@ -69,8 +66,7 @@ FILE
PsiElement(IDENTIFIER)('x')
PsiWhiteSpace(' ')
MvBinaryOpImpl(BINARY_OP)
PsiElement(/)('/')
PsiElement(=)('=')
PsiElement(/=)('/=')
PsiWhiteSpace(' ')
MvLitExprImpl(LIT_EXPR)
PsiElement(INTEGER_LITERAL)('1')
Expand All @@ -83,8 +79,7 @@ FILE
PsiElement(IDENTIFIER)('x')
PsiWhiteSpace(' ')
MvBinaryOpImpl(BINARY_OP)
PsiElement(%)('%')
PsiElement(=)('=')
PsiElement(%=)('%=')
PsiWhiteSpace(' ')
MvLitExprImpl(LIT_EXPR)
PsiElement(INTEGER_LITERAL)('1')
Expand All @@ -97,8 +92,7 @@ FILE
PsiElement(IDENTIFIER)('x')
PsiWhiteSpace(' ')
MvBinaryOpImpl(BINARY_OP)
PsiElement(&)('&')
PsiElement(=)('=')
PsiElement(&=)('&=')
PsiWhiteSpace(' ')
MvLitExprImpl(LIT_EXPR)
PsiElement(INTEGER_LITERAL)('1')
Expand All @@ -111,8 +105,7 @@ FILE
PsiElement(IDENTIFIER)('x')
PsiWhiteSpace(' ')
MvBinaryOpImpl(BINARY_OP)
PsiElement(|)('|')
PsiElement(=)('=')
PsiElement(|=)('|=')
PsiWhiteSpace(' ')
MvLitExprImpl(LIT_EXPR)
PsiElement(INTEGER_LITERAL)('1')
Expand All @@ -125,8 +118,7 @@ FILE
PsiElement(IDENTIFIER)('x')
PsiWhiteSpace(' ')
MvBinaryOpImpl(BINARY_OP)
PsiElement(^)('^')
PsiElement(=)('=')
PsiElement(^=)('^=')
PsiWhiteSpace(' ')
MvLitExprImpl(LIT_EXPR)
PsiElement(INTEGER_LITERAL)('1')
Expand Down
Loading