Skip to content

Ability to set packages and/or directories that should be indexed by analysis server for code completion #35680

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

Closed
lambdabaa opened this issue Jan 16, 2019 · 7 comments
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@lambdabaa
Copy link
Contributor

lambdabaa commented Jan 16, 2019

Currently analysis server only suggests code completions which are already imported. For example, if I have two sibling classes Foo and Bar and ask analysis server for completions on Ba^ from foo.dart before importing bar.dart, Bar will not be suggested. Likewise, if I ask for Proces, Process will not be suggested from dart:io unless I've already imported it.

In order to have Java parity in IntelliJ, we need to be able to do this, and code completion will be much more useful overall with this ability.

The additional API I would like is something along the lines of completion.setActiveTargets(List<String> targets).

/cc @jwren @scheglov @bwilkerson

@lambdabaa lambdabaa added the legacy-area-analyzer Use area-devexp instead. label Jan 16, 2019
@scheglov
Copy link
Contributor

scheglov commented Jan 17, 2019

Some additional clarification.

If both files are in the same project, or even in analysis roots set in DAS, then these folders are already know to DAS, and we could provide completion suggestions from them. Same is for .package file - we parse these files, and can learn about additional directories with Dart files from which we can give suggestions.

The problem starts when we live in mono-repository, where in theory everything is available for completion. In this case we require the client to provide additional directories for DAS to parse and provide suggestions from. It is OK is these new directories will have some intersection with analysis roots, we always can merge two sets.

@bwilkerson
Copy link
Member

The additional API I would like ...

Are you asking for a server protocol, or a method on some class (and if the latter, which class)?

Independent of the actual API, what I think you're asking for is a way to set the scope for finding names to suggest. It isn't clear to me that we ought to allow this to be set. The alternative is that we pick the "right" scope and just use it everywhere without the need/ability for it to be set externally.

There are a few scopes that I can think of, each described as a delta from the previous scope:

  • the name scope of the enclosing library (the names declared in the library and the names that are imported), which is what we currently use,
  • all of the names from the transitive closure of libraries reachable from the enclosing library,
  • all of the names declared in the current package (the package containing the enclosing library),
  • all of the names in all packages that the current package depends on,
  • all of the names in the workspace containing the current package.

So which of those scopes would match our user's expectations?

The problem starts when we live in mono-repository, where in theory everything is available for completion.

I think we can solve that problem by choosing a scope that is smaller than the whole workspace.

@lambdabaa
Copy link
Contributor Author

Are you asking for a server protocol, or a method on some class (and if the latter, which class)?

A new protocol message {"method": "completion.setActiveTargets", "params": {"targets": List<String>}}.

Independent of the actual API, what I think you're asking for is a way to set the scope for finding names to suggest. It isn't clear to me that we ought to allow this to be set. The alternative is that we pick the "right" scope and just use it everywhere without the need/ability for it to be set externally.

Yes. What I really want is for all of my declared dependencies (whether pub or bazel) to show up in code completion results. If analysis server is able to make that determination, that's even better :).

So which of those scopes would match our user's expectations?

My expectation coming from Java development with IntelliJ + Bazel is that there's a build file which declares my project's sources and dependencies. A dependency might be a package or a library. Any interfaces, classes, etc defined in one of my sources or dependencies would be available in code completion, but I don't expect that the transitive closure (dependencies' dependencies) will show up as code completion suggestions.

@bwilkerson
Copy link
Member

I think we agree. That sounds like the most reasonable choice.

But just to be a little more precise, I think we want to suggest

  • all of the names declared in the current library,
  • all of the public names defined in other libraries in the current package, and
  • all of the public names from public libraries in the packages that the current package depends on directly.

@zoechi
Copy link
Contributor

zoechi commented Jan 17, 2019

Related to #25820, #21931

@stereotype441 stereotype441 added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Jan 17, 2019
@lambdabaa
Copy link
Contributor Author

We're developing the analysis server protocol changes in https://dart-review.googlesource.com/c/sdk/+/90125.

dart-bot pushed a commit that referenced this issue Jan 31, 2019
interest in receiving not yet imported code completions from specific
packages and libraries.

The design of this interaction aims to avoid forcing analysis server
to send every public symbol from dependent packages at every completion.
Instead, clients express interest in certain packages and/or libraries
and analysis server will notify with the entire set of symbols for
each of them only once. Then it is the client's responsibility to save
these pre-computed completions in memory, and analysis server can
instruct the client on how to union them with the base set of completion
suggestions at code completion time.

Bug: #35680
Change-Id: I42c6fddc5d8daa2b546daa81456c0992e3ef547d
Reviewed-on: https://dart-review.googlesource.com/c/90125
Commit-Queue: Ari Aye <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
@lambdabaa
Copy link
Contributor Author

We did it, thanks @bwilkerson and @scheglov for all of your work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants