Warn on for-in-array range variable assignment in loop body

According to KT-21354, this should be a warning in 1.2 and before, and
no warning (with changed semantics) in 1.3 and later.
NB there are some false positives in this check.

 #KT-21354 In Progress
 #KT-21321 In Progress
This commit is contained in:
Dmitry Petrov
2017-12-04 13:50:28 +03:00
parent 7118a4bf80
commit 7a6f80606b
11 changed files with 335 additions and 1 deletions
@@ -0,0 +1,84 @@
/*
* 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.resolve.jvm.checkers
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.config.LanguageFeature
import org.jetbrains.kotlin.descriptors.CallableDescriptor
import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor
import org.jetbrains.kotlin.descriptors.impl.SyntheticFieldDescriptor
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.psi.KtBinaryExpression
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.KtForExpression
import org.jetbrains.kotlin.psi.KtSimpleNameExpression
import org.jetbrains.kotlin.psi.psiUtil.parents
import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
import org.jetbrains.kotlin.resolve.calls.checkers.AdditionalTypeChecker
import org.jetbrains.kotlin.resolve.calls.context.ResolutionContext
import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue
import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValueFactory
import org.jetbrains.kotlin.resolve.jvm.diagnostics.ErrorsJvm
import org.jetbrains.kotlin.types.KotlinType
object JvmArrayVariableInLoopAssignmentChecker : AdditionalTypeChecker {
override fun checkType(
expression: KtExpression,
expressionType: KotlinType,
expressionTypeWithSmartCast: KotlinType,
c: ResolutionContext<*>
) {
if (c.languageVersionSettings.supportsFeature(LanguageFeature.ProperForInArrayLoopRangeVariableAssignmentSemantic)) return
val binaryExpression = expression as? KtBinaryExpression ?: return
if (binaryExpression.operationToken != KtTokens.EQ) return
val lhsExpression = binaryExpression.left as? KtSimpleNameExpression ?: return
val resolvedCall = lhsExpression.getResolvedCall(c.trace.bindingContext) ?: return
val variableDescriptor = resolvedCall.resultingDescriptor as? LocalVariableDescriptor ?: return
if (variableDescriptor is SyntheticFieldDescriptor) return
if (variableDescriptor.isDelegated) return
val variableType = variableDescriptor.returnType
if (!KotlinBuiltIns.isArrayOrPrimitiveArray(variableType)) return
if (!isOuterForLoopRangeVariable(expression, variableDescriptor, c)) return
val dataFlowValueKind = DataFlowValueFactory.createDataFlowValue(lhsExpression, variableType, c).kind
if (dataFlowValueKind != DataFlowValue.Kind.STABLE_VARIABLE) return
c.trace.report(ErrorsJvm.ASSIGNMENT_TO_ARRAY_LOOP_VARIABLE.on(lhsExpression))
}
private fun isOuterForLoopRangeVariable(
expression: KtExpression,
variableDescriptor: CallableDescriptor,
c: ResolutionContext<*>
): Boolean {
for (parent in expression.parents) {
if (parent is KtForExpression) {
val rangeExpression = parent.loopRange as? KtSimpleNameExpression ?: continue
val rangeResolvedCall = rangeExpression.getResolvedCall(c.trace.bindingContext) ?: continue
if (rangeResolvedCall.resultingDescriptor == variableDescriptor) {
return true
}
}
}
return false
}
}
@@ -127,6 +127,9 @@ public class DefaultErrorMessagesJvm implements DefaultErrorMessages.Extension {
MAP.put(JAVA_MODULE_DOES_NOT_EXPORT_PACKAGE, "Symbol is declared in module ''{0}'' which does not export package ''{1}''", STRING, STRING);
MAP.put(API_VERSION_IS_AT_LEAST_ARGUMENT_SHOULD_BE_CONSTANT, "'apiVersionIsAtLeast' argument should be a constant expression");
MAP.put(ASSIGNMENT_TO_ARRAY_LOOP_VARIABLE, "Assignment to a for-in-array loop range variable. Behavior may change in Kotlin 1.3. " +
"See https://youtrack.jetbrains.com/issue/KT-21354 for more details");
}
@NotNull
@@ -109,6 +109,8 @@ public interface ErrorsJvm {
DiagnosticFactory0<KtExpression> API_VERSION_IS_AT_LEAST_ARGUMENT_SHOULD_BE_CONSTANT = DiagnosticFactory0.create(WARNING);
DiagnosticFactory0<KtExpression> ASSIGNMENT_TO_ARRAY_LOOP_VARIABLE = DiagnosticFactory0.create(WARNING);
enum NullabilityInformationSource {
KOTLIN {
@NotNull
@@ -62,7 +62,8 @@ object JvmPlatformConfigurator : PlatformConfigurator(
JavaNullabilityChecker(),
RuntimeAssertionsTypeChecker,
JavaGenericVarianceViolationTypeChecker,
JavaTypeAccessibilityChecker()
JavaTypeAccessibilityChecker(),
JvmArrayVariableInLoopAssignmentChecker
),
additionalClassifierUsageCheckers = listOf(
@@ -0,0 +1,103 @@
// !LANGUAGE: -ProperForInArrayLoopRangeVariableAssignmentSemantic
// !DIAGNOSTICS: -UNUSED_VALUE
// SKIP_TXT
fun testArrayCapturedInLocalFun() {
var xs = arrayOf("a", "b", "c")
fun updateXs() {
xs = arrayOf("d", "e", "f")
}
for (x in xs) {
println(x)
updateXs()
}
}
fun testArrayCapturedInLabmda() {
var xs = arrayOf("a", "b", "c")
val updateXs = { xs = arrayOf("d", "e", "f") }
for (x in xs) {
println(x)
updateXs()
}
}
fun testArrayCapturedInInlineLambda() {
var xs = arrayOf("a", "b", "c")
for (x in xs) {
println(x)
run {
xs = arrayOf("d", "e", "f")
}
}
}
fun testArrayCapturedInLocalObject() {
var xs = arrayOf("a", "b", "c")
val updateXs = object : () -> Unit {
override fun invoke() {
xs = arrayOf("d", "e", "f")
}
}
for (x in xs) {
println(x)
updateXs()
}
}
fun testArrayCapturedInLocalClass() {
var xs = arrayOf("a", "b", "c")
class LocalClass {
fun updateXs() {
xs = arrayOf("d", "e", "f")
}
}
val updater = LocalClass()
for (x in xs) {
println(x)
updater.updateXs()
}
}
fun testCapturedInLambdaAfterLoop() {
// NB false positive
var xs = intArrayOf(1, 2, 3)
for (x in xs) {
println(x)
<!ASSIGNMENT_TO_ARRAY_LOOP_VARIABLE!>xs<!> = intArrayOf(4, 5, 6)
}
val lambda = { xs = intArrayOf() }
lambda()
}
fun testCapturedInLambdaInLoopAfterAssignment() {
// NB false positive
var xs = intArrayOf(1, 2, 3)
for (x in xs) {
println(x)
<!ASSIGNMENT_TO_ARRAY_LOOP_VARIABLE!>xs<!> = intArrayOf(4, 5, 6)
val lambda = { xs = intArrayOf() }
lambda()
}
}
fun testCapturedInNonChangingClosure() {
// NB false positive
var xs = intArrayOf(1, 2, 3)
val lambda = { println(xs) }
for (x in xs) {
println(x)
<!ASSIGNMENT_TO_ARRAY_LOOP_VARIABLE!>xs<!> = intArrayOf(4, 5, 6)
lambda()
}
}
@@ -0,0 +1,16 @@
// !LANGUAGE: -ProperForInArrayLoopRangeVariableAssignmentSemantic
// !DIAGNOSTICS: -UNUSED_VALUE
// SKIP_TXT
class Delegate<T>(var v: T) {
operator fun getValue(thisRef: Any?, kProp: Any?) = v
operator fun setValue(thisRef: Any?, kProp: Any?, value: T) { v = value }
}
fun testLocalDelegatedProperty() {
var xs by Delegate(arrayOf("a", "b", "c"))
for (x in xs) {
println(x)
xs = arrayOf("d", "e", "f")
}
}
@@ -0,0 +1,19 @@
// !LANGUAGE: +ProperForInArrayLoopRangeVariableAssignmentSemantic
// !DIAGNOSTICS: -UNUSED_VALUE
// SKIP_TXT
fun testObjectArray() {
var xs = arrayOf("a", "b", "c")
for (x in xs) {
println(x)
xs = arrayOf("d", "e", "f")
}
}
fun testPrimitiveArray() {
var xs = intArrayOf(1, 2, 3)
for (x in xs) {
println(x)
xs = intArrayOf(4, 5, 6)
}
}
@@ -0,0 +1,39 @@
// !LANGUAGE: -ProperForInArrayLoopRangeVariableAssignmentSemantic
// !DIAGNOSTICS: -UNUSED_VALUE
// SKIP_TXT
fun testObjectArray() {
var xs = arrayOf("a", "b", "c")
for (x in xs) {
println(x)
<!ASSIGNMENT_TO_ARRAY_LOOP_VARIABLE!>xs<!> = arrayOf("d", "e", "f")
}
}
fun testPrimitiveArray() {
var xs = intArrayOf(1, 2, 3)
for (x in xs) {
println(x)
<!ASSIGNMENT_TO_ARRAY_LOOP_VARIABLE!>xs<!> = intArrayOf(4, 5, 6)
}
}
var global = arrayOf("a", "b", "c")
fun testGlobalArray() {
for (x in global) {
println(x)
global = arrayOf("d", "e", "f")
}
}
fun testAssignmentNotInLoop() {
var xs = intArrayOf(1, 2, 3)
println(xs)
xs = intArrayOf(7, 8, 9)
for (x in xs) {
println(x)
}
xs = intArrayOf(4, 5, 6)
println(xs)
}
@@ -1746,6 +1746,39 @@ public class DiagnosticsTestWithStdLibGenerated extends AbstractDiagnosticsTestW
}
}
@TestMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class ForInArrayLoop extends AbstractDiagnosticsTestWithStdLib {
public void testAllFilesPresentInForInArrayLoop() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
}
@TestMetadata("capturedRangeVariableAssignmentBefore13.kt")
public void testCapturedRangeVariableAssignmentBefore13() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop/capturedRangeVariableAssignmentBefore13.kt");
doTest(fileName);
}
@TestMetadata("rangeLocalDelegatedPropertyAssignmentBefore13.kt")
public void testRangeLocalDelegatedPropertyAssignmentBefore13() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop/rangeLocalDelegatedPropertyAssignmentBefore13.kt");
doTest(fileName);
}
@TestMetadata("rangeVariableAssignment13.kt")
public void testRangeVariableAssignment13() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop/rangeVariableAssignment13.kt");
doTest(fileName);
}
@TestMetadata("rangeVariableAssignmentBefore13.kt")
public void testRangeVariableAssignmentBefore13() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop/rangeVariableAssignmentBefore13.kt");
doTest(fileName);
}
}
@TestMetadata("compiler/testData/diagnostics/testsWithStdLib/functionLiterals")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
@@ -1746,6 +1746,39 @@ public class DiagnosticsTestWithStdLibUsingJavacGenerated extends AbstractDiagno
}
}
@TestMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class ForInArrayLoop extends AbstractDiagnosticsTestWithStdLibUsingJavac {
public void testAllFilesPresentInForInArrayLoop() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop"), Pattern.compile("^(.+)\\.kt$"), TargetBackend.ANY, true);
}
@TestMetadata("capturedRangeVariableAssignmentBefore13.kt")
public void testCapturedRangeVariableAssignmentBefore13() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop/capturedRangeVariableAssignmentBefore13.kt");
doTest(fileName);
}
@TestMetadata("rangeLocalDelegatedPropertyAssignmentBefore13.kt")
public void testRangeLocalDelegatedPropertyAssignmentBefore13() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop/rangeLocalDelegatedPropertyAssignmentBefore13.kt");
doTest(fileName);
}
@TestMetadata("rangeVariableAssignment13.kt")
public void testRangeVariableAssignment13() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop/rangeVariableAssignment13.kt");
doTest(fileName);
}
@TestMetadata("rangeVariableAssignmentBefore13.kt")
public void testRangeVariableAssignmentBefore13() throws Exception {
String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/testsWithStdLib/forInArrayLoop/rangeVariableAssignmentBefore13.kt");
doTest(fileName);
}
}
@TestMetadata("compiler/testData/diagnostics/testsWithStdLib/functionLiterals")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
@@ -75,6 +75,7 @@ enum class LanguageFeature(
RestrictionOfWrongAnnotationsWithUseSiteTargetsOnTypes(KOTLIN_1_3),
ProhibitInnerClassesOfGenericClassExtendingThrowable(KOTLIN_1_3),
ProperVisibilityForCompanionObjectInstanceField(KOTLIN_1_3),
ProperForInArrayLoopRangeVariableAssignmentSemantic(KOTLIN_1_3),
StrictJavaNullabilityAssertions(sinceVersion = null, defaultState = State.DISABLED),