Skip to content

Commit 19aeb7c

Browse files
committed
Add some local mutability analysis to hide the intention when class is 100% mutable
1 parent eeb8a4e commit 19aeb7c

7 files changed

Lines changed: 111 additions & 2 deletions

File tree

src/main/kotlin/com/vk/kphpstorm/intentions/AddImmutableClassAnnotationIntention.kt

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ import com.intellij.codeInspection.util.IntentionName
66
import com.intellij.openapi.editor.Editor
77
import com.intellij.openapi.project.Project
88
import com.intellij.psi.PsiElement
9+
import com.intellij.psi.search.LocalSearchScope
10+
import com.intellij.psi.search.searches.ReferencesSearch
11+
import com.intellij.psi.util.findParentInFile
12+
import com.jetbrains.php.lang.psi.elements.AssignmentExpression
913
import com.jetbrains.php.lang.psi.elements.PhpClass
1014
import com.vk.kphpstorm.inspections.helpers.PhpDocPsiBuilder
1115
import com.vk.kphpstorm.kphptags.KphpImmutableClassDocTag
@@ -24,10 +28,46 @@ class AddImmutableClassAnnotationIntention : PsiElementBaseIntentionAction() {
2428
}
2529

2630
val klass = element.parent as PhpClass
27-
val klassDocNode = klass.docComment ?: return true
31+
val klassDocNode = klass.docComment
2832

2933
// do not suggest if already present
30-
return !KphpImmutableClassDocTag.existsInDocComment(klassDocNode)
34+
if (klassDocNode != null && KphpImmutableClassDocTag.existsInDocComment(klassDocNode)) {
35+
return false
36+
}
37+
38+
return !isClassLocallyImmutable(klass)
39+
}
40+
41+
/**
42+
* Simple local class mutability check. If there is any field mutation in class,
43+
* the class is mutable. The only exception is the class constructor
44+
*/
45+
private fun isClassLocallyImmutable(klass: PhpClass): Boolean {
46+
val searchScope = LocalSearchScope(klass)
47+
for (field in klass.fields) {
48+
val hasAnyMutation = ReferencesSearch.search(field, searchScope).any { ref ->
49+
val element = ref.element
50+
51+
isMutatingOp(element) && !isInClassConstructor(klass, element)
52+
}
53+
54+
if (hasAnyMutation) {
55+
return true
56+
}
57+
}
58+
59+
return false
60+
}
61+
62+
private fun isMutatingOp(psiElement: PsiElement): Boolean {
63+
val parent = psiElement.parent
64+
65+
return parent is AssignmentExpression
66+
}
67+
68+
private fun isInClassConstructor(klass: PhpClass, psiElement: PsiElement): Boolean {
69+
val classConstructor = klass.constructor
70+
return classConstructor != null && psiElement.findParentInFile { e -> e == classConstructor } != null
3171
}
3272

3373
override fun invoke(
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
class <caret>C1 {
4+
public int $field;
5+
6+
public function foo() {
7+
$this->field = 1; // mutate the field
8+
}
9+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
class <caret>C1 {
4+
public int $field;
5+
6+
public function __construct(int $arg)
7+
{
8+
$this->field = $arg; // field mutation in constructor, that's ok
9+
}
10+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
/**
4+
* @kphp-immutable-class
5+
*/
6+
class <caret>C1 {
7+
public int $field;
8+
9+
public function __construct(int $arg)
10+
{
11+
$this->field = $arg; // field mutation in constructor, that's ok
12+
}
13+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
class <caret>C1 {
4+
public int $field;
5+
}
6+
7+
public function foo()
8+
{
9+
$v = new C1();
10+
$v->field = 1; // mutation outside of a class, that's ok
11+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
/**
4+
* @kphp-immutable-class
5+
*/
6+
class <caret>C1 {
7+
public int $field;
8+
}
9+
10+
public function foo()
11+
{
12+
$v = new C1();
13+
$v->field = 1; // mutation outside of a class, that's ok
14+
}

src/test/kotlin/com/vk/kphpstorm/testing/tests/AddImmutableAnnoIntentionTest.kt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,16 @@ class AddImmutableAnnoIntentionTest : IntentionTestBase(AddImmutableClassAnnotat
1919
fun testAddImmutableAnno4() {
2020
assertNoIntention("kphp_intentions/immutable_class_intention-4.nointention.php")
2121
}
22+
23+
fun testAddImmutableAnno5() {
24+
assertNoIntention("kphp_intentions/immutable_class_intention-5.nointention.php")
25+
}
26+
27+
fun testAddImmutableAnno6() {
28+
runIntention("kphp_intentions/immutable_class_intention-6.fixture.php")
29+
}
30+
31+
fun testAddImmutableAnno7() {
32+
runIntention("kphp_intentions/immutable_class_intention-7.fixture.php")
33+
}
2234
}

0 commit comments

Comments
 (0)