Skip to content

Parameterize if_switch_linter() not to require switch() for "highly-complicated" clauses #2322

Description

@MichaelChirico

if_switch_linter() is too strict -- in some cases,using switch() is actually quite awkward.

if (x == 'a') {
  do_1()
  do_2()
  do_3()
  if (y > 2) {
    do_4()
    do_5()
  }
} else if (x == 'b') {
  do_6()
  do_7()
  do_8()
  do_9()
  do_10()
  for (i in 1:10) {
    do_11()
    do_12(0
  }
} else if (x == 'c') {
  do_13()
  do_14()
  do_15()
  do_16()
  do_17()
} else if (x == 'd') {
  do_18()
  do_19()
  do_20()
  do_21()
  do_22()
}

Would become

switch(x,
  a = {
    do_1()
    do_2()
    do_3()
    if (y > 2) {
      do_4()
      do_5()
    }
  },
  b = {
    do_6()
    do_7()
    do_8()
    do_9()
    do_10()
    for (i in 1:10) {
      do_11()
      do_12(0
    }
  },
  c = {
    do_13()
    do_14()
    do_15()
    do_16()
    do_17()
  },
  d = {
    do_18()
    do_19()
    do_20()
    do_21()
    do_22()
  }
)

Note the increased nesting, and the distance between switch(x and the x value b = {.

It would be good to come up with a rule to weaken if_switch_linter() to not apply to cases where the branches are "overly complicated". The trouble is coming up with a good rule for what that means (e.g. cyclocomp_linter() only cares about branching, not the number of expressions).

One simple option is to just count the number of "child expressions" in each branch, and not lint if, say max(expressions) > complexity where complexity= is an argument set by the user, perhaps with a sensible default like 3L.

Metadata

Metadata

Assignees

No one assigned

    Labels

    featurea feature request or enhancement

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions