-
Notifications
You must be signed in to change notification settings - Fork 4
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(frontend-compiler-plugin): check whether using a `YieldingContex…
…t` when not necessary (#698)
- Loading branch information
Showing
9 changed files
with
368 additions
and
6 deletions.
There are no files selected for viewing
95 changes: 95 additions & 0 deletions
95
compiler-plugin-test/src/test/kotlin/it/unibo/collektive/test/UnnecessaryYieldingSpec.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| /* | ||
| * Copyright (c) 2025, Danilo Pianini, Nicolas Farabegoli, Elisa Tronetti, | ||
| * and all authors listed in the `build.gradle.kts` and the generated `pom.xml` file. | ||
| * | ||
| * This file is part of Collektive, and is distributed under the terms of the Apache License 2.0, | ||
| * as described in the LICENSE file in this project's repository's top directory. | ||
| */ | ||
|
|
||
| package it.unibo.collektive.test | ||
|
|
||
| import io.github.subjekt.Subjekt.subjekt | ||
| import io.github.subjekt.generators.FilesGenerator.toTempFiles | ||
| import io.kotest.core.spec.style.FreeSpec | ||
| import io.kotest.data.forAll | ||
| import io.kotest.data.headers | ||
| import io.kotest.data.row | ||
| import io.kotest.data.table | ||
| import it.unibo.collektive.test.util.CompileUtils.getTestingProgram | ||
| import it.unibo.collektive.test.util.CompileUtils.noWarning | ||
| import it.unibo.collektive.test.util.CompileUtils.warning | ||
|
|
||
| class UnnecessaryYieldingSpec : FreeSpec({ | ||
|
|
||
| fun expectedWarning(construct: String): String = | ||
| """ | ||
| The yielding block inside the '$construct' construct may not be necessary for this use case, as the | ||
| expression that is exchanged is the same as the one yielded inside the 'yielding' block. | ||
| Consider switching to the same construct without the 'yielding' block. | ||
| """.trimIndent() | ||
|
|
||
| "When using a construct with yielding" - { | ||
| val testSubjects = | ||
| subjekt { | ||
| addSource("src/test/resources/subjekt/UnnecessaryYieldingContext.yaml") | ||
| }.toTempFiles() | ||
|
|
||
| val constructs = | ||
| table( | ||
| headers("construct"), | ||
| row("evolving"), | ||
| row("exchanging"), | ||
| row("sharing"), | ||
| ) | ||
|
|
||
| forAll(constructs) { construct -> | ||
|
|
||
| "performing a data exchange inside the '$construct' construct and yielding the same value" - { | ||
|
|
||
| "with a simple expression" - { | ||
| val subjectName = "UnnecessaryYielding${construct.replaceFirstChar(Char::uppercase)}Simple" | ||
| val code = testSubjects.getTestingProgram(subjectName) | ||
|
|
||
| "should compile producing a warning" - { | ||
| code shouldCompileWith | ||
| warning( | ||
| expectedWarning(construct), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| "with a complex expression" - { | ||
| val subjectName = "UnnecessaryYielding${construct.replaceFirstChar(Char::uppercase)}Complex" | ||
| val code = testSubjects.getTestingProgram(subjectName) | ||
|
|
||
| "should compile producing a warning" - { | ||
| code shouldCompileWith | ||
| warning( | ||
| expectedWarning(construct), | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| "performing a data exchange inside the '$construct' construct and yielding a different value" - { | ||
| val subjectName = "NecessaryYielding${construct.replaceFirstChar(Char::uppercase)}" | ||
| val code = testSubjects.getTestingProgram(subjectName) | ||
|
|
||
| "should compile without warnings" - { | ||
| code shouldCompileWith noWarning | ||
| } | ||
| } | ||
|
|
||
| "performing a data exchange inside the '$construct' construct and yielding a different value with " + | ||
| "a different type" - { | ||
| val subjectName = "NecessaryYieldingDifferentType${construct.replaceFirstChar(Char::uppercase)}" | ||
| val code = testSubjects.getTestingProgram(subjectName) | ||
|
|
||
| "should compile without warnings" - { | ||
| code shouldCompileWith noWarning | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
102 changes: 102 additions & 0 deletions
102
compiler-plugin-test/src/test/resources/subjekt/UnnecessaryYieldingContext.yaml
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| --- | ||
| name: UnnecessaryYieldingContext | ||
| config: | ||
| codePreamble: |- | ||
| /** | ||
| * This file has been auto-generated with Subjekt (https://github.com/FreshMag/subjekt). | ||
| */ | ||
| import it.unibo.collektive.aggregate.api.Aggregate | ||
| import it.unibo.collektive.field.operations.max | ||
| import it.unibo.collektive.field.operations.maxBy | ||
| import it.unibo.collektive.aggregate.api.operators.neighboringViaExchange | ||
| import it.unibo.collektive.aggregate.api.operators.share | ||
| import it.unibo.collektive.aggregate.api.operators.sharing | ||
| import kotlin.math.floor | ||
| parameters: | ||
| - name: UNNECESSARY_YIELDING_SIMPLE | ||
| values: | ||
| - |- | ||
| evolving(initial) { | ||
| it.yielding { it } | ||
| } | ||
| - |- | ||
| exchanging(initial) { | ||
| it.yielding { it } | ||
| } | ||
| - |- | ||
| sharing(initial) { | ||
| it.max(initial).yielding { it.max(initial) } | ||
| } | ||
| - name: UNNECESSARY_YIELDING_COMPLEX | ||
| values: | ||
| - |- | ||
| evolving(initial) { | ||
| val value = it + 1 | ||
| floor(value.toDouble()).toInt().yielding { floor(value.toDouble()).toInt() } | ||
| } | ||
| - |- | ||
| exchanging(initial) { | ||
| val field = it.map { it / 2.toDouble() } | ||
| field.map(::floor).map { it.toInt() }.yielding { field.map(::floor).map { it.toInt() } } | ||
| } | ||
| - |- | ||
| sharing(initial) { | ||
| val maxValue = it.maxBy(0) { it } | ||
| floor(maxValue.toDouble() / 2).toInt().yielding { floor(maxValue.toDouble() / 2).toInt() } | ||
| } | ||
| - name: NECESSARY_YIELDING | ||
| values: | ||
| - |- | ||
| evolving(initial) { | ||
| it.yielding { it + 1 } | ||
| } | ||
| - |- | ||
| exchanging(initial) { | ||
| it.yielding { it.map { it + 1 } } | ||
| } | ||
| - |- | ||
| sharing(initial) { | ||
| val maxValue = it.maxBy(0) { it } | ||
| maxValue.yielding { maxValue + 1 } | ||
| } | ||
| - name: NECESSARY_YIELDING_DIFFERENT_TYPE | ||
| values: | ||
| - |- | ||
| evolving(initial) { | ||
| it.yielding { "Test$it" } | ||
| } | ||
| - |- | ||
| exchanging(initial) { | ||
| it.yielding { it.map { "Test${it + 1}" } } | ||
| } | ||
| - |- | ||
| sharing(initial) { | ||
| val maxValue = it.maxBy(0) { it } | ||
| maxValue.yielding { "Test${maxValue + 1}" } | ||
| } | ||
| macros: | ||
| def: aggregateBlock(specificCode) | ||
| values: | ||
| - |- | ||
| fun Aggregate<Int>.entry() { | ||
| val initial = 0 | ||
| ${{ specificCode }} | ||
| } | ||
| subjects: | ||
| - name: UnnecessaryYielding${{ prettify(UNNECESSARY_YIELDING_SIMPLE) }}Simple | ||
| code: ${{ aggregateBlock(UNNECESSARY_YIELDING_SIMPLE) }} | ||
|
|
||
| - name: UnnecessaryYielding${{ prettify(UNNECESSARY_YIELDING_COMPLEX) }}Complex | ||
| code: ${{ aggregateBlock(UNNECESSARY_YIELDING_COMPLEX) }} | ||
|
|
||
| - name: NecessaryYielding${{ prettify(NECESSARY_YIELDING) }} | ||
| code: ${{ aggregateBlock(NECESSARY_YIELDING) }} | ||
|
|
||
| - name: NecessaryYieldingDifferentType${{ prettify(NECESSARY_YIELDING_DIFFERENT_TYPE) }} | ||
| code: ${{ aggregateBlock(NECESSARY_YIELDING_DIFFERENT_TYPE) }} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
75 changes: 75 additions & 0 deletions
75
compiler-plugin/src/main/kotlin/it/unibo/collektive/frontend/checkers/UnnecessaryYielding.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| /* | ||
| * Copyright (c) 2025, Danilo Pianini, Nicolas Farabegoli, Elisa Tronetti, | ||
| * and all authors listed in the `build.gradle.kts` and the generated `pom.xml` file. | ||
| * | ||
| * This file is part of Collektive, and is distributed under the terms of the Apache License 2.0, | ||
| * as described in the LICENSE file in this project's repository's top directory. | ||
| */ | ||
|
|
||
| package it.unibo.collektive.frontend.checkers | ||
|
|
||
| import it.unibo.collektive.frontend.checkers.CheckersUtility.fqName | ||
| import it.unibo.collektive.frontend.checkers.CheckersUtility.functionName | ||
| import it.unibo.collektive.frontend.visitors.YieldingUnnecessaryUsageVisitor | ||
| import it.unibo.collektive.utils.common.AggregateFunctionNames.EVOLVING_FUNCTION_FQ_NAME | ||
| import it.unibo.collektive.utils.common.AggregateFunctionNames.EXCHANGING_FUNCTION_FQ_NAME | ||
| import it.unibo.collektive.utils.common.AggregateFunctionNames.SHARING_FUNCTION_FQ_NAME | ||
| import org.jetbrains.kotlin.diagnostics.DiagnosticReporter | ||
| import org.jetbrains.kotlin.diagnostics.reportOn | ||
| import org.jetbrains.kotlin.fir.analysis.checkers.MppCheckerKind | ||
| import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext | ||
| import org.jetbrains.kotlin.fir.analysis.checkers.expression.FirFunctionCallChecker | ||
| import org.jetbrains.kotlin.fir.expressions.FirFunctionCall | ||
|
|
||
| /** | ||
| * Checker for unnecessary yielding contexts. | ||
| * | ||
| * This checker is responsible for finding constructs like `evolving`, `exchanging` and `sharing` that are called with | ||
| * a yielded expression that is the same as the one that is exchanged, resulting in an unnecessary yielding context. | ||
| * | ||
| * For example: | ||
| * | ||
| * ```kotlin | ||
| * sharing(initial) { | ||
| * // ... | ||
| * value.yielding { value } | ||
| * } | ||
| * ``` | ||
| * | ||
| * Should generate a warning indicating to switch to the `share` construct, as in the following: | ||
| * | ||
| * ```kotlin | ||
| * share(initial) { | ||
| * // ... | ||
| * value | ||
| * } | ||
| * ``` | ||
| */ | ||
| object UnnecessaryYielding : FirFunctionCallChecker(MppCheckerKind.Common) { | ||
| private val constructs = | ||
| listOf( | ||
| EVOLVING_FUNCTION_FQ_NAME, | ||
| EXCHANGING_FUNCTION_FQ_NAME, | ||
| SHARING_FUNCTION_FQ_NAME, | ||
| ) | ||
|
|
||
| private fun FirFunctionCall.usesAnUnnecessaryYieldingContext(): Boolean = | ||
| with(YieldingUnnecessaryUsageVisitor()) { | ||
| containsUnnecessaryYielding() | ||
| } | ||
|
|
||
| override fun check( | ||
| expression: FirFunctionCall, | ||
| context: CheckerContext, | ||
| reporter: DiagnosticReporter, | ||
| ) { | ||
| if (expression.fqName() in constructs && expression.usesAnUnnecessaryYieldingContext()) { | ||
| reporter.reportOn( | ||
| expression.calleeReference.source, | ||
| FirCollektiveErrors.UNNECESSARY_YIELDING_CONTEXT, | ||
| expression.functionName(), | ||
| context, | ||
| ) | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.