Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: package to be linted should be installed #1137

Closed
dpprdan opened this issue May 16, 2022 · 2 comments · Fixed by #2155
Closed

docs: package to be linted should be installed #1137

dpprdan opened this issue May 16, 2022 · 2 comments · Fixed by #2155
Milestone

Comments

@dpprdan
Copy link
Contributor

dpprdan commented May 16, 2022

Linting a package that is not installed may generate false positives for (at least) object_usage_linter, see e.g. r-lib/actions#557

  1. Why is that exactly? Is object_usage_linter (and probably others) use namespace of installed package #91 still relevant here?
  2. (Where) should this be documented?

Re 2: I took

lintr/README.md

Lines 217 to 218 in fff7808

If you want to run `lintr` on [Travis-CI](https://travis-ci.org), you will need
to have Travis install the package first. This can be done by adding the
to mean that the package to be linted must be installed, but I guess it just means that lintr needs to be installed (which is self-evident, though, I suppose)? Anyway, my first thought was to document it at the beginning of the the

lintr/README.md

Line 192 in fff7808

### For packages

section, because it is also valid for GHA.

But the "package should be installed" applies locally as well, so maybe it should be in the lint_package() docs? Maybe something like

Note: To avoid false positives, the package to be linted needs to be installed.

@AshesITR
Copy link
Collaborator

Not sure if useful info, but using devtools::load_all() should suffice, no actual installation required.

The problem is lintr only lints one file at a time so it can't see function definitions from around the codebase during linting.
As a work-around, the package can be loaded in order to make all symbols visible in the globalenv.

A similar issue will happen when linting regular files that source() other functions.

I'm actually confused why simply installing the linted package fixed the issue at all.

@MichaelChirico MichaelChirico added this to the 3.0.1 milestone Jul 27, 2022
@AshesITR AshesITR mentioned this issue Dec 5, 2022
14 tasks
@MichaelChirico MichaelChirico modified the milestones: 3.0.3, 3.1.0 Mar 20, 2023
@MichaelChirico MichaelChirico modified the milestones: 3.1.0, 3.1.1 Jun 30, 2023
MeWu-IDM added a commit to InstituteforDiseaseModeling/PACE-HRH that referenced this issue Aug 7, 2023
MeWu-IDM added a commit to InstituteforDiseaseModeling/PACE-HRH that referenced this issue Aug 7, 2023
* add link check

* add installation

* update linters based on custom style guide

* use .lintr as default settings

* fix issue

* fix update error

* workaround for object_usage_lintr

see r-lib/lintr#1137

* fix comma
@MichaelChirico
Copy link
Collaborator

I'm thinking the way to document this is with tags -- it only really applies to executing linters, so we should strive to reflect our understanding in ?executing_linters:

lintr/R/linter_tag_docs.R

Lines 93 to 101 in fa914d0

#' Code executing linters
#' @name executing_linters
#' @description
#' Linters that evaluate parts of the linted code, such as loading referenced packages.
#' These linters should not be used with untrusted code, and may need dependencies of the linted package or project to
#' be available in order to function correctly.
#' @evalRd rd_linters("executing")
#' @seealso [linters] for a complete list of linters available in lintr.
NULL

And maybe mention this class of linters in the README/intro vignette.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants