azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Detect recursive property accessors #KT-17221 Fixed

Browse Source
This commit is contained in:
Dmitry Neverov
2017-05-12 06:02:57 +02:00
committed by Mikhail Glukhikh
parent 20f5023c48
commit 3b4dafe691
12 changed files with 376 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
idea/resources/inspectionDescriptions/RecursivePropertyAccessor.html
+23
View File
@@ -0,0 +1,23 @@
<html>
<body>
Reports recursive property accessor calls which can end up with StackOverflowError
<p>
For example:
<code><pre>
<b>class</b> A {
<b>var</b> x = 0
<b>get</b>() {
<b>return</b> x //recursive getter call
}
<b>var</b> y = 0
<b>set</b>(value) {
<b>if</b> (value > 0) {
y = value //recursive setter call
}
}
}
</pre></code>
</p>
</body>
</html>
idea/src/META-INF/plugin.xml
+8
View File
@@ -2172,6 +2172,14 @@
language="kotlin"
/>
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.RecursivePropertyAccessorInspection"
displayName="Recursive property accessor"
groupName="Kotlin"
enabledByDefault="true"
level="WARNING"
language="kotlin"
/>
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>
idea/src/org/jetbrains/kotlin/idea/highlighter/KotlinRecursiveCallLineMarkerProvider.kt
+3
View File
@@ -27,6 +27,7 @@ import com.intellij.psi.PsiElement
import org.jetbrains.kotlin.descriptors.ClassDescriptor
import org.jetbrains.kotlin.descriptors.SimpleFunctionDescriptor
import org.jetbrains.kotlin.idea.caches.resolve.analyze
import org.jetbrains.kotlin.idea.inspections.RecursivePropertyAccessorInspection
import org.jetbrains.kotlin.idea.util.getThisReceiverOwner
import org.jetbrains.kotlin.lexer.KtToken
import org.jetbrains.kotlin.name.Name
@@ -75,6 +76,8 @@ class KotlinRecursiveCallLineMarkerProvider : LineMarkerProvider {
}
private fun isRecursiveCall(element: KtElement): Boolean {
if (RecursivePropertyAccessorInspection.isRecursivePropertyAccess(element)) return true
if (RecursivePropertyAccessorInspection.isRecursiveSyntheticPropertyAccess(element)) return true
// Fast check for names without resolve
val resolveName = getCallNameFromPsi(element) ?: return false
val enclosingFunction = getEnclosingFunction(element, false) ?: return false
idea/src/org/jetbrains/kotlin/idea/inspections/RecursivePropertyAccessorInspection.kt
+118
View File
@@ -0,0 +1,118 @@
/*
* Copyright 2010-2017 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jetbrains.kotlin.idea.inspections
import com.intellij.codeInspection.*
import com.intellij.openapi.project.Project
import com.intellij.psi.PsiElementVisitor
import com.intellij.psi.util.PsiTreeUtil
import org.jetbrains.kotlin.idea.caches.resolve.analyze
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.getParentOfType
import org.jetbrains.kotlin.psi.psiUtil.getStrictParentOfType
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.BindingContext.DECLARATION_TO_DESCRIPTOR
import org.jetbrains.kotlin.resolve.BindingContext.REFERENCE_TARGET
import org.jetbrains.kotlin.synthetic.SyntheticJavaPropertyDescriptor
class RecursivePropertyAccessorInspection : AbstractKotlinInspection() {
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor {
return object : KtVisitorVoid() {
override fun visitSimpleNameExpression(expression: KtSimpleNameExpression) {
super.visitSimpleNameExpression(expression)
if (isRecursivePropertyAccess(expression)) {
holder.registerProblem(expression,
"Recursive property accessor",
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
ReplaceWithFieldFix())
}
else if (isRecursiveSyntheticPropertyAccess(expression)) {
holder.registerProblem(expression,
"Recursive synthetic property accessor",
ProblemHighlightType.GENERIC_ERROR_OR_WARNING)
}
}
}
}
class ReplaceWithFieldFix : LocalQuickFix {
override fun getName() = "Replace with 'field'"
override fun getFamilyName() = name
override fun applyFix(project: Project, descriptor: ProblemDescriptor) {
val expression = descriptor.psiElement as KtExpression
val factory = KtPsiFactory(expression)
expression.replace(factory.createExpression("field"))
}
}
companion object {
private fun KtBinaryExpression?.isAssignmentTo(expression: KtSimpleNameExpression): Boolean =
this != null && KtPsiUtil.isAssignment(this) && PsiTreeUtil.isAncestor(left, expression, false)
private fun isSameAccessor(expression: KtSimpleNameExpression, isGetter: Boolean): Boolean {
val binaryExpr = expression.getStrictParentOfType<KtBinaryExpression>()
if (isGetter) {
if (binaryExpr.isAssignmentTo(expression)) {
return KtTokens.AUGMENTED_ASSIGNMENTS.contains(binaryExpr?.operationToken)
}
return true
}
else /* isSetter */ {
if (binaryExpr.isAssignmentTo(expression)) {
return true
}
val unaryExpr = expression.getStrictParentOfType<KtUnaryExpression>()
if (unaryExpr?.operationToken.let { it == KtTokens.PLUSPLUS || it == KtTokens.MINUSMINUS }) {
return true
}
}
return false
}
fun isRecursivePropertyAccess(element: KtElement): Boolean {
if (element !is KtSimpleNameExpression) return false
val propertyAccessor = element.getParentOfType<KtDeclarationWithBody>(true) as? KtPropertyAccessor ?: return false
if (element.text != propertyAccessor.property.name) return false
val bindingContext = element.analyze()
val target = bindingContext[REFERENCE_TARGET, element]
if (target != bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, propertyAccessor.property]) return false
return isSameAccessor(element, propertyAccessor.isGetter)
}
fun isRecursiveSyntheticPropertyAccess(element: KtElement): Boolean {
if (element !is KtSimpleNameExpression) return false
val namedFunction = element.getParentOfType<KtDeclarationWithBody>(true) as? KtNamedFunction ?: return false
val name = namedFunction.name ?: return false
val referencedName = element.text.capitalize()
val isGetter = name == "get$referencedName"
val isSetter = name == "set$referencedName"
if (!isGetter && !isSetter) return false
val bindingContext = element.analyze()
val syntheticDescriptor = bindingContext[REFERENCE_TARGET, element] as? SyntheticJavaPropertyDescriptor ?: return false
val namedFunctionDescriptor = bindingContext[DECLARATION_TO_DESCRIPTOR, namedFunction]
if (namedFunctionDescriptor != syntheticDescriptor.getMethod &&
namedFunctionDescriptor != syntheticDescriptor.setMethod) return false
return isSameAccessor(element, isGetter)
}
}
}
idea/testData/codeInsight/lineMarker/recursiveCall/propertyAccessors.kt
Vendored
+27
View File
@@ -0,0 +1,27 @@
class A {
var x = 0
set(value) {
<lineMarker descr="Recursive call">x</lineMarker>++
<lineMarker descr="Recursive call">x</lineMarker>+=1
<lineMarker descr="Recursive call">x</lineMarker> = value
}
var y = 0
get() {
println("$<lineMarker descr="Recursive call">y</lineMarker>")
<lineMarker descr="Recursive call">y</lineMarker>++
<lineMarker descr="Recursive call">y</lineMarker> += 1
return <lineMarker descr="Recursive call">y</lineMarker>
}
var z = 0
get() = <lineMarker descr="Recursive call">z</lineMarker>
var field = 0
get() {
return if (field != 0) field else -1
}
set(value) {
if (value >= 0) field = value
}
}
idea/testData/inspections/recursivePropertyAccessor/a/JavaInterface.java
Vendored
+7
View File
@@ -0,0 +1,7 @@
package a;
public interface JavaInterface {
String getSomething();
void setSomething(String value);
}
idea/testData/inspections/recursivePropertyAccessor/inspectionData/expected.xml
Vendored
+119
View File
@@ -0,0 +1,119 @@
<problems>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>4</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>5</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>6</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>11</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>12</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>13</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>14</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>18</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursiveSyntheticPropertyAccessor.kt</file>
<line>9</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="temp:///src/recursiveSyntheticPropertyAccessor.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive synthetic property accessor</description>
</problem>
<problem>
<file>recursiveSyntheticPropertyAccessor.kt</file>
<line>14</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="temp:///src/recursiveSyntheticPropertyAccessor.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive synthetic property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>27</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="temp:///src/recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>31</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="temp:///src/recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
<problem>
<file>recursivePropertyAccessors.kt</file>
<line>38</line>
<module>light_idea_test_case</module>
<entry_point TYPE="file" FQNAME="temp:///src/recursivePropertyAccessors.kt" />
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Recursive property accessor</problem_class>
<description>Recursive property accessor</description>
</problem>
</problems>
idea/testData/inspections/recursivePropertyAccessor/inspectionData/inspections.test
Vendored
+1
View File
@@ -0,0 +1 @@
// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.RecursivePropertyAccessorInspection
idea/testData/inspections/recursivePropertyAccessor/recursivePropertyAccessors.kt
Vendored
+41
View File
@@ -0,0 +1,41 @@
class A {
var x = 0
set(value) {
x++
x += 1
x = value
}
var y = 0
get() {
println("$y")
y++
y += 1
return y
}
var z = 0
get() = z
var w
set(value) {
field = w + value
}
var field = 0
get() {
this.field
return if (field != 0) field else -1
}
set(value) {
this.field = value
if (value >= 0) field = value
}
companion object {
var g = 0
set(value) {
A.g = 99
}
}
}
idea/testData/inspections/recursivePropertyAccessor/recursiveSyntheticPropertyAccessor.kt
Vendored
+17
View File
@@ -0,0 +1,17 @@
import a.JavaInterface
class B {
var something: String = "123"
class Nested : JavaInterface {
override fun setSomething(value: String) {
val x = something // OK
something = value
}
override fun getSomething(): String {
something = "456" // OK
return something
}
}
}
idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java
+6
View File
@@ -233,6 +233,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest {
doTest(fileName);
}
@TestMetadata("recursivePropertyAccessor/inspectionData/inspections.test")
public void testRecursivePropertyAccessor_inspectionData_Inspections_test() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/recursivePropertyAccessor/inspectionData/inspections.test");
doTest(fileName);
}
@TestMetadata("redundantIf/inspectionData/inspections.test")
public void testRedundantIf_inspectionData_Inspections_test() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/redundantIf/inspectionData/inspections.test");
idea/tests/org/jetbrains/kotlin/idea/codeInsight/LineMarkersTestGenerated.java
+6
View File
@@ -281,6 +281,12 @@ public class LineMarkersTestGenerated extends AbstractLineMarkersTest {
doTest(fileName);
}
@TestMetadata("propertyAccessors.kt")
public void testPropertyAccessors() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/codeInsight/lineMarker/recursiveCall/propertyAccessors.kt");
doTest(fileName);
}
@TestMetadata("severalCallsInOneLine.kt")
public void testSeveralCallsInOneLine() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("idea/testData/codeInsight/lineMarker/recursiveCall/severalCallsInOneLine.kt");