Codecov Report

Merging #2273 (53f6fd7) into main (71c7e31) will not change coverage.
The diff coverage is n/a.

❗ Current head 53f6fd7 differs from pull request most recent head 530bc92. Consider uploading reports for the commit 530bc92 to get more accurate results

@@           Coverage Diff           @@
##             main    #2273   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files         114      114           
  Lines        5258     5258           
=======================================
  Hits         5239     5239           
  Misses         19       19           

WDYT about integrating it into conjunct_test_linter()? It detects a similar problem with stopifnot().

WDYT about integrating it into conjunct_test_linter()? It detects a similar problem with stopifnot().

I lean towards "no", since it's really a particular thing for stopifnot() (e.g. testthat::expect_true() nor assertthat::assert_that() use this behavior).

That said, we don't have to stretch the definition of "conjunct" too much to make it work -- all(x) being x[1L] && ... && x[n]...

Metadata and vectorization tests would be nice.

vectorization tests

I think for linters built with make_linter_from_xpath(), that's overkill -- better to test make_linter_from_xpath() directly

If the linter ever gets more complex functionality in the future, these tests will ensure that it still works?

If the linter ever gets more complex functionality in the future, these tests will ensure that it still works?

I think the next commit would be the one to owe new tests.

OTOH, it doesn't really hurt to add new tests. And more importantly, there is a new test of the XPath logic here, namely that it's not somehow over- or under-counting lints. Not sure we've come across that being an issue in the past, but still justifies the new test IMO.