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

Some warnings have wrong line numbers #432

Closed
miker985 opened this issue Nov 19, 2019 · 4 comments
Closed

Some warnings have wrong line numbers #432

miker985 opened this issue Nov 19, 2019 · 4 comments
Labels
bug an unexpected problem or unintended behavior

Comments

@miker985
Copy link

Issue: lintr is reporting a valid issue on the wrong line number.

Example file:

#' A function with no issues.
#'
#' @export
foo <- function(capitalize = FALSE) {
  if (capitalize) {
    "Foo"
  } else {
    "foo"
  }
}


#' Some other function with issues
bar <- function() {
  root <- library_root()
}

Output

R -e 'library(lintr); lint("test.R"); sessionInfo()'

R version 3.6.1 (2019-07-05) -- "Action of the Toes"
Copyright (C) 2019 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> library(lintr); lint("test.R"); sessionInfo()
/home/miker985/code/lbd.loader/test.R:2:3: warning: local variable ‘root’ assigned but may not be used
  root <- library_root()
  ^~~~
/home/miker985/code/lbd.loader/test.R:2:11: warning: no visible global function definition for ‘library_root’
  root <- library_root()
          ^~~~~~~~~~~~
R version 3.6.1 (2019-07-05)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Debian GNU/Linux 10 (buster)

Matrix products: default
BLAS/LAPACK: /opt/intel/compilers_and_libraries_2019.4.243/linux/mkl/lib/intel64_lin/libmkl_gf_lp64.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C
 [9] LC_ADDRESS=C               LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] lintr_2.0.0

loaded via a namespace (and not attached):
 [1] Rcpp_1.0.1         codetools_0.2-16   ps_1.3.0           crayon_1.3.4
 [5] withr_2.1.2        rprojroot_1.3-2    assertthat_0.2.1   R6_2.4.0
 [9] backports_1.1.4    magrittr_1.5       stringi_1.4.3      lazyeval_0.2.2
[13] rstudioapi_0.10    remotes_2.1.0      callr_3.3.0        rex_1.1.2
[17] xml2_1.2.0         cyclocomp_1.1.0    desc_1.2.0         tools_3.6.1
[21] stringr_1.4.0      xfun_0.8           compiler_3.6.1     processx_3.4.0
[25] xmlparsedata_1.0.3 knitr_1.23
>
>
  • /home/miker985/code/lbd.loader/test.R:2:3: warning: local variable ‘root’ assigned but may not be used
  • /home/miker985/code/lbd.loader/test.R:2:11: warning: no visible global function definition for ‘library_root’

Neither of these issues occur on line 2 of the file. They do occur on line 2 of the function, but that needs to be translated back to the overall file number.

# nolint can be used to squelch, but only if it's placed on line 2.

@russHyde
Copy link
Collaborator

Yes I've noticed this when running lintr in vim/syntastic. It looks like the code-lines are mangled in object_usage_linter

russHyde added a commit to russHyde/lintr that referenced this issue Dec 16, 2019
In r-lib#432 it was noted that the line-number for object-usage-linter
warnings were often incorrect. They were being reported relative to the
start of the defining function, rather than the start of the file.

A test was added that throws an object-usage-lint on line-2 of the
function but line-5 of the file.
@russHyde russHyde added the bug an unexpected problem or unintended behavior label Jan 6, 2020
@russHyde
Copy link
Collaborator

russHyde commented Jan 6, 2020

@miker985 I think I've fixed this in #440 . Would you be happy to review the pull request?

@miker985
Copy link
Author

miker985 commented Jan 6, 2020

Awesome! Thank you @russHyde

I've downloaded your fork and manually installed/tested. It now works as expected.

R -e 'lintr::lint("test.R");'


R version 3.6.1 (2019-07-05) -- "Action of the Toes"
Copyright (C) 2019 The R Foundation for Statistical Computing
Platform: x86_64-pc-linux-gnu (64-bit)

R is free software and comes with ABSOLUTELY NO WARRANTY.
You are welcome to redistribute it under certain conditions.
Type 'license()' or 'licence()' for distribution details.

  Natural language support but running in an English locale

R is a collaborative project with many contributors.
Type 'contributors()' for more information and
'citation()' on how to cite R or R packages in publications.

Type 'demo()' for some demos, 'help()' for on-line help, or
'help.start()' for an HTML browser interface to help.
Type 'q()' to quit R.

> lintr::lint("test.R");
/home/miker985/tmp/lintr-issue-432/test.R:15:3: warning: local variable ‘root’ assigned but may not be used
  root <- library_root()
  ^~~~
/home/miker985/tmp/lintr-issue-432/test.R:15:11: warning: no visible global function definition for ‘library_root’
  root <- library_root()
          ^~~~~~~~~~~~
> 
> 

I've left a positive comment on the PR. Let me know if there's something else I can do.

russHyde added a commit to russHyde/lintr that referenced this issue Jan 16, 2020
In r-lib#432 it was noted that the line-number for object-usage-linter
warnings were often incorrect. They were being reported relative to the
start of the defining function, rather than the start of the file.

A test was added that throws an object-usage-lint on line-2 of the
function but line-5 of the file.

Also,

- any pre-existing knitr-format tests were expanded so that the observed
lints occurred on an explicit `line_number`

- updated NEWS.md
russHyde added a commit that referenced this issue Jan 17, 2020
In #432 it was noted that the line-number for object-usage-linter
warnings were often incorrect. They were being reported relative to the
start of the defining function, rather than the start of the file.

A test was added that throws an object-usage-lint on line-2 of the
function but line-5 of the file.

Also,

- any pre-existing knitr-format tests were expanded so that the observed
lints occurred on an explicit `line_number`

- updated NEWS.md
@russHyde
Copy link
Collaborator

Fixed in #440 . Closing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

2 participants