Linting code chunks in Rmd not working consistently with knitr

[renkun-ken]

Consider the following Rmd (saved to e.g. lintr-test.Rmd):

```{r}
x<-1

x<-2

x<-3

Linting the file has the following output:

lintr::lint("lintr-test.Rmd")
/home/renkun/Workspaces/testR/lintr-test.Rmd:2:2: style: Put spaces around all infix operators. x<-1 ~^~~ /home/renkun/Workspaces/testR/lintr-test.Rmd:10:2: style: Put spaces around all infix operators. x<-3 ~^~~

To my understanding, it should lint the first two R chunks and ignores the third one.

However, the current chunk extraction does not seem to work consistently with knitr in that knitr treats r and R as the same. The theorem is brought up at https://github.com/REditorSupport/languageserver/issues/421 where bookdown seems to provide some markdown extensions where additional engines are implemented (as introduced at https://bookdown.org/yihui/bookdown/markdown-extensions-by-bookdown.html).

I think the expected behavior should be linting the R or r chunks and ignores all other chunks.

[AshesITR]

Related to #796

[russHyde]

Reproduced this:

```{R}
# "R" engine is a synonym for "r"
a<-1
a

This code block, when added to `./tests/testthat/knitr_formats/test.Rmd` throws no assignment lints
But, the code block is evaluated by knitr when knitting to .html.

This seems to happen because "R" is one of `knitr::knit_engines` list, not because of case-insensitivity of the knitr engine-matching code

[russHyde]

Also, lintr should attack any code blocks that have engine="Rscript"

```{Rscript}
# "Rscript" engine wraps the code in a block in a randomScript and calls
# `Rscript randomScript.R 2>&1`
#

b<-1
b

[russHyde]

One area that I'm not sure how to handle are custom knitr engines

Some of these are used to run R code, and so should be linted, eg, details and targets

Other packages add custom knitr engines that don't run R code (eg, nomnoml; I believe this is the case for the tufte "marginfigure" engine mentioned in #796) and so should be ignored.

Perhaps the lintr config should allow the user to add additional engines whose code should be linted.

[russHyde]

I agree re the bug for {therom} example:

```{notAnEngine}
a<-1

Throws an assignment lint in current master

[russHyde]

So my thoughts: If engine is r, R or Rscript then code block should be linted If engine is defined in engines: c("targets") stub of .lintr it should be linted Code blocks should be disregarded for all other engines, including those that are possible misspellings of the defined knitr engines (eg, there should be no error on a ```{Rscrypt}``` block)

[renkun-ken]

So my thoughts: If engine is r, R or Rscript then code block should be linted If engine is defined in engines: c("targets") stub of .lintr it should be linted Code blocks should be disregarded for all other engines, including those that are possible misspellings of the defined knitr engines (eg, there should be no error on a ```{Rscrypt}``` block)

Makes perfect sense.

[AshesITR]

Sounds like a good idea. Maybe the option name should be more specific to rmd, e.g. rmd_engines?

Also we should use the existing default mechanism to set r, R, and Rscript as default rmd_engines. This would allow the usecase of mixing regular r code chunks with R code chunks which are intentionally bad but still evaluated. You could then put rmd_engines: "r"into .lintr to actively suppress linting of R code chunks without cluttering everything with #nolints.

[MichaelChirico]

I was going to suggest rmd_engines as well, so agree with that & everything. great suggestions!

[russHyde]

Ha, I hadn't thought of using ```{R} bad-code-here ``` and ```{r} lint this code ```. Do you know if the different engines are a specifically RMD thing? Do .Rnw (etc) use these non-R engines?

[AshesITR]

A quick google-fu turned up this gem from 2010. So it seems to work via custom drivers. knitr_engines might also be a good name, WDYT?

[russHyde]

knitr_engines seems more appropriate. Thanks everyone.

[russHyde]

Summary of aims:

---

With no knitr_engines configured (in either .lintr or the lint() arguments), {lintr} will lint all code blocks with "r", "R" or "Rscript" engines.

---

With a .lintr config like this (or with knitr_engines = ... in the lint() arguments):

// .lintr
knitr_engines: with_defaults("targets", default = default_knitr_engines)

{lintr} would lint all code blocks for "r", "R", "Rscript" and "targets" engines

---

With .lintr configured like this:

knitr_engines: c("r")
// or
knitr_engines: with_defaults(R = NULL, Rscript = NULL, default = default_knitr_engines)

{lintr} would only lint blocks with the "r" engine.

Does this seem OK everyone, @jimhester ?

[jimhester]

sure, sounds good.