[FIR] Prohibit confusing syntax inside when branches

^KT-48385

compiler/fir/checkers/checkers-component-generator/src/org/jetbrains/kotlin/fir/checkers/generator/diagnostics/FirDiagnosticsList.kt

@@ -1117,6 +1117,7 @@ object DIAGNOSTICS_LIST : DiagnosticList("FirErrors") {
         }
         val COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT by error<PsiElement>(PositioningStrategy.COMMAS)
         val DUPLICATE_LABEL_IN_WHEN by warning<KtElement>()
+        val CONFUSING_BRANCH_CONDITION by deprecationError<PsiElement>(LanguageFeature.ProhibitConfusingSyntaxInWhenBranches)
     }
 
     val CONTEXT_TRACKING by object : DiagnosticGroup("Context tracking") {

compiler/fir/checkers/gen/org/jetbrains/kotlin/fir/analysis/diagnostics/FirErrors.kt

@@ -9,6 +9,7 @@ import com.intellij.psi.PsiElement
 import com.intellij.psi.impl.source.tree.LeafPsiElement
 import org.jetbrains.kotlin.config.LanguageFeature
 import org.jetbrains.kotlin.config.LanguageFeature.ProhibitAssigningSingleElementsToVarargsInNamedForm
+import org.jetbrains.kotlin.config.LanguageFeature.ProhibitConfusingSyntaxInWhenBranches
 import org.jetbrains.kotlin.config.LanguageFeature.ProhibitInvisibleAbstractMethodsInSuperclasses
 import org.jetbrains.kotlin.config.LanguageFeature.ProhibitNonReifiedArraysAsReifiedTypeArguments
 import org.jetbrains.kotlin.config.LanguageFeature.ProhibitUseSiteTargetAnnotationsOnSuperTypes
@@ -596,6 +597,7 @@ object FirErrors {
     val ILLEGAL_DECLARATION_IN_WHEN_SUBJECT by error1<KtElement, String>()
     val COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT by error0<PsiElement>(SourceElementPositioningStrategies.COMMAS)
     val DUPLICATE_LABEL_IN_WHEN by warning0<KtElement>()
+    val CONFUSING_BRANCH_CONDITION by deprecationError0<PsiElement>(ProhibitConfusingSyntaxInWhenBranches)
 
     // Context tracking
     val TYPE_PARAMETER_IS_NOT_AN_EXPRESSION by error1<KtSimpleNameExpression, FirTypeParameterSymbol>()

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/CommonExpressionCheckers.kt

@@ -7,6 +7,7 @@ package org.jetbrains.kotlin.fir.analysis.checkers
 
 import org.jetbrains.kotlin.fir.analysis.checkers.expression.*
 import org.jetbrains.kotlin.fir.analysis.checkers.syntax.FirCommaInWhenConditionChecker
+import org.jetbrains.kotlin.fir.analysis.checkers.syntax.FirConfusingWhenBranchSyntaxChecker
 
 object CommonExpressionCheckers : ExpressionCheckers() {
     override val annotationCallCheckers: Set<FirAnnotationCallChecker>
@@ -77,6 +78,7 @@ object CommonExpressionCheckers : ExpressionCheckers() {
             FirWhenConditionChecker,
             FirWhenSubjectChecker,
             FirCommaInWhenConditionChecker,
+            FirConfusingWhenBranchSyntaxChecker,
         )
 
     override val loopExpressionCheckers: Set<FirLoopExpressionChecker>

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/syntax/FirConfusingWhenBranchSyntaxChecker.kt

@@ -0,0 +1,118 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * 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.fir.analysis.checkers.syntax
+
+import com.intellij.lang.LighterASTNode
+import com.intellij.psi.PsiElement
+import com.intellij.psi.tree.TokenSet
+import com.intellij.util.diff.FlyweightCapableTreeStructure
+import org.jetbrains.kotlin.ElementTypeUtils.getOperationSymbol
+import org.jetbrains.kotlin.KtNodeTypes.*
+import org.jetbrains.kotlin.fir.FirLightSourceElement
+import org.jetbrains.kotlin.fir.FirPsiSourceElement
+import org.jetbrains.kotlin.fir.FirRealPsiSourceElement
+import org.jetbrains.kotlin.fir.FirSourceElement
+import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
+import org.jetbrains.kotlin.fir.analysis.checkers.getChildren
+import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors
+import org.jetbrains.kotlin.fir.analysis.diagnostics.isExpression
+import org.jetbrains.kotlin.fir.analysis.diagnostics.reportOn
+import org.jetbrains.kotlin.fir.expressions.FirWhenExpression
+import org.jetbrains.kotlin.lexer.KtTokens.*
+import org.jetbrains.kotlin.psi.*
+
+object FirConfusingWhenBranchSyntaxChecker : FirExpressionSyntaxChecker<FirWhenExpression, PsiElement>() {
+    private val prohibitedTokens = TokenSet.create(
+        IN_KEYWORD, NOT_IN,
+        LT, LTEQ, GT, GTEQ,
+        EQEQ, EXCLEQ, EQEQEQ, EXCLEQEQEQ,
+        ANDAND, OROR
+    )
+
+    override fun checkLightTree(
+        element: FirWhenExpression,
+        source: FirSourceElement,
+        context: CheckerContext,
+        reporter: DiagnosticReporter
+    ) {
+        if (element.subject == null && element.subjectVariable == null) return
+        val tree = source.treeStructure
+        val entries = source.lighterASTNode.getChildren(tree).filter { it.tokenType == WHEN_ENTRY }
+        val offset = source.startOffset - source.lighterASTNode.startOffset
+        for (entry in entries) {
+            for (node in entry.getChildren(tree)) {
+                val expression = when (node.tokenType) {
+                    WHEN_CONDITION_EXPRESSION -> node.getChildren(tree).firstOrNull { it.isExpression() }
+                    WHEN_CONDITION_IN_RANGE -> node.getChildren(tree)
+                        .firstOrNull { it.tokenType != OPERATION_REFERENCE && it.isExpression()}
+                    else -> null
+                } ?: continue
+                checkConditionExpression(offset, expression, tree, context, reporter)
+            }
+        }
+    }
+
+    private fun checkConditionExpression(
+        offset: Int,
+        expression: LighterASTNode,
+        tree: FlyweightCapableTreeStructure<LighterASTNode>,
+        context: CheckerContext,
+        reporter: DiagnosticReporter
+    ) {
+        val shouldReport = when (expression.tokenType) {
+            IS_EXPRESSION -> true
+            BINARY_EXPRESSION -> {
+                val operationTokenName = expression.getChildren(tree).first { it.tokenType == OPERATION_REFERENCE }.toString()
+                val operationToken = operationTokenName.getOperationSymbol()
+                operationToken in prohibitedTokens
+            }
+            else -> false
+        }
+        if (shouldReport) {
+            val source = FirLightSourceElement(expression, offset + expression.startOffset, offset + expression.endOffset, tree)
+            reporter.reportOn(source, FirErrors.CONFUSING_BRANCH_CONDITION, context)
+        }
+    }
+
+    override fun checkPsi(
+        element: FirWhenExpression,
+        source: FirPsiSourceElement,
+        psi: PsiElement,
+        context: CheckerContext,
+        reporter: DiagnosticReporter
+    ) {
+        if (element.subject == null && element.subjectVariable == null) return
+        val whenExpression = psi as KtWhenExpression
+        if (whenExpression.subjectExpression == null && whenExpression.subjectVariable == null) return
+        for (entry in whenExpression.entries) {
+            for (condition in entry.conditions) {
+                checkCondition(condition, context, reporter)
+            }
+        }
+    }
+
+    private fun checkCondition(condition: KtWhenCondition, context: CheckerContext, reporter: DiagnosticReporter) {
+        when (condition) {
+            is KtWhenConditionWithExpression -> checkConditionExpression(condition.expression, context, reporter)
+            is KtWhenConditionInRange -> checkConditionExpression(condition.rangeExpression, context, reporter)
+        }
+    }
+
+    private fun checkConditionExpression(rawExpression: KtExpression?, context: CheckerContext, reporter: DiagnosticReporter) {
+        if (rawExpression == null) return
+        if (rawExpression is KtParenthesizedExpression) return
+        val shouldReport = when (val expression = KtPsiUtil.safeDeparenthesize(rawExpression)) {
+            is KtIsExpression -> true
+            is KtBinaryExpression -> expression.operationToken in prohibitedTokens
+            else -> false
+        }
+        if (shouldReport) {
+            val source = FirRealPsiSourceElement(rawExpression)
+            reporter.reportOn(source, FirErrors.CONFUSING_BRANCH_CONDITION, context)
+        }
+    }
+}

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/diagnostics/FirDefaultErrorMessages.kt

@@ -107,6 +107,7 @@ import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONFLICTING_OVERL
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONFLICTING_PROJECTION
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONFLICTING_PROJECTION_IN_TYPEALIAS_EXPANSION
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONFLICTING_UPPER_BOUNDS
+import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONFUSING_BRANCH_CONDITION
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONSTRUCTOR_IN_INTERFACE
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONSTRUCTOR_IN_OBJECT
 import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors.CONST_VAL_NOT_TOP_LEVEL_OR_OBJECT
@@ -1520,6 +1521,10 @@ class FirDefaultErrorMessages {
             )
             map.put(COMMA_IN_WHEN_CONDITION_WITHOUT_ARGUMENT, "Deprecated syntax. Use '||' instead of commas in when-condition for 'when' without argument")
             map.put(DUPLICATE_LABEL_IN_WHEN, "Duplicate label in when")
+            map.put(
+                CONFUSING_BRANCH_CONDITION,
+                "The logical expressions may be understood ambiguously in when with subject branches. Please wrap it with parenthesis"
+            )
 
             // Context tracking
             map.put(TYPE_PARAMETER_IS_NOT_AN_EXPRESSION, "Type parameter ''{0}'' is not an expression", SYMBOL)

compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/BaseConverter.kt

@@ -10,6 +10,7 @@ import com.intellij.openapi.util.Ref
 import com.intellij.psi.TokenType
 import com.intellij.psi.tree.IElementType
 import com.intellij.util.diff.FlyweightCapableTreeStructure
+import org.jetbrains.kotlin.ElementTypeUtils.isExpression
 import org.jetbrains.kotlin.KtNodeTypes
 import org.jetbrains.kotlin.fir.*
 import org.jetbrains.kotlin.fir.builder.BaseFirBuilder

compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/ConverterUtil.kt

@@ -6,8 +6,6 @@
 package org.jetbrains.kotlin.fir.lightTree.converter
 
 import com.intellij.lang.LighterASTNode
-import com.intellij.psi.tree.IElementType
-import org.jetbrains.kotlin.KtNodeType
 import org.jetbrains.kotlin.KtNodeTypes.*
 import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.descriptors.Visibilities
@@ -20,25 +18,16 @@ import org.jetbrains.kotlin.fir.declarations.builder.buildProperty
 import org.jetbrains.kotlin.fir.declarations.impl.FirDeclarationStatusImpl
 import org.jetbrains.kotlin.fir.expressions.FirBlock
 import org.jetbrains.kotlin.fir.expressions.FirExpression
-import org.jetbrains.kotlin.fir.expressions.builder.*
+import org.jetbrains.kotlin.fir.expressions.builder.FirCallBuilder
+import org.jetbrains.kotlin.fir.expressions.builder.buildArgumentList
+import org.jetbrains.kotlin.fir.expressions.builder.buildBlock
+import org.jetbrains.kotlin.fir.expressions.builder.buildComponentCall
 import org.jetbrains.kotlin.fir.fakeElement
 import org.jetbrains.kotlin.fir.lightTree.fir.DestructuringDeclaration
 import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
-import org.jetbrains.kotlin.lexer.KtSingleValueToken
-import org.jetbrains.kotlin.lexer.KtTokens
 import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.name.SpecialNames
-import org.jetbrains.kotlin.parsing.KotlinExpressionParsing
 import org.jetbrains.kotlin.psi.KtPsiUtil
-import org.jetbrains.kotlin.psi.stubs.elements.KtConstantExpressionElementType
-import org.jetbrains.kotlin.psi.stubs.elements.KtStringTemplateExpressionElementType
-
-private val expressionSet = listOf(
-    REFERENCE_EXPRESSION,
-    DOT_QUALIFIED_EXPRESSION,
-    LAMBDA_EXPRESSION,
-    FUN
-)
 
 fun String?.nameAsSafeName(defaultName: String = ""): Name {
     return when {
@@ -48,28 +37,10 @@ fun String?.nameAsSafeName(defaultName: String = ""): Name {
     }
 }
 
-fun String.getOperationSymbol(): IElementType {
-    KotlinExpressionParsing.ALL_OPERATIONS.types.forEach {
-        if (it is KtSingleValueToken && it.value == this) return it
-    }
-    if (this == "as?") return KtTokens.AS_SAFE
-    return KtTokens.IDENTIFIER
-}
-
 fun LighterASTNode.getAsStringWithoutBacktick(): String {
     return this.toString().replace("`", "")
 }
 
-fun LighterASTNode.isExpression(): Boolean {
-    return when (this.tokenType) {
-        is KtNodeType,
-        is KtConstantExpressionElementType,
-        is KtStringTemplateExpressionElementType,
-        in expressionSet -> true
-        else -> false
-    }
-}
-
 fun <T : FirCallBuilder> T.extractArgumentsFrom(container: List<FirExpression>): T {
     argumentList = buildArgumentList {
         arguments += container

compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/DeclarationsConverter.kt

@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.fir.lightTree.converter
 import com.intellij.lang.LighterASTNode
 import com.intellij.psi.TokenType
 import com.intellij.util.diff.FlyweightCapableTreeStructure
+import org.jetbrains.kotlin.ElementTypeUtils.isExpression
 import org.jetbrains.kotlin.KtNodeTypes
 import org.jetbrains.kotlin.KtNodeTypes.*
 import org.jetbrains.kotlin.builtins.StandardNames

compiler/fir/raw-fir/light-tree2fir/src/org/jetbrains/kotlin/fir/lightTree/converter/ExpressionsConverter.kt

@@ -8,6 +8,8 @@ package org.jetbrains.kotlin.fir.lightTree.converter
 import com.intellij.lang.LighterASTNode
 import com.intellij.psi.TokenType
 import com.intellij.util.diff.FlyweightCapableTreeStructure
+import org.jetbrains.kotlin.ElementTypeUtils.getOperationSymbol
+import org.jetbrains.kotlin.ElementTypeUtils.isExpression
 import org.jetbrains.kotlin.KtNodeTypes.*
 import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.descriptors.Visibilities
@@ -837,7 +839,7 @@ class ExpressionsConverter(
                 it.tokenType == OPERATION_REFERENCE -> {
                     conditionSource = it.toFirSourceElement()
                 }
-                else -> if (it.isExpression()) firExpression = getAsFirExpression(it)
+                else -> if (it.isExpression()) firExpression = getAsFirExpression(it, "No range in condition with range")
             }
         }