From c53ae2c2329b3a451a33ce462dcb79afda87a498 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 14 Sep 2023 09:34:54 -0400 Subject: [PATCH 01/26] Add Annotation and gutter icon to feature flags --- gradle/libs.versions.toml | 1 + skate-plugin/build.gradle.kts | 1 + .../com/slack/sgp/intellij/Constants.kt | 22 +++ .../slack/sgp/intellij/SkatePluginSettings.kt | 14 ++ .../featureflags/FeatureFlagExtractor.kt | 108 +++++++++++ .../featureflags/featureFlagAnnotator.kt | 177 ++++++++++++++++++ .../slack/sgp/intellij/util/ProjectUtils.kt | 24 +++ .../src/main/resources/META-INF/plugin.xml | 1 + 8 files changed, 348 insertions(+) create mode 100644 skate-plugin/src/main/kotlin/com/slack/sgp/intellij/Constants.kt create mode 100644 skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt create mode 100644 skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt create mode 100644 skate-plugin/src/main/kotlin/com/slack/sgp/intellij/util/ProjectUtils.kt diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 0112c596a..4d60f141e 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -78,6 +78,7 @@ gradlePlugins-wire = { module = "com.squareup.wire:wire-gradle-plugin", version. guava = "com.google.guava:guava:32.1.2-jre" kotlinCliUtil = "com.slack.cli:kotlin-cli-util:2.2.1" kotlin-bom = { module = "org.jetbrains.kotlin:kotlin-bom", version.ref = "kotlin" } +kotlin-compilerEmbeddable = { module = "org.jetbrains.kotlin:kotlin-compiler-embeddable", version.ref = "kotlin" } kotlin-reflect = { module = "org.jetbrains.kotlin:kotlin-reflect", version.ref = "kotlin" } ktfmt = { module = "com.facebook:ktfmt", version.ref = "ktfmt" } jgrapht = "org.jgrapht:jgrapht-core:1.5.2" diff --git a/skate-plugin/build.gradle.kts b/skate-plugin/build.gradle.kts index 0d6d1c33d..5587ada82 100644 --- a/skate-plugin/build.gradle.kts +++ b/skate-plugin/build.gradle.kts @@ -85,6 +85,7 @@ tasks // endregion dependencies { + implementation(libs.kotlin.compilerEmbeddable) implementation(libs.bugsnag) { exclude(group = "org.slf4j") } testImplementation(libs.junit) testImplementation(libs.truth) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/Constants.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/Constants.kt new file mode 100644 index 000000000..30916a5ea --- /dev/null +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/Constants.kt @@ -0,0 +1,22 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.sgp.intellij + +import com.intellij.openapi.util.Key + +// Note this is a hack to allow tests to not use KotlinLanguage class due to classloading problems +// with IJ and detekt. +val TEST_KOTLIN_LANGUAGE_ID_KEY = Key.create("__tests_only_kotlin_id__") diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt index 0e91de915..2f684d2d8 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt @@ -38,8 +38,22 @@ class SkatePluginSettings : SimplePersistentStateComponent>() + } + + fun setFeatureFlagsForPsiFile(psiFile: com.intellij.psi.PsiFile, flags: List) { + featureFlagCache[psiFile] = flags + } + + fun getFeatureFlagsForPsiFile(psiFile: com.intellij.psi.PsiFile): List? { + return featureFlagCache[psiFile] + } + /** + * Extracts the names of feature flags from the provided PSI file. Only processes Kotlin files. + * + * @param psiFile The PSI representation of the file to process. + * @return A list of feature flag names in a file + */ + fun extractFeatureFlags(psiFile: com.intellij.psi.PsiFile): List { + if (!isKtFile(psiFile)) { + LOG.info("$psiFile is not a KtFile") + return emptyList() + } + LOG.info("Getting Enum Entries") + val enumsWithAnnotation = findAnnotatedEnums(psiFile) + LOG.info("Enums with Annotations: $enumsWithAnnotation") + return enumsWithAnnotation + } + + fun isKtFile(obj: Any): Boolean { + return obj.javaClass.getName() == "org.jetbrains.kotlin.psi.KtFile" + } + + fun findAnnotatedEnums(psiFile: Any): List { + val annotatedEnumEntries = mutableListOf() + + fun addIfAnnotatedEnum(element: Any) { + if (isKtEnumEntry(element) && hasFeatureFlagAnnotation(element)) { + element.javaClass + .getMethod("getName") + .invoke(element) + .takeIf { it is String } + ?.let { annotatedEnumEntries.add(it as String) } + } + } + + fun recurse(element: Any) { + addIfAnnotatedEnum(element) + // Traverse children + element.javaClass.methods + .find { it.name == "getChildren" } + ?.let { method -> (method.invoke(element) as? Array<*>)?.forEach { recurse(it!!) } } + } + recurse(psiFile) + return annotatedEnumEntries + } + + /** + * Checks if a given enum entry is a feature flag. An enum is considered a feature flag if it's + * within a Kotlin class and has an annotation named "FeatureFlag". + * + * @param element The enum entry to check. + * @return true if the enum entry is a feature flag, false otherwise. + */ + fun hasFeatureFlagAnnotation(element: Any): Boolean { + val annotationEntriesMethod = element.javaClass.getMethod("getAnnotationEntries") + val annotationEntries = annotationEntriesMethod.invoke(element) as? List<*> + return annotationEntries?.any { + val shortNameMethod = it!!.javaClass.getMethod("getShortName") + val shortName = shortNameMethod.invoke(it) + shortName?.toString() == "FeatureFlag" + } + ?: false + } + + fun isKtEnumEntry(element: Any): Boolean { + LOG.info("Checking if element is KtEnumEntry") + val result = element.javaClass.name == "org.jetbrains.kotlin.psi.KtEnumEntry" + LOG.info("Element isKtEnumEntry: $result") + return result + } +} diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt new file mode 100644 index 000000000..346cbbfa4 --- /dev/null +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt @@ -0,0 +1,177 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.sgp.intellij.featureflags + +import com.intellij.codeInsight.intention.IntentionAction +import com.intellij.icons.AllIcons +import com.intellij.ide.BrowserUtil +import com.intellij.lang.annotation.AnnotationHolder +import com.intellij.lang.annotation.ExternalAnnotator +import com.intellij.lang.annotation.HighlightSeverity +import com.intellij.openapi.actionSystem.AnAction +import com.intellij.openapi.actionSystem.AnActionEvent +import com.intellij.openapi.components.service +import com.intellij.openapi.diagnostic.Logger +import com.intellij.openapi.editor.Editor +import com.intellij.openapi.editor.markup.GutterIconRenderer +import com.intellij.openapi.project.Project +import com.intellij.openapi.util.TextRange +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiFile +import com.intellij.psi.util.PsiTreeUtil +import com.intellij.util.IconUtil +import com.slack.sgp.intellij.SkatePluginSettings +import com.slack.sgp.intellij.TEST_KOTLIN_LANGUAGE_ID_KEY +import com.slack.sgp.intellij.util.isLinkifiedFeatureFlagsEnabled +import java.net.URI +import javax.swing.Icon +import org.jetbrains.kotlin.idea.KotlinLanguage + +class FeatureFlagAnnotator : ExternalAnnotator>() { + private val LOG: Logger = Logger.getInstance(FeatureFlagAnnotator::class.java) + private val extractor = FeatureFlagExtractor() + + override fun collectInformation(file: PsiFile): PsiFile? { + if (!isKotlinFeatureFile(file)) { + LOG.info("Skipping non-Feature Kotlin file: $file") + return null + } + LOG.info("Getting CollectedInformation for file: $file") + if (extractor.getFeatureFlagsForPsiFile(file) == null) { + LOG.info("Extracting featureFlags") + val flags = extractor.extractFeatureFlags(file) + LOG.info("FeatureFlags found: $flags") + extractor.setFeatureFlagsForPsiFile(file, flags) + } + return file + } + + override fun doAnnotate(collectedInfo: PsiFile): List { + LOG.info("Starting Annotation") + if (!collectedInfo.project.isLinkifiedFeatureFlagsEnabled() || !isKotlinFile(collectedInfo)) { + LOG.info("This is not a kotline file or FF not enabled for file: $collectedInfo") + return emptyList() + } + + val flagsForFile = extractor.getFeatureFlagsForPsiFile(collectedInfo) + LOG.info("Transform Feature Flag method called") + return transformToFeatureFlagSymbols(collectedInfo, flagsForFile) + } + + private fun transformToFeatureFlagSymbols( + psiFile: PsiFile, + flags: List? + ): List { + LOG.info("Transforming feature flags: $flags") + if (flags == null || flags.isEmpty()) { + LOG.info("No flags to transform. Flags, returning empty list") + return emptyList() + } + + val settings = psiFile.project.service() + val baseUrl = settings.featureFlagBaseUrl.orEmpty() + LOG.info("BaseURL is : $baseUrl") + return flags.mapNotNull { flag -> + val textRange = findTextRangeForFlag(psiFile, flag) + if (textRange != null) { + val url = "$baseUrl?q=$flag" + FeatureFlagSymbol(textRange, url) + } else { + null + } + } + } + + private fun findTextRangeForFlag(psiFile: PsiFile, flag: String): TextRange? { + LOG.info("Finding text range for a flag: $flag") + // Use Pita's approach of traversing the PSI structure to find the text range + val elements = PsiTreeUtil.findChildrenOfType(psiFile, PsiElement::class.java) + for (element in elements) { + if (element.text == flag) { + LOG.info("Text range found: ${element.textRange}") + return element.textRange + } + } + return null + } + + private fun isKotlinFile(psiFile: PsiFile): Boolean = + psiFile.language.id == KotlinLanguage.INSTANCE.id || + psiFile.getUserData(TEST_KOTLIN_LANGUAGE_ID_KEY) == KotlinLanguage.INSTANCE.id + + private fun isKotlinFeatureFile(psiFile: PsiFile): Boolean = psiFile.name.endsWith("Feature.kt") + + override fun apply( + file: PsiFile, + annotationResult: List, + holder: AnnotationHolder + ) { + for (symbol in annotationResult) { + LOG.info("Applying Annotation for a symbol: $symbol") + val textRange = symbol.textRange + val message = "Open on Houston: ${symbol.url}" + holder + .newAnnotation( + HighlightSeverity.INFORMATION, + "More details about feature flag on Houston..." + ) + .range(textRange) + .needsUpdateOnTyping(true) + .withFix(UrlIntentionAction(message, symbol.url)) + .gutterIconRenderer(MyGutterIconRenderer(symbol.url)) + .create() + } + } +} + +class FeatureFlagSymbol(val textRange: TextRange, val url: String) + +class UrlIntentionAction( + private val message: String, + private val url: String, +) : IntentionAction { + override fun getText(): String = message + + override fun getFamilyName(): String = text + + override fun isAvailable(project: Project, editor: Editor?, file: PsiFile?) = true + + override fun invoke(project: Project, editor: Editor?, file: PsiFile?) { + BrowserUtil.browse(URI(url)) + } + + override fun startInWriteAction(): Boolean { + return false + } +} + +class MyGutterIconRenderer(private val url: String) : GutterIconRenderer() { + + private val scaledWebIcon: Icon = IconUtil.scale(AllIcons.General.Web, null, 0.5f) + + override fun getIcon() = scaledWebIcon + + override fun getClickAction() = + object : AnAction() { + override fun actionPerformed(e: AnActionEvent) { + BrowserUtil.browse(url) + } + } + + override fun equals(other: Any?) = other is MyGutterIconRenderer && other.url == url + + override fun hashCode() = url.hashCode() +} diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/util/ProjectUtils.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/util/ProjectUtils.kt new file mode 100644 index 000000000..8615d034c --- /dev/null +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/util/ProjectUtils.kt @@ -0,0 +1,24 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.sgp.intellij.util + +import com.intellij.openapi.components.service +import com.intellij.openapi.project.Project +import com.slack.sgp.intellij.SkatePluginSettings + +fun Project.settings(): SkatePluginSettings = service() + +fun Project.isLinkifiedFeatureFlagsEnabled(): Boolean = settings().isLinkifiedFeatureFlagsEnabled diff --git a/skate-plugin/src/main/resources/META-INF/plugin.xml b/skate-plugin/src/main/resources/META-INF/plugin.xml index b6aa9819e..1a6a9edbd 100644 --- a/skate-plugin/src/main/resources/META-INF/plugin.xml +++ b/skate-plugin/src/main/resources/META-INF/plugin.xml @@ -30,6 +30,7 @@ serviceImplementation="com.slack.sgp.intellij.SkateProjectServiceImpl"/> + From 1b83c8fedd8fa1fb4707b91345fd8811b852cc1d Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Wed, 20 Sep 2023 13:38:42 -0400 Subject: [PATCH 02/26] Add tests --- skate-plugin/build.gradle.kts | 7 +- .../featureflags/FeatureFlagExtractor.kt | 4 - .../featureflags/FeatureFlagAnnotatortest.kt | 91 +++++++++++++++++++ 3 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatortest.kt diff --git a/skate-plugin/build.gradle.kts b/skate-plugin/build.gradle.kts index 5587ada82..b569869aa 100644 --- a/skate-plugin/build.gradle.kts +++ b/skate-plugin/build.gradle.kts @@ -17,7 +17,10 @@ repositories { mavenCentral() } // Configure Gradle IntelliJ Plugin // Read more: https://plugins.jetbrains.com/docs/intellij/tools-gradle-intellij-plugin.html -intellij { plugins.add("org.intellij.plugins.markdown") } +intellij { + plugins.add("org.intellij.plugins.markdown") + plugins.add("org.jetbrains.kotlin") +} fun isGitHash(hash: String): Boolean { if (hash.length != 40) { @@ -85,7 +88,7 @@ tasks // endregion dependencies { - implementation(libs.kotlin.compilerEmbeddable) + compileOnly(libs.kotlin.compilerEmbeddable) implementation(libs.bugsnag) { exclude(group = "org.slf4j") } testImplementation(libs.junit) testImplementation(libs.truth) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 3933ac04d..1d62e5260 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -43,10 +43,6 @@ class FeatureFlagExtractor { * @return A list of feature flag names in a file */ fun extractFeatureFlags(psiFile: com.intellij.psi.PsiFile): List { - if (!isKtFile(psiFile)) { - LOG.info("$psiFile is not a KtFile") - return emptyList() - } LOG.info("Getting Enum Entries") val enumsWithAnnotation = findAnnotatedEnums(psiFile) LOG.info("Enums with Annotations: $enumsWithAnnotation") diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatortest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatortest.kt new file mode 100644 index 000000000..fc372bb11 --- /dev/null +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatortest.kt @@ -0,0 +1,91 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.sgp.intellij.featureflags + +import com.google.common.truth.Truth.assertThat +import com.intellij.openapi.components.service +import com.intellij.psi.PsiFile +import com.intellij.testFramework.fixtures.LightPlatformCodeInsightFixture4TestCase +import com.slack.sgp.intellij.SkatePluginSettings +import org.junit.Test + +class FeatureFlagAnnotatortest : LightPlatformCodeInsightFixture4TestCase() { + + @Test + fun `test returns empty list when linkify is disabled`() { + assertThat(runAnnotator(enabled = false)).isEmpty() + } + + @Test + fun `test report symbols when linkify is enabled`() { + assertThat(runAnnotator(enabled = true)).isNotEmpty() + } + + @Test + fun `test extraction of feature flags from provided content`() { + val featureFlagExtractor = FeatureFlagExtractor() + val content = + """ + @FeatureFlags(ForComplianceFeature::class) + enum class ComplianceFeature(override val dependencies: Set = emptySet()) : + FeatureFlagEnum { + /** Enables validation of app-scoped environment variant */ + @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) ANDROID_ENV_VARIANT_VALIDATION, + @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) + ANDROID_GOV_SLACK_CUSTOM_AWARENESS, + @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH; + override val key: String by computeKey() + } + """ + + // Create a KtFile from the content + val psiFile = createKotlinFile("ComplianceFeature.kt", content) + val featureFlags = featureFlagExtractor.extractFeatureFlags(psiFile) + // Assertions + assertTrue(featureFlags.contains("ANDROID_ENV_VARIANT_VALIDATION")) + assertTrue(featureFlags.contains("ANDROID_GOV_SLACK_CUSTOM_AWARENESS")) + assertTrue(featureFlags.contains("ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH")) + } + + private fun runAnnotator(enabled: Boolean): List { + project.service().isLinkifiedFeatureFlagsEnabled = enabled + val content = + """ + @FeatureFlags(ForComplianceFeature::class) + enum class ComplianceFeature(override val dependencies: Set = emptySet()) : + FeatureFlagEnum { + /** Enables validation of app-scoped environment variant */ + @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) ANDROID_ENV_VARIANT_VALIDATION, + @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) + ANDROID_GOV_SLACK_CUSTOM_AWARENESS, + @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH; + override val key: String by computeKey() + } + """ + val file = createKotlinFile("ComplianceFeature.kt", content) + FeatureFlagAnnotator().collectInformation(file) + return FeatureFlagAnnotator().doAnnotate(file) + } + + private fun createKotlinFile( + name: String, + text: String, + ): PsiFile { + return myFixture.configureByText(name, text) + } +} From 99c4006f4e3ff5dc4650118a3a3ecc48551eae54 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 21 Sep 2023 13:49:38 -0400 Subject: [PATCH 03/26] Fix java.lang.NoClassDefFoundError --- skate-plugin/build.gradle.kts | 2 +- .../slack/sgp/intellij/featureflags/featureFlagAnnotator.kt | 5 ++++- skate-plugin/src/main/resources/META-INF/plugin.xml | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/skate-plugin/build.gradle.kts b/skate-plugin/build.gradle.kts index b569869aa..754994d04 100644 --- a/skate-plugin/build.gradle.kts +++ b/skate-plugin/build.gradle.kts @@ -88,8 +88,8 @@ tasks // endregion dependencies { - compileOnly(libs.kotlin.compilerEmbeddable) implementation(libs.bugsnag) { exclude(group = "org.slf4j") } + compileOnly(libs.kotlin.compilerEmbeddable) testImplementation(libs.junit) testImplementation(libs.truth) } diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt index 346cbbfa4..de52fafaf 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt @@ -23,6 +23,7 @@ import com.intellij.lang.annotation.ExternalAnnotator import com.intellij.lang.annotation.HighlightSeverity import com.intellij.openapi.actionSystem.AnAction import com.intellij.openapi.actionSystem.AnActionEvent +import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.components.service import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.editor.Editor @@ -85,7 +86,9 @@ class FeatureFlagAnnotator : ExternalAnnotator> val baseUrl = settings.featureFlagBaseUrl.orEmpty() LOG.info("BaseURL is : $baseUrl") return flags.mapNotNull { flag -> - val textRange = findTextRangeForFlag(psiFile, flag) + val textRange = ApplicationManager.getApplication().runReadAction { + findTextRangeForFlag(psiFile, flag) + } if (textRange != null) { val url = "$baseUrl?q=$flag" FeatureFlagSymbol(textRange, url) diff --git a/skate-plugin/src/main/resources/META-INF/plugin.xml b/skate-plugin/src/main/resources/META-INF/plugin.xml index 1a6a9edbd..eab295a82 100644 --- a/skate-plugin/src/main/resources/META-INF/plugin.xml +++ b/skate-plugin/src/main/resources/META-INF/plugin.xml @@ -21,6 +21,7 @@ Read more: https://plugins.jetbrains.com/docs/intellij/plugin-compatibility.html --> com.intellij.modules.platform org.intellij.plugins.markdown + org.jetbrains.kotlin From 1de3ef7f38db6ee9a02ae52587c1e59b24f4db85 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 21 Sep 2023 14:15:50 -0400 Subject: [PATCH 04/26] Run spotless --- .../sgp/intellij/featureflags/featureFlagAnnotator.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt index de52fafaf..db418d5a7 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt @@ -86,9 +86,10 @@ class FeatureFlagAnnotator : ExternalAnnotator> val baseUrl = settings.featureFlagBaseUrl.orEmpty() LOG.info("BaseURL is : $baseUrl") return flags.mapNotNull { flag -> - val textRange = ApplicationManager.getApplication().runReadAction { - findTextRangeForFlag(psiFile, flag) - } + val textRange = + ApplicationManager.getApplication().runReadAction { + findTextRangeForFlag(psiFile, flag) + } if (textRange != null) { val url = "$baseUrl?q=$flag" FeatureFlagSymbol(textRange, url) From 78d6ae649fe82257ab104ae93c4876031dd8e241 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 21 Sep 2023 14:33:55 -0400 Subject: [PATCH 05/26] fix url --- .../slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt | 4 ++-- .../slack/sgp/intellij/featureflags/featureFlagAnnotator.kt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 1d62e5260..826e37b68 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -84,7 +84,7 @@ class FeatureFlagExtractor { * @param element The enum entry to check. * @return true if the enum entry is a feature flag, false otherwise. */ - fun hasFeatureFlagAnnotation(element: Any): Boolean { + private fun hasFeatureFlagAnnotation(element: Any): Boolean { val annotationEntriesMethod = element.javaClass.getMethod("getAnnotationEntries") val annotationEntries = annotationEntriesMethod.invoke(element) as? List<*> return annotationEntries?.any { @@ -95,7 +95,7 @@ class FeatureFlagExtractor { ?: false } - fun isKtEnumEntry(element: Any): Boolean { + private fun isKtEnumEntry(element: Any): Boolean { LOG.info("Checking if element is KtEnumEntry") val result = element.javaClass.name == "org.jetbrains.kotlin.psi.KtEnumEntry" LOG.info("Element isKtEnumEntry: $result") diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt index db418d5a7..1763805bc 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt @@ -91,7 +91,7 @@ class FeatureFlagAnnotator : ExternalAnnotator> findTextRangeForFlag(psiFile, flag) } if (textRange != null) { - val url = "$baseUrl?q=$flag" + val url = "$baseUrl?q=${flag.lowercase()}" FeatureFlagSymbol(textRange, url) } else { null From 4c0937d302d8038dfead41fca00016d9ee8dd208 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 21 Sep 2023 16:46:16 -0400 Subject: [PATCH 06/26] Remove redundant logging --- .../featureflags/FeatureFlagExtractor.kt | 30 ++++++++---------- .../featureflags/featureFlagAnnotator.kt | 31 +++---------------- 2 files changed, 17 insertions(+), 44 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 826e37b68..1812beb71 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -18,13 +18,14 @@ package com.slack.sgp.intellij.featureflags import com.intellij.openapi.diagnostic.Logger /** - * Extracts feature flags from Kotlin files. It searches for enum entries annotated with - * 'FeatureFlag'. + * Responsible for extracting feature flags. Searches for enum entries annotated with 'FeatureFlag' + * to identify feature flags. */ class FeatureFlagExtractor { - private val LOG: Logger = Logger.getInstance(FeatureFlagExtractor::class.java) + private val log: Logger = Logger.getInstance(FeatureFlagExtractor::class.java) + // Caches the feature flags for a given PSI file to optimize repeated lookups companion object { private val featureFlagCache = mutableMapOf>() } @@ -43,17 +44,14 @@ class FeatureFlagExtractor { * @return A list of feature flag names in a file */ fun extractFeatureFlags(psiFile: com.intellij.psi.PsiFile): List { - LOG.info("Getting Enum Entries") + log.info("Looking for feature flags in a file: ${psiFile.toString()}") val enumsWithAnnotation = findAnnotatedEnums(psiFile) - LOG.info("Enums with Annotations: $enumsWithAnnotation") + log.info("Found feature flags: $enumsWithAnnotation") return enumsWithAnnotation } - fun isKtFile(obj: Any): Boolean { - return obj.javaClass.getName() == "org.jetbrains.kotlin.psi.KtFile" - } - - fun findAnnotatedEnums(psiFile: Any): List { + /** Recursively searches the given PSI file for enums with 'FeatureFlag' annotations. */ + private fun findAnnotatedEnums(psiFile: Any): List { val annotatedEnumEntries = mutableListOf() fun addIfAnnotatedEnum(element: Any) { @@ -68,7 +66,6 @@ class FeatureFlagExtractor { fun recurse(element: Any) { addIfAnnotatedEnum(element) - // Traverse children element.javaClass.methods .find { it.name == "getChildren" } ?.let { method -> (method.invoke(element) as? Array<*>)?.forEach { recurse(it!!) } } @@ -78,8 +75,7 @@ class FeatureFlagExtractor { } /** - * Checks if a given enum entry is a feature flag. An enum is considered a feature flag if it's - * within a Kotlin class and has an annotation named "FeatureFlag". + * Determines if a given PSI element is an enum entry annotated with 'FeatureFlag'. * * @param element The enum entry to check. * @return true if the enum entry is a feature flag, false otherwise. @@ -91,14 +87,14 @@ class FeatureFlagExtractor { val shortNameMethod = it!!.javaClass.getMethod("getShortName") val shortName = shortNameMethod.invoke(it) shortName?.toString() == "FeatureFlag" - } - ?: false + } ?: false } + /** Checks if the given PSI element is a Kotlin enum entry. */ private fun isKtEnumEntry(element: Any): Boolean { - LOG.info("Checking if element is KtEnumEntry") + log.info("Checking if element is a Kotlin Enum Entry") val result = element.javaClass.name == "org.jetbrains.kotlin.psi.KtEnumEntry" - LOG.info("Element isKtEnumEntry: $result") + log.info("Element is Kotlin Enum Entry: $result") return result } } diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt index 1763805bc..1c31fea61 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt @@ -25,7 +25,6 @@ import com.intellij.openapi.actionSystem.AnAction import com.intellij.openapi.actionSystem.AnActionEvent import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.components.service -import com.intellij.openapi.diagnostic.Logger import com.intellij.openapi.editor.Editor import com.intellij.openapi.editor.markup.GutterIconRenderer import com.intellij.openapi.project.Project @@ -42,33 +41,25 @@ import javax.swing.Icon import org.jetbrains.kotlin.idea.KotlinLanguage class FeatureFlagAnnotator : ExternalAnnotator>() { - private val LOG: Logger = Logger.getInstance(FeatureFlagAnnotator::class.java) private val extractor = FeatureFlagExtractor() override fun collectInformation(file: PsiFile): PsiFile? { if (!isKotlinFeatureFile(file)) { - LOG.info("Skipping non-Feature Kotlin file: $file") return null } - LOG.info("Getting CollectedInformation for file: $file") if (extractor.getFeatureFlagsForPsiFile(file) == null) { - LOG.info("Extracting featureFlags") val flags = extractor.extractFeatureFlags(file) - LOG.info("FeatureFlags found: $flags") extractor.setFeatureFlagsForPsiFile(file, flags) } return file } override fun doAnnotate(collectedInfo: PsiFile): List { - LOG.info("Starting Annotation") if (!collectedInfo.project.isLinkifiedFeatureFlagsEnabled() || !isKotlinFile(collectedInfo)) { - LOG.info("This is not a kotline file or FF not enabled for file: $collectedInfo") return emptyList() } val flagsForFile = extractor.getFeatureFlagsForPsiFile(collectedInfo) - LOG.info("Transform Feature Flag method called") return transformToFeatureFlagSymbols(collectedInfo, flagsForFile) } @@ -76,15 +67,11 @@ class FeatureFlagAnnotator : ExternalAnnotator> psiFile: PsiFile, flags: List? ): List { - LOG.info("Transforming feature flags: $flags") - if (flags == null || flags.isEmpty()) { - LOG.info("No flags to transform. Flags, returning empty list") + if (flags.isNullOrEmpty()) { return emptyList() } + val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() - val settings = psiFile.project.service() - val baseUrl = settings.featureFlagBaseUrl.orEmpty() - LOG.info("BaseURL is : $baseUrl") return flags.mapNotNull { flag -> val textRange = ApplicationManager.getApplication().runReadAction { @@ -100,16 +87,8 @@ class FeatureFlagAnnotator : ExternalAnnotator> } private fun findTextRangeForFlag(psiFile: PsiFile, flag: String): TextRange? { - LOG.info("Finding text range for a flag: $flag") - // Use Pita's approach of traversing the PSI structure to find the text range val elements = PsiTreeUtil.findChildrenOfType(psiFile, PsiElement::class.java) - for (element in elements) { - if (element.text == flag) { - LOG.info("Text range found: ${element.textRange}") - return element.textRange - } - } - return null + return elements.firstOrNull { it.text == flag }?.textRange } private fun isKotlinFile(psiFile: PsiFile): Boolean = @@ -124,15 +103,13 @@ class FeatureFlagAnnotator : ExternalAnnotator> holder: AnnotationHolder ) { for (symbol in annotationResult) { - LOG.info("Applying Annotation for a symbol: $symbol") - val textRange = symbol.textRange val message = "Open on Houston: ${symbol.url}" holder .newAnnotation( HighlightSeverity.INFORMATION, "More details about feature flag on Houston..." ) - .range(textRange) + .range(symbol.textRange) .needsUpdateOnTyping(true) .withFix(UrlIntentionAction(message, symbol.url)) .gutterIconRenderer(MyGutterIconRenderer(symbol.url)) From d7f4a0042f2a3c048c30f2f533701ef9dcd98b2d Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 21 Sep 2023 17:16:24 -0400 Subject: [PATCH 07/26] Test cleanup --- ...tortest.kt => FeatureFlagAnnotatorTest.kt} | 51 +++++++------------ 1 file changed, 17 insertions(+), 34 deletions(-) rename skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/{FeatureFlagAnnotatortest.kt => FeatureFlagAnnotatorTest.kt} (72%) diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatortest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt similarity index 72% rename from skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatortest.kt rename to skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt index fc372bb11..e248bbb82 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatortest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt @@ -22,7 +22,21 @@ import com.intellij.testFramework.fixtures.LightPlatformCodeInsightFixture4TestC import com.slack.sgp.intellij.SkatePluginSettings import org.junit.Test -class FeatureFlagAnnotatortest : LightPlatformCodeInsightFixture4TestCase() { +class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { + private val fileContent = + """ + @FeatureFlags(ForComplianceFeature::class) + enum class ComplianceFeature(override val dependencies: Set = emptySet()) : + FeatureFlagEnum { + /** Enables validation of app-scoped environment variant */ + @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) ANDROID_ENV_VARIANT_VALIDATION, + @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) + ANDROID_GOV_SLACK_CUSTOM_AWARENESS, + @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH; + override val key: String by computeKey() + } + """ @Test fun `test returns empty list when linkify is disabled`() { @@ -37,25 +51,8 @@ class FeatureFlagAnnotatortest : LightPlatformCodeInsightFixture4TestCase() { @Test fun `test extraction of feature flags from provided content`() { val featureFlagExtractor = FeatureFlagExtractor() - val content = - """ - @FeatureFlags(ForComplianceFeature::class) - enum class ComplianceFeature(override val dependencies: Set = emptySet()) : - FeatureFlagEnum { - /** Enables validation of app-scoped environment variant */ - @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) ANDROID_ENV_VARIANT_VALIDATION, - @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) - ANDROID_GOV_SLACK_CUSTOM_AWARENESS, - @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) - ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH; - override val key: String by computeKey() - } - """ - - // Create a KtFile from the content - val psiFile = createKotlinFile("ComplianceFeature.kt", content) + val psiFile = createKotlinFile("ComplianceFeature.kt", fileContent) val featureFlags = featureFlagExtractor.extractFeatureFlags(psiFile) - // Assertions assertTrue(featureFlags.contains("ANDROID_ENV_VARIANT_VALIDATION")) assertTrue(featureFlags.contains("ANDROID_GOV_SLACK_CUSTOM_AWARENESS")) assertTrue(featureFlags.contains("ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH")) @@ -63,21 +60,7 @@ class FeatureFlagAnnotatortest : LightPlatformCodeInsightFixture4TestCase() { private fun runAnnotator(enabled: Boolean): List { project.service().isLinkifiedFeatureFlagsEnabled = enabled - val content = - """ - @FeatureFlags(ForComplianceFeature::class) - enum class ComplianceFeature(override val dependencies: Set = emptySet()) : - FeatureFlagEnum { - /** Enables validation of app-scoped environment variant */ - @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) ANDROID_ENV_VARIANT_VALIDATION, - @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) - ANDROID_GOV_SLACK_CUSTOM_AWARENESS, - @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) - ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH; - override val key: String by computeKey() - } - """ - val file = createKotlinFile("ComplianceFeature.kt", content) + val file = createKotlinFile("ComplianceFeature.kt", fileContent) FeatureFlagAnnotator().collectInformation(file) return FeatureFlagAnnotator().doAnnotate(file) } From 38de1b709b7bac4cde025d7a37902f7d349c9ff4 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Fri, 22 Sep 2023 10:03:38 -0400 Subject: [PATCH 08/26] File rename and updating imports --- ...eatureFlagAnnotator.kt => FeatureFlagAnnotator.kt} | 0 .../sgp/intellij/featureflags/FeatureFlagExtractor.kt | 11 ++++++----- 2 files changed, 6 insertions(+), 5 deletions(-) rename skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/{featureFlagAnnotator.kt => FeatureFlagAnnotator.kt} (100%) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt similarity index 100% rename from skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/featureFlagAnnotator.kt rename to skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 1812beb71..633f7e24e 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -16,6 +16,7 @@ package com.slack.sgp.intellij.featureflags import com.intellij.openapi.diagnostic.Logger +import com.intellij.psi.PsiFile /** * Responsible for extracting feature flags. Searches for enum entries annotated with 'FeatureFlag' @@ -27,14 +28,14 @@ class FeatureFlagExtractor { // Caches the feature flags for a given PSI file to optimize repeated lookups companion object { - private val featureFlagCache = mutableMapOf>() + private val featureFlagCache = mutableMapOf>() } - fun setFeatureFlagsForPsiFile(psiFile: com.intellij.psi.PsiFile, flags: List) { + fun setFeatureFlagsForPsiFile(psiFile: PsiFile, flags: List) { featureFlagCache[psiFile] = flags } - fun getFeatureFlagsForPsiFile(psiFile: com.intellij.psi.PsiFile): List? { + fun getFeatureFlagsForPsiFile(psiFile: PsiFile): List? { return featureFlagCache[psiFile] } /** @@ -43,8 +44,8 @@ class FeatureFlagExtractor { * @param psiFile The PSI representation of the file to process. * @return A list of feature flag names in a file */ - fun extractFeatureFlags(psiFile: com.intellij.psi.PsiFile): List { - log.info("Looking for feature flags in a file: ${psiFile.toString()}") + fun extractFeatureFlags(psiFile: PsiFile): List { + log.info("Looking for feature flags in a file: $psiFile") val enumsWithAnnotation = findAnnotatedEnums(psiFile) log.info("Found feature flags: $enumsWithAnnotation") return enumsWithAnnotation From 784371932e076fc1fe70b972b1dc386e7119fea0 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Fri, 22 Sep 2023 11:54:11 -0400 Subject: [PATCH 09/26] Remove gutter icon, add lazy loading and add clear intent with top level runReadAction --- .../featureflags/FeatureFlagAnnotator.kt | 34 ++----------------- 1 file changed, 3 insertions(+), 31 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt index 1c31fea61..ab2ecd87c 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt @@ -16,32 +16,26 @@ package com.slack.sgp.intellij.featureflags import com.intellij.codeInsight.intention.IntentionAction -import com.intellij.icons.AllIcons import com.intellij.ide.BrowserUtil import com.intellij.lang.annotation.AnnotationHolder import com.intellij.lang.annotation.ExternalAnnotator import com.intellij.lang.annotation.HighlightSeverity -import com.intellij.openapi.actionSystem.AnAction -import com.intellij.openapi.actionSystem.AnActionEvent -import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.runReadAction import com.intellij.openapi.components.service import com.intellij.openapi.editor.Editor -import com.intellij.openapi.editor.markup.GutterIconRenderer import com.intellij.openapi.project.Project import com.intellij.openapi.util.TextRange import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile import com.intellij.psi.util.PsiTreeUtil -import com.intellij.util.IconUtil import com.slack.sgp.intellij.SkatePluginSettings import com.slack.sgp.intellij.TEST_KOTLIN_LANGUAGE_ID_KEY import com.slack.sgp.intellij.util.isLinkifiedFeatureFlagsEnabled import java.net.URI -import javax.swing.Icon import org.jetbrains.kotlin.idea.KotlinLanguage class FeatureFlagAnnotator : ExternalAnnotator>() { - private val extractor = FeatureFlagExtractor() + private val extractor by lazy { FeatureFlagExtractor() } override fun collectInformation(file: PsiFile): PsiFile? { if (!isKotlinFeatureFile(file)) { @@ -73,10 +67,7 @@ class FeatureFlagAnnotator : ExternalAnnotator> val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() return flags.mapNotNull { flag -> - val textRange = - ApplicationManager.getApplication().runReadAction { - findTextRangeForFlag(psiFile, flag) - } + val textRange = runReadAction { findTextRangeForFlag(psiFile, flag) } if (textRange != null) { val url = "$baseUrl?q=${flag.lowercase()}" FeatureFlagSymbol(textRange, url) @@ -112,7 +103,6 @@ class FeatureFlagAnnotator : ExternalAnnotator> .range(symbol.textRange) .needsUpdateOnTyping(true) .withFix(UrlIntentionAction(message, symbol.url)) - .gutterIconRenderer(MyGutterIconRenderer(symbol.url)) .create() } } @@ -138,21 +128,3 @@ class UrlIntentionAction( return false } } - -class MyGutterIconRenderer(private val url: String) : GutterIconRenderer() { - - private val scaledWebIcon: Icon = IconUtil.scale(AllIcons.General.Web, null, 0.5f) - - override fun getIcon() = scaledWebIcon - - override fun getClickAction() = - object : AnAction() { - override fun actionPerformed(e: AnActionEvent) { - BrowserUtil.browse(url) - } - } - - override fun equals(other: Any?) = other is MyGutterIconRenderer && other.url == url - - override fun hashCode() = url.hashCode() -} From a945d8c1683f019ba3b86066d5997a10e122b8d9 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Fri, 22 Sep 2023 12:38:58 -0400 Subject: [PATCH 10/26] Test cleanup --- .../featureflags/FeatureFlagAnnotatorTest.kt | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt index e248bbb82..8dda11a78 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt @@ -25,17 +25,12 @@ import org.junit.Test class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { private val fileContent = """ - @FeatureFlags(ForComplianceFeature::class) - enum class ComplianceFeature(override val dependencies: Set = emptySet()) : - FeatureFlagEnum { - /** Enables validation of app-scoped environment variant */ - @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) ANDROID_ENV_VARIANT_VALIDATION, - @FeatureFlag(defaultValue = false, minimization = UNAUTHENTICATED) - ANDROID_GOV_SLACK_CUSTOM_AWARENESS, - @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) - ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH; - override val key: String by computeKey() - } + enum class TestFeatures : + FeatureFlagEnum { + @FeatureFlag FLAG_ONE, + @FeatureFlag FLAG_TWO, + @FeatureFlag FLAG_THREE, + } """ @Test @@ -51,16 +46,16 @@ class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { @Test fun `test extraction of feature flags from provided content`() { val featureFlagExtractor = FeatureFlagExtractor() - val psiFile = createKotlinFile("ComplianceFeature.kt", fileContent) + val psiFile = createKotlinFile("TestFeature.kt", fileContent) val featureFlags = featureFlagExtractor.extractFeatureFlags(psiFile) - assertTrue(featureFlags.contains("ANDROID_ENV_VARIANT_VALIDATION")) - assertTrue(featureFlags.contains("ANDROID_GOV_SLACK_CUSTOM_AWARENESS")) - assertTrue(featureFlags.contains("ANDROID_GOV_SLACK_CUSTOM_AWARENESS_TEAM_SWITCH")) + assertTrue(featureFlags.contains("FLAG_ONE")) + assertTrue(featureFlags.contains("FLAG_TWO")) + assertTrue(featureFlags.contains("FLAG_THREE")) } private fun runAnnotator(enabled: Boolean): List { project.service().isLinkifiedFeatureFlagsEnabled = enabled - val file = createKotlinFile("ComplianceFeature.kt", fileContent) + val file = createKotlinFile("TestFeature.kt", fileContent) FeatureFlagAnnotator().collectInformation(file) return FeatureFlagAnnotator().doAnnotate(file) } From 533826c9d66876ebec61e66beeb910c2d452aeb7 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Fri, 22 Sep 2023 12:40:59 -0400 Subject: [PATCH 11/26] Add syntax highlight --- .../slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt index 8dda11a78..0f0210397 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt @@ -23,6 +23,7 @@ import com.slack.sgp.intellij.SkatePluginSettings import org.junit.Test class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { + // language=kotlin private val fileContent = """ enum class TestFeatures : From 69f3cf42ecf827e82d4fcee2f3a86b1a2cd6a329 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Fri, 22 Sep 2023 13:04:49 -0400 Subject: [PATCH 12/26] Update url reference --- .../main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt index 2f684d2d8..08c91c50e 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt @@ -45,7 +45,7 @@ class SkatePluginSettings : SimplePersistentStateComponent Date: Mon, 25 Sep 2023 09:45:08 -0400 Subject: [PATCH 13/26] Step 1: Removing usage of reflection and using intellij's PSI framework --- .../featureflags/FeatureFlagAnnotator.kt | 24 ++------ .../featureflags/FeatureFlagExtractor.kt | 59 ++++--------------- .../featureflags/PsiElementExtensions.kt | 49 +++++++++++++++ .../featureflags/FeatureFlagAnnotatorTest.kt | 18 ++++-- 4 files changed, 77 insertions(+), 73 deletions(-) create mode 100644 skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt index ab2ecd87c..f90dd78cf 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt @@ -20,14 +20,11 @@ import com.intellij.ide.BrowserUtil import com.intellij.lang.annotation.AnnotationHolder import com.intellij.lang.annotation.ExternalAnnotator import com.intellij.lang.annotation.HighlightSeverity -import com.intellij.openapi.application.runReadAction import com.intellij.openapi.components.service import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project -import com.intellij.openapi.util.TextRange import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile -import com.intellij.psi.util.PsiTreeUtil import com.slack.sgp.intellij.SkatePluginSettings import com.slack.sgp.intellij.TEST_KOTLIN_LANGUAGE_ID_KEY import com.slack.sgp.intellij.util.isLinkifiedFeatureFlagsEnabled @@ -59,27 +56,14 @@ class FeatureFlagAnnotator : ExternalAnnotator> private fun transformToFeatureFlagSymbols( psiFile: PsiFile, - flags: List? + flags: List? ): List { if (flags.isNullOrEmpty()) { return emptyList() } val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() - return flags.mapNotNull { flag -> - val textRange = runReadAction { findTextRangeForFlag(psiFile, flag) } - if (textRange != null) { - val url = "$baseUrl?q=${flag.lowercase()}" - FeatureFlagSymbol(textRange, url) - } else { - null - } - } - } - - private fun findTextRangeForFlag(psiFile: PsiFile, flag: String): TextRange? { - val elements = PsiTreeUtil.findChildrenOfType(psiFile, PsiElement::class.java) - return elements.firstOrNull { it.text == flag }?.textRange + return flags.map { flag -> FeatureFlagSymbol(flag, "$baseUrl?q=${flag.text}") } } private fun isKotlinFile(psiFile: PsiFile): Boolean = @@ -100,7 +84,7 @@ class FeatureFlagAnnotator : ExternalAnnotator> HighlightSeverity.INFORMATION, "More details about feature flag on Houston..." ) - .range(symbol.textRange) + .range(symbol.element) .needsUpdateOnTyping(true) .withFix(UrlIntentionAction(message, symbol.url)) .create() @@ -108,7 +92,7 @@ class FeatureFlagAnnotator : ExternalAnnotator> } } -class FeatureFlagSymbol(val textRange: TextRange, val url: String) +class FeatureFlagSymbol(val element: PsiElement, val url: String) class UrlIntentionAction( private val message: String, diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 633f7e24e..8c06f9e19 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -16,6 +16,7 @@ package com.slack.sgp.intellij.featureflags import com.intellij.openapi.diagnostic.Logger +import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile /** @@ -28,14 +29,14 @@ class FeatureFlagExtractor { // Caches the feature flags for a given PSI file to optimize repeated lookups companion object { - private val featureFlagCache = mutableMapOf>() + private val featureFlagCache = mutableMapOf>() } - fun setFeatureFlagsForPsiFile(psiFile: PsiFile, flags: List) { + fun setFeatureFlagsForPsiFile(psiFile: PsiFile, flags: List) { featureFlagCache[psiFile] = flags } - fun getFeatureFlagsForPsiFile(psiFile: PsiFile): List? { + fun getFeatureFlagsForPsiFile(psiFile: PsiFile): List? { return featureFlagCache[psiFile] } /** @@ -44,58 +45,22 @@ class FeatureFlagExtractor { * @param psiFile The PSI representation of the file to process. * @return A list of feature flag names in a file */ - fun extractFeatureFlags(psiFile: PsiFile): List { + fun extractFeatureFlags(psiFile: PsiFile): List { log.info("Looking for feature flags in a file: $psiFile") - val enumsWithAnnotation = findAnnotatedEnums(psiFile) + val enumsWithAnnotation = findAnnotatedEnumElements(psiFile) log.info("Found feature flags: $enumsWithAnnotation") return enumsWithAnnotation } - /** Recursively searches the given PSI file for enums with 'FeatureFlag' annotations. */ - private fun findAnnotatedEnums(psiFile: Any): List { - val annotatedEnumEntries = mutableListOf() - - fun addIfAnnotatedEnum(element: Any) { - if (isKtEnumEntry(element) && hasFeatureFlagAnnotation(element)) { - element.javaClass - .getMethod("getName") - .invoke(element) - .takeIf { it is String } - ?.let { annotatedEnumEntries.add(it as String) } + private fun findAnnotatedEnumElements(psiFile: PsiFile): List { + val annotatedEnumEntries = mutableListOf() + fun recurse(element: PsiElement) { + if (element.isEnum() && element.hasAnnotation("FeatureFlag")) { + element.getEnumIdentifier()?.let { annotatedEnumEntries.add(it) } } - } - - fun recurse(element: Any) { - addIfAnnotatedEnum(element) - element.javaClass.methods - .find { it.name == "getChildren" } - ?.let { method -> (method.invoke(element) as? Array<*>)?.forEach { recurse(it!!) } } + element.children.forEach { recurse(it) } } recurse(psiFile) return annotatedEnumEntries } - - /** - * Determines if a given PSI element is an enum entry annotated with 'FeatureFlag'. - * - * @param element The enum entry to check. - * @return true if the enum entry is a feature flag, false otherwise. - */ - private fun hasFeatureFlagAnnotation(element: Any): Boolean { - val annotationEntriesMethod = element.javaClass.getMethod("getAnnotationEntries") - val annotationEntries = annotationEntriesMethod.invoke(element) as? List<*> - return annotationEntries?.any { - val shortNameMethod = it!!.javaClass.getMethod("getShortName") - val shortName = shortNameMethod.invoke(it) - shortName?.toString() == "FeatureFlag" - } ?: false - } - - /** Checks if the given PSI element is a Kotlin enum entry. */ - private fun isKtEnumEntry(element: Any): Boolean { - log.info("Checking if element is a Kotlin Enum Entry") - val result = element.javaClass.name == "org.jetbrains.kotlin.psi.KtEnumEntry" - log.info("Element is Kotlin Enum Entry: $result") - return result - } } diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt new file mode 100644 index 000000000..11d8d29c3 --- /dev/null +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.sgp.intellij.featureflags + +import com.intellij.psi.PsiElement +import com.intellij.psi.util.childrenOfType + +fun PsiElement.getFirstChild(elementType: String): PsiElement? { + return this.childrenOfType().firstOrNull { + it.node.elementType.toString() == elementType + } +} + +fun PsiElement.getChildren(elementType: String): List { + return this.childrenOfType().filter { it.node.elementType.toString() == elementType } +} + +fun PsiElement.getAnnotationEntries(): List { + return this.getFirstChild("MODIFIER_LIST")?.getChildren("ANNOTATION_ENTRY") ?: listOf() +} + +fun PsiElement.getAnnotationName(): String? { + return this.getFirstChild("CONSTRUCTOR_CALLEE")?.text +} + +fun PsiElement.isEnum(): Boolean { + return this.node?.elementType?.toString() == "ENUM_ENTRY" +} + +fun PsiElement.hasAnnotation(name: String): Boolean { + return this.getAnnotationEntries().any { it.getAnnotationName() == name } +} + +fun PsiElement.getEnumIdentifier(): PsiElement? { + return this.getFirstChild("IDENTIFIER") +} diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt index 0f0210397..f7ce5fb27 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt @@ -28,9 +28,13 @@ class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { """ enum class TestFeatures : FeatureFlagEnum { - @FeatureFlag FLAG_ONE, - @FeatureFlag FLAG_TWO, - @FeatureFlag FLAG_THREE, + @Deprecated("test") + @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + FLAG_ONE, + @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + FLAG_TWO, + @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + FLAG_THREE, } """ @@ -49,9 +53,11 @@ class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { val featureFlagExtractor = FeatureFlagExtractor() val psiFile = createKotlinFile("TestFeature.kt", fileContent) val featureFlags = featureFlagExtractor.extractFeatureFlags(psiFile) - assertTrue(featureFlags.contains("FLAG_ONE")) - assertTrue(featureFlags.contains("FLAG_TWO")) - assertTrue(featureFlags.contains("FLAG_THREE")) + val convertPsiElementToText = featureFlags.map { it.text } + assertTrue(convertPsiElementToText.size == 3) + assertTrue(convertPsiElementToText.contains("FLAG_ONE")) + assertTrue(convertPsiElementToText.contains("FLAG_TWO")) + assertTrue(convertPsiElementToText.contains("FLAG_THREE")) } private fun runAnnotator(enabled: Boolean): List { From 252b25b00fef199674f7b82f9895f3f943dfa6fc Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Mon, 25 Sep 2023 13:07:35 -0400 Subject: [PATCH 14/26] Streamline annotator --- .../featureflags/FeatureFlagAnnotator.kt | 69 ++++++++----------- .../featureflags/FeatureFlagExtractor.kt | 14 +--- .../featureflags/FeatureFlagAnnotatorTest.kt | 7 +- 3 files changed, 32 insertions(+), 58 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt index f90dd78cf..18894a354 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt @@ -31,46 +31,22 @@ import com.slack.sgp.intellij.util.isLinkifiedFeatureFlagsEnabled import java.net.URI import org.jetbrains.kotlin.idea.KotlinLanguage -class FeatureFlagAnnotator : ExternalAnnotator>() { - private val extractor by lazy { FeatureFlagExtractor() } - - override fun collectInformation(file: PsiFile): PsiFile? { - if (!isKotlinFeatureFile(file)) { - return null - } - if (extractor.getFeatureFlagsForPsiFile(file) == null) { - val flags = extractor.extractFeatureFlags(file) - extractor.setFeatureFlagsForPsiFile(file, flags) - } - return file - } - - override fun doAnnotate(collectedInfo: PsiFile): List { - if (!collectedInfo.project.isLinkifiedFeatureFlagsEnabled() || !isKotlinFile(collectedInfo)) { +class FeatureFlagAnnotator : ExternalAnnotator, List>() { + + override fun collectInformation(file: PsiFile): List { + if ( + !file.project.isLinkifiedFeatureFlagsEnabled() || + !isKotlinFile(file) || + !isKotlinFeatureFile(file) + ) { return emptyList() } - - val flagsForFile = extractor.getFeatureFlagsForPsiFile(collectedInfo) - return transformToFeatureFlagSymbols(collectedInfo, flagsForFile) + val flags = FeatureFlagExtractor.extractFeatureFlags(file) + return transformToFeatureFlagSymbols(file, flags) } - private fun transformToFeatureFlagSymbols( - psiFile: PsiFile, - flags: List? - ): List { - if (flags.isNullOrEmpty()) { - return emptyList() - } - val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() - - return flags.map { flag -> FeatureFlagSymbol(flag, "$baseUrl?q=${flag.text}") } - } - - private fun isKotlinFile(psiFile: PsiFile): Boolean = - psiFile.language.id == KotlinLanguage.INSTANCE.id || - psiFile.getUserData(TEST_KOTLIN_LANGUAGE_ID_KEY) == KotlinLanguage.INSTANCE.id - - private fun isKotlinFeatureFile(psiFile: PsiFile): Boolean = psiFile.name.endsWith("Feature.kt") + override fun doAnnotate(collectedInfo: List): List = + collectedInfo override fun apply( file: PsiFile, @@ -78,18 +54,29 @@ class FeatureFlagAnnotator : ExternalAnnotator> holder: AnnotationHolder ) { for (symbol in annotationResult) { - val message = "Open on Houston: ${symbol.url}" + val message = "Open at: ${symbol.url}" holder - .newAnnotation( - HighlightSeverity.INFORMATION, - "More details about feature flag on Houston..." - ) + .newAnnotation(HighlightSeverity.INFORMATION, "Open for more details.") .range(symbol.element) .needsUpdateOnTyping(true) .withFix(UrlIntentionAction(message, symbol.url)) .create() } } + + private fun transformToFeatureFlagSymbols( + psiFile: PsiFile, + flags: List? + ): List { + val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() + return flags.orEmpty().map { flag -> FeatureFlagSymbol(flag, "$baseUrl?q=${flag.text}") } + } + + private fun isKotlinFile(psiFile: PsiFile): Boolean = + psiFile.language.id == KotlinLanguage.INSTANCE.id || + psiFile.getUserData(TEST_KOTLIN_LANGUAGE_ID_KEY) == KotlinLanguage.INSTANCE.id + + private fun isKotlinFeatureFile(psiFile: PsiFile): Boolean = psiFile.name.endsWith("Feature.kt") } class FeatureFlagSymbol(val element: PsiElement, val url: String) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 8c06f9e19..043a86918 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -23,22 +23,10 @@ import com.intellij.psi.PsiFile * Responsible for extracting feature flags. Searches for enum entries annotated with 'FeatureFlag' * to identify feature flags. */ -class FeatureFlagExtractor { +object FeatureFlagExtractor { private val log: Logger = Logger.getInstance(FeatureFlagExtractor::class.java) - // Caches the feature flags for a given PSI file to optimize repeated lookups - companion object { - private val featureFlagCache = mutableMapOf>() - } - - fun setFeatureFlagsForPsiFile(psiFile: PsiFile, flags: List) { - featureFlagCache[psiFile] = flags - } - - fun getFeatureFlagsForPsiFile(psiFile: PsiFile): List? { - return featureFlagCache[psiFile] - } /** * Extracts the names of feature flags from the provided PSI file. Only processes Kotlin files. * diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt index f7ce5fb27..539b1b1b0 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt @@ -50,9 +50,8 @@ class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { @Test fun `test extraction of feature flags from provided content`() { - val featureFlagExtractor = FeatureFlagExtractor() val psiFile = createKotlinFile("TestFeature.kt", fileContent) - val featureFlags = featureFlagExtractor.extractFeatureFlags(psiFile) + val featureFlags = FeatureFlagExtractor.extractFeatureFlags(psiFile) val convertPsiElementToText = featureFlags.map { it.text } assertTrue(convertPsiElementToText.size == 3) assertTrue(convertPsiElementToText.contains("FLAG_ONE")) @@ -63,8 +62,8 @@ class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { private fun runAnnotator(enabled: Boolean): List { project.service().isLinkifiedFeatureFlagsEnabled = enabled val file = createKotlinFile("TestFeature.kt", fileContent) - FeatureFlagAnnotator().collectInformation(file) - return FeatureFlagAnnotator().doAnnotate(file) + val flags = FeatureFlagAnnotator().collectInformation(file) + return FeatureFlagAnnotator().doAnnotate(flags) } private fun createKotlinFile( From 3d793946ee4cb54b388a873d84ce7576699e5376 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Tue, 26 Sep 2023 20:22:00 -0400 Subject: [PATCH 15/26] Strongly typed Psi checks and some more cleanup --- .../featureflags/FeatureFlagAnnotator.kt | 17 +------- .../featureflags/FeatureFlagExtractor.kt | 27 +++++++----- .../featureflags/PsiElementExtensions.kt | 43 +++++++++---------- .../featureflags/FeatureFlagAnnotatorTest.kt | 13 +++--- 4 files changed, 45 insertions(+), 55 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt index 18894a354..89ebc66a5 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt @@ -20,12 +20,10 @@ import com.intellij.ide.BrowserUtil import com.intellij.lang.annotation.AnnotationHolder import com.intellij.lang.annotation.ExternalAnnotator import com.intellij.lang.annotation.HighlightSeverity -import com.intellij.openapi.components.service import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile -import com.slack.sgp.intellij.SkatePluginSettings import com.slack.sgp.intellij.TEST_KOTLIN_LANGUAGE_ID_KEY import com.slack.sgp.intellij.util.isLinkifiedFeatureFlagsEnabled import java.net.URI @@ -41,8 +39,7 @@ class FeatureFlagAnnotator : ExternalAnnotator, List): List = @@ -57,21 +54,13 @@ class FeatureFlagAnnotator : ExternalAnnotator, List? - ): List { - val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() - return flags.orEmpty().map { flag -> FeatureFlagSymbol(flag, "$baseUrl?q=${flag.text}") } - } - private fun isKotlinFile(psiFile: PsiFile): Boolean = psiFile.language.id == KotlinLanguage.INSTANCE.id || psiFile.getUserData(TEST_KOTLIN_LANGUAGE_ID_KEY) == KotlinLanguage.INSTANCE.id @@ -79,8 +68,6 @@ class FeatureFlagAnnotator : ExternalAnnotator, List { - log.info("Looking for feature flags in a file: $psiFile") - val enumsWithAnnotation = findAnnotatedEnumElements(psiFile) - log.info("Found feature flags: $enumsWithAnnotation") - return enumsWithAnnotation - } - - private fun findAnnotatedEnumElements(psiFile: PsiFile): List { - val annotatedEnumEntries = mutableListOf() + fun extractFeatureFlags(psiFile: PsiFile): List { + val annotatedEnumEntries = mutableListOf() + val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() fun recurse(element: PsiElement) { - if (element.isEnum() && element.hasAnnotation("FeatureFlag")) { - element.getEnumIdentifier()?.let { annotatedEnumEntries.add(it) } + if (element is KtEnumEntry && element.hasAnnotation("FeatureFlag")) { + val keyValue = element.getAnnotation("FeatureFlag")?.getKeyArgumentValue("key") + element.getEnumIdentifier()?.let { + annotatedEnumEntries.add( + FeatureFlagSymbol(it, "$baseUrl?q=${keyValue ?: it.text.lowercase()}") + ) + } } element.children.forEach { recurse(it) } } @@ -52,3 +55,5 @@ object FeatureFlagExtractor { return annotatedEnumEntries } } + +data class FeatureFlagSymbol(val element: LeafPsiElement, val url: String) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt index 11d8d29c3..1a5d53418 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt @@ -16,34 +16,31 @@ package com.slack.sgp.intellij.featureflags import com.intellij.psi.PsiElement +import com.intellij.psi.impl.source.tree.LeafPsiElement import com.intellij.psi.util.childrenOfType - -fun PsiElement.getFirstChild(elementType: String): PsiElement? { - return this.childrenOfType().firstOrNull { - it.node.elementType.toString() == elementType - } -} - -fun PsiElement.getChildren(elementType: String): List { - return this.childrenOfType().filter { it.node.elementType.toString() == elementType } -} - -fun PsiElement.getAnnotationEntries(): List { - return this.getFirstChild("MODIFIER_LIST")?.getChildren("ANNOTATION_ENTRY") ?: listOf() -} - -fun PsiElement.getAnnotationName(): String? { - return this.getFirstChild("CONSTRUCTOR_CALLEE")?.text +import org.jetbrains.kotlin.lexer.KtTokens +import org.jetbrains.kotlin.psi.KtAnnotationEntry +import org.jetbrains.kotlin.psi.KtEnumEntry +import org.jetbrains.uast.UExpression +import org.jetbrains.uast.toUElement + +fun KtEnumEntry.hasAnnotation(name: String): Boolean { + return this.getAnnotationEntries().any { it.shortName?.asString() == name } } -fun PsiElement.isEnum(): Boolean { - return this.node?.elementType?.toString() == "ENUM_ENTRY" +fun KtEnumEntry.getAnnotation(name: String): KtAnnotationEntry? { + return this.getAnnotationEntries().firstOrNull { it.shortName?.asString() == name } } -fun PsiElement.hasAnnotation(name: String): Boolean { - return this.getAnnotationEntries().any { it.getAnnotationName() == name } +fun PsiElement.getEnumIdentifier(): LeafPsiElement? { + return this.childrenOfType().firstOrNull { it.elementType == KtTokens.IDENTIFIER } } -fun PsiElement.getEnumIdentifier(): PsiElement? { - return this.getFirstChild("IDENTIFIER") +fun KtAnnotationEntry.getKeyArgumentValue(key: String): String? { + val argumentExpression = + this.valueArguments + .firstOrNull { it.getArgumentName()?.asName?.asString() == key } + ?.getArgumentExpression() + return (argumentExpression as? PsiElement)?.toUElement(UExpression::class.java)?.evaluate() + as? String } diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt index 539b1b1b0..7e199d8bf 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt @@ -29,7 +29,7 @@ class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { enum class TestFeatures : FeatureFlagEnum { @Deprecated("test") - @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + @FeatureFlag(defaultValue = false, key ="test_flag_one", minimization = AUTHENTICATED) FLAG_ONE, @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) FLAG_TWO, @@ -50,13 +50,14 @@ class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { @Test fun `test extraction of feature flags from provided content`() { + project.service().featureFlagBaseUrl = "test.com" val psiFile = createKotlinFile("TestFeature.kt", fileContent) val featureFlags = FeatureFlagExtractor.extractFeatureFlags(psiFile) - val convertPsiElementToText = featureFlags.map { it.text } - assertTrue(convertPsiElementToText.size == 3) - assertTrue(convertPsiElementToText.contains("FLAG_ONE")) - assertTrue(convertPsiElementToText.contains("FLAG_TWO")) - assertTrue(convertPsiElementToText.contains("FLAG_THREE")) + val flagUrls = featureFlags.map { it.url } + assertTrue(flagUrls.size == 3) + assertTrue(flagUrls.contains("test.com?q=test_flag_one")) + assertTrue(flagUrls.contains("test.com?q=flag_two")) + assertTrue(flagUrls.contains("test.com?q=flag_three")) } private fun runAnnotator(enabled: Boolean): List { From c3029fe9511fda0c2fed599f4cafab4bdfd46f49 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Tue, 26 Sep 2023 20:40:44 -0400 Subject: [PATCH 16/26] Splitting up tests to their respective test classes --- .../featureflags/BaseFeatureFlagTest.kt | 43 +++++++++++++++++++ .../featureflags/FeatureFlagAnnotatorTest.kt | 37 +--------------- .../featureflags/FeatureFlagExtractorTest.kt | 35 +++++++++++++++ 3 files changed, 79 insertions(+), 36 deletions(-) create mode 100644 skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/BaseFeatureFlagTest.kt create mode 100644 skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/BaseFeatureFlagTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/BaseFeatureFlagTest.kt new file mode 100644 index 000000000..4a9384d02 --- /dev/null +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/BaseFeatureFlagTest.kt @@ -0,0 +1,43 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.sgp.intellij.featureflags + +import com.intellij.psi.PsiFile +import com.intellij.testFramework.fixtures.LightPlatformCodeInsightFixture4TestCase + +abstract class BaseFeatureFlagTest : LightPlatformCodeInsightFixture4TestCase() { + // language=kotlin + protected val fileContent = + """ + enum class TestFeatures : + FeatureFlagEnum { + @Deprecated("test") + @FeatureFlag(defaultValue = false, key ="test_flag_one", minimization = AUTHENTICATED) + FLAG_ONE, + @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + FLAG_TWO, + @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) + FLAG_THREE, + } + """ + + protected fun createKotlinFile( + name: String, + text: String, + ): PsiFile { + return myFixture.configureByText(name, text) + } +} diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt index 7e199d8bf..c33e4c04c 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt @@ -17,26 +17,10 @@ package com.slack.sgp.intellij.featureflags import com.google.common.truth.Truth.assertThat import com.intellij.openapi.components.service -import com.intellij.psi.PsiFile -import com.intellij.testFramework.fixtures.LightPlatformCodeInsightFixture4TestCase import com.slack.sgp.intellij.SkatePluginSettings import org.junit.Test -class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { - // language=kotlin - private val fileContent = - """ - enum class TestFeatures : - FeatureFlagEnum { - @Deprecated("test") - @FeatureFlag(defaultValue = false, key ="test_flag_one", minimization = AUTHENTICATED) - FLAG_ONE, - @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) - FLAG_TWO, - @FeatureFlag(defaultValue = false, minimization = AUTHENTICATED) - FLAG_THREE, - } - """ +class FeatureFlagAnnotatorTest : BaseFeatureFlagTest() { @Test fun `test returns empty list when linkify is disabled`() { @@ -48,29 +32,10 @@ class FeatureFlagAnnotatorTest : LightPlatformCodeInsightFixture4TestCase() { assertThat(runAnnotator(enabled = true)).isNotEmpty() } - @Test - fun `test extraction of feature flags from provided content`() { - project.service().featureFlagBaseUrl = "test.com" - val psiFile = createKotlinFile("TestFeature.kt", fileContent) - val featureFlags = FeatureFlagExtractor.extractFeatureFlags(psiFile) - val flagUrls = featureFlags.map { it.url } - assertTrue(flagUrls.size == 3) - assertTrue(flagUrls.contains("test.com?q=test_flag_one")) - assertTrue(flagUrls.contains("test.com?q=flag_two")) - assertTrue(flagUrls.contains("test.com?q=flag_three")) - } - private fun runAnnotator(enabled: Boolean): List { project.service().isLinkifiedFeatureFlagsEnabled = enabled val file = createKotlinFile("TestFeature.kt", fileContent) val flags = FeatureFlagAnnotator().collectInformation(file) return FeatureFlagAnnotator().doAnnotate(flags) } - - private fun createKotlinFile( - name: String, - text: String, - ): PsiFile { - return myFixture.configureByText(name, text) - } } diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt new file mode 100644 index 000000000..ea4f6d131 --- /dev/null +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2023 Slack Technologies, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.slack.sgp.intellij.featureflags + +import com.intellij.openapi.components.service +import com.slack.sgp.intellij.SkatePluginSettings +import org.junit.Test + +class FeatureFlagExtractorTest : BaseFeatureFlagTest() { + + @Test + fun `test extraction of feature flags from provided content`() { + project.service().featureFlagBaseUrl = "test.com" + val psiFile = createKotlinFile("TestFeature.kt", fileContent) + val featureFlags = FeatureFlagExtractor.extractFeatureFlags(psiFile) + val flagUrls = featureFlags.map { it.url } + assertTrue(flagUrls.size == 3) + assertTrue(flagUrls.contains("test.com?q=test_flag_one")) + assertTrue(flagUrls.contains("test.com?q=flag_two")) + assertTrue(flagUrls.contains("test.com?q=flag_three")) + } +} From 73c96a802cf749a005d4ffe81e86b25af01dc09e Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Wed, 27 Sep 2023 10:40:29 -0400 Subject: [PATCH 17/26] Further cleanup --- .../featureflags/FeatureFlagExtractor.kt | 18 ++++++++---------- .../featureflags/PsiElementExtensions.kt | 4 ---- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 457bc9485..9a5e0adae 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -16,7 +16,6 @@ package com.slack.sgp.intellij.featureflags import com.intellij.openapi.components.service -import com.intellij.openapi.diagnostic.Logger import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile import com.intellij.psi.impl.source.tree.LeafPsiElement @@ -29,24 +28,23 @@ import org.jetbrains.kotlin.psi.KtEnumEntry */ object FeatureFlagExtractor { - private val log: Logger = Logger.getInstance(FeatureFlagExtractor::class.java) - /** * Extracts the names of feature flags from the provided PSI file. Only processes Kotlin files. * * @param psiFile The PSI representation of the file to process. - * @return A list of feature flag names in a file */ fun extractFeatureFlags(psiFile: PsiFile): List { val annotatedEnumEntries = mutableListOf() val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() fun recurse(element: PsiElement) { - if (element is KtEnumEntry && element.hasAnnotation("FeatureFlag")) { - val keyValue = element.getAnnotation("FeatureFlag")?.getKeyArgumentValue("key") - element.getEnumIdentifier()?.let { - annotatedEnumEntries.add( - FeatureFlagSymbol(it, "$baseUrl?q=${keyValue ?: it.text.lowercase()}") - ) + if (element is KtEnumEntry) { + element.getAnnotation("FeatureFlag")?.let { annotation -> + val keyValue = annotation.getKeyArgumentValue("key") + element.getEnumIdentifier()?.let { identifier -> + annotatedEnumEntries.add( + FeatureFlagSymbol(identifier, "$baseUrl?q=${keyValue ?: identifier.text.lowercase()}") + ) + } } } element.children.forEach { recurse(it) } diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt index 1a5d53418..5d5f75adc 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt @@ -24,10 +24,6 @@ import org.jetbrains.kotlin.psi.KtEnumEntry import org.jetbrains.uast.UExpression import org.jetbrains.uast.toUElement -fun KtEnumEntry.hasAnnotation(name: String): Boolean { - return this.getAnnotationEntries().any { it.shortName?.asString() == name } -} - fun KtEnumEntry.getAnnotation(name: String): KtAnnotationEntry? { return this.getAnnotationEntries().firstOrNull { it.shortName?.asString() == name } } From ec81f4417cd0409bed9ab368d3bdb606b12ea717 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Wed, 27 Sep 2023 15:42:12 -0400 Subject: [PATCH 18/26] Use UAST API instead of using traversal APIs and add tests --- .../slack/sgp/intellij/SkatePluginSettings.kt | 7 +++ .../featureflags/FeatureFlagAnnotator.kt | 3 +- .../featureflags/FeatureFlagExtractor.kt | 55 +++++++++++++------ .../featureflags/PsiElementExtensions.kt | 28 +++------- .../featureflags/BaseFeatureFlagTest.kt | 6 ++ .../featureflags/FeatureFlagAnnotatorTest.kt | 2 + .../featureflags/FeatureFlagExtractorTest.kt | 54 ++++++++++++++++-- 7 files changed, 109 insertions(+), 46 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt index 08c91c50e..ad49acea2 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt @@ -50,10 +50,17 @@ class SkatePluginSettings : SimplePersistentStateComponent, List { - val annotatedEnumEntries = mutableListOf() + // Ensure baseUrl and flagAnnotation are not empty when the feature flag linkifying is enabled + // Ensure baseUrl ends with query param - "?q=" val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() - fun recurse(element: PsiElement) { - if (element is KtEnumEntry) { - element.getAnnotation("FeatureFlag")?.let { annotation -> - val keyValue = annotation.getKeyArgumentValue("key") - element.getEnumIdentifier()?.let { identifier -> - annotatedEnumEntries.add( - FeatureFlagSymbol(identifier, "$baseUrl?q=${keyValue ?: identifier.text.lowercase()}") - ) - } + val flagAnnotation = + psiFile.project.service().featureFlagAnnotation.orEmpty() + require(baseUrl.isNotBlank()) { BASE_URL_EMPTY_ERROR } + require(baseUrl.endsWith("?q=")) { BASE_URL_QUERY_PARAM_ERROR } + require(flagAnnotation.isNotBlank()) { ANNOTATION_EMPTY_ERROR } + + if (psiFile !is KtFile) return emptyList() + val uFile = psiFile.toUElementOfType() ?: return emptyList() + + return uFile + .allClassesAndInnerClasses() + .filter { it.isEnum } + .flatMap { enumClass -> enumClass.uastDeclarations.filterIsInstance() } + .mapNotNull { enumConstant -> + enumConstant.findAnnotation(flagAnnotation)?.let { flagAnnotation -> + val key = + flagAnnotation.findAttributeValue("key")?.evaluateString()?.takeUnless { + it.trim().isBlank() + } ?: enumConstant.name.lowercase(Locale.US) + val textRange = enumConstant.sourcePsi?.textRange ?: return@mapNotNull null + FeatureFlagSymbol(textRange, "$baseUrl$key") } } - element.children.forEach { recurse(it) } - } - recurse(psiFile) - return annotatedEnumEntries + .toList() } } -data class FeatureFlagSymbol(val element: LeafPsiElement, val url: String) +data class FeatureFlagSymbol(val range: TextRange, val url: String) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt index 5d5f75adc..88142cf71 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt @@ -15,28 +15,14 @@ */ package com.slack.sgp.intellij.featureflags -import com.intellij.psi.PsiElement -import com.intellij.psi.impl.source.tree.LeafPsiElement -import com.intellij.psi.util.childrenOfType -import org.jetbrains.kotlin.lexer.KtTokens -import org.jetbrains.kotlin.psi.KtAnnotationEntry -import org.jetbrains.kotlin.psi.KtEnumEntry -import org.jetbrains.uast.UExpression -import org.jetbrains.uast.toUElement +import org.jetbrains.uast.UClass +import org.jetbrains.uast.UFile -fun KtEnumEntry.getAnnotation(name: String): KtAnnotationEntry? { - return this.getAnnotationEntries().firstOrNull { it.shortName?.asString() == name } +fun UFile.allClassesAndInnerClasses(): Sequence { + return classes.asSequence().flatMap(UClass::asSequenceWithInnerClasses) } -fun PsiElement.getEnumIdentifier(): LeafPsiElement? { - return this.childrenOfType().firstOrNull { it.elementType == KtTokens.IDENTIFIER } -} - -fun KtAnnotationEntry.getKeyArgumentValue(key: String): String? { - val argumentExpression = - this.valueArguments - .firstOrNull { it.getArgumentName()?.asName?.asString() == key } - ?.getArgumentExpression() - return (argumentExpression as? PsiElement)?.toUElement(UExpression::class.java)?.evaluate() - as? String +fun UClass.asSequenceWithInnerClasses(): Sequence { + return sequenceOf(this) + .plus(innerClasses.asSequence().flatMap(UClass::asSequenceWithInnerClasses)) } diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/BaseFeatureFlagTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/BaseFeatureFlagTest.kt index 4a9384d02..9a0a6daca 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/BaseFeatureFlagTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/BaseFeatureFlagTest.kt @@ -22,6 +22,12 @@ abstract class BaseFeatureFlagTest : LightPlatformCodeInsightFixture4TestCase() // language=kotlin protected val fileContent = """ + package slack.featureflag + + annotation class FeatureFlag(defaultValue: Boolean, val key: String = "", minimization: Minimization) + + enum class Minimization { AUTHENTICATED } + enum class TestFeatures : FeatureFlagEnum { @Deprecated("test") diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt index c33e4c04c..45ce73e4a 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotatorTest.kt @@ -34,6 +34,8 @@ class FeatureFlagAnnotatorTest : BaseFeatureFlagTest() { private fun runAnnotator(enabled: Boolean): List { project.service().isLinkifiedFeatureFlagsEnabled = enabled + project.service().featureFlagBaseUrl = "test.com?q=" + project.service().featureFlagAnnotation = "slack.featureflag.FeatureFlag" val file = createKotlinFile("TestFeature.kt", fileContent) val flags = FeatureFlagAnnotator().collectInformation(file) return FeatureFlagAnnotator().doAnnotate(flags) diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt index ea4f6d131..cf66695f5 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt @@ -15,21 +15,65 @@ */ package com.slack.sgp.intellij.featureflags +import com.google.common.truth.Truth.assertThat import com.intellij.openapi.components.service import com.slack.sgp.intellij.SkatePluginSettings +import com.slack.sgp.intellij.featureflags.FeatureFlagExtractor.ANNOTATION_EMPTY_ERROR +import com.slack.sgp.intellij.featureflags.FeatureFlagExtractor.BASE_URL_EMPTY_ERROR +import com.slack.sgp.intellij.featureflags.FeatureFlagExtractor.BASE_URL_QUERY_PARAM_ERROR import org.junit.Test class FeatureFlagExtractorTest : BaseFeatureFlagTest() { + @Test + fun `test throws error when featureFlagBaseUrl is empty`() { + project.service().featureFlagBaseUrl = "" + val file = createKotlinFile("TestFeature.kt", fileContent) + try { + FeatureFlagAnnotator().collectInformation(file) + fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") + } catch (e: IllegalArgumentException) { + assertThat(e).hasMessageThat().contains(BASE_URL_EMPTY_ERROR) + } + } + + @Test + fun `test throws error when featureFlagBaseUrl doesn't end with query param`() { + project.service().featureFlagBaseUrl = "https://example.com/" + val file = createKotlinFile("TestFeature.kt", fileContent) + try { + FeatureFlagAnnotator().collectInformation(file) + fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") + } catch (e: IllegalArgumentException) { + assertThat(e).hasMessageThat().contains(BASE_URL_QUERY_PARAM_ERROR) + } + } + + @Test + fun `test throws error when featureFlagAnnotation is empty`() { + project.service().featureFlagBaseUrl = "test.com?q=" + project.service().featureFlagAnnotation = "" + val file = createKotlinFile("TestFeature.kt", fileContent) + try { + FeatureFlagAnnotator().collectInformation(file) + fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") + } catch (e: IllegalArgumentException) { + assertThat(e).hasMessageThat().contains(ANNOTATION_EMPTY_ERROR) + } + } + @Test fun `test extraction of feature flags from provided content`() { - project.service().featureFlagBaseUrl = "test.com" + project.service().featureFlagBaseUrl = "test.com?q=" + project.service().featureFlagAnnotation = "slack.featureflag.FeatureFlag" val psiFile = createKotlinFile("TestFeature.kt", fileContent) val featureFlags = FeatureFlagExtractor.extractFeatureFlags(psiFile) val flagUrls = featureFlags.map { it.url } - assertTrue(flagUrls.size == 3) - assertTrue(flagUrls.contains("test.com?q=test_flag_one")) - assertTrue(flagUrls.contains("test.com?q=flag_two")) - assertTrue(flagUrls.contains("test.com?q=flag_three")) + assertThat(flagUrls) + .containsExactly( + "test.com?q=test_flag_one", + "test.com?q=flag_two", + "test.com?q=flag_three", + ) } } From 54b1affb9b8ac47f0e2da2b2e35008216e2fb264 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Wed, 27 Sep 2023 15:51:21 -0400 Subject: [PATCH 19/26] Remove redundant check --- .../com/slack/sgp/intellij/Constants.kt | 22 ------------------- .../featureflags/FeatureFlagAnnotator.kt | 9 ++------ .../featureflags/FeatureFlagExtractor.kt | 2 -- 3 files changed, 2 insertions(+), 31 deletions(-) delete mode 100644 skate-plugin/src/main/kotlin/com/slack/sgp/intellij/Constants.kt diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/Constants.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/Constants.kt deleted file mode 100644 index 30916a5ea..000000000 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/Constants.kt +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright (C) 2023 Slack Technologies, LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.slack.sgp.intellij - -import com.intellij.openapi.util.Key - -// Note this is a hack to allow tests to not use KotlinLanguage class due to classloading problems -// with IJ and detekt. -val TEST_KOTLIN_LANGUAGE_ID_KEY = Key.create("__tests_only_kotlin_id__") diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt index 69e246dab..38e7e98b6 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt @@ -23,17 +23,16 @@ import com.intellij.lang.annotation.HighlightSeverity import com.intellij.openapi.editor.Editor import com.intellij.openapi.project.Project import com.intellij.psi.PsiFile -import com.slack.sgp.intellij.TEST_KOTLIN_LANGUAGE_ID_KEY import com.slack.sgp.intellij.util.isLinkifiedFeatureFlagsEnabled import java.net.URI -import org.jetbrains.kotlin.idea.KotlinLanguage +import org.jetbrains.kotlin.psi.KtFile class FeatureFlagAnnotator : ExternalAnnotator, List>() { override fun collectInformation(file: PsiFile): List { if ( !file.project.isLinkifiedFeatureFlagsEnabled() || - !isKotlinFile(file) || + file !is KtFile || !isKotlinFeatureFile(file) ) { return emptyList() @@ -60,10 +59,6 @@ class FeatureFlagAnnotator : ExternalAnnotator, List { // Ensure baseUrl and flagAnnotation are not empty when the feature flag linkifying is enabled // Ensure baseUrl ends with query param - "?q=" - val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() - val flagAnnotation = - psiFile.project.service().featureFlagAnnotation.orEmpty() + val settings = psiFile.project.service() + val baseUrl = settings.featureFlagBaseUrl.orEmpty() + val flagAnnotation = settings.featureFlagAnnotation.orEmpty() require(baseUrl.isNotBlank()) { BASE_URL_EMPTY_ERROR } require(baseUrl.endsWith("?q=")) { BASE_URL_QUERY_PARAM_ERROR } require(flagAnnotation.isNotBlank()) { ANNOTATION_EMPTY_ERROR } diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/UastExtensions.kt similarity index 100% rename from skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/PsiElementExtensions.kt rename to skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/UastExtensions.kt From 627ff133121adeb7e001afa047bf0c54c818f068 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Wed, 27 Sep 2023 16:43:49 -0400 Subject: [PATCH 21/26] Fix tests --- gradle/libs.versions.toml | 1 - .../slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 99895884f..239adfb11 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -78,7 +78,6 @@ gradlePlugins-wire = { module = "com.squareup.wire:wire-gradle-plugin", version. guava = "com.google.guava:guava:32.1.2-jre" kotlinCliUtil = "com.slack.cli:kotlin-cli-util:2.2.1" kotlin-bom = { module = "org.jetbrains.kotlin:kotlin-bom", version.ref = "kotlin" } -kotlin-compilerEmbeddable = { module = "org.jetbrains.kotlin:kotlin-compiler-embeddable", version.ref = "kotlin" } kotlin-reflect = { module = "org.jetbrains.kotlin:kotlin-reflect", version.ref = "kotlin" } ktfmt = { module = "com.facebook:ktfmt", version.ref = "ktfmt" } jgrapht = "org.jgrapht:jgrapht-core:1.5.2" diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 08825da4e..3327cebde 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -44,9 +44,9 @@ object FeatureFlagExtractor { fun extractFeatureFlags(psiFile: PsiFile): List { // Ensure baseUrl and flagAnnotation are not empty when the feature flag linkifying is enabled // Ensure baseUrl ends with query param - "?q=" - val settings = psiFile.project.service() - val baseUrl = settings.featureFlagBaseUrl.orEmpty() - val flagAnnotation = settings.featureFlagAnnotation.orEmpty() + val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() + val flagAnnotation = + psiFile.project.service().featureFlagAnnotation.orEmpty() require(baseUrl.isNotBlank()) { BASE_URL_EMPTY_ERROR } require(baseUrl.endsWith("?q=")) { BASE_URL_QUERY_PARAM_ERROR } require(flagAnnotation.isNotBlank()) { ANNOTATION_EMPTY_ERROR } From d7408ef9b557b2edb115483256a528c510bd9005 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Wed, 27 Sep 2023 17:16:22 -0400 Subject: [PATCH 22/26] Fix tests --- .../sgp/intellij/featureflags/FeatureFlagExtractor.kt | 6 +++--- .../sgp/intellij/featureflags/FeatureFlagExtractorTest.kt | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 3327cebde..08825da4e 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -44,9 +44,9 @@ object FeatureFlagExtractor { fun extractFeatureFlags(psiFile: PsiFile): List { // Ensure baseUrl and flagAnnotation are not empty when the feature flag linkifying is enabled // Ensure baseUrl ends with query param - "?q=" - val baseUrl = psiFile.project.service().featureFlagBaseUrl.orEmpty() - val flagAnnotation = - psiFile.project.service().featureFlagAnnotation.orEmpty() + val settings = psiFile.project.service() + val baseUrl = settings.featureFlagBaseUrl.orEmpty() + val flagAnnotation = settings.featureFlagAnnotation.orEmpty() require(baseUrl.isNotBlank()) { BASE_URL_EMPTY_ERROR } require(baseUrl.endsWith("?q=")) { BASE_URL_QUERY_PARAM_ERROR } require(flagAnnotation.isNotBlank()) { ANNOTATION_EMPTY_ERROR } diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt index cf66695f5..59b6ea5e9 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt @@ -30,7 +30,7 @@ class FeatureFlagExtractorTest : BaseFeatureFlagTest() { project.service().featureFlagBaseUrl = "" val file = createKotlinFile("TestFeature.kt", fileContent) try { - FeatureFlagAnnotator().collectInformation(file) + FeatureFlagExtractor.extractFeatureFlags(file) fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") } catch (e: IllegalArgumentException) { assertThat(e).hasMessageThat().contains(BASE_URL_EMPTY_ERROR) @@ -39,10 +39,10 @@ class FeatureFlagExtractorTest : BaseFeatureFlagTest() { @Test fun `test throws error when featureFlagBaseUrl doesn't end with query param`() { - project.service().featureFlagBaseUrl = "https://example.com/" + project.service().featureFlagBaseUrl = "test.com" val file = createKotlinFile("TestFeature.kt", fileContent) try { - FeatureFlagAnnotator().collectInformation(file) + FeatureFlagExtractor.extractFeatureFlags(file) fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") } catch (e: IllegalArgumentException) { assertThat(e).hasMessageThat().contains(BASE_URL_QUERY_PARAM_ERROR) @@ -55,7 +55,7 @@ class FeatureFlagExtractorTest : BaseFeatureFlagTest() { project.service().featureFlagAnnotation = "" val file = createKotlinFile("TestFeature.kt", fileContent) try { - FeatureFlagAnnotator().collectInformation(file) + FeatureFlagExtractor.extractFeatureFlags(file) fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") } catch (e: IllegalArgumentException) { assertThat(e).hasMessageThat().contains(ANNOTATION_EMPTY_ERROR) From d1a586ee7e7422421220df56cd4b6c5709b1eed5 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 28 Sep 2023 11:45:08 -0400 Subject: [PATCH 23/26] Add UI configuration to take user input for Feature Flag values and ensure flag name and URL value is not empty when Feature Flag is enabled --- .../slack/sgp/intellij/SkatePluginSettings.kt | 2 +- .../featureflags/FeatureFlagExtractor.kt | 12 +---- .../slack/sgp/intellij/ui/SkateConfigUI.kt | 54 +++++++++++++++++++ .../resources/messages/skateBundle.properties | 5 ++ .../featureflags/FeatureFlagExtractorTest.kt | 40 -------------- 5 files changed, 61 insertions(+), 52 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt index 9882be7f6..1bd79a072 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/SkatePluginSettings.kt @@ -78,7 +78,7 @@ class SkatePluginSettings : SimplePersistentStateComponent { - // Ensure baseUrl and flagAnnotation are not empty when the feature flag linkifying is enabled - // Ensure baseUrl ends with query param - "?q=" val settings = psiFile.project.service() - val baseUrl = settings.featureFlagBaseUrl.orEmpty() + val baseUrl = settings.featureFlagBaseUrl val flagAnnotation = settings.featureFlagAnnotation.orEmpty() - require(baseUrl.isNotBlank()) { BASE_URL_EMPTY_ERROR } - require(baseUrl.endsWith("?q=")) { BASE_URL_QUERY_PARAM_ERROR } - require(flagAnnotation.isNotBlank()) { ANNOTATION_EMPTY_ERROR } val uFile = psiFile.toUElementOfType() ?: return emptyList() diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/ui/SkateConfigUI.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/ui/SkateConfigUI.kt index f9cc96dac..86307ba42 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/ui/SkateConfigUI.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/ui/SkateConfigUI.kt @@ -18,10 +18,14 @@ package com.slack.sgp.intellij.ui import com.intellij.openapi.project.Project import com.intellij.openapi.ui.DialogPanel import com.intellij.openapi.vfs.LocalFileSystem +import com.intellij.ui.components.JBCheckBox +import com.intellij.ui.dsl.builder.Cell import com.intellij.ui.dsl.builder.Panel import com.intellij.ui.dsl.builder.bindSelected import com.intellij.ui.dsl.builder.bindText import com.intellij.ui.dsl.builder.panel +import com.intellij.ui.dsl.builder.selected +import com.intellij.ui.layout.ComponentPredicate import com.slack.sgp.intellij.SkateBundle import com.slack.sgp.intellij.SkatePluginSettings import java.io.File @@ -34,6 +38,7 @@ internal class SkateConfigUI( fun createPanel(): DialogPanel = panel { checkBoxRow() choosePathRow() + featureFlagSettings() } private fun Panel.checkBoxRow() { @@ -71,4 +76,53 @@ internal class SkateConfigUI( .enabled(settings.isWhatsNewEnabled) } } + + private fun Panel.featureFlagSettings() { + lateinit var linkifiedFeatureFlagsCheckBox: Cell + + row(SkateBundle.message("skate.configuration.enableFeatureFlagLinking.title")) { + linkifiedFeatureFlagsCheckBox = + checkBox(SkateBundle.message("skate.configuration.enableFeatureFlagLinking.description")) + .bindSelected( + getter = { settings.isLinkifiedFeatureFlagsEnabled }, + setter = { settings.isLinkifiedFeatureFlagsEnabled = it } + ) + } + + bindAndValidateTextFieldRow( + titleMessageKey = "skate.configuration.featureFlagAnnotation.title", + getter = { settings.featureFlagAnnotation }, + setter = { settings.featureFlagAnnotation = it }, + errorMessageKey = "skate.configuration.featureFlagFieldEmpty.error", + enabledCondition = linkifiedFeatureFlagsCheckBox.selected + ) + + bindAndValidateTextFieldRow( + titleMessageKey = "skate.configuration.featureFlagBaseUrl.title", + getter = { settings.featureFlagBaseUrl }, + setter = { settings.featureFlagBaseUrl = it }, + errorMessageKey = "skate.configuration.featureFlagFieldEmpty.error", + enabledCondition = linkifiedFeatureFlagsCheckBox.selected + ) + } + + private fun Panel.bindAndValidateTextFieldRow( + titleMessageKey: String, + getter: () -> String?, + setter: (String) -> Unit, + errorMessageKey: String, + enabledCondition: ComponentPredicate? = null + ) { + row(SkateBundle.message(titleMessageKey)) { + textField() + .bindText(getter = { getter().orEmpty() }, setter = setter) + .validationOnApply { + if (it.text.isBlank()) error(SkateBundle.message(errorMessageKey)) else null + } + .validationOnInput { + if (it.text.isBlank()) error(SkateBundle.message(errorMessageKey)) else null + } + .apply { enabledCondition?.let { enabledIf(it) } } + } + } } diff --git a/skate-plugin/src/main/resources/messages/skateBundle.properties b/skate-plugin/src/main/resources/messages/skateBundle.properties index e6d066416..0303e3871 100644 --- a/skate-plugin/src/main/resources/messages/skateBundle.properties +++ b/skate-plugin/src/main/resources/messages/skateBundle.properties @@ -4,3 +4,8 @@ skate.configuration.choosePath.dialog.title=Choose Changelog File skate.configuration.enableWhatsNew.title=Enable What's New Panel skate.configuration.enableWhatsNew.description=If enabled, shows the What's New Panel if there are new changes detected in a specified changelog file. skate.modelTranslator.description=Generate translator body +skate.configuration.enableFeatureFlagLinking.title=Enable Feature Flag Linking +skate.configuration.enableFeatureFlagLinking.description=If enabled, feature flag URL will be shown in the quick documentation tooltip. +skate.configuration.featureFlagBaseUrl.title=Enter URL to be added as a prefix to flag annotation +skate.configuration.featureFlagAnnotation.title=Feature flag annotation name +skate.configuration.featureFlagFieldEmpty.error=Field cannot be empty when feature flag Linking is enabled diff --git a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt index 59b6ea5e9..40689f76e 100644 --- a/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt +++ b/skate-plugin/src/test/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractorTest.kt @@ -18,50 +18,10 @@ package com.slack.sgp.intellij.featureflags import com.google.common.truth.Truth.assertThat import com.intellij.openapi.components.service import com.slack.sgp.intellij.SkatePluginSettings -import com.slack.sgp.intellij.featureflags.FeatureFlagExtractor.ANNOTATION_EMPTY_ERROR -import com.slack.sgp.intellij.featureflags.FeatureFlagExtractor.BASE_URL_EMPTY_ERROR -import com.slack.sgp.intellij.featureflags.FeatureFlagExtractor.BASE_URL_QUERY_PARAM_ERROR import org.junit.Test class FeatureFlagExtractorTest : BaseFeatureFlagTest() { - @Test - fun `test throws error when featureFlagBaseUrl is empty`() { - project.service().featureFlagBaseUrl = "" - val file = createKotlinFile("TestFeature.kt", fileContent) - try { - FeatureFlagExtractor.extractFeatureFlags(file) - fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") - } catch (e: IllegalArgumentException) { - assertThat(e).hasMessageThat().contains(BASE_URL_EMPTY_ERROR) - } - } - - @Test - fun `test throws error when featureFlagBaseUrl doesn't end with query param`() { - project.service().featureFlagBaseUrl = "test.com" - val file = createKotlinFile("TestFeature.kt", fileContent) - try { - FeatureFlagExtractor.extractFeatureFlags(file) - fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") - } catch (e: IllegalArgumentException) { - assertThat(e).hasMessageThat().contains(BASE_URL_QUERY_PARAM_ERROR) - } - } - - @Test - fun `test throws error when featureFlagAnnotation is empty`() { - project.service().featureFlagBaseUrl = "test.com?q=" - project.service().featureFlagAnnotation = "" - val file = createKotlinFile("TestFeature.kt", fileContent) - try { - FeatureFlagExtractor.extractFeatureFlags(file) - fail("Expected an IllegalArgumentException to be thrown, but it wasn't.") - } catch (e: IllegalArgumentException) { - assertThat(e).hasMessageThat().contains(ANNOTATION_EMPTY_ERROR) - } - } - @Test fun `test extraction of feature flags from provided content`() { project.service().featureFlagBaseUrl = "test.com?q=" From d93844e0a024067cd817e125f9a3144a414d2618 Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 28 Sep 2023 11:47:25 -0400 Subject: [PATCH 24/26] assign conditions to a variable for easier readability --- .../sgp/intellij/featureflags/FeatureFlagAnnotator.kt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt index 38e7e98b6..293f9b385 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagAnnotator.kt @@ -30,11 +30,10 @@ import org.jetbrains.kotlin.psi.KtFile class FeatureFlagAnnotator : ExternalAnnotator, List>() { override fun collectInformation(file: PsiFile): List { - if ( - !file.project.isLinkifiedFeatureFlagsEnabled() || - file !is KtFile || - !isKotlinFeatureFile(file) - ) { + val isEligibleForLinkifiedFeatureProcessing = + file.project.isLinkifiedFeatureFlagsEnabled() && file is KtFile && isKotlinFeatureFile(file) + + if (!isEligibleForLinkifiedFeatureProcessing) { return emptyList() } return FeatureFlagExtractor.extractFeatureFlags(file) From dacb0bc36c03ecd9a4fa4f1e49f4cba8bec0f91d Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 28 Sep 2023 11:49:28 -0400 Subject: [PATCH 25/26] Early return when textRange not found --- .../com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt index 64dae9831..d942d834c 100644 --- a/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt +++ b/skate-plugin/src/main/kotlin/com/slack/sgp/intellij/featureflags/FeatureFlagExtractor.kt @@ -49,11 +49,11 @@ object FeatureFlagExtractor { .flatMap { enumClass -> enumClass.uastDeclarations.filterIsInstance() } .mapNotNull { enumConstant -> enumConstant.findAnnotation(flagAnnotation)?.let { flagAnnotation -> + val textRange = enumConstant.sourcePsi?.textRange ?: return@mapNotNull null val key = flagAnnotation.findAttributeValue("key")?.evaluateString()?.takeUnless { it.trim().isBlank() } ?: enumConstant.name.lowercase(Locale.US) - val textRange = enumConstant.sourcePsi?.textRange ?: return@mapNotNull null FeatureFlagSymbol(textRange, "$baseUrl$key") } } From 27765be4408312ce5a894cce83499ad76a90ad3a Mon Sep 17 00:00:00 2001 From: Arpita Patel Date: Thu, 28 Sep 2023 12:56:37 -0400 Subject: [PATCH 26/26] Improved settings screen message --- .../src/main/resources/messages/skateBundle.properties | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/skate-plugin/src/main/resources/messages/skateBundle.properties b/skate-plugin/src/main/resources/messages/skateBundle.properties index 0303e3871..a1b93bd2a 100644 --- a/skate-plugin/src/main/resources/messages/skateBundle.properties +++ b/skate-plugin/src/main/resources/messages/skateBundle.properties @@ -5,7 +5,7 @@ skate.configuration.enableWhatsNew.title=Enable What's New Panel skate.configuration.enableWhatsNew.description=If enabled, shows the What's New Panel if there are new changes detected in a specified changelog file. skate.modelTranslator.description=Generate translator body skate.configuration.enableFeatureFlagLinking.title=Enable Feature Flag Linking -skate.configuration.enableFeatureFlagLinking.description=If enabled, feature flag URL will be shown in the quick documentation tooltip. -skate.configuration.featureFlagBaseUrl.title=Enter URL to be added as a prefix to flag annotation -skate.configuration.featureFlagAnnotation.title=Feature flag annotation name -skate.configuration.featureFlagFieldEmpty.error=Field cannot be empty when feature flag Linking is enabled +skate.configuration.enableFeatureFlagLinking.description=If enabled, show inspections for opening feature flag links from corresponding enum entries. +skate.configuration.featureFlagBaseUrl.title=Base URL for feature flag linking. The flag key will be appended to the end of this +skate.configuration.featureFlagAnnotation.title=Feature flag annotation's fully-qualified name (com.example.FeatureFlag) +skate.configuration.featureFlagFieldEmpty.error=Field cannot be empty when feature flag linking is enabled