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

FR: Gazelle resolve class directives #155

Open
caseyduquettesc opened this issue Feb 28, 2023 · 1 comment
Open

FR: Gazelle resolve class directives #155

caseyduquettesc opened this issue Feb 28, 2023 · 1 comment

Comments

@caseyduquettesc
Copy link
Contributor

For the most part, resolving dependencies by packages works pretty well and the cases where it does not aren't hard to workaround, however, there's one use case I found where it would be a much better user experience if class-level resolve directives were supported.

For a concrete example, I'll be using everyone's favorite (sarcasm), lombok

# gazelle:resolve java lombok //:lombok

java_plugin(
    name = "lombok_plugin",
    generates_api = 1,
    processor_class = "lombok.launch.AnnotationProcessorHider$AnnotationProcessor",
    deps = [artifact("org.projectlombok:lombok")],
)

java_library(
    name = "lombok",
    exported_plugins = [":lombok_plugin"],
    neverlink = 1,
    visibility = ["//visibility:public"],
    exports = [artifact("org.projectlombok:lombok")],
)

So, here we have declared that any importer of lombok.* shall require //:lombok instead of @maven//:org_projectlombok_lombok. This mostly works, but what happens when a package tries to use the CustomLog annotation?

import lombok.CustomLog;

@CustomLog
public class MyClass {}

CustomLog injects a logger of a class specified in a config file according to the javadoc.

image

So let's say we want to inject a logger of type com.mycompany.myapp.logger.AppLogger

lombok.log.custom.declaration = com.mycompany.myapp.logger.AppLogger

What I would normally do in this situation is add a dependency on com.mycompany.myapp.logger.AppLogger to //:lombok, however our logging class depends on a StructuredLogging class (log lines), which depends on lombok.{Builder,Value}, so that would create a cycle in the graph.

Barring any refactoring, what I'd like to be able to do is say "any imports of lombok.CustomLog resolve to labelA and any imports of lombok.* resolve to labelB"

# gazelle:resolve java lombok.CustomLog //:logging
# gazelle:resolve java lombok //:lombok
@illicitonion
Copy link
Collaborator

In general, I think doing class rather than package based resolution where we can makes sense. I say where we can because we can't always reliably do so, the major exceptions that come to mind are:

  1. Generated code - both from things like annotation processers, and from things like protoc - we can't reliably know what classes some code may generate.
  2. Third party libraries - we rely on rules_jvm_external's lockfile which indexes a list of packages (e.g. in this example) - while we could have rules_jvm_external list out every class in every jar, that would make the lockfile huge (and also very churn-y when versions change).

Right now in our code, we treat packages and classes interchangeably quite often - #153 is a step towards not doing that so much - it introduces specific types for packages and classes, and makes clear where each is being used. It also documents that this gazelle plugin assumes quite strongly that people follow standard java naming conventions. Once that lands, I think being able to support class-specific resolves, even without doing general class-based resolution and keeping with packages as the primary resolution key, would be quite simple.

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

No branches or pull requests

2 participants