pipes (%>%) in single lines get marked by lintr

[GregorDeCillia]

I just installed this package and faced an issue with single line pipes. I have a function that basically looks like this.

myFun <- function() {
  a %>% b() %>% c()
}

lintr::lint_package() adds a style-marker here because of missing newlines. Is this behavior expected? Here is an actual repex: r-webutils/suitr@4f6f97e

[russHyde]

That's funny. There is a pipe_continuation_linter. But that's supposed to allow pipelines that all fit on one line.
In fact, there's a unit test for this setting in test-pipe_continuation_linter.R:

# Pipe expressions on a single line are ignored
  expect_lint("foo %>% bar() %>% baz()", NULL, pipe_continuation_linter)

[russHyde]

Further checks:

Passes without comment:

expect_lint("my_fun <- function(){\n  a %>% b()\n}\n", NULL, pipe_continuation_linter)

Also, Passes without comment:

expect_lint("a %>% b() %>% c()\n", NULL, pipe_continuation_linter)

But, Throws Error:

expect_lint("my_fun <- function(){\n  a %>% b() %>% c()\n}\n", NULL, pipe_continuation_linter)
Error: got 1 lints instead of 0
list(filename = "/tmp/Rtmpl4hRNb/file4e435a9e6e93", line_number = 2, column_number = 7, type = "style", message = "`%>%` should always have a space before it and a new line after it, unless the full pipeline fits on one line.", line = "  a %>% b() %>% c()", ranges = list(c(5, 7)), linter = "pipe_continuation_linter")

[russHyde]

At present, the pipe_continuation_linter identifies expressions based on two factors:

• does the expression span more than one line
• within that expression, does the pipe character occur more than once within a given line

The reprex contains an expression that spans multiple lines (the definition of my_fun) and within that expression, there is a line containing the magrittr pipe multiple times. But, the pipeline is present in a subexpression of the function definition, and that subexpression only spans one line. There may be a way to modify the xml searches to play better with nested expressions.

I suspect that nesting pipelines in other settings may throw the same lint (eg, in the body of a with or a test_that).

# should these throw a pipe-continuation lint?
# - in each case the main expression spans > 1 line, but the pipeline is isolated on a single line
myFun <- function() {
  a %>% b() %>% c()
}

with(
  diamonds,
  x %>% head(10) %>% tail(5)
)

test_that('blah', {
    test_data <- diamonds %>% head(10) %>% tail(5)
    # do something with test_data
})

[GregorDeCillia]

I think the examples with function and test_that should not throw lints. For some packages, it just makes sense to include pipes in functions and unit tests. In such cases, it should also be allowed to put multiple pipes in one line.

I have no strong opinion about the withr::with_* family of functions, since I rarely use them but my first reaction is to not throw linters

An example, where linters are appropriate in my opinion are functions that have multiple NSE arguments like the dplyr::mutate.

# this should throw a linter
dplyr::mutate(
  mtcars,
  am = am %>% as.character() %>% as.factor(),
  cyl = cyl %>% as.character() %>% as.factor()
)

[russHyde]

OK. The final example complicates things a bit in my mind (since each pipeline it contains is restricted to a single codeline).

I was thinking we might be able to do something like:

• Identify all lines that contain multiple pipe characters
• For each such line, if there is an expression that starts and ends on that line and encases all the pipe characters, disregard the line; otherwise throw a lint

[GregorDeCillia]

I approve this suggestion. In this case one could also adapt the message produced by the linter from

style: %>% should always have a space before it and a new line after it, unless the full pipeline fits on one line.

to something that reflects this logic more closely.

[russHyde]

#408