[R-package] [ci] added more linters

[jameslamb]

This PR adds a few more R linters:

• absolute_path_linter: prevent paths like ~/repos/
• nonportable_path_linter: prevent paths like C:\Users\ which only work on certain operating systems
• undesirable_function_linter: prevent the use of some functions which could cause issues, specifically:
  • dyn.load() / dyn.unload(): direct calls to use symbols in .dll / .so files. My expectation is that someone using this in a PR either doesn't understand how to write R packages that call C++ code or is introducing something that will be very difficult to maintain.
  • help(): call up help documentation. This should only be used interactively, not in code checked into this repository.
  • install.packages(): install new libraries, e.g. from CRAN. This should only be used interactively, not in code checked into this repository.
  • ifelse(): This is a convenient but dangerous function, because its output type depends on whether or not the input is all-TRUE, all-FALSE, or a mix.
    • 
  • rbind(): stack two data frames on top of each other (similar to pd.concat() in pandas). data.table::rbindlist() is faster and safer in the case where the data frames have different columns
  • is.list(): check if an object is a list. I found that in all uses in lightgbm, we are checking if something is exactly a list, not "list-like". is.list() will be TRUE on a data.table object, so using identical(class(x), "list") is safer.
  • require(): load a package if it hasn't been loaded yet and raise a warning if it is not available. library() is preferred, since hitting a loud error right away that says "this package does not exist" makes debugging easier than getting some of error like "function 'something' not found" further down in the code
• undesirable_operator_linter: prevent the use of certain operators in R code, specifically:
  • %>%, %.%, %..%: variations of "pipe" operators, commonly used in some R packages. The lightgbm package doesn't currently use this programming model, and preventing it with this linter is about preventing a mix of different programming styles in the codebase.
  • ?: call up help documentation. This should only be used interactively, not in code checked into this repository.

This PR also re-ordered the list of linters in .ci/lint_r_code.R to alphabetize them for readability.

[StrikerRUS]

LGTM, as it passes CI tests!

[jameslamb]

@Laurae2 can you take a look when you have a chance? I'm making the R code more opinionated here, would like to be sure you agree with it before merging.

[jameslamb]

gently ping @Laurae2 for a review

[Laurae2]

Follow alphabet order

[jameslamb]

ah yeah! Good catch

[Laurae2]

@jameslamb I looked thoroughly and I agree with the changes.

Do you think we should also add cbind() to undesirable_function_linter?

[jameslamb]

@jameslamb I looked thoroughly and I agree with the changes.

Do you think we should also add cbind() to undesirable_function_linter?

Yep I agree! I'll do that

[jameslamb]

@jameslamb I looked thoroughly and I agree with the changes.
Do you think we should also add cbind() to undesirable_function_linter?

Yep I agree! I'll do that

@Laurae2 I had to change one of the demos where we were using cbind(). Could you take a look and let me know if it's ok? b60e94b

[jameslamb]

I just rebased to master to catch up with recent changes.

[StrikerRUS]

nit.
Dots for the consistency with other descriptions.

[jameslamb]

nit.
Dots for the consistency with other descriptions.

no problem, thanks!

by the way I just pulled this and rebased it to current master. None of the changes on master fail these new linting rules, so we can merge this as-is (without force pushing the rebased version of this branch I have locally) once it builds

[StrikerRUS]

@jameslamb Did you see this warning:

* checking R code for possible problems ... NOTE
multiple.tree.plot.interpretation: no visible binding for global
  variable ‘bar_color’
Undefined global functions or variables:
  bar_color

I guess it's from this PR.

[jameslamb]

@jameslamb Did you see this warning:

* checking R code for possible problems ... NOTE
multiple.tree.plot.interpretation: no visible binding for global
  variable ‘bar_color’
Undefined global functions or variables:
  bar_color

I guess it's from this PR.

@StrikerRUS I did see that a bit after we merged, working on #2901 I was trying to avoid any more R PRs since we have so many open right now, but I think this one would be easy to fix. This comes from the fact that data.table uses non-standard evaluation. so tree_interpretation[, bar_color] (where tree_interpretation is a data.table) will work correctly, but static analysis of the code makes it look like a variable that doesn't exist is being referenced.

I'll submit a PR right now

[github-actions]

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.