How should the exclusions = ... argument work?

[AshesITR]

Follow up to #539.

How should lint, lint_dir and lint_package handle exclusions specified through the manual argument?

[russHyde]

I flagged this in #539. When working on a related issue (ensuring that files could be excluded by relative-paths; russHyde#2), @joaopmatias noted that the exclusions-handling is different for files defined in the .lintr, compared to those defined in the 'exclusions' argument.

The following integration test was added to test-lint_package.R:

test_that(                                                                                                                                     
   "`lint_package` does not depend on path to pkg - with exclusions argument", {                                                               
     # The test checks the results of lint_package when the excluded regions are                                                               
     # given as an argument of the function and are specified using absolute and                                                               
     # relative paths                                                                                                                          
                                                                                                                                               
      pkg_path <- file.path("dummy_packages", "assignmentLinter")                                                                              
                                                                                                                                               
      # Create list of exclusions relative to the whole of `abc.R` and the first                                                               
     # line of `jkl.R`                                                                                                                         
     exclusions <- list(normalizePath(file.path(pkg_path, 'R/abc.R')),                                                                         
                        'R/jkl.R' = 1)                                                                                                         
                                                                                                                                               
      expected_lines <- c("mno = 789")                                                                                                         
     lints_from_outside <- lint_package(                                                                                                       
       pkg_path, linters = list(assignment_linter), exclusions = exclusions                                                                    
     )                                                                                                                                         
                                                                                                                                               
      expect_equal(                                                                                                                            
       as.data.frame(lints_from_outside)[["line"]], expected_lines                                                                             
     )                                                                                                                                         
 })

We added a full-path and a relative-path to the exclusions. A single lint was expected in the non-excluded lines of the file that was excluded using relative paths.

I realise now that we should have had a parse_settings = FALSE argument in the call to lint_package; with this modification, the exclusions appear to be handled properly.

Happy to have this issue closed if you agree @AshesITR

By 'properly', according to the docs, exclusions should be specified relative to an assumed project-root. They can also be specified as full-paths (though this doesn't appear mentioned in the docs). It doesn't specify what will happen when no project-root can be found (should exclusions be relative to the directory passed to 'lint_dir'?) though that's probably a minor issue

[AshesITR]

I'd expect exclusions to work relative to the path argument, maybe even regardless of where the project root was.

e.g. exclusions in lint_dir("R", exclusions = "RcppExports.R") should behave identical to exclusions in lint_dir(".", exclusions = "R/RcppExports.R").
What do you think about that?

[russHyde]

I'd be happy with that, I also think that exclusions specified in a .lintr file should be relative to the position of the .lintr file (rather than a project root that may be in a parent directory) and that two different directories could respect different .lintr files (eg, allowing a different line-length in ./docs than in ./scripts).

These are quite big changes that differ from the documented function of lintr, so would have to wait until v3 I guess.

I'll investigate what happens for a 'free' project (one with no .git, .lintr, or .Rproj) when lint_dir is called with exclusions.

[AshesITR]

Thanks for doing that.

The .lintr exclusions should definitely be relative to the file (already). I think to docs need clarification on this point.

I'm not even sure what happens atm if lint_package() is called on a package with e.g. .lintr and R/.lintr. Maybe 2.x could support multiple .lintr configurations in some way.

Those changes should not have compatibility issues because this is a previously unsupported configuration.