Drop Experimental.Impact, simplify related code and tests

See https://github.com/Kotlin/KEEP/issues/95#issuecomment-383889404

Drop Experimental.changesMayBreak, Experimental.Impact, the concept of
signature/body usage, same module exemption. Make the majority of tests
single-module because there is now no difference in the checker between
usages from the same module or from another module
This commit is contained in:
Alexander Udalov
2018-04-25 19:16:28 +02:00
parent 1719eecb67
commit 4f53a54115
66 changed files with 350 additions and 1026 deletions
@@ -234,16 +234,14 @@ public interface Errors {
DiagnosticFactory1<PsiElement, FqName> ILLEGAL_KOTLIN_VERSION_STRING_VALUE = DiagnosticFactory1.create(ERROR);
DiagnosticFactory1<PsiElement, String> NEWER_VERSION_IN_SINCE_KOTLIN = DiagnosticFactory1.create(WARNING);
DiagnosticFactory2<PsiElement, FqName, Boolean> EXPERIMENTAL_API_USAGE = DiagnosticFactory2.create(WARNING);
DiagnosticFactory2<PsiElement, FqName, Boolean> EXPERIMENTAL_API_USAGE_ERROR = DiagnosticFactory2.create(ERROR);
DiagnosticFactory1<PsiElement, FqName> EXPERIMENTAL_API_USAGE = DiagnosticFactory1.create(WARNING);
DiagnosticFactory1<PsiElement, FqName> EXPERIMENTAL_API_USAGE_ERROR = DiagnosticFactory1.create(ERROR);
DiagnosticFactory2<PsiElement, FqName, DeclarationDescriptor> EXPERIMENTAL_OVERRIDE = DiagnosticFactory2.create(WARNING);
DiagnosticFactory2<PsiElement, FqName, DeclarationDescriptor> EXPERIMENTAL_OVERRIDE_ERROR = DiagnosticFactory2.create(ERROR);
DiagnosticFactory0<KtAnnotationEntry> USE_EXPERIMENTAL_WITHOUT_ARGUMENTS = DiagnosticFactory0.create(WARNING);
DiagnosticFactory1<KtAnnotationEntry, FqName> USE_EXPERIMENTAL_ARGUMENT_IS_NOT_MARKER = DiagnosticFactory1.create(WARNING);
DiagnosticFactory1<KtAnnotationEntry, FqName> USE_EXPERIMENTAL_ARGUMENT_HAS_NON_COMPILATION_IMPACT = DiagnosticFactory1.create(ERROR);
DiagnosticFactory0<KtAnnotationEntry> EXPERIMENTAL_ANNOTATION_WITH_NO_IMPACT = DiagnosticFactory0.create(ERROR);
DiagnosticFactory1<KtAnnotationEntry, String> EXPERIMENTAL_ANNOTATION_WITH_WRONG_TARGET = DiagnosticFactory1.create(ERROR);
// Const
@@ -15,7 +15,6 @@ import org.jetbrains.kotlin.diagnostics.Diagnostic;
import org.jetbrains.kotlin.diagnostics.DiagnosticFactory;
import org.jetbrains.kotlin.diagnostics.Errors;
import org.jetbrains.kotlin.metadata.deserialization.VersionRequirement;
import org.jetbrains.kotlin.name.FqName;
import org.jetbrains.kotlin.resolve.VarianceConflictDiagnosticData;
import org.jetbrains.kotlin.types.KotlinTypeKt;
import org.jetbrains.kotlin.util.OperatorNameConventions;
@@ -23,7 +22,6 @@ import org.jetbrains.kotlin.utils.addToStdlib.AddToStdlibKt;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.ServiceLoader;
@@ -142,25 +140,14 @@ public class DefaultErrorMessages {
MAP.put(ILLEGAL_KOTLIN_VERSION_STRING_VALUE, "Invalid @{0} annotation value (should be ''major.minor'' or ''major.minor.patch'')", TO_STRING);
MAP.put(NEWER_VERSION_IN_SINCE_KOTLIN, "The version is greater than the specified API version {0}", STRING);
RenderingContext.Key<FqName> getExperimentalFqName = new RenderingContext.Key<FqName>("getExperimentalFqName") {
@Override
public FqName compute(@NotNull Collection<?> objectsToRender) {
return (FqName) CollectionsKt.first(objectsToRender);
}
};
DiagnosticParameterRenderer<Boolean> renderUseExperimental = (useExperimentalAllowed, c) ->
useExperimentalAllowed ? " or ''@UseExperimental(" + c.get(getExperimentalFqName) + "::class)''" : "";
MAP.put(EXPERIMENTAL_API_USAGE, "This declaration is experimental and its usage should be marked with ''@{0}''{1}", TO_STRING, renderUseExperimental);
MAP.put(EXPERIMENTAL_API_USAGE_ERROR, "This declaration is experimental and its usage must be marked with ''@{0}''{1}", TO_STRING, renderUseExperimental);
MAP.put(EXPERIMENTAL_API_USAGE, "This declaration is experimental and its usage should be marked with ''@{0}'' or ''@UseExperimental({0}::class)''", TO_STRING);
MAP.put(EXPERIMENTAL_API_USAGE_ERROR, "This declaration is experimental and its usage must be marked with ''@{0}'' or ''@UseExperimental({0}::class)''", TO_STRING);
MAP.put(EXPERIMENTAL_OVERRIDE, "This declaration overrides experimental member of supertype ''{1}'' and should be annotated with ''@{0}''", TO_STRING, NAME);
MAP.put(EXPERIMENTAL_OVERRIDE_ERROR, "This declaration overrides experimental member of supertype ''{1}'' and must be annotated with ''@{0}''", TO_STRING, NAME);
MAP.put(USE_EXPERIMENTAL_WITHOUT_ARGUMENTS, "@UseExperimental without any arguments has no effect");
MAP.put(USE_EXPERIMENTAL_ARGUMENT_IS_NOT_MARKER, "Annotation ''{0}'' is not an experimental API marker, therefore its usage in @UseExperimental is ignored", TO_STRING);
MAP.put(USE_EXPERIMENTAL_ARGUMENT_HAS_NON_COMPILATION_IMPACT,
"Experimental annotation ''{0}'' has impact other than COMPILATION, therefore its usage in @UseExperimental is forbidden", TO_STRING);
MAP.put(EXPERIMENTAL_ANNOTATION_WITH_NO_IMPACT, "Experimental annotation with changesMayBreak = [] is not allowed");
MAP.put(EXPERIMENTAL_ANNOTATION_WITH_WRONG_TARGET, "Experimental annotation cannot be used on the following code elements: {0}. Please remove these targets", STRING);
MAP.put(REDUNDANT_MODIFIER, "Modifier ''{0}'' is redundant because ''{1}'' is present", TO_STRING, TO_STRING);
@@ -34,8 +34,6 @@ object ExperimentalMarkerDeclarationAnnotationChecker : AdditionalAnnotationChec
checkUseExperimentalUsage(annotationClasses, trace, entry)
}
ExperimentalUsageChecker.EXPERIMENTAL_FQ_NAME -> {
val impact = (annotation.allValueArguments[ExperimentalUsageChecker.IMPACT] as? ArrayValue)?.value
checkExperimentalUsage(impact, trace, entry)
isAnnotatedWithExperimental = true
}
}
@@ -54,31 +52,22 @@ object ExperimentalMarkerDeclarationAnnotationChecker : AdditionalAnnotationChec
for (annotationClass in annotationClasses) {
val classDescriptor =
(annotationClass as? KClassValue)?.value?.constructor?.declarationDescriptor as? ClassDescriptor
?: continue
(annotationClass as? KClassValue)?.value?.constructor?.declarationDescriptor as? ClassDescriptor ?: continue
val experimentality = with(ExperimentalUsageChecker) {
classDescriptor.loadExperimentalityForMarkerAnnotation()
}
if (experimentality == null) {
trace.report(Errors.USE_EXPERIMENTAL_ARGUMENT_IS_NOT_MARKER.on(entry, classDescriptor.fqNameSafe))
} else if (!experimentality.isCompilationOnly) {
trace.report(Errors.USE_EXPERIMENTAL_ARGUMENT_HAS_NON_COMPILATION_IMPACT.on(entry, experimentality.annotationFqName))
}
}
}
private fun checkExperimentalUsage(impact: List<ConstantValue<*>>?, trace: BindingTrace, entry: KtAnnotationEntry) {
if (impact != null && impact.isEmpty()) {
trace.report(Errors.EXPERIMENTAL_ANNOTATION_WITH_NO_IMPACT.on(entry))
}
}
private fun checkMarkerTargets(entries: List<KtAnnotationEntry>, trace: BindingTrace) {
val targetEntry =
entries.associate { entry -> entry to trace.bindingContext.get(BindingContext.ANNOTATION, entry) }
.entries
.firstOrNull { (_, descriptor) -> descriptor != null && descriptor.fqName == KotlinBuiltIns.FQ_NAMES.target }
?: return
?: return
val (entry, descriptor) = targetEntry
val allowedTargets = AnnotationChecker.loadAnnotationTargets(descriptor!!) ?: return
val wrongTargets = allowedTargets.intersect(WRONG_TARGETS_FOR_MARKER)
@@ -26,7 +26,9 @@ import org.jetbrains.kotlin.diagnostics.Errors
import org.jetbrains.kotlin.incremental.components.NoLookupLocation
import org.jetbrains.kotlin.name.FqName
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.KtAnnotated
import org.jetbrains.kotlin.psi.KtDeclaration
import org.jetbrains.kotlin.psi.KtNamedDeclaration
import org.jetbrains.kotlin.resolve.*
import org.jetbrains.kotlin.resolve.calls.checkers.CallChecker
import org.jetbrains.kotlin.resolve.calls.checkers.CallCheckerContext
@@ -44,20 +46,11 @@ import org.jetbrains.kotlin.utils.addIfNotNull
class ExperimentalUsageChecker(project: Project) : CallChecker {
private val moduleAnnotationsResolver = ModuleAnnotationsResolver.getInstance(project)
internal data class Experimentality(
val markerDescriptor: ClassDescriptor,
val annotationFqName: FqName,
val severity: Severity,
val impact: List<Impact>
) {
val isCompilationOnly: Boolean get() = impact.all(Impact.COMPILATION::equals)
internal data class Experimentality(val annotationFqName: FqName, val severity: Severity) {
enum class Severity { WARNING, ERROR }
enum class Impact { COMPILATION, LINKAGE_OR_RUNTIME }
companion object {
val DEFAULT_SEVERITY = Severity.ERROR
val DEFAULT_IMPACT = listOf(Impact.COMPILATION, Impact.LINKAGE_OR_RUNTIME)
}
}
@@ -74,11 +67,6 @@ class ExperimentalUsageChecker(project: Project) : CallChecker {
private val WARNING_LEVEL = Name.identifier("WARNING")
private val ERROR_LEVEL = Name.identifier("ERROR")
internal val IMPACT = Name.identifier("changesMayBreak")
private val COMPILATION_IMPACT = Name.identifier("COMPILATION")
private val LINKAGE_IMPACT = Name.identifier("LINKAGE")
private val RUNTIME_IMPACT = Name.identifier("RUNTIME")
private fun checkExperimental(
descriptor: DeclarationDescriptor,
element: PsiElement,
@@ -88,14 +76,13 @@ class ExperimentalUsageChecker(project: Project) : CallChecker {
val experimentalities = descriptor.loadExperimentalities(moduleAnnotationsResolver)
if (experimentalities.isNotEmpty()) {
checkExperimental(
experimentalities, element, context.trace.bindingContext, context.languageVersionSettings,
context.moduleDescriptor
) { experimentality, isBodyUsageOfSourceOnlyExperimentality ->
val diagnostic = when (experimentality.severity) {
experimentalities, element, context.trace.bindingContext, context.languageVersionSettings
) { (annotationFqName, severity) ->
val diagnostic = when (severity) {
Experimentality.Severity.WARNING -> Errors.EXPERIMENTAL_API_USAGE
Experimentality.Severity.ERROR -> Errors.EXPERIMENTAL_API_USAGE_ERROR
}
context.trace.report(diagnostic.on(element, experimentality.annotationFqName, isBodyUsageOfSourceOnlyExperimentality))
context.trace.report(diagnostic.on(element, annotationFqName))
}
}
}
@@ -105,28 +92,19 @@ class ExperimentalUsageChecker(project: Project) : CallChecker {
element: PsiElement,
bindingContext: BindingContext,
languageVersionSettings: LanguageVersionSettings,
module: ModuleDescriptor,
report: (experimentality: Experimentality, isBodyUsageOfCompilationExperimentality: Boolean) -> Unit
report: (Experimentality) -> Unit
) {
val isBodyUsageExceptPublicInline = element.isBodyUsage(bindingContext, allowPublicInline = false)
val isBodyUsage = isBodyUsageExceptPublicInline || element.isBodyUsage(bindingContext, allowPublicInline = true)
for (experimentality in experimentalities) {
val isBodyUsageOfCompilationExperimentality =
experimentality.isCompilationOnly && isBodyUsage
val isBodyUsageInSameModule =
experimentality.markerDescriptor.module == module && isBodyUsageExceptPublicInline
val annotationFqName = experimentality.annotationFqName
val isExperimentalityAccepted =
isBodyUsageInSameModule ||
(isBodyUsageOfCompilationExperimentality &&
element.hasContainerAnnotatedWithUseExperimental(annotationFqName, bindingContext, languageVersionSettings)) ||
element.propagates(annotationFqName, bindingContext, languageVersionSettings)
element.hasContainerAnnotatedWithUseExperimental(
annotationFqName, bindingContext, languageVersionSettings
) || element.propagates(
annotationFqName, bindingContext, languageVersionSettings
)
if (!isExperimentalityAccepted) {
report(experimentality, isBodyUsageOfCompilationExperimentality)
report(experimentality)
}
}
}
@@ -162,46 +140,7 @@ class ExperimentalUsageChecker(project: Project) : CallChecker {
else -> Experimentality.DEFAULT_SEVERITY
}
val impact = (experimental.allValueArguments[IMPACT] as? ArrayValue)?.value?.mapNotNull { impact ->
when ((impact as? EnumValue)?.enumEntryName) {
COMPILATION_IMPACT -> Experimentality.Impact.COMPILATION
LINKAGE_IMPACT, RUNTIME_IMPACT -> Experimentality.Impact.LINKAGE_OR_RUNTIME
else -> null
}
} ?: Experimentality.DEFAULT_IMPACT
return Experimentality(this, fqNameSafe, severity, impact)
}
// Returns true if this element appears in the body of some function and is not visible in any non-local declaration signature.
// If that's the case, one can opt-in to using the corresponding experimental API by annotating this element (or any of its
// enclosing declarations) with @UseExperimental(X::class), not requiring propagation of the experimental annotation to the call sites.
// (Note that this is allowed only if X's impact is [COMPILATION].)
private fun PsiElement.isBodyUsage(bindingContext: BindingContext, allowPublicInline: Boolean): Boolean {
return anyParentMatches { element, parent ->
element == (parent as? KtDeclarationWithBody)?.bodyExpression?.takeIf {
allowPublicInline || !parent.isPublicInline(bindingContext)
} ||
element == (parent as? KtDeclarationWithInitializer)?.initializer ||
element == (parent as? KtClassInitializer)?.body ||
element == (parent as? KtParameter)?.defaultValue ||
element == (parent as? KtSuperTypeCallEntry)?.valueArgumentList ||
element == (parent as? KtDelegatedSuperTypeEntry)?.delegateExpression ||
element == (parent as? KtPropertyDelegate)?.expression
}
}
private fun PsiElement.isPublicInline(bindingContext: BindingContext): Boolean {
val descriptor = when (this) {
is KtFunction -> bindingContext.get(BindingContext.FUNCTION, this)
is KtPropertyAccessor -> bindingContext.get(BindingContext.PROPERTY_ACCESSOR, this)
else -> null
}
return descriptor != null && descriptor.isInline && descriptor.effectiveVisibility().let {
it == EffectiveVisibility.Public ||
it == EffectiveVisibility.ProtectedBound ||
it is EffectiveVisibility.Protected
}
return Experimentality(fqNameSafe, severity)
}
// Checks whether any of the non-local enclosing declarations is annotated with annotationFqName, effectively requiring
@@ -263,18 +202,14 @@ class ExperimentalUsageChecker(project: Project) : CallChecker {
// module annotations. For now, we only check deprecations because this is needed to correctly retire unneeded compiler arguments.
val deprecationResolver = DeprecationResolver(LockBasedStorageManager(), languageVersionSettings)
fun checkAnnotation(fqName: String, allowNonCompilationImpact: Boolean): Boolean {
fun checkAnnotation(fqName: String): Boolean {
val descriptor = module.resolveClassByFqName(FqName(fqName), NoLookupLocation.FOR_NON_TRACKED_SCOPE)
val experimentality = descriptor?.loadExperimentalityForMarkerAnnotation()
val message = when {
descriptor == null ->
"Experimental API marker $fqName is unresolved. " +
"Please make sure it's present in the module dependencies"
"Experimental API marker $fqName is unresolved. Please make sure it's present in the module dependencies"
experimentality == null ->
"Class $fqName is not an experimental API marker annotation"
!allowNonCompilationImpact && !experimentality.impact.all(Experimentality.Impact.COMPILATION::equals) ->
"Experimental API marker $fqName has impact other than COMPILATION, " +
"therefore it can't be used with -Xuse-experimental"
else -> {
for (deprecation in deprecationResolver.getDeprecations(descriptor)) {
val report = when (deprecation.deprecationLevel) {
@@ -292,12 +227,8 @@ class ExperimentalUsageChecker(project: Project) : CallChecker {
return false
}
val validExperimental =
languageVersionSettings.getFlag(AnalysisFlag.experimental)
.filter { checkAnnotation(it, allowNonCompilationImpact = true) }
val validUseExperimental =
languageVersionSettings.getFlag(AnalysisFlag.useExperimental)
.filter { checkAnnotation(it, allowNonCompilationImpact = false) }
val validExperimental = languageVersionSettings.getFlag(AnalysisFlag.experimental).filter(::checkAnnotation)
val validUseExperimental = languageVersionSettings.getFlag(AnalysisFlag.useExperimental).filter(::checkAnnotation)
for (fqName in validExperimental.intersect(validUseExperimental)) {
reportError("'-Xuse-experimental=$fqName' has no effect because '-Xexperimental=$fqName' is used")
@@ -323,21 +254,18 @@ class ExperimentalUsageChecker(project: Project) : CallChecker {
member.loadExperimentalities(moduleAnnotationsResolver).map { experimentality -> experimentality to member }
}.toMap()
val module = descriptor.module
for ((experimentality, member) in experimentalOverridden) {
checkExperimental(
listOf(experimentality), declaration, context.trace.bindingContext, context.languageVersionSettings, module
) { _, _ ->
val diagnostic = when (experimentality.severity) {
listOf(experimentality), declaration, context.trace.bindingContext, context.languageVersionSettings
) { (annotationFqName, severity) ->
val diagnostic = when (severity) {
Experimentality.Severity.WARNING -> Errors.EXPERIMENTAL_OVERRIDE
Experimentality.Severity.ERROR -> Errors.EXPERIMENTAL_OVERRIDE_ERROR
}
val reportOn = (declaration as? KtNamedDeclaration)?.nameIdentifier ?: declaration
context.trace.report(diagnostic.on(reportOn, experimentality.annotationFqName, member.containingDeclaration))
context.trace.report(diagnostic.on(reportOn, annotationFqName, member.containingDeclaration))
}
}
}
}
}