Equivalence with `lintr`

[etiennebacher]

https://lintr.r-lib.org/dev/reference/#individual-linters

• absolute_path_linter - Hard, see Absolute paths detection is broken #42
• any_duplicated_linter
• any_is_na_linter
• assignment_linter
• backport_linter
• boolean_arithmetic_linter
• brace_linter
• class_equals_linter
• commas_linter
• commented_code_linter - Not possible since this only appears as kind "comment" in tree-sitter. Potentially: Highlight ROxygen tags r-lib/tree-sitter-r#68
• comparison_negation_linter
• condition_call_linter
• condition_message_linter
• conjunct_test_linter
• consecutive_assertion_linter
• cyclocomp_linter
• duplicate_argument_linter
• empty_assignment_linter
• equals_na_linter
• expect_comparison_linter
• expect_identical_linter
• expect_length_linter
• expect_named_linter
• expect_not_linter
• expect_null_linter
• expect_s3_class_linter
• expect_s4_class_linter
• expect_true_false_linter
• expect_type_linter
• extraction_operator_linter - deprecated
• fixed_regex_linter
• for_loop_index_linter
• function_argument_linter
• function_left_parentheses_linter
• function_return_linter
• if_not_else_linter
• if_switch_linter
• ifelse_censor_linter Won't implement, I think it makes the code less readable
• implicit_assignment_linter
• implicit_integer_linter Won't implement, I think this is rarely useful and makes the code less readable
• indentation_linter
• infix_spaces_linter
• inner_combine_linter
• is_numeric_linter
• keyword_quote_linter
• length_levels_linter
• length_test_linter
• lengths_linter
• library_call_linter
• line_length_linter
• list_comparison_linter
• literal_coercion_linter
• matrix_apply_linter
• missing_argument_linter
• missing_package_linter won't do: this requires semantic analysis, can't be done with ast-grep
• namespace_linter won't do: this requires semantic analysis, can't be done with ast-grep
• nested_ifelse_linter
• nested_pipe_linter
• nonportable_path_linter
• nrow_subset_linter
• numeric_leading_zero_linter
• nzchar_linter
• object_length_linter
• object_name_linter
• object_overwrite_linter
• object_usage_linter won't do: this requires semantic analysis, can't be done with ast-grep
• one_call_pipe_linter
• outer_negation_linter
• package_hooks_linter
• paren_body_linter
• paste_linter
• pipe_call_linter
• pipe_consistency_linter
• pipe_continuation_linter
• pipe_return_linter
• print_linter
• quotes_linter
• redundant_equals_linter
• redundant_ifelse_linter
• regex_subset_linter
• rep_len_linter
• repeat_linter
• routine_registration_linter
• sample_int_linter
• scalar_in_linter
• semicolon_linter removed: this is related to formatting only and handled by air
• seq_linter
• sort_linter
• spaces_inside_linter
• spaces_left_parentheses_linter
• sprintf_linter
• stopifnot_all_linter
• string_boundary_linter
• strings_as_factors_linter
• system_file_linter
• T_and_F_symbol_linter
• terminal_close_linter
• todo_comment_linter
• trailing_blank_lines_linter
• trailing_whitespace_linter
• undesirable_function_linter
• undesirable_operator_linter
• unnecessary_concatenation_linter
• unnecessary_lambda_linter
• unnecessary_nesting_linter
• unnecessary_placeholder_linter
• unreachable_code_linter
• unused_import_linter won't do: this requires semantic analysis, can't be done with ast-grep
• vector_logic_linter
• which_grepl_linter
• whitespace_linter
• yoda_test_linter

[etiennebacher]

For unnecessary_nested_if_linter: https://ast-grep.github.io/playground.html#eyJtb2RlIjoiQ29uZmlnIiwibGFuZyI6ImphdmFzY3JpcHQiLCJxdWVyeSI6IiIsInJld3JpdGUiOiJsb2dnZXIubG9nKCRNQVRDSCkiLCJzdHJpY3RuZXNzIjoic21hcnQiLCJzZWxlY3RvciI6IiIsImNvbmZpZyI6InJ1bGU6XG4gIGtpbmQ6IGlmX3N0YXRlbWVudFxuICBoYXM6XG4gICAga2luZDogc3RhdGVtZW50X2Jsb2NrXG4gICAgaGFzOiAgXG4gICAgICBraW5kOiBpZl9zdGF0ZW1lbnRcbiAgICAgIHN0b3BCeTogbmVpZ2hib3JcbiAgICBub3Q6XG4gICAgICBoYXM6XG4gICAgICAgIG50aENoaWxkOiAyXG4iLCJzb3VyY2UiOiJpZiAoQSkge1xuICBpZiAoQikge1xuICAgIGNvbnNvbGUubG9nKCdtYXRjaGVkIGluIG1ldGF2YXIhJylcbiAgfVxufVxuXG5pZiAoQSkge1xuICBpZiAoQikge1xuICAgIGNvbnNvbGUubG9nKCdtYXRjaGVkIGluIG1ldGF2YXIhJylcbiAgfVxuICBsZXQgeCA9IDE7XG59XG5cbmlmIChBKSB7XG4gIGxldCB4ID0gMTtcbiAgaWYgKEIpIHtcbiAgICBjb25zb2xlLmxvZygnbWF0Y2hlZCBpbiBtZXRhdmFyIScpXG4gIH1cbn0ifQ==

[etiennebacher]

I think a linter shouldn't take care of whitespaces and other things that don't modify the AST but are "purely aesthetic", that's the role of a formatter (and I don't think ast-grep is the right tool for this). So I remove from the list all linters related to formatting.

[tanho63]

Looking to implement this for my org, so I'm trying to go through this and I think this is the state lintr support as of today? TBD = on the checklist above but not yet checked off, Unknown = not on checklist above. Is this correct?

linter	flir::list_linters()	lintr::default_linters	lintr::all_linters	flir support comments	note
absolute_path_linter			lintr	TBD - difficult
any_duplicated_linter	flir		lintr	Yes
any_is_na_linter	flir		lintr	Yes
assignment_linter		lintr_default	lintr	Yes	split into sub-linters: right, equal
backport_linter			lintr	TBD
boolean_arithmetic_linter			lintr	TBD
brace_linter		lintr_default	lintr	No - styling
class_equals_linter	flir		lintr	Yes
commas_linter		lintr_default	lintr	No - styling
commented_code_linter		lintr_default	lintr	No - not possible
comparison_negation_linter			lintr	Unknown
condition_call_linter			lintr	Unknown
condition_message_linter	flir		lintr	Yes
conjunct_test_linter			lintr	TBD
consecutive_assertion_linter			lintr	TBD
consecutive_mutate_linter			lintr	TBD
cyclocomp_linter			lintr	TBD
double_assignment_linter	flir			Yes
duplicate_argument_linter	flir		lintr	Yes
empty_assignment_linter	flir		lintr	Yes
equal_assignment_linter	flir	lintr_default		Yes
equals_na_linter	flir	lintr_default	lintr	Yes
expect_comparison_linter	flir		lintr	Yes
expect_identical_linter	flir		lintr	Yes
expect_length_linter	flir		lintr	Yes
expect_named_linter	flir		lintr	Yes
expect_not_linter	flir		lintr	Yes
expect_null_linter	flir		lintr	Yes
expect_s3_class_linter			lintr	Unknown
expect_s4_class_linter			lintr	Unknown
expect_true_false_linter	flir		lintr	Yes
expect_type_linter	flir		lintr	Yes
fixed_regex_linter			lintr	TBD
for_loop_index_linter	flir		lintr	Yes
function_argument_linter			lintr	TBD
function_left_parentheses_linter		lintr_default	lintr	No - styling
function_return_linter	flir		lintr	Yes
if_not_else_linter			lintr	TBD
if_switch_linter			lintr	Unknown
ifelse_censor_linter			lintr	No - author decision
implicit_assignment_linter	flir		lintr	Yes
implicit_integer_linter			lintr	No - author decision
indentation_linter		lintr_default	lintr	No - styling
infix_spaces_linter		lintr_default	lintr	No - styling
inner_combine_linter			lintr	TBD
is_numeric_linter	flir		lintr	Yes
keyword_quote_linter			lintr	TBD
length_levels_linter	flir		lintr	Yes
length_test_linter	flir		lintr	Yes
lengths_linter	flir		lintr	Yes
library_call_linter	flir		lintr	Yes
line_length_linter		lintr_default	lintr	No - styling
list_comparison_linter	flir		lintr	Yes
literal_coercion_linter	flir		lintr	Yes
matrix_apply_linter	flir		lintr	Yes
missing_argument_linter	flir		lintr	Yes
missing_package_linter			lintr	No - not possible
namespace_linter			lintr	No - not possible
nested_ifelse_linter	flir		lintr	Yes
nested_pipe_linter			lintr	Unknown
nonportable_path_linter			lintr	TBD
nrow_subset_linter			lintr	Unknown
numeric_leading_zero_linter	flir		lintr	Yes
nzchar_linter			lintr	Unknown
object_length_linter		lintr_default	lintr	TBD
object_name_linter		lintr_default	lintr	TBD
object_overwrite_linter			lintr	Unknown
object_usage_linter		lintr_default	lintr	No - not possible
one_call_pipe_linter			lintr	Unknown
outer_negation_linter	flir		lintr	Yes
package_hooks_linter	flir		lintr	Yes
paren_body_linter		lintr_default	lintr	No - styling
paste_linter	flir		lintr	Yes
pipe_call_linter			lintr	TBD
pipe_consistency_linter			lintr	TBD
pipe_continuation_linter		lintr_default	lintr	No - styling
pipe_return_linter			lintr	Unknown
print_linter			lintr	Unknown
quotes_linter		lintr_default	lintr	TBD
redundant_equals_linter	flir		lintr	Yes
redundant_ifelse_linter	flir		lintr	Yes
regex_subset_linter			lintr	TBD
rep_len_linter	flir		lintr	Yes
repeat_linter			lintr	TBD
return_linter		lintr_default	lintr	TBD
right_assignment_linter	flir	lintr_default		Yes
routine_registration_linter			lintr	TBD
sample_int_linter	flir		lintr	Yes
scalar_in_linter			lintr	TBD
semicolon_linter		lintr_default	lintr	No - styling
seq_linter	flir	lintr_default	lintr	Yes
sort_linter	flir		lintr	Yes
spaces_inside_linter		lintr_default	lintr	No - styling
spaces_left_parentheses_linter		lintr_default	lintr	No - styling
sprintf_linter			lintr	TBD
stopifnot_all_linter	flir		lintr	Yes
string_boundary_linter			lintr	TBD
strings_as_factors_linter			lintr	TBD
system_file_linter			lintr	TBD
t_and_f_symbol_linter	flir	lintr_default	lintr	Yes
terminal_close_linter			lintr	Unknown
todo_comment_linter	flir		lintr	Yes
trailing_blank_lines_linter		lintr_default	lintr	No - styling
trailing_whitespace_linter		lintr_default	lintr	No - styling
undesirable_function_linter	flir		lintr	Yes
undesirable_operator_linter	flir		lintr	Yes
unnecessary_concatenation_linter			lintr	TBD
unnecessary_lambda_linter			lintr	TBD
unnecessary_nesting_linter	flir		lintr	Yes
unnecessary_placeholder_linter			lintr	TBD
unreachable_code_linter			lintr	Yes	not a default / listed in list_linters?
unused_import_linter			lintr	No - not possible
vector_logic_linter		lintr_default	lintr	TBD
which_grepl_linter	flir		lintr	Yes
whitespace_linter		lintr_default	lintr	No - styling
yoda_test_linter			lintr	TBD

[etiennebacher]

Hi @tanho63, thank you, that's super helpful!

I have added the "Unknown" ones to the checklist, they must have been added after I created the initial list about one year ago. I must say I didn't pay much attention to the difference between all_linters() and default_linters(), as I prefer including all linters by default. I think it's more natural for a casual user to think "this linter is annoying, there must be a way to disable it" rather than "let me explore the hidden linters provided by the tool".

I'm unlikely to implement the new linters in the next weeks (except maybe trivial ones) as I have other open source projects that take most of my time, but feel free to open a PR (even an incomplete one) if you want to take a shot and need some guidance or review (see the contributing guide too).

[tanho63]

Ha, I’m doing this for work so my plan was mostly to identify what we already had and reimplement, rather than adding a whole bunch of new linters to annoy my colleagues more 😆 the fix_lints is going to be great!

The one thing I’d want that this doesn’t have (and that I’d chip in on) is no browser() in committed code

[etiennebacher]

The one thing I’d want that this doesn’t have (and that I’d chip in on) is no browser() in committed code

Yeah that's just a regular lint, no fix for that. Part of the reason is stylistic. If you have some code like that:

x <- mean(c(1, 2, 3))

browser()

print("hi")

then you'd want the automatic fix to remove the call and reduce the whitespace gap, which cannot be done with flir.

[etiennebacher]

At the same time, if you still want to automatically remove the call and don't care about the white space, then you can add a custom rule: https://flir.etiennebacher.com/articles/adding_rules

[tanho63]

Ah hm I think I can just lint before whacking it with air format. Thanks, I’ll give it a whirl

[tanho63]

Just confirming this works fine for me, I'll push a sample of my setup to a git repo somewhere to demonstrate flir + air precommit in a docker image

[tanho63]

pushed here: https://github.com/tanho63/precommit-r

(I think I did the flir stuff right but would welcome suggestions!)

[etiennebacher]

Thanks for sharing! The rule for browser() looks good. I don't know if the rule for todo_ticket matches all the cases you have in mind, but you can experiment with lint_text() (if you haven't already).