starlark-rust icon indicating copy to clipboard operation
starlark-rust copied to clipboard

Go to definition should be able to handle `ctx.attr(s).foo`

Open MaartenStaa opened this issue 11 months ago • 3 comments

Currently, go to definition for baz in a dotted foo.bar.baz finds a (top-level) struct definition for foo with attribute bar. The implementor of LspContext should be able to provide hints to the LSP about e.g. ctx.attr.foo, which has something of a special meaning in Buck(2) and Bazel, but not necessarily in Starlark itself. This issue is mainly to discuss what the API might look like, as briefly discussed in https://github.com/facebookexperimental/starlark-rust/pull/92#issuecomment-1700739048.

As a first idea, I came up with this:

// on LspContext
fn get_dotted_identifier_resolve_options<'a>(
    &self,
    current_file: &LspUrl,
    within_function: Option<&'a str>,
    segments: &[&'a str],
) -> anyhow::Result<Vec<ResolveOptions<'a>>>;

// outside LspContext
pub struct ResolveOptions<'a> {
    scope: ResolveScope,
    matcher: ResolveMatcher<'a>,
}

pub enum ResolveScope {
    OnlyTopLevel,
    Current,
}

pub enum ResolveMatcher<'a> {
    OneOf(Vec<ResolveMatcher<'a>>),
    Ident(&'a str),
    Assignment {
        lhs: Option<Box<ResolveMatcher<'a>>>,
        rhs: Option<Box<ResolveMatcher<'a>>>,
    },
    FunctionCall {
        function_name: Option<Box<ResolveMatcher<'a>>>,
        parameters: Option<Box<ParameterMatcher<'a>>>,
    },
    ObjectLiteral {
        key: Option<&'a str>,
        value: Option<Box<ResolveMatcher<'a>>>,
    },
}

enum ParameterMatcher<'a> {
    PositionalParameter {
        index: Option<usize>,
        value: Option<ResolveMatcher<'a>>,
    },
    NamedParameter {
        name: Option<&'a str>,
        value: Option<ResolveMatcher<'a>>,
    },
    // Maybe also something for *args and **kwargs
}

This would allow the LspContext to check for the ctx.attr pattern, and instruct the LSP to look for the appropriate AST nodes. For Bazel, that might look something like this:

fn get_dotted_identifier_resolve_options<'a>(
    &self,
    _current_file: &LspUrl,
    within_function: Option<&'a str>,
    segments: &[&'a str],
) -> anyhow::Result<Vec<ResolveOptions<'a>>> {
    let mut result = vec![];
    if within_function.is_none()
        || segments.len() < 3
        || segments[0] != "ctx"
        || segments[1] != "attr"
    {
        return Ok(result);
    }

    for function_name in ["rule", "repository_rule"] {
        result.push(ResolveOptions {
            scope: ResolveScope::OnlyTopLevel,
            matcher: ResolveMatcher::FunctionCall {
                function_name: Some(Box::new(ResolveMatcher::Ident(function_name))),
                parameters: Some(vec![
                    ParameterMatcher::NamedParameter {
                        name: Some("implementation"),
                        value: Some(ResolveMatcher::Ident(within_function.unwrap())),
                    },
                    ParameterMatcher::NamedParameter {
                        name: Some("attrs"),
                        value: Some(ResolveMatcher::ObjectLiteral {
                            key: Some(segments[2]),
                            value: None,
                        }),
                    },
                ]),
            },
        })
    }

    Ok(result)
}

The current LSP implementation to look for the struct definition would be written as follows (though I think it shouldn't be up to the LspContext to have to define this—it would only define additional lookup patterns).

ResolveOptions {
    // The currently impl is actually `ResolveScope::OnlyTopLevel`, but
    // it really should be `::Current`.
    scope: ResolveScope::Current,
    matcher: ResolveMatcher::Assignment {
        lhs: Some(Box::new(ResolveMatcher::Ident(segments[0]))),
        rhs: Some(Box::new(ResolveMatcher::FunctionCall {
            function_name: Some(Box::new(ResolveMatcher::Ident("struct"))),
            parameters: Some(vec![ParameterMatcher::NamedParameter {
                name: Some(segments[1]),
                value: None,
            }]),
        })),
    },
}

Another option would be to make the AST public (#77), and simply provide callbacks that take in the AST node and some information about the identifier being looked up, and simply returns true or false, or perhaps the matching AST node (so you can match on a function call, but jump to some specific argument). Actually, I suppose the proposal above needs some mechanic as well to do the same. Any thoughts?

MaartenStaa avatar Sep 01 '23 14:09 MaartenStaa