From e20e097a639151773ba75756561666f19d8b4a13 Mon Sep 17 00:00:00 2001 From: nd Date: Tue, 9 May 2017 09:31:32 +0200 Subject: [PATCH] Stricter type checks in 'convert to range' intention (#1073) #KT-17079 #KT-17762 Fixed --- ...vertTwoComparisonsToRangeCheckIntention.kt | 70 ++++++++++++++++--- .../charInt.kt | 5 ++ .../compareToComparableSameType.kt | 10 +++ .../compareToComparableSameType.kt.after | 10 +++ .../compareToDifferentTypes.kt | 9 +++ .../compareToSameType.kt | 11 +++ .../doubleToInt1.kt | 4 ++ .../doubleToInt1.kt.after | 4 ++ .../doubleToInt2.kt | 4 ++ .../doubleToInt2.kt.after | 4 ++ .../intentions/IntentionTestGenerated.java | 36 ++++++++++ 11 files changed, 156 insertions(+), 11 deletions(-) create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/charInt.kt create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt.after create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToDifferentTypes.kt create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToSameType.kt create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt.after create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt create mode 100644 idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt.after diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertTwoComparisonsToRangeCheckIntention.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertTwoComparisonsToRangeCheckIntention.kt index 6ba89d0a772..128e697e2f7 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertTwoComparisonsToRangeCheckIntention.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/ConvertTwoComparisonsToRangeCheckIntention.kt @@ -24,9 +24,15 @@ import org.jetbrains.kotlin.idea.inspections.IntentionBasedInspection import org.jetbrains.kotlin.idea.intentions.branchedTransformations.evaluatesTo import org.jetbrains.kotlin.lexer.KtTokens import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.calls.callUtil.getType import org.jetbrains.kotlin.resolve.constants.evaluate.ConstantExpressionEvaluator import org.jetbrains.kotlin.types.KotlinType +import org.jetbrains.kotlin.types.typeUtil.builtIns +import org.jetbrains.kotlin.types.typeUtil.immediateSupertypes +import org.jetbrains.kotlin.types.typeUtil.isConstructedFromClassWithGivenFqName +import org.jetbrains.kotlin.types.typeUtil.isPrimitiveNumberType class ConvertTwoComparisonsToRangeCheckInspection : IntentionBasedInspection( ConvertTwoComparisonsToRangeCheckIntention::class @@ -102,8 +108,7 @@ class ConvertTwoComparisonsToRangeCheckIntention : SelfTargetingOffsetIndependen private fun generateRangeExpressionData( value: KtExpression, min: KtExpression, max: KtExpression, incrementMinByOne: Boolean, decrementMaxByOne: Boolean ): RangeExpressionData? { - fun KtExpression.getChangeBy(number: Int): String? { - val context = analyze() + fun KtExpression.getChangeBy(context: BindingContext, number: Int): String? { val type = getType(context) ?: return null if (!type.isValidTypeForIncrementDecrementByOne()) return null @@ -124,21 +129,64 @@ class ConvertTwoComparisonsToRangeCheckIntention : SelfTargetingOffsetIndependen // To avoid possible side effects if (!min.isSimple() || !max.isSimple()) return null - if (incrementMinByOne || decrementMaxByOne) { - if (!value.getType(value.analyze()).isValidTypeForIncrementDecrementByOne()) return null + val context = value.analyze() + val valType = value.getType(context) + val minType = min.getType(context) + val maxType = max.getType(context) + + if (valType == null || minType == null || maxType == null) return null + + if (!valType.isComparable()) return null + + var minVal = min + var maxVal = max + + if (minType != valType || maxType != valType) { + //numbers can be compared to numbers of different types + if (valType.isPrimitiveNumberType() && minType.isPrimitiveNumberType() && maxType.isPrimitiveNumberType()) { + //char is comparable to chars only + if (KotlinBuiltIns.isChar(valType) || KotlinBuiltIns.isChar(minType) || KotlinBuiltIns.isChar(maxType)) return null + + if (valType.isFloatingPoint()) { + if (minType.isInteger()) + minVal = KtPsiFactory(minVal).createExpression(getDoubleConstant(min, minType, context) ?: return null) + if (maxType.isInteger()) + maxVal = KtPsiFactory(maxVal).createExpression(getDoubleConstant(max, maxType, context) ?: return null) + } + } else { + return null + } } - val minText = if (incrementMinByOne) min.getChangeBy(1) else min.text - val maxText = if (decrementMaxByOne) max.getChangeBy(-1) else max.text + if (incrementMinByOne || decrementMaxByOne) { + if (!valType.isValidTypeForIncrementDecrementByOne()) return null + } + + val minText = if (incrementMinByOne) minVal.getChangeBy(context, 1) else minVal.text + val maxText = if (decrementMaxByOne) maxVal.getChangeBy(context, -1) else maxVal.text return RangeExpressionData(value, minText ?: return null, maxText ?: return null) } + private fun getDoubleConstant(intExpr: KtExpression, type: KotlinType, context: BindingContext): String? { + val intConst = ConstantExpressionEvaluator.getConstant(intExpr, context)?.getValue(type) ?: return null + return (intConst as? Number)?.toDouble()?.toString() + } + + private fun KotlinType.isComparable() = DescriptorUtils.isSubtypeOfClass(this, this.builtIns.getComparable()) + + private fun KotlinType.isFloatingPoint(): Boolean { + return KotlinBuiltIns.isFloat(this) || KotlinBuiltIns.isDouble(this) + } + + private fun KotlinType.isInteger(): Boolean { + return KotlinBuiltIns.isInt(this) || + KotlinBuiltIns.isLong(this) || + KotlinBuiltIns.isShort(this) || + KotlinBuiltIns.isByte(this) + } + private fun KotlinType?.isValidTypeForIncrementDecrementByOne(): Boolean { this ?: return false - return KotlinBuiltIns.isInt(this) || - KotlinBuiltIns.isLong(this) || - KotlinBuiltIns.isShort(this) || - KotlinBuiltIns.isByte(this) || - KotlinBuiltIns.isChar(this) + return this.isInteger()|| KotlinBuiltIns.isChar(this) } } \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/charInt.kt b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/charInt.kt new file mode 100644 index 00000000000..c369cd18ebd --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/charInt.kt @@ -0,0 +1,5 @@ +// IS_APPLICABLE: false +// DISABLE-ERRORS +fun foo(bar: Char) { + bar > 1 && 2 > bar +} \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt new file mode 100644 index 00000000000..3d79e016445 --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt @@ -0,0 +1,10 @@ +// WITH_RUNTIME +class A(val _value: Int): Comparable { + override operator fun compareTo(other: A) = _value.compareTo(other._value) +} + +val low = A(0) +val high = A(100) +fun test(a: A): Boolean { + return low <= a && a <= high +} \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt.after b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt.after new file mode 100644 index 00000000000..dc098fea158 --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt.after @@ -0,0 +1,10 @@ +// WITH_RUNTIME +class A(val _value: Int): Comparable { + override operator fun compareTo(other: A) = _value.compareTo(other._value) +} + +val low = A(0) +val high = A(100) +fun test(a: A): Boolean { + return a in low..high +} \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToDifferentTypes.kt b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToDifferentTypes.kt new file mode 100644 index 00000000000..3d890fe82a7 --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToDifferentTypes.kt @@ -0,0 +1,9 @@ +// IS_APPLICABLE: false +// WITH_RUNTIME +class A(val _value: Int) { + operator fun compareTo(other: Int) = _value.compareTo(other) +} + +fun test(a: A): Boolean { + return a >= 0 && a <= 100 +} \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToSameType.kt b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToSameType.kt new file mode 100644 index 00000000000..7eab3cbeb52 --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToSameType.kt @@ -0,0 +1,11 @@ +// IS_APPLICABLE: false +// WITH_RUNTIME +class A(val _value: Int) { + operator fun compareTo(other: A) = _value.compareTo(other._value) +} + +val low = A(0) +val high = A(100) +fun test(a: A): Boolean { + return low <= a && a <= high +} \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt new file mode 100644 index 00000000000..2f76dcffaa5 --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun foo(bar: Double) { + 0 <= bar && bar <= 10.0 +} \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt.after b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt.after new file mode 100644 index 00000000000..9f8b05c6b0e --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun foo(bar: Double) { + bar in 0.0..10.0 +} \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt new file mode 100644 index 00000000000..53467c541a3 --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun foo(bar: Double) { + 0 <= bar && bar <= 10 +} \ No newline at end of file diff --git a/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt.after b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt.after new file mode 100644 index 00000000000..9f8b05c6b0e --- /dev/null +++ b/idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt.after @@ -0,0 +1,4 @@ +// WITH_RUNTIME +fun foo(bar: Double) { + bar in 0.0..10.0 +} \ No newline at end of file diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 73dc2300eb6..74e2234b355 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -7098,12 +7098,48 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("charInt.kt") + public void testCharInt() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertTwoComparisonsToRangeCheck/charInt.kt"); + doTest(fileName); + } + + @TestMetadata("compareToComparableSameType.kt") + public void testCompareToComparableSameType() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToComparableSameType.kt"); + doTest(fileName); + } + + @TestMetadata("compareToDifferentTypes.kt") + public void testCompareToDifferentTypes() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToDifferentTypes.kt"); + doTest(fileName); + } + + @TestMetadata("compareToSameType.kt") + public void testCompareToSameType() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertTwoComparisonsToRangeCheck/compareToSameType.kt"); + doTest(fileName); + } + @TestMetadata("double.kt") public void testDouble() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertTwoComparisonsToRangeCheck/double.kt"); doTest(fileName); } + @TestMetadata("doubleToInt1.kt") + public void testDoubleToInt1() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt1.kt"); + doTest(fileName); + } + + @TestMetadata("doubleToInt2.kt") + public void testDoubleToInt2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertTwoComparisonsToRangeCheck/doubleToInt2.kt"); + doTest(fileName); + } + @TestMetadata("flipped.kt") public void testFlipped() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/convertTwoComparisonsToRangeCheck/flipped.kt");