r-lib / lintr

Static Code Analysis for R
https://lintr.r-lib.org
Other
1.18k stars 184 forks source link

All TODO should refer to GitHub issues #2450

Closed MichaelChirico closed 7 months ago

MichaelChirico commented 8 months ago

For now we use a lot of e.g. TODO(michaelchirico), it's better to track an issue instead.

After fixing, remove user names as a valid option for todo_regex_linter() in our .lintr.

MichaelChirico commented 8 months ago

To fix:

                                            filename line_number                                                                                               line
 1:                    R/boolean_arithmetic_linter.R          66                                                            # TODO(michaelchirico): customize this?
 2:                         R/expect_length_linter.R          24                                     # TODO(michaelchirico): also catch expect_true(length(x) == 1)
 3:                       R/get_source_expressions.R         644                              # TODO(michaelchirico): vectorize this loop away. the tricky part is,
 4:                           R/indentation_linter.R         295                                    # TODO(AshesITR) when updating supported R version to R >= 4.1:
 5:                         R/inner_combine_linter.R          39                # TODO(michaelchirico): the need to spell out specific arguments is pretty brittle,
 6:                            R/is_numeric_linter.R          37                       # TODO(michaelchirico): this should also cover is.double(x) || is.integer(x)
 7:                            R/is_numeric_linter.R          39                      # TODO(michaelchirico): consdier capturing any(class(x) == "numeric/integer")
 8:                            R/is_numeric_linter.R          41                            # TODO(michaelchirico): also catch inherits(x, c("numeric", "integer"))
 9:                            R/is_numeric_linter.R          58                              # TODO(michaelchirico): include typeof(x) %in% c("integer", "double")
10:                         R/keyword_quote_linter.R          49                                      # TODO(michaelchirico): offer a stricter version of this that
12:                       R/list_comparison_linter.R          24                       # TODO(michaelchirico): extend to cases where using simplify=FALSE implies a
13:                      R/literal_coercion_linter.R         101                    # TODO(michaelchirico): this recommends '1' to replace as.numeric(1), where our
14:                          R/object_usage_linter.R         212                  # TODO(AshesITR): Remove this in the future, if no bugs arise from this safeguard
15:                             R/shared_constants.R         260     # TODO(michaelchirico): consider dropping tryCatch() here if we're more confident in our logic
16:             R/unnecessary_concatenation_linter.R         107         # TODO(michaelchirico): can we handle this all inside the XPath with reasonable concision?
17:                    R/unnecessary_lambda_linter.R         163                            # TODO(michaelchirico): further message customization is possible here,
18:               R/unnecessary_placeholder_linter.R          36                                 # TODO(michaelchirico): handle R4.2.0 native placeholder _ as well
19:                         R/unused_import_linter.R         112                                      # TODO(michaelchirico): instead of vectorizing over packages,
21:      tests/testthat/test-any_duplicated_linter.R          53                   # TODO(michaelchirico): try and match data.table- and dplyr-specific versions of
22:     tests/testthat/test-expect_s3_class_linter.R          64                 # TODO(michaelchirico): consider more carefully which sorts of class(x) %in% . and
23:         tests/testthat/test-fixed_regex_linter.R         273                      # TODO(michaelchirico): one difference for stringr functions vs. base is that
24:         tests/testthat/test-fixed_regex_linter.R         281                     # TODO(michaelchirico): we could in principle build in logic to detect whether
25:       tests/testthat/test-ifelse_censor_linter.R          85                # TODO(michaelchirico): how easy would it be to strip parens when considering lint?
26:         tests/testthat/test-if_not_else_linter.R          87                                  # TODO(michaelchirico): should if (A != B) be considered as well?
27:       tests/testthat/test-inner_combine_linter.R          92                               # TODO(michaelchirico): fix the code so these edge cases are covered
28:              tests/testthat/test-knitr_formats.R          97                                                                                  # TODO(AshesITR):
29:              tests/testthat/test-knitr_formats.R         105                                                                            # TODO(AshesITR): #1043
30:              tests/testthat/test-nzchar_linter.R          20                             # TODO(michaelchirico): check compatibility of nchar(., allowNA=TRUE).
31:       tests/testthat/test-one_call_pipe_linter.R          25                   # TODO(michaelchirico): actually, this should lint twice -- we're too aggressive
32:  tests/testthat/test-unnecessary_lambda_linter.R         170                    # TODO(michaelchirico): this is just purrr::flatten(x). We should write another
33: tests/testthat/test-unnecessary_nesting_linter.R          30                     # TODO(michaelchirico): consider if there's a nice easy pattern to enforce for
34:    tests/testthat/test-unreachable_code_linter.R         725                                      # TODO(michaelchirico): extend to work on switch() statements
35:    tests/testthat/test-unreachable_code_linter.R         748                                     # TODO(michaelchirico): This could also apply to cases without
36:           tests/testthat/test-yoda_test_linter.R          65                      # TODO(michaelchirico): Should this be extended to RUnit tests? It seems yes,
37:           tests/testthat/test-yoda_test_linter.R          69                    # TODO(michaelchirico): What sorts of combinations of literals can be included?
38:           tests/testthat/test-yoda_test_linter.R          73                 # TODO(michaelchirico): The logic could also be extended to "tests" inside regular
                                            filename line_number                                                                                               line
MichaelChirico commented 8 months ago

(apologies for the inbox spam today 🙃)