Skip to content

fix #432 - Object usage line numbers #440

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

Merged
merged 1 commit into from
Jan 17, 2020

Conversation

russHyde
Copy link
Collaborator

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.

- Modified object-usage-linter.R so that it reported the line-number from the original file, rather than the line-number of the encasing function

This led to several of the knitr-format tests failing because lints were throwing in a different order. The original tests were misordered , and have been modified.

Briefly:
Each of the knitr-format tests had code blocks that look like this:

~~~~
a = 1
~~~~

~~~~
b <- function(x) {
  d = 1
}

~~~~

The lints should be ordered:
- assignment (`=` for a)
- local variable ‘d’ assigned but may not be used
- assignment (`=` for d)
- trailing blank line

I updated the tests and added the explicit line-numbers where the lints
should throw to test-knitr_formats.R

@miker985
Copy link

miker985 commented Jan 6, 2020

I can confirm this fixes the reported issue in #432

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 russHyde force-pushed the object-usage-line-numbers branch from 83f8e85 to 4f8de87 Compare January 16, 2020 16:39
@russHyde
Copy link
Collaborator Author

russHyde commented Jan 16, 2020

I've added a NEWS entry and squashed down to a single commit. Would you be able to review this @jimhester so that I can look at #445 (which is a slightly different bug)

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

Successfully merging this pull request may close these issues.

3 participants