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

Compiler dependencies with plus one and strict deps #1052

Open
liucijus opened this issue May 28, 2020 · 4 comments
Open

Compiler dependencies with plus one and strict deps #1052

liucijus opened this issue May 28, 2020 · 4 comments
Labels
dep-tracking Strict/unused deps, label collections related issues

Comments

@liucijus
Copy link
Collaborator

In some cases Scala compiler needs some deps to be on the classpath, which are not directly mentioned in the source code of the target. Such deps should come transitively with plus-one. Otherwise it is a coupling on the client side, which will will break if implementation target changes its dependencies. This becomes an issue if such coupling happens to an external dep implementation.

Repro: https://github.com/wix-playground/scala-unused-deps/tree/master/type_annotations

a.Hello depends on b.LoggingProvider, but it also asks to add //type_annotation/c, which is implementation detail of b.LoggingProvider.

If implementation of LoggingProvider is changed to use d.BetterLogger, it requires not only to add d to b, but also d to a, and remove c from a.

@liucijus
Copy link
Collaborator Author

One way to address this problem is to export c on b

@Jamie5
Copy link
Contributor

Jamie5 commented May 28, 2020

Have not actually tested to be for sure but I think the reason you need to add the dependency is because you are calling log on a type of c.Logger, so in fact a's source code does use c directly. If however on b it was declared as val logger: LoggerInterface = c.Logger(), (where LoggerInterface is declared in some package that's neither c nor d), then I think a wouldn't have to depend on c or d.

@ittaiz is this behavior what we want? (it seems reasonable to me personally but not sure if people agree with that).

@Jamie5
Copy link
Contributor

Jamie5 commented May 28, 2020

@liucijus did not quite see the relation to #867 (comment) as you mentioned in the other issue.

@liucijus
Copy link
Collaborator Author

@Jamie5 just want to say that we may need more complex heuristic to decide where the dep belongs in plus-one mode

@liucijus liucijus added the dep-tracking Strict/unused deps, label collections related issues label Mar 3, 2021
gergelyfabian pushed a commit to gergelyfabian/rules_scala that referenced this issue May 31, 2022
With second preview the constructors of records now get the same access
modifier than the class (see JDK-8242479).

For "record WithoutFields" this leads to an private empty default
constructor which is filtered out by PrivateEmptyNoArgConstructorFilter.

Removing private modifiers from records to have consistent results
between first and second preview.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dep-tracking Strict/unused deps, label collections related issues
Projects
None yet
Development

No branches or pull requests

2 participants