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

Add support for SignatureAtPos #135

Merged
merged 5 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
146 changes: 146 additions & 0 deletions decoder/signature.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
package decoder

import (
"bytes"
"fmt"
"strings"

"github.com/hashicorp/hcl-lang/lang"
"github.com/hashicorp/hcl-lang/schema"
"github.com/hashicorp/hcl/v2"
"github.com/hashicorp/hcl/v2/hclsyntax"
)

// SignatureAtPos returns a function signature for the given pos if pos
// is inside a FunctionCallExpr
func (d *PathDecoder) SignatureAtPos(filename string, pos hcl.Pos) (*lang.FunctionSignature, error) {
file, err := d.fileByName(filename)
if err != nil {
return nil, err
}

body, err := d.bodyForFileAndPos(filename, file, pos)
if err != nil {
return nil, err
}

var signature *lang.FunctionSignature
hclsyntax.VisitAll(body, func(node hclsyntax.Node) hcl.Diagnostics {
if !node.Range().ContainsPos(pos) {
return nil // Node not under cursor
}
Comment on lines +28 to +31
Copy link
Member

Choose a reason for hiding this comment

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

This is smart, but it doesn't take the schema into account, which means that we provide signature help in all contexts/expressions, regardless of whether function calls are expected there or not.

In most contexts, users will less likely type the function name - where it would be nitpicking, but TypeDeclaration also uses FunctionCallExpr to express various types like list, set etc. and we do want to avoid the signature help there, or do we not? Otherwise we'd end up with signature help e.g. representing the list() function, rather than the type, wouldn't we?

I wouldn't expect you to account for nested expressions (although that would probably be a long-term ideal we can 1st solve that in AnyExpression and then maybe reuse here later) nor prevent signatureHelp for mismatching types.

I think that we could still check whether the outermost constraint for the expression is AnyExpression though?

Copy link
Member Author

Choose a reason for hiding this comment

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

With the current implementation, the feature is decoupled from any expression work and can work stand-alone. Also, nested expressions/function calls should work out of the box, as we always try to find the specific FunctionCallExpr node.

There is no overlap between the functions in the schema and the type declarations. So, even though list( or tuple( is technically a FunctionCallExpr, no signature help should come up. Only for Terraform < 0.15 do we have the deprecated list and map functions which will trigger signature help. Is it worth extending the code to account for that? Adding schema and constraint checks while walking the AST sounds expensive.

Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't think it's expensive in terms of performance (not significantly more than the implementation of hclsyntax.VisitAll). I can totally see it being expensive in terms of effort/complexity though and I can see why this is worth shipping as-is and how it can deliver immense value even if we don't account for these edge cases. 👍🏻

So with that in mind I'd just leave a note either in a comment, or in a new issue to help us remember to revisit this and/or explain why we took a slightly different approach here which doesn't take schema into account and call the PR done.


fNode, isFunc := node.(*hclsyntax.FunctionCallExpr)
if !isFunc {
return nil // No function call expression
}

f, ok := d.pathCtx.Functions[fNode.Name]
if !ok {
return nil // Unknown function
}

if len(f.Params) == 0 && f.VarParam == nil {
signature = &lang.FunctionSignature{
Name: fmt.Sprintf("%s(%s) %s", fNode.Name, parameterNamesAsString(f), f.ReturnType.FriendlyName()),
Description: lang.Markdown(f.Description),
}

return nil // Function accepts no parameters, return early
}

pRange := hcl.RangeBetween(fNode.OpenParenRange, fNode.CloseParenRange)
if !pRange.ContainsPos(pos) {
return nil // Not inside parenthesis
}

activePar := 0 // default to first parameter
foundActivePar := false
lastArgEndPos := fNode.OpenParenRange.Start
lastArgIdx := 0
for i, v := range fNode.Args {
// We overshot the argument and stop
if v.Range().Start.Byte > pos.Byte {
break
}
if v.Range().ContainsPos(pos) || v.Range().End.Byte == pos.Byte {
activePar = i
foundActivePar = true
break
}
lastArgEndPos = v.Range().End
lastArgIdx = i
}

if !foundActivePar {
recoveredBytes := recoverLeftBytes(file.Bytes, pos, func(byteOffset int, r rune) bool {
return r == ',' && byteOffset > lastArgEndPos.Byte
})
trimmedBytes := bytes.TrimRight(recoveredBytes, " \t\n")
if string(trimmedBytes) == "," {
activePar = lastArgIdx + 1
}
}

paramsLen := len(f.Params)
if f.VarParam != nil {
paramsLen += 1
}

if activePar >= paramsLen && f.VarParam == nil {
return nil // too many arguments passed to the function
}

if activePar >= paramsLen {
// there are multiple variadic arguments passed, so
// we want to highlight the variadic parameter in the
// function signature
activePar = paramsLen - 1
}

parameters := make([]lang.FunctionParameter, 0, paramsLen)
for _, p := range f.Params {
parameters = append(parameters, lang.FunctionParameter{
Name: p.Name,
Description: lang.Markdown(p.Description),
})
}
if f.VarParam != nil {
parameters = append(parameters, lang.FunctionParameter{
Name: f.VarParam.Name,
Description: lang.Markdown(f.VarParam.Description),
})
}
signature = &lang.FunctionSignature{
Name: fmt.Sprintf("%s(%s) %s", fNode.Name, parameterNamesAsString(f), f.ReturnType.FriendlyName()),
Description: lang.Markdown(f.Description),
Parameters: parameters,
ActiveParameter: uint32(activePar),
}

return nil // We don't want to add any diagnostics
})

return signature, nil
}

// parameterNamesAsString returns a string containing all function parameters
// with their respective types.
//
// Useful for displaying as part of a function signature.
func parameterNamesAsString(fs schema.FunctionSignature) string {
paramsLen := len(fs.Params)
if fs.VarParam != nil {
paramsLen += 1
}
names := make([]string, 0, paramsLen)

for _, p := range fs.Params {
names = append(names, fmt.Sprintf("%s %s", p.Name, p.Type.FriendlyName()))
}
if fs.VarParam != nil {
names = append(names, fmt.Sprintf("…%s %s", fs.VarParam.Name, fs.VarParam.Type.FriendlyName()))
}

return strings.Join(names, ", ")
}
Loading