Skip to content

Commit

Permalink
feat(treesitter)!: default to correct behavior for quantified captures (
Browse files Browse the repository at this point in the history
neovim#30193)

For context, see neovim#24738. Before
that PR, Nvim did not correctly handle captures with quantifiers. That
PR made the correct behavior opt-in to minimize breaking changes, with
the intention that the correct behavior would eventually become the
default. Users can still opt-in to the old (incorrect) behavior for now,
but this option will eventually be removed completely.

BREAKING CHANGE: Any plugin which uses `Query:iter_matches()` must
update their call sites to expect an array of nodes in the `match`
table, rather than a single node.
  • Loading branch information
gpanders authored Sep 1, 2024
1 parent 318c041 commit 6913c5e
Show file tree
Hide file tree
Showing 9 changed files with 42 additions and 68 deletions.
6 changes: 5 additions & 1 deletion runtime/doc/news.txt
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,11 @@ PLUGINS

TREESITTER

• TODO
|Query:iter_matches()| correctly returns all matching nodes in a match
instead of only the last node. This means that the returned table maps
capture IDs to a list of nodes that need to be iterated over. For
backwards compatibility, an option `all=false` (only return the last
matching node) is provided that will be removed in a future release.

TUI

Expand Down
31 changes: 11 additions & 20 deletions runtime/doc/treesitter.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1036,9 +1036,8 @@ add_directive({name}, {handler}, {opts})
the same name
{all}? (`boolean`) Use the correct implementation of the
match table where capture IDs map to a list of nodes
instead of a single node. Defaults to false (for backward
compatibility). This option will eventually become the
default and removed.
instead of a single node. Defaults to true. This option
will be removed in a future release.

*vim.treesitter.query.add_predicate()*
add_predicate({name}, {handler}, {opts})
Expand All @@ -1049,14 +1048,13 @@ add_predicate({name}, {handler}, {opts})
{handler} (`fun(match: table<integer,TSNode[]>, pattern: integer, source: integer|string, predicate: any[], metadata: vim.treesitter.query.TSMetadata)`)
• see |vim.treesitter.query.add_directive()| for argument
meanings
{opts} (`table`) A table with the following fields:
{opts} (`table?`) A table with the following fields:
{force}? (`boolean`) Override an existing predicate of
the same name
{all}? (`boolean`) Use the correct implementation of the
match table where capture IDs map to a list of nodes
instead of a single node. Defaults to false (for backward
compatibility). This option will eventually become the
default and removed.
instead of a single node. Defaults to true. This option
will be removed in a future release.

edit({lang}) *vim.treesitter.query.edit()*
Opens a live editor to query the buffer you started from.
Expand Down Expand Up @@ -1216,14 +1214,8 @@ Query:iter_matches({node}, {source}, {start}, {stop}, {opts})
indices to a list of nodes, and metadata from any directives processing
the match.

WARNING: Set `all=true` to ensure all matching nodes in a match are
returned, otherwise only the last node in a match is returned, breaking
captures involving quantifiers such as `(comment)+ @comment`. The default
option `all=false` is only provided for backward compatibility and will be
removed after Nvim 0.10.

Example: >lua
for pattern, match, metadata in cquery:iter_matches(tree:root(), bufnr, 0, -1, { all = true }) do
for pattern, match, metadata in cquery:iter_matches(tree:root(), bufnr, 0, -1) do
for id, nodes in pairs(match) do
local name = query.captures[id]
for _, node in ipairs(nodes) do
Expand All @@ -1248,12 +1240,11 @@ Query:iter_matches({node}, {source}, {start}, {stop}, {opts})
start depth for each match. This is used to prevent
traversing too deep into a tree.
• match_limit (integer) Set the maximum number of
in-progress matches (Default: 256).
• all (boolean) When set, the returned match table maps
capture IDs to a list of nodes. Older versions of
iter_matches incorrectly mapped capture IDs to a single
node, which is incorrect behavior. This option will
eventually become the default and removed.
in-progress matches (Default: 256). all (boolean) When
`false` (default `true`), the returned table maps capture
IDs to a single (last) node instead of the full list of
matching nodes. This option is only for backward
compatibility and will be removed in a future release.

Return: ~
(`fun(): integer, table<integer, TSNode[]>, vim.treesitter.query.TSMetadata`)
Expand Down
4 changes: 1 addition & 3 deletions runtime/lua/vim/treesitter/_fold.lua
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ local function compute_folds_levels(bufnr, info, srow, erow, parse_injections)

-- Collect folds starting from srow - 1, because we should first subtract the folds that end at
-- srow - 1 from the level of srow - 1 to get accurate level of srow.
for _, match, metadata in
query:iter_matches(tree:root(), bufnr, math.max(srow - 1, 0), erow, { all = true })
do
for _, match, metadata in query:iter_matches(tree:root(), bufnr, math.max(srow - 1, 0), erow) do
for id, nodes in pairs(match) do
if query.captures[id] == 'fold' then
local range = ts.get_range(nodes[1], bufnr, metadata[id])
Expand Down
2 changes: 1 addition & 1 deletion runtime/lua/vim/treesitter/_query_linter.lua
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ function M.lint(buf, opts)
parser:parse()
parser:for_each_tree(function(tree, ltree)
if ltree:lang() == 'query' then
for _, match, _ in query:iter_matches(tree:root(), buf, 0, -1, { all = true }) do
for _, match, _ in query:iter_matches(tree:root(), buf, 0, -1) do
local lang_context = {
lang = lang,
parser_info = parser_info,
Expand Down
8 changes: 1 addition & 7 deletions runtime/lua/vim/treesitter/languagetree.lua
Original file line number Diff line number Diff line change
Expand Up @@ -833,13 +833,7 @@ function LanguageTree:_get_injections()
local start_line, _, end_line, _ = root_node:range()

for pattern, match, metadata in
self._injection_query:iter_matches(
root_node,
self._source,
start_line,
end_line + 1,
{ all = true }
)
self._injection_query:iter_matches(root_node, self._source, start_line, end_line + 1)
do
local lang, combined, ranges = self:_get_injection(match, metadata)
if lang then
Expand Down
28 changes: 11 additions & 17 deletions runtime/lua/vim/treesitter/query.lua
Original file line number Diff line number Diff line change
Expand Up @@ -620,16 +620,16 @@ local directive_handlers = {
--- @field force? boolean
---
--- Use the correct implementation of the match table where capture IDs map to
--- a list of nodes instead of a single node. Defaults to false (for backward
--- compatibility). This option will eventually become the default and removed.
--- a list of nodes instead of a single node. Defaults to true. This option will
--- be removed in a future release.
--- @field all? boolean

--- Adds a new predicate to be used in queries
---
---@param name string Name of the predicate, without leading #
---@param handler fun(match: table<integer,TSNode[]>, pattern: integer, source: integer|string, predicate: any[], metadata: vim.treesitter.query.TSMetadata)
--- - see |vim.treesitter.query.add_directive()| for argument meanings
---@param opts vim.treesitter.query.add_predicate.Opts
---@param opts? vim.treesitter.query.add_predicate.Opts
function M.add_predicate(name, handler, opts)
-- Backward compatibility: old signature had "force" as boolean argument
if type(opts) == 'boolean' then
Expand All @@ -642,7 +642,7 @@ function M.add_predicate(name, handler, opts)
error(string.format('Overriding existing predicate %s', name))
end

if opts.all then
if opts.all ~= false then
predicate_handlers[name] = handler
else
--- @param match table<integer, TSNode[]>
Expand Down Expand Up @@ -894,16 +894,10 @@ end
--- index of the pattern in the query, a table mapping capture indices to a list
--- of nodes, and metadata from any directives processing the match.
---
--- WARNING: Set `all=true` to ensure all matching nodes in a match are
--- returned, otherwise only the last node in a match is returned, breaking captures
--- involving quantifiers such as `(comment)+ @comment`. The default option
--- `all=false` is only provided for backward compatibility and will be removed
--- after Nvim 0.10.
---
--- Example:
---
--- ```lua
--- for pattern, match, metadata in cquery:iter_matches(tree:root(), bufnr, 0, -1, { all = true }) do
--- for pattern, match, metadata in cquery:iter_matches(tree:root(), bufnr, 0, -1) do
--- for id, nodes in pairs(match) do
--- local name = query.captures[id]
--- for _, node in ipairs(nodes) do
Expand All @@ -925,9 +919,9 @@ end
--- - max_start_depth (integer) if non-zero, sets the maximum start depth
--- for each match. This is used to prevent traversing too deep into a tree.
--- - match_limit (integer) Set the maximum number of in-progress matches (Default: 256).
--- - all (boolean) When set, the returned match table maps capture IDs to a list of nodes.
--- Older versions of iter_matches incorrectly mapped capture IDs to a single node, which is
--- incorrect behavior. This option will eventually become the default and removed.
--- - all (boolean) When `false` (default `true`), the returned table maps capture IDs to a single
--- (last) node instead of the full list of matching nodes. This option is only for backward
--- compatibility and will be removed in a future release.
---
---@return (fun(): integer, table<integer, TSNode[]>, vim.treesitter.query.TSMetadata): pattern id, match, metadata
function Query:iter_matches(node, source, start, stop, opts)
Expand Down Expand Up @@ -960,10 +954,10 @@ function Query:iter_matches(node, source, start, stop, opts)

local captures = match:captures()

if not opts.all then
if opts.all == false then
-- Convert the match table into the old buggy version for backward
-- compatibility. This is slow. Plugin authors, if you're reading this, set the "all"
-- option!
-- compatibility. This is slow, but we only do it when the caller explicitly opted into it by
-- setting `all` to `false`.
local old_match = {} ---@type table<integer, TSNode>
for k, v in pairs(captures or {}) do
old_match[k] = v[#v]
Expand Down
2 changes: 1 addition & 1 deletion runtime/lua/vim/ui.lua
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ function M._get_urls()
local query = vim.treesitter.query.get(lang, 'highlights')
if query then
local tree = assert(ltree:tree_for_range(range))
for _, match, metadata in query:iter_matches(tree:root(), bufnr, row, row + 1, { all = true }) do
for _, match, metadata in query:iter_matches(tree:root(), bufnr, row, row + 1) do
for id, nodes in pairs(match) do
for _, node in ipairs(nodes) do
if vim.treesitter.node_contains(node, range) then
Expand Down
9 changes: 3 additions & 6 deletions test/functional/treesitter/parser_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,7 @@ print()
vim.treesitter.query.parse('c', '((number_literal) @number (#set! "key" "value"))')
local parser = vim.treesitter.get_parser(0, 'c')

local _, _, metadata =
query:iter_matches(parser:parse()[1]:root(), 0, 0, -1, { all = true })()
local _, _, metadata = query:iter_matches(parser:parse()[1]:root(), 0, 0, -1)()
return metadata.key
end)

Expand All @@ -612,8 +611,7 @@ print()
)
local parser = vim.treesitter.get_parser(0, 'c')

local _, _, metadata =
query:iter_matches(parser:parse()[1]:root(), 0, 0, -1, { all = true })()
local _, _, metadata = query:iter_matches(parser:parse()[1]:root(), 0, 0, -1)()
local _, nested_tbl = next(metadata)
return nested_tbl.key
end)
Expand All @@ -633,8 +631,7 @@ print()
)
local parser = vim.treesitter.get_parser(0, 'c')

local _, _, metadata =
query:iter_matches(parser:parse()[1]:root(), 0, 0, -1, { all = true })()
local _, _, metadata = query:iter_matches(parser:parse()[1]:root(), 0, 0, -1)()
local _, nested_tbl = next(metadata)
return nested_tbl
end)
Expand Down
20 changes: 8 additions & 12 deletions test/functional/treesitter/query_spec.lua
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ void ui_refresh(void)
local parser = vim.treesitter.get_parser(0, 'c')
local tree = parser:parse()[1]
local res = {}
for pattern, match in cquery:iter_matches(tree:root(), 0, 7, 14, { all = true }) do
for pattern, match in cquery:iter_matches(tree:root(), 0, 7, 14) do
-- can't transmit node over RPC. just check the name and range
local mrepr = {}
for cid, nodes in pairs(match) do
Expand Down Expand Up @@ -211,7 +211,7 @@ void ui_refresh(void)
local parser = vim.treesitter.get_parser(0, 'c')
local tree = parser:parse()[1]
local res = {}
for pattern, match in cquery:iter_matches(tree:root(), 0, 7, 14, { all = true }) do
for pattern, match in cquery:iter_matches(tree:root(), 0, 7, 14) do
-- can't transmit node over RPC. just check the name and range
local mrepr = {}
for cid, nodes in pairs(match) do
Expand Down Expand Up @@ -260,7 +260,7 @@ void ui_refresh(void)
local parser = vim.treesitter.get_parser(0, 'c')
local tree = parser:parse()[1]
local res = {}
for pattern, match in cquery:iter_matches(tree:root(), 0, 7, 14, { all = true }) do
for pattern, match in cquery:iter_matches(tree:root(), 0, 7, 14) do
-- can't transmit node over RPC. just check the name and range
local mrepr = {}
for cid, nodes in pairs(match) do
Expand Down Expand Up @@ -307,7 +307,7 @@ void ui_refresh(void)
local parser = vim.treesitter.get_parser(0, 'c')
local tree = parser:parse()[1]
local res = {}
for pattern, match in cquery:iter_matches(tree:root(), 0, 0, -1, { all = true }) do
for pattern, match in cquery:iter_matches(tree:root(), 0, 0, -1) do
-- can't transmit node over RPC. just check the name and range
local mrepr = {}
for cid, nodes in pairs(match) do
Expand Down Expand Up @@ -418,7 +418,7 @@ void ui_refresh(void)
local parser = vim.treesitter.get_parser(0, 'c')
local tree = parser:parse()[1]
local res = {}
for pattern, match in cquery:iter_matches(tree:root(), 0, 0, -1, { all = true }) do
for pattern, match in cquery:iter_matches(tree:root(), 0, 0, -1) do
-- can't transmit node over RPC. just check the name and range
local mrepr = {}
for cid, nodes in pairs(match) do
Expand Down Expand Up @@ -464,11 +464,7 @@ void ui_refresh(void)

local parser = vim.treesitter.get_parser(0, 'c')

-- Time bomb: update this in 0.12
if vim.fn.has('nvim-0.12') == 1 then
return 'Update this test to remove this message and { all = true } from add_predicate'
end
query.add_predicate('is-main?', is_main, { all = true })
query.add_predicate('is-main?', is_main)

local query0 = query.parse('c', custom_query0)

Expand Down Expand Up @@ -496,7 +492,7 @@ void ui_refresh(void)

local parser = vim.treesitter.get_parser(0, 'c')

query.add_predicate('is-main?', is_main, true)
query.add_predicate('is-main?', is_main, { all = false, force = true })

local query0 = query.parse('c', custom_query0)

Expand Down Expand Up @@ -650,7 +646,7 @@ void ui_refresh(void)
local parser = vim.treesitter.get_parser(0, 'c')
local tree = parser:parse()[1]
local res = {}
for pattern, match in cquery:iter_matches(tree:root(), 0, 7, 14) do
for pattern, match in cquery:iter_matches(tree:root(), 0, 7, 14, { all = false }) do
local mrepr = {}
for cid, node in pairs(match) do
table.insert(mrepr, { '@' .. cquery.captures[cid], node:type(), node:range() })
Expand Down

0 comments on commit 6913c5e

Please sign in to comment.