Refactor symbol search

[lionel-]

Progress towards posit-dev/positron#3822.

• Errors are now consistently propagated as Err and only logged at the very top.

• We no longer recurse blindly into all node types except argument lists (this is the current restriction but note that we should recurse in arg lists in the future for e.g. lapply and test_that). Instead, we only recurse in handlers of specific node types. For instance, comment sections are only expected in expression lists, and thus only handled in the expression list handler.

• We no longer pass around a mut ref of the parent. Instead we pass a store of children by value and return it once done. If a handler inserts a note in the outline, it creates a new store, recurse into its children with this new store, and adds itself to the store of its caller.

  This simpler approach is easier to understand and should solve the lifetime issues encountered in Feature/nested-outline #593.

• Bunch of tests.

[lionel-]

I'm thinking that only index_expression_list() (added in this PR) needs to keep track of the section stack and pass the currently active store to its children.

Now that all methods own their store, we no longer need any ref or mut ref. So index_expression_list() will be able to push and pop section stores out of its stack and we only work with owned values that we move around.

Does that makes sense?

[DavisVaughan]

Looks like a nice simplification!

[DavisVaughan]

One thing that I really like from the Biome group is the with_<optional_feature>() pattern

fn with_children(mut self, children) -> Self {
  self.children = children;
  self
}

So it becomes

new_symbol(symbol, kind, range).with_children(children)

It would require a DocumentSymbolExt trait so this is fine if you want to keep it as is, just thought id point it out as a pattern to keep in mind

[lionel-]

That does seem like a nice pattern!

[kv9898]

I'm thinking that only index_expression_list() (added in this PR) needs to keep track of the section stack and pass the currently active store to its children.

Now that all methods own their store, we no longer need any ref or mut ref. So index_expression_list() will be able to push and pop section stores out of its stack and we only work with owned values that we move around.

Does that makes sense?

Oh yes it does. I wrote a new pull request to this specific branch in #606.

My approach was to use a store-vector to store a "flattened" version of the nested structure, which can be edited easily during comment section handling and be assembled in the end of index_expression_list.

Note that this process involves assigning a level to any symbols involved. For the non-comment symbols, I assigned them to usize::MAX to avoid them becoming parents.