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

Project Dependency modification (private) API #6868

Merged
merged 15 commits into from
Jan 5, 2024

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Dec 20, 2023

This PR is an iteration after private testing (closed) #6849 and should land in master branch. Side PRs are already part of the master, this PR contains consolidated changes from sdedic/feature/project-dependencies-add_base branch. I'll copy the description of the original PR here:


Overview and heads-up

The project.dependencies API is still private - intentionally. The plan was to first implement a prototype client as a validation of the APIs, then publish. This PR adds a prototype of abstract "add dependency" operation tot he APIs.

I'll implement remove operation next. I think still need to (somehow) address the BOM and/or dependency-management concept. As a "Saturday project", I still plan to convert Maven graph dependency viewer into a general dependency viewer, so it will work for gradle too.

I do not expect the project dependency API to mature in NB 21 timeframe; the target is NB 23.

Gradle experimental module

In order to implement project modification, I've added gradle.dependencies to Java cluster - this module has some dependencies that do not fit for gradle.java - and I do not want to mess the dependencies further. As the impementation progressed, it turned out that it could be beneficial to clean up gradle.java and move the dependency query implementation to gradle.dependencies as well.

Breaking change in a private API

The Scope had abstract operations so the client can evaluate that some reported scope actually propagates to an abstract one, like Scopes.COMPILE. Imagine that the client gets a dependency declared in gradle's compileOnly and is interested to find out if it makes it to runtime. But it seems more useful to delegate this evaluation to the buildsystem-specific provider instead. In this PR I removed the implies and includes metods from the Scope, which become a simple extensible enum only.

This breaking change was retrofitted to implementations. The API is not even friend, so no official compatibility was broken. The API will evolve as private for some time to incorporate feedback from early adopters and comments.

Concept overview

The operation is described in API as a refactoring: a ModificationResult is produced, with no changes to the documents and files. Since textual difference structures are only part of java APIs, I decided to use LSP API structures, that wel describe intended modifications: otherwise I would more or less copied them over.
I propose using LSP APIs as a general communication mechanism, and even promote their usage to refactoring.api itself.
So the result of dependency modification is a ProjectModificationResult that can be either commit()ed by IDE clients, or presented as a preview using WorkspaceEdit structure - and possibly it can be applied by a LSP client.

Dependency add

Currently add and remove is defined, and only add implemented. Add or remove can add/remove a list of dependencies. Either with specific version, or omit the version - this is where the BOM included in the project is useful, but currently the client has no way how to even check that it can omit the version, or resolve an appropriate version -- I'll think about that in later phases and possibly add.

The dependency add may fail if a dependency is already defined. It is possible to include optiosn to ignore such duplicates, so dependencies not present are added and those already present silently ignored.

LSP operations

Two LSP commands are implemented:

  • nbls.project.dependencies.find that searches for dependencies in specified scope(s) and report dependencies that can reach to those scopes. Optionally with all children. The operation utilizes the ProjectDependencies.findDependencies API.
  • nbls.project.dependencies.change that acutally modifies the dependencies.

@sdedic sdedic added API Change [ci] enable extra API related tests Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) Gradle [ci] enable "build tools" tests LSP [ci] enable Language Server Protocol tests Maven [ci] enable "build tools" tests VSCode Extension [ci] enable VSCode Extension tests enterprise [ci] enable enterprise job labels Dec 20, 2023
@sdedic sdedic added this to the NB21 milestone Dec 20, 2023
@sdedic sdedic self-assigned this Dec 20, 2023
Copy link
Contributor

@MartinBalin MartinBalin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested by me and others. Works!

@sdedic sdedic requested a review from lahodaj January 2, 2024 13:45
@sdedic sdedic force-pushed the sdedic/feature/project-dependency-add_base2 branch from cd86854 to 575448b Compare January 4, 2024 16:00
@sdedic
Copy link
Member Author

sdedic commented Jan 4, 2024

Rebased on latest master + adapted to maven's changes in master.

Copy link
Contributor

@dbalek dbalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me.

Copy link
Contributor

@lahodaj lahodaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable.

@sdedic sdedic force-pushed the sdedic/feature/project-dependency-add_base2 branch from 19372de to a5b7e7f Compare January 5, 2024 15:21
@sdedic
Copy link
Member Author

sdedic commented Jan 5, 2024

Sorry, the last commit (adapting to concurrent changes in master) was missing from the rebase :-/

@sdedic sdedic merged commit 041c2c8 into master Jan 5, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Change [ci] enable extra API related tests enterprise [ci] enable enterprise job Gradle [ci] enable "build tools" tests Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) LSP [ci] enable Language Server Protocol tests Maven [ci] enable "build tools" tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants