Inline content module dependencies explicitly indicate plugin or module #1289
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
novotnyr
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR refactors inline content module dependencies to explicitly distinguish between plugin and module references and updates the resolver accordingly.
- Splits
InlineDeclaredModuleV2Dependencyinto two sealed subclasses (PluginandModule) with distincttoStringformats and factory methods. - Adjusts
InlineModuleDescriptorResolverto mapPluginV2DependencyandModuleV2Dependencyto the new subclasses. - Adds and updates tests to cover both inline plugin and module dependency cases.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| intellij-plugin-structure/tests/src/test/kotlin/com/jetbrains/plugin/structure/intellij/plugin/InlineDeclaredModuleV2DependencyTest.kt | Replaced of factory with onPlugin, added onModule test. |
| intellij-plugin-structure/structure-intellij/src/test/kotlin/com/jetbrains/plugin/structure/intellij/plugin/module/InlineModuleDescriptorResolverTest.kt | Added tests for resolver picking plugin vs. module dependencies. |
| intellij-plugin-structure/structure-intellij/src/main/java/com/jetbrains/plugin/structure/intellij/plugin/module/InlineModuleDescriptorResolver.kt | Changed dependency mapping to a when on PluginV2Dependency/ModuleV2Dependency. |
| intellij-plugin-structure/structure-intellij/src/main/java/com/jetbrains/plugin/structure/intellij/plugin/InlineDeclaredModuleV2Dependency.kt | Refactored into sealed class with Plugin and Module data subclasses and new factory methods. |
Comments suppressed due to low confidence (3)
intellij-plugin-structure/structure-intellij/src/main/java/com/jetbrains/plugin/structure/intellij/plugin/InlineDeclaredModuleV2Dependency.kt:56
- [nitpick] The nested subclass name 'Module' shadows the imported
Moduletype and may cause confusion. Consider renaming this subclass to something likeModuleDependencyto avoid ambiguity.
data class Module(
intellij-plugin-structure/structure-intellij/src/main/java/com/jetbrains/plugin/structure/intellij/plugin/InlineDeclaredModuleV2Dependency.kt:82
- The
onPluginfactory method takes aPluginIdwhileonModuletakes aString. For consistency and clarity, use the same type for both IDs (e.g.,String) or clearly alias/convert between them.
fun onPlugin(
intellij-plugin-structure/structure-intellij/src/main/java/com/jetbrains/plugin/structure/intellij/plugin/module/InlineModuleDescriptorResolver.kt:132
- The
else -> itbranch allows unsupportedPluginDependencytypes to pass through silently. Consider filtering or explicitly handling only the expected types to prevent unintended dependencies from slipping into the result.
dependencies += when (it) {
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Inline content module dependencies have explicit indication of plugin or module.
Split the
InlineDeclaredModuleV2Dependencyinto sealed class hierarchy:Pluginfor<plugin>dependency in inline CDATA content module<dependencies>elementModulefor<module>dependency in inline CDATA content module<dependencies>elementAdjust the resolution mechanism as well.