Skip to content

Implement BuildToolModuleHandler for Scala Projects to register Bazel Imported Scala Projects #7704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Implement BuildToolModuleHandler for Scala Projects to register Bazel Imported Scala Projects #7704

wants to merge 6 commits into from

Conversation

Friendseeker
Copy link

@Friendseeker Friendseeker commented May 27, 2025 •
edited
Loading

Checklist

  • I have filed an issue about this change and discussed potential changes with the maintainers.
  • I have received the approval from the maintainers to make this change.
  • This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: #7703

Description of this change

Use BuildToolModuleHandler API to register that the project is already imported via Bazel.

Before the change

image

After the change

image

@google-cla Google CLA
Copy link

google-cla bot commented May 27, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added product: IntelliJ IntelliJ plugin awaiting-review Awaiting review from Bazel team on PRs labels May 27, 2025
@Friendseeker Friendseeker changed the title Register Implement BuildToolModuleHandler for Scala Projects May 27, 2025
@Friendseeker Friendseeker changed the title Implement BuildToolModuleHandler for Scala Projects Implement BuildToolModuleHandler for Scala Projects to register Bazel Imported Scala Projects May 27, 2025
@Friendseeker Friendseeker changed the title Implement BuildToolModuleHandler for Scala Projects to register Bazel Imported Scala Projects Implement BuildToolModuleHandler for Scala Projects to register Bazel Imported Scala Projects May 27, 2025
@odisseus odisseus removed the awaiting-review Awaiting review from Bazel team on PRs label May 28, 2025
@Friendseeker
Initial Implementation
6188259
@Friendseeker
Add adapters for BuildToolModuleHandlerAdapter
868320b
@Friendseeker
Add #api251 marker
1a3dfca
@Friendseeker
Add newline at end of files
4170a78
@Friendseeker
Add compatibility shim for intellij-oss-oldest-stable
3a8cee4
@Friendseeker
Amend #api251 marker to #api250
f892786 
It turned out #api251 Intellij already has BuildToolModuleHandler

If v251 shim do not implement BuildToolModuleHandler, an runtime would be thrown

com.intellij.diagnostic.PluginException: Extension com.google.idea.blaze.scala.BlazeScalaBuildToolModuleHandler does not implement interface
  org.jetbrains.sbt.project.BuildToolModuleHandler
  (adapter=SimpleConstructorInjectionAdapter(implementation=com.google.idea.blaze.scala.BlazeScalaBuildToolModuleHandler, plugin=PluginDescriptor(name=Bazel for
  IntelliJ, id=com.google.idea.bazel.ijwb, descriptorPath=plugin.xml, path=~/Library/Application Support/JetBrains/IntelliJIdea2025.1/plugins/ijwb,
  version=9999-api-version-251, package=null, isBundled=false)))
      at com.intellij.serviceContainer.ComponentManagerImpl.createError(ComponentManagerImpl.kt:989)
      at com.intellij.openapi.extensions.impl.ExtensionPointImpl.checkExtensionType(ExtensionPointImpl.kt:210)
      at com.intellij.openapi.extensions.impl.ExtensionPointImpl.processAdapter(ExtensionPointImpl.kt:428)
      at com.intellij.openapi.extensions.impl.ExtensionPointImpl.createExtensionInstances(ExtensionPointImpl.kt:377)
      at com.intellij.openapi.extensions.impl.ExtensionPointImpl.getExtensionList(ExtensionPointImpl.kt:223)
      at com.intellij.openapi.extensions.ExtensionPointName.getExtensionList(ExtensionPointName.kt:54)
      at org.jetbrains.sbt.project.BuildToolModuleHandler.isHandledByBuildTool(BuildToolModuleHandler.java:15)
      at org.jetbrains.sbt.project.SbtProjectUtil$.isHandledByOtherBuildTool(SbtProjectUtil.scala:34)
      at org.jetbrains.sbt.project.SbtProjectUtil$.isInSbtProject(SbtProjectUtil.scala:29)
      at org.jetbrains.sbt.project.SbtProjectImportStateService.isImported(SbtProjectImportStateService.scala:31)
      at org.jetbrains.sbt.codeInsight.daemon.SbtProjectImportStateProblemHighlightFilter.shouldHighlight(SbtProjectImportStateProblemHighlightFilter.scala:23)/
@odisseus odisseus requested a review from sellophane May 29, 2025 14:15
@sellophane
Copy link
Collaborator

Is buildToolModuleHandler endpoint already present in 243? Because if not, then I'm quite surprised that all tests are green...

@odisseus odisseus self-requested a review May 29, 2025 16:39
@Friendseeker
Copy link
Author

Friendseeker commented May 29, 2025 •
edited
Loading

Is buildToolModuleHandler endpoint already present in 243? Because if not, then I'm quite surprised that all tests are green...

BuildToolModuleHandler is added to address the ticket https://youtrack.jetbrains.com/issue/SCL-23350

The ticket has fix version 2025.1.516

The exact commit JetBrains/intellij-scala@e7a7738 is also only in idea251.x branch.

Therefore, it should not be present in 243.

If there's any compatibility concern, I can downgrade my IntelliJ idea to 243 and do manual testing. Are there any particular scenarios I should test?

This is my first time developing patch for a Jetbrains IDE plugin. I don't know about the best practice around compatibility, therefore would appreciate any guidance from you or other reviewers.

LeFrosch
LeFrosch requested changes May 30, 2025
View reviewed changes
sdkcompat/v252/BUILD
@@ -34,6 +34,10 @@ java_library(
]),
intellij_ue = glob([
"com/google/idea/sdkcompat/javascript/**", #api223
"com/google/idea/sdkcompat/scala/**", #api250
Copy link
Collaborator

@LeFrosch LeFrosch May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API tag seems wrong here, please use <version where the API changed> - 1. So that if we drop the previous version we can just grep for the API tag.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly the API was introduced in 251, please use 243 as the API tag.

sdkcompat/v252/com/google/idea/sdkcompat/scala/BuildToolModuleHandlerAdapter.java
/**
* Compatibility shim for BuildToolModuleHandler interface.
*/
public interface BuildToolModuleHandlerAdapter extends BuildToolModuleHandler {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the correct API tag to the interface as well.

@LeFrosch
Copy link
Collaborator

Also regarding the concern from @sellophane. If the extension point buildToolModuleHandler is not available in 243 and we register it unconditionally in the xml, this might prevent the scala part from loading. Furthermore, it seems that most other extensions are registered in scala/src/META-INF/scala-contents.xml rather then scala/src/META-INF/blaze-scala.xml.

If this is the case and we cannot register this extension in 243, you would need to register it manually like we did it in CLion for some time. For example:

intellij/clwb/sdkcompat/v243/com/google/idea/blaze/clwb/CLionNotificationProvider.kt

Lines 102 to 114 in 9443d50

@JvmStatic
fun register(project: Project) {
unregisterGenericProvider(project)
val projectFixes = ProjectFixesProvider.Companion.EP_NAME.point
projectFixes.registerExtension(CLionNotificationProvider())
val projectNotifications = EditorNotificationWarningProvider.Companion.EP_NAME.point
projectNotifications.registerExtension(CLionNotificationProvider())
val widgetStatus = WidgetStatusProvider.Companion.EP_NAME.point
widgetStatus.registerExtension(CLionNotificationProvider())
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LeFrosch LeFrosch LeFrosch requested changes

@mai93 mai93 Awaiting requested review from mai93 mai93 is a code owner

@ujohnny ujohnny Awaiting requested review from ujohnny ujohnny is a code owner

@agluszak agluszak Awaiting requested review from agluszak agluszak is a code owner

@sellophane sellophane Awaiting requested review from sellophane

@odisseus odisseus Awaiting requested review from odisseus

Requested changes must be addressed to merge this pull request.

Assignees

@sellophane sellophane

Labels
product: IntelliJ IntelliJ plugin
Projects
Bazel IntelliJ Plugin
Status: Untriaged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Friendseeker @sellophane @LeFrosch @odisseus