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

Typing () returns wrong set of signatures (textDocument/signatureHelp) #1015

Merged
merged 1 commit into from
Apr 30, 2019

Conversation

snjeza
Copy link
Contributor

@snjeza snjeza commented Apr 29, 2019

Fixes #1009

Signed-off-by: Snjezana Peco snjezana.peco@redhat.com

@snjeza snjeza requested review from gorkem and fbricon April 29, 2019 17:44
@@ -80,6 +88,37 @@ public SignatureHelp signatureHelp(TextDocumentPositionParams position, IProgres
return help;
}

private boolean isValid(ICompilationUnit unit, int offset, IProgressMonitor monitor) {
CompilationUnit ast = CoreASTProvider.getInstance().getAST(unit, CoreASTProvider.WAIT_YES, monitor);
Copy link
Contributor

Choose a reason for hiding this comment

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

instead of traversing the whole document, couldn't we find the closest enclosing node instead?

Copy link
Contributor Author

@snjeza snjeza Apr 29, 2019

Choose a reason for hiding this comment

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

I have updated the PR.

public boolean visit(MethodRef node) {
if (node.getStartPosition() <= offset && (node.getStartPosition() + node.getLength()) >= offset) {
valid[0] = true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

once we found something, we should stop visiting the AST

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@Override
public void endVisit(MethodInvocation node) {
if (node.getStartPosition() <= offset && (node.getStartPosition() + node.getLength()) >= offset) {
valid[0] = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

will this method be called after visit(MethodRef) or they're really for 2 different node types?

Copy link
Contributor Author

@snjeza snjeza Apr 29, 2019

Choose a reason for hiding this comment

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

They are different node types.

org.eclipse.jdt.core.dom.MethodInvocation
Method invocation expression AST node type.
org.eclipse.jdt.core.dom.MethodRef
AST node for a method or constructor reference within a doc comment (Javadoc). The principal uses of these are in "@see" and "@link" tag elements, for references to method and constructor members.

I have added a new test.

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't need signature help in javadoc, or do we?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It could be useful.

@snjeza snjeza force-pushed the issue-1009 branch 2 times, most recently from 3a59f30 to 4450d9f Compare April 29, 2019 21:28
@fbricon
Copy link
Contributor

fbricon commented Apr 30, 2019

Signature help stops after the 1st parameter if the current method returns something:
Apr-30-2019 11-23-06

This doesn't happen without the PR

Signed-off-by: Snjezana Peco <snjezana.peco@redhat.com>
@fbricon fbricon merged commit 924ae9f into eclipse-jdtls:master Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typing () returns wrong set of signatures (textDocument/signatureHelp)
2 participants