[MichaelChirico]

Continues the chain from #2818 to #2829

I wasn't satisfied with the approach to just comment out & ignore large swaths of the test suite needed to implement # nofuzz through now, especially after the comment_injection_fuzzer turned out to require such large # nofuzz regions, but I was stuck on how to proceed with specific exclusions.

I think the approach taken here is pretty satisfactory: as a preprocessing step, we convert the # nofuzz markup into new code in the suite that signals activated/deactivated fuzzers for each execution of expect_lint().

The approach could be improved -- it can be a bit of a pain to debug, e.g. when expect_lint(file = .) was erroring due to encoding issues, the error causes the test to exit and thus fails to run the activate_fuzzers() step, leading to cascading confusing errors it's hard to pin down. And there's no simple workaround here, as e.g. using a withr::defer() approach to ensure the activate_fuzzers() step is run is not straightforward.

But I think there's enough good progress here (and it turned up a few more true positive fixes!) that it is ready for review.

[MichaelChirico]

Here is Gemini's review:

---

Prompt:

Use the R gh package (which is already installed) to

 1. read the bug 'https://github.com/r-lib/lintr/issues/2191'. Be sure to read existing comments.
 2. read the diff of the PR 'https://github.com/r-lib/lintr/pull/2832'
 3. read the existing PR review comments for further context
 4. read the git history on this branch (you are on the checked out branch locally, and the target is 'main')
   + there are a very large number of commits on this branch. commits from '3882350fd8a77980301e8ce4dc676017713c601f' to 'ad856367de3b7995c484bcbfcf69a7d1c57e0f66' are already in main; 'fb7866f813b54c2eb07f06b24462a62efca4d0ce' to '357f56f4ed4e07a95d6f3a01e55ea9ca44cce605' are new logic; commits after that basically consist of merging in later commits to main, de-conflicting, iterating. **Please base your final review only on the final diff from 'fuzz-assignment' to 'main'.**. The point of the exercise of reviewing current commits is to be sure you see other things that might have been tried already.

With this context in hand, provide a thorough, constructive review of the pull request.

Focus on readability, and check for gaps in logic or missing tests. Could the same fix be done more readably, more robustly, or any way better? Does it fit with the
style of the package?

Extirpate any instinct to be overly polite/groveling. Pushback is welcome. Politeness in the review is not an asset -- the only goal is to have the best submitted code possible.

---

Response:

This pull request introduces a powerful and innovative fuzz testing framework to the lintr
package, aimed at ensuring consistency across equivalent R constructs (e.g., function(...) vs.
(...), <- vs. =, and $ vs. @). The core of the implementation lies in a new GHA workflow and a
set of scripts in .dev/ that mutate test content before running expect_lint().

Overall Architecture
The approach of injecting the fuzzer directly into expect_lint() via source manipulation in
.dev/ast_fuzz_test.R is clever. It allows the fuzzer to intercept every test case that uses
expect_lint() without requiring a massive refactor of the existing test suite. The use of #
fuzzer disable: comments to handle known edge cases or intentional inconsistencies is
a sound design choice, providing a granular "escape valve" while keeping the tests themselves
visible.

Code Quality and Robustness

• .dev/maybe_fuzz_content.R: The fuzzing logic is robust. Specifically, the strategy of
  re-parsing the mutated file after each fuzzer is applied (pd <- error_or_parse_data(f))
  ensures that subsequent fuzzers operate on valid and up-to-date parse data. This correctly
  handles cases where one fuzzer might change line or column numbers for the next.
• Linter Updates: The updates to any_duplicated_linter, is_numeric_linter, and
  keyword_quote_linter to use strip_comments_from_subtree() are excellent. They demonstrate
  the value of the fuzzer in identifying where our XPath logic was too fragile (e.g., the fix
  in keyword_quote_linter.R using not(self::COMMENT) is a perfect example).

Areas for Improvement

1. Bug in strip_comments_from_subtree() (Nodesets):
   In R/xml_utils.R, the check if (length(xml_find_first(expr, ".//COMMENT")) == 0L) is buggy
   when expr is a nodeset. xml_find_first on a nodeset returns a nodeset of the same length;
   length() will thus be > 0 even if no comments are found (it returns missing nodes).

   • Recommendation: Use all(is.na(xml_find_first(expr, ".//COMMENT"))) or similar to
     correctly detect if any node in the set needs stripping.

2. any_duplicated_linter Context Sensitivity:
   The change in any_duplicated_linter.R to run XPaths on a stripped subtree (xml_parent() |>
   strip_comments_from_subtree()) is much cleaner. However, I noticed some parts of
   length_unique_xpath (like the .N and n() matches) use parent::expr/parent::expr/parent::expr.
   Since strip_comments_from_subtree returns a cloned tree, any XPath attempting to climb above
   the comparison will fail.

   • Verification: My testing confirmed that these still work if they only go up to the ==
     call itself (which is the root of the stripped tree), but it's a fragile pattern. We
     should consider making these XPaths fully local to the comparison expression.

3. Fuzzer Expansion:
   The assignment_fuzzer currently only swaps <- and =. Expanding this to include right
   assignment -> (perhaps by wrapping the LHS in ()) would catch even more consistency gaps,
   although it is admittedly more complex to implement.

4. Readability of ast_fuzz_test.R:
   The script uses grep and readLines to patch expect_lint.R. While functional, this is a bit
   brittle if the formatting of expect_lint.R changes slightly. It might be worth adding more
   defensive checks or using a more robust patching mechanism if this becomes a permanent part of
   the dev cycle.

Minor Style Note
In any_duplicated_linter.R, the nrow lint message is hardcoded to anyDuplicated(DF$col) == 0L
.... While consistent with some older linters, it would be a nice touch to use the actual
object name found in the code, especially since the fuzzer might be using various names.

Conclusion
This is a high-quality PR that significantly improves the long-term maintainability and
consistency of the package. The "fuzzer torture" has already yielded several bug fixes in
existing linters. Once the nodeset check in xml_utils.R is addressed, this is ready to go.

---

[MichaelChirico]

There is some useful feedback in there, I will address it soon.

[MichaelChirico]

Fixed items 1 & 2.

For 3, I'm not sure a -> swapper is worth the overhead it will take to put -> in an always-valid way. Anyway, filed #3005.

For 4, we can revisit this if it becomes an annoyance.