using rules_scala documented scalafmt support breaks scala dependency discovery

[gergelyfabian]

If you define a scalafmt rule as documented in rules_scala it won't be recognized by intellij plugin as a Scala target, as the rule names for rules_scala are hard-coded.

https://github.com/bazelbuild/rules_scala/blob/master/docs/phase_scalafmt.md

E.g. you define and use scalafmt_scala_binary or scalafmt_scala_library rules, then those won't be recognized as Scala, but rather Java rules.

In effect the external dependencies for such targets are not discovered by IntelliJ.

The reason is that the Scala targets generated by rules_scala do not expose a jdeps interface, hence intellij BlazeJavaWorkspaceImporter cannot find the dependencies for them (assuming they are not in the working set).

I've created a possible fix in gergelyfabian@265d376.

[gergelyfabian]

As rules_scala maintainer, @ittaiz, what do you think about this? Should it be solved the way I proposed in gergelyfabian@265d376, or do we need some changes also/rather to rules_scala, like making the scalafmt functionality into officially supported rule names?
Or maybe the rules_scala documentation should be changed to say that I should create rule names that I know that the plugin would recognize:

load("//scala:advanced_usage/scala.bzl", "make_scala_binary", "make_scala_library")
load("//scala/scalafmt:phase_scalafmt_ext.bzl", "ext_scalafmt")

scala_binary = make_scala_binary(ext_scalafmt)
scala_library = make_scala_library(ext_scalafmt)

And then use these alternative scala_library/scala_binary rules...

Anyhow, I think the root cause is that intellij plugin in hard coding the expected rules_scala rule names, while rules_scala provides a macro interface, that enables you create new scala rules with specific settings (that the Scalafmt docs suggest to use). I'd maybe call it an API mismatch.

[ittaiz]

That’s a great question.
Q- if we would have exposed jdeps then it wouldn’t have mattered how the rule is called (assuming JavaInfo of course)?
If so I think that maybe we should consider exposing jdeps (optionally maybe?).

Main problem is that the IJ team capacity for community related changes like generic support of scala or something is very limited these days (AFAIK)

[gergelyfabian]

Yes, I think jdeps support would solve the issue. It works ok for Java, and Scala rule names are detected as Java by default (if the name happens to be not hard coded in the intellij plugin's Scala rule detection). My example fix any how would easily break if someone would use the rules_scala rule macros in another way, and call the created rules in another way (it fixes the issue for now though). Also, the potential jdeps support in rules_scala would mean, that the intellij plugin wouldn't need to have this hard coded logic for Scala rule name detection, and maybe it could even be treated as general Java targets? In any case it seems to remove complexity in the big picture instead of adding it, moving to a common API. It would be great to have this kind of support in rules_scala.. Ittai Zeidman <[email protected]> ezt írta (időpont: 2020. máj. 15., P 19:55):

…

That’s a great question. Q- if we would have exposed jdeps then it wouldn’t have mattered how the rule is called (assuming JavaInfo of course)? If so I think that maybe we should consider exposing jdeps (optionally maybe?). Main problem is that the IJ team capacity for community related changes like generic support of scala or something is very limited these days (AFAIK) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1824 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJGRRJ2QOTK5JDNNLJBFILRRV6XXANCNFSM4M7SOYBA> .

[ittaiz]

1. I think custom scala support was needed for a few reasons and not only target identification so maybe more will be lost?
2. Need to remember jdeps support doesn’t come for free (impl, maintenance, performance)
3. Sounds like a documentation PR in rules_scala would definitely be valuable for now

[gergelyfabian]

Regarding the documentation PR, what should be the general advice at this moment?

Mention that intellij plugin supports only a set of names, and using custom-named rules, defined by using macros (including the call to add scalafmt support) makes intellij plugin not recognize those as Scala targets? That if you need extended support from rules_scala (including scalafmt), you should rather name those rules the same way as in rules_scala (and use them from your own scope)? Is this the best workaround we have currently? Anything else?

[ittaiz]

I think so.
Anything else you can think of?

[gergelyfabian]

I sent the documentation PR for rules_scala: bazelbuild/rules_scala#1047

[odisseus]

I found a workaround for this issue that does not require forking the entire Bazel plugin.

It is possible to create a tiny plugin that would extend the Bazel plugin with a custom TargetKindProvider which would define the language and rule type for custom kind names.

This should go into plugin.xml:

<extensions defaultExtensionNs="com.google.idea.blaze">
  <TargetKindProvider implementation="com.foobar.bazel.kind.FoobarScalaBlazeRules"/>
</extensions>

The kind provider code could look like this:

package com.foobar.bazel.kind;

import com.google.common.collect.ImmutableSet;
import com.google.idea.blaze.base.model.primitives.Kind;
import com.google.idea.blaze.base.model.primitives.LanguageClass;
import com.google.idea.blaze.base.model.primitives.RuleType;

public class FoobarScalaBlazeRules implements Kind.Provider {

  private static final Kind SCALA_FOOBAR_LIBRARY =
      Kind.Provider.create("_scala_foobar_library", LanguageClass.SCALA, RuleType.LIBRARY);

  @Override
  public ImmutableSet<Kind> getTargetKinds() {
    return ImmutableSet.of(SCALA_FOOBAR_LIBRARY);
  }
}

Of course, you would have to build this plugin and distribute it within your organization, but this is probably easier than maintaining a fork of the Bazel plugin and tracking the upstream changes.

[github-actions]

Thank you for contributing to the IntelliJ repository! This issue has been marked as stale since it has not had any activity in the last 6 months. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-maintainer". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[gergelyfabian]

I think this is still valid.