Implement linter tags file compatible lintr 3.0.0

[m-kolomanski]

Issue #102

Description

Added linters.csv file in /inst/lintr/ with linters and tags, compatiblie with linter 3.0.0. and lintr::available_linters(), lintr::available_tags() functions.

Added unit tests for this feature, although those need to be run with devtools::check() as the mentioned functions expect the package to be installed. Otherwise, related tests will be skipped.

Definition of Done

• The change is thoroughly documented.
• The CI passes (R CMD check, linter, unit tests, spelling).
• Any generated files have been updated (e.g. .Rd files with roxygen2::roxygenise())

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.21%. Comparing base (e5658e4) to head (597430b).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #130   +/-   ##
=======================================
  Coverage   97.21%   97.21%           
=======================================
  Files          23       23           
  Lines        1112     1112           
=======================================
  Hits         1081     1081           
  Misses         31       31           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[radbasa]

To switch between local file and package file, how about checking if it is running under R CMD check? Refer to https://testthat.r-lib.org/reference/is_testing.html.

Also, take a look at {rprojroot} functions to determine the root path for the two different situations.

[m-kolomanski]

Thanks for the pointers, the following could be done to clean up the code a little:

linters_csv_location <- file.path(
  rprojroot::find_root_file(criterion = rprojroot::is_r_package),
  "lintr/linters.csv"
)

and later in the tests:

skip_if(!is_checking())

But I don't think I follow the logic with switching files? To my understanding, changing the linters_csv_location based on execution context does not matter, as it is the lintr::available_tags() function that is failing to read the file when running devtools::test(), and we cannot change the bahaviour of that function. Am I missing or misunderstanding something?

[radbasa]

It might be worth adding @evalRd rd_tags("<function_name>") to the roxygen blocks.

https://lintr.r-lib.org/articles/creating_linters.html#writing-the-linter

[m-kolomanski]

It might be worth adding @evalRd rd_tags("<function_name>") to the roxygen blocks.

https://lintr.r-lib.org/articles/creating_linters.html#writing-the-linter

@radbasa

rd_tags is not exported from lintr package, therefore this would require creating ./R/linters_tags.R helper file with this function definition.

Since the rd_tags function from lintr package generates tags with links that forward to doc pages for specific tags, this would need to be followed with rd_linters function and documentation file for each of the tags, with list of linters that have that particular tag assigned.

Alternatively, a different schema could be created for this usecase (like section with list of tags, without links to further doc).

What do you think would be the best for this case?

For details, see rd_tags(), rd_linters(), linter_tags_docs.R file and generated example doc for lintr.

[radbasa]

@m-kolomanski Try looking at the following lines of code to use non-exported functions from packages:

box.linters/R/get_box_module_exports.R

Lines 21 to 24 in e5658e4

	parse_spec <- get0("parse_spec", envir = base::loadNamespace("box"))
	find_mod <- get0("find_mod.box$mod_spec", envir = base::loadNamespace("box"))
	load_mod <- get0("load_mod.box$mod_info", envir = base::loadNamespace("box"))
	namespace_info <- get0("namespace_info", envir = base::loadNamespace("box"))

Maybe, it will work. Maybe, it won't.

[m-kolomanski]

@m-kolomanski Try looking at the following lines of code to use non-exported functions from packages:

box.linters/R/get_box_module_exports.R

Lines 21 to 24 in e5658e4

	parse_spec <- get0("parse_spec", envir = base::loadNamespace("box"))
	find_mod <- get0("find_mod.box$mod_spec", envir = base::loadNamespace("box"))
	load_mod <- get0("load_mod.box$mod_info", envir = base::loadNamespace("box"))
	namespace_info <- get0("namespace_info", envir = base::loadNamespace("box"))

Maybe, it will work. Maybe, it won't.

@radbasa Importing the functions themselves does work, however both needed functions contain call to lintr::available_linters() function, which fails to find the tags when roxygen2::roxygenise() is run (similarly to when devtools::test() is run).

[radbasa]

@m-kolomanski Try looking at the following lines of code to use non-exported functions from packages:

box.linters/R/get_box_module_exports.R

Lines 21 to 24 in e5658e4

	parse_spec <- get0("parse_spec", envir = base::loadNamespace("box"))
	find_mod <- get0("find_mod.box$mod_spec", envir = base::loadNamespace("box"))
	load_mod <- get0("load_mod.box$mod_info", envir = base::loadNamespace("box"))
	namespace_info <- get0("namespace_info", envir = base::loadNamespace("box"))

Maybe, it will work. Maybe, it won't.

@radbasa Importing the functions themselves does work, however both needed functions contain call to lintr::available_linters() function, which fails to find the tags when roxygen2::roxygenise() is run (similarly to when devtools::test() is run).

@jakubnowicki It would be nice if we had a list of linter tags similar to lintr Meta-tooling. But, that is automatically generated by lintr:::rd_tags() and lintr:::rd_linters() that have a lot of internal dependencies.

Do we have time to manually write documentation pages for these tags? We have 12 tags, and 15 functions.

[jakubnowicki]

@m-kolomanski Try looking at the following lines of code to use non-exported functions from packages:

box.linters/R/get_box_module_exports.R

Lines 21 to 24 in e5658e4

	parse_spec <- get0("parse_spec", envir = base::loadNamespace("box"))
	find_mod <- get0("find_mod.box$mod_spec", envir = base::loadNamespace("box"))
	load_mod <- get0("load_mod.box$mod_info", envir = base::loadNamespace("box"))
	namespace_info <- get0("namespace_info", envir = base::loadNamespace("box"))

Maybe, it will work. Maybe, it won't.

@radbasa Importing the functions themselves does work, however both needed functions contain call to lintr::available_linters() function, which fails to find the tags when roxygen2::roxygenise() is run (similarly to when devtools::test() is run).

@jakubnowicki It would be nice if we had a list of linter tags similar to lintr Meta-tooling. But, that is automatically generated by lintr:::rd_tags() and lintr:::rd_linters() that have a lot of internal dependencies.

Do we have time to manually write documentation pages for these tags? We have 12 tags, and 15 functions.

I would rather leave it for the next release.

[radbasa]

LGTM