Should xml_find_function_calls() land on the parent expr instead?

[MichaelChirico]

WDYT about xml_find_function_calls() landing on //SYMBOL_FUNCTION_CALL/parent::expr? It seems like most XPaths start there.

Originally posted by @MichaelChirico in #2357 (comment)

---

Alternatively, it could be an option that is TRUE by default. Or with levels, land_on = c("parent", "self", "parent::parent"). I think that would make the issue of documentation easier.

[AshesITR]

Adding 3.2.0 because we can't change the public API easily once it got to CRAN.

[MichaelChirico]

Here's some not-very-scientific analysis from combing manually over the codebase:

Categorizing the XPath that is run following $xml_find_function_calls() depending on the chain of /.../ conditions prefixing them, we get the following rough frequency:

1. parent::expr/following-sibling:: - 27 times
2. parent::expr/parent::expr - 22 times
3. parent::expr - 9 times
4. parent::expr/parent::expr/parent::expr - 4 times
5. self::SYMBOL_FUNCTION_CALL - 3 times

My memory here is that there was not a dramatic performance difference from landing on any of SYMBOL_FUNCTION_CALL, SYMBOL_FUCNTION_CALL/parent::expr, or SYMBOL_FUNCTION_CALL/parent::expr/parent::expr.

So I think landing on parent::expr is the most reasonable default -- most commonly, conditions in linters using $xml_find_function_calls() operate from the expr containing the SYMBOL_FUNCTION_CALL.

I'm not sure there's much benefit to exposing an option for this default, given the lack of real performance implications. Better to just consistently land on SYMBOL_FUNCTION_CALL/parent::expr.

If that makes sense, I can send out a PR making the change.