search

r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.19k stars 185 forks source link

Parsing source files on excluded files #504

Open csgillespie opened 4 years ago

csgillespie commented 4 years ago

When I call lintr::lint on a file, it runs get_source_expression regardless if the file should be excluded

I think if we move the get_source_expression down a few lines, this should solve the issue.

The use case here is I run lintr::lint on a bunch of .Rmd files. But occasionally I want to exclude the files from linting.

AshesITR commented 4 years ago

Exlusions only take effect for lint_dir() and lint_package(). What is your expected behaviour for calling lint() on an excluded file?

csgillespie commented 4 years ago

I suppose I was expecting it to return NULL

russHyde commented 3 years ago

Exclusions can be passed into lint() as part of the dots argument (or pulled in from the .lintr config). This is quite useful when excluding specific lines within a file. It seems reasonable to exit-early from linting excluded files, though it may not lead to any significant performance improvements.

Do we know if get_source_expressions is ran for all excluded files when calling lint_[package|dir]?

AshesITR commented 3 years ago

get_source_expressions is only called by lint() IINM, so completely excluded files are optimized away by lint_dir() and not read at all.

russHyde commented 3 years ago

When calling lint_dir, filenames that are completely excluded are filtered out by lint_dir, and all other files are passed along (one-by-one) to lint along with the set of exclusions. Since the exclusions are passed into lint from lint_dir the completely-excluded files could be filtered out within lint. This would make lint respect any complete-exclusions parsed from the .lintr or passed in by the user (though I'll admit it seems strange to completely-exclude a file that you pass into lint).

If a file is completely excluded in the exclusions: blah section of .lintr, and lint(that_file) is called, any lints that are found in that file are not reported (they are filtered out when calling exclude(lints, ...) just before returning from lint). This means that filtering-out completely-excluded files is performed twice during the lint_dir workflow and once in the lint workflow. So there's a bit of duplicated logic in there.

It might make sense to push the complete-exclusion filtering into lint and remove it from both lint_dir and exclude

MichaelChirico commented 6 months ago

This will be easier after #2135