Add IntelliJAspectStrategy for the new IntelliJ aspect - #8356
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new IntelliJ split aspect strategy (intellij_aspect_sdk) to replace the legacy bundled aspect, refactoring AspectStrategy to support multiple strategies via providers. It cleans up the codebase by removing legacy aspect-related classes, custom Kotlin patches, and several local proto definitions in favor of the SDK's protos. Feedback on the changes highlights a compilation error in IntelliJAspectWriter.kt due to a type mismatch in WorkspaceRoot.relativize, and suggests replacing getOrNull() with standard Java orElse(null) in IntelliJAspectStrategy.kt to avoid JVM-specific imports.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| try { | ||
| deployAspectZip( | ||
| workspaceRoot = workspaceRoot.path(), | ||
| relativeDestination = workspaceRoot.relativize(normalized), |
There was a problem hiding this comment.
The WorkspaceRoot.relativize method typically expects a File rather than a Path, and returns a WorkspacePath instead of a standard java.nio.file.Path. Since deployAspectZip expects a Path for relativeDestination, calling workspaceRoot.relativize(normalized) will cause a compilation error. Relativizing on workspaceRoot.path() (which is a Path) resolves this type mismatch.
| relativeDestination = workspaceRoot.relativize(normalized), | |
| relativeDestination = workspaceRoot.path().relativize(normalized), |
| val aspects = Aspects.forRules(toAspectRules(activeLanguages)).mapNotNull { aspect -> | ||
| resolve(project, "${aspect.pkg}/${aspect.file}").map { "$it%${aspect.aspect}" }.getOrNull() | ||
| } |
There was a problem hiding this comment.
Using getOrNull() on Optional requires importing kotlin.jvm.optionals.getOrNull which is JVM-specific and requires Kotlin 1.8+. Using the standard Java orElse(null) is cleaner, more compatible, and allows you to remove the unused import on line 30.
| val aspects = Aspects.forRules(toAspectRules(activeLanguages)).mapNotNull { aspect -> | |
| resolve(project, "${aspect.pkg}/${aspect.file}").map { "$it%${aspect.aspect}" }.getOrNull() | |
| } | |
| val aspects = Aspects.forRules(toAspectRules(activeLanguages)).mapNotNull { aspect -> | |
| resolve(project, "${aspect.pkg}/${aspect.file}").map { "$it%${aspect.aspect}" }.orElse(null) | |
| } |
Adds a new aspect strategy (IntelliJAspectStarategy) for the new Intllij aspect (https://github.com/JetBrains/intellij-aspect).
The new aspect is disabled by default but can be enabled with the
bazel.sync.use.intellij.aspectregistry key or form the command line for testing with--//clwb:use_intellij_aspect=trueflag.