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

FIR/FIR IDE: Use entire FirVariableAssignment when reporting UNSAFE_CALL

Browse Source 
(e.g., `nullable.a = b`), and use positioning strategies to locate the
dot in the LHS expression.

Without it, only the callee reference is reported on, which makes the
highlighting of the error and application of quickfixes incorrect in the
IDE.

Also fixed issue with annotated and/or labeled expressions on LHS of
assignment (e.g., `(@Ann label@ i) = 34`).
This commit is contained in:
Mark Punzalan
2021-05-13 09:24:29 +00:00
committed by Ilya Kirillov
parent 1d9247ae0f
commit d2b8204fdc
28 changed files with 342 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files
compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/DiagnosticData.kt
+2 -1
View File
@@ -58,7 +58,7 @@ enum class PositioningStrategy(private val strategy: String? = null) {
VALUE_ARGUMENTS,
SUPERTYPES_LIST,
RETURN_WITH_LABEL,
ASSIGNMENT_VALUE,
PROPERTY_INITIALIZER,
WHOLE_ELEMENT,
INT_LITERAL_OUT_OF_RANGE,
FLOAT_LITERAL_OUT_OF_RANGE,
@@ -71,6 +71,7 @@ enum class PositioningStrategy(private val strategy: String? = null) {
RESERVED_UNDERSCORE,
QUESTION_MARK_BY_TYPE,
ANNOTATION_USE_SITE,
ASSIGNMENT_LHS,
;
compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt
+2 -2
View File
@@ -55,7 +55,7 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
val ASSIGNMENT_IN_EXPRESSION_CONTEXT by error<KtBinaryExpression>()
val BREAK_OR_CONTINUE_OUTSIDE_A_LOOP by error<PsiElement>()
val NOT_A_LOOP_LABEL by error<PsiElement>()
val VARIABLE_EXPECTED by error<PsiElement>()
val VARIABLE_EXPECTED by error<PsiElement>(PositioningStrategy.ASSIGNMENT_LHS)
val DELEGATION_IN_INTERFACE by error<PsiElement>()
val NESTED_CLASS_NOT_ALLOWED by error<KtNamedDeclaration>(PositioningStrategy.DECLARATION_NAME) {
parameter<String>("declaration")
@@ -629,7 +629,7 @@ object DIAGNOSTICS_LIST : DiagnosticList() {
parameter<ConeKotlinType>("expectedType")
parameter<ConeKotlinType>("actualType")
}
val INITIALIZER_TYPE_MISMATCH by error<KtProperty>(PositioningStrategy.ASSIGNMENT_VALUE) {
val INITIALIZER_TYPE_MISMATCH by error<KtProperty>(PositioningStrategy.PROPERTY_INITIALIZER) {
parameter<ConeKotlinType>("expectedType")
parameter<ConeKotlinType>("actualType")
}
compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt
+2 -2
View File
@@ -87,7 +87,7 @@ object FirErrors {
val ASSIGNMENT_IN_EXPRESSION_CONTEXT by error0<KtBinaryExpression>()
val BREAK_OR_CONTINUE_OUTSIDE_A_LOOP by error0<PsiElement>()
val NOT_A_LOOP_LABEL by error0<PsiElement>()
val VARIABLE_EXPECTED by error0<PsiElement>()
val VARIABLE_EXPECTED by error0<PsiElement>(SourceElementPositioningStrategies.ASSIGNMENT_LHS)
val DELEGATION_IN_INTERFACE by error0<PsiElement>()
val NESTED_CLASS_NOT_ALLOWED by error1<KtNamedDeclaration, String>(SourceElementPositioningStrategies.DECLARATION_NAME)
val INCORRECT_CHARACTER_LITERAL by error0<PsiElement>()
@@ -384,7 +384,7 @@ object FirErrors {
val CONST_VAL_WITHOUT_INITIALIZER by error0<KtProperty>(SourceElementPositioningStrategies.CONST_MODIFIER)
val CONST_VAL_WITH_NON_CONST_INITIALIZER by error0<KtExpression>()
val WRONG_SETTER_PARAMETER_TYPE by error2<KtTypeReference, ConeKotlinType, ConeKotlinType>()
val INITIALIZER_TYPE_MISMATCH by error2<KtProperty, ConeKotlinType, ConeKotlinType>(SourceElementPositioningStrategies.ASSIGNMENT_VALUE)
val INITIALIZER_TYPE_MISMATCH by error2<KtProperty, ConeKotlinType, ConeKotlinType>(SourceElementPositioningStrategies.PROPERTY_INITIALIZER)
val GETTER_VISIBILITY_DIFFERS_FROM_PROPERTY_VISIBILITY by error0<KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
val SETTER_VISIBILITY_INCONSISTENT_WITH_PROPERTY_VISIBILITY by error0<KtModifierListOwner>(SourceElementPositioningStrategies.VISIBILITY_MODIFIER)
val WRONG_SETTER_RETURN_TYPE by error0<KtTypeReference>()
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/components/ErrorNodeDiagnosticCollectorComponent.kt
+2 -2
View File
@@ -47,9 +47,9 @@ class ErrorNodeDiagnosticCollectorComponent(
override fun visitErrorNamedReference(errorNamedReference: FirErrorNamedReference, data: CheckerContext) {
val source = errorNamedReference.source ?: return
val qualifiedAccessOrAnnotationCall = data.qualifiedAccessOrAnnotationCalls.lastOrNull()?.takeIf {
// Use the source of the enclosing FirQualifiedAccessExpression if it is exactly the call to the erroneous callee.
// Use the source of the enclosing FirQualifiedAccess if it is exactly the call to the erroneous callee.
when (it) {
is FirQualifiedAccessExpression -> it.calleeReference == errorNamedReference
is FirQualifiedAccess -> it.calleeReference == errorNamedReference
is FirAnnotationCall -> it.calleeReference == errorNamedReference
else -> false
}
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/LightTreePositioningStrategies.kt
+58
View File
@@ -425,6 +425,13 @@ object LightTreePositioningStrategies {
endOffset: Int,
tree: FlyweightCapableTreeStructure<LighterASTNode>
): List<TextRange> {
if (node.tokenType == KtNodeTypes.BINARY_EXPRESSION &&
tree.findDescendantByTypes(node, KtTokens.ALL_ASSIGNMENTS) != null
) {
tree.findDescendantByType(node, KtNodeTypes.DOT_QUALIFIED_EXPRESSION)?.let {
return markElement(tree.dotOperator(it) ?: it, startOffset, endOffset, tree, node)
}
}
if (node.tokenType == KtNodeTypes.DOT_QUALIFIED_EXPRESSION) {
return markElement(tree.dotOperator(node) ?: node, startOffset, endOffset, tree, node)
}
@@ -707,6 +714,33 @@ object LightTreePositioningStrategies {
}
}
val ASSIGNMENT_LHS: LightTreePositioningStrategy = object : LightTreePositioningStrategy() {
override fun mark(
node: LighterASTNode,
startOffset: Int,
endOffset: Int,
tree: FlyweightCapableTreeStructure<LighterASTNode>
): List<TextRange> {
if ((node.tokenType == KtNodeTypes.BINARY_EXPRESSION &&
tree.findDescendantByTypes(node, KtTokens.ALL_ASSIGNMENTS) != null) ||
((node.tokenType == KtNodeTypes.PREFIX_EXPRESSION || node.tokenType == KtNodeTypes.POSTFIX_EXPRESSION) &&
tree.findDescendantByTypes(node, KtTokens.INCREMENT_AND_DECREMENT) != null)
) {
val lhs = if (node.tokenType == KtNodeTypes.PREFIX_EXPRESSION) {
tree.lastChildExpression(node)
} else {
tree.firstChildExpression(node)
}
lhs?.let {
tree.unwrapParenthesesLabelsAndAnnotations(it)?.let { unwrapped ->
return markElement(unwrapped, startOffset, endOffset, tree, node)
}
}
}
return super.mark(node, startOffset, endOffset, tree)
}
}
val ANNOTATION_USE_SITE: LightTreePositioningStrategy = object : LightTreePositioningStrategy() {
override fun mark(
node: LighterASTNode,
@@ -810,6 +844,18 @@ private fun FlyweightCapableTreeStructure<LighterASTNode>.referenceExpression(
return result
}
private fun FlyweightCapableTreeStructure<LighterASTNode>.unwrapParenthesesLabelsAndAnnotations(node: LighterASTNode): LighterASTNode? {
var unwrapped = node
while (true) {
unwrapped = when (unwrapped.tokenType) {
KtNodeTypes.PARENTHESIZED -> firstChildExpression(unwrapped) ?: return unwrapped
KtNodeTypes.LABELED_EXPRESSION -> lastChildExpression(unwrapped) ?: return unwrapped
KtNodeTypes.ANNOTATED_EXPRESSION -> firstChildExpression(unwrapped) ?: return unwrapped
else -> return unwrapped
}
}
}
private fun FlyweightCapableTreeStructure<LighterASTNode>.findExpressionDeep(node: LighterASTNode): LighterASTNode? =
findFirstDescendant(node) { it.isExpression() }
@@ -898,6 +944,18 @@ fun FlyweightCapableTreeStructure<LighterASTNode>.selector(node: LighterASTNode)
}
fun FlyweightCapableTreeStructure<LighterASTNode>.firstChildExpression(node: LighterASTNode): LighterASTNode? {
val childrenRef = Ref<Array<LighterASTNode?>>()
getChildren(node, childrenRef)
return childrenRef.get()?.firstOrNull { it?.isExpression() == true }
}
fun FlyweightCapableTreeStructure<LighterASTNode>.lastChildExpression(node: LighterASTNode): LighterASTNode? {
val childrenRef = Ref<Array<LighterASTNode?>>()
getChildren(node, childrenRef)
return childrenRef.get()?.lastOrNull { it?.isExpression() == true }
}
fun FlyweightCapableTreeStructure<LighterASTNode>.findChildByType(node: LighterASTNode, type: IElementType): LighterASTNode? {
val childrenRef = Ref<Array<LighterASTNode?>>()
getChildren(node, childrenRef)
compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/SourceElementPositioningStrategies.kt
+7 -2
View File
@@ -208,9 +208,9 @@ object SourceElementPositioningStrategies {
PositioningStrategies.RETURN_WITH_LABEL
)
val ASSIGNMENT_VALUE = SourceElementPositioningStrategy(
val PROPERTY_INITIALIZER = SourceElementPositioningStrategy(
LightTreePositioningStrategies.LAST_CHILD,
PositioningStrategies.ASSIGNMENT_VALUE
PositioningStrategies.PROPERTY_INITIALIZER
)
val WHOLE_ELEMENT = SourceElementPositioningStrategy(
@@ -247,4 +247,9 @@ object SourceElementPositioningStrategies {
LightTreePositioningStrategies.ANNOTATION_USE_SITE,
PositioningStrategies.ANNOTATION_USE_SITE
)
val ASSIGNMENT_LHS = SourceElementPositioningStrategy(
LightTreePositioningStrategies.ASSIGNMENT_LHS,
PositioningStrategies.ASSIGNMENT_LHS
)
}
compiler/fir/raw-fir/psi2fir/tests/org/jetbrains/kotlin/fir/builder/AbstractRawFirBuilderTestCase.kt
+16 -5
View File
@@ -83,20 +83,31 @@ abstract class AbstractRawFirBuilderTestCase : KtParsingTestCase(
if (!result.add(this)) {
return result
}
propertyLoop@ for (property in this::class.memberProperties) {
val childElement = property.getter.apply { isAccessible = true }.call(this)
for (property in this::class.memberProperties) {
if (hasNoAcceptAndTransform(this::class.simpleName, property.name)) continue
when (childElement) {
is FirNoReceiverExpression -> continue@propertyLoop
when (val childElement = property.getter.apply { isAccessible = true }.call(this)) {
is FirNoReceiverExpression -> continue
is FirElement -> childElement.traverseChildren(result)
is List<*> -> childElement.filterIsInstance<FirElement>().forEach { it.traverseChildren(result) }
else -> continue@propertyLoop
else -> continue
}
}
return result
}
private val firImplClassPropertiesWithNoAcceptAndTransform = mapOf(
"FirResolvedImportImpl" to "delegate",
"FirErrorTypeRefImpl" to "delegatedTypeRef",
"FirResolvedTypeRefImpl" to "delegatedTypeRef"
)
private fun hasNoAcceptAndTransform(className: String?, propertyName: String): Boolean {
if (className == null) return false
return firImplClassPropertiesWithNoAcceptAndTransform[className] == propertyName
}
private fun FirFile.visitChildren(): Set<FirElement> =
ConsistencyVisitor().let {
this@visitChildren.accept(it)
compiler/fir/raw-fir/raw-fir.common/src/org/jetbrains/kotlin/fir/builder/BaseFirBuilder.kt
+36 -39
View File
@@ -445,9 +445,8 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
*/
// TODO:
// 1. Support receiver capturing for `array.b++` (elementType == ARRAY_ACCESS_EXPRESSION).
// 2. Support receiver capturing for `a?.b++` (elementType == SAFE_ACCESS_EXPRESSION).
// 3. Add box test cases for #1 and #2 where receiver expression has side effects.
// 1. Support receiver capturing for `a?.b++` (elementType == SAFE_ACCESS_EXPRESSION).
// 2. Add box test cases for #1 where receiver expression has side effects.
fun generateIncrementOrDecrementBlock(
baseExpression: T,
operationReference: T?,
@@ -456,21 +455,8 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
prefix: Boolean,
convert: T.() -> FirExpression
): FirExpression {
// NOTE: By removing surrounding parentheses and labels, FirLabels will NOT be created for those labels.
// This should be fine since the label is meaningless and unusable for a ++/-- argument.
var unwrappedArgument = argument
while (true) {
unwrappedArgument = when (unwrappedArgument?.elementType) {
PARENTHESIZED -> unwrappedArgument?.getExpressionInParentheses()
LABELED_EXPRESSION -> unwrappedArgument?.getLabeledExpression()
else -> break
}
}
if (unwrappedArgument == null) {
return buildErrorExpression {
diagnostic = ConeSimpleDiagnostic("Inc/dec without operand", DiagnosticKind.Syntax)
}
val unwrappedArgument = argument.unwrap() ?: return buildErrorExpression {
diagnostic = ConeSimpleDiagnostic("Inc/dec without operand", DiagnosticKind.Syntax)
}
if (unwrappedArgument.elementType == DOT_QUALIFIED_EXPRESSION) {
@@ -566,6 +552,20 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
}
}
private fun T?.unwrap(): T? {
// NOTE: By removing surrounding parentheses and labels, FirLabels will NOT be created for those labels.
// This should be fine since the label is meaningless and unusable for a ++/-- argument or assignment LHS.
var unwrapped = this
while (true) {
unwrapped = when (unwrapped?.elementType) {
PARENTHESIZED -> unwrapped?.getExpressionInParentheses()
LABELED_EXPRESSION -> unwrapped?.getLabeledExpression()
ANNOTATED_EXPRESSION -> unwrapped?.getAnnotatedExpression()
else -> return unwrapped
}
}
}
/**
* given:
* a.b++
@@ -860,12 +860,6 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
}
}
}
PARENTHESIZED -> {
return initializeLValue(left.getExpressionInParentheses(), convertQualified)
}
ANNOTATED_EXPRESSION -> {
return initializeLValue(left.getAnnotatedExpression(), convertQualified)
}
}
}
return buildErrorNamedReference {
@@ -881,19 +875,19 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
operation: FirOperation,
convert: T.() -> FirExpression
): FirStatement {
val tokenType = this?.elementType
if (tokenType == PARENTHESIZED) {
return this!!.getExpressionInParentheses().generateAssignment(baseSource, rhs, value, operation, convert)
val unwrappedLhs = this.unwrap() ?: return buildErrorExpression {
diagnostic = ConeSimpleDiagnostic("Inc/dec without operand", DiagnosticKind.Syntax)
}
val tokenType = unwrappedLhs.elementType
if (tokenType == ARRAY_ACCESS_EXPRESSION) {
require(this != null)
if (operation == FirOperation.ASSIGN) {
context.arraySetArgument[this] = value
context.arraySetArgument[unwrappedLhs] = value
}
return if (operation == FirOperation.ASSIGN) {
this.convert()
unwrappedLhs.convert()
} else {
generateAugmentedArraySetCall(baseSource, operation, rhs, convert)
generateAugmentedArraySetCall(unwrappedLhs, baseSource, operation, rhs, convert)
}
}
@@ -924,7 +918,7 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
return buildVariableAssignment {
source = baseSource
rValue = value
calleeReference = initializeLValue(this@generateAssignment) { convert() as? FirQualifiedAccess }
calleeReference = initializeLValue(unwrappedLhs) { convert() as? FirQualifiedAccess }
}
}
@@ -950,7 +944,8 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
return safeCallNonAssignment
}
private fun T.generateAugmentedArraySetCall(
private fun generateAugmentedArraySetCall(
unwrappedReceiver: T,
baseSource: FirSourceElement?,
operation: FirOperation,
rhs: T?,
@@ -959,12 +954,13 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
return buildAugmentedArraySetCall {
source = baseSource
this.operation = operation
assignCall = generateAugmentedCallForAugmentedArraySetCall(operation, rhs, convert)
setGetBlock = generateSetGetBlockForAugmentedArraySetCall(baseSource, operation, rhs, convert)
assignCall = generateAugmentedCallForAugmentedArraySetCall(unwrappedReceiver, operation, rhs, convert)
setGetBlock = generateSetGetBlockForAugmentedArraySetCall(unwrappedReceiver, baseSource, operation, rhs, convert)
}
}
private fun T.generateAugmentedCallForAugmentedArraySetCall(
private fun generateAugmentedCallForAugmentedArraySetCall(
unwrappedReceiver: T,
operation: FirOperation,
rhs: T?,
convert: T.() -> FirExpression
@@ -977,7 +973,7 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
calleeReference = buildSimpleNamedReference {
name = FirOperationNameConventions.ASSIGNMENTS.getValue(operation)
}
explicitReceiver = convert()
explicitReceiver = unwrappedReceiver.convert()
argumentList = buildArgumentList {
arguments += rhs?.convert() ?: buildErrorExpression(
null,
@@ -989,7 +985,8 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
}
private fun T.generateSetGetBlockForAugmentedArraySetCall(
private fun generateSetGetBlockForAugmentedArraySetCall(
unwrappedReceiver: T,
baseSource: FirSourceElement?,
operation: FirOperation,
rhs: T?,
@@ -1005,7 +1002,7 @@ abstract class BaseFirBuilder<T>(val baseSession: FirSession, val context: Conte
* }
*/
return buildBlock {
val baseCall = convert() as FirFunctionCall
val baseCall = unwrappedReceiver.convert() as FirFunctionCall
val arrayVariable = generateTemporaryVariable(
baseModuleData,