[fix] now sources of thrift dependencies are included as dependencies

[abrams27]

Now thrift dependencies not included in the project are still visible - IntelliJ resolves used dependencies properly

Im collecting sources in aspects, then im using them for thrift libraries as dependencies , and then I'm returning them in thrift language plugin as dependencySources

---

fixes #8

---

testing:

bazel test //server/src/test/java/org/jetbrains/bsp/bazel/server/sync/dependencytree:DependencyTreeTest

& manual & e2e soon....

[lukaszwawrzyk]

Thanks for aspect code cleanup :p

There is one problem here with functionality - dependencies are not transitive, so it will work for 1 level nesting.

As for the implementation, we don't need aspect for it, we already have dependecies (even transitive) and sources. For java it is more complex as these are source jars rather than sources. Also for java the field is already computed by bazel and it is easy to just take it and use.

The correct solution for this issue is to do something similar to I have done initially for java but I had to replace it with current implementation as it was too slow. We should have implementation like this:

Set<URI> dependencySources(TargetInfo target, DependencyTree dependencyTree) {
  var deps = dependencyTree.transitiveDependenciesExceptRootTargets(target)
  return deps.filter(dep -> dep.getKind() == "thrift_something").map(dep -> dep.getSources()).map(toUri);
}

Side note: we may consider if we should send each source as separate uri or group by parent dirs (map(path -> path.getParent()).distinct()).

The whole complecity lies in implementation of DependencyTree. It should build a graph from all TargetInfo instances and using it, respond to queries like transitiveDependenciesExceptRootTargets quickly.

My original implementation was just traversing dependencies using recursive function for each target and it was super slow. Maybe we need to use a similar data structure to the depset that is used in bazel aspects. It might be even enough to compute it on demand instead of what I did initially (for each target), because it will only be requested by thrift targets that are part of project import scope.

Note: Why ExceptRootTargets?
Pardon my paint

The idea is to not traverse tree of dependencies that are already in the root targets, because these dependencies will be included by these targets anyway.

[abrams27]

I see the idea, it makes sense

Thanks for aspect code cleanup :p

There is one problem here with functionality - dependencies are not transitive, so it will work for 1 level nesting.

As for the implementation, we don't need aspect for it, we already have dependecies (even transitive) and sources. For java it is more complex as these are source jars rather than sources. Also for java the field is already computed by bazel and it is easy to just take it and use.
The correct solution for this issue is to do something similar to I have done initially for java but I had to replace it with current implementation as it was too slow. We should have implementation like this:

Set<URI> dependencySources(TargetInfo target, DependencyTree dependencyTree) {
  var deps = dependencyTree.transitiveDependenciesExceptRootTargets(target)
  return deps.filter(dep -> dep.getKind() == "thrift_something").map(dep -> dep.getSources()).map(toUri);
}

yea, but we are getting it for free in aspects - everything is calculated only once in a proper order. We can calculate transitive sources as well

Side note: we may consider if we should send each source as separate uri or group by parent dirs (map(path -> path.getParent()).distinct()).

I would prefer to stay with the files - otherwise we would be in the same situation as we are now - multiple targets in the same directory - only issues. We want to attach only one file not everything in the directory, since potentially it could be veery big, or contain irrelevant information (and IntelliJ knows how to resolve it, yea)

The whole complecity lies in implementation of DependencyTree. It should build a graph from all TargetInfo instances and using it, respond to queries like transitiveDependenciesExceptRootTargets quickly.

My original implementation was just traversing dependencies using recursive function for each target and it was super slow. Maybe we need to use a similar data structure to the depset that is used in bazel aspects. It might be even enough to compute it on demand instead of what I did initially (for each target), because it will only be requested by thrift targets that are part of project import scope.

Note: Why ExceptRootTargets? Pardon my paint The idea is to not traverse tree of dependencies that are already in the root targets, because these dependencies will be included by these targets anyway.

beautiful paint! duplications are not good, but probably we can provide it with duplicates, at the end of the day paths are the same - in dependencies and exported dependencies from imported modules (this is the case for java now anyway afaik)

[abrams27]

one more thing, lets consider this case:

our dependency uses target K which is imported as well to the project but B doesn't directly depend on it. since E won't export K (and its dependencies) because it will be attached just as a source - so we shouldn't exclude all targets roots, but only targets roots directly dependent

[abrams27]

I will wait for #203 and then I'll try to write some e2e test for it