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

Move detecting of illegal call of contract function into call checker

Browse Source 
(#KT-26153, #KT-26191) fixed
This commit is contained in:
Dmitriy Novozhilov
2019-03-01 12:39:00 +03:00
parent 54e5cce9be
commit 2f5843f764
12 changed files with 177 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingDiagnosticsCollector.kt
+2
View File
@@ -37,6 +37,8 @@ interface ContractParsingDiagnosticsCollector {
}
class TraceBasedCollector(private val bindingTrace: BindingTrace, mainCall: KtExpression) : ContractParsingDiagnosticsCollector {
constructor(callContext: ContractCallContext) : this(callContext.trace, callContext.contractCallExpression)
private val reportedErrors: MutableList<Diagnostic> = mutableListOf()
private val mainCallReportTarget = (mainCall as? KtCallExpression)?.calleeExpression ?: mainCall
compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt
+6 -30
View File
@@ -23,13 +23,12 @@ import org.jetbrains.kotlin.contracts.description.ContractDescription
import org.jetbrains.kotlin.contracts.description.ContractProviderKey
import org.jetbrains.kotlin.contracts.description.LazyContractProvider
import org.jetbrains.kotlin.descriptors.*
import org.jetbrains.kotlin.psi.KtExpression
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.isContractDescriptionCallPsiCheck
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.BindingTrace
import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
import org.jetbrains.kotlin.resolve.scopes.LexicalScope
import org.jetbrains.kotlin.resolve.scopes.LexicalScopeKind
import org.jetbrains.kotlin.storage.StorageManager
class ContractParsingServices(val languageVersionSettings: LanguageVersionSettings, private val storageManager: StorageManager) {
@@ -42,12 +41,12 @@ class ContractParsingServices(val languageVersionSettings: LanguageVersionSettin
*
* Otherwise, it may lead to inconsistent resolve state and failed assertions
*/
fun checkContractAndRecordIfPresent(expression: KtExpression, trace: BindingTrace, scope: LexicalScope, isFirstStatement: Boolean) {
fun checkContractAndRecordIfPresent(expression: KtExpression, trace: BindingTrace, scope: LexicalScope) {
// Fastpath. Note that it doesn't violates invariant described in KDoc, because 'isContractDescriptionCallPsiCheck'
// is a *necessary* (but not sufficient, actually) condition for presence of 'LazyContractProvider'
if (!expression.isContractDescriptionCallPsiCheck()) return
val callContext = ContractCallContext(expression, isFirstStatement, scope, trace)
val callContext = ContractCallContext(expression, scope, trace)
val contractProviderIfAny =
(scope.ownerDescriptor as? FunctionDescriptor)?.getUserData(ContractProviderKey) as? LazyContractProvider?
var resultingContractDescription: ContractDescription? = null
@@ -68,14 +67,14 @@ class ContractParsingServices(val languageVersionSettings: LanguageVersionSettin
* ideally, it should satisfy following condition: null returned <=> at least one error was reported
*/
private fun parseContractAndReportErrors(callContext: ContractCallContext): ContractDescription? {
val collector = TraceBasedCollector(callContext.trace, callContext.contractCallExpression)
val collector = TraceBasedCollector(callContext)
try {
checkFeatureEnabled(collector)
checkContractAllowedHere(collector, callContext)
val contractNotAllowed = callContext.bindingContext[BindingContext.CONTRACT_NOT_ALLOWED, callContext.contractCallExpression] == true
// Small optimization: do not even try to parse contract if we already have errors
if (collector.hasErrors()) return null
if (collector.hasErrors() || contractNotAllowed) return null
val parsedContract = PsiContractParserDispatcher(collector, callContext, storageManager).parseContract()
@@ -100,35 +99,12 @@ class ContractParsingServices(val languageVersionSettings: LanguageVersionSettin
}
}
private fun checkContractAllowedHere(collector: ContractParsingDiagnosticsCollector, callContext: ContractCallContext) {
val functionDescriptor = callContext.ownerDescriptor as? FunctionDescriptor
val scope = callContext.scope
if (!callContext.isFirstStatement)
collector.contractNotAllowed("Contract should be the first statement")
if (functionDescriptor == null || functionDescriptor is PropertyAccessorDescriptor)
collector.contractNotAllowed("Contracts are allowed only for functions")
if (callContext.ownerDescriptor.containingDeclaration !is PackageFragmentDescriptor
|| scope.kind != LexicalScopeKind.CODE_BLOCK
|| (scope.parent as? LexicalScope)?.kind != LexicalScopeKind.FUNCTION_INNER_SCOPE
)
collector.contractNotAllowed("Contracts are allowed only for top-level functions")
if (functionDescriptor?.isOperator == true) collector.contractNotAllowed("Contracts are not allowed for operator functions")
if (functionDescriptor?.isOverridable == true) collector.contractNotAllowed("Contracts are not allowed for open functions")
}
private fun KtExpression.isContractDescriptionCallPreciseCheck(context: BindingContext): Boolean =
getResolvedCall(context)?.resultingDescriptor?.isContractCallDescriptor() ?: false
}
class ContractCallContext(
val contractCallExpression: KtExpression,
val isFirstStatement: Boolean,
val scope: LexicalScope,
val trace: BindingTrace
) {
compiler/frontend/src/org/jetbrains/kotlin/resolve/BindingContext.java
+1
View File
@@ -98,6 +98,7 @@ public interface BindingContext {
WritableSlice<KtExpression, DataFlowInfo> DATA_FLOW_INFO_BEFORE = new BasicWritableSlice<>(DO_NOTHING);
WritableSlice<KtExpression, KotlinType> EXPECTED_EXPRESSION_TYPE = new BasicWritableSlice<>(DO_NOTHING);
WritableSlice<KtElement, Computation> EXPRESSION_EFFECTS = Slices.createSimpleSlice();
WritableSlice<KtElement, Boolean> CONTRACT_NOT_ALLOWED = Slices.createSimpleSlice();
WritableSlice<KtFunction, KotlinType> EXPECTED_RETURN_TYPE = new BasicWritableSlice<>(DO_NOTHING);
WritableSlice<KtExpression, DataFlowInfo> DATAFLOW_INFO_AFTER_CONDITION = Slices.createSimpleSlice();
WritableSlice<VariableDescriptor, DataFlowValue> BOUND_INITIALIZER_VALUE = Slices.createSimpleSlice();
compiler/frontend/src/org/jetbrains/kotlin/resolve/PlatformConfiguratorBase.kt
+1 -1
View File
@@ -46,7 +46,7 @@ private val DEFAULT_CALL_CHECKERS = listOf(
UnderscoreUsageChecker, AssigningNamedArgumentToVarargChecker(),
PrimitiveNumericComparisonCallChecker, LambdaWithSuspendModifierCallChecker,
UselessElvisCallChecker(), ResultTypeWithNullableOperatorsChecker(), NullableVarargArgumentCallChecker,
NamedFunAsExpressionChecker
NamedFunAsExpressionChecker, ContractNotAllowedCallChecker
)
private val DEFAULT_TYPE_CHECKERS = emptyList<AdditionalTypeChecker>()
private val DEFAULT_CLASSIFIER_USAGE_CHECKERS = listOf(
compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/checkers/ContractNotAllowedCallChecker.kt
+74
View File
@@ -0,0 +1,74 @@
/*
* 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.resolve.calls.checkers
import com.intellij.psi.PsiElement
import org.jetbrains.kotlin.contracts.parsing.isContractCallDescriptor
import org.jetbrains.kotlin.descriptors.FunctionDescriptor
import org.jetbrains.kotlin.descriptors.PackageFragmentDescriptor
import org.jetbrains.kotlin.descriptors.PropertyAccessorDescriptor
import org.jetbrains.kotlin.descriptors.isOverridable
import org.jetbrains.kotlin.diagnostics.Errors
import org.jetbrains.kotlin.psi.KtBlockExpression
import org.jetbrains.kotlin.psi.KtDotQualifiedExpression
import org.jetbrains.kotlin.psi.KtElement
import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
import org.jetbrains.kotlin.resolve.scopes.LexicalScope
import org.jetbrains.kotlin.resolve.scopes.LexicalScopeKind
object ContractNotAllowedCallChecker : CallChecker {
override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) {
if (reportOn !is KtElement) return
val descriptor = resolvedCall.resultingDescriptor as? FunctionDescriptor ?: return
if (!descriptor.isContractCallDescriptor()) return
val callElement = resolvedCall.call.callElement
var hasErrors = false
fun contractNotAllowed(message: String) {
hasErrors = true
context.trace.report(Errors.CONTRACT_NOT_ALLOWED.on(reportOn, message))
}
val scope = context.scope
val functionDescriptor = scope.ownerDescriptor as? FunctionDescriptor
if (functionDescriptor == null || functionDescriptor is PropertyAccessorDescriptor)
contractNotAllowed("Contracts are allowed only for functions")
var inFunctionBodyBlock = true
if (scope.ownerDescriptor.containingDeclaration !is PackageFragmentDescriptor
|| scope.kind != LexicalScopeKind.CODE_BLOCK
|| (scope.parent as? LexicalScope)?.kind != LexicalScopeKind.FUNCTION_INNER_SCOPE
) {
if (scope.kind == LexicalScopeKind.FUNCTION_INNER_SCOPE) {
contractNotAllowed("Contracts are allowed only in function body block")
inFunctionBodyBlock = false
} else {
contractNotAllowed("Contracts are allowed only for top-level functions")
}
}
if (functionDescriptor?.isOperator == true) contractNotAllowed("Contracts are not allowed for operator functions")
if (functionDescriptor?.isOverridable == true) contractNotAllowed("Contracts are not allowed for open functions")
var parent = callElement.parent
var element = callElement
if (parent is KtDotQualifiedExpression) {
element = parent
parent = parent.parent
}
if ((parent !is KtBlockExpression || parent.children.first { it is KtElement } != element) && inFunctionBodyBlock) {
contractNotAllowed("Contract should be the first statement")
}
if (hasErrors) {
context.trace.record(BindingContext.CONTRACT_NOT_ALLOWED, callElement, true)
}
}
}
compiler/frontend/src/org/jetbrains/kotlin/types/expressions/ExpressionTypingServices.java
+1 -2
View File
@@ -295,9 +295,8 @@ public class ExpressionTypingServices {
}
blockLevelVisitor = new ExpressionTypingVisitorDispatcher.ForBlock(expressionTypingComponents, annotationChecker, scope);
expressionTypingComponents.contractParsingServices.checkContractAndRecordIfPresent(statementExpression, context.trace, scope, isFirstStatement);
if (isFirstStatement) {
expressionTypingComponents.contractParsingServices.checkContractAndRecordIfPresent(statementExpression, context.trace, scope);
isFirstStatement = false;
}
}
compiler/testData/diagnostics/testsWithStdLib/contracts/dsl/errors/notFirstStatement.kt
Vendored
+66 -2
View File
@@ -1,12 +1,76 @@
// !LANGUAGE: +AllowContractsForCustomFunctions +UseReturnsEffect
// !USE_EXPERIMENTAL: kotlin.contracts.ExperimentalContracts
// !DIAGNOSTICS: -INVISIBLE_REFERENCE -INVISIBLE_MEMBER
// !DIAGNOSTICS: -INVISIBLE_REFERENCE -INVISIBLE_MEMBER -UNUSED_VARIABLE -REDUNDANT_LABEL_WARNING -UNUSED_PARAMETER -NOTHING_TO_INLINE -CAST_NEVER_SUCCEEDS
// Issues: KT-26153, KT-26191
import kotlin.contracts.*
fun foo(y: Boolean) {
val <!UNUSED_VARIABLE!>x<!>: Int = 42
val x: Int = 42
<!CONTRACT_NOT_ALLOWED("Contract should be the first statement")!>contract<!> {
returns() implies y
}
}
inline fun case1(block: () -> Unit) {
val contracts = listOf(
<!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}, <!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}
)
block()
}
inline fun case_2(block: () -> Unit) = <!CONTRACT_NOT_ALLOWED("Contracts are allowed only in function body block")!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}
fun case_3(block: () -> Unit) {
class Class {
fun innerFun(block2: () -> Unit) {
<!CONTRACT_NOT_ALLOWED("Contracts are allowed only for top-level functions")!>contract<!> {
callsInPlace(block2, InvocationKind.EXACTLY_ONCE)
}
block2()
}
}
return
}
inline fun case_4(number: Int?): Boolean {
val cond = number != null
<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(false) implies (cond)
} as ContractBuilder
return number == null
}
inline fun case_5(cond: Boolean): Boolean {
run {
<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (cond)
}
}
return true
}
inline fun case_6(cond: Boolean): Boolean {
run {
val x = 10
<!CONTRACT_NOT_ALLOWED, CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (cond)
}
}
return true
}
fun case_7(cond: Boolean): Boolean {
fun innerFun() {
<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (cond)
}
}
return true
}
compiler/testData/diagnostics/testsWithStdLib/contracts/dsl/errors/notFirstStatement.txt
Vendored
+7
View File
@@ -1,3 +1,10 @@
package
public inline fun case1(/*0*/ block: () -> kotlin.Unit): kotlin.Unit
public inline fun case_2(/*0*/ block: () -> kotlin.Unit): kotlin.Unit
public fun case_3(/*0*/ block: () -> kotlin.Unit): kotlin.Unit
public inline fun case_4(/*0*/ number: kotlin.Int?): kotlin.Boolean
public inline fun case_5(/*0*/ cond: kotlin.Boolean): kotlin.Boolean
public inline fun case_6(/*0*/ cond: kotlin.Boolean): kotlin.Boolean
public fun case_7(/*0*/ cond: kotlin.Boolean): kotlin.Boolean
public fun foo(/*0*/ y: kotlin.Boolean): kotlin.Unit
compiler/tests-spec/testData/diagnostics/notLinked/contracts/declarations/contractBuilder/common/neg/1.kt
Vendored
+6 -12
View File
@@ -38,74 +38,68 @@ inline fun case_3(block: () -> Unit) {
/*
* TESTCASE NUMBER: 4
* UNEXPECTED BEHAVIOUR
* ISSUES: KT-26191
*/
inline fun case_4(block: () -> Unit) {
.0009
return contract {
return <!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}
}
/*
* TESTCASE NUMBER: 5
* UNEXPECTED BEHAVIOUR
* ISSUES: KT-26191
*/
fun case_5(value_1: Int?) {
println("!")
contract {
<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (value_1 != null)
} <!CAST_NEVER_SUCCEEDS!>as<!> ContractBuilder
}
/*
* TESTCASE NUMBER: 6
* UNEXPECTED BEHAVIOUR
* ISSUES: KT-26191
*/
fun case_6(value_1: Int?) {
100 + 10
throw Exception(contract {
throw Exception(<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (value_1 != null)
}.toString())
}
/*
* TESTCASE NUMBER: 7
* UNEXPECTED BEHAVIOUR
* ISSUES: KT-26191
*/
fun case_7(value_1: Int?) {
for (i in 0..10) {
println(i)
}
return contract {
return <!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (value_1 != null)
}
}
/*
* TESTCASE NUMBER: 8
* UNEXPECTED BEHAVIOUR
* ISSUES: KT-26191
*/
fun case_8(value_1: Int?) {
val f = 10 - 20
val g = contract {
val g = <!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (value_1 != null)
}
}
/*
* TESTCASE NUMBER: 9
* UNEXPECTED BEHAVIOUR
* ISSUES: KT-26191
*/
fun case_9(number: Int?): Boolean {
val value_1 = number != null
contract {
<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(false) implies (value_1)
} <!CAST_NEVER_SUCCEEDS!>as<!> ContractBuilder
return number == null
compiler/tests-spec/testData/diagnostics/notLinked/contracts/declarations/contractBuilder/common/neg/14.kt
Vendored
+7 -8
View File
@@ -7,7 +7,6 @@
* SECTIONS: contracts, declarations, contractBuilder, common
* NUMBER: 14
* DESCRIPTION: Contract is first statement in control flow terms, but not in tokens order terms.
* UNEXPECTED BEHAVIOUR
* ISSUES: KT-26153
* HELPERS: functions
*/
@@ -16,17 +15,17 @@ import kotlin.contracts.*
// TESTCASE NUMBER: 1
inline fun case_1(block: () -> Unit) {
return contract {
return <!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}
}
// TESTCASE NUMBER: 2
fun case_2() = contract { }
fun case_2() = <!CONTRACT_NOT_ALLOWED!>contract<!> { }
// TESTCASE NUMBER: 3
inline fun case_3(block: () -> Unit) {
val value_1 = contract {
val value_1 = <!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}
block()
@@ -34,7 +33,7 @@ inline fun case_3(block: () -> Unit) {
// TESTCASE NUMBER: 4
inline fun case_4(block: () -> Unit) {
(contract {
(<!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
})
return block()
@@ -42,7 +41,7 @@ inline fun case_4(block: () -> Unit) {
// TESTCASE NUMBER: 5
inline fun case_5(block: () -> Unit) {
<!REDUNDANT_LABEL_WARNING!>test@<!> contract {
<!REDUNDANT_LABEL_WARNING!>test@<!> <!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}
return block()
@@ -50,14 +49,14 @@ inline fun case_5(block: () -> Unit) {
// TESTCASE NUMBER: 6
inline fun case_6(block: () -> Unit) {
throw Exception(contract {
throw Exception(<!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}.toString())
}
// TESTCASE NUMBER: 7
inline fun case_7(block: () -> Unit) {
funWithAnyArg(contract {
funWithAnyArg(<!CONTRACT_NOT_ALLOWED!>contract<!> {
callsInPlace(block, InvocationKind.EXACTLY_ONCE)
})
}
compiler/tests-spec/testData/diagnostics/notLinked/contracts/declarations/contractBuilder/common/neg/16.kt
Vendored
+5 -6
View File
@@ -7,7 +7,6 @@
* SECTIONS: contracts, declarations, contractBuilder, common
* NUMBER: 16
* DESCRIPTION: Contract isn't in the first position.
* UNEXPECTED BEHAVIOUR
* ISSUES: KT-26191
*/
@@ -16,7 +15,7 @@ import kotlin.contracts.*
// TESTCASE NUMBER: 1
fun case_1(value_1: Int?) {
println("!")
contract {
<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (value_1 != null)
} <!CAST_NEVER_SUCCEEDS!>as<!> ContractBuilder
}
@@ -24,7 +23,7 @@ fun case_1(value_1: Int?) {
// TESTCASE NUMBER: 2
fun case_2(value_1: Int?) {
100 + 10
throw Exception(contract {
throw Exception(<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (value_1 != null)
}.toString())
}
@@ -34,7 +33,7 @@ fun case_3(value_1: Int?) {
for (i in 0..10) {
println(i)
}
return contract {
return <!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (value_1 != null)
}
}
@@ -42,7 +41,7 @@ fun case_3(value_1: Int?) {
// TESTCASE NUMBER: 4
fun case_4(value_1: Int?) {
val f = 10 - 20
val g = contract {
val g = <!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(true) implies (value_1 != null)
}
}
@@ -50,7 +49,7 @@ fun case_4(value_1: Int?) {
// TESTCASE NUMBER: 5
fun case_5(number: Int?): Boolean {
val value_1 = number != null
contract {
<!CONTRACT_NOT_ALLOWED!>contract<!> {
returns(false) implies (value_1)
} <!CAST_NEVER_SUCCEEDS!>as<!> ContractBuilder
return number == null
compiler/tests-spec/testData/diagnostics/notLinked/contracts/declarations/contractBuilder/common/neg/5.kt
Vendored
+1 -1
View File
@@ -48,6 +48,6 @@ fun case_5(): Boolean? {
// TESTCASE NUMBER: 6
fun case_6(value_1: Boolean): Boolean? {
contract { returns(null) implies <!ERROR_IN_CONTRACT_DESCRIPTION, TYPE_MISMATCH!>contract { returns(null) implies (!value_1) }<!> }
contract { returns(null) implies <!ERROR_IN_CONTRACT_DESCRIPTION, TYPE_MISMATCH!><!CONTRACT_NOT_ALLOWED, CONTRACT_NOT_ALLOWED!>contract<!> { returns(null) implies (!value_1) }<!> }
return null
}