feat: argument to disable linting of ( for auto-printing
#2919
Conversation
| @@ -48,6 +48,7 @@ | |||
| * `object_usage_linter()` lints missing packages that may cause false positives (#2872, @AshesITR) | |||
| * New argument `include_s4_slots` for the `xml_find_function_calls()` entry in the `get_source_expressions()` to govern whether calls of the form `s4Obj@fun()` are included in the result (#2820, @MichaelChirico). | |||
| * `sprintf_linter()` lints `sprintf()` and `gettextf()` calls when a constant string is passed to `fmt` (#2894, @Bisaloo). | |||
| * `implicit_assignment_linter()` gains argument `allow_print` to disable lints for the use of `(` for auto-printing (#2919, @TimTaylor). | |||
There was a problem hiding this comment.
I think allow_implicit_print may be better, WDYT? My only hesitation is doubling "implicit" in the linter & argument names...
There was a problem hiding this comment.
allow_bracket_print?
I'm already wary of implicit as a description as it already doesn't reflect how I think of my usage of this lint ... basically "stop people accidentally using <- in function calls". I'd prefer not to use the term more if possible.
| @@ -116,6 +130,9 @@ implicit_assignment_linter <- function(except = c("bquote", "expression", "expr" | |||
| bad_expr <- xml_find_all(xml, xpath) | |||
|
|
|||
| print_only <- !is.na(xml_find_first(bad_expr, "parent::expr[parent::exprlist and *[1][self::OP-LEFT-PAREN]]")) | |||
| if (allow_print) { | |||
| bad_expr <- bad_expr[!print_only] | |||
There was a problem hiding this comment.
This works, I think it will be ideal to work this in to the up-front XPath logic, rather than including those hits in the search only to drop them.
Have a try and if you get stuck we can mark it as a follow-up issue.
There was a problem hiding this comment.
I thought that initially but then I realised you need to do the print_only for the error message anyway so this seemed to be the most KISS approach. WDY?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2919 +/- ##
=======================================
Coverage 99.28% 99.28%
=======================================
Files 129 129
Lines 7266 7269 +3
=======================================
+ Hits 7214 7217 +3
Misses 52 52 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Feels like a false positive to me - maybe we don't need an argument for this? IINM this only removes top-level |
That would be great but the custom print error message seems like it was a deliberate choice to lint for.
I believe so (but take my agreement with caution as I deliberately avoided delving in to the XPath logic due to the pre-existing custom error). |
|
Oh, I see, sorry. |
Adds an argument to
implicit_assignment_linterto disable linting of(for auto-printing.closes #2916