lintr
lintr copied to clipboard
Investigate further warning message seen in lint GHA workflow
cf. https://github.com/r-lib/lintr/actions/runs/3240547631/jobs/5311303504
Warning message:
In add_exclusions(exclusions, excluded_lines, linters_string, exclude_linter_sep, :
Could not find linter named ‘one_linter’ in the list of active linters. Make sure the linter is uniquely identified by the given name or prefix.
It's coming from here, not sure why it's only happening in that workflow though:
https://github.com/r-lib/lintr/blob/4b37029f989629287e910fa3aba46ab15d1975cb/tests/testthat/test-unreachable_code_linter.R#L124-L138
There is a new warning now, in addition to the existing one:
1: In parse_check_usage(fun, known_used_symbols = known_used_symbols, :
Possible bug in lintr: Couldn't parse usage message ‘<anonymous> : <anonymous> : <anonymous>: local variable 'col2' assigned but may not be used
’. Ignoring 2 usage warnings. Please report an issue at https://github.com/r-lib/lintr/issues.
cf. https://github.com/r-lib/lintr/actions/runs/3240547631/jobs/5311303504
Warning message: In add_exclusions(exclusions, excluded_lines, linters_string, exclude_linter_sep, : Could not find linter named ‘one_linter’ in the list of active linters. Make sure the linter is uniquely identified by the given name or prefix.
That should easily be fixed by changing the nolint directive like we do in other tests.
There is a new warning now, in addition to the existing one:
1: In parse_check_usage(fun, known_used_symbols = known_used_symbols, : Possible bug in lintr: Couldn't parse usage message ‘<anonymous> : <anonymous> : <anonymous>: local variable 'col2' assigned but may not be used ’. Ignoring 2 usage warnings. Please report an issue at https://github.com/r-lib/lintr/issues.
Those were previously silently ignored I presume. I might try running with options(warn = 3)
later to find the source.
Surprised that these weren't caught here, but maybe they are not coming from the /tests
folder:
https://github.com/r-lib/lintr/blob/1ac48e94ef550945d94c0e47b43a9eb8041b5f00/.github/workflows/test-package-vigilant.yaml#L30
They are emitted by lint_package()
, not by test()
. That's why.
WDYT about using options(warn = 2)
in the linting workflows as well?
lint()
isn't supposed to produce warnings if everything is in order, so that might be a good way to catch these early.
I have an update on the check usage problem:
single_quotes_linter.R houses the issue, but this looks like a bug in codetools triggered by our use of assignments within with()
:
codetools::checkUsage(function() {
with(
list(),
a <- 1
)
}, skipWith = TRUE)
#> <anonymous>: local variable 'a' assigned but may not be used
codetools::checkUsage(function() {
with(
list(),
a <- 1
)
}, skipWith = FALSE)
#> <anonymous>: local variable 'a' assigned but may not be used (<text>:8-11)
Created on 2022-11-09 with reprex v2.0.2
Thanks for tracking down the source of this! Should this be reported in r-devel mailing list?
As a temp workaround, we could avoid assignments. That is, change the following
https://github.com/r-lib/lintr/blob/c814159f98eee8004319a027e1a24b7887455e9f/R/single_quotes_linter.R#L52-L64
to
with(content[str_idx[id], ], {
Lint(
filename = source_expression$filename,
line_number = line1,
column_number = col1,
type = "style",
message = "Only use double-quotes.",
line = source_expression$file_lines[[line1]],
ranges = list(c(col1, if (line1 == line2) col2 else nchar(line)))
)
})
Not a big fan of the refactoring, as it makes the code harder to read.
Maybe instead write col1 <- content$col1[str_idx[id]]
etc. for all the variables from content?
That would allow us to drop the with()
alltogether.
I also see this warning locally now if I run (e.g.) the following code:
> lintr::lint_package(linters = lintr::commented_code_linter())
|================================================================================================| 100%
Warning message:
In add_exclusions(exclusions, excluded_lines, linters_string, exclude_linter_sep, :
Could not find linter named ‘one_linter’ in the list of active linters. Make sure the linter is uniquely identified by the given name or prefix.
It's thrown by the same file, and not by executing the test, because the test suite defines one_linter()
.
This is a general problem with how # nolint: specific_linter.
works. I think we can safely ignore the warning; an alternative is to refactor that test to use a linter actually in our test suite.
I believe we've discussed alternatives for this warning before but not really gotten anywhere so far.
cf. https://github.com/r-lib/lintr/actions/runs/3240547631/jobs/5311303504
Warning message: In add_exclusions(exclusions, excluded_lines, linters_string, exclude_linter_sep, : Could not find linter named ‘one_linter’ in the list of active linters. Make sure the linter is uniquely identified by the given name or prefix.
That should easily be fixed by changing the nolint directive like we do in other tests.
Using # TestNolint: one_linter.
or whatever other nolint directive seems to be a good way to solve the issue IMO.