Skip to content
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.

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

Add SemanticDB support #1329

Closed
wants to merge 1 commit into from
Closed

Conversation

olafurpg
Copy link
Contributor

@olafurpg olafurpg commented Jan 7, 2022

Description

This PR adds an flag to enable the SemanticDB compiler plugin during Scala compilation.

The SemanticDB compiler adds overhead to compilation so it should be disabled by default. The goal of this PR is to make it easy for users to dynamically enable SemanticDB in certain environments like CI via Bazel config options or system environment variables.

Motivation

SemanticDB is used by several tools like Scalafix and Metals so it would be nice to have this functionality included out-of-the-box with rules_scala. Example use-cases include

@olafurpg
Copy link
Contributor Author

olafurpg commented Jan 7, 2022

I'm opening this PR as a draft because it's not working correctly yet. I'm hitting on the following error that I don't understand:

❯ cd examples/testing/scalatest_repositories && bazel test //...
ERROR: /private/var/tmp/_bazel_olafurpg/59302ffe484af653f34e0ae041d042f7/external/io_bazel_rules_scala/scala/support/BUILD:4:14: in (an implicit dependency) attribute of scala_library rule @io_bazel_rules_scala//scala/support:test_reporter: '@io_bazel_rules_scala//scala/private/toolchain_deps:semanticdb_scalac' does not produce any scala_library $semanticdb_scalac_plugin files (expected .jar)
ERROR: Analysis of target '//example:example' failed; build aborted: Analysis of target '@io_bazel_rules_scala//scala/support:test_reporter' failed
INFO: Elapsed time: 0.119s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (0 packages loaded, 0 targets configured)
FAILED: Build did NOT complete successfully (0 packages loaded, 0 targets configured)

Any advice on how to debug that?

@liucijus
Copy link
Collaborator

It's probably bootstrapping problem (can't build rules scala targets before rules scala is built). You can try doing similar thing like dependecy analyzer does using scala_library_for_plugin_bootstrapping

❯ cd examples/testing/scalatest_repositories && bazel test //...
ERROR: /private/var/tmp/_bazel_olafurpg/59302ffe484af653f34e0ae041d042f7/external/io_bazel_rules_scala/scala/support/BUILD:4:14: in (an implicit dependency) attribute of scala_library rule @io_bazel_rules_scala//scala/support:test_reporter: '@io_bazel_rules_scala//scala/private/toolchain_deps:semanticdb_scalac' does not produce any scala_library $semanticdb_scalac_plugin files (expected .jar)

@liucijus
Copy link
Collaborator

A few questions:

  • Can it be a separate phase?
  • What about Scala 3 support?
  • Is SemanticDB output reproducible?

Thanks for the effort, @olafurpg!

@olafurpg
Copy link
Contributor Author

@liucijus Thank you for taking a look 🙏 I'll investigate scala_library_for_plugin_bootstrapping

Can it be a separate phase?

My eventual hope is that we can add dedicated semanticdb() target types that only emit SemanticDB to maximize build parallelism and caching. I elaborate a bit on the motivation for using that approach in the comment here https://github.com/sourcegraph/sourcegraph/issues/2982#issuecomment-1006670213 Maybe scala_library could be configured to automatically generate these semanticdb targets, which can have the attribute that hides them from bazel build //... commands (I can't remember what the attribute is called)

What about Scala 3 support?

Scala 3 has built-in support for SemanticDB with the -Xsemanticdb compiler option. It would be nice if rules_scala was able to transparently use the correct options for Scala 2 (-Xplugin:/path/to/semanticdb.jar) and Scala 3 (-Xsemanticdb).

Is SemanticDB output reproducible?

Good question. It should be as long as the typechecked Scala trees are reproducible (it's a bug otherwise). If you call a macro that generates non-reproducible output then the generated SemanticDB might also have minor deviations between compilations.

@liucijus
Copy link
Collaborator

Scala 3 has built-in support for SemanticDB with the -Xsemanticdb compiler option. It would be nice if rules_scala was able to transparently use the correct options for Scala 2 (-Xplugin:/path/to/semanticdb.jar) and Scala 3 (-Xsemanticdb).

We can get this working by checking Scala version in Bazel macros, like here: https://github.com/bazelbuild/rules_scala/blob/master/third_party/dependency_analyzer/src/main/BUILD#L25

@liucijus
Copy link
Collaborator

My eventual hope is that we can add dedicated semanticdb() target types that only emit SemanticDB to maximize build parallelism and caching. I elaborate a bit on the motivation for using that approach in the comment here sourcegraph/sourcegraph#2982 (comment) Maybe scala_library could be configured to automatically generate these semanticdb targets, which can have the attribute that hides them from bazel build //... commands (I can't remember what the attribute is called)

Will having a separate target for SemanticDB output require running scalac again? I guess a configurable phase is more flexible approach as it allows to opt in features in scala_library.

In Bazel builds output needed by tools is usually generated using aspects. But I'm not sure if this can work with plugins, but would be a good area to research as it would be idiomatic approach.

@liucijus
Copy link
Collaborator

Good question. It should be as long as the typechecked Scala trees are reproducible (it's a bug otherwise). If you call a macro that generates non-reproducible output then the generated SemanticDB might also have minor deviations between compilations.

We must be very careful here

@ricochet1k
Copy link
Contributor

I have a modified version of this PR that works (writes semanticdb output to bazel-bin) here: https://github.com/ThriveFinancial/rules_scala/tree/mpeterson/semanticdb

@mackenziestarr
Copy link
Contributor

hey @ricochet1k @olafurpg do either of you have plans to continue working on this? i tested out @ricochet1k's changes and they seemed to work for me - maybe we aren't too far from productionizing this as a feature, i'd be happy to pitch in as well

@crt-31
Copy link
Contributor

crt-31 commented Nov 30, 2023

This ticket can probably be closed. SemanticDB support was added in #1508

@liucijus liucijus closed this Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants