lintr icon indicating copy to clipboard operation
lintr copied to clipboard

Bad error handling for invalid `Lint()`s

Open AshesITR opened this issue 4 years ago • 2 comments

When a linter returns a Lint with an invalid (NA) column_number, as has recently happened in some regex-based linters, the error message is extremely confusing:

Error in rep.int(character, length) : invalid 'times' value
Calls: <Anonymous> ... print.lint -> cat -> highlight_string -> fill_with -> paste0
Execution halted

IMO Lint() should do some mild verification during construction so that invalid Lint()s can't be created successfully.

Here are a bunch of checks we could add:

  1. No NAs are allowed anywhere.
  2. 1L <= column_number <= nchar(line) + 1L.
  3. line_number >= 1L.
  4. ranges is a list of length 2 integer(ish) vectors, or NULL.
  5. 1L <= ranges[[i]][1L] <= ranges[[i]][2L] <= nchar(line) + 1L for all i.

WDYT?

AshesITR avatar Feb 17 '21 22:02 AshesITR

Would NULL be allowed for all of these? I'm thinking of lints that aren't as specific as the line/column level (e.g. file-level lints or line-level lints), which might make sense to leave one/both of line_number / column_number blank.

For ranges, don't we have cases where ranges[2L] might apply to a different line?

MichaelChirico avatar Feb 18 '21 07:02 MichaelChirico

IINM NULLs are allowed in some places, but ranges are constrained to a single line only (or NULL), as we can currently only flag one line.

Line and column numbers could default to 1 as that would always be valid.

AshesITR avatar Feb 18 '21 08:02 AshesITR