lint() could "just work" on expressions without needing a newline

[MichaelChirico]

It's a bit of a mental burden to always remember to add \n to expressions when doing ad hoc linting, especially since expect_lint() doesn't need it.

I think it should be fine for lint() to accept an expression as the first argument, in which case:

1. Check file.exists(filename)
2. If not, try parse(filename)
3. Fail

Alternatively, we could create an analogue to styler::style_text(), lintr::lint_text(), designed for ad-hoc linting.

I lean towards doing this through lint() since it's so close to handling this already.

[MichaelChirico]

Related: #503.

[AshesITR]

Why? lintr::lint(text = ...) works fine, no?

Also lintr::lint("does_not_exist.R") would no longer error with file not found.

[MichaelChirico]

Also lintr::lint("does_not_exist.R") would no longer error with file not found.

In my preliminary work I have stop("Expected to find a file name or an R expression, got ", filename), I think that's better than the current:

lintr::lint("does_not_exist.R")
Error in file(con, "r") : cannot open the connection
In addition: Warning message:
In file(con, "r") :
  cannot open file 'does_not_exist.R': No such file or directory

And yes, I had forgotten about text= when filing the issue, but I still regularly use lintr::lint("expr\n", linter) interactively. I could try and change habits, but if we're going to support the \n approach, I think we can try parsing as well.

Alternatively, we could shut off the \n path and force filename to be a file name, otherwise use text=.

WDYT?

[AshesITR]

Maybe we should deprecate the use of "\n" in filename for the next release, pointing to text=?

[MichaelChirico]

I don't like that idea, unless we want to use a new lint_text() instead. current API is too convenient for ad hoc work.

[AshesITR]

Alternatively, we could shut off the \n path and force filename to be a file name, otherwise use text=.

Seemed like a less offensive way to phase out this usage. I'm open to exporting a thin wrapper lint_text(text, ...) that calls lint(text = text, ...).

[MichaelChirico]

but the wrapper can't be quite that thin -- ..1 will become filename which we don't want.

[AshesITR]

True. Probably need to either construct a call() or allow filename = NULL in lintr as a substitute

needs_tempfile <- missing(filename) || rex::re_matches(filename, rex::rex(newline))

[MichaelChirico]

Why? lintr::lint(text = ...) works fine, no?

Also lintr::lint("does_not_exist.R") would no longer error with file not found.

IIUC the concern here is avoided by checking file.exists(filename) before attempting parse(text = filename). So this feature is built into what's currently an error. In fact it would just replace this region:

https://github.com/r-lib/lintr/blob/6e52b41d9ff5f8a4f5209151ec37a2e12b60ab84/R/lint.R#L42

Instead of looking for \n, check if filename parses. So the code complexity actually stays the same.

I still like the idea of lint(<expr>) working, esp. since it makes switching between interactive lint() and testing expect_lint() that tiny bit easier.

[AshesITR]

Checking file.exist() would yield FALSE and parse gladly accepts "does_not_exist.R", putting us back at square one.

I'm wondering if we should deprecate using \n in filename, since text= is present now.

For interactive use, moving text= before the dots would allow partial matching hacks to get lint(t="...") if you want to save on keystrokes?

[MichaelChirico]

Checking file.exist() would yield FALSE and parse gladly accepts "does_not_exist.R", putting us back at square one.

I get your point now -- OK, yes, that's not great. And workarounds will be messy or require a bigger refactor (e.g. merging with the pattern= detection logic of lint_dir(). Not worth it overall.

I'm wondering if we should deprecate using \n in filename, since text= is present now.

It's more convenient to do lint("expr\n", linter()) than lint(t="expr", linters=linter()). Let's just leave things as they are.