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

Further improvements for lengths_linter() #1749

Merged
merged 9 commits into from
Oct 27, 2022

Conversation

IndrajeetPatil
Copy link
Collaborator

@IndrajeetPatil IndrajeetPatil commented Oct 23, 2022

Closes #1573

library(lintr)

lint(
  text = "sapply(FUN = length, x)",
  linters = lengths_linter()
)
#> <text>:1:1: warning: [lengths_linter] Use lengths() to find the length of each element in a list.
#> sapply(FUN = length, x)
#> ^~~~~~~~~~~~~~~~~~~~~~~

lint(
  text = "x |> sapply(length)",
  linters = lengths_linter()
)
#> <text>:1:6: warning: [lengths_linter] Use lengths() to find the length of each element in a list.
#> x |> sapply(length)
#>      ^~~~~~~~~~~~~~

lint(
  text = "map_int(.f = length, x)",
  linters = lengths_linter()
)
#> <text>:1:1: warning: [lengths_linter] Use lengths() to find the length of each element in a list.
#> map_int(.f = length, x)
#> ^~~~~~~~~~~~~~~~~~~~~~~

lint(
  text = "x |> map_int(length)",
  linters = lengths_linter()
)
#> <text>:1:6: warning: [lengths_linter] Use lengths() to find the length of each element in a list.
#> x |> map_int(length)
#>      ^~~~~~~~~~~~~~~

Created on 2022-10-23 with reprex v2.0.2

@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2022

Codecov Report

Merging #1749 (0418e50) into main (15a9578) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 0418e50 differs from pull request most recent head f999174. Consider uploading reports for the commit f999174 to get more accurate results

@@           Coverage Diff           @@
##             main    #1749   +/-   ##
=======================================
  Coverage   98.87%   98.87%           
=======================================
  Files         109      109           
  Lines        4620     4621    +1     
=======================================
+ Hits         4568     4569    +1     
  Misses         52       52           
Impacted Files Coverage Δ
R/lengths_linter.R 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@AshesITR
Copy link
Collaborator

Ca we test that vapply(x, fun, integer(length(y))) is allowed?

@IndrajeetPatil
Copy link
Collaborator Author

Can we test that vapply(x, fun, integer(length(y))) is allowed?

@AshesITR Added a test. It is currently allowed.

@AshesITR
Copy link
Collaborator

LGTM. @MichaelChirico PTAL, I remember we discussed about position() = 3 in your original PR.

@MichaelChirico
Copy link
Collaborator

I think in this case it's OK; this bug was filed as follow-up to that comment:

#1568 (comment)

@IndrajeetPatil IndrajeetPatil merged commit 8a12bf0 into main Oct 27, 2022
@IndrajeetPatil IndrajeetPatil deleted the 1573_lengths_linter_improv branch October 27, 2022 07:01
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.

Further improvements for lengths_linter()
4 participants