lsp/eval: correct resolution of package-level fields

+74 -45

2 changed files

expand all

internal

lsp

eval

eval.go

eval_test.go

+74 -41

internal/lsp/eval/eval.go

··· 1589 1589 // evaluated tracks whether this frame has been evaluated, ensuring 1590 1590 // it is only evaluated once. 1591 1591 evaluated bool 1592 + // embedded tracks whether this frame is embedded within its parent 1593 + // frame. This is useful to tell apart frames which share the same 1594 + // navigable. For example: 1595 + // 1596 + // x: y 1597 + // {y: _} 1598 + // 1599 + // The embedded frame uses the same navigable as the surrounding 1600 + // frame, but must be treated separately so that the path `y` does 1601 + // not resolve to the `y` field within the embedded frame. 1602 + // 1603 + // Note this embedded field is set for more than just normal cue 1604 + // embeddings - it is also set for things the LSP treats like 1605 + // embeddings, such as disjunctions and comprehensions. 1606 + embedded bool 1592 1607 // parent is the parent frame. 1593 1608 parent *frame 1594 1609 // childFrames contains every frame that is a child of this ··· 1708 1723 // frame f. This is a light wrapper around 1709 1724 // [fileEvaluator.newFrame]. See those docs for more details on the 1710 1725 // arguments to this function. 1711 - func (f *frame) newFrame(node ast.Node, nav *navigable) *frame { 1726 + func (f *frame) newFrame(node ast.Node, nav *navigable, embedded bool) *frame { 1712 1727 child := f.fileEvaluator.newFrame(f, node, nav) 1728 + child.embedded = embedded 1713 1729 f.childFrames = append(f.childFrames, child) 1714 1730 return child 1715 1731 } ··· 1749 1765 // navigable, so that they can all be found when resolving 1750 1766 // imports of this package, in some other package. 1751 1767 1752 - childFr := f.newFrame(nil, f.fileEvaluator.evaluator.pkgDecls) 1768 + childFr := f.newFrame(nil, f.fileEvaluator.evaluator.pkgDecls, false) 1753 1769 if fscache.IsPhantomPackage(node) { 1754 1770 // For packages with invented names, be careful to avoid 1755 1771 // returning file coordinates that use the length of the ··· 1810 1826 f.fileEvaluator.importSpecNavigables = importSpecNavigables 1811 1827 } 1812 1828 nav, found := importSpecNavigables[*ip] 1813 - childFr := f.newFrame(node, nav) 1829 + childFr := f.newFrame(node, nav, false) 1814 1830 if !found { 1815 1831 importSpecNavigables[*ip] = childFr.navigable 1816 1832 } ··· 1903 1919 } 1904 1920 1905 1921 case *ast.EmbedDecl: 1906 - f.newFrame(node.Expr, f.navigable) 1922 + f.newFrame(node.Expr, f.navigable, true) 1907 1923 1908 1924 case *ast.PostfixExpr: 1909 1925 switch node.Op { ··· 1914 1930 default: 1915 1931 // Currently should never happen. Just in case though, 1916 1932 // behave the same as Unary. 1917 - f.newFrame(node.X, nil) 1933 + f.newFrame(node.X, nil, false) 1918 1934 } 1919 1935 1920 1936 case *ast.ParenExpr: 1921 1937 unprocessed = append(unprocessed, node.X) 1922 1938 1923 1939 case *ast.UnaryExpr: 1924 - f.newFrame(node.X, nil) 1940 + f.newFrame(node.X, nil, false) 1925 1941 1926 1942 case *ast.BinaryExpr: 1927 1943 switch node.Op { 1928 1944 case token.AND: 1929 - f.newFrame(node.X, f.navigable).addRange(node) 1930 - f.newFrame(node.Y, f.navigable).addRange(node) 1945 + f.newFrame(node.X, f.navigable, true).addRange(node) 1946 + f.newFrame(node.Y, f.navigable, true).addRange(node) 1931 1947 case token.OR: 1932 - lhsNav := f.newFrame(node.X, nil).navigable 1933 - rhsNav := f.newFrame(node.Y, nil).navigable 1948 + lhsNav := f.newFrame(node.X, nil, false).navigable 1949 + rhsNav := f.newFrame(node.Y, nil, false).navigable 1934 1950 f.navigable.ensureResolvesTo([]*navigable{lhsNav, rhsNav}) 1935 1951 lhsNav.recordUsage(node, f) 1936 1952 rhsNav.recordUsage(node, f) 1937 1953 default: 1938 - f.newFrame(node.X, nil) 1939 - f.newFrame(node.Y, nil) 1954 + f.newFrame(node.X, nil, false) 1955 + f.newFrame(node.Y, nil, false) 1940 1956 } 1941 1957 1942 1958 case *ast.Alias: ··· 1946 1962 unprocessed = append(unprocessed, node.Expr) 1947 1963 1948 1964 case *ast.Ellipsis: 1949 - childFr := f.newFrame(node.Type, nil) 1965 + childFr := f.newFrame(node.Type, nil, false) 1950 1966 childFr.key = node 1951 1967 childFr.addRange(node) 1952 1968 // The navigable needs a name so that [UsagesForOffset] will ··· 1965 1981 case *ast.CallExpr: 1966 1982 resolvable = append(resolvable, node.Fun) 1967 1983 for _, arg := range node.Args { 1968 - f.newFrame(arg, nil) 1984 + f.newFrame(arg, nil, false) 1969 1985 } 1970 1986 1971 1987 case *ast.Ident, *ast.SelectorExpr, *ast.IndexExpr: ··· 1986 2002 clause := node.Clauses[0] 1987 2003 unprocessed = append(unprocessed, clause) 1988 2004 if fallback := node.Fallback; fallback != nil { 1989 - f.newFrame(fallback.Body, f.navigable) 2005 + f.newFrame(fallback.Body, f.navigable, true) 1990 2006 } 1991 2007 // We don't know how many child frames we'll need to 1992 2008 // process clause. So we stash whatever remains of this ··· 2013 2029 } 2014 2030 2015 2031 case *ast.IfClause: 2016 - f.newFrame(node.Condition, nil) 2032 + f.newFrame(node.Condition, nil, false) 2017 2033 2018 2034 comprehensionTail := comprehensionsStash[node] 2019 - f.newFrame(comprehensionTail, f.navigable) 2035 + f.newFrame(comprehensionTail, f.navigable, true) 2020 2036 2021 2037 case *ast.ForClause: 2022 - f.newFrame(node.Source, nil) 2038 + f.newFrame(node.Source, nil, false) 2023 2039 2024 - stack := frameStack{f.newFrame(nil, nil)} 2040 + stack := frameStack{f.newFrame(nil, nil, false)} 2025 2041 2026 2042 if key := node.Key; key != nil { 2027 2043 stack.push(key, stack.peek().newBinding(key, nil)) ··· 2031 2047 } 2032 2048 2033 2049 comprehensionTail := comprehensionsStash[node] 2034 - stack.push(comprehensionTail, stack.peek().newFrame(comprehensionTail, f.navigable)) 2050 + tailFr := stack.peek().newFrame(comprehensionTail, f.navigable, true) 2051 + stack.push(comprehensionTail, tailFr) 2035 2052 2036 2053 case *ast.LetClause: 2037 2054 ident := node.Ident 2038 2055 // A let clause might or might not be within a comprehension. 2039 2056 if comprehensionTail, found := comprehensionsStash[node]; found { 2040 2057 // We're within a wider comprehension. 2041 - f.newFrame(node.Expr, nil) 2058 + f.newFrame(node.Expr, nil, false) 2042 2059 2043 - stack := frameStack{f.newFrame(nil, nil)} 2060 + stack := frameStack{f.newFrame(nil, nil, false)} 2044 2061 stack.push(ident, stack.peek().newBinding(ident, nil)) 2045 - stack.push(comprehensionTail, stack.peek().newFrame(comprehensionTail, f.navigable)) 2062 + tailFr := stack.peek().newFrame(comprehensionTail, f.navigable, true) 2063 + stack.push(comprehensionTail, tailFr) 2046 2064 2047 2065 } else { 2048 2066 // We're not within a wider comprehension: the binding ··· 2055 2073 comprehensionTail := comprehensionsStash[node] 2056 2074 if ident := node.Ident; ident != nil { 2057 2075 // Assignment form: try x = expr { ... } 2058 - f.newFrame(node.Expr, nil) 2076 + f.newFrame(node.Expr, nil, false) 2059 2077 2060 - stack := frameStack{f.newFrame(nil, nil)} 2078 + stack := frameStack{f.newFrame(nil, nil, false)} 2061 2079 stack.push(ident, stack.peek().newBinding(ident, nil)) 2062 - stack.push(comprehensionTail, stack.peek().newFrame(comprehensionTail, f.navigable)) 2080 + tailFr := stack.peek().newFrame(comprehensionTail, f.navigable, true) 2081 + stack.push(comprehensionTail, tailFr) 2063 2082 } else { 2064 2083 // Struct form: try { ... } 2065 - f.newFrame(comprehensionTail, f.navigable) 2084 + f.newFrame(comprehensionTail, f.navigable, true) 2066 2085 } 2067 2086 2068 2087 case *ast.Field: ··· 2078 2097 2079 2098 aliasIdent := fieldDecl.aliasIdent 2080 2099 if aliasIdent == nil { 2081 - childFr = f.newFrame(node.Value, childNav) 2100 + childFr = f.newFrame(node.Value, childNav, false) 2082 2101 2083 2102 } else if fieldDecl.aliasInParentScope { 2084 - childFr = f.newFrame(node.Value, childNav) 2103 + childFr = f.newFrame(node.Value, childNav, false) 2085 2104 f.appendBinding(aliasIdent.Name, childFr) 2086 2105 2087 2106 } else { 2088 - wrapper := f.newFrame(nil, nil) 2107 + wrapper := f.newFrame(nil, nil, false) 2089 2108 wrapper.addRange(node) 2090 2109 wrapper.navigable.ensureResolvesTo([]*navigable{f.navigable}) 2091 - childFr = wrapper.newFrame(node.Value, childNav) 2110 + childFr = wrapper.newFrame(node.Value, childNav, false) 2092 2111 2093 2112 if expr := fieldDecl.aliasValue; expr != nil { 2094 2113 wrapper.newBinding(aliasIdent, expr) ··· 2127 2146 // field label is a simple ident with no aliases and no 2128 2147 // expressions in the field label. Therefore, 2129 2148 // len(fieldDecl.exprs) > 0 implies !strings.HasPrefix(keyName, "__") 2130 - childFr.parent.newFrame(fieldDecl, nil) 2149 + childFr.parent.newFrame(fieldDecl, nil, false) 2131 2150 } 2132 2151 2133 2152 for _, attr := range node.Attrs { ··· 2154 2173 remoteFilename = filepath.ToSlash(remoteFilename) 2155 2174 fieldName := strings.TrimPrefix(remoteFilename, dir) 2156 2175 fieldNameIdent := ast.NewIdent(fieldName) 2157 - grandChildFr := childFr.newFrame(fieldNameIdent, childFr.navigable.ensureNavigable(fieldName)) 2176 + grandChildFr := childFr.newFrame(fieldNameIdent, childFr.navigable.ensureNavigable(fieldName), false) 2158 2177 grandChildFr.addRange(attr) 2159 2178 childFr.appendBinding(fieldName, grandChildFr) 2160 2179 childFrs = append(childFrs, grandChildFr) ··· 2320 2339 // newBinding creates and returns a new [frame], and stores it under 2321 2340 // the given name in the current frame only. 2322 2341 func (f *frame) newBinding(key *ast.Ident, unprocessed ast.Node) *frame { 2323 - childFr := f.newFrame(unprocessed, nil) 2342 + childFr := f.newFrame(unprocessed, nil, false) 2324 2343 name := key.Name 2325 2344 f.appendBinding(name, childFr) 2326 2345 if !strings.HasPrefix(name, "__") { ··· 2332 2351 end: key.End(), 2333 2352 valueFrame: childFr, 2334 2353 } 2335 - f.newFrame(expr, nil) 2354 + f.newFrame(expr, nil, false) 2336 2355 childFr.key = key 2337 2356 } 2338 2357 return childFr ··· 2516 2535 2517 2536 default: 2518 2537 component = nil 2519 - childFr := f.newFrame(curExpr, nil) 2538 + childFr := f.newFrame(curExpr, nil, false) 2520 2539 if nextEnd.IsValid() { 2521 2540 childFr.end = nextEnd 2522 2541 } ··· 2560 2579 func (f *frame) resolvePathRoot(name string, requireIdent bool) (*navigable, string) { 2561 2580 frameOrig := f 2562 2581 for ; f != nil; f = f.parent { 2582 + fNav := f.navigable 2563 2583 if childFrs, found := f.bindings[name]; found { 2564 2584 if len(childFrs) == 1 { 2565 2585 nav := childFrs[0].navigable ··· 2575 2595 // name has been resolved to an alias which had a 2576 2596 // normal ident or basiclit field name. Switch to that 2577 2597 // name. 2578 - return f.navigable, nav.name 2598 + return fNav, nav.name 2579 2599 } 2580 2600 } 2581 2601 2582 2602 if !requireIdent { 2583 - return f.navigable, name 2603 + return fNav, name 2584 2604 } 2585 2605 // If name lexically matches a non-alias, it must be matching 2586 2606 // an ident and not a basiclit. But that ident can come from ··· 2595 2615 if !identFound { 2596 2616 continue 2597 2617 } 2598 - return f.navigable, name 2618 + return fNav, name 2599 2619 } 2600 2620 2601 2621 if f.isFileFrame() { 2602 2622 // If we've got this far, we're allowed to inspect the 2603 2623 // (shared) navigable bindings directly without having to go 2604 2624 // via our bindings. 2605 - if _, found := f.navigable.bindings[name]; found { 2606 - return f.navigable, name 2625 + if nav, found := fNav.bindings[name]; found { 2626 + // We must only return fNav if the binding has been made 2627 + // by a frame which is *not* embedded. 2628 + for _, fr := range fNav.frames { 2629 + if fr.embedded { 2630 + continue 2631 + } 2632 + for _, childFr := range fr.bindings[name] { 2633 + // check that the childFr has the right navigable 2634 + // otherwise we could get tripped up by aliases etc. 2635 + if childFr.navigable == nav { 2636 + return fNav, name 2637 + } 2638 + } 2639 + } 2607 2640 } 2608 2641 // Support for the Self experiment: 2609 2642 parentNav := frameOrig.navigable.parent

-4

internal/lsp/eval/eval_test.go

··· 3390 3390 `, 3391 3391 expectDefinitions: map[position][]position{ 3392 3392 ln(2, 2, "b"): {ln(2, 1, "b"), ln(3, 1, "b")}, 3393 - ln(10, 2, "d"): {ln(10, 1, "d")}, // this is WRONG. Should not resolve at all. 3394 3393 ln(12, 2, "g"): {ln(12, 1, "g"), ln(13, 1, "g")}, 3395 3394 3396 3395 ln(1, 1, "x"): {self}, ··· 4624 4623 } 4625 4624 `, 4626 4625 expectDefinitions: map[position][]position{ 4627 - // TODO: It's highly doubtful that this is what's really wanted 4628 - ln(6, 1, "z"): {ln(4, 1, "z"), ln(11, 1, "z"), ln(19, 1, "z")}, 4629 - 4630 4626 ln(7, 1, "x"): {ln(2, 1, "x")}, 4631 4627 ln(14, 1, "x"): {ln(2, 1, "x")}, 4632 4628 ln(22, 1, "x"): {ln(2, 1, "x")},

Configure Feed

Configure Feed