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

Implement dependency add API (still private) for projects. #6849

Conversation

sdedic
Copy link
Member

@sdedic sdedic commented Dec 14, 2023

I am a little pressed by the time ... so I've split work into several PRs. Fixes for bugs and enhancements that could be separated from the feature itself were filed as separate PRs:

These commits were picked up to the sdedic/feature/project-dependency-add_base branch in the Apache NB repository. I expect these separate PRs to be merged first - then I'll update the _base' branch to the updated master. That branch will serve as the feature branch and will be eventually merged to master` after testing and fixing.

This PR serves as diff for the feature itself. Outline of what was done:

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.

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 Gradle [ci] enable "build tools" tests Maven [ci] enable "build tools" tests VSCode Extension [ci] enable VSCode Extension tests labels Dec 14, 2023
@sdedic sdedic added this to the NB21 milestone Dec 14, 2023
@sdedic sdedic self-assigned this Dec 14, 2023
@sdedic sdedic force-pushed the sdedic/feature/project-dependency-add branch from e434f04 to 79622b0 Compare December 14, 2023 21:52
@apache apache locked and limited conversation to collaborators Dec 14, 2023
@apache apache unlocked this conversation Dec 14, 2023
@sdedic sdedic added LSP [ci] enable Language Server Protocol tests Java [ci] enable extra Java tests (java.completion, java.source.base, java.hints, refactoring.java, form) labels Dec 14, 2023
@sdedic sdedic changed the title Implement dependency add API for projects. Implement dependency add API (still private) for projects. Dec 15, 2023
@sdedic
Copy link
Member Author

sdedic commented Dec 15, 2023

Note the DocumentChangesConverter might be a candidate to be promoted to a lsp.api or refactoring.api (should the refactoring adopt lsp.api as a bridge) utility: it converts a series of changes made to a Swing Document to a proper LSP API stream of non-overlapping TextEdits.

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.

We will build from apache:sdedic/feature branch and test against it before merging into master.

@sdedic
Copy link
Member Author

sdedic commented Dec 15, 2023

I wonder how is it possible that the tests succeeded for me locally for JDK17 only, while they fail same as on CI with JDK 11+21 ... there were clearly missing dependencies for xdm and editor.lib modules ... but they were missing regardless of the JDK version. Miracle !

@sdedic sdedic force-pushed the sdedic/feature/project-dependency-add branch from 4494b4f to 8dc6ffe Compare December 17, 2023 21:29
@sdedic
Copy link
Member Author

sdedic commented Dec 17, 2023

branch sdedic/feature/project-dependency-add_base updated from master. This PR was rebased on the _base's head.

@sdedic sdedic merged commit b10b1f8 into apache:sdedic/feature/project-dependency-add_base Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

2 participants