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

extend available_linters(tags = ), add linters_with_tags() #1015

Merged
merged 23 commits into from
Mar 30, 2022

Conversation

AshesITR
Copy link
Collaborator

Fixes #980

I moved the with_defaults() test from test-settings.R to test-with.R and moved the implementation from zzz.R to with.R.

with_tags() also allows us to test that default_linters is synchronized to available_linters(tags = "default") so I did that.

@AshesITR AshesITR requested a review from MichaelChirico March 28, 2022 20:30
@AshesITR
Copy link
Collaborator Author

Maybe adding a crossref from available_linters() to with_tags() is a good idea?
And maybe also from ?linters to help discover the new feature.
Since with_tags() can be used as a drop-in replacement for with_defaults() as long as default = is missing, I think with_tags() could even be documented to become the standard way of configuring lintr. WDYT?

@AshesITR
Copy link
Collaborator Author

Not sure what is up with codecov() erring out for all.equal() when ubuntu-release doesn't and both run R 4.1.3...

@AshesITR
Copy link
Collaborator Author

AshesITR commented Mar 28, 2022

2022-03-28T20:45:06.8497300Z ##[error]Error: Failure in `/tmp/RtmpjHIBqn/R_LIBS2488420bd9ed/lintr/lintr-tests/testthat.Rout.fail`
2022-03-28T20:45:06.8506140Z ol_linter\": target, current do not match when deparsed"
2022-03-28T20:45:06.8506879Z - "Component \"trailing_blank_lines_linter\": target, current do not match when deparsed"
2022-03-28T20:45:06.8507591Z - "Component \"trailing_whitespace_linter\": target, current do not match when deparsed"
2022-03-28T20:45:06.8508204Z - "Component \"vector_logic_linter\": target, current do not match when deparsed"
2022-03-28T20:45:06.8508735Z + "Component \"line_length_linter\": Component \"length\": Mean relative difference: 0.5"

Sounds like the all.equal() check in codecov compares deparse(fun)...

Locally irreproducible:

f1 <- with_tags()[["vector_logic_linter"]]
f2 <- with_defaults()[["vector_logic_linter"]]
all.equal(f1, f2)
#> [1] TRUE
all.equal(deparse(f1), deparse(f2))
#> [1] TRUE

EDIT: Found the problem. covr::report() modifies functions to count line hits. These modifications cause inconsistent deparse() results in our case it seems.

@MichaelChirico
Copy link
Collaborator

EDIT: Found the problem. covr::report() modifies functions to count line hits. These modifications cause inconsistent deparse() results in our case it seems.

seems like a covr bug... will that block submission here?

@AshesITR
Copy link
Collaborator Author

Nope. Added a skip nevertheless.

NEWS.md Outdated Show resolved Hide resolved
@AshesITR AshesITR changed the title extend available_linters(tags = ), add with_tags() extend available_linters(tags = ), add linters_with_tags() Mar 29, 2022
NEWS.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@MichaelChirico MichaelChirico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting closer, thanks!

R/with.R Outdated Show resolved Hide resolved
R/with.R Outdated Show resolved Hide resolved
R/with.R Outdated Show resolved Hide resolved
R/with.R Show resolved Hide resolved
R/with.R Outdated Show resolved Hide resolved
R/with.R Outdated Show resolved Hide resolved
tests/testthat/test-with.R Show resolved Hide resolved
@AshesITR
Copy link
Collaborator Author

AshesITR commented Mar 30, 2022

PR fails seem to originate elsewhere.
Also, the test warnings() are back it seems...

https://github.com/r-lib/lintr/runs/5749209064?check_suite_focus=true#step:6:177

EDIT: 🤦🏻 calling the internal function build_linter was the culprit.

@MichaelChirico MichaelChirico merged commit 036a606 into master Mar 30, 2022
@MichaelChirico MichaelChirico deleted the feature/available_linters-tags branch March 30, 2022 21:55
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.

tags= argument for available_linters()
2 participants