From 339c55505a07ac1d6901d1c2229e0f6f85b75b7c Mon Sep 17 00:00:00 2001 From: Dmitry Savvinov Date: Tue, 23 Oct 2018 18:10:33 +0300 Subject: [PATCH] Refactor logic of reporting errors in contracts Rename 'doCheckContract' into 'parseContractAndReportErrors', emphasizing that it works with definitely call to 'contract' from stdlib. In particular, it means that it should either return non-null value or report some errors. Note that this commit doesn't change behavior of this code (modulo cases when something in 'parseContract' throws exception), but just refactors the code to be more clear and easy to reason about. --- .../ContractParsingDiagnosticsCollector.kt | 30 ++++++------- .../parsing/ContractParsingServices.kt | 42 +++++++++++++------ 2 files changed, 45 insertions(+), 27 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingDiagnosticsCollector.kt b/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingDiagnosticsCollector.kt index 958b1d3179d..7b3fa089034 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingDiagnosticsCollector.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingDiagnosticsCollector.kt @@ -7,7 +7,6 @@ package org.jetbrains.kotlin.contracts.parsing import org.jetbrains.kotlin.config.LanguageFeature import org.jetbrains.kotlin.config.LanguageVersionSettings -import org.jetbrains.kotlin.config.LanguageVersionSettingsImpl import org.jetbrains.kotlin.diagnostics.Diagnostic import org.jetbrains.kotlin.diagnostics.Errors import org.jetbrains.kotlin.psi.KtCallExpression @@ -19,49 +18,52 @@ interface ContractParsingDiagnosticsCollector { fun unsupportedFeature(languageVersionSettings: LanguageVersionSettings) fun contractNotAllowed(message: String) fun badDescription(message: String, reportOn: KtElement) + fun addFallbackErrorIfNecessary() - fun flushDiagnostics(parsingFailed: Boolean) + fun flushDiagnostics() fun hasErrors(): Boolean object EMPTY : ContractParsingDiagnosticsCollector { override fun contractNotAllowed(message: String) {} override fun badDescription(message: String, reportOn: KtElement) {} - override fun unsupportedFeature(languageVersionSettings: LanguageVersionSettings) { } + override fun unsupportedFeature(languageVersionSettings: LanguageVersionSettings) {} + override fun addFallbackErrorIfNecessary() { } - override fun flushDiagnostics(parsingFailed: Boolean) {} + override fun flushDiagnostics() {} override fun hasErrors(): Boolean = false } } class TraceBasedCollector(private val bindingTrace: BindingTrace, mainCall: KtExpression) : ContractParsingDiagnosticsCollector { - private val diagnostics: MutableList = mutableListOf() + private val reportedErrors: MutableList = mutableListOf() private val mainCallReportTarget = (mainCall as? KtCallExpression)?.calleeExpression ?: mainCall override fun contractNotAllowed(message: String) { - diagnostics += Errors.CONTRACT_NOT_ALLOWED.on(mainCallReportTarget, message) + reportedErrors += Errors.CONTRACT_NOT_ALLOWED.on(mainCallReportTarget, message) } override fun badDescription(message: String, reportOn: KtElement) { - diagnostics += Errors.ERROR_IN_CONTRACT_DESCRIPTION.on(reportOn, message) + reportedErrors += Errors.ERROR_IN_CONTRACT_DESCRIPTION.on(reportOn, message) } override fun unsupportedFeature(languageVersionSettings: LanguageVersionSettings) { - diagnostics += Errors.UNSUPPORTED_FEATURE.on( + reportedErrors += Errors.UNSUPPORTED_FEATURE.on( mainCallReportTarget, LanguageFeature.AllowContractsForCustomFunctions to languageVersionSettings ) } - override fun flushDiagnostics(parsingFailed: Boolean) { - if (parsingFailed && diagnostics.isEmpty()) { - diagnostics += Errors.ERROR_IN_CONTRACT_DESCRIPTION.on(mainCallReportTarget, "Error in contract description") - } + override fun addFallbackErrorIfNecessary() { + if (reportedErrors.isEmpty()) + reportedErrors += Errors.ERROR_IN_CONTRACT_DESCRIPTION.on(mainCallReportTarget, "Error in contract description") + } - diagnostics.forEach { bindingTrace.report(it) } + override fun flushDiagnostics() { + reportedErrors.forEach { bindingTrace.report(it) } } - override fun hasErrors(): Boolean = diagnostics.isNotEmpty() + override fun hasErrors(): Boolean = reportedErrors.isNotEmpty() } \ No newline at end of file diff --git a/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt b/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt index 05e3d6d9a1a..19a0853eb7e 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/contracts/parsing/ContractParsingServices.kt @@ -48,16 +48,13 @@ class ContractParsingServices(val languageVersionSettings: LanguageVersionSettin // is a *necessary* (but not sufficient, actually) condition for presence of 'LazyContractProvider' if (!expression.isContractDescriptionCallPsiCheck()) return - val callContext = ContractCallContext(expression, isFirstStatement, scope, trace.bindingContext) + val callContext = ContractCallContext(expression, isFirstStatement, scope, trace) val contractProviderIfAny = (scope.ownerDescriptor as? FunctionDescriptor)?.getUserData(ContractProviderKey) var resultingContractDescription: ContractDescription? = null try { if (!callContext.isContractDescriptionCallPreciseCheck()) return - - val collector = TraceBasedCollector(trace, expression) - resultingContractDescription = doCheckContract(collector, callContext) - collector.flushDiagnostics(parsingFailed = resultingContractDescription == null) + resultingContractDescription = parseContractAndReportErrors(callContext) } finally { contractProviderIfAny?.setContractDescription(resultingContractDescription) } @@ -66,14 +63,32 @@ class ContractParsingServices(val languageVersionSettings: LanguageVersionSettin private fun ContractCallContext.isContractDescriptionCallPreciseCheck(): Boolean = contractCallExpression.isContractDescriptionCallPreciseCheck(bindingContext) - private fun doCheckContract(collector: ContractParsingDiagnosticsCollector, callContext: ContractCallContext): ContractDescription? { - checkFeatureEnabled(collector) - checkContractAllowedHere(collector, callContext) + /** + * This function deals with some call that is guaranteed to resolve to 'contract' from stdlib, so, + * 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) - return if (!collector.hasErrors()) - PsiContractParserDispatcher(collector, callContext).parseContract() - else - null + try { + checkFeatureEnabled(collector) + checkContractAllowedHere(collector, callContext) + + // Small optimization: do not even try to parse contract if we already have errors + if (collector.hasErrors()) return null + + val parsedContract = PsiContractParserDispatcher(collector, callContext).parseContract() + + // Make sure that at least generic error will be reported if we couldn't parse contract + // (null returned => at least one error was reported) + if (parsedContract == null) collector.addFallbackErrorIfNecessary() + + // Make sure that we don't return non-null value if there were some errors + // (null returned <= at least one error was reported) + return parsedContract?.takeUnless { collector.hasErrors() } + } finally { + collector.flushDiagnostics() + } } private fun checkFeatureEnabled(collector: ContractParsingDiagnosticsCollector) { @@ -117,8 +132,9 @@ class ContractCallContext( val contractCallExpression: KtExpression, val isFirstStatement: Boolean, val scope: LexicalScope, - val bindingContext: BindingContext + val trace: BindingTrace ) { val ownerDescriptor: DeclarationDescriptor = scope.ownerDescriptor val functionDescriptor: FunctionDescriptor = ownerDescriptor as FunctionDescriptor + val bindingContext: BindingContext = trace.bindingContext } \ No newline at end of file