Add inspection for check/require/checkNotNull/requireNotNull

#KT-30640 Fixed
#KT-22412 Fixed
This commit is contained in:
Toshiaki Kameyama
2019-04-10 10:32:29 +03:00
committed by Mikhail Glukhikh
parent 64780293d3
commit 90b0ea73dc
30 changed files with 458 additions and 40 deletions

View File

@@ -3428,6 +3428,15 @@
language="kotlin"
/>
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.ReplaceGuardClauseWithFunctionCallInspection"
displayName="Replace guard clause with kotlin's function call"
groupPath="Kotlin"
groupName="Style issues"
enabledByDefault="true"
level="WEAK WARNING"
language="kotlin"
/>
<referenceImporter implementation="org.jetbrains.kotlin.idea.quickfix.KotlinReferenceImporter"/>
<fileType.fileViewProviderFactory filetype="KJSM" implementationClass="com.intellij.psi.ClassFileViewProviderFactory"/>

View File

@@ -0,0 +1,11 @@
<html>
<body>
This inspection reports guard clause that can be replaced with kotlin's function call. For example:
<br /><br />
<pre>
fun test(foo: Int?) {
if (foo == null) throw IllegalArgumentException("foo") // Replace guard clause with kotlin's function call
}
</pre>
</body>
</html>

View File

@@ -0,0 +1,129 @@
/*
* Copyright 2010-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
* that can be found in the license/LICENSE.txt file.
*/
package org.jetbrains.kotlin.idea.inspections
import com.intellij.openapi.editor.Editor
import com.intellij.openapi.project.Project
import org.jetbrains.kotlin.idea.caches.resolve.resolveToCall
import org.jetbrains.kotlin.idea.core.ShortenReferences
import org.jetbrains.kotlin.idea.core.replaced
import org.jetbrains.kotlin.idea.intentions.callExpression
import org.jetbrains.kotlin.idea.util.CommentSaver
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
class ReplaceGuardClauseWithFunctionCallInspection : AbstractApplicabilityBasedInspection<KtIfExpression>(
KtIfExpression::class.java
) {
companion object {
private const val ILLEGAL_STATE_EXCEPTION = "IllegalStateException"
private const val ILLEGAL_ARGUMENT_EXCEPTION = "IllegalArgumentException"
}
private enum class KotlinFunction(val functionName: String) {
CHECK("check"), CHECK_NOT_NULL("checkNotNull"), REQUIRE("require"), REQUIRE_NOT_NULL("requireNotNull");
val fqName: String
get() = "kotlin.$functionName"
}
override fun inspectionText(element: KtIfExpression) = "Replace guard clause with kotlin's function call"
override val defaultFixText = "Replace with kotlin's function call"
override fun fixText(element: KtIfExpression) =
element.getKotlinFunction()?.let { "Replace with '${it.functionName}()' call" } ?: defaultFixText
override fun isApplicable(element: KtIfExpression): Boolean {
if (element.condition == null) return false
val call = element.getCallExpression() ?: return false
val calleeText = call.calleeExpression?.text ?: return false
val valueArguments = call.valueArguments
if (valueArguments.size > 1) return false
if (calleeText != ILLEGAL_STATE_EXCEPTION && calleeText != ILLEGAL_ARGUMENT_EXCEPTION) return false
val fqName = call.resolveToCall()?.resultingDescriptor?.fqNameSafe?.parent()
return fqName == FqName("kotlin.$calleeText") || fqName == FqName("java.lang.$calleeText")
}
override fun applyTo(element: KtIfExpression, project: Project, editor: Editor?) {
val condition = element.condition ?: return
val call = element.getCallExpression() ?: return
val argument = call.valueArguments.firstOrNull()?.getArgumentExpression()
val commentSaver = CommentSaver(element)
val psiFactory = KtPsiFactory(element)
val replaced = when (val kotlinFunction = element.getKotlinFunction(call)) {
KotlinFunction.CHECK, KotlinFunction.REQUIRE -> {
val (excl, newCondition) = if (condition is KtPrefixExpression && condition.operationToken == KtTokens.EXCL) {
"" to (condition.baseExpression ?: return)
} else {
"!" to condition
}
val newExpression = if (argument == null) {
psiFactory.createExpressionByPattern("${kotlinFunction.fqName}($excl$0)", newCondition)
} else {
psiFactory.createExpressionByPattern("${kotlinFunction.fqName}($excl$0) { $1 }", newCondition, argument)
}
val replaced = element.replaced(newExpression)
val newCall = (replaced as? KtDotQualifiedExpression)?.callExpression
val negatedExpression = newCall?.valueArguments?.firstOrNull()?.getArgumentExpression() as? KtPrefixExpression
if (negatedExpression != null) {
SimplifyNegatedBinaryExpressionInspection.simplifyNegatedBinaryExpressionIfNeeded(negatedExpression)
}
replaced
}
KotlinFunction.CHECK_NOT_NULL, KotlinFunction.REQUIRE_NOT_NULL -> {
val nullCheckedExpression = condition.notNullCheckExpression() ?: return
val newExpression = if (argument == null) {
psiFactory.createExpressionByPattern("${kotlinFunction.fqName}($0)", nullCheckedExpression)
} else {
psiFactory.createExpressionByPattern("${kotlinFunction.fqName}($0) { $1 }", nullCheckedExpression, argument)
}
element.replaced(newExpression)
}
else -> return
}
commentSaver.restore(replaced)
ShortenReferences.DEFAULT.process(replaced)
}
private fun KtIfExpression.getCallExpression(): KtCallExpression? {
val throwExpression = this.then?.let {
it as? KtThrowExpression ?: (it as? KtBlockExpression)?.statements?.singleOrNull() as? KtThrowExpression
} ?: return null
return throwExpression.thrownExpression?.let {
it as? KtCallExpression ?: (it as? KtQualifiedExpression)?.callExpression
}
}
private fun KtIfExpression.getKotlinFunction(call: KtCallExpression? = getCallExpression()): KotlinFunction? {
val calleeText = call?.calleeExpression?.text ?: return null
val isNotNullCheck = condition.notNullCheckExpression() != null
return when (calleeText) {
ILLEGAL_STATE_EXCEPTION -> if (isNotNullCheck) KotlinFunction.CHECK_NOT_NULL else KotlinFunction.CHECK
ILLEGAL_ARGUMENT_EXCEPTION -> if (isNotNullCheck) KotlinFunction.REQUIRE_NOT_NULL else KotlinFunction.REQUIRE
else -> null
}
}
private fun KtExpression?.notNullCheckExpression(): KtExpression? {
if (this == null) return null
if (this !is KtBinaryExpression) return null
if (this.operationToken != KtTokens.EQEQ) return null
val left = this.left ?: return null
val right = this.right ?: return null
return when {
right.isNullConstant() -> left
left.isNullConstant() -> right
else -> null
}
}
private fun KtExpression.isNullConstant(): Boolean {
return (this as? KtConstantExpression)?.text == KtTokens.NULL_KEYWORD.value
}
}

View File

@@ -26,25 +26,6 @@ import org.jetbrains.kotlin.psi.*
class SimplifyNegatedBinaryExpressionInspection : AbstractApplicabilityBasedInspection<KtPrefixExpression>(KtPrefixExpression::class.java) {
private fun IElementType.negate(): KtSingleValueToken? = when (this) {
KtTokens.IN_KEYWORD -> KtTokens.NOT_IN
KtTokens.NOT_IN -> KtTokens.IN_KEYWORD
KtTokens.IS_KEYWORD -> KtTokens.NOT_IS
KtTokens.NOT_IS -> KtTokens.IS_KEYWORD
KtTokens.EQEQ -> KtTokens.EXCLEQ
KtTokens.EXCLEQ -> KtTokens.EQEQ
KtTokens.LT -> KtTokens.GTEQ
KtTokens.GTEQ -> KtTokens.LT
KtTokens.GT -> KtTokens.LTEQ
KtTokens.LTEQ -> KtTokens.GT
else -> null
}
override fun inspectionHighlightRangeInElement(element: KtPrefixExpression) = element.operationReference.textRangeIn(element)
override fun inspectionText(element: KtPrefixExpression) = "Negated operation should be simplified"
@@ -59,31 +40,65 @@ class SimplifyNegatedBinaryExpressionInspection : AbstractApplicabilityBasedInsp
}
override fun isApplicable(element: KtPrefixExpression): Boolean {
if (element.operationToken != KtTokens.EXCL) return false
val expression = KtPsiUtil.deparenthesize(element.baseExpression) as? KtOperationExpression ?: return false
when (expression) {
is KtIsExpression -> if (expression.typeReference == null) return false
is KtBinaryExpression -> if (expression.left == null || expression.right == null) return false
else -> return false
}
return (expression.operationReference.getReferencedNameElementType() as? KtSingleValueToken)?.negate() != null
return element.canBeSimplified()
}
override fun applyTo(element: KtPrefixExpression, project: Project, editor: Editor?) {
val expression = KtPsiUtil.deparenthesize(element.baseExpression) ?: return
val operation = (expression as KtOperationExpression).operationReference.getReferencedNameElementType().negate()?.value ?: return
element.simplify()
}
val psiFactory = KtPsiFactory(expression)
val newExpression = when (expression) {
is KtIsExpression ->
psiFactory.createExpressionByPattern("$0 $1 $2", expression.leftHandSide, operation, expression.typeReference!!)
is KtBinaryExpression ->
psiFactory.createExpressionByPattern("$0 $1 $2", expression.left!!, operation, expression.right!!)
else ->
throw IllegalArgumentException()
companion object {
fun simplifyNegatedBinaryExpressionIfNeeded(expression: KtPrefixExpression) {
if (expression.canBeSimplified()) expression.simplify()
}
private fun KtPrefixExpression.canBeSimplified(): Boolean {
if (operationToken != KtTokens.EXCL) return false
val expression = KtPsiUtil.deparenthesize(baseExpression) as? KtOperationExpression ?: return false
when (expression) {
is KtIsExpression -> if (expression.typeReference == null) return false
is KtBinaryExpression -> if (expression.left == null || expression.right == null) return false
else -> return false
}
return (expression.operationReference.getReferencedNameElementType() as? KtSingleValueToken)?.negate() != null
}
private fun KtPrefixExpression.simplify() {
val expression = KtPsiUtil.deparenthesize(baseExpression) ?: return
val operation =
(expression as KtOperationExpression).operationReference.getReferencedNameElementType().negate()?.value ?: return
val psiFactory = KtPsiFactory(expression)
val newExpression = when (expression) {
is KtIsExpression ->
psiFactory.createExpressionByPattern("$0 $1 $2", expression.leftHandSide, operation, expression.typeReference!!)
is KtBinaryExpression ->
psiFactory.createExpressionByPattern("$0 $1 $2", expression.left ?: return, operation, expression.right ?: return)
else ->
throw IllegalArgumentException()
}
replace(newExpression)
}
private fun IElementType.negate(): KtSingleValueToken? = when (this) {
KtTokens.IN_KEYWORD -> KtTokens.NOT_IN
KtTokens.NOT_IN -> KtTokens.IN_KEYWORD
KtTokens.IS_KEYWORD -> KtTokens.NOT_IS
KtTokens.NOT_IS -> KtTokens.IS_KEYWORD
KtTokens.EQEQ -> KtTokens.EXCLEQ
KtTokens.EXCLEQ -> KtTokens.EQEQ
KtTokens.LT -> KtTokens.GTEQ
KtTokens.GTEQ -> KtTokens.LT
KtTokens.GT -> KtTokens.LTEQ
KtTokens.LTEQ -> KtTokens.GT
else -> null
}
element.replace(newExpression)
}
}

View File

@@ -107,6 +107,7 @@ object J2KPostProcessingRegistrarImpl : J2KPostProcessingRegistrar {
registerInspectionBasedProcessing(SimplifyAssertNotNullInspection())
registerIntentionBasedProcessing(RemoveRedundantCallsOfConversionMethodsIntention())
registerInspectionBasedProcessing(JavaMapForEachInspection())
registerInspectionBasedProcessing(ReplaceGuardClauseWithFunctionCallInspection())
registerDiagnosticBasedProcessing<KtBinaryExpressionWithTypeRHS>(Errors.USELESS_CAST) { element, _ ->

View File

@@ -0,0 +1 @@
org.jetbrains.kotlin.idea.inspections.ReplaceGuardClauseWithFunctionCallInspection

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'check()' call
// WITH_RUNTIME
fun test(b: Boolean) {
<caret>if (b) throw IllegalStateException()
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'check()' call
// WITH_RUNTIME
fun test(b: Boolean) {
check(!b)
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'checkNotNull()' call
// WITH_RUNTIME
fun test(foo: Int?) {
<caret>if (foo == null) throw IllegalStateException()
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'checkNotNull()' call
// WITH_RUNTIME
fun test(foo: Int?) {
checkNotNull(foo)
}

View File

@@ -0,0 +1,5 @@
// PROBLEM: none
// WITH_RUNTIME
fun test(b: Boolean) {
<caret>if (b) throw IndexOutOfBoundsException()
}

View File

@@ -0,0 +1,7 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(b: Boolean) {
<caret>if (b) {
throw IllegalArgumentException("test")
}
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(b: Boolean) {
require(!b) { "test" }
}

View File

@@ -0,0 +1,7 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(b: Boolean) {
<caret>if (b) {
throw java.lang.IllegalArgumentException("test")
}
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(b: Boolean) {
require(!b) { "test" }
}

View File

@@ -0,0 +1,9 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(b: Boolean) {
<caret>if (b) {
// comment1
throw IllegalArgumentException()
// comment2
}
}

View File

@@ -0,0 +1,7 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(b: Boolean) {
// comment1
// comment2
require(!b)
}

View File

@@ -0,0 +1,9 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(b: Boolean) {
<caret>if (b) {
// comment1
throw IllegalArgumentException("test")
// comment2
}
}

View File

@@ -0,0 +1,9 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(b: Boolean) {
require(!b) {
// comment1
"test"
// comment2
}
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(foo: Boolean) {
<caret>if (!foo) throw IllegalArgumentException("test")
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(foo: Boolean) {
require(foo) { "test" }
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(foo: Int) {
<caret>if (foo != 0) throw IllegalArgumentException("test")
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'require()' call
// WITH_RUNTIME
fun test(foo: Int) {
require(foo == 0) { "test" }
}

View File

@@ -0,0 +1,7 @@
// FIX: Replace with 'requireNotNull()' call
// WITH_RUNTIME
fun test(foo: Int?) {
<caret>if (foo == null) {
throw IllegalArgumentException("test")
}
}

View File

@@ -0,0 +1,5 @@
// FIX: Replace with 'requireNotNull()' call
// WITH_RUNTIME
fun test(foo: Int?) {
requireNotNull(foo) { "test" }
}

View File

@@ -8902,6 +8902,121 @@ public class LocalInspectionTestGenerated extends AbstractLocalInspectionTest {
}
}
@TestMetadata("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class ReplaceGuardClauseWithFunctionCall extends AbstractLocalInspectionTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
}
public void testAllFilesPresentInReplaceGuardClauseWithFunctionCall() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
}
@TestMetadata("notTargetException.kt")
public void testNotTargetException() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/notTargetException.kt");
}
@TestMetadata("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/check")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class Check extends AbstractLocalInspectionTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
}
public void testAllFilesPresentInCheck() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/check"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
}
@TestMetadata("basic.kt")
public void testBasic() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/check/basic.kt");
}
}
@TestMetadata("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/checkNotNull")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class CheckNotNull extends AbstractLocalInspectionTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
}
public void testAllFilesPresentInCheckNotNull() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/checkNotNull"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
}
@TestMetadata("basic.kt")
public void testBasic() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/checkNotNull/basic.kt");
}
}
@TestMetadata("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class Require extends AbstractLocalInspectionTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
}
public void testAllFilesPresentInRequire() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
}
@TestMetadata("basic.kt")
public void testBasic() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/basic.kt");
}
@TestMetadata("basic2.kt")
public void testBasic2() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/basic2.kt");
}
@TestMetadata("comment.kt")
public void testComment() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/comment.kt");
}
@TestMetadata("comment2.kt")
public void testComment2() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/comment2.kt");
}
@TestMetadata("not.kt")
public void testNot() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/not.kt");
}
@TestMetadata("notEq.kt")
public void testNotEq() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/require/notEq.kt");
}
}
@TestMetadata("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class RequireNotNull extends AbstractLocalInspectionTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, TargetBackend.ANY, testDataFilePath);
}
public void testAllFilesPresentInRequireNotNull() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull"), Pattern.compile("^([\\w\\-_]+)\\.(kt|kts)$"), TargetBackend.ANY, true);
}
@TestMetadata("basic.kt")
public void testBasic() throws Exception {
runTest("idea/testData/inspectionsLocal/replaceGuardClauseWithFunctionCall/requireNotNull/basic.kt");
}
}
}
@TestMetadata("idea/testData/inspectionsLocal/replaceJavaStaticMethodWithKotlinAnalog")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)

View File

@@ -0,0 +1,7 @@
public class Test {
void test(String s) {
if (s == null) {
throw new IllegalArgumentException("s should not be null");
}
}
}

View File

@@ -0,0 +1,5 @@
class Test {
internal fun test(s: String?) {
requireNotNull(s) { "s should not be null" }
}
}

View File

@@ -3779,6 +3779,11 @@ public class JavaToKotlinConverterForWebDemoTestGenerated extends AbstractJavaTo
runTest("j2k/testData/fileOrElement/postProcessing/GetOperator.java");
}
@TestMetadata("GuardClause.java")
public void testGuardClause() throws Exception {
runTest("j2k/testData/fileOrElement/postProcessing/GuardClause.java");
}
@TestMetadata("IfNullReturnToElvis.java")
public void testIfNullReturnToElvis() throws Exception {
runTest("j2k/testData/fileOrElement/postProcessing/IfNullReturnToElvis.java");

View File

@@ -3779,6 +3779,11 @@ public class JavaToKotlinConverterSingleFileTestGenerated extends AbstractJavaTo
runTest("j2k/testData/fileOrElement/postProcessing/GetOperator.java");
}
@TestMetadata("GuardClause.java")
public void testGuardClause() throws Exception {
runTest("j2k/testData/fileOrElement/postProcessing/GuardClause.java");
}
@TestMetadata("IfNullReturnToElvis.java")
public void testIfNullReturnToElvis() throws Exception {
runTest("j2k/testData/fileOrElement/postProcessing/IfNullReturnToElvis.java");