-
-
Notifications
You must be signed in to change notification settings - Fork 38
Stability configuration files in compiler plugin #181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
mlewandowskiphysitrack
wants to merge
12
commits into
skydoves:main
Choose a base branch
from
mlewandowskiphysitrack:stability-configuration-files-in-compiler-plugin
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
1bd20fa
Copy StabilityConfigParser from stability-gradle to stability-compiler
mlewandowskiphysitrack 107e179
Introduce stabilityConfigurationFile option (multiple values allowed)…
mlewandowskiphysitrack ccf4a87
Parse stability configuration files in compiler plugin and use matche…
mlewandowskiphysitrack 94d6c05
Add new option for stability configuration files to gradle plugin and…
mlewandowskiphysitrack 45c3001
Deprecate stabilityConfigurationFiles option on StabilityValidationCo…
mlewandowskiphysitrack a2159db
Update docs: deprecate stabilityConfigurationFiles in stabilityValida…
mlewandowskiphysitrack 3ee3634
API dump
mlewandowskiphysitrack 3e6d7cd
Add compiler test for checking if stabilityConfigurationFile is taken…
mlewandowskiphysitrack 5b8d90a
Add negative tests for stability configuration files handling
mlewandowskiphysitrack f6df6d3
Fix missing space in deprecation message
mlewandowskiphysitrack e8091a7
Wrap stability config parsing in the same try/catch as the dependency…
mlewandowskiphysitrack fd97a5a
Make stability config parsing fail open
mlewandowskiphysitrack File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
59 changes: 59 additions & 0 deletions
59
compiler-tests/src/test/data/dump/ir/StabilityConfigurationFile.fir.kt.txt
This file contains hidden or 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,59 @@ | ||
| data class Wrapper { | ||
| val value: ToBeStable | ||
| field = value | ||
| get | ||
|
|
||
| constructor(value: ToBeStable) /* primary */ { | ||
| super/*Any*/() | ||
| /* <init>() */ | ||
|
|
||
| } | ||
|
|
||
| operator fun component1(): ToBeStable { | ||
| return <this>.#value | ||
| } | ||
|
|
||
| fun copy(value: ToBeStable = <this>.#value): Wrapper { | ||
| return Wrapper(value = value) | ||
| } | ||
|
|
||
| override operator fun equals(other: Any?): Boolean { | ||
| when { | ||
| EQEQEQ(arg0 = <this>, arg1 = other) -> return true | ||
| } | ||
| when { | ||
| other !is Wrapper -> return false | ||
| } | ||
| val tmp_0: Wrapper = other /*as Wrapper */ | ||
| when { | ||
| EQEQ(arg0 = <this>.#value, arg1 = tmp_0.#value).not() -> return false | ||
| } | ||
| return true | ||
| } | ||
|
|
||
| override fun hashCode(): Int { | ||
| return <this>.#value.hashCode() | ||
| } | ||
|
|
||
| override fun toString(): String { | ||
| return "Wrapper(" + "value=" + <this>.#value + ")" | ||
| } | ||
|
|
||
| } | ||
|
|
||
| interface ToBeStable { | ||
| } | ||
|
|
||
| @Composable | ||
| fun ComposableToBeSkippable(wrapper: Wrapper) { | ||
| val _tracker: RecompositionTracker = rememberRecompositionTracker(composableName = "ComposableToBeSkippable", tag = "", threshold = 2, fqName = "ComposableToBeSkippable", isAutoTraced = true) | ||
| _tracker.trackParameter(name = "wrapper", type = "<root>.Wrapper", value = wrapper, isStable = true) | ||
| val _startTime: Long = nanoTime() | ||
| try // COMPOSITE { | ||
| println(message = wrapper.toString()) | ||
| // } | ||
| finally // COMPOSITE { | ||
| _tracker.recordDuration(startTimeNanos = _startTime) | ||
| _tracker.logIfThresholdMet() | ||
| // } | ||
| } |
17 changes: 17 additions & 0 deletions
17
compiler-tests/src/test/data/dump/ir/StabilityConfigurationFile.kt
This file contains hidden or 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,17 @@ | ||
| // DUMP_KT_IR | ||
| // ENABLE_TRACE_ALL | ||
| // STABILITY_CONFIGURATION_FILES: compiler-tests/src/test/data/dump/ir/stability_config.conf | ||
|
|
||
| import androidx.compose.runtime.Composable | ||
|
|
||
| interface ToBeStable | ||
|
|
||
| data class Wrapper(val value: ToBeStable) | ||
|
|
||
| // The following line in StabilityConfigurationFile.fir.kt.txt confirms the stability_config.conf was taken into account: | ||
| // _tracker.trackParameter(name = "wrapper", type = "<root>.Wrapper", value = wrapper, isStable = true) | ||
| // The test will fail if isStable = false (meaning ToBeStable was not marked as stable, hence Wrapper is also not stable). | ||
| @Composable | ||
| fun ComposableToBeSkippable(wrapper: Wrapper) { | ||
| println(wrapper.toString()) | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
This file contains hidden or 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 @@ | ||
| ToBeStable |
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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,24 @@ | ||
| # Stability configuration files | ||
|
|
||
| You can provide stability configuration files to tell compiler plugin which types should be treated as stable, even if the compiler marks them as unstable. This uses the same format as the [Compose compiler's stability configuration file](https://developer.android.com/develop/ui/compose/performance/stability/fix#configuration-file), so you can reuse the same file for both the compiler and stability validation. | ||
|
|
||
| ```kotlin | ||
| composeStabilityAnalyzer { | ||
| stabilityConfigurationFiles.add( | ||
| rootProject.layout.projectDirectory.file("stability_config.conf") | ||
| ) | ||
| } | ||
| ``` | ||
|
|
||
| The configuration file contains fully-qualified type names, one per line. Lines starting with `//` are treated as comments. Wildcard patterns are supported: `*` matches a single package segment and `**` matches across package boundaries. | ||
|
|
||
| ``` | ||
| // stability_config.conf | ||
| com.google.firebase.auth.FirebaseUser | ||
| com.example.generated.* | ||
| com.example.models.** | ||
| ``` | ||
|
|
||
| When these files are configured, the compiler plugin treats any parameter type matching these patterns as stable. | ||
|
|
||
| This is particularly useful when your project already uses a stability configuration file for the Compose compiler. By pointing `stabilityConfigurationFiles` to the same file, the stability validation respects the same overrides, keeping the two in sync. |
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ import org.jetbrains.kotlin.compiler.plugin.CommandLineProcessor | |
| import org.jetbrains.kotlin.compiler.plugin.ExperimentalCompilerApi | ||
| import org.jetbrains.kotlin.config.CompilerConfiguration | ||
| import org.jetbrains.kotlin.config.CompilerConfigurationKey | ||
| import java.io.File | ||
|
|
||
| public object StabilityAnalyzerConfigurationKeys { | ||
| public val KEY_ENABLED: CompilerConfigurationKey<Boolean> = | ||
|
|
@@ -37,6 +38,9 @@ public object StabilityAnalyzerConfigurationKeys { | |
|
|
||
| public val KEY_TRACE_ALL_THRESHOLD: CompilerConfigurationKey<Int> = | ||
| CompilerConfigurationKey<Int>("traceAllThreshold") | ||
|
|
||
| public val KEY_STABILITY_CONFIGURATION_FILES: CompilerConfigurationKey<List<File>> = | ||
| CompilerConfigurationKey<List<File>>("stabilityConfigurationFiles") | ||
| } | ||
|
|
||
| @OptIn(ExperimentalCompilerApi::class) | ||
|
|
@@ -79,6 +83,14 @@ public class StabilityAnalyzerCommandLineProcessor : CommandLineProcessor { | |
| description = "Recomposition count threshold for auto-traced composables", | ||
| required = false, | ||
| ) | ||
|
|
||
| public val OPTION_STABILITY_CONFIGURATION_FILE: CliOption = CliOption( | ||
| optionName = "stabilityConfigurationFile", | ||
| valueDescription = "<path>", | ||
| description = "Stability configuration file to take into account when analyzing stability", | ||
| required = false, | ||
| allowMultipleOccurrences = true, | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed to pass multiple files |
||
| ) | ||
| } | ||
|
|
||
| override val pluginId: String = PLUGIN_ID | ||
|
|
@@ -89,6 +101,7 @@ public class StabilityAnalyzerCommandLineProcessor : CommandLineProcessor { | |
| OPTION_PROJECT_DEPENDENCIES, | ||
| OPTION_TRACE_ALL, | ||
| OPTION_TRACE_ALL_THRESHOLD, | ||
| OPTION_STABILITY_CONFIGURATION_FILE, | ||
| ) | ||
|
|
||
| override fun processOption( | ||
|
|
@@ -121,6 +134,11 @@ public class StabilityAnalyzerCommandLineProcessor : CommandLineProcessor { | |
| StabilityAnalyzerConfigurationKeys.KEY_TRACE_ALL_THRESHOLD, | ||
| value.toIntOrNull() ?: 2, | ||
| ) | ||
|
|
||
| OPTION_STABILITY_CONFIGURATION_FILE -> configuration.appendList( | ||
| StabilityAnalyzerConfigurationKeys.KEY_STABILITY_CONFIGURATION_FILES, | ||
| File(value), | ||
| ) | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or 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 hidden or 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
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.